Re: [PATCH 8/8] KVM: Emulation MSI-X mask bits for assigned devices

Michael S. Tsirkin
2010-10-20T22:30:34Z

On Wed, Oct 20, 2010 at 04:26:32PM +0800, Sheng Yang wrote:
> This patch enable per-vector mask for assigned devices using MSI-X.
>
> Signed-off-by: Sheng Yang <sheng@linux.intel.com>
> ---
> Documentation/kvm/api.txt | 22 ++++++++++++++++
> arch/x86/kvm/x86.c | 6 ++++
> include/linux/kvm.h | 8 +++++-
> virt/kvm/assigned-dev.c | 60 +++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 95 insertions(+), 1 deletions(-)
>
> diff --git a/Documentation/kvm/api.txt b/Documentation/kvm/api.txt
> index d82d637..f324a50 100644
> --- a/Documentation/kvm/api.txt
> +++ b/Documentation/kvm/api.txt
> @@ -1087,6 +1087,28 @@ of 4 instructions that make up a hypercall.
> If any additional field gets added to this structure later on, a bit for that
> additional piece of information will be set in the flags bitmap.
>
> +4.47 KVM_ASSIGN_REG_MSIX_MMIO
> +
> +Capability: KVM_CAP_DEVICE_MSIX_MASK
> +Architectures: x86
> +Type: vm ioctl
> +Parameters: struct kvm_assigned_msix_mmio (in)
> +Returns: 0 on success, !0 on error
> +
> +struct kvm_assigned_msix_mmio {
> + /* Assigned device's ID */
> + __u32 assigned_dev_id;
> + /* MSI-X table MMIO address */
> + __u64 base_addr;
> + /* Must be 0 */
> + __u32 flags;
> + /* Must be 0, reserved for future use */
> + __u64 reserved;
> +};
> +
> +This ioctl would enable in-kernel MSI-X emulation, which would handle MSI-X
> +mask bit in the kernel.
> +
> 5. The kvm_run structure
>
> Application code obtains a pointer to the kvm_run structure by
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index fc62546..ba07a2f 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -1927,6 +1927,8 @@ int kvm_dev_ioctl_check_extension(long ext)
> case KVM_CAP_X86_ROBUST_SINGLESTEP:
> case KVM_CAP_XSAVE:
> case KVM_CAP_ENABLE_CAP:
> + case KVM_CAP_DEVICE_MSIX_EXT:
> + case KVM_CAP_DEVICE_MSIX_MASK:
> r = 1;
> break;
> case KVM_CAP_COALESCED_MMIO:
> @@ -2717,6 +2719,10 @@ static int kvm_vcpu_ioctl_enable_cap(struct kvm_vcpu *vcpu,
> return -EINVAL;
>
> switch (cap->cap) {
> + case KVM_CAP_DEVICE_MSIX_EXT:
> + vcpu->kvm->arch.msix_flags_enabled = true;
> + r = 0;
> + break;
> default:
> r = -EINVAL;
> break;
> diff --git a/include/linux/kvm.h b/include/linux/kvm.h
> index 0a7bd34..1494ed0 100644
> --- a/include/linux/kvm.h
> +++ b/include/linux/kvm.h
> @@ -540,6 +540,10 @@ struct kvm_ppc_pvinfo {
> #endif
> #define KVM_CAP_PPC_GET_PVINFO 57
> #define KVM_CAP_PPC_IRQ_LEVEL 58
> +#ifdef __KVM_HAVE_MSIX
> +#define KVM_CAP_DEVICE_MSIX_EXT 59
> +#define KVM_CAP_DEVICE_MSIX_MASK 60
> +#endif
>
> #ifdef KVM_CAP_IRQ_ROUTING
>
> @@ -671,6 +675,8 @@ struct kvm_clock_data {
> #define KVM_XEN_HVM_CONFIG _IOW(KVMIO, 0x7a, struct kvm_xen_hvm_config)
> #define KVM_SET_CLOCK _IOW(KVMIO, 0x7b, struct kvm_clock_data)
> #define KVM_GET_CLOCK _IOR(KVMIO, 0x7c, struct kvm_clock_data)
> +#define KVM_ASSIGN_REG_MSIX_MMIO _IOW(KVMIO, 0x7d, \
> + struct kvm_assigned_msix_mmio)
> /* Available with KVM_CAP_PIT_STATE2 */
> #define KVM_GET_PIT2 _IOR(KVMIO, 0x9f, struct kvm_pit_state2)
> #define KVM_SET_PIT2 _IOW(KVMIO, 0xa0, struct kvm_pit_state2)
> @@ -802,7 +808,7 @@ struct kvm_assigned_msix_mmio {
> __u32 assigned_dev_id;
> __u64 base_addr;
> __u32 flags;
> - __u32 reserved[2];
> + __u64 reserved;
> };
>
> #endif /* __LINUX_KVM_H */
> diff --git a/virt/kvm/assigned-dev.c b/virt/kvm/assigned-dev.c
> index 5d2adc4..9573194 100644
> --- a/virt/kvm/assigned-dev.c
> +++ b/virt/kvm/assigned-dev.c
> @@ -17,6 +17,8 @@
> #include <linux/pci.h>
> #include <linux/interrupt.h>
> #include <linux/slab.h>
> +#include <linux/irqnr.h>
> +
> #include "irq.h"
>
> static struct kvm_assigned_dev_kernel *kvm_find_assigned_dev(struct list_head *head,
> @@ -169,6 +171,14 @@ static void deassign_host_irq(struct kvm *kvm,
> */
> if (assigned_dev->irq_requested_type & KVM_DEV_IRQ_HOST_MSIX) {
> int i;
> +#ifdef __KVM_HAVE_MSIX
> + if (assigned_dev->msix_mmio_base) {

This special-casing of 0 address is not a great idea IMHO.
Let's just use a flag.

> + mutex_lock(&kvm->slots_lock);
> + kvm_io_bus_unregister_dev(kvm, KVM_MMIO_BUS,
> + &assigned_dev->msix_mmio_dev);
> + mutex_unlock(&kvm->slots_lock);
> + }
> +#endif
> for (i = 0; i < assigned_dev->entries_nr; i++)
> disable_irq_nosync(assigned_dev->
> host_msix_entries[i].vector);
> @@ -318,6 +328,15 @@ static int assigned_device_enable_host_msix(struct kvm *kvm,
> goto err;
> }
>
> + if (dev->msix_mmio_base) {
> + mutex_lock(&kvm->slots_lock);
> + r = kvm_io_bus_register_dev(kvm, KVM_MMIO_BUS,
> + &dev->msix_mmio_dev);
> + mutex_unlock(&kvm->slots_lock);
> + if (r)
> + goto err;
> + }
> +
> return 0;
> err:
> for (i -= 1; i >= 0; i--)
> @@ -870,6 +889,31 @@ static const struct kvm_io_device_ops msix_mmio_ops = {
> .write = msix_mmio_write,
> };
>
> +static int kvm_vm_ioctl_register_msix_mmio(struct kvm *kvm,
> + struct kvm_assigned_msix_mmio *msix_mmio)
> +{
> + int r = 0;
> + struct kvm_assigned_dev_kernel *adev;
> +
> + mutex_lock(&kvm->lock);
> + adev = kvm_find_assigned_dev(&kvm->arch.assigned_dev_head,
> + msix_mmio->assigned_dev_id);
> + if (!adev) {
> + r = -EINVAL;
> + goto out;
> + }
> + if (msix_mmio->base_addr == 0) {
> + r = -EINVAL;
> + goto out;
> + }
> + adev->msix_mmio_base = msix_mmio->base_addr;
> +
> + kvm_iodevice_init(&adev->msix_mmio_dev, &msix_mmio_ops);
> +out:
> + mutex_unlock(&kvm->lock);
> +
> + return r;
> +}
> #endif
>
> long kvm_vm_ioctl_assigned_device(struct kvm *kvm, unsigned ioctl,
> @@ -982,6 +1026,22 @@ long kvm_vm_ioctl_assigned_device(struct kvm *kvm, unsigned ioctl,
> goto out;
> break;
> }
> + case KVM_ASSIGN_REG_MSIX_MMIO: {
> + struct kvm_assigned_msix_mmio msix_mmio;
> +
> + r = -EFAULT;
> + if (copy_from_user(&msix_mmio, argp, sizeof(msix_mmio)))
> + goto out;
> +
> + r = -EINVAL;
> + if (msix_mmio.flags != 0 || msix_mmio.reserved != 0)
> + goto out;
> +
> + r = kvm_vm_ioctl_register_msix_mmio(kvm, &msix_mmio);
> + if (r)
> + goto out;
> + break;
> + }
> #endif
> }
> out:
> --
> 1.7.0.1