Re: [PATCH 1/2] kvm/x86: Hyper-V synthetic interrupt controller
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
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
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
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
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
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
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; +