[PATCH v2 0/1] x86/kvm/hyper-v: Add support to SYNIC exit on EOM

[PATCH v2 0/1] x86/kvm/hyper-v: Add support to SYNIC exit on EOM

[Jon Doron]

According to the TLFS:
"A write to the end of message (EOM) register by the guest causes the
hypervisor to scan the internal message buffer queue(s) associated with
the virtual processor.

If a message buffer queue contains a queued message buffer, the hypervisor
attempts to deliver the message.

Message delivery succeeds if the SIM page is enabled and the message slot
corresponding to the SINTx is empty (that is, the message type in the
header is set to HvMessageTypeNone).
If a message is successfully delivered, its corresponding internal message
buffer is dequeued and marked free.
If the corresponding SINTx is not masked, an edge-triggered interrupt is
delivered (that is, the corresponding bit in the IRR is set).

This register can be used by guests to poll for messages. It can also be
used as a way to drain the message queue for a SINTx that has
been disabled (that is, masked)."

So basically this means that we need to exit on EOM so the hypervisor
will have a chance to send all the pending messages regardless of the
SCONTROL mechnaisim.

v2:
Minor fixes from code review

Jon Doron (1):
  x86/kvm/hyper-v: Add support to SYNIC exit on EOM

 arch/x86/include/asm/kvm_host.h |  1 +
 arch/x86/kvm/hyperv.c           | 67 +++++++++++++++++++++++++++++----
 arch/x86/kvm/hyperv.h           |  1 +
 arch/x86/kvm/x86.c              |  5 +++
 include/uapi/linux/kvm.h        |  1 +
 5 files changed, 67 insertions(+), 8 deletions(-)

-- 
2.24.1

[PATCH v2 1/1] x86/kvm/hyper-v: Add support to SYNIC exit on EOM

[Jon Doron]

According to the TLFS a write to the EOM register by the guest
causes the hypervisor to scan for any pending messages and if there
are any it will try to deliver them.

To do this we must exit so any pending messages can be written.

Signed-off-by: Jon Doron <arilou@gmail.com>
---
 arch/x86/include/asm/kvm_host.h |  1 +
 arch/x86/kvm/hyperv.c           | 67 +++++++++++++++++++++++++++++----
 arch/x86/kvm/hyperv.h           |  1 +
 arch/x86/kvm/x86.c              |  5 +++
 include/uapi/linux/kvm.h        |  1 +
 5 files changed, 67 insertions(+), 8 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 42a2d0d3984a..048a1db488e2 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -548,6 +548,7 @@ struct kvm_vcpu_hv_synic {
 	DECLARE_BITMAP(vec_bitmap, 256);
 	bool active;
 	bool dont_zero_synic_pages;
+	bool enable_eom_exit;
 };
 
 /* Hyper-V per vcpu emulation context */
diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index bcefa9d4e57e..4bf35452ae5c 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -186,6 +186,51 @@ static void kvm_hv_notify_acked_sint(struct kvm_vcpu *vcpu, u32 sint)
 	srcu_read_unlock(&kvm->irq_srcu, idx);
 }
 
+static int synic_read_msg_hdr(struct kvm_vcpu_hv_synic *synic, u32 sint,
+			      struct hv_message_header *msg)
+{
+	struct kvm_vcpu *vcpu = synic_to_vcpu(synic);
+	int msg_off = offsetof(struct hv_message_page, sint_message[sint]);
+	gfn_t msg_page_gfn;
+	int r;
+
+	if (!(synic->msg_page & HV_SYNIC_SIMP_ENABLE))
+		return -ENOENT;
+
+	msg_page_gfn = synic->msg_page >> PAGE_SHIFT;
+
+	r = kvm_vcpu_read_guest_page(vcpu, msg_page_gfn, msg, msg_off,
+				     sizeof(*msg));
+	if (r < 0)
+		return r;
+
+	return 0;
+}
+
+static bool synic_should_exit_on_eom(struct kvm_vcpu_hv_synic *synic)
+{
+	int i;
+
+	if (!synic->enable_eom_exit)
+		return false;
+
+	for (i = 0; i < ARRAY_SIZE(synic->sint); i++) {
+		struct hv_message_header hv_hdr;
+		/*
+		 * If we failed to read from the msg slot then we treat this
+		 * msg slot as free
+		 */
+		if (synic_read_msg_hdr(synic, i, &hv_hdr) < 0)
+			continue;
+
+		/* See if this msg slot has a pending message */
+		if (hv_hdr.message_flags.msg_pending == 1)
+			return true;
+	}
+
+	return false;
+}
+
 static void synic_exit(struct kvm_vcpu_hv_synic *synic, u32 msr)
 {
 	struct kvm_vcpu *vcpu = synic_to_vcpu(synic);
@@ -254,6 +299,9 @@ static int synic_set_msr(struct kvm_vcpu_hv_synic *synic,
 
 		for (i = 0; i < ARRAY_SIZE(synic->sint); i++)
 			kvm_hv_notify_acked_sint(vcpu, i);
+
+		if (!host && synic_should_exit_on_eom(synic))
+			synic_exit(synic, msr);
 		break;
 	}
 	case HV_X64_MSR_SINT0 ... HV_X64_MSR_SINT15:
@@ -571,8 +619,9 @@ static int synic_deliver_msg(struct kvm_vcpu_hv_synic *synic, u32 sint,
 	struct hv_message_header hv_hdr;
 	int r;
 
-	if (!(synic->msg_page & HV_SYNIC_SIMP_ENABLE))
-		return -ENOENT;
+	r = synic_read_msg_hdr(synic, sint, &hv_hdr);
+	if (r < 0)
+		return r;
 
 	msg_page_gfn = synic->msg_page >> PAGE_SHIFT;
 
@@ -582,12 +631,6 @@ static int synic_deliver_msg(struct kvm_vcpu_hv_synic *synic, u32 sint,
 	 * is only called in vcpu context so the entire update is atomic from
 	 * guest POV and thus the exact order here doesn't matter.
 	 */
-	r = kvm_vcpu_read_guest_page(vcpu, msg_page_gfn, &hv_hdr.message_type,
-				     msg_off + offsetof(struct hv_message,
-							header.message_type),
-				     sizeof(hv_hdr.message_type));
-	if (r < 0)
-		return r;
 
 	if (hv_hdr.message_type != HVMSG_NONE) {
 		if (no_retry)
@@ -785,6 +828,14 @@ int kvm_hv_activate_synic(struct kvm_vcpu *vcpu, bool dont_zero_synic_pages)
 	return 0;
 }
 
+int kvm_hv_synic_enable_eom(struct kvm_vcpu *vcpu)
+{
+	struct kvm_vcpu_hv_synic *synic = vcpu_to_synic(vcpu);
+
+	synic->enable_eom_exit = true;
+	return 0;
+}
+
 static bool kvm_hv_msr_partition_wide(u32 msr)
 {
 	bool r = false;
diff --git a/arch/x86/kvm/hyperv.h b/arch/x86/kvm/hyperv.h
index 757cb578101c..ff89f0ff103c 100644
--- a/arch/x86/kvm/hyperv.h
+++ b/arch/x86/kvm/hyperv.h
@@ -56,6 +56,7 @@ void kvm_hv_irq_routing_update(struct kvm *kvm);
 int kvm_hv_synic_set_irq(struct kvm *kvm, u32 vcpu_id, u32 sint);
 void kvm_hv_synic_send_eoi(struct kvm_vcpu *vcpu, int vector);
 int kvm_hv_activate_synic(struct kvm_vcpu *vcpu, bool dont_zero_synic_pages);
+int kvm_hv_synic_enable_eom(struct kvm_vcpu *vcpu);
 
 void kvm_hv_vcpu_init(struct kvm_vcpu *vcpu);
 void kvm_hv_vcpu_postcreate(struct kvm_vcpu *vcpu);
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 3bf2ecafd027..1615be238806 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3350,6 +3350,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
 	case KVM_CAP_HYPERV_SPIN:
 	case KVM_CAP_HYPERV_SYNIC:
 	case KVM_CAP_HYPERV_SYNIC2:
+	case KVM_CAP_HYPERV_SYNIC_EOM:
 	case KVM_CAP_HYPERV_VP_INDEX:
 	case KVM_CAP_HYPERV_EVENTFD:
 	case KVM_CAP_HYPERV_TLBFLUSH:
@@ -4209,6 +4210,10 @@ static int kvm_vcpu_ioctl_enable_cap(struct kvm_vcpu *vcpu,
 		return -EINVAL;
 
 	switch (cap->cap) {
+	case KVM_CAP_HYPERV_SYNIC_EOM:
+		kvm_hv_synic_enable_eom(vcpu);
+		return 0;
+
 	case KVM_CAP_HYPERV_SYNIC2:
 		if (cap->args[0])
 			return -EINVAL;
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 428c7dde6b4b..78172ad156d8 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -1017,6 +1017,7 @@ struct kvm_ppc_resize_hpt {
 #define KVM_CAP_S390_VCPU_RESETS 179
 #define KVM_CAP_S390_PROTECTED 180
 #define KVM_CAP_PPC_SECURE_GUEST 181
+#define KVM_CAP_HYPERV_SYNIC_EOM 182
 
 #ifdef KVM_CAP_IRQ_ROUTING
 
-- 
2.24.1

Re: [PATCH v2 0/1] x86/kvm/hyper-v: Add support to SYNIC exit on EOM

[Roman Kagan]

On Thu, Apr 16, 2020 at 11:38:46AM +0300, Jon Doron wrote:
> According to the TLFS:
> "A write to the end of message (EOM) register by the guest causes the
> hypervisor to scan the internal message buffer queue(s) associated with
> the virtual processor.
> 
> If a message buffer queue contains a queued message buffer, the hypervisor
> attempts to deliver the message.
> 
> Message delivery succeeds if the SIM page is enabled and the message slot
> corresponding to the SINTx is empty (that is, the message type in the
> header is set to HvMessageTypeNone).
> If a message is successfully delivered, its corresponding internal message
> buffer is dequeued and marked free.
> If the corresponding SINTx is not masked, an edge-triggered interrupt is
> delivered (that is, the corresponding bit in the IRR is set).
> 
> This register can be used by guests to poll for messages. It can also be
> used as a way to drain the message queue for a SINTx that has
> been disabled (that is, masked)."

Doesn't this work already?

> So basically this means that we need to exit on EOM so the hypervisor
> will have a chance to send all the pending messages regardless of the
> SCONTROL mechnaisim.

I might be misinterpreting the spec, but my understanding is that
SCONTROL {en,dis}ables the message queueing completely.  What the quoted
part means is that a write to EOM should trigger the message source to
push a new message into the slot, regardless of whether the SINT was
masked or not.

And this (I think, haven't tested) should already work.  The userspace
just keeps using the SINT route as it normally does, posting
notifications to the corresponding irqfd when posting a message, and
waiting on the resamplerfd for the message slot to become free.  If the
SINT is masked KVM will skip injecting the interrupt, that's it.

Roman.

Re: [PATCH v2 0/1] x86/kvm/hyper-v: Add support to SYNIC exit on EOM

[Jon Doron]

On 16/04/2020, Roman Kagan wrote:
>On Thu, Apr 16, 2020 at 11:38:46AM +0300, Jon Doron wrote:
>> According to the TLFS:
>> "A write to the end of message (EOM) register by the guest causes the
>> hypervisor to scan the internal message buffer queue(s) associated with
>> the virtual processor.
>>
>> If a message buffer queue contains a queued message buffer, the hypervisor
>> attempts to deliver the message.
>>
>> Message delivery succeeds if the SIM page is enabled and the message slot
>> corresponding to the SINTx is empty (that is, the message type in the
>> header is set to HvMessageTypeNone).
>> If a message is successfully delivered, its corresponding internal message
>> buffer is dequeued and marked free.
>> If the corresponding SINTx is not masked, an edge-triggered interrupt is
>> delivered (that is, the corresponding bit in the IRR is set).
>>
>> This register can be used by guests to poll for messages. It can also be
>> used as a way to drain the message queue for a SINTx that has
>> been disabled (that is, masked)."
>
>Doesn't this work already?
>

Well if you dont have SCONTROL and a GSI associated with the SINT then 
it does not...

>> So basically this means that we need to exit on EOM so the hypervisor
>> will have a chance to send all the pending messages regardless of the
>> SCONTROL mechnaisim.
>
>I might be misinterpreting the spec, but my understanding is that
>SCONTROL {en,dis}ables the message queueing completely.  What the quoted
>part means is that a write to EOM should trigger the message source to
>push a new message into the slot, regardless of whether the SINT was
>masked or not.
>
>And this (I think, haven't tested) should already work.  The userspace
>just keeps using the SINT route as it normally does, posting
>notifications to the corresponding irqfd when posting a message, and
>waiting on the resamplerfd for the message slot to become free.  If the
>SINT is masked KVM will skip injecting the interrupt, that's it.
>
>Roman.

That's what I was thinking originally as well, but then i noticed KDNET 
as a VMBus client (and it basically runs before anything else) is 
working in this polling mode, where SCONTROL is disabled and it just 
loops, and if it saw there is a PENDING message flag it will issue an 
EOM to indicate it has free the slot.
(There are a bunch of patches i sent on the QEMU mailing list as well 
  where i CCed you, I will probably revise it a bit but was hoping to get 
  KVM sorted out first).

Cheers,
-- Jon.

Re: [PATCH v2 0/1] x86/kvm/hyper-v: Add support to SYNIC exit on EOM

[Roman Kagan]

On Thu, Apr 16, 2020 at 03:54:30PM +0300, Jon Doron wrote:
> On 16/04/2020, Roman Kagan wrote:
> > On Thu, Apr 16, 2020 at 11:38:46AM +0300, Jon Doron wrote:
> > > According to the TLFS:
> > > "A write to the end of message (EOM) register by the guest causes the
> > > hypervisor to scan the internal message buffer queue(s) associated with
> > > the virtual processor.
> > > 
> > > If a message buffer queue contains a queued message buffer, the hypervisor
> > > attempts to deliver the message.
> > > 
> > > Message delivery succeeds if the SIM page is enabled and the message slot
> > > corresponding to the SINTx is empty (that is, the message type in the
> > > header is set to HvMessageTypeNone).
> > > If a message is successfully delivered, its corresponding internal message
> > > buffer is dequeued and marked free.
> > > If the corresponding SINTx is not masked, an edge-triggered interrupt is
> > > delivered (that is, the corresponding bit in the IRR is set).
> > > 
> > > This register can be used by guests to poll for messages. It can also be
> > > used as a way to drain the message queue for a SINTx that has
> > > been disabled (that is, masked)."
> > 
> > Doesn't this work already?
> > 
> 
> Well if you dont have SCONTROL and a GSI associated with the SINT then it
> does not...

Yes you do need both of these.

> > > So basically this means that we need to exit on EOM so the hypervisor
> > > will have a chance to send all the pending messages regardless of the
> > > SCONTROL mechnaisim.
> > 
> > I might be misinterpreting the spec, but my understanding is that
> > SCONTROL {en,dis}ables the message queueing completely.  What the quoted
> > part means is that a write to EOM should trigger the message source to
> > push a new message into the slot, regardless of whether the SINT was
> > masked or not.
> > 
> > And this (I think, haven't tested) should already work.  The userspace
> > just keeps using the SINT route as it normally does, posting
> > notifications to the corresponding irqfd when posting a message, and
> > waiting on the resamplerfd for the message slot to become free.  If the
> > SINT is masked KVM will skip injecting the interrupt, that's it.
> > 
> > Roman.
> 
> That's what I was thinking originally as well, but then i noticed KDNET as a
> VMBus client (and it basically runs before anything else) is working in this
> polling mode, where SCONTROL is disabled and it just loops, and if it saw
> there is a PENDING message flag it will issue an EOM to indicate it has free
> the slot.

Who sets up the message page then?  Doesn't it enabe SCONTROL as well?

Note that, even if you don't see it being enabled by Windows, it can be
enabled by the firmware and/or by the bootloader.

Can you perhaps try with the SeaBIOS from
https://src.openvz.org/projects/UP/repos/seabios branch hv-scsi?  It
enables SCONTROL and leaves it that way.

I'd also suggest tracing kvm_msr events (both reads and writes) for
SCONTROL and SIMP msrs, to better understand the picture.

So far the change you propose appears too heavy to work around the
problem of disabled SCONTROL.  You seem to be better off just making
sure it's enabled (either by the firmware or slighly violating the spec
and initializing to enabled from the start), and sticking to the
existing infrastructure for posting messages.

> (There are a bunch of patches i sent on the QEMU mailing list as well  where
> i CCed you, I will probably revise it a bit but was hoping to get  KVM
> sorted out first).

I'll look through the archive, should be there, thanks.

Roman.

Re: [PATCH v2 0/1] x86/kvm/hyper-v: Add support to SYNIC exit on EOM

[Jon Doron]

On 17/04/2020, Roman Kagan wrote:
>On Thu, Apr 16, 2020 at 03:54:30PM +0300, Jon Doron wrote:
>> On 16/04/2020, Roman Kagan wrote:
>> > On Thu, Apr 16, 2020 at 11:38:46AM +0300, Jon Doron wrote:
>> > > According to the TLFS:
>> > > "A write to the end of message (EOM) register by the guest causes the
>> > > hypervisor to scan the internal message buffer queue(s) associated with
>> > > the virtual processor.
>> > >
>> > > If a message buffer queue contains a queued message buffer, the hypervisor
>> > > attempts to deliver the message.
>> > >
>> > > Message delivery succeeds if the SIM page is enabled and the message slot
>> > > corresponding to the SINTx is empty (that is, the message type in the
>> > > header is set to HvMessageTypeNone).
>> > > If a message is successfully delivered, its corresponding internal message
>> > > buffer is dequeued and marked free.
>> > > If the corresponding SINTx is not masked, an edge-triggered interrupt is
>> > > delivered (that is, the corresponding bit in the IRR is set).
>> > >
>> > > This register can be used by guests to poll for messages. It can also be
>> > > used as a way to drain the message queue for a SINTx that has
>> > > been disabled (that is, masked)."
>> >
>> > Doesn't this work already?
>> >
>>
>> Well if you dont have SCONTROL and a GSI associated with the SINT then it
>> does not...
>
>Yes you do need both of these.
>
>> > > So basically this means that we need to exit on EOM so the hypervisor
>> > > will have a chance to send all the pending messages regardless of the
>> > > SCONTROL mechnaisim.
>> >
>> > I might be misinterpreting the spec, but my understanding is that
>> > SCONTROL {en,dis}ables the message queueing completely.  What the quoted
>> > part means is that a write to EOM should trigger the message source to
>> > push a new message into the slot, regardless of whether the SINT was
>> > masked or not.
>> >
>> > And this (I think, haven't tested) should already work.  The userspace
>> > just keeps using the SINT route as it normally does, posting
>> > notifications to the corresponding irqfd when posting a message, and
>> > waiting on the resamplerfd for the message slot to become free.  If the
>> > SINT is masked KVM will skip injecting the interrupt, that's it.
>> >
>> > Roman.
>>
>> That's what I was thinking originally as well, but then i noticed KDNET as a
>> VMBus client (and it basically runs before anything else) is working in this
>> polling mode, where SCONTROL is disabled and it just loops, and if it saw
>> there is a PENDING message flag it will issue an EOM to indicate it has free
>> the slot.
>
>Who sets up the message page then?  Doesn't it enabe SCONTROL as well?
>

KdNet is the one setting the SIMP and it's not setting the SCONTROL, ill 
paste output of KVM traces for the relevant MSRs

>Note that, even if you don't see it being enabled by Windows, it can be
>enabled by the firmware and/or by the bootloader.
>
>Can you perhaps try with the SeaBIOS from
>https://src.openvz.org/projects/UP/repos/seabios branch hv-scsi?  It
>enables SCONTROL and leaves it that way.
>
>I'd also suggest tracing kvm_msr events (both reads and writes) for
>SCONTROL and SIMP msrs, to better understand the picture.
>
>So far the change you propose appears too heavy to work around the
>problem of disabled SCONTROL.  You seem to be better off just making
>sure it's enabled (either by the firmware or slighly violating the spec
>and initializing to enabled from the start), and sticking to the
>existing infrastructure for posting messages.
>

I guess there is something I'm missing here but let's say the BIOS would 
have set the SCONTROL but the OS is not setting it, who is in charge of 
handling the interrupts?

>> (There are a bunch of patches i sent on the QEMU mailing list as well  where
>> i CCed you, I will probably revise it a bit but was hoping to get  KVM
>> sorted out first).
>
>I'll look through the archive, should be there, thanks.
>
>Roman.

I tried testing with both the SeaBIOS branch you have suggested and the 
EDK2, unfortunately I could not get the EDK2 build to identify my VM 
drive to boot from (not sure why)

Here is an output of KVM trace for the relevant MSRs (SCONTROL and SIMP)

QEMU Default BIOS
-----------------
  qemu-system-x86-613   [000] ....  1121.080722: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000080 data 0x0 host 1
  qemu-system-x86-613   [000] ....  1121.080722: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0x0 host 1
  qemu-system-x86-613   [000] .N..  1121.095592: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000080 data 0x0 host 1
  qemu-system-x86-613   [000] .N..  1121.095592: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0x0 host 1
Choose Windows DebugEntry
  qemu-system-x86-613   [001] ....  1165.185227: kvm_msr: msr_read 40000083 = 0x0
  qemu-system-x86-613   [001] ....  1165.185255: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0xfa1001 host 0
  qemu-system-x86-613   [001] ....  1165.185255: kvm_msr: msr_write 40000083 = 0xfa1001
  qemu-system-x86-613   [001] ....  1165.193206: kvm_msr: msr_read 40000083 = 0xfa1001
  qemu-system-x86-613   [001] ....  1165.193236: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0xfa1000 host 0
  qemu-system-x86-613   [001] ....  1165.193237: kvm_msr: msr_write 40000083 = 0xfa1000


SeaBIOS hv-scsci
----------------
  qemu-system-x86-656   [001] ....  1313.072714: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000080 data 0x0 host 1
  qemu-system-x86-656   [001] ....  1313.072714: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0x0 host 1
  qemu-system-x86-656   [001] ....  1313.087752: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000080 data 0x0 host 1
  qemu-system-x86-656   [001] ....  1313.087752: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0x0 host 1
  qemu-system-x86-656   [001] ....  1313.156675: kvm_msr: msr_read 40000083 = 0x0
  qemu-system-x86-656   [001] ....  1313.156680: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0x7fffe001 host 0
Choose Windows DebugEntry
  qemu-system-x86-656   [001] ....  1313.156680: kvm_msr: msr_write 40000083 = 0x7fffe001
  qemu-system-x86-656   [001] ....  1313.162111: kvm_msr: msr_read 40000080 = 0x0
  qemu-system-x86-656   [001] ....  1313.162118: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000080 data 0x1 host 0
  qemu-system-x86-656   [001] ....  1313.162119: kvm_msr: msr_write 40000080 = 0x1
  qemu-system-x86-656   [001] ....  1313.246758: kvm_msr: msr_read 40000083 = 0x7fffe001
  qemu-system-x86-656   [001] ....  1313.246764: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0x0 host 0
  qemu-system-x86-656   [001] ....  1313.246764: kvm_msr: msr_write 40000083 = 0x0
  qemu-system-x86-656   [001] ....  1348.904727: kvm_msr: msr_read 40000083 = 0x0
  qemu-system-x86-656   [001] ....  1348.904771: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0xfa1001 host 0
  qemu-system-x86-656   [001] ....  1348.904772: kvm_msr: msr_write 40000083 = 0xfa1001
  qemu-system-x86-656   [001] ....  1348.919170: kvm_msr: msr_read 40000083 = 0xfa1001
  qemu-system-x86-656   [001] ....  1348.919183: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0xfa1000 host 0
  qemu-system-x86-656   [001] ....  1348.919183: kvm_msr: msr_write 40000083 = 0xfa1000


  I could not get the EDK2 setup to work though
  (https://src.openvz.org/projects/UP/repos/edk2 branch hv-scsi)

It does not detect my VM hard drive not sure why (this is how i  
configured it:
  -drive file=./win10.qcow2,format=qcow2,if=none,id=drive_disk0 \
  -device virtio-blk-pci,drive=drive_disk0 \

(Is there something special i need to configure it order for it to 
  work?, I tried building EDK2 with and without SMM_REQUIRE and 
  SECURE_BOOT_ENABLE)


But in general it sounds like there is something I dont fully 
understand when SCONTROL is enabled, then a GSI is associated with this 
SintRoute.

Then when the guest triggers an EOI via the APIC we will trigger the GSI 
notification, which will give us another go on trying to copy the 
message into it's slot.

So is it the OS that is in charge of setting the EOI? If so then it 
needs to be aware of SCONTROL being enabled and just having it left set 
by the BIOS might not be enough?

Also in the TLFS (looking at v6) they mention that message queueing has 
"3 exit conditions", which will cause the hypervisor to try and attempt 
to deliver the additional messages.

The 3 exit conditions they refer to are:
* Another message buffer is queued.
* The guest indicates the “end of interrupt” by writing to the APIC’s 
   EOI register.
* The guest indicates the “end of message” by writing to the SynIC’s EOM 
   register.

Also notice this additional exit is only if there is a pending message 
and not for every EOM.

Thanks,
-- Jon.

Re: [PATCH v2 0/1] x86/kvm/hyper-v: Add support to SYNIC exit on EOM

[Jon Doron]

On 18/04/2020, Jon Doron wrote:
>On 17/04/2020, Roman Kagan wrote:
>>On Thu, Apr 16, 2020 at 03:54:30PM +0300, Jon Doron wrote:
>>>On 16/04/2020, Roman Kagan wrote:
>>>> On Thu, Apr 16, 2020 at 11:38:46AM +0300, Jon Doron wrote:
>>>> > According to the TLFS:
>>>> > "A write to the end of message (EOM) register by the guest causes the
>>>> > hypervisor to scan the internal message buffer queue(s) associated with
>>>> > the virtual processor.
>>>> >
>>>> > If a message buffer queue contains a queued message buffer, the hypervisor
>>>> > attempts to deliver the message.
>>>> >
>>>> > Message delivery succeeds if the SIM page is enabled and the message slot
>>>> > corresponding to the SINTx is empty (that is, the message type in the
>>>> > header is set to HvMessageTypeNone).
>>>> > If a message is successfully delivered, its corresponding internal message
>>>> > buffer is dequeued and marked free.
>>>> > If the corresponding SINTx is not masked, an edge-triggered interrupt is
>>>> > delivered (that is, the corresponding bit in the IRR is set).
>>>> >
>>>> > This register can be used by guests to poll for messages. It can also be
>>>> > used as a way to drain the message queue for a SINTx that has
>>>> > been disabled (that is, masked)."
>>>>
>>>> Doesn't this work already?
>>>>
>>>
>>>Well if you dont have SCONTROL and a GSI associated with the SINT then it
>>>does not...
>>
>>Yes you do need both of these.
>>
>>>> > So basically this means that we need to exit on EOM so the hypervisor
>>>> > will have a chance to send all the pending messages regardless of the
>>>> > SCONTROL mechnaisim.
>>>>
>>>> I might be misinterpreting the spec, but my understanding is that
>>>> SCONTROL {en,dis}ables the message queueing completely.  What the quoted
>>>> part means is that a write to EOM should trigger the message source to
>>>> push a new message into the slot, regardless of whether the SINT was
>>>> masked or not.
>>>>
>>>> And this (I think, haven't tested) should already work.  The userspace
>>>> just keeps using the SINT route as it normally does, posting
>>>> notifications to the corresponding irqfd when posting a message, and
>>>> waiting on the resamplerfd for the message slot to become free.  If the
>>>> SINT is masked KVM will skip injecting the interrupt, that's it.
>>>>
>>>> Roman.
>>>
>>>That's what I was thinking originally as well, but then i noticed KDNET as a
>>>VMBus client (and it basically runs before anything else) is working in this
>>>polling mode, where SCONTROL is disabled and it just loops, and if it saw
>>>there is a PENDING message flag it will issue an EOM to indicate it has free
>>>the slot.
>>
>>Who sets up the message page then?  Doesn't it enabe SCONTROL as well?
>>
>
>KdNet is the one setting the SIMP and it's not setting the SCONTROL, 
>ill paste output of KVM traces for the relevant MSRs
>
>>Note that, even if you don't see it being enabled by Windows, it can be
>>enabled by the firmware and/or by the bootloader.
>>
>>Can you perhaps try with the SeaBIOS from
>>https://src.openvz.org/projects/UP/repos/seabios branch hv-scsi?  It
>>enables SCONTROL and leaves it that way.
>>
>>I'd also suggest tracing kvm_msr events (both reads and writes) for
>>SCONTROL and SIMP msrs, to better understand the picture.
>>
>>So far the change you propose appears too heavy to work around the
>>problem of disabled SCONTROL.  You seem to be better off just making
>>sure it's enabled (either by the firmware or slighly violating the spec
>>and initializing to enabled from the start), and sticking to the
>>existing infrastructure for posting messages.
>>
>
>I guess there is something I'm missing here but let's say the BIOS 
>would have set the SCONTROL but the OS is not setting it, who is in 
>charge of handling the interrupts?
>
>>>(There are a bunch of patches i sent on the QEMU mailing list as well  where
>>>i CCed you, I will probably revise it a bit but was hoping to get  KVM
>>>sorted out first).
>>
>>I'll look through the archive, should be there, thanks.
>>
>>Roman.
>
>I tried testing with both the SeaBIOS branch you have suggested and 
>the EDK2, unfortunately I could not get the EDK2 build to identify my 
>VM drive to boot from (not sure why)
>
>Here is an output of KVM trace for the relevant MSRs (SCONTROL and SIMP)
>
>QEMU Default BIOS
>-----------------
> qemu-system-x86-613   [000] ....  1121.080722: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000080 data 0x0 host 1
> qemu-system-x86-613   [000] ....  1121.080722: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0x0 host 1
> qemu-system-x86-613   [000] .N..  1121.095592: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000080 data 0x0 host 1
> qemu-system-x86-613   [000] .N..  1121.095592: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0x0 host 1
>Choose Windows DebugEntry
> qemu-system-x86-613   [001] ....  1165.185227: kvm_msr: msr_read 40000083 = 0x0
> qemu-system-x86-613   [001] ....  1165.185255: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0xfa1001 host 0
> qemu-system-x86-613   [001] ....  1165.185255: kvm_msr: msr_write 40000083 = 0xfa1001
> qemu-system-x86-613   [001] ....  1165.193206: kvm_msr: msr_read 40000083 = 0xfa1001
> qemu-system-x86-613   [001] ....  1165.193236: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0xfa1000 host 0
> qemu-system-x86-613   [001] ....  1165.193237: kvm_msr: msr_write 40000083 = 0xfa1000
>
>
>SeaBIOS hv-scsci
>----------------
> qemu-system-x86-656   [001] ....  1313.072714: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000080 data 0x0 host 1
> qemu-system-x86-656   [001] ....  1313.072714: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0x0 host 1
> qemu-system-x86-656   [001] ....  1313.087752: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000080 data 0x0 host 1
> qemu-system-x86-656   [001] ....  1313.087752: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0x0 host 1
> qemu-system-x86-656   [001] ....  1313.156675: kvm_msr: msr_read 40000083 = 0x0
> qemu-system-x86-656   [001] ....  1313.156680: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0x7fffe001 host 0
>Choose Windows DebugEntry
> qemu-system-x86-656   [001] ....  1313.156680: kvm_msr: msr_write 40000083 = 0x7fffe001
> qemu-system-x86-656   [001] ....  1313.162111: kvm_msr: msr_read 40000080 = 0x0
> qemu-system-x86-656   [001] ....  1313.162118: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000080 data 0x1 host 0
> qemu-system-x86-656   [001] ....  1313.162119: kvm_msr: msr_write 40000080 = 0x1
> qemu-system-x86-656   [001] ....  1313.246758: kvm_msr: msr_read 40000083 = 0x7fffe001
> qemu-system-x86-656   [001] ....  1313.246764: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0x0 host 0
> qemu-system-x86-656   [001] ....  1313.246764: kvm_msr: msr_write 40000083 = 0x0
> qemu-system-x86-656   [001] ....  1348.904727: kvm_msr: msr_read 40000083 = 0x0
> qemu-system-x86-656   [001] ....  1348.904771: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0xfa1001 host 0
> qemu-system-x86-656   [001] ....  1348.904772: kvm_msr: msr_write 40000083 = 0xfa1001
> qemu-system-x86-656   [001] ....  1348.919170: kvm_msr: msr_read 40000083 = 0xfa1001
> qemu-system-x86-656   [001] ....  1348.919183: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0xfa1000 host 0
> qemu-system-x86-656   [001] ....  1348.919183: kvm_msr: msr_write 40000083 = 0xfa1000
>
>
> I could not get the EDK2 setup to work though
> (https://src.openvz.org/projects/UP/repos/edk2 branch hv-scsi)
>
>It does not detect my VM hard drive not sure why (this is how i  
>configured it:
> -drive file=./win10.qcow2,format=qcow2,if=none,id=drive_disk0 \
> -device virtio-blk-pci,drive=drive_disk0 \
>
>(Is there something special i need to configure it order for it to  
>work?, I tried building EDK2 with and without SMM_REQUIRE and  
>SECURE_BOOT_ENABLE)
>
>
>But in general it sounds like there is something I dont fully 
>understand when SCONTROL is enabled, then a GSI is associated with 
>this SintRoute.
>
>Then when the guest triggers an EOI via the APIC we will trigger the 
>GSI notification, which will give us another go on trying to copy the 
>message into it's slot.
>
>So is it the OS that is in charge of setting the EOI? If so then it 
>needs to be aware of SCONTROL being enabled and just having it left 
>set by the BIOS might not be enough?
>
>Also in the TLFS (looking at v6) they mention that message queueing 
>has "3 exit conditions", which will cause the hypervisor to try and 
>attempt to deliver the additional messages.
>
>The 3 exit conditions they refer to are:
>* Another message buffer is queued.
>* The guest indicates the “end of interrupt” by writing to the APIC’s   
>EOI register.
>* The guest indicates the “end of message” by writing to the SynIC’s 
>EOM   register.
>
>Also notice this additional exit is only if there is a pending message 
>and not for every EOM.
>
>Thanks,
>-- Jon.

Hi Roman

Any other thoughts/suggestions about this?

Thanks,
-- Jon.

Re: [PATCH v2 0/1] x86/kvm/hyper-v: Add support to SYNIC exit on EOM

[Roman Kagan]

On Sat, Apr 18, 2020 at 09:41:27AM +0300, Jon Doron wrote:
> On 17/04/2020, Roman Kagan wrote:
> > On Thu, Apr 16, 2020 at 03:54:30PM +0300, Jon Doron wrote:
> > > On 16/04/2020, Roman Kagan wrote:
> > > > On Thu, Apr 16, 2020 at 11:38:46AM +0300, Jon Doron wrote:
> > > > > According to the TLFS:
> > > > > "A write to the end of message (EOM) register by the guest causes the
> > > > > hypervisor to scan the internal message buffer queue(s) associated with
> > > > > the virtual processor.
> > > > >
> > > > > If a message buffer queue contains a queued message buffer, the hypervisor
> > > > > attempts to deliver the message.
> > > > >
> > > > > Message delivery succeeds if the SIM page is enabled and the message slot
> > > > > corresponding to the SINTx is empty (that is, the message type in the
> > > > > header is set to HvMessageTypeNone).
> > > > > If a message is successfully delivered, its corresponding internal message
> > > > > buffer is dequeued and marked free.
> > > > > If the corresponding SINTx is not masked, an edge-triggered interrupt is
> > > > > delivered (that is, the corresponding bit in the IRR is set).
> > > > >
> > > > > This register can be used by guests to poll for messages. It can also be
> > > > > used as a way to drain the message queue for a SINTx that has
> > > > > been disabled (that is, masked)."
> > > >
> > > > Doesn't this work already?
> > > >
> > > 
> > > Well if you dont have SCONTROL and a GSI associated with the SINT then it
> > > does not...
> > 
> > Yes you do need both of these.
> > 
> > > > > So basically this means that we need to exit on EOM so the hypervisor
> > > > > will have a chance to send all the pending messages regardless of the
> > > > > SCONTROL mechnaisim.
> > > >
> > > > I might be misinterpreting the spec, but my understanding is that
> > > > SCONTROL {en,dis}ables the message queueing completely.  What the quoted
> > > > part means is that a write to EOM should trigger the message source to
> > > > push a new message into the slot, regardless of whether the SINT was
> > > > masked or not.
> > > >
> > > > And this (I think, haven't tested) should already work.  The userspace
> > > > just keeps using the SINT route as it normally does, posting
> > > > notifications to the corresponding irqfd when posting a message, and
> > > > waiting on the resamplerfd for the message slot to become free.  If the
> > > > SINT is masked KVM will skip injecting the interrupt, that's it.
> > > >
> > > > Roman.
> > > 
> > > That's what I was thinking originally as well, but then i noticed KDNET as a
> > > VMBus client (and it basically runs before anything else) is working in this
> > > polling mode, where SCONTROL is disabled and it just loops, and if it saw
> > > there is a PENDING message flag it will issue an EOM to indicate it has free
> > > the slot.
> > 
> > Who sets up the message page then?  Doesn't it enabe SCONTROL as well?
> > 
> 
> KdNet is the one setting the SIMP and it's not setting the SCONTROL, ill
> paste output of KVM traces for the relevant MSRs
> 
> > Note that, even if you don't see it being enabled by Windows, it can be
> > enabled by the firmware and/or by the bootloader.
> > 
> > Can you perhaps try with the SeaBIOS from
> > https://src.openvz.org/projects/UP/repos/seabios branch hv-scsi?  It
> > enables SCONTROL and leaves it that way.
> > 
> > I'd also suggest tracing kvm_msr events (both reads and writes) for
> > SCONTROL and SIMP msrs, to better understand the picture.
> > 
> > So far the change you propose appears too heavy to work around the
> > problem of disabled SCONTROL.  You seem to be better off just making
> > sure it's enabled (either by the firmware or slighly violating the spec
> > and initializing to enabled from the start), and sticking to the
> > existing infrastructure for posting messages.
> > 
> 
> I guess there is something I'm missing here but let's say the BIOS would
> have set the SCONTROL but the OS is not setting it, who is in charge of
> handling the interrupts?

SCONTROL doesn't enable the interrupts, it enables SynIC as a whole.
The interrupts are enabled via individual SINTx msrs.  This SeaBIOS
branch does exactly this: it enables the SynIC via SCONTROL, and then
specific SynIC functionality via SIMP/SIEFP, but doesn't activate SINTx
and works in polling mode.

I agree that this global SCONTROL switch seems redundant but it appears
to match the spec.

> > > (There are a bunch of patches i sent on the QEMU mailing list as well  where
> > > i CCed you, I will probably revise it a bit but was hoping to get  KVM
> > > sorted out first).
> > 
> > I'll look through the archive, should be there, thanks.
> > 
> > Roman.
> 
> I tried testing with both the SeaBIOS branch you have suggested and the
> EDK2, unfortunately I could not get the EDK2 build to identify my VM drive
> to boot from (not sure why)
> 
> Here is an output of KVM trace for the relevant MSRs (SCONTROL and SIMP)
> 
> QEMU Default BIOS
> -----------------
>  qemu-system-x86-613   [000] ....  1121.080722: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000080 data 0x0 host 1
>  qemu-system-x86-613   [000] ....  1121.080722: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0x0 host 1
>  qemu-system-x86-613   [000] .N..  1121.095592: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000080 data 0x0 host 1
>  qemu-system-x86-613   [000] .N..  1121.095592: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0x0 host 1
> Choose Windows DebugEntry
>  qemu-system-x86-613   [001] ....  1165.185227: kvm_msr: msr_read 40000083 = 0x0
>  qemu-system-x86-613   [001] ....  1165.185255: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0xfa1001 host 0
>  qemu-system-x86-613   [001] ....  1165.185255: kvm_msr: msr_write 40000083 = 0xfa1001
>  qemu-system-x86-613   [001] ....  1165.193206: kvm_msr: msr_read 40000083 = 0xfa1001
>  qemu-system-x86-613   [001] ....  1165.193236: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0xfa1000 host 0
>  qemu-system-x86-613   [001] ....  1165.193237: kvm_msr: msr_write 40000083 = 0xfa1000
> 
> 
> SeaBIOS hv-scsci
> ----------------
>  qemu-system-x86-656   [001] ....  1313.072714: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000080 data 0x0 host 1
>  qemu-system-x86-656   [001] ....  1313.072714: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0x0 host 1
>  qemu-system-x86-656   [001] ....  1313.087752: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000080 data 0x0 host 1
>  qemu-system-x86-656   [001] ....  1313.087752: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0x0 host 1

Initialization (host == 1)

>  qemu-system-x86-656   [001] ....  1313.156675: kvm_msr: msr_read 40000083 = 0x0
>  qemu-system-x86-656   [001] ....  1313.156680: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0x7fffe001 host 0
> Choose Windows DebugEntry

I guess this is a bit misplaced timewise, BIOS is still working here

>  qemu-system-x86-656   [001] ....  1313.156680: kvm_msr: msr_write 40000083 = 0x7fffe001

BIOS sets up message page

>  qemu-system-x86-656   [001] ....  1313.162111: kvm_msr: msr_read 40000080 = 0x0
>  qemu-system-x86-656   [001] ....  1313.162118: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000080 data 0x1 host 0
>  qemu-system-x86-656   [001] ....  1313.162119: kvm_msr: msr_write 40000080 = 0x1

BIOS activates SCONTROL

>  qemu-system-x86-656   [001] ....  1313.246758: kvm_msr: msr_read 40000083 = 0x7fffe001
>  qemu-system-x86-656   [001] ....  1313.246764: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0x0 host 0
>  qemu-system-x86-656   [001] ....  1313.246764: kvm_msr: msr_write 40000083 = 0x0

BIOS clears message page (it's not needed once the VMBus device was
brought up)

I guess the choice of Windows DebugEntry appeared somewhere here.

>  qemu-system-x86-656   [001] ....  1348.904727: kvm_msr: msr_read 40000083 = 0x0
>  qemu-system-x86-656   [001] ....  1348.904771: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0xfa1001 host 0
>  qemu-system-x86-656   [001] ....  1348.904772: kvm_msr: msr_write 40000083 = 0xfa1001

Bootloader (debug stub?) sets up the message page

>  qemu-system-x86-656   [001] ....  1348.919170: kvm_msr: msr_read 40000083 = 0xfa1001
>  qemu-system-x86-656   [001] ....  1348.919183: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0xfa1000 host 0
>  qemu-system-x86-656   [001] ....  1348.919183: kvm_msr: msr_write 40000083 = 0xfa1000

Message page is being disabled again.

I guess you only filtered SCONTROL and SIMP, skipping e.g. SVERSION,
GUEST_OS_ID, HYPERCALL, etc., which are also part of the exchange here.

>  I could not get the EDK2 setup to work though
>  (https://src.openvz.org/projects/UP/repos/edk2 branch hv-scsi)
> 
> It does not detect my VM hard drive not sure why (this is how i  configured
> it:
>  -drive file=./win10.qcow2,format=qcow2,if=none,id=drive_disk0 \
>  -device virtio-blk-pci,drive=drive_disk0 \
> 
> (Is there something special i need to configure it order for it to  work?, I
> tried building EDK2 with and without SMM_REQUIRE and  SECURE_BOOT_ENABLE)

No special configuration I can think of.

> But in general it sounds like there is something I dont fully understand
> when SCONTROL is enabled, then a GSI is associated with this SintRoute.
> 
> Then when the guest triggers an EOI via the APIC we will trigger the GSI
> notification, which will give us another go on trying to copy the message
> into it's slot.

Right.

> So is it the OS that is in charge of setting the EOI?

Yes.

> If so then it needs to
> be aware of SCONTROL being enabled and just having it left set by the BIOS
> might not be enough?

Yes it needs to be aware of SCONTROL being enabled.  However, this
awareness may be based on a pure assumption that the previous entity
(BIOS or bootloader) did it already.

> Also in the TLFS (looking at v6) they mention that message queueing has "3
> exit conditions", which will cause the hypervisor to try and attempt to
> deliver the additional messages.
> 
> The 3 exit conditions they refer to are:
> * Another message buffer is queued.
> * The guest indicates the “end of interrupt” by writing to the APIC’s   EOI
> register.
> * The guest indicates the “end of message” by writing to the SynIC’s EOM
> register.
> 
> Also notice this additional exit is only if there is a pending message and
> not for every EOM.

This meaning of "exit" doesn't trivially correspond to what we have in
KVM.  A write to an msr does cause a vmexit.  Then KVM notifies resample
eventfds for all SINTs that have them set up, no matter if there's a
pending message in the slot.  It may be slightly more optimal to only
notify those having indicated a pending message, but I don't see the
current behavior break anything or violate the spec, so, as EOMs are not
used on fast paths, I woudn't bother optimizing.

Roman.

Re: [PATCH v2 0/1] x86/kvm/hyper-v: Add support to SYNIC exit on EOM

[Jon Doron]

On 24/04/2020, Roman Kagan wrote:
>On Sat, Apr 18, 2020 at 09:41:27AM +0300, Jon Doron wrote:
>> On 17/04/2020, Roman Kagan wrote:
>> > On Thu, Apr 16, 2020 at 03:54:30PM +0300, Jon Doron wrote:
>> > > On 16/04/2020, Roman Kagan wrote:
>> > > > On Thu, Apr 16, 2020 at 11:38:46AM +0300, Jon Doron wrote:
>> > > > > According to the TLFS:
>> > > > > "A write to the end of message (EOM) register by the guest causes the
>> > > > > hypervisor to scan the internal message buffer queue(s) associated with
>> > > > > the virtual processor.
>> > > > >
>> > > > > If a message buffer queue contains a queued message buffer, the hypervisor
>> > > > > attempts to deliver the message.
>> > > > >
>> > > > > Message delivery succeeds if the SIM page is enabled and the message slot
>> > > > > corresponding to the SINTx is empty (that is, the message type in the
>> > > > > header is set to HvMessageTypeNone).
>> > > > > If a message is successfully delivered, its corresponding internal message
>> > > > > buffer is dequeued and marked free.
>> > > > > If the corresponding SINTx is not masked, an edge-triggered interrupt is
>> > > > > delivered (that is, the corresponding bit in the IRR is set).
>> > > > >
>> > > > > This register can be used by guests to poll for messages. It can also be
>> > > > > used as a way to drain the message queue for a SINTx that has
>> > > > > been disabled (that is, masked)."
>> > > >
>> > > > Doesn't this work already?
>> > > >
>> > >
>> > > Well if you dont have SCONTROL and a GSI associated with the SINT then it
>> > > does not...
>> >
>> > Yes you do need both of these.
>> >
>> > > > > So basically this means that we need to exit on EOM so the hypervisor
>> > > > > will have a chance to send all the pending messages regardless of the
>> > > > > SCONTROL mechnaisim.
>> > > >
>> > > > I might be misinterpreting the spec, but my understanding is that
>> > > > SCONTROL {en,dis}ables the message queueing completely.  What the quoted
>> > > > part means is that a write to EOM should trigger the message source to
>> > > > push a new message into the slot, regardless of whether the SINT was
>> > > > masked or not.
>> > > >
>> > > > And this (I think, haven't tested) should already work.  The userspace
>> > > > just keeps using the SINT route as it normally does, posting
>> > > > notifications to the corresponding irqfd when posting a message, and
>> > > > waiting on the resamplerfd for the message slot to become free.  If the
>> > > > SINT is masked KVM will skip injecting the interrupt, that's it.
>> > > >
>> > > > Roman.
>> > >
>> > > That's what I was thinking originally as well, but then i noticed KDNET as a
>> > > VMBus client (and it basically runs before anything else) is working in this
>> > > polling mode, where SCONTROL is disabled and it just loops, and if it saw
>> > > there is a PENDING message flag it will issue an EOM to indicate it has free
>> > > the slot.
>> >
>> > Who sets up the message page then?  Doesn't it enabe SCONTROL as well?
>> >
>>
>> KdNet is the one setting the SIMP and it's not setting the SCONTROL, ill
>> paste output of KVM traces for the relevant MSRs
>>
>> > Note that, even if you don't see it being enabled by Windows, it can be
>> > enabled by the firmware and/or by the bootloader.
>> >
>> > Can you perhaps try with the SeaBIOS from
>> > https://src.openvz.org/projects/UP/repos/seabios branch hv-scsi?  It
>> > enables SCONTROL and leaves it that way.
>> >
>> > I'd also suggest tracing kvm_msr events (both reads and writes) for
>> > SCONTROL and SIMP msrs, to better understand the picture.
>> >
>> > So far the change you propose appears too heavy to work around the
>> > problem of disabled SCONTROL.  You seem to be better off just making
>> > sure it's enabled (either by the firmware or slighly violating the spec
>> > and initializing to enabled from the start), and sticking to the
>> > existing infrastructure for posting messages.
>> >
>>
>> I guess there is something I'm missing here but let's say the BIOS would
>> have set the SCONTROL but the OS is not setting it, who is in charge of
>> handling the interrupts?
>
>SCONTROL doesn't enable the interrupts, it enables SynIC as a whole.
>The interrupts are enabled via individual SINTx msrs.  This SeaBIOS
>branch does exactly this: it enables the SynIC via SCONTROL, and then
>specific SynIC functionality via SIMP/SIEFP, but doesn't activate SINTx
>and works in polling mode.
>
>I agree that this global SCONTROL switch seems redundant but it appears
>to match the spec.
>
>> > > (There are a bunch of patches i sent on the QEMU mailing list as well  where
>> > > i CCed you, I will probably revise it a bit but was hoping to get  KVM
>> > > sorted out first).
>> >
>> > I'll look through the archive, should be there, thanks.
>> >
>> > Roman.
>>
>> I tried testing with both the SeaBIOS branch you have suggested and the
>> EDK2, unfortunately I could not get the EDK2 build to identify my VM drive
>> to boot from (not sure why)
>>
>> Here is an output of KVM trace for the relevant MSRs (SCONTROL and SIMP)
>>
>> QEMU Default BIOS
>> -----------------
>>  qemu-system-x86-613   [000] ....  1121.080722: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000080 data 0x0 host 1
>>  qemu-system-x86-613   [000] ....  1121.080722: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0x0 host 1
>>  qemu-system-x86-613   [000] .N..  1121.095592: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000080 data 0x0 host 1
>>  qemu-system-x86-613   [000] .N..  1121.095592: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0x0 host 1
>> Choose Windows DebugEntry
>>  qemu-system-x86-613   [001] ....  1165.185227: kvm_msr: msr_read 40000083 = 0x0
>>  qemu-system-x86-613   [001] ....  1165.185255: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0xfa1001 host 0
>>  qemu-system-x86-613   [001] ....  1165.185255: kvm_msr: msr_write 40000083 = 0xfa1001
>>  qemu-system-x86-613   [001] ....  1165.193206: kvm_msr: msr_read 40000083 = 0xfa1001
>>  qemu-system-x86-613   [001] ....  1165.193236: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0xfa1000 host 0
>>  qemu-system-x86-613   [001] ....  1165.193237: kvm_msr: msr_write 40000083 = 0xfa1000
>>
>>
>> SeaBIOS hv-scsci
>> ----------------
>>  qemu-system-x86-656   [001] ....  1313.072714: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000080 data 0x0 host 1
>>  qemu-system-x86-656   [001] ....  1313.072714: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0x0 host 1
>>  qemu-system-x86-656   [001] ....  1313.087752: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000080 data 0x0 host 1
>>  qemu-system-x86-656   [001] ....  1313.087752: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0x0 host 1
>
>Initialization (host == 1)
>
>>  qemu-system-x86-656   [001] ....  1313.156675: kvm_msr: msr_read 40000083 = 0x0
>>  qemu-system-x86-656   [001] ....  1313.156680: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0x7fffe001 host 0
>> Choose Windows DebugEntry
>
>I guess this is a bit misplaced timewise, BIOS is still working here
>
>>  qemu-system-x86-656   [001] ....  1313.156680: kvm_msr: msr_write 40000083 = 0x7fffe001
>
>BIOS sets up message page
>
>>  qemu-system-x86-656   [001] ....  1313.162111: kvm_msr: msr_read 40000080 = 0x0
>>  qemu-system-x86-656   [001] ....  1313.162118: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000080 data 0x1 host 0
>>  qemu-system-x86-656   [001] ....  1313.162119: kvm_msr: msr_write 40000080 = 0x1
>
>BIOS activates SCONTROL
>
>>  qemu-system-x86-656   [001] ....  1313.246758: kvm_msr: msr_read 40000083 = 0x7fffe001
>>  qemu-system-x86-656   [001] ....  1313.246764: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0x0 host 0
>>  qemu-system-x86-656   [001] ....  1313.246764: kvm_msr: msr_write 40000083 = 0x0
>
>BIOS clears message page (it's not needed once the VMBus device was
>brought up)
>
>I guess the choice of Windows DebugEntry appeared somewhere here.
>
>>  qemu-system-x86-656   [001] ....  1348.904727: kvm_msr: msr_read 40000083 = 0x0
>>  qemu-system-x86-656   [001] ....  1348.904771: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0xfa1001 host 0
>>  qemu-system-x86-656   [001] ....  1348.904772: kvm_msr: msr_write 40000083 = 0xfa1001
>
>Bootloader (debug stub?) sets up the message page
>
>>  qemu-system-x86-656   [001] ....  1348.919170: kvm_msr: msr_read 40000083 = 0xfa1001
>>  qemu-system-x86-656   [001] ....  1348.919183: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0xfa1000 host 0
>>  qemu-system-x86-656   [001] ....  1348.919183: kvm_msr: msr_write 40000083 = 0xfa1000
>
>Message page is being disabled again.
>
>I guess you only filtered SCONTROL and SIMP, skipping e.g. SVERSION,
>GUEST_OS_ID, HYPERCALL, etc., which are also part of the exchange here.
>

Right my bad :( if you want I can re-run the test with the others as 
well (do you need me to?)

>>  I could not get the EDK2 setup to work though
>>  (https://src.openvz.org/projects/UP/repos/edk2 branch hv-scsi)
>>
>> It does not detect my VM hard drive not sure why (this is how i  configured
>> it:
>>  -drive file=./win10.qcow2,format=qcow2,if=none,id=drive_disk0 \
>>  -device virtio-blk-pci,drive=drive_disk0 \
>>
>> (Is there something special i need to configure it order for it to  work?, I
>> tried building EDK2 with and without SMM_REQUIRE and  SECURE_BOOT_ENABLE)
>
>No special configuration I can think of.
>
>> But in general it sounds like there is something I dont fully understand
>> when SCONTROL is enabled, then a GSI is associated with this SintRoute.
>>
>> Then when the guest triggers an EOI via the APIC we will trigger the GSI
>> notification, which will give us another go on trying to copy the message
>> into it's slot.
>
>Right.
>
>> So is it the OS that is in charge of setting the EOI?
>
>Yes.
>
>> If so then it needs to
>> be aware of SCONTROL being enabled and just having it left set by the BIOS
>> might not be enough?
>
>Yes it needs to be aware of SCONTROL being enabled.  However, this
>awareness may be based on a pure assumption that the previous entity
>(BIOS or bootloader) did it already.
>
>> Also in the TLFS (looking at v6) they mention that message queueing has "3
>> exit conditions", which will cause the hypervisor to try and attempt to
>> deliver the additional messages.
>>
>> The 3 exit conditions they refer to are:
>> * Another message buffer is queued.
>> * The guest indicates the “end of interrupt” by writing to the APIC’s   EOI
>> register.
>> * The guest indicates the “end of message” by writing to the SynIC’s EOM
>> register.
>>
>> Also notice this additional exit is only if there is a pending message and
>> not for every EOM.
>
>This meaning of "exit" doesn't trivially correspond to what we have in
>KVM.  A write to an msr does cause a vmexit.  Then KVM notifies resample
>eventfds for all SINTs that have them set up, no matter if there's a
>pending message in the slot.  It may be slightly more optimal to only
>notify those having indicated a pending message, but I don't see the
>current behavior break anything or violate the spec, so, as EOMs are not
>used on fast paths, I woudn't bother optimizing.
>
>Roman.

Hi Roman,

So based on your answer I got to the following conclusions (correct if 
they are wrong).

First of the one in charge of setting the SCONTROL in the 1st place is 
the BIOS (I dont have a real Hyper-V setup so I cannot really debug it 
and see, not sure which BIOS they have or if we can "rip" it out and run 
it through KVM and see how things look like this way).

If the BIOS has not set the SCONTROL I would expect the OS to have 
something along the lines:
if (!(get_scontrol() & ENABLED))
     set_scontrol(ENABLED);

So I started looking through the entire Windows system looking what can 
set SCONTROL, I believe I have found the flow to be the following:

VMBus.sys imports winhv.sys (which is an export library) winhv.sys will 
set the SCONTROL prior to VMBus DriverEntry starting here is the 
complete flow:
winhv!DllInitialize -> winhv!WinHvpInitialize -> 
winhv!WinHvReportPresentHypervisor -> winhv!WinHvpConnectToSynic -> 
winhv!WinHvpEnableSynic

Eventually WinHvpEnableSynic will simply set SCONTROL (for future 
reference if anyone needs to look into how HyperV register access works 
in Windows it seems like there is an enum representing all the HyperV 
registers and to access it there are helper functions to Get/Set.
SCONTROL value in the enum is 0x0a0010 .

winhv.sys simply provides very simple API to access the Sints i.e 
(WinHvSetSint / WinHvSetEndOfMessage / WinHvSetSintOnCurrentProcessor / 
  WinHvGetSintMessage / etc.)

So basically it seems like the OS does not really care if the BIOS has 
setup the SCONTROL or not, and does so always (if it can) unfortunately 
in my flow (via kdnet) VMBus is not loaded yet and so does winhv.sys so 
they "fallback" into this Polling mode.

So that covers the OS part, after that I have tried looking for relevant 
code in bootmgr and winload (which are Windows boot loader part (like 
grub) and I could not find any code that might setup SCONTROL.

 From your experience with this did you see Hyper-V BIOS simply setting 
the SCONTROL? Perhaps if that's the case then the correct fix needs to 
be in the SeaBIOS and the EDK .

I tried to see if Hyper-V supports giving it a BIOS but could not find 
anyway of doing this, so it just might be that Hyper-V assumes the BIOS 
is in charge of setting up SCONTROL for all the boot loader components.

But in a way it sounds weird because I would expect to see KDNet working 
with the ACPI to trigger the GSI but I could not find any relevant code 
that might do that.

As I write this I think I'm starting to get your point just to make sure 
I understand it:

1. When a new SintRoute is created we associate it with a GSI
2. When an EOM is set, we trigger all the GSIs so QEMU will get 
    execution time and send all pending messages if it can.

So basically like you said everything "works" from our perspective 
regardless if the system has setup SCONTROL or not, because you trigger 
the interrupt to QEMU regardless of SCONTROL so it can clear the pending 
message.

If that's indeed the case then probably the only thing needs fixing in 
my scenario is in QEMU where it should not really care for the SCONTROL 
if it's enabled or not.

Sounds about right?

Thanks,
-- Jon.

Re: [PATCH v2 0/1] x86/kvm/hyper-v: Add support to SYNIC exit on EOM

[Jon Doron]

On 25/04/2020, Jon Doron wrote:
>On 24/04/2020, Roman Kagan wrote:
>>On Sat, Apr 18, 2020 at 09:41:27AM +0300, Jon Doron wrote:
>>>On 17/04/2020, Roman Kagan wrote:
>>>> On Thu, Apr 16, 2020 at 03:54:30PM +0300, Jon Doron wrote:
>>>> > On 16/04/2020, Roman Kagan wrote:
>>>> > > On Thu, Apr 16, 2020 at 11:38:46AM +0300, Jon Doron wrote:
>>>> > > > According to the TLFS:
>>>> > > > "A write to the end of message (EOM) register by the guest causes the
>>>> > > > hypervisor to scan the internal message buffer queue(s) associated with
>>>> > > > the virtual processor.
>>>> > > >
>>>> > > > If a message buffer queue contains a queued message buffer, the hypervisor
>>>> > > > attempts to deliver the message.
>>>> > > >
>>>> > > > Message delivery succeeds if the SIM page is enabled and the message slot
>>>> > > > corresponding to the SINTx is empty (that is, the message type in the
>>>> > > > header is set to HvMessageTypeNone).
>>>> > > > If a message is successfully delivered, its corresponding internal message
>>>> > > > buffer is dequeued and marked free.
>>>> > > > If the corresponding SINTx is not masked, an edge-triggered interrupt is
>>>> > > > delivered (that is, the corresponding bit in the IRR is set).
>>>> > > >
>>>> > > > This register can be used by guests to poll for messages. It can also be
>>>> > > > used as a way to drain the message queue for a SINTx that has
>>>> > > > been disabled (that is, masked)."
>>>> > >
>>>> > > Doesn't this work already?
>>>> > >
>>>> >
>>>> > Well if you dont have SCONTROL and a GSI associated with the SINT then it
>>>> > does not...
>>>>
>>>> Yes you do need both of these.
>>>>
>>>> > > > So basically this means that we need to exit on EOM so the hypervisor
>>>> > > > will have a chance to send all the pending messages regardless of the
>>>> > > > SCONTROL mechnaisim.
>>>> > >
>>>> > > I might be misinterpreting the spec, but my understanding is that
>>>> > > SCONTROL {en,dis}ables the message queueing completely.  What the quoted
>>>> > > part means is that a write to EOM should trigger the message source to
>>>> > > push a new message into the slot, regardless of whether the SINT was
>>>> > > masked or not.
>>>> > >
>>>> > > And this (I think, haven't tested) should already work.  The userspace
>>>> > > just keeps using the SINT route as it normally does, posting
>>>> > > notifications to the corresponding irqfd when posting a message, and
>>>> > > waiting on the resamplerfd for the message slot to become free.  If the
>>>> > > SINT is masked KVM will skip injecting the interrupt, that's it.
>>>> > >
>>>> > > Roman.
>>>> >
>>>> > That's what I was thinking originally as well, but then i noticed KDNET as a
>>>> > VMBus client (and it basically runs before anything else) is working in this
>>>> > polling mode, where SCONTROL is disabled and it just loops, and if it saw
>>>> > there is a PENDING message flag it will issue an EOM to indicate it has free
>>>> > the slot.
>>>>
>>>> Who sets up the message page then?  Doesn't it enabe SCONTROL as well?
>>>>
>>>
>>>KdNet is the one setting the SIMP and it's not setting the SCONTROL, ill
>>>paste output of KVM traces for the relevant MSRs
>>>
>>>> Note that, even if you don't see it being enabled by Windows, it can be
>>>> enabled by the firmware and/or by the bootloader.
>>>>
>>>> Can you perhaps try with the SeaBIOS from
>>>> https://src.openvz.org/projects/UP/repos/seabios branch hv-scsi?  It
>>>> enables SCONTROL and leaves it that way.
>>>>
>>>> I'd also suggest tracing kvm_msr events (both reads and writes) for
>>>> SCONTROL and SIMP msrs, to better understand the picture.
>>>>
>>>> So far the change you propose appears too heavy to work around the
>>>> problem of disabled SCONTROL.  You seem to be better off just making
>>>> sure it's enabled (either by the firmware or slighly violating the spec
>>>> and initializing to enabled from the start), and sticking to the
>>>> existing infrastructure for posting messages.
>>>>
>>>
>>>I guess there is something I'm missing here but let's say the BIOS would
>>>have set the SCONTROL but the OS is not setting it, who is in charge of
>>>handling the interrupts?
>>
>>SCONTROL doesn't enable the interrupts, it enables SynIC as a whole.
>>The interrupts are enabled via individual SINTx msrs.  This SeaBIOS
>>branch does exactly this: it enables the SynIC via SCONTROL, and then
>>specific SynIC functionality via SIMP/SIEFP, but doesn't activate SINTx
>>and works in polling mode.
>>
>>I agree that this global SCONTROL switch seems redundant but it appears
>>to match the spec.
>>
>>>> > (There are a bunch of patches i sent on the QEMU mailing list as well  where
>>>> > i CCed you, I will probably revise it a bit but was hoping to get  KVM
>>>> > sorted out first).
>>>>
>>>> I'll look through the archive, should be there, thanks.
>>>>
>>>> Roman.
>>>
>>>I tried testing with both the SeaBIOS branch you have suggested and the
>>>EDK2, unfortunately I could not get the EDK2 build to identify my VM drive
>>>to boot from (not sure why)
>>>
>>>Here is an output of KVM trace for the relevant MSRs (SCONTROL and SIMP)
>>>
>>>QEMU Default BIOS
>>>-----------------
>>> qemu-system-x86-613   [000] ....  1121.080722: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000080 data 0x0 host 1
>>> qemu-system-x86-613   [000] ....  1121.080722: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0x0 host 1
>>> qemu-system-x86-613   [000] .N..  1121.095592: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000080 data 0x0 host 1
>>> qemu-system-x86-613   [000] .N..  1121.095592: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0x0 host 1
>>>Choose Windows DebugEntry
>>> qemu-system-x86-613   [001] ....  1165.185227: kvm_msr: msr_read 40000083 = 0x0
>>> qemu-system-x86-613   [001] ....  1165.185255: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0xfa1001 host 0
>>> qemu-system-x86-613   [001] ....  1165.185255: kvm_msr: msr_write 40000083 = 0xfa1001
>>> qemu-system-x86-613   [001] ....  1165.193206: kvm_msr: msr_read 40000083 = 0xfa1001
>>> qemu-system-x86-613   [001] ....  1165.193236: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0xfa1000 host 0
>>> qemu-system-x86-613   [001] ....  1165.193237: kvm_msr: msr_write 40000083 = 0xfa1000
>>>
>>>
>>>SeaBIOS hv-scsci
>>>----------------
>>> qemu-system-x86-656   [001] ....  1313.072714: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000080 data 0x0 host 1
>>> qemu-system-x86-656   [001] ....  1313.072714: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0x0 host 1
>>> qemu-system-x86-656   [001] ....  1313.087752: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000080 data 0x0 host 1
>>> qemu-system-x86-656   [001] ....  1313.087752: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0x0 host 1
>>
>>Initialization (host == 1)
>>
>>> qemu-system-x86-656   [001] ....  1313.156675: kvm_msr: msr_read 40000083 = 0x0
>>> qemu-system-x86-656   [001] ....  1313.156680: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0x7fffe001 host 0
>>>Choose Windows DebugEntry
>>
>>I guess this is a bit misplaced timewise, BIOS is still working here
>>
>>> qemu-system-x86-656   [001] ....  1313.156680: kvm_msr: msr_write 40000083 = 0x7fffe001
>>
>>BIOS sets up message page
>>
>>> qemu-system-x86-656   [001] ....  1313.162111: kvm_msr: msr_read 40000080 = 0x0
>>> qemu-system-x86-656   [001] ....  1313.162118: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000080 data 0x1 host 0
>>> qemu-system-x86-656   [001] ....  1313.162119: kvm_msr: msr_write 40000080 = 0x1
>>
>>BIOS activates SCONTROL
>>
>>> qemu-system-x86-656   [001] ....  1313.246758: kvm_msr: msr_read 40000083 = 0x7fffe001
>>> qemu-system-x86-656   [001] ....  1313.246764: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0x0 host 0
>>> qemu-system-x86-656   [001] ....  1313.246764: kvm_msr: msr_write 40000083 = 0x0
>>
>>BIOS clears message page (it's not needed once the VMBus device was
>>brought up)
>>
>>I guess the choice of Windows DebugEntry appeared somewhere here.
>>
>>> qemu-system-x86-656   [001] ....  1348.904727: kvm_msr: msr_read 40000083 = 0x0
>>> qemu-system-x86-656   [001] ....  1348.904771: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0xfa1001 host 0
>>> qemu-system-x86-656   [001] ....  1348.904772: kvm_msr: msr_write 40000083 = 0xfa1001
>>
>>Bootloader (debug stub?) sets up the message page
>>
>>> qemu-system-x86-656   [001] ....  1348.919170: kvm_msr: msr_read 40000083 = 0xfa1001
>>> qemu-system-x86-656   [001] ....  1348.919183: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0xfa1000 host 0
>>> qemu-system-x86-656   [001] ....  1348.919183: kvm_msr: msr_write 40000083 = 0xfa1000
>>
>>Message page is being disabled again.
>>
>>I guess you only filtered SCONTROL and SIMP, skipping e.g. SVERSION,
>>GUEST_OS_ID, HYPERCALL, etc., which are also part of the exchange here.
>>
>
>Right my bad :( if you want I can re-run the test with the others as 
>well (do you need me to?)
>
>>> I could not get the EDK2 setup to work though
>>> (https://src.openvz.org/projects/UP/repos/edk2 branch hv-scsi)
>>>
>>>It does not detect my VM hard drive not sure why (this is how i  configured
>>>it:
>>> -drive file=./win10.qcow2,format=qcow2,if=none,id=drive_disk0 \
>>> -device virtio-blk-pci,drive=drive_disk0 \
>>>
>>>(Is there something special i need to configure it order for it to  work?, I
>>>tried building EDK2 with and without SMM_REQUIRE and  SECURE_BOOT_ENABLE)
>>
>>No special configuration I can think of.
>>
>>>But in general it sounds like there is something I dont fully understand
>>>when SCONTROL is enabled, then a GSI is associated with this SintRoute.
>>>
>>>Then when the guest triggers an EOI via the APIC we will trigger the GSI
>>>notification, which will give us another go on trying to copy the message
>>>into it's slot.
>>
>>Right.
>>
>>>So is it the OS that is in charge of setting the EOI?
>>
>>Yes.
>>
>>>If so then it needs to
>>>be aware of SCONTROL being enabled and just having it left set by the BIOS
>>>might not be enough?
>>
>>Yes it needs to be aware of SCONTROL being enabled.  However, this
>>awareness may be based on a pure assumption that the previous entity
>>(BIOS or bootloader) did it already.
>>
>>>Also in the TLFS (looking at v6) they mention that message queueing has "3
>>>exit conditions", which will cause the hypervisor to try and attempt to
>>>deliver the additional messages.
>>>
>>>The 3 exit conditions they refer to are:
>>>* Another message buffer is queued.
>>>* The guest indicates the “end of interrupt” by writing to the APIC’s   EOI
>>>register.
>>>* The guest indicates the “end of message” by writing to the SynIC’s EOM
>>>register.
>>>
>>>Also notice this additional exit is only if there is a pending message and
>>>not for every EOM.
>>
>>This meaning of "exit" doesn't trivially correspond to what we have in
>>KVM.  A write to an msr does cause a vmexit.  Then KVM notifies resample
>>eventfds for all SINTs that have them set up, no matter if there's a
>>pending message in the slot.  It may be slightly more optimal to only
>>notify those having indicated a pending message, but I don't see the
>>current behavior break anything or violate the spec, so, as EOMs are not
>>used on fast paths, I woudn't bother optimizing.
>>
>>Roman.
>
>Hi Roman,
>
>So based on your answer I got to the following conclusions (correct if 
>they are wrong).
>
>First of the one in charge of setting the SCONTROL in the 1st place is 
>the BIOS (I dont have a real Hyper-V setup so I cannot really debug it 
>and see, not sure which BIOS they have or if we can "rip" it out and 
>run it through KVM and see how things look like this way).
>
>If the BIOS has not set the SCONTROL I would expect the OS to have 
>something along the lines:
>if (!(get_scontrol() & ENABLED))
>    set_scontrol(ENABLED);
>
>So I started looking through the entire Windows system looking what 
>can set SCONTROL, I believe I have found the flow to be the following:
>
>VMBus.sys imports winhv.sys (which is an export library) winhv.sys 
>will set the SCONTROL prior to VMBus DriverEntry starting here is the 
>complete flow:
>winhv!DllInitialize -> winhv!WinHvpInitialize -> 
>winhv!WinHvReportPresentHypervisor -> winhv!WinHvpConnectToSynic -> 
>winhv!WinHvpEnableSynic
>
>Eventually WinHvpEnableSynic will simply set SCONTROL (for future 
>reference if anyone needs to look into how HyperV register access 
>works in Windows it seems like there is an enum representing all the 
>HyperV registers and to access it there are helper functions to 
>Get/Set.
>SCONTROL value in the enum is 0x0a0010 .
>
>winhv.sys simply provides very simple API to access the Sints i.e 
>(WinHvSetSint / WinHvSetEndOfMessage / WinHvSetSintOnCurrentProcessor 
>/  WinHvGetSintMessage / etc.)
>
>So basically it seems like the OS does not really care if the BIOS has 
>setup the SCONTROL or not, and does so always (if it can) 
>unfortunately in my flow (via kdnet) VMBus is not loaded yet and so 
>does winhv.sys so they "fallback" into this Polling mode.
>
>So that covers the OS part, after that I have tried looking for 
>relevant code in bootmgr and winload (which are Windows boot loader 
>part (like grub) and I could not find any code that might setup 
>SCONTROL.
>
>From your experience with this did you see Hyper-V BIOS simply setting 
>the SCONTROL? Perhaps if that's the case then the correct fix needs to 
>be in the SeaBIOS and the EDK .
>
>I tried to see if Hyper-V supports giving it a BIOS but could not find 
>anyway of doing this, so it just might be that Hyper-V assumes the 
>BIOS is in charge of setting up SCONTROL for all the boot loader 
>components.
>
>But in a way it sounds weird because I would expect to see KDNet 
>working with the ACPI to trigger the GSI but I could not find any 
>relevant code that might do that.
>
>As I write this I think I'm starting to get your point just to make 
>sure I understand it:
>
>1. When a new SintRoute is created we associate it with a GSI
>2. When an EOM is set, we trigger all the GSIs so QEMU will get    
>execution time and send all pending messages if it can.
>
>So basically like you said everything "works" from our perspective 
>regardless if the system has setup SCONTROL or not, because you 
>trigger the interrupt to QEMU regardless of SCONTROL so it can clear 
>the pending message.
>
>If that's indeed the case then probably the only thing needs fixing in 
>my scenario is in QEMU where it should not really care for the 
>SCONTROL if it's enabled or not.
>
>Sounds about right?
>
>Thanks,
>-- Jon.

Hi Roman, any chance you can have a quick look at this and see if I 
understood you correctly?

Thanks,
-- Jon.

Re: [PATCH v2 0/1] x86/kvm/hyper-v: Add support to SYNIC exit on EOM

[Roman Kagan]

On Sat, Apr 25, 2020 at 09:16:37AM +0300, Jon Doron wrote:
> On 24/04/2020, Roman Kagan wrote:
> > On Sat, Apr 18, 2020 at 09:41:27AM +0300, Jon Doron wrote:
> > > On 17/04/2020, Roman Kagan wrote:
> > > > On Thu, Apr 16, 2020 at 03:54:30PM +0300, Jon Doron wrote:
> > > > > On 16/04/2020, Roman Kagan wrote:
> > > > > > On Thu, Apr 16, 2020 at 11:38:46AM +0300, Jon Doron wrote:
> > > > > > > According to the TLFS:
> > > > > > > "A write to the end of message (EOM) register by the guest causes the
> > > > > > > hypervisor to scan the internal message buffer queue(s) associated with
> > > > > > > the virtual processor.
> > > > > > >
> > > > > > > If a message buffer queue contains a queued message buffer, the hypervisor
> > > > > > > attempts to deliver the message.
> > > > > > >
> > > > > > > Message delivery succeeds if the SIM page is enabled and the message slot
> > > > > > > corresponding to the SINTx is empty (that is, the message type in the
> > > > > > > header is set to HvMessageTypeNone).
> > > > > > > If a message is successfully delivered, its corresponding internal message
> > > > > > > buffer is dequeued and marked free.
> > > > > > > If the corresponding SINTx is not masked, an edge-triggered interrupt is
> > > > > > > delivered (that is, the corresponding bit in the IRR is set).
> > > > > > >
> > > > > > > This register can be used by guests to poll for messages. It can also be
> > > > > > > used as a way to drain the message queue for a SINTx that has
> > > > > > > been disabled (that is, masked)."
> > > > > >
> > > > > > Doesn't this work already?
> > > > > >
> > > > >
> > > > > Well if you dont have SCONTROL and a GSI associated with the SINT then it
> > > > > does not...
> > > >
> > > > Yes you do need both of these.
> > > >
> > > > > > > So basically this means that we need to exit on EOM so the hypervisor
> > > > > > > will have a chance to send all the pending messages regardless of the
> > > > > > > SCONTROL mechnaisim.
> > > > > >
> > > > > > I might be misinterpreting the spec, but my understanding is that
> > > > > > SCONTROL {en,dis}ables the message queueing completely.  What the quoted
> > > > > > part means is that a write to EOM should trigger the message source to
> > > > > > push a new message into the slot, regardless of whether the SINT was
> > > > > > masked or not.
> > > > > >
> > > > > > And this (I think, haven't tested) should already work.  The userspace
> > > > > > just keeps using the SINT route as it normally does, posting
> > > > > > notifications to the corresponding irqfd when posting a message, and
> > > > > > waiting on the resamplerfd for the message slot to become free.  If the
> > > > > > SINT is masked KVM will skip injecting the interrupt, that's it.
> > > > > >
> > > > > > Roman.
> > > > >
> > > > > That's what I was thinking originally as well, but then i noticed KDNET as a
> > > > > VMBus client (and it basically runs before anything else) is working in this
> > > > > polling mode, where SCONTROL is disabled and it just loops, and if it saw
> > > > > there is a PENDING message flag it will issue an EOM to indicate it has free
> > > > > the slot.
> > > >
> > > > Who sets up the message page then?  Doesn't it enabe SCONTROL as well?
> > > >
> > > 
> > > KdNet is the one setting the SIMP and it's not setting the SCONTROL, ill
> > > paste output of KVM traces for the relevant MSRs
> > > 
> > > > Note that, even if you don't see it being enabled by Windows, it can be
> > > > enabled by the firmware and/or by the bootloader.
> > > >
> > > > Can you perhaps try with the SeaBIOS from
> > > > https://src.openvz.org/projects/UP/repos/seabios branch hv-scsi?  It
> > > > enables SCONTROL and leaves it that way.
> > > >
> > > > I'd also suggest tracing kvm_msr events (both reads and writes) for
> > > > SCONTROL and SIMP msrs, to better understand the picture.
> > > >
> > > > So far the change you propose appears too heavy to work around the
> > > > problem of disabled SCONTROL.  You seem to be better off just making
> > > > sure it's enabled (either by the firmware or slighly violating the spec
> > > > and initializing to enabled from the start), and sticking to the
> > > > existing infrastructure for posting messages.
> > > >
> > > 
> > > I guess there is something I'm missing here but let's say the BIOS would
> > > have set the SCONTROL but the OS is not setting it, who is in charge of
> > > handling the interrupts?
> > 
> > SCONTROL doesn't enable the interrupts, it enables SynIC as a whole.
> > The interrupts are enabled via individual SINTx msrs.  This SeaBIOS
> > branch does exactly this: it enables the SynIC via SCONTROL, and then
> > specific SynIC functionality via SIMP/SIEFP, but doesn't activate SINTx
> > and works in polling mode.
> > 
> > I agree that this global SCONTROL switch seems redundant but it appears
> > to match the spec.
> > 
> > > > > (There are a bunch of patches i sent on the QEMU mailing list as well  where
> > > > > i CCed you, I will probably revise it a bit but was hoping to get  KVM
> > > > > sorted out first).
> > > >
> > > > I'll look through the archive, should be there, thanks.
> > > >
> > > > Roman.
> > > 
> > > I tried testing with both the SeaBIOS branch you have suggested and the
> > > EDK2, unfortunately I could not get the EDK2 build to identify my VM drive
> > > to boot from (not sure why)
> > > 
> > > Here is an output of KVM trace for the relevant MSRs (SCONTROL and SIMP)
> > > 
> > > QEMU Default BIOS
> > > -----------------
> > >  qemu-system-x86-613   [000] ....  1121.080722: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000080 data 0x0 host 1
> > >  qemu-system-x86-613   [000] ....  1121.080722: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0x0 host 1
> > >  qemu-system-x86-613   [000] .N..  1121.095592: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000080 data 0x0 host 1
> > >  qemu-system-x86-613   [000] .N..  1121.095592: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0x0 host 1
> > > Choose Windows DebugEntry
> > >  qemu-system-x86-613   [001] ....  1165.185227: kvm_msr: msr_read 40000083 = 0x0
> > >  qemu-system-x86-613   [001] ....  1165.185255: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0xfa1001 host 0
> > >  qemu-system-x86-613   [001] ....  1165.185255: kvm_msr: msr_write 40000083 = 0xfa1001
> > >  qemu-system-x86-613   [001] ....  1165.193206: kvm_msr: msr_read 40000083 = 0xfa1001
> > >  qemu-system-x86-613   [001] ....  1165.193236: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0xfa1000 host 0
> > >  qemu-system-x86-613   [001] ....  1165.193237: kvm_msr: msr_write 40000083 = 0xfa1000
> > > 
> > > 
> > > SeaBIOS hv-scsci
> > > ----------------
> > >  qemu-system-x86-656   [001] ....  1313.072714: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000080 data 0x0 host 1
> > >  qemu-system-x86-656   [001] ....  1313.072714: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0x0 host 1
> > >  qemu-system-x86-656   [001] ....  1313.087752: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000080 data 0x0 host 1
> > >  qemu-system-x86-656   [001] ....  1313.087752: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0x0 host 1
> > 
> > Initialization (host == 1)
> > 
> > >  qemu-system-x86-656   [001] ....  1313.156675: kvm_msr: msr_read 40000083 = 0x0
> > >  qemu-system-x86-656   [001] ....  1313.156680: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0x7fffe001 host 0
> > > Choose Windows DebugEntry
> > 
> > I guess this is a bit misplaced timewise, BIOS is still working here
> > 
> > >  qemu-system-x86-656   [001] ....  1313.156680: kvm_msr: msr_write 40000083 = 0x7fffe001
> > 
> > BIOS sets up message page
> > 
> > >  qemu-system-x86-656   [001] ....  1313.162111: kvm_msr: msr_read 40000080 = 0x0
> > >  qemu-system-x86-656   [001] ....  1313.162118: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000080 data 0x1 host 0
> > >  qemu-system-x86-656   [001] ....  1313.162119: kvm_msr: msr_write 40000080 = 0x1
> > 
> > BIOS activates SCONTROL
> > 
> > >  qemu-system-x86-656   [001] ....  1313.246758: kvm_msr: msr_read 40000083 = 0x7fffe001
> > >  qemu-system-x86-656   [001] ....  1313.246764: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0x0 host 0
> > >  qemu-system-x86-656   [001] ....  1313.246764: kvm_msr: msr_write 40000083 = 0x0
> > 
> > BIOS clears message page (it's not needed once the VMBus device was
> > brought up)
> > 
> > I guess the choice of Windows DebugEntry appeared somewhere here.
> > 
> > >  qemu-system-x86-656   [001] ....  1348.904727: kvm_msr: msr_read 40000083 = 0x0
> > >  qemu-system-x86-656   [001] ....  1348.904771: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0xfa1001 host 0
> > >  qemu-system-x86-656   [001] ....  1348.904772: kvm_msr: msr_write 40000083 = 0xfa1001
> > 
> > Bootloader (debug stub?) sets up the message page
> > 
> > >  qemu-system-x86-656   [001] ....  1348.919170: kvm_msr: msr_read 40000083 = 0xfa1001
> > >  qemu-system-x86-656   [001] ....  1348.919183: kvm_hv_synic_set_msr: vcpu_id 0 msr 0x40000083 data 0xfa1000 host 0
> > >  qemu-system-x86-656   [001] ....  1348.919183: kvm_msr: msr_write 40000083 = 0xfa1000
> > 
> > Message page is being disabled again.
> > 
> > I guess you only filtered SCONTROL and SIMP, skipping e.g. SVERSION,
> > GUEST_OS_ID, HYPERCALL, etc., which are also part of the exchange here.
> > 
> 
> Right my bad :( if you want I can re-run the test with the others as well
> (do you need me to?)

No, I just wanted to make sure my assumptions are not completely wrong.

> > >  I could not get the EDK2 setup to work though
> > >  (https://src.openvz.org/projects/UP/repos/edk2 branch hv-scsi)
> > > 
> > > It does not detect my VM hard drive not sure why (this is how i  configured
> > > it:
> > >  -drive file=./win10.qcow2,format=qcow2,if=none,id=drive_disk0 \
> > >  -device virtio-blk-pci,drive=drive_disk0 \
> > > 
> > > (Is there something special i need to configure it order for it to  work?, I
> > > tried building EDK2 with and without SMM_REQUIRE and  SECURE_BOOT_ENABLE)
> > 
> > No special configuration I can think of.
> > 
> > > But in general it sounds like there is something I dont fully understand
> > > when SCONTROL is enabled, then a GSI is associated with this SintRoute.
> > > 
> > > Then when the guest triggers an EOI via the APIC we will trigger the GSI
> > > notification, which will give us another go on trying to copy the message
> > > into it's slot.
> > 
> > Right.
> > 
> > > So is it the OS that is in charge of setting the EOI?
> > 
> > Yes.
> > 
> > > If so then it needs to
> > > be aware of SCONTROL being enabled and just having it left set by the BIOS
> > > might not be enough?
> > 
> > Yes it needs to be aware of SCONTROL being enabled.  However, this
> > awareness may be based on a pure assumption that the previous entity
> > (BIOS or bootloader) did it already.
> > 
> > > Also in the TLFS (looking at v6) they mention that message queueing has "3
> > > exit conditions", which will cause the hypervisor to try and attempt to
> > > deliver the additional messages.
> > > 
> > > The 3 exit conditions they refer to are:
> > > * Another message buffer is queued.
> > > * The guest indicates the “end of interrupt” by writing to the APIC’s   EOI
> > > register.
> > > * The guest indicates the “end of message” by writing to the SynIC’s EOM
> > > register.
> > > 
> > > Also notice this additional exit is only if there is a pending message and
> > > not for every EOM.
> > 
> > This meaning of "exit" doesn't trivially correspond to what we have in
> > KVM.  A write to an msr does cause a vmexit.  Then KVM notifies resample
> > eventfds for all SINTs that have them set up, no matter if there's a
> > pending message in the slot.  It may be slightly more optimal to only
> > notify those having indicated a pending message, but I don't see the
> > current behavior break anything or violate the spec, so, as EOMs are not
> > used on fast paths, I woudn't bother optimizing.
> 
> So based on your answer I got to the following conclusions (correct if they
> are wrong).
> 
> First of the one in charge of setting the SCONTROL in the 1st place is the
> BIOS (I dont have a real Hyper-V setup so I cannot really debug it and see,
> not sure which BIOS they have or if we can "rip" it out and run it through
> KVM and see how things look like this way).
> 
> If the BIOS has not set the SCONTROL I would expect the OS to have something
> along the lines:
> if (!(get_scontrol() & ENABLED))
>     set_scontrol(ENABLED);
> 
> So I started looking through the entire Windows system looking what can set
> SCONTROL, I believe I have found the flow to be the following:
> 
> VMBus.sys imports winhv.sys (which is an export library) winhv.sys will set
> the SCONTROL prior to VMBus DriverEntry starting here is the complete flow:
> winhv!DllInitialize -> winhv!WinHvpInitialize ->
> winhv!WinHvReportPresentHypervisor -> winhv!WinHvpConnectToSynic ->
> winhv!WinHvpEnableSynic
> 
> Eventually WinHvpEnableSynic will simply set SCONTROL (for future reference
> if anyone needs to look into how HyperV register access works in Windows it
> seems like there is an enum representing all the HyperV registers and to
> access it there are helper functions to Get/Set.
> SCONTROL value in the enum is 0x0a0010 .
> 
> winhv.sys simply provides very simple API to access the Sints i.e
> (WinHvSetSint / WinHvSetEndOfMessage / WinHvSetSintOnCurrentProcessor /
> WinHvGetSintMessage / etc.)
> 
> So basically it seems like the OS does not really care if the BIOS has setup
> the SCONTROL or not, and does so always (if it can) unfortunately in my flow
> (via kdnet) VMBus is not loaded yet and so does winhv.sys so they "fallback"
> into this Polling mode.
> 
> So that covers the OS part, after that I have tried looking for relevant
> code in bootmgr and winload (which are Windows boot loader part (like grub)
> and I could not find any code that might setup SCONTROL.
> 
> From your experience with this did you see Hyper-V BIOS simply setting the
> SCONTROL? Perhaps if that's the case then the correct fix needs to be in the
> SeaBIOS and the EDK .

All this makes perfect sense to me.  Unfortunately it's hard to reason
about the supposed interaction of the components when all you have at
hand are opensource guests on Hyper-V and Windows on an opensource
hypervisor.

E.g. the SeaBIOS code contains a kludge to disable SIMP as soon as vmbus
setup and device enumeration is complete, because otherwise Windows 2016
skips activating the message page, and the hypervisor happily writes to
the stale message page set up by the BIOS.

> I tried to see if Hyper-V supports giving it a BIOS but could not find
> anyway of doing this, so it just might be that Hyper-V assumes the BIOS is
> in charge of setting up SCONTROL for all the boot loader components.

I didn't manage to get Hyper-V load a custom BIOS myself.  However I
think you should be able to tell the state the MSRs are left in by the
BIOS on Hyper-V with a custom Linux that would log it prior to
adjusting.  I don't have ready access to a Hyper-V machine so can't help
here.

> But in a way it sounds weird because I would expect to see KDNet working
> with the ACPI to trigger the GSI but I could not find any relevant code that
> might do that.

To the best of my knowledge SINTs aren't registered with ACPI.

> As I write this I think I'm starting to get your point just to make sure I
> understand it:
> 
> 1. When a new SintRoute is created we associate it with a GSI

Right

> 2. When an EOM is set, we trigger all the GSIs so QEMU will get    execution
> time and send all pending messages if it can.

It's in the opposite direction: when EOM is written by the guest, this
triggers the resampler eventfds associated with the GSIs.  As a result,
QEMU gets notified and may retry posting respective messages.  But yes,
the general idea is right.

> So basically like you said everything "works" from our perspective
> regardless if the system has setup SCONTROL or not, because you trigger the
> interrupt to QEMU regardless of SCONTROL so it can clear the pending
> message.

This is how it works in KVM, indeed.

> If that's indeed the case then probably the only thing needs fixing in my
> scenario is in QEMU where it should not really care for the SCONTROL if it's
> enabled or not.

Right.  However, even this shouldn't be necessary as SeaBIOS from that
branch would enable SCONTROL and leave it that way when passing the
control over to the bootloader, so, unless something explicitly clears
SCONTROL, it should remain set thereafter.  I'd rather try going ahead
with that scheme first, because making QEMU ignore SCONTROL appears to
violate the spec.

Thanks,
Roman.

Re: [PATCH v2 0/1] x86/kvm/hyper-v: Add support to SYNIC exit on EOM

[Vitaly Kuznetsov]

Roman Kagan <rvkagan@yandex-team.ru> writes:

> On Sat, Apr 25, 2020 at 09:16:37AM +0300, Jon Doron wrote:
>
>> If that's indeed the case then probably the only thing needs fixing in my
>> scenario is in QEMU where it should not really care for the SCONTROL if it's
>> enabled or not.
>
> Right.  However, even this shouldn't be necessary as SeaBIOS from that
> branch would enable SCONTROL and leave it that way when passing the
> control over to the bootloader, so, unless something explicitly clears
> SCONTROL, it should remain set thereafter.  I'd rather try going ahead
> with that scheme first, because making QEMU ignore SCONTROL appears to
> violate the spec.

FWIW, I just checked 'genuine' Hyper-V 2016 with

diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
index fd51bac11b46..c5ea759728d9 100644
--- a/arch/x86/hyperv/hv_init.c
+++ b/arch/x86/hyperv/hv_init.c
@@ -314,10 +314,14 @@ void __init hyperv_init(void)
        u64 guest_id, required_msrs;
        union hv_x64_msr_hypercall_contents hypercall_msr;
        int cpuhp, i;
+       u64 val;
 
        if (x86_hyper_type != X86_HYPER_MS_HYPERV)
                return;
 
+       hv_get_synic_state(val);
+       printk("Hyper-V: SCONTROL state: %llx\n", val);
+
        /* Absolutely required MSRs */
        required_msrs = HV_X64_MSR_HYPERCALL_AVAILABLE |
                HV_X64_MSR_VP_INDEX_AVAILABLE;


and it seems the default state of HV_X64_MSR_SCONTROL is '1', we should
probably do the same. Is there any reason to *not* do this in KVM when
KVM_CAP_HYPERV_SYNIC[,2] is enabled?

-- 
Vitaly

Re: [PATCH v2 0/1] x86/kvm/hyper-v: Add support to SYNIC exit on EOM

[Roman Kagan]

On Mon, May 04, 2020 at 05:55:10PM +0200, Vitaly Kuznetsov wrote:
> Roman Kagan <rvkagan@yandex-team.ru> writes:
> 
> > On Sat, Apr 25, 2020 at 09:16:37AM +0300, Jon Doron wrote:
> >
> >> If that's indeed the case then probably the only thing needs fixing in my
> >> scenario is in QEMU where it should not really care for the SCONTROL if it's
> >> enabled or not.
> >
> > Right.  However, even this shouldn't be necessary as SeaBIOS from that
> > branch would enable SCONTROL and leave it that way when passing the
> > control over to the bootloader, so, unless something explicitly clears
> > SCONTROL, it should remain set thereafter.  I'd rather try going ahead
> > with that scheme first, because making QEMU ignore SCONTROL appears to
> > violate the spec.
> 
> FWIW, I just checked 'genuine' Hyper-V 2016 with
> 
> diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
> index fd51bac11b46..c5ea759728d9 100644
> --- a/arch/x86/hyperv/hv_init.c
> +++ b/arch/x86/hyperv/hv_init.c
> @@ -314,10 +314,14 @@ void __init hyperv_init(void)
>         u64 guest_id, required_msrs;
>         union hv_x64_msr_hypercall_contents hypercall_msr;
>         int cpuhp, i;
> +       u64 val;
>  
>         if (x86_hyper_type != X86_HYPER_MS_HYPERV)
>                 return;
>  
> +       hv_get_synic_state(val);
> +       printk("Hyper-V: SCONTROL state: %llx\n", val);
> +
>         /* Absolutely required MSRs */
>         required_msrs = HV_X64_MSR_HYPERCALL_AVAILABLE |
>                 HV_X64_MSR_VP_INDEX_AVAILABLE;

Thanks for having done this check!

> and it seems the default state of HV_X64_MSR_SCONTROL is '1', we should
> probably do the same.

This is the state the OS sees, after the firmware.  You'd see the same
with QEMU/KVM if you used Hyper-V-aware SeaBIOS or OVMF.

> Is there any reason to *not* do this in KVM when
> KVM_CAP_HYPERV_SYNIC[,2] is enabled?

Yes there is: quoting Hyper-V TLFS v6.0 11.8.1:

  At virtual processor creation time and upon processor reset, the value
  of this SCONTROL (SynIC control register) is 0x0000000000000000. Thus,
  message queuing and event flag notifications will be disabled.

And, even if we decide to violate the spec it's better done in
userspace, loading the initial value and adjusting the synic state at
vcpu reset.

However leaving it up to the guest (firmware or OS) looks more natural
to me.

Thanks,
Roman.

Re: [PATCH v2 0/1] x86/kvm/hyper-v: Add support to SYNIC exit on EOM

[Jon Doron]

On 05/05/2020, Roman Kagan wrote:
>On Mon, May 04, 2020 at 05:55:10PM +0200, Vitaly Kuznetsov wrote:
>> Roman Kagan <rvkagan@yandex-team.ru> writes:
>>
>> > On Sat, Apr 25, 2020 at 09:16:37AM +0300, Jon Doron wrote:
>> >
>> >> If that's indeed the case then probably the only thing needs fixing in my
>> >> scenario is in QEMU where it should not really care for the SCONTROL if it's
>> >> enabled or not.
>> >
>> > Right.  However, even this shouldn't be necessary as SeaBIOS from that
>> > branch would enable SCONTROL and leave it that way when passing the
>> > control over to the bootloader, so, unless something explicitly clears
>> > SCONTROL, it should remain set thereafter.  I'd rather try going ahead
>> > with that scheme first, because making QEMU ignore SCONTROL appears to
>> > violate the spec.
>>
>> FWIW, I just checked 'genuine' Hyper-V 2016 with
>>
>> diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
>> index fd51bac11b46..c5ea759728d9 100644
>> --- a/arch/x86/hyperv/hv_init.c
>> +++ b/arch/x86/hyperv/hv_init.c
>> @@ -314,10 +314,14 @@ void __init hyperv_init(void)
>>         u64 guest_id, required_msrs;
>>         union hv_x64_msr_hypercall_contents hypercall_msr;
>>         int cpuhp, i;
>> +       u64 val;
>>
>>         if (x86_hyper_type != X86_HYPER_MS_HYPERV)
>>                 return;
>>
>> +       hv_get_synic_state(val);
>> +       printk("Hyper-V: SCONTROL state: %llx\n", val);
>> +
>>         /* Absolutely required MSRs */
>>         required_msrs = HV_X64_MSR_HYPERCALL_AVAILABLE |
>>                 HV_X64_MSR_VP_INDEX_AVAILABLE;
>
>Thanks for having done this check!
>
>> and it seems the default state of HV_X64_MSR_SCONTROL is '1', we should
>> probably do the same.
>
>This is the state the OS sees, after the firmware.  You'd see the same
>with QEMU/KVM if you used Hyper-V-aware SeaBIOS or OVMF.
>
>> Is there any reason to *not* do this in KVM when
>> KVM_CAP_HYPERV_SYNIC[,2] is enabled?
>
>Yes there is: quoting Hyper-V TLFS v6.0 11.8.1:
>
>  At virtual processor creation time and upon processor reset, the value
>  of this SCONTROL (SynIC control register) is 0x0000000000000000. Thus,
>  message queuing and event flag notifications will be disabled.
>
>And, even if we decide to violate the spec it's better done in
>userspace, loading the initial value and adjusting the synic state at
>vcpu reset.
>
>However leaving it up to the guest (firmware or OS) looks more natural
>to me.
>
>Thanks,
>Roman.

I under where you are coming from in the idea of leaving it to the OS 
but I think in this specific case it does not make much sense, after all 
HyperV has it's own proprietary BIOS which Windows assumes has setup 
some of the MSRs, since we dont have that BIOS we need to "emulate" it's 
behaviour.

I also feel like the best approach should be in QEMU in case VMBus 
device exists it will also setup the SCONTROL to ENABLED, this way you 
are not bound to have a special BIOS in case you have decided to use 
HyperV advanced features like VMBus.

Cheers,
-- Jon.

Re: [PATCH v2 0/1] x86/kvm/hyper-v: Add support to SYNIC exit on EOM

[Roman Kagan]

On Tue, May 05, 2020 at 01:38:21PM +0300, Jon Doron wrote:
> On 05/05/2020, Roman Kagan wrote:
> > On Mon, May 04, 2020 at 05:55:10PM +0200, Vitaly Kuznetsov wrote:
> > > and it seems the default state of HV_X64_MSR_SCONTROL is '1', we should
> > > probably do the same.
> > 
> > This is the state the OS sees, after the firmware.  You'd see the same
> > with QEMU/KVM if you used Hyper-V-aware SeaBIOS or OVMF.
> > 
> > > Is there any reason to *not* do this in KVM when
> > > KVM_CAP_HYPERV_SYNIC[,2] is enabled?
> > 
> > Yes there is: quoting Hyper-V TLFS v6.0 11.8.1:
> > 
> >  At virtual processor creation time and upon processor reset, the value
> >  of this SCONTROL (SynIC control register) is 0x0000000000000000. Thus,
> >  message queuing and event flag notifications will be disabled.
> > 
> > And, even if we decide to violate the spec it's better done in
> > userspace, loading the initial value and adjusting the synic state at
> > vcpu reset.
> > 
> > However leaving it up to the guest (firmware or OS) looks more natural
> > to me.
> 
> I under where you are coming from in the idea of leaving it to the OS

I'm coming from the HyperV spec, see the quote above.

> but I think in this specific case it does not make much sense, after
> all HyperV has it's own proprietary BIOS which Windows assumes has
> setup some of the MSRs, since we dont have that BIOS we need to
> "emulate" it's behaviour.

We don't have that BIOS, but we have another BIOS which does the same
and is not proprietary.  Using it allows to do synic message posting
even with a non-compliant guest OS which doesn't properly enable
SCONTROL on its own.  (Note that there used to be no problem with this
so far, this must be specific to your use case.)

I'm failing to see why this is a stumbling block for the work you're
doing.

And I'm not convinced we need to work around a non-compliant guest with
kludges to KVM or QEMU (including back-compat stuff as that would change
the existing behavior), when the desired effect can be achieved with the
existing code.

Thanks,
Roman.

Re: [PATCH v2 0/1] x86/kvm/hyper-v: Add support to SYNIC exit on EOM

[Jon Doron]

On 05/05/2020, Roman Kagan wrote:
>On Tue, May 05, 2020 at 01:38:21PM +0300, Jon Doron wrote:
>> On 05/05/2020, Roman Kagan wrote:
>> > On Mon, May 04, 2020 at 05:55:10PM +0200, Vitaly Kuznetsov wrote:
>> > > and it seems the default state of HV_X64_MSR_SCONTROL is '1', we should
>> > > probably do the same.
>> >
>> > This is the state the OS sees, after the firmware.  You'd see the same
>> > with QEMU/KVM if you used Hyper-V-aware SeaBIOS or OVMF.
>> >
>> > > Is there any reason to *not* do this in KVM when
>> > > KVM_CAP_HYPERV_SYNIC[,2] is enabled?
>> >
>> > Yes there is: quoting Hyper-V TLFS v6.0 11.8.1:
>> >
>> >  At virtual processor creation time and upon processor reset, the value
>> >  of this SCONTROL (SynIC control register) is 0x0000000000000000. Thus,
>> >  message queuing and event flag notifications will be disabled.
>> >
>> > And, even if we decide to violate the spec it's better done in
>> > userspace, loading the initial value and adjusting the synic state at
>> > vcpu reset.
>> >
>> > However leaving it up to the guest (firmware or OS) looks more natural
>> > to me.
>>
>> I under where you are coming from in the idea of leaving it to the OS
>
>I'm coming from the HyperV spec, see the quote above.
>
>> but I think in this specific case it does not make much sense, after
>> all HyperV has it's own proprietary BIOS which Windows assumes has
>> setup some of the MSRs, since we dont have that BIOS we need to
>> "emulate" it's behaviour.
>
>We don't have that BIOS, but we have another BIOS which does the same
>and is not proprietary.  Using it allows to do synic message posting
>even with a non-compliant guest OS which doesn't properly enable
>SCONTROL on its own.  (Note that there used to be no problem with this
>so far, this must be specific to your use case.)
>
>I'm failing to see why this is a stumbling block for the work you're
>doing.
>
>And I'm not convinced we need to work around a non-compliant guest with
>kludges to KVM or QEMU (including back-compat stuff as that would change
>the existing behavior), when the desired effect can be achieved with the
>existing code.
>
>Thanks,
>Roman.

Thanks Roman, I see your point, it's important for me to get the EDK2 
working properly not sure why it's not working for me.

Do you know by any chance if the EDK2 hyperv patches were submitted and 
if they were why they were not merged in?

Thanks,
-- Jon.

Re: [PATCH v2 0/1] x86/kvm/hyper-v: Add support to SYNIC exit on EOM

[Roman Kagan]

On Wed, May 06, 2020 at 07:49:29AM +0300, Jon Doron wrote:
> Thanks Roman, I see your point, it's important for me to get the EDK2
> working properly not sure why it's not working for me.

As I wrote a good deal of that code I hope I should be able to help (and
I'd be interested, too).  How exactly does the "not working" look like?

Also I'm a bit confused as to why UEFI is critical for the work you're
doing?  Can't it be made to work with BIOS first?

> Do you know by any chance if the EDK2 hyperv patches were submitted and if
> they were why they were not merged in?

I do, as I'm probably the only one who could have submitted them :)

No they were not submitted.  Neither were the ones for SeaBIOS nor iPXE.
The reason was that I had found no way to use alternative firmware with
HyperV, so the only environment where that would be useful and testable
was QEMU with VMBus.  Therefore I thought it made no sense to submit
them until VMBus landed in QEMU.

Thanks,
Roman.

Re: [PATCH v2 0/1] x86/kvm/hyper-v: Add support to SYNIC exit on EOM

[Jon Doron]

On 06/05/2020, Roman Kagan wrote:
>On Wed, May 06, 2020 at 07:49:29AM +0300, Jon Doron wrote:
>> Thanks Roman, I see your point, it's important for me to get the EDK2
>> working properly not sure why it's not working for me.
>
>As I wrote a good deal of that code I hope I should be able to help (and
>I'd be interested, too).  How exactly does the "not working" look like?
>

Basically when I built the BIOS from the hv-scsi branch you pointed me 
out to, the BIOS did not see the virtio-blk device to boot from, I 
usually take the BIOS from (https://www.kraxel.org/repos/) but I will 
try to build the latest EDK2 and see if it identifies the virtio-blk 
device and boots from it, if that's the case perhaps i just need to 
rebase your branch over the latest master of EDK2.

>Also I'm a bit confused as to why UEFI is critical for the work you're
>doing?  Can't it be made to work with BIOS first?
>

The reason I want to have the UEFI option is because I need SecureBoot 
to turn on VBS.

>> Do you know by any chance if the EDK2 hyperv patches were submitted and if
>> they were why they were not merged in?
>
>I do, as I'm probably the only one who could have submitted them :)
>
>No they were not submitted.  Neither were the ones for SeaBIOS nor iPXE.
>The reason was that I had found no way to use alternative firmware with
>HyperV, so the only environment where that would be useful and testable
>was QEMU with VMBus.  Therefore I thought it made no sense to submit
>them until VMBus landed in QEMU.
>
>Thanks,
>Roman.

Heh I see, well I'm really happy that you are here helping so we can try 
and finally add VMBus to QEMU, I realize it's a big effort but I'm 
willing to spend the time and do the required changes...

I'm working this only during my free time so things takes me longer than 
usual (sorry for that..)

I will keep update on results once I get to test with latest EDK2 :) 

Thanks,
-- Jon.

Re: [PATCH v2 0/1] x86/kvm/hyper-v: Add support to SYNIC exit on EOM

[Jon Doron]

On 07/05/2020, Jon Doron wrote:
>On 06/05/2020, Roman Kagan wrote:
>>On Wed, May 06, 2020 at 07:49:29AM +0300, Jon Doron wrote:
>>>Thanks Roman, I see your point, it's important for me to get the EDK2
>>>working properly not sure why it's not working for me.
>>
>>As I wrote a good deal of that code I hope I should be able to help (and
>>I'd be interested, too).  How exactly does the "not working" look like?
>>
>
>Basically when I built the BIOS from the hv-scsi branch you pointed me 
>out to, the BIOS did not see the virtio-blk device to boot from, I 
>usually take the BIOS from (https://www.kraxel.org/repos/) but I will 
>try to build the latest EDK2 and see if it identifies the virtio-blk 
>device and boots from it, if that's the case perhaps i just need to 
>rebase your branch over the latest master of EDK2.
>
>>Also I'm a bit confused as to why UEFI is critical for the work you're
>>doing?  Can't it be made to work with BIOS first?
>>
>
>The reason I want to have the UEFI option is because I need SecureBoot 
>to turn on VBS.
>
>>>Do you know by any chance if the EDK2 hyperv patches were submitted and if
>>>they were why they were not merged in?
>>
>>I do, as I'm probably the only one who could have submitted them :)
>>
>>No they were not submitted.  Neither were the ones for SeaBIOS nor iPXE.
>>The reason was that I had found no way to use alternative firmware with
>>HyperV, so the only environment where that would be useful and testable
>>was QEMU with VMBus.  Therefore I thought it made no sense to submit
>>them until VMBus landed in QEMU.
>>
>>Thanks,
>>Roman.
>
>Heh I see, well I'm really happy that you are here helping so we can 
>try and finally add VMBus to QEMU, I realize it's a big effort but I'm 
>willing to spend the time and do the required changes...
>
>I'm working this only during my free time so things takes me longer 
>than usual (sorry for that..)
>
>I will keep update on results once I get to test with latest EDK2 :)
>
>Thanks,
>-- Jon.

Hi, just wanted to update you I did some stupid mistake when I did the 
UEFI setup test (that's why I could not boot my Win10).

I suggest we will abandon this patch, and try to keep going on the QEMU 
VMBus patchset.

And perhaps submit a very basic patch to SeaBIOS and EDK2 which just 
enable SCONTROL.

Does that sound like a good plan to you?

Thanks,
-- Jon.

Re: [PATCH v2 0/1] x86/kvm/hyper-v: Add support to SYNIC exit on EOM

[Roman Kagan]

On Fri, May 08, 2020 at 05:29:54PM +0300, Jon Doron wrote:
> Hi, just wanted to update you I did some stupid mistake when I did the UEFI
> setup test (that's why I could not boot my Win10).
> 
> I suggest we will abandon this patch, and try to keep going on the QEMU
> VMBus patchset.
> 
> And perhaps submit a very basic patch to SeaBIOS and EDK2 which just enable
> SCONTROL.
> 
> Does that sound like a good plan to you?

Absolutely.

Thanks,
Roman.