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

Sheng Yang
2009-02-17T01:35:34Z

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.