From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Subject: Re: [PATCH v4] KVM: Introduce direct MSI message injection for in-kernel irqchips Date: Fri, 30 Mar 2012 09:45:35 +0200 Message-ID: <4F75649F.206@siemens.com> References: <4F734EB3.20500@siemens.com> <4F748AAD.2040103@siemens.com> <4F74B484.30607@siemens.com> <20120329194140.GB16715@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: Avi Kivity , Marcelo Tosatti , kvm , Eric Northup To: "Michael S. Tsirkin" Return-path: Received: from goliath.siemens.de ([192.35.17.28]:23594 "EHLO goliath.siemens.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755342Ab2C3Hpn (ORCPT ); Fri, 30 Mar 2012 03:45:43 -0400 In-Reply-To: <20120329194140.GB16715@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On 2012-03-29 21:41, Michael S. Tsirkin wrote: > On Thu, Mar 29, 2012 at 09:14:12PM +0200, Jan Kiszka wrote: >> Currently, MSI messages can only be injected to in-kernel irqchips by >> defining a corresponding IRQ route for each message. This is not only >> unhandy if the MSI messages are generated "on the fly" by user space, >> IRQ routes are a limited resource that user space has to manage >> carefully. >> >> By providing a direct injection path, we can both avoid using up limited >> resources and simplify the necessary steps for user land. >> >> Signed-off-by: Jan Kiszka >> --- >> >> Changes in v4: >> - Fix the build by factoring out kvm_send_userspace_msi: >> irqchip_in_kernel is not generically available. But abstracting it >> for all arch is tricky and therefore left to the poor people who have >> to introduce non-x86 irqchip support to this x86-focused corner. >> >> Lesson (probably not) learned: Never underestimate the complexity of >> trivial changes. >> >> Documentation/virtual/kvm/api.txt | 21 +++++++++++++++++++++ >> arch/x86/kvm/Kconfig | 1 + >> include/linux/kvm.h | 11 +++++++++++ >> include/linux/kvm_host.h | 2 ++ >> virt/kvm/Kconfig | 3 +++ >> virt/kvm/irq_comm.c | 14 ++++++++++++++ >> virt/kvm/kvm_main.c | 14 ++++++++++++++ >> 7 files changed, 66 insertions(+), 0 deletions(-) >> >> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt >> index 81ff39f..ed27d1b 100644 >> --- a/Documentation/virtual/kvm/api.txt >> +++ b/Documentation/virtual/kvm/api.txt >> @@ -1482,6 +1482,27 @@ See KVM_ASSIGN_DEV_IRQ for the data structure. The target device is specified >> by assigned_dev_id. In the flags field, only KVM_DEV_ASSIGN_MASK_INTX is >> evaluated. >> >> +4.61 KVM_SIGNAL_MSI >> + >> +Capability: KVM_CAP_SIGNAL_MSI >> +Architectures: x86 >> +Type: vm ioctl >> +Parameters: struct kvm_msi (in) >> +Returns: >0 on delivery, 0 if guest blocked the MSI, and -1 on error >> + >> +Directly inject a MSI message. Only valid with in-kernel irqchip that handles >> +MSI messages. >> + >> +struct kvm_msi { >> + __u32 address_lo; >> + __u32 address_hi; >> + __u32 data; >> + __u32 flags; >> + __u8 pad[16]; >> +}; >> + >> +No flags are defined so far. The corresponding field must be 0. >> + >> 4.62 KVM_CREATE_SPAPR_TCE >> >> Capability: KVM_CAP_SPAPR_TCE >> diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig >> index 1a7fe86..a28f338 100644 >> --- a/arch/x86/kvm/Kconfig >> +++ b/arch/x86/kvm/Kconfig >> @@ -36,6 +36,7 @@ config KVM >> select TASKSTATS >> select TASK_DELAY_ACCT >> select PERF_EVENTS >> + select HAVE_KVM_MSI >> ---help--- >> Support hosting fully virtualized guest machines using hardware >> virtualization extensions. You will need a fairly recent >> diff --git a/include/linux/kvm.h b/include/linux/kvm.h >> index 7a9dd4b..225b452 100644 >> --- a/include/linux/kvm.h >> +++ b/include/linux/kvm.h >> @@ -590,6 +590,7 @@ struct kvm_ppc_pvinfo { >> #define KVM_CAP_SYNC_REGS 74 >> #define KVM_CAP_PCI_2_3 75 >> #define KVM_CAP_KVMCLOCK_CTRL 76 >> +#define KVM_CAP_SIGNAL_MSI 77 >> >> #ifdef KVM_CAP_IRQ_ROUTING >> >> @@ -715,6 +716,14 @@ struct kvm_one_reg { >> __u64 addr; >> }; >> >> +struct kvm_msi { >> + __u32 address_lo; >> + __u32 address_hi; >> + __u32 data; >> + __u32 flags; >> + __u8 pad[16]; >> +}; >> + >> /* >> * ioctls for VM fds >> */ >> @@ -789,6 +798,8 @@ struct kvm_s390_ucas_mapping { >> /* Available with KVM_CAP_PCI_2_3 */ >> #define KVM_ASSIGN_SET_INTX_MASK _IOW(KVMIO, 0xa4, \ >> struct kvm_assigned_pci_dev) >> +/* Available with KVM_CAP_SIGNAL_MSI */ >> +#define KVM_SIGNAL_MSI _IOW(KVMIO, 0xa5, struct kvm_msi) >> >> /* >> * ioctls for vcpu fds >> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h >> index 6cf158c..35c69d6 100644 >> --- a/include/linux/kvm_host.h >> +++ b/include/linux/kvm_host.h >> @@ -774,6 +774,8 @@ int kvm_set_irq_routing(struct kvm *kvm, >> unsigned flags); >> void kvm_free_irq_routing(struct kvm *kvm); >> >> +int kvm_send_userspace_msi(struct kvm *kvm, struct kvm_msi *msi); >> + >> #else >> >> static inline void kvm_free_irq_routing(struct kvm *kvm) {} >> diff --git a/virt/kvm/Kconfig b/virt/kvm/Kconfig >> index f63ccb0..28694f4 100644 >> --- a/virt/kvm/Kconfig >> +++ b/virt/kvm/Kconfig >> @@ -18,3 +18,6 @@ config KVM_MMIO >> >> config KVM_ASYNC_PF >> bool >> + >> +config HAVE_KVM_MSI >> + bool >> diff --git a/virt/kvm/irq_comm.c b/virt/kvm/irq_comm.c >> index 9f614b4..a6a0365 100644 >> --- a/virt/kvm/irq_comm.c >> +++ b/virt/kvm/irq_comm.c >> @@ -138,6 +138,20 @@ int kvm_set_msi(struct kvm_kernel_irq_routing_entry *e, >> return kvm_irq_delivery_to_apic(kvm, NULL, &irq); >> } >> >> +int kvm_send_userspace_msi(struct kvm *kvm, struct kvm_msi *msi) >> +{ >> + struct kvm_kernel_irq_routing_entry route; >> + >> + if (!irqchip_in_kernel(kvm) || msi->flags != 0) >> + return -EINVAL; >> + >> + route.msi.address_lo = msi->address_lo; >> + route.msi.address_hi = msi->address_hi; >> + route.msi.data = msi->data; >> + >> + return kvm_set_msi(&route, kvm, KVM_USERSPACE_IRQ_SOURCE_ID, 1); >> +} >> + >> /* >> * Return value: >> * < 0 Interrupt was ignored (masked or not delivered for other reasons) >> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c >> index a612bc8..5addfa8 100644 >> --- a/virt/kvm/kvm_main.c >> +++ b/virt/kvm/kvm_main.c >> @@ -2063,6 +2063,17 @@ static long kvm_vm_ioctl(struct file *filp, >> mutex_unlock(&kvm->lock); >> break; >> #endif >> +#ifdef CONFIG_HAVE_KVM_MSI >> + case KVM_SIGNAL_MSI: { >> + struct kvm_msi msi; >> + >> + r = -EFAULT; >> + if (copy_from_user(&msi, argp, sizeof msi)) >> + goto out; > > Wait a second, we should validate flags: you say > "this field should be 0" so we should check it. Yep, see kvm_send_userspace_msi. Jan > >> + r = kvm_send_userspace_msi(kvm, &msi); >> + break; >> + } >> +#endif >> default: >> r = kvm_arch_vm_ioctl(filp, ioctl, arg); >> if (r == -ENOTTY) >> @@ -2191,6 +2202,9 @@ static long kvm_dev_ioctl_check_extension_generic(long arg) >> case KVM_CAP_SET_BOOT_CPU_ID: >> #endif >> case KVM_CAP_INTERNAL_ERROR_DATA: >> +#ifdef CONFIG_HAVE_KVM_MSI >> + case KVM_CAP_SIGNAL_MSI: >> +#endif >> return 1; >> #ifdef CONFIG_HAVE_KVM_IRQCHIP >> case KVM_CAP_IRQ_ROUTING: >> -- >> 1.7.3.4 -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux