Re: [PATCH 6/8] KVM: assigned dev: Preparation for mask support in userspace

Marcelo Tosatti
2010-10-22T15:05:13Z

On Wed, Oct 20, 2010 at 04:26:30PM +0800, Sheng Yang wrote:
> The feature wouldn't be enabled until later patch set msix_flags_enabled. It
> would be enabled along with mask support in kernel.
>
> Signed-off-by: Sheng Yang <sheng@linux.intel.com>
> ---
> arch/x86/include/asm/kvm_host.h | 2 ++
> include/linux/kvm.h | 6 +++++-
> include/linux/kvm_host.h | 1 +
> virt/kvm/assigned-dev.c | 39 +++++++++++++++++++++++++++++++++++++++
> 4 files changed, 47 insertions(+), 1 deletions(-)
>
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index e209078..2bb69ba 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -456,6 +456,8 @@ struct kvm_arch {
> /* fields used by HYPER-V emulation */
> u64 hv_guest_os_id;
> u64 hv_hypercall;
> +
> + bool msix_flags_enabled;
> };
>
> struct kvm_vm_stat {
> diff --git a/include/linux/kvm.h b/include/linux/kvm.h
> index 919ae53..a699ec9 100644
> --- a/include/linux/kvm.h
> +++ b/include/linux/kvm.h
> @@ -787,11 +787,15 @@ struct kvm_assigned_msix_nr {
> };
>
> #define KVM_MAX_MSIX_PER_DEV 256
> +
> +#define KVM_MSIX_FLAG_MASK 1
> +
> struct kvm_assigned_msix_entry {
> __u32 assigned_dev_id;
> __u32 gsi;
> __u16 entry; /* The index of entry in the MSI-X table */
> - __u16 padding[3];
> + __u16 flags;
> + __u16 padding[2];
> };
>
> #endif /* __LINUX_KVM_H */
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index 30f83cd..81a6284 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -438,6 +438,7 @@ struct kvm_irq_ack_notifier {
> };
>
> #define KVM_ASSIGNED_MSIX_PENDING 0x1
> +#define KVM_ASSIGNED_MSIX_MASK 0x2
> struct kvm_guest_msix_entry {
> u32 vector;
> u16 entry;
> diff --git a/virt/kvm/assigned-dev.c b/virt/kvm/assigned-dev.c
> index 7c98928..bf96ea7 100644
> --- a/virt/kvm/assigned-dev.c
> +++ b/virt/kvm/assigned-dev.c
> @@ -666,11 +666,35 @@ msix_nr_out:
> return r;
> }
>
> +static void update_msix_mask(struct kvm_assigned_dev_kernel *assigned_dev,
> + int index)
> +{
> + int irq;
> + struct irq_desc *desc;
> +
> + if (!assigned_dev->dev->msix_enabled ||
> + !(assigned_dev->irq_requested_type & KVM_DEV_IRQ_HOST_MSIX))
> + return;
> +
> + irq = assigned_dev->host_msix_entries[index].vector;
> + BUG_ON(irq == 0);
> + desc = irq_to_desc(irq);
> + BUG_ON(!desc->msi_desc);
> +
> + if (assigned_dev->guest_msix_entries[index].flags &
> + KVM_ASSIGNED_MSIX_MASK) {
> + desc->chip->mask(irq);
> + flush_work(&assigned_dev->interrupt_work);
> + } else
> + desc->chip->unmask(irq);
> +}

Bypassing irq handling code like this is wrong (see all the state
keeping in kernel/irq/handle.c).

You need a guarantee that MSIX per-vector mask is used for
disable_irq/enable_irq, right? I can't see how this provides it.