Re: [PATCH 1/3] KVM: Ioctls for init MSI-X entry

[Sheng Yang <sheng@linux.intel.com>]

On Tuesday 17 February 2009 07:23:23 Marcelo Tosatti wrote:
> On Mon, Feb 16, 2009 at 01:49:29PM +0800, Sheng Yang wrote:
> > (oops... fix a typo)
> >
> > Introduce KVM_SET_MSIX_NR and KVM_SET_MSIX_ENTRY two ioctls.
> >
> > This two ioctls are used by userspace to specific guest device MSI-X
> > entry number and correlate MSI-X entry with GSI during the initialization
> > stage.
> >
> > MSI-X should be well initialzed before enabling.
> >
> > Don't support change MSI-X entry number for now.
> >
> > Signed-off-by: Sheng Yang <sheng@linux.intel.com>
> > ---
> >  include/linux/kvm.h      |   16 +++++++
> >  include/linux/kvm_host.h |    3 +
> >  virt/kvm/kvm_main.c      |  103
> > ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 122
> > insertions(+), 0 deletions(-)
> >
> > diff --git a/include/linux/kvm.h b/include/linux/kvm.h
> > index 2163b3d..a2dfbe0 100644
> > --- a/include/linux/kvm.h
> > +++ b/include/linux/kvm.h
> > @@ -475,6 +475,8 @@ struct kvm_irq_routing {
> >  #define KVM_ASSIGN_IRQ _IOR(KVMIO, 0x70, \
> >  			    struct kvm_assigned_irq)
> >  #define KVM_REINJECT_CONTROL      _IO(KVMIO, 0x71)
> > +#define KVM_SET_MSIX_NR _IOR(KVMIO, 0x72, struct kvm_assigned_msix_nr)
> > +#define KVM_SET_MSIX_ENTRY _IOR(KVMIO, 0x73, struct
> > kvm_assigned_msix_entry)
> >
> >  /*
> >   * ioctls for vcpu fds
> > @@ -595,4 +597,18 @@ struct kvm_assigned_irq {
> >  #define KVM_DEV_IRQ_ASSIGN_MSI_ACTION	KVM_DEV_IRQ_ASSIGN_ENABLE_MSI
> >  #define KVM_DEV_IRQ_ASSIGN_ENABLE_MSI	(1 << 0)
> >
> > +struct kvm_assigned_msix_nr {
> > +	__u32 assigned_dev_id;
> > +	__u16 entry_nr;
> > +	__u16 padding;
> > +};
> > +
> > +#define KVM_MAX_MSIX_PER_DEV		512
> > +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];
> > +};
> > +
> >  #endif
> > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> > index 7c7096d..ac4d63c 100644
> > --- a/include/linux/kvm_host.h
> > +++ b/include/linux/kvm_host.h
> > @@ -326,9 +326,12 @@ struct kvm_assigned_dev_kernel {
> >  	int assigned_dev_id;
> >  	int host_busnr;
> >  	int host_devfn;
> > +	int entries_nr;
> >  	int host_irq;
> >  	bool host_irq_disabled;
> > +	struct msix_entry *host_msix_entries;
> >  	int guest_irq;
> > +	struct msix_entry *guest_msix_entries;
> >  #define KVM_ASSIGNED_DEV_GUEST_INTX	(1 << 0)
> >  #define KVM_ASSIGNED_DEV_GUEST_MSI	(1 << 1)
> >  #define KVM_ASSIGNED_DEV_HOST_INTX	(1 << 8)
> > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> > index 266bdaf..bb4aa73 100644
> > --- a/virt/kvm/kvm_main.c
> > +++ b/virt/kvm/kvm_main.c
> > @@ -1593,6 +1593,87 @@ static int kvm_vcpu_ioctl_set_sigmask(struct
> > kvm_vcpu *vcpu, sigset_t *sigset) return 0;
> >  }
> >
> > +static int kvm_vm_ioctl_set_msix_nr(struct kvm *kvm,
> > +				    struct kvm_assigned_msix_nr *entry_nr)
> > +{
> > +	int r = 0;
> > +	struct kvm_assigned_dev_kernel *adev;
> > +
> > +	mutex_lock(&kvm->lock);
> > +
> > +	adev = kvm_find_assigned_dev(&kvm->arch.assigned_dev_head,
> > +				      entry_nr->assigned_dev_id);
> > +	if (!adev) {
> > +		r = -EINVAL;
> > +		goto msix_nr_out;
> > +	}
> > +
> > +	if (adev->entries_nr == 0) {
> > +		adev->entries_nr = entry_nr->entry_nr;
> > +		if (adev->entries_nr == 0 ||
>
>                                     <= 0 ?

Oh, yeah, that's right. :)

I would change the type of adev->entries_nr to unsigned int.

-- 
regards
Yang, Sheng

>
> > +		    adev->entries_nr >= KVM_MAX_MSIX_PER_DEV)
>
> Otherwise looks OK, thanks.