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

[Marcelo Tosatti <mtosatti@redhat.com>]

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.