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>,
Manish Kurup <manish.kurup@broadcom.com>
Subject: Re: [RFC, v1 0/3] msi support for platform devices
Date: Tue, 17 Nov 2020 11:55:08 +0530 [thread overview]
Message-ID: <CAHLZf_uQBzQndGo1vtPtrUd2KXk+im=A9evowggzk6U=5vEvAg@mail.gmail.com> (raw)
In-Reply-To: <c78d2706-f406-32ab-1637-bd0c9f459e23@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 6483 bytes --]
Hi Eric,
On Mon, Nov 16, 2020 at 6:44 PM Auger Eric <eric.auger@redhat.com> wrote:
>
> Hi Vikas,
>
> On 11/13/20 6:24 PM, Vikas Gupta wrote:
> > Hi Eric,
> >
> > On Fri, Nov 13, 2020 at 12:10 AM Auger Eric <eric.auger@redhat.com> wrote:
> >>
> >> Hi Vikas,
> >>
> >> On 11/12/20 6:58 PM, Vikas Gupta wrote:
> >>> This RFC adds support for MSI for platform devices.
> >>> a) MSI(s) is/are added in addition to the normal interrupts.
> >>> b) The vendor specific MSI configuration can be done using
> >>> callbacks which is implemented as msi module.
> >>> c) Adds a msi handling module for the Broadcom platform devices.
> >>>
> >>> Changes from:
> >>> -------------
> >>> v0 to v1:
> >>> i) Removed MSI device flag VFIO_DEVICE_FLAGS_MSI.
> >>> ii) Add MSI(s) at the end of the irq list of platform IRQs.
> >>> MSI(s) with first entry of MSI block has count and flag
> >>> information.
> >>> IRQ list: Allocation for IRQs + MSIs are allocated as below
> >>> Example: if there are 'n' IRQs and 'k' MSIs
> >>> -------------------------------------------------------
> >>> |IRQ-0|IRQ-1|....|IRQ-n|MSI-0|MSI-1|MSI-2|......|MSI-k|
> >>> -------------------------------------------------------
> >> I have not taken time yet to look at your series, but to me you should have
> >> |IRQ-0|IRQ-1|....|IRQ-n|MSI|MSIX
> >> then for setting a given MSIX (i) you would select the MSIx index and
> >> then set start=i count=1.
> >
> > As per your suggestion, we should have, if there are n-IRQs, k-MSIXs
> > and m-MSIs, allocation of IRQs should be done as below
> >
> > |IRQ0|IRQ1|......|IRQ-(n-1)|MSI|MSIX|
> > | |
> > |
> > |MSIX0||MSIX1||MSXI2|....|MSIX-(k-1)|
> > |MSI0||MSI1||MSI2|....|MSI-(m-1)|
> No I really meant this list of indices: IRQ0|IRQ1|......|IRQ-(n-1)|MSI|MSIX|
> and potentially later on IRQ0|IRQ1|......|IRQ-(n-1)|MSI|MSIX| ERR| REQ
> if ERR/REQ were to be added.
I agree on this. Actually the map I drew incorrectly above but wanted
to demonstrate the same. It was a child-parent relationship for MSI
and its members and similarly for MSIX as well.
>
> I think the userspace could query the total number of indices using
> VFIO_DEVICE_GET_INFO and retrieve num_irqs (corresponding to the n wire
> interrupts + MSI index + MSIX index)
>
> Then userspace can loop on all the indices using
> VFIO_DEVICE_GET_IRQ_INFO. For each index it uses count to determine the
> first indices related to wire interrupts (count = 1). Then comes the MSI
> index, and after the MSI index. If any of those is supported, count >1,
> otherwise count=0. The only thing I am dubious about is can the device
> use a single MSI/MSIX? Because my hypothesis here is we use count to
> discriminate between wire first indices and other indices.
I believe count can be one as well, especially for ERR/REQ as you
mentioned above.I think we can not rely on the count > 1. Now, this is
blocking and we are not left with options unless we consider adding
more enums in flags in vfio_irq_info to tell userspace that particular
index is wired, MSI, MSIX etc. for the platform device.
What do you think?
>
>
>
> > With this implementation user space can know that, at indexes n and
> > n+1, edge triggered interrupts are present.
> note wired interrupts can also be edge ones.
> > We may add an element in vfio_platform_irq itself to allocate MSIs/MSIXs
> > struct vfio_platform_irq{
> > .....
> > .....
> > struct vfio_platform_irq *block; => this points to the block
> > allocation for MSIs/MSIXs and all msi/msix are type of IRQs.As wired interrupts and MSI interrupts coexist, I would store in vdev an
> array of wired interrupts (the existing vdev->irqs) and a new array for
> MSI(x) as done in the PCI code.
>
> vdev->ctx = kcalloc(nvec, sizeof(struct vfio_pci_irq_ctx), GFP_KERNEL);
>
> Does it make sense?
Yes, we can use similar kinds of allocations.
Thanks,
Vikas
>
> > };
> > OR
> > Another structure can be defined in 'vfio_pci_private.h'
> > struct vfio_msi_ctx {
> > struct eventfd_ctx *trigger;
> > char *name;
> > };
> > and
> > struct vfio_platform_irq {
> > .....
> > .....
> > struct vfio_msi_ctx *block; => this points to the block allocation
> > for MSIs/MSIXs
> > };
> > Which of the above two options sounds OK to you? Please suggest.
> >
> >> to me individual MSIs are encoded in the subindex and not in the index.
> >> The index just selects the "type" of interrupt.
> >>
> >> For PCI you just have:
> >> VFIO_PCI_INTX_IRQ_INDEX,
> >> VFIO_PCI_MSI_IRQ_INDEX, -> MSI index and then you play with
> >> start/count
> >> VFIO_PCI_MSIX_IRQ_INDEX,
> >> VFIO_PCI_ERR_IRQ_INDEX,
> >> VFIO_PCI_REQ_IRQ_INDEX,
> >>
> >> (include/uapi/linux/vfio.h)
> >
> > In pci case, type of interrupts is fixed so they can be 'indexed' by
> > these enums but for VFIO platform user space will need to iterate all
> > (num_irqs) indexes to know at which indexes edge triggered interrupts
> > are present.
> indeed, but can't you loop over all indices looking until count !=1? At
> this point you know if have finished emurating the wires. Holds if
> MSI(x) count !=1 of course.
>
> Thanks
>
> Eric
>
> >
> > Thanks,
> > Vikas
> >>
> >> Thanks
> >>
> >> Eric
> >>> MSI-0 will have count=k set and flags set accordingly.
> >>>
> >>> Vikas Gupta (3):
> >>> vfio/platform: add support for msi
> >>> vfio/platform: change cleanup order
> >>> vfio/platform: add Broadcom msi module
> >>>
> >>> drivers/vfio/platform/Kconfig | 1 +
> >>> drivers/vfio/platform/Makefile | 1 +
> >>> drivers/vfio/platform/msi/Kconfig | 9 +
> >>> drivers/vfio/platform/msi/Makefile | 2 +
> >>> .../vfio/platform/msi/vfio_platform_bcmplt.c | 74 ++++++
> >>> drivers/vfio/platform/vfio_platform_common.c | 86 ++++++-
> >>> drivers/vfio/platform/vfio_platform_irq.c | 238 +++++++++++++++++-
> >>> drivers/vfio/platform/vfio_platform_private.h | 23 ++
> >>> 8 files changed, 419 insertions(+), 15 deletions(-)
> >>> create mode 100644 drivers/vfio/platform/msi/Kconfig
> >>> create mode 100644 drivers/vfio/platform/msi/Makefile
> >>> create mode 100644 drivers/vfio/platform/msi/vfio_platform_bcmplt.c
> >>>
> >>
>
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4163 bytes --]
next prev parent reply other threads:[~2020-11-17 6:25 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 [this message]
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
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_uQBzQndGo1vtPtrUd2KXk+im=A9evowggzk6U=5vEvAg@mail.gmail.com' \
--to=vikas.gupta@broadcom.com \
--cc=alex.williamson@redhat.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 \
/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).