Re: [PATCH 1/2] kvm/x86: Hyper-V synthetic interrupt controller

2015-10-09 Thread Paolo Bonzini


On 09/10/2015 17:53, Roman Kagan wrote:
> > I really don't like this auto-EOI extension, but I guess that's the
> > spec. :( If it wasn't for it, you could do everything very easily in
> > userspace using Google's proposed MSR exit.
> I guess you're right.  We'd probably have to (ab)use MSI for SINT
> delivery, though.

Not really an issue, as MSI on x86 is really just the external entry
point into the LAPIC, it makes sense that it be the external interface
into KVM's virtualized LAPIC.  Userspace split irqchip is (ab)using MSI
routes the same way.

> Anyway the need to implement auto-EOI rules that out.

Yup.  I look forward to reviewing v2!

Paolo
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH 2/2] kvm/x86: Hyper-V kvm exit

2015-10-09 Thread Paolo Bonzini


On 09/10/2015 16:53, Roman Kagan wrote:
>> > Why is this exit necessary?
> The guest writes to synic-related MSRs and that should take "immediate"
> effect.
> 
> E.g. it may decide to disable or relocate the message page by writing to
> SIMP MSR.  The host is then supposed to stop accessing the old message
> page before the vCPU proceeds to the next instruction.  Hence the exit,
> to allow the userspace to react accordingly before reentering the guest.

Ok, thanks!

Paolo
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH 1/2] kvm/x86: Hyper-V synthetic interrupt controller

2015-10-09 Thread Paolo Bonzini
Christian, the question for you is towards the end...

On 09/10/2015 15:39, Denis V. Lunev wrote:
> diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
> index 62cf8c9..15c3c02 100644
> --- a/arch/x86/kvm/hyperv.c
> +++ b/arch/x86/kvm/hyperv.c
> @@ -23,13 +23,265 @@
>  
>  #include "x86.h"
>  #include "lapic.h"
> +#include "ioapic.h"
>  #include "hyperv.h"
>  
>  #include 
> +#include 
>  #include 
>  
>  #include "trace.h"
>  
> +static inline u64 synic_read_sint(struct kvm_vcpu_hv_synic *synic, int sint)
> +{
> + return atomic64_read(&synic->sint[sint]);
> +}
> +
> +static inline int synic_get_sint_vector(u64 sint_value)
> +{
> + if (sint_value & HV_SYNIC_SINT_MASKED)
> + return -1;
> + return sint_value & HV_SYNIC_SINT_VECTOR_MASK;
> +}
> +
> +static bool synic_has_active_vector(struct kvm_vcpu_hv_synic *synic,
> + int vector, int sint_to_skip, int sint_mask)
> +{
> + u64 sint_value;
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(synic->sint); i++) {
> + if (i == sint_to_skip)
> + continue;
> + sint_value = synic_read_sint(synic, i);
> + if ((synic_get_sint_vector(sint_value) == vector) &&
> + ((sint_mask == 0) || (sint_value & sint_mask)))

Coding style, no parentheses around && or ||:

if (synic_get_sint_vector(sint_value) == vector &&
(sint_mask == 0 || sint_value & sint_mask)


> + return true;
> + }
> + return false;
> +}
> +
> +static int synic_set_sint(struct kvm_vcpu_hv_synic *synic, int sint, u64 
> data)
> +{
> + int vector;
> +
> + vector = data & HV_SYNIC_SINT_VECTOR_MASK;
> + if (vector < 16)
> + return 1;
> + /*
> +  * Guest may configure multiple SINTs to use the same vector, so
> +  * we maintain a bitmap of vectors handled by synic, and a
> +  * bitmap of vectors with auto-eoi behavoir.  The bitmaps are

Typo (behavior).

> +  * updated here, and atomically queried on fast paths.
> +  */
> +
> + if (!(data & HV_SYNIC_SINT_MASKED)) {
> + __set_bit(vector, synic->vec_bitmap);
> + if (data & HV_SYNIC_SINT_AUTO_EOI)
> + __set_bit(vector, synic->auto_eoi_bitmap);
> + } else {
> + if (!synic_has_active_vector(synic, vector, sint, 0))
> + __clear_bit(vector, synic->vec_bitmap);
> + if (!synic_has_active_vector(synic, vector, sint,
> +  HV_SYNIC_SINT_AUTO_EOI))
> + __clear_bit(vector, synic->auto_eoi_bitmap);

I think you could do the clears after the atomic64_set?  Then you do not
need anymore the third argument to synic_has_active_vector.  Actually I
think it's simpler if you just make two functions,
synic_is_vector_connected and synic_is_vector_auto_eoi.  There is some
code duplication, but the functions are trivial.

> @@ -123,6 +125,15 @@ int kvm_set_msi(struct kvm_kernel_irq_routing_entry *e,
>   return kvm_irq_delivery_to_apic(kvm, NULL, &irq, NULL);
>  }
>  
> +int kvm_hv_set_sint(struct kvm_kernel_irq_routing_entry *e,
> + struct kvm *kvm, int irq_source_id, int level,
> + bool line_status)
> +{
> + if (!level)
> + return -1;
> +
> + return kvm_hv_synic_set_irq(kvm, e->hv_sint.vcpu, e->hv_sint.sint);
> +}
>  
>  static int kvm_set_msi_inatomic(struct kvm_kernel_irq_routing_entry *e,
>struct kvm *kvm)
> @@ -289,6 +300,11 @@ int kvm_set_routing_entry(struct 
> kvm_kernel_irq_routing_entry *e,
>   e->msi.address_hi = ue->u.msi.address_hi;
>   e->msi.data = ue->u.msi.data;
>   break;
> + case KVM_IRQ_ROUTING_HV_SINT:
> + e->set = kvm_hv_set_sint;
> + e->hv_sint.vcpu = ue->u.hv_sint.vcpu;
> + e->hv_sint.sint = ue->u.hv_sint.sint;
> + break;
>   default:
>   goto out;
>   }
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index 944b38a..63edbec 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -41,6 +41,7 @@
>  #include "trace.h"
>  #include "x86.h"
>  #include "cpuid.h"
> +#include "hyperv.h"
>  
>  #ifndef CONFIG_X86_64
>  #define mod_64(x, y) ((x) - (y) * div64_u64(x, y))
> @@ -128,11 +129,6 @@ static inline int apic_enabled(struct kvm_lapic *apic)
>   (LVT_MASK | APIC_MODE_MASK | APIC_INPUT_POLARITY | \
>APIC_LVT_REMOTE_IRR | APIC_LVT_LEVEL_TRIGGER)
>  
> -static inline int kvm_apic_id(struct kvm_lapic *apic)
> -{
> - return (kvm_apic_get_reg(apic, APIC_ID) >> 24) & 0xff;
> -}
> -
>  /* The logical map is definitely wrong if we have multiple
>   * modes at the same time.  (Physical map is always right.)
>   */
> @@ -972,6 +968,9 @@ static int apic_set_eoi(struct kvm_lapic *apic)
>   apic_clear_isr(vector, apic);
>   apic_update_ppr(apic);
>  

Re: [PATCH 2/2] kvm/x86: Hyper-V kvm exit

2015-10-09 Thread Paolo Bonzini


On 09/10/2015 15:39, Denis V. Lunev wrote:
> From: Andrey Smetanin 
> 
> A new vcpu exit is introduced to notify the userspace of the
> changes in Hyper-V synic configuraion triggered by guest writing to the
> corresponding MSRs.
> 
> Signed-off-by: Andrey Smetanin 
> Reviewed-by: Roman Kagan 
> Signed-off-by: Denis V. Lunev 
> CC: Vitaly Kuznetsov 
> CC: "K. Y. Srinivasan" 
> CC: Gleb Natapov 
> CC: Paolo Bonzini 

Why is this exit necessary?

Paolo
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


[PATCH 0/2] Hyper-V synthetic interrupt controller

2015-10-09 Thread Denis V. Lunev
This patchset implements the KVM part of the synthetic interrupt
controller (synic) which is a building block of the Hyper-V
paravirtualized device bus (vmbus).

Synic is a lapic extension, which is controlled via MSRs and maintains
for each vCPU
 - 16 synthetic interrupt "lines" (SINT's); each can be configured to
   trigger a specific interrupt vector optionally with auto-EOI
   semantics
 - a message page in the guest memory with 16 256-byte per-SINT message
   slots
 - an event flag page in the guest memory with 16 2048-bit per-SINT
   event flag areas

The host triggers a SINT whenever it delivers a new message to the
corresponding slot or flips an event flag bit in the corresponding area.
The guest informs the host that it can try delivering a message by
explicitly asserting EOI in lapic or writing to End-Of-Message (EOM)
MSR.

The userspace (qemu) triggers interrupts and receives EOM notifications
via irqfd with resampler; for that, a GSI is allocated for each
configured SINT, and irq_routing api is extended to support GSI-SINT
mapping.

Besides, a new vcpu exit is introduced to notify the userspace of the
changes in synic configuraion triggered by guest writing to the
corresponding MSRs.

Signed-off-by: Andrey Smetanin 
Reviewed-by: Roman Kagan 
Signed-off-by: Denis V. Lunev 
CC: Vitaly Kuznetsov 
CC: "K. Y. Srinivasan" 
CC: Gleb Natapov 
CC: Paolo Bonzini 

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


[PATCH 2/2] kvm/x86: Hyper-V kvm exit

2015-10-09 Thread Denis V. Lunev
From: Andrey Smetanin 

A new vcpu exit is introduced to notify the userspace of the
changes in Hyper-V synic configuraion triggered by guest writing to the
corresponding MSRs.

Signed-off-by: Andrey Smetanin 
Reviewed-by: Roman Kagan 
Signed-off-by: Denis V. Lunev 
CC: Vitaly Kuznetsov 
CC: "K. Y. Srinivasan" 
CC: Gleb Natapov 
CC: Paolo Bonzini 
---
 Documentation/virtual/kvm/api.txt |  6 ++
 arch/x86/include/asm/kvm_host.h   |  1 +
 arch/x86/kvm/hyperv.c | 17 +
 arch/x86/kvm/x86.c|  6 ++
 include/linux/kvm_host.h  |  1 +
 include/uapi/linux/kvm.h  | 17 +
 6 files changed, 48 insertions(+)

diff --git a/Documentation/virtual/kvm/api.txt 
b/Documentation/virtual/kvm/api.txt
index 34cc068..cffe670 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -3331,6 +3331,12 @@ the userspace IOAPIC should process the EOI and 
retrigger the interrupt if
 it is still asserted.  Vector is the LAPIC interrupt vector for which the
 EOI was received.
 
+   /* KVM_EXIT_HYPERV */
+struct kvm_hyperv_exit hyperv;
+Indicates that the VCPU's exits into userspace to process some tasks
+related with Hyper-V emulation. Currently used to synchronize modified
+Hyper-V synic state with userspace.
+
/* Fix the size of the union. */
char padding[256];
};
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index e614a543..f515e01 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -392,6 +392,7 @@ struct kvm_vcpu_hv {
u64 hv_vapic;
s64 runtime_offset;
struct kvm_vcpu_hv_synic synic;
+   struct kvm_hyperv_exit exit;
 };
 
 struct kvm_vcpu_arch {
diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index 15c3c02..174ce041 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -91,6 +91,20 @@ static int synic_set_sint(struct kvm_vcpu_hv_synic *synic, 
int sint, u64 data)
return 0;
 }
 
+static void synic_exit(struct kvm_vcpu_hv_synic *synic, u32 msr)
+{
+   struct kvm_vcpu *vcpu = synic_to_vcpu(synic);
+   struct kvm_vcpu_hv *hv_vcpu = &vcpu->arch.hyperv;
+
+   hv_vcpu->exit.type = KVM_EXIT_HYPERV_SYNIC;
+   hv_vcpu->exit.u.synic.msr = msr;
+   hv_vcpu->exit.u.synic.control = synic->control;
+   hv_vcpu->exit.u.synic.evt_page = synic->evt_page;
+   hv_vcpu->exit.u.synic.msg_page = synic->msg_page;
+
+   kvm_make_request(KVM_REQ_HV_EXIT, vcpu);
+}
+
 static int synic_set_msr(struct kvm_vcpu_hv_synic *synic,
 u32 msr, u64 data, bool host)
 {
@@ -103,6 +117,7 @@ static int synic_set_msr(struct kvm_vcpu_hv_synic *synic,
switch (msr) {
case HV_X64_MSR_SCONTROL:
synic->control = data;
+   synic_exit(synic, msr);
break;
case HV_X64_MSR_SVERSION:
if (!host) {
@@ -119,6 +134,7 @@ static int synic_set_msr(struct kvm_vcpu_hv_synic *synic,
break;
}
synic->evt_page = data;
+   synic_exit(synic, msr);
break;
case HV_X64_MSR_SIMP:
if (data & HV_SYNIC_SIMP_ENABLE)
@@ -128,6 +144,7 @@ static int synic_set_msr(struct kvm_vcpu_hv_synic *synic,
break;
}
synic->msg_page = data;
+   synic_exit(synic, msr);
break;
case HV_X64_MSR_EOM: {
int i;
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 7580e9c..4c80d18 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -6335,6 +6335,12 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
r = 0;
goto out;
}
+   if (kvm_check_request(KVM_REQ_HV_EXIT, vcpu)) {
+   vcpu->run->exit_reason = KVM_EXIT_HYPERV;
+   vcpu->run->hyperv = vcpu->arch.hyperv.exit;
+   r = 0;
+   goto out;
+   }
}
 
/*
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 30fac73..d80b031 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -143,6 +143,7 @@ static inline bool is_error_page(struct page *page)
 #define KVM_REQ_HV_CRASH  27
 #define KVM_REQ_IOAPIC_EOI_EXIT   28
 #define KVM_REQ_HV_RESET  29
+#define KVM_REQ_HV_EXIT   30
 
 #define KVM_USERSPACE_IRQ_SOURCE_ID0
 #define KVM_IRQFD_RESAMPLE_IRQ_SOURCE_ID   1
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 27ce460..6e32f75 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -154,6 +154,20 @@ struct kvm_s390_skeys {
__u32 flags;
__u32 reserved[9];
 };
+
+struct kvm_hyperv_exit {
+#defin

[PATCH 1/2] kvm/x86: Hyper-V synthetic interrupt controller

2015-10-09 Thread Denis V. Lunev
From: Andrey Smetanin 

Synic is a lapic extension, which is controlled via MSRs and maintains
for each vCPU
 - 16 synthetic interrupt "lines" (SINT's); each can be configured to
   trigger a specific interrupt vector optionally with auto-EOI
   semantics
 - a message page in the guest memory with 16 256-byte per-SINT message
   slots
 - an event flag page in the guest memory with 16 2048-bit per-SINT
   event flag areas

The host triggers a SINT whenever it delivers a new message to the
corresponding slot or flips an event flag bit in the corresponding area.
The guest informs the host that it can try delivering a message by
explicitly asserting EOI in lapic or writing to End-Of-Message (EOM)
MSR.

The userspace (qemu) triggers interrupts and receives EOM notifications
via irqfd with resampler; for that, a GSI is allocated for each
configured SINT, and irq_routing api is extended to support GSI-SINT
mapping.

Signed-off-by: Andrey Smetanin 
Reviewed-by: Roman Kagan 
Signed-off-by: Denis V. Lunev 
CC: Vitaly Kuznetsov 
CC: "K. Y. Srinivasan" 
CC: Gleb Natapov 
CC: Paolo Bonzini 
---
 arch/powerpc/kvm/mpic.c |  18 +++
 arch/s390/kvm/interrupt.c   |  18 +++
 arch/x86/include/asm/kvm_host.h |  14 +++
 arch/x86/kvm/hyperv.c   | 266 
 arch/x86/kvm/hyperv.h   |  20 +++
 arch/x86/kvm/irq_comm.c |  16 +++
 arch/x86/kvm/lapic.c|  15 ++-
 arch/x86/kvm/lapic.h|   5 +
 arch/x86/kvm/x86.c  |   4 +
 drivers/hv/hyperv_vmbus.h   |   5 -
 include/linux/kvm_host.h|  12 ++
 include/uapi/linux/hyperv.h |  12 ++
 include/uapi/linux/kvm.h|   8 ++
 virt/kvm/arm/vgic.c |  18 +++
 virt/kvm/eventfd.c  |  35 +-
 virt/kvm/irqchip.c  |  24 +++-
 16 files changed, 475 insertions(+), 15 deletions(-)

diff --git a/arch/powerpc/kvm/mpic.c b/arch/powerpc/kvm/mpic.c
index 6249cdc..01e7fb4 100644
--- a/arch/powerpc/kvm/mpic.c
+++ b/arch/powerpc/kvm/mpic.c
@@ -1850,3 +1850,21 @@ int kvm_set_routing_entry(struct 
kvm_kernel_irq_routing_entry *e,
 out:
return r;
 }
+
+/* Hyper-V Synic not implemented */
+int kvm_hv_set_sint(struct kvm_kernel_irq_routing_entry *e,
+   struct kvm *kvm, int irq_source_id, int level,
+   bool line_status)
+{
+   return -ENOTSUP;
+}
+
+int kvm_hv_get_sint_gsi(struct kvm_vcpu *vcpu, u32 sint)
+{
+   return -ENOTSUP;
+}
+
+int kvm_hv_set_sint_gsi(struct kvm *kvm, u32 vcpu_id, u32 sint, int gsi)
+{
+   return -ENOTSUP;
+}
diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
index 5c2c169..7fa8d9d 100644
--- a/arch/s390/kvm/interrupt.c
+++ b/arch/s390/kvm/interrupt.c
@@ -2285,3 +2285,21 @@ int kvm_s390_get_irq_state(struct kvm_vcpu *vcpu, __u8 
__user *buf, int len)
 
return n;
 }
+
+/* Hyper-V Synic not implemented */
+int kvm_hv_set_sint(struct kvm_kernel_irq_routing_entry *e,
+   struct kvm *kvm, int irq_source_id, int level,
+   bool line_status)
+{
+   return -ENOTSUP;
+}
+
+int kvm_hv_get_sint_gsi(struct kvm_vcpu *vcpu, u32 sint)
+{
+   return -ENOTSUP;
+}
+
+int kvm_hv_set_sint_gsi(struct kvm *kvm, u32 vcpu_id, u32 sint, int gsi)
+{
+   return -ENOTSUP;
+}
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index cdbdb55..e614a543 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -25,6 +25,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -374,10 +375,23 @@ struct kvm_mtrr {
struct list_head head;
 };
 
+/* Hyper-V synthetic interrupt controller */
+struct kvm_vcpu_hv_synic {
+   u64 version;
+   u64 control;
+   u64 msg_page;
+   u64 evt_page;
+   atomic64_t sint[HV_SYNIC_SINT_COUNT];
+   atomic_t sint_to_gsi[HV_SYNIC_SINT_COUNT];
+   DECLARE_BITMAP(auto_eoi_bitmap, 256);
+   DECLARE_BITMAP(vec_bitmap, 256);
+};
+
 /* Hyper-V per vcpu emulation context */
 struct kvm_vcpu_hv {
u64 hv_vapic;
s64 runtime_offset;
+   struct kvm_vcpu_hv_synic synic;
 };
 
 struct kvm_vcpu_arch {
diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index 62cf8c9..15c3c02 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -23,13 +23,265 @@
 
 #include "x86.h"
 #include "lapic.h"
+#include "ioapic.h"
 #include "hyperv.h"
 
 #include 
+#include 
 #include 
 
 #include "trace.h"
 
+static inline u64 synic_read_sint(struct kvm_vcpu_hv_synic *synic, int sint)
+{
+   return atomic64_read(&synic->sint[sint]);
+}
+
+static inline int synic_get_sint_vector(u64 sint_value)
+{
+   if (sint_value & HV_SYNIC_SINT_MASKED)
+   return -1;
+   return sint_value & HV_SYNIC_SINT_VECTOR_MASK;
+}
+
+static bool synic_has_active_vector(struct kvm_vcpu_hv_synic *synic,
+   int vector, int sint_to_skip, int sint_mask)
+{
+   u64 sint_value;
+