kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Vikas Gupta <vikas.gupta@broadcom.com>
To: Auger Eric <eric.auger@redhat.com>
Cc: Alex Williamson <alex.williamson@redhat.com>,
	Cornelia Huck <cohuck@redhat.com>,
	kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
	Vikram Prakash <vikram.prakash@broadcom.com>,
	Srinath Mannam <srinath.mannam@broadcom.com>,
	Ashwin Kamath <ashwin.kamath@broadcom.com>,
	Zac Schroff <zachary.schroff@broadcom.com>,
	Manish Kurup <manish.kurup@broadcom.com>
Subject: Re: [RFC v2 1/1] vfio/platform: add support for msi
Date: Thu, 3 Dec 2020 20:20:33 +0530	[thread overview]
Message-ID: <CAHLZf_tgZ7H76H3WjqXrSQeCC_CmKLrY6t46Ce-7Qo10TpMVZg@mail.gmail.com> (raw)
In-Reply-To: <73830e85-035f-88ac-7aec-a818e83c2d5a@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 23277 bytes --]

Hi Eric,

On Wed, Dec 2, 2020 at 8:14 PM Auger Eric <eric.auger@redhat.com> wrote:
>
> Hi Vikas,
>
> On 11/24/20 5:16 PM, Vikas Gupta wrote:
> > MSI support for platform devices.
> >
> > Signed-off-by: Vikas Gupta <vikas.gupta@broadcom.com>
> > ---
> >  drivers/vfio/platform/vfio_platform_common.c  |  99 ++++++-
> >  drivers/vfio/platform/vfio_platform_irq.c     | 260 +++++++++++++++++-
> >  drivers/vfio/platform/vfio_platform_private.h |  16 ++
> >  include/uapi/linux/vfio.h                     |  43 +++
> >  4 files changed, 401 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c
> > index c0771a9567fb..b0bfc0f4ee1f 100644
> > --- a/drivers/vfio/platform/vfio_platform_common.c
> > +++ b/drivers/vfio/platform/vfio_platform_common.c
> > @@ -16,6 +16,7 @@
> >  #include <linux/types.h>
> >  #include <linux/uaccess.h>
> >  #include <linux/vfio.h>
> > +#include <linux/nospec.h>
> >
> >  #include "vfio_platform_private.h"
> >
> > @@ -344,9 +345,16 @@ static long vfio_platform_ioctl(void *device_data,
> >
> >       } else if (cmd == VFIO_DEVICE_GET_IRQ_INFO) {
> >               struct vfio_irq_info info;
> > +             struct vfio_info_cap caps = { .buf = NULL, .size = 0 };
> > +             struct vfio_irq_info_cap_msi *msi_info = NULL;
> > +             unsigned long capsz;
> > +             int ext_irq_index = vdev->num_irqs - vdev->num_ext_irqs;
> >
> >               minsz = offsetofend(struct vfio_irq_info, count);
> >
> > +             /* For backward compatibility, cannot require this */
> > +             capsz = offsetofend(struct vfio_irq_info, cap_offset);
> > +
> >               if (copy_from_user(&info, (void __user *)arg, minsz))
> >                       return -EFAULT;
> >
> > @@ -356,9 +364,89 @@ static long vfio_platform_ioctl(void *device_data,
> >               if (info.index >= vdev->num_irqs)
> >                       return -EINVAL;
> >
> > +             if (info.argsz >= capsz)
> > +                     minsz = capsz;
> > +
> > +             if (info.index == ext_irq_index) {
> nit: n case we add new ext indices afterwards, I would check info.index
> -  ext_irq_index against an VFIO_EXT_IRQ_MSI define.
> > +                     struct vfio_irq_info_cap_type cap_type = {
> > +                             .header.id = VFIO_IRQ_INFO_CAP_TYPE,
> > +                             .header.version = 1 };
> > +                     int i;
> > +                     int ret;
> > +                     int num_msgs;
> > +                     size_t msi_info_size;
> > +                     struct vfio_platform_irq *irq;
> nit: I think generally the opposite order (length) is chosen. This also
> would better match the existing style in this file
Ok will fix it
> > +
> > +                     info.index = array_index_nospec(info.index,
> > +                                                     vdev->num_irqs);
> > +
> > +                     irq = &vdev->irqs[info.index];
> > +
> > +                     info.flags = irq->flags;
> I think this can be removed given [*]
Sure.
> > +                     cap_type.type = irq->type;
> > +                     cap_type.subtype = irq->subtype;
> > +
> > +                     ret = vfio_info_add_capability(&caps,
> > +                                                    &cap_type.header,
> > +                                                    sizeof(cap_type));
> > +                     if (ret)
> > +                             return ret;
> > +
> > +                     num_msgs = irq->num_ctx;
> do would want to return the cap even if !num_ctx?
If num_ctx = 0 then VFIO_IRQ_INFO_CAP_MSI_DESCS should not be written.
I`ll take care of same.
> > +
> > +                     msi_info_size = struct_size(msi_info, msgs, num_msgs);
> > +
> > +                     msi_info = kzalloc(msi_info_size, GFP_KERNEL);
> > +                     if (!msi_info) {
> > +                             kfree(caps.buf);
> > +                             return -ENOMEM;
> > +                     }
> > +
> > +                     msi_info->header.id = VFIO_IRQ_INFO_CAP_MSI_DESCS;
> > +                     msi_info->header.version = 1;
> > +                     msi_info->nr_msgs = num_msgs;
> > +
> > +                     for (i = 0; i < num_msgs; i++) {
> > +                             struct vfio_irq_ctx *ctx = &irq->ctx[i];
> > +
> > +                             msi_info->msgs[i].addr_lo = ctx->msg.address_lo;
> > +                             msi_info->msgs[i].addr_hi = ctx->msg.address_hi;
> > +                             msi_info->msgs[i].data = ctx->msg.data;
> > +                     }
> > +
> > +                     ret = vfio_info_add_capability(&caps, &msi_info->header,
> > +                                                    msi_info_size);
> > +                     if (ret) {
> > +                             kfree(msi_info);
> > +                             kfree(caps.buf);
> > +                             return ret;
> > +                     }
> > +             }
> > +
> >               info.flags = vdev->irqs[info.index].flags;
> [*]
Will fix it.
> >               info.count = vdev->irqs[info.index].count;
> >
> > +             if (caps.size) {
> > +                     info.flags |= VFIO_IRQ_INFO_FLAG_CAPS;
> > +                     if (info.argsz < sizeof(info) + caps.size) {
> > +                             info.argsz = sizeof(info) + caps.size;
> > +                             info.cap_offset = 0;
> > +                     } else {
> > +                             vfio_info_cap_shift(&caps, sizeof(info));
> > +                             if (copy_to_user((void __user *)arg +
> > +                                               sizeof(info), caps.buf,
> > +                                               caps.size)) {
> > +                                     kfree(msi_info);
> > +                                     kfree(caps.buf);
> > +                                     return -EFAULT;
> > +                             }
> > +                             info.cap_offset = sizeof(info);
> > +                     }
> > +
> > +                     kfree(msi_info);
> > +                     kfree(caps.buf);
> > +             }
> > +
> >               return copy_to_user((void __user *)arg, &info, minsz) ?
> >                       -EFAULT : 0;
> >
> > @@ -366,6 +454,7 @@ static long vfio_platform_ioctl(void *device_data,
> >               struct vfio_irq_set hdr;
> >               u8 *data = NULL;
> >               int ret = 0;
> > +             int max;
> >               size_t data_size = 0;
> >
> >               minsz = offsetofend(struct vfio_irq_set, count);
> > @@ -373,8 +462,14 @@ static long vfio_platform_ioctl(void *device_data,
> >               if (copy_from_user(&hdr, (void __user *)arg, minsz))
> >                       return -EFAULT;
> >
> > -             ret = vfio_set_irqs_validate_and_prepare(&hdr, vdev->num_irqs,
> > -                                              vdev->num_irqs, &data_size);
> > +             if (hdr.index >= vdev->num_irqs)
> > +                     return -EINVAL;
> > +
> > +             max = vdev->irqs[hdr.index].count;
> > +
> > +             ret = vfio_set_irqs_validate_and_prepare(&hdr, max,
> > +                                                      vdev->num_irqs,
> > +                                                      &data_size);
> >               if (ret)
> >                       return ret;
> >
> > diff --git a/drivers/vfio/platform/vfio_platform_irq.c b/drivers/vfio/platform/vfio_platform_irq.c
> > index c5b09ec0a3c9..4066223e5b2e 100644
> > --- a/drivers/vfio/platform/vfio_platform_irq.c
> > +++ b/drivers/vfio/platform/vfio_platform_irq.c
> > @@ -8,10 +8,12 @@
> >
> >  #include <linux/eventfd.h>
> >  #include <linux/interrupt.h>
> > +#include <linux/eventfd.h>
> >  #include <linux/slab.h>
> >  #include <linux/types.h>
> >  #include <linux/vfio.h>
> >  #include <linux/irq.h>
> > +#include <linux/msi.h>
> >
> >  #include "vfio_platform_private.h"
> >
> > @@ -253,6 +255,195 @@ static int vfio_platform_set_irq_trigger(struct vfio_platform_device *vdev,
> >       return 0;
> >  }
> >
> > +/* MSI/MSIX */
> > +static irqreturn_t vfio_msihandler(int irq, void *arg)
> > +{
> > +     struct eventfd_ctx *trigger = arg;
> > +
> > +     eventfd_signal(trigger, 1);
> > +     return IRQ_HANDLED;
> > +}
> > +
> > +static void msi_write(struct msi_desc *desc, struct msi_msg *msg)
> > +{
> > +     int i;
> > +     struct vfio_platform_irq *irq;
> > +     u16 index = desc->platform.msi_index;
> > +     struct device *dev = msi_desc_to_dev(desc);
> > +     struct vfio_device *device = dev_get_drvdata(dev);
> > +     struct vfio_platform_device *vdev = (struct vfio_platform_device *)
> > +                                             vfio_device_data(device);
> > +
> > +     for (i = 0; i < vdev->num_irqs; i++)
> > +             if (vdev->irqs[i].type == VFIO_IRQ_TYPE_MSI)
> > +                     irq = &vdev->irqs[i];
> > +
> > +     irq->ctx[index].msg.address_lo = msg->address_lo;
> > +     irq->ctx[index].msg.address_hi = msg->address_hi;
> > +     irq->ctx[index].msg.data = msg->data;
> > +}
> > +
> > +static int vfio_msi_enable(struct vfio_platform_device *vdev,
> > +                        struct vfio_platform_irq *irq, int nvec)
> > +{
> > +     int ret;
> > +     int msi_idx = 0;
> > +     struct msi_desc *desc;
> > +     struct device *dev = vdev->device;
> > +
> > +     irq->ctx = kcalloc(nvec, sizeof(struct vfio_irq_ctx), GFP_KERNEL);
> > +     if (!irq->ctx)
> > +             return -ENOMEM;
> > +
> > +     /* Allocate platform MSIs */
> > +     ret = platform_msi_domain_alloc_irqs(dev, nvec, msi_write);
> > +     if (ret < 0) {
> > +             kfree(irq->ctx);
> > +             return ret;
> > +     }
> > +
> > +     for_each_msi_entry(desc, dev) {
> > +             irq->ctx[msi_idx].hwirq = desc->irq;
> > +             msi_idx++;
> > +     }
> > +
> > +     irq->num_ctx = nvec;
> > +     irq->config_msi = 1;
> > +
> > +     return 0;
> > +}
> > +
> > +static int vfio_msi_set_vector_signal(struct vfio_platform_irq *irq,
> > +                                   int vector, int fd)
> > +{
> > +     struct eventfd_ctx *trigger;
> > +     int irq_num, ret;
> > +
> > +     if (vector < 0 || vector >= irq->num_ctx)
> > +             return -EINVAL;
> > +
> > +     irq_num = irq->ctx[vector].hwirq;
> > +
> > +     if (irq->ctx[vector].trigger) {
> > +             free_irq(irq_num, irq->ctx[vector].trigger);
> > +             kfree(irq->ctx[vector].name);
> > +             eventfd_ctx_put(irq->ctx[vector].trigger);
> > +             irq->ctx[vector].trigger = NULL;
> > +     }
> > +
> > +     if (fd < 0)
> > +             return 0;
> > +
> > +     irq->ctx[vector].name = kasprintf(GFP_KERNEL,
> > +                                       "vfio-msi[%d]", vector);
> > +     if (!irq->ctx[vector].name)
> > +             return -ENOMEM;
> > +
> > +     trigger = eventfd_ctx_fdget(fd);
> > +     if (IS_ERR(trigger)) {
> > +             kfree(irq->ctx[vector].name);
> > +             return PTR_ERR(trigger);
> > +     }
> > +
> > +     ret = request_irq(irq_num, vfio_msihandler, 0,
> > +                       irq->ctx[vector].name, trigger);
> > +     if (ret) {
> > +             kfree(irq->ctx[vector].name);
> > +             eventfd_ctx_put(trigger);
> > +             return ret;
> > +     }
> > +
> > +     irq->ctx[vector].trigger = trigger;
> > +
> > +     return 0;
> > +}
> > +
> > +static int vfio_msi_set_block(struct vfio_platform_irq *irq, unsigned int start,
> > +                           unsigned int count, int32_t *fds)
> > +{
> > +     int i, j, ret = 0;
> > +
> > +     if (start >= irq->num_ctx || start + count > irq->num_ctx)
> > +             return -EINVAL;
> > +
> > +     for (i = 0, j = start; i < count && !ret; i++, j++) {
> > +             int fd = fds ? fds[i] : -1;
> > +
> > +             ret = vfio_msi_set_vector_signal(irq, j, fd);
> > +     }
> > +
> > +     if (ret) {
> > +             for (--j; j >= (int)start; j--)
> > +                     vfio_msi_set_vector_signal(irq, j, -1);
> > +     }
> > +
> > +     return ret;
> > +}
> > +
> > +static void vfio_msi_disable(struct vfio_platform_device *vdev,
> > +                          struct vfio_platform_irq *irq)
> > +{
> > +     struct device *dev = vdev->device;
> > +
> > +     vfio_msi_set_block(irq, 0, irq->num_ctx, NULL);
> > +
> > +     platform_msi_domain_free_irqs(dev);
> > +
> > +     irq->config_msi = 0;
> > +     irq->num_ctx = 0;
> > +
> > +     kfree(irq->ctx);
> > +}
> > +
> > +static int vfio_set_msi_trigger(struct vfio_platform_device *vdev,
> > +                             unsigned int index, unsigned int start,
> > +                             unsigned int count, uint32_t flags, void *data)
> > +{
> > +     int i;
> > +     struct vfio_platform_irq *irq = &vdev->irqs[index];
> > +
> > +     if (start + count > irq->count)
> > +             return -EINVAL;
> > +
> > +     if (!count && (flags & VFIO_IRQ_SET_DATA_NONE)) {
> > +             vfio_msi_disable(vdev, irq);
> > +             return 0;
> > +     }
> > +
> > +     if (flags & VFIO_IRQ_SET_DATA_EVENTFD) {
> > +             s32 *fds = data;
> > +             int ret;
> > +
> > +             if (irq->config_msi)
> > +                     return vfio_msi_set_block(irq, start, count,
> > +                                               fds);
> > +             ret = vfio_msi_enable(vdev, irq, start + count);
> > +             if (ret)
> > +                     return ret;
> > +
> > +             ret = vfio_msi_set_block(irq, start, count, fds);
> > +             if (ret)
> > +                     vfio_msi_disable(vdev, irq);
> > +
> > +             return ret;
> > +     }
> > +
> > +     for (i = start; i < start + count; i++) {
> > +             if (!irq->ctx[i].trigger)
> > +                     continue;
> > +             if (flags & VFIO_IRQ_SET_DATA_NONE) {
> > +                     eventfd_signal(irq->ctx[i].trigger, 1);
> > +             } else if (flags & VFIO_IRQ_SET_DATA_BOOL) {
> > +                     u8 *bools = data;
> > +
> > +                     if (bools[i - start])
> > +                             eventfd_signal(irq->ctx[i].trigger, 1);
> > +             }
> > +     }
> > +
> > +     return 0;
> > +}
> > +
> >  int vfio_platform_set_irqs_ioctl(struct vfio_platform_device *vdev,
> >                                uint32_t flags, unsigned index, unsigned start,
> >                                unsigned count, void *data)
> > @@ -261,16 +452,29 @@ int vfio_platform_set_irqs_ioctl(struct vfio_platform_device *vdev,
> >                   unsigned start, unsigned count, uint32_t flags,
> >                   void *data) = NULL;
> >
> > -     switch (flags & VFIO_IRQ_SET_ACTION_TYPE_MASK) {
> > -     case VFIO_IRQ_SET_ACTION_MASK:
> > -             func = vfio_platform_set_irq_mask;
> > -             break;
> > -     case VFIO_IRQ_SET_ACTION_UNMASK:
> > -             func = vfio_platform_set_irq_unmask;
> > -             break;
> > -     case VFIO_IRQ_SET_ACTION_TRIGGER:
> > -             func = vfio_platform_set_irq_trigger;
> > -             break;
> > +     struct vfio_platform_irq *irq = &vdev->irqs[index];
> > +
> > +     if (irq->type == VFIO_IRQ_TYPE_MSI) {
> > +             switch (flags & VFIO_IRQ_SET_ACTION_TYPE_MASK) {
> > +             case VFIO_IRQ_SET_ACTION_MASK:
> > +             case VFIO_IRQ_SET_ACTION_UNMASK:
> > +                     break;
> > +             case VFIO_IRQ_SET_ACTION_TRIGGER:
> > +                     func = vfio_set_msi_trigger;
> > +                     break;
> > +             }
> > +     } else {
> > +             switch (flags & VFIO_IRQ_SET_ACTION_TYPE_MASK) {
> > +             case VFIO_IRQ_SET_ACTION_MASK:
> > +                     func = vfio_platform_set_irq_mask;
> > +                     break;
> > +             case VFIO_IRQ_SET_ACTION_UNMASK:
> > +                     func = vfio_platform_set_irq_unmask;
> > +                     break;
> > +             case VFIO_IRQ_SET_ACTION_TRIGGER:
> > +                     func = vfio_platform_set_irq_trigger;
> > +                     break;
> > +             }
> >       }
> >
> >       if (!func)
> > @@ -281,12 +485,21 @@ int vfio_platform_set_irqs_ioctl(struct vfio_platform_device *vdev,
> >
> >  int vfio_platform_irq_init(struct vfio_platform_device *vdev)
> >  {
> > -     int cnt = 0, i;
> > +     int i;
> > +     int cnt = 0;
> > +     int num_irqs;
> > +     struct device *dev = vdev->device;
> >
> >       while (vdev->get_irq(vdev, cnt) >= 0)
> >               cnt++;
> >
> > -     vdev->irqs = kcalloc(cnt, sizeof(struct vfio_platform_irq), GFP_KERNEL);
> > +     num_irqs = cnt;
> > +
> > +     if (dev->msi_domain)
> > +             num_irqs++;
> > +
> > +     vdev->irqs = kcalloc(num_irqs, sizeof(struct vfio_platform_irq),
> > +                          GFP_KERNEL);
> >       if (!vdev->irqs)
> >               return -ENOMEM;
> >
> > @@ -309,7 +522,19 @@ int vfio_platform_irq_init(struct vfio_platform_device *vdev)
> >               vdev->irqs[i].masked = false;
> >       }
> >
> > -     vdev->num_irqs = cnt;
> > +     /*
> > +      * MSI block is added at last index and its an ext irq
> it is
> > +      */
> > +     if (dev->msi_domain) {
> > +             vdev->irqs[i].flags = VFIO_IRQ_INFO_EVENTFD;
> > +             vdev->irqs[i].count = NR_IRQS;
> why NR_IRQS?
 Since different devices can have different numbers of MSI(s) so we
need to initialize with max possible values. Can you please suggest if
this value does not seem appropriate?
> > +             vdev->irqs[i].hwirq = 0;
> > +             vdev->irqs[i].masked = false;
> > +             vdev->irqs[i].type = VFIO_IRQ_TYPE_MSI;
> > +             vdev->num_ext_irqs = 1;
> > +     }
> > +
> > +     vdev->num_irqs = num_irqs;
> >
> >       return 0;
> >  err:
> > @@ -321,8 +546,13 @@ void vfio_platform_irq_cleanup(struct vfio_platform_device *vdev)
> >  {
> >       int i;
> >
> > -     for (i = 0; i < vdev->num_irqs; i++)
> > -             vfio_set_trigger(vdev, i, -1, NULL);
> > +     for (i = 0; i < vdev->num_irqs; i++) {
> > +             if (vdev->irqs[i].type == VFIO_IRQ_TYPE_MSI)
> > +                     vfio_set_msi_trigger(vdev, i, 0, 0,
> > +                                          VFIO_IRQ_SET_DATA_NONE, NULL);
> > +             else
> > +                     vfio_set_trigger(vdev, i, -1, NULL);
> > +     }
> >
> >       vdev->num_irqs = 0;
> >       kfree(vdev->irqs);
> > diff --git a/drivers/vfio/platform/vfio_platform_private.h b/drivers/vfio/platform/vfio_platform_private.h
> > index 289089910643..7bbb05988705 100644
> > --- a/drivers/vfio/platform/vfio_platform_private.h
> > +++ b/drivers/vfio/platform/vfio_platform_private.h
> > @@ -9,6 +9,7 @@
> >
> >  #include <linux/types.h>
> >  #include <linux/interrupt.h>
> > +#include <linux/msi.h>
> >
> >  #define VFIO_PLATFORM_OFFSET_SHIFT   40
> >  #define VFIO_PLATFORM_OFFSET_MASK (((u64)(1) << VFIO_PLATFORM_OFFSET_SHIFT) - 1)
> > @@ -19,9 +20,18 @@
> >  #define VFIO_PLATFORM_INDEX_TO_OFFSET(index) \
> >       ((u64)(index) << VFIO_PLATFORM_OFFSET_SHIFT)
> >
> > +struct vfio_irq_ctx {
> > +     int                     hwirq;
> > +     char                    *name;
> > +     struct msi_msg          msg;
> > +     struct eventfd_ctx      *trigger;
> > +};
> > +
> >  struct vfio_platform_irq {
> >       u32                     flags;
> >       u32                     count;
> > +     int                     num_ctx;
> > +     struct vfio_irq_ctx     *ctx;
> >       int                     hwirq;
> >       char                    *name;
> >       struct eventfd_ctx      *trigger;
> > @@ -29,6 +39,11 @@ struct vfio_platform_irq {
> >       spinlock_t              lock;
> >       struct virqfd           *unmask;
> >       struct virqfd           *mask;
> > +
> > +     /* for extended irqs */
> > +     u32                     type;
> > +     u32                     subtype;
> > +     int                     config_msi;
> >  };
> >
> >  struct vfio_platform_region {
> > @@ -46,6 +61,7 @@ struct vfio_platform_device {
> >       u32                             num_regions;
> >       struct vfio_platform_irq        *irqs;
> >       u32                             num_irqs;
> > +     int                             num_ext_irqs;
> >       int                             refcnt;
> >       struct mutex                    igate;
> >       struct module                   *parent_module;
> > diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> > index 2f313a238a8f..598d1c944283 100644
> > --- a/include/uapi/linux/vfio.h
> > +++ b/include/uapi/linux/vfio.h
> > @@ -697,11 +697,54 @@ struct vfio_irq_info {
> >  #define VFIO_IRQ_INFO_MASKABLE               (1 << 1)
> >  #define VFIO_IRQ_INFO_AUTOMASKED     (1 << 2)
> >  #define VFIO_IRQ_INFO_NORESIZE               (1 << 3)
> > +#define VFIO_IRQ_INFO_FLAG_CAPS              (1 << 4) /* Info supports caps */
> >       __u32   index;          /* IRQ index */
> >       __u32   count;          /* Number of IRQs within this index */
> > +     __u32   cap_offset;     /* Offset within info struct of first cap */
> >  };
> >  #define VFIO_DEVICE_GET_IRQ_INFO     _IO(VFIO_TYPE, VFIO_BASE + 9)
> >
> > +/*
> > + * The irq type capability allows IRQs unique to a specific device or
> > + * class of devices to be exposed.
> > + *
> > + * The structures below define version 1 of this capability.
> > + */
> > +#define VFIO_IRQ_INFO_CAP_TYPE               3
> > +
> > +struct vfio_irq_info_cap_type {
> > +     struct vfio_info_cap_header header;
> > +     __u32 type;     /* global per bus driver */
> > +     __u32 subtype;  /* type specific */
> > +};
> > +
> > +/*
> > + * List of IRQ types, global per bus driver.
> > + * If you introduce a new type, please add it here.
> > + */
> > +
> > +/* Non PCI devices having MSI(s) support */
> > +#define VFIO_IRQ_TYPE_MSI            (1)
> > +
> > +/*
> > + * The msi capability allows the user to use the msi msg to
> > + * configure the iova for the msi configuration.
> > + * The structures below define version 1 of this capability.
> > + */
> > +#define VFIO_IRQ_INFO_CAP_MSI_DESCS  4
> > +
> > +struct vfio_irq_msi_msg {
> > +     __u32   addr_lo;
> > +     __u32   addr_hi;
> > +     __u32   data;
> > +};
> > +
> > +struct vfio_irq_info_cap_msi {
> > +     struct vfio_info_cap_header header;
> > +     __u32 nr_msgs;
> I think you should align a __u32   reserved field to have a 64b alignment
Sure.
> > +     struct vfio_irq_msi_msg msgs[];
> Please can you clarify why this cap is needed versus your prior approach.
In the previous patch, the reset module was configuring the device
with MSI msg/data now since the module is not available, user space
needs to have this data.
With this approach userspace just needs the pairs <msg and ctx >
associated with the MSI(s) and it can choose to configure the MSI(s)
sources accordingly.
Let me know if this approach does not look appropriate.

Thanks,
Vikas
> > +};
> > +
> >  /**
> >   * VFIO_DEVICE_SET_IRQS - _IOW(VFIO_TYPE, VFIO_BASE + 10, struct vfio_irq_set)
> >   *
> >
> Thanks
>
> Eric
>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4163 bytes --]

  reply	other threads:[~2020-12-03 14:51 UTC|newest]

Thread overview: 54+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-05  6:02 [RFC, v0 0/3] msi support for platform devices Vikas Gupta
2020-11-05  6:02 ` [RFC, v0 1/3] vfio/platform: add support for msi Vikas Gupta
2020-11-05  7:08   ` Alex Williamson
2020-11-06  2:54     ` Vikas Gupta
2020-11-06  3:12       ` Alex Williamson
2020-11-09  6:41         ` Vikas Gupta
2020-11-09 15:18           ` Auger Eric
2020-11-09 15:28           ` Alex Williamson
2020-11-10 11:06             ` Vikas Gupta
2020-11-09 15:05       ` Auger Eric
2020-11-10 11:01         ` Vikas Gupta
2020-11-05  6:02 ` [RFC, v0 2/3] vfio/platform: change cleanup order Vikas Gupta
2020-11-05  6:02 ` [RFC, v0 3/3] vfio/platform: add Broadcom msi module Vikas Gupta
2020-11-12 17:58 ` [RFC, v1 0/3] msi support for platform devices Vikas Gupta
2020-11-12 17:58   ` [RFC v1 1/3] vfio/platform: add support for msi Vikas Gupta
2020-11-12 17:58   ` [RFC v1 2/3] vfio/platform: change cleanup order Vikas Gupta
2020-11-12 17:58   ` [RFC v1 3/3] vfio/platform: add Broadcom msi module Vikas Gupta
2020-11-12 18:40   ` [RFC, v1 0/3] msi support for platform devices Auger Eric
2020-11-13 17:24     ` Vikas Gupta
2020-11-16 13:14       ` Auger Eric
2020-11-17  6:25         ` Vikas Gupta
2020-11-17  8:05           ` Auger Eric
2020-11-17  8:25             ` Auger Eric
2020-11-17 16:36               ` Vikas Gupta
2020-11-18 11:00                 ` Auger Eric
2020-11-24 16:16   ` [RFC, v2 0/1] " Vikas Gupta
2020-11-24 16:16     ` [RFC v2 1/1] vfio/platform: add support for msi Vikas Gupta
2020-12-02 14:44       ` Auger Eric
2020-12-03 14:50         ` Vikas Gupta [this message]
2020-12-07 20:43           ` Auger Eric
2020-12-10  7:34             ` Vikas Gupta
2020-12-11  8:40               ` Auger Eric
2020-12-02 14:43     ` [RFC, v2 0/1] msi support for platform devices Auger Eric
2020-12-03 14:39       ` Vikas Gupta
2020-12-14 17:45     ` [RFC, v3 0/2] " Vikas Gupta
2020-12-14 17:45       ` [RFC v3 1/2] vfio/platform: add support for msi Vikas Gupta
2020-12-22 17:27         ` Auger Eric
2021-01-05  5:53           ` Vikas Gupta
2021-01-12  9:00             ` Auger Eric
2021-01-15  6:26               ` Vikas Gupta
2021-01-15  9:25                 ` Auger Eric
2020-12-14 17:45       ` [RFC v3 2/2] vfio/platform: msi: add Broadcom platform devices Vikas Gupta
2021-01-12  9:22         ` Auger Eric
2021-01-15  6:35           ` Vikas Gupta
2021-01-15  9:24             ` Auger Eric
2021-01-19 22:45               ` Alex Williamson
2021-01-20 10:22                 ` Auger Eric
2021-01-29 17:24       ` [RFC v4 0/3] msi support for " Vikas Gupta
2021-01-29 17:24         ` [RFC v4 1/3] vfio/platform: add support for msi Vikas Gupta
2021-02-08 13:30           ` Auger Eric
2021-01-29 17:24         ` [RFC v4 2/3] vfio/platform: change cleanup order Vikas Gupta
2021-02-08 13:31           ` Auger Eric
2021-01-29 17:24         ` [RFC v4 3/3] vfio: platform: reset: add msi support Vikas Gupta
2021-02-08 15:27           ` Auger Eric

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAHLZf_tgZ7H76H3WjqXrSQeCC_CmKLrY6t46Ce-7Qo10TpMVZg@mail.gmail.com \
    --to=vikas.gupta@broadcom.com \
    --cc=alex.williamson@redhat.com \
    --cc=ashwin.kamath@broadcom.com \
    --cc=cohuck@redhat.com \
    --cc=eric.auger@redhat.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=manish.kurup@broadcom.com \
    --cc=srinath.mannam@broadcom.com \
    --cc=vikram.prakash@broadcom.com \
    --cc=zachary.schroff@broadcom.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).