From: Auger Eric <eric.auger@redhat.com>
To: Vikas Gupta <vikas.gupta@broadcom.com>,
Alex Williamson <alex.williamson@redhat.com>
Cc: Cornelia Huck <cohuck@redhat.com>,
kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
Vikram Prakash <vikram.prakash@broadcom.com>
Subject: Re: [RFC, v0 1/3] vfio/platform: add support for msi
Date: Mon, 9 Nov 2020 16:18:07 +0100 [thread overview]
Message-ID: <eec028cb-6c80-4560-e138-2b567f821a5c@redhat.com> (raw)
In-Reply-To: <CAHLZf_uan_nbewxRgTtbkAAk1rGAggq_3Z4EgtRqsPryt58eOw@mail.gmail.com>
Hi Vikas,
On 11/9/20 7:41 AM, Vikas Gupta wrote:
> Hi Alex,
>
> On Fri, Nov 6, 2020 at 8:42 AM Alex Williamson
> <alex.williamson@redhat.com> wrote:
>>
>> On Fri, 6 Nov 2020 08:24:26 +0530
>> Vikas Gupta <vikas.gupta@broadcom.com> wrote:
>>
>>> Hi Alex,
>>>
>>> On Thu, Nov 5, 2020 at 12:38 PM Alex Williamson
>>> <alex.williamson@redhat.com> wrote:
>>>>
>>>> On Thu, 5 Nov 2020 11:32:55 +0530
>>>> Vikas Gupta <vikas.gupta@broadcom.com> wrote:
>>>>
>>>>> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
>>>>> index 2f313a238a8f..aab051e8338d 100644
>>>>> --- a/include/uapi/linux/vfio.h
>>>>> +++ b/include/uapi/linux/vfio.h
>>>>> @@ -203,6 +203,7 @@ struct vfio_device_info {
>>>>> #define VFIO_DEVICE_FLAGS_AP (1 << 5) /* vfio-ap device */
>>>>> #define VFIO_DEVICE_FLAGS_FSL_MC (1 << 6) /* vfio-fsl-mc device */
>>>>> #define VFIO_DEVICE_FLAGS_CAPS (1 << 7) /* Info supports caps */
>>>>> +#define VFIO_DEVICE_FLAGS_MSI (1 << 8) /* Device supports msi */
>>>>> __u32 num_regions; /* Max region index + 1 */
>>>>> __u32 num_irqs; /* Max IRQ index + 1 */
>>>>> __u32 cap_offset; /* Offset within info struct of first cap */
>>>>
>>>> This doesn't make any sense to me, MSIs are just edge triggered
>>>> interrupts to userspace, so why isn't this fully described via
>>>> VFIO_DEVICE_GET_IRQ_INFO? If we do need something new to describe it,
>>>> this seems incomplete, which indexes are MSI (IRQ_INFO can describe
>>>> that)? We also already support MSI with vfio-pci, so a global flag for
>>>> the device advertising this still seems wrong. Thanks,
>>>>
>>>> Alex
>>>>
>>> Since VFIO platform uses indexes for IRQ numbers so I think MSI(s)
>>> cannot be described using indexes.
>>
>> That would be news for vfio-pci which has been describing MSIs with
>> sub-indexes within indexes since vfio started.
>>
>>> In the patch set there is no difference between MSI and normal
>>> interrupt for VFIO_DEVICE_GET_IRQ_INFO.
>>
>> Then what exactly is a global device flag indicating? Does it indicate
>> all IRQs are MSI?
>
> No, it's not indicating that all are MSI.
> The rationale behind adding the flag to tell user-space that platform
> device supports MSI as well. As you mentioned recently added
> capabilities can help on this, I`ll go through that.
>
>>
>>> The patch set adds MSI(s), say as an extension, to the normal
>>> interrupts and handled accordingly.
>>
>> So we have both "normal" IRQs and MSIs? How does the user know which
>> indexes are which?
>
> With this patch set, I think this is missing and user space cannot
> know that particular index is MSI interrupt.
> For platform devices there is no such mechanism, like index and
> sub-indexes to differentiate between legacy, MSI or MSIX as it’s there
> in PCI.
Wht can't you use the count field (as per vfio_pci_get_irq_count())?
> I believe for a particular IRQ index if the flag
> VFIO_IRQ_INFO_NORESIZE is used then user space can know which IRQ
> index has MSI(s). Does it make sense?
I don't think it is the same semantics.
Thanks
Eric
> Suggestions on this would be helpful.
>
> Thanks,
> Vikas
>>
>>> Do you see this is a violation? If
>>
>> Seems pretty unclear and dubious use of a global device flag.
>>
>>> yes, then we`ll think of other possible ways to support MSI for the
>>> platform devices.
>>> Macro VFIO_DEVICE_FLAGS_MSI can be changed to any other name if it
>>> collides with an already supported vfio-pci or if not necessary, we
>>> can remove this flag.
>>
>> If nothing else you're using a global flag to describe a platform
>> device specific augmentation. We've recently added capabilities on the
>> device info return that would be more appropriate for this, but
>> fundamentally I don't understand why the irq info isn't sufficient.
>> Thanks,
>>
>> Alex
>>
next prev parent reply other threads:[~2020-11-09 15:18 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 [this message]
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
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=eec028cb-6c80-4560-e138-2b567f821a5c@redhat.com \
--to=eric.auger@redhat.com \
--cc=alex.williamson@redhat.com \
--cc=cohuck@redhat.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=vikas.gupta@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).