lkml.org 
[lkml]   [2023]   [Mar]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [RFC PATCH 8/8] vfio/pci: Clear VFIO_IRQ_INFO_NORESIZE for MSI-X
    From
    Hi Alex,

    On 3/17/2023 4:01 PM, Alex Williamson wrote:
    > On Fri, 17 Mar 2023 15:21:09 -0700
    > Reinette Chatre <reinette.chatre@intel.com> wrote:
    >
    >> Hi Alex,
    >>
    >> On 3/17/2023 2:05 PM, Alex Williamson wrote:
    >>> On Wed, 15 Mar 2023 13:59:28 -0700
    >>> Reinette Chatre <reinette.chatre@intel.com> wrote:
    >>>
    >>>> Dynamic MSI-X is supported. Clear VFIO_IRQ_INFO_NORESIZE
    >>>> to provide guidance to user space.
    >>>>
    >>>> Signed-off-by: Reinette Chatre <reinette.chatre@intel.com>
    >>>> ---
    >>>> drivers/vfio/pci/vfio_pci_core.c | 2 +-
    >>>> include/uapi/linux/vfio.h | 3 +++
    >>>> 2 files changed, 4 insertions(+), 1 deletion(-)
    >>>>
    >>>> diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
    >>>> index ae0e161c7fc9..1d071ee212a7 100644
    >>>> --- a/drivers/vfio/pci/vfio_pci_core.c
    >>>> +++ b/drivers/vfio/pci/vfio_pci_core.c
    >>>> @@ -1111,7 +1111,7 @@ static int vfio_pci_ioctl_get_irq_info(struct vfio_pci_core_device *vdev,
    >>>> if (info.index == VFIO_PCI_INTX_IRQ_INDEX)
    >>>> info.flags |=
    >>>> (VFIO_IRQ_INFO_MASKABLE | VFIO_IRQ_INFO_AUTOMASKED);
    >>>> - else
    >>>> + else if (info.index != VFIO_PCI_MSIX_IRQ_INDEX)
    >>>> info.flags |= VFIO_IRQ_INFO_NORESIZE;
    >>>>
    >>>
    >>> I think we need to check pci_msix_can_alloc_dyn(), right? Thanks,
    >>
    >> Can pci_msix_can_alloc_dyn() ever return false?
    >>
    >> I cannot see how pci_msix_can_alloc_dyn() can return false when
    >> considering the VFIO PCI MSI-X flow:
    >>
    >> vfio_msi_enable(..., ..., msix == true)
    >> pci_alloc_irq_vectors(..., ..., ..., flag == PCI_IRQ_MSIX)
    >> pci_alloc_irq_vectors_affinity()
    >> __pci_enable_msix_range()
    >> pci_setup_msix_device_domain()
    >> pci_create_device_domain(..., &pci_msix_template, ...)
    >>
    >> The template used above, pci_msix_template, has MSI_FLAG_PCI_MSIX_ALLOC_DYN
    >> hardcoded. This is the flag that pci_msix_can_alloc_dyn() tests for.
    >>
    >> If indeed pci_msix_can_alloc_dyn() can return false in the VFIO PCI MSI-X
    >> usage then this series needs to be reworked to continue supporting
    >> existing allocation mechanism as well as dynamic MSI-X allocation. Which
    >> allocation mechanism to use would be depend on pci_msix_can_alloc_dyn().
    >>
    >> Alternatively, if you agree that VFIO PCI MSI-X can expect
    >> pci_msix_can_alloc_dyn() to always return true then I should perhaps
    >> add a test in vfio_msi_enable() that confirms this is the case. This would
    >> cause vfio_msi_enable() to fail if dynamic MSI-X is not possible, perhaps even
    >> complain loudly with a WARN.
    >
    > pci_setup_msix_device_domain() says it returns true if:
    >
    > * True when:
    > * - The device does not have a MSI parent irq domain associated,
    > * which keeps the legacy architecture specific and the global
    > * PCI/MSI domain models working
    > * - The MSI-X domain exists already
    > * - The MSI-X domain was successfully allocated
    >
    > That first one seems concerning, dynamic allocation only works on irq
    > domain configurations. What does that exclude and do we care about any
    > of them for vfio-pci? Minimally this suggests a dependency on
    > IRQ_DOMAIN, which we don't currently have, but I'm not sure if
    > supporting irq domains is the same as having irq domains. Thanks,

    Just to confirm, as I mentioned in [1] I do plan to rework this solution
    to support both allocation mechanisms, using pci_msix_can_alloc_dyn()
    to pick which one to use. Thank you very much for pointing out this
    gap to me.

    Reinette

    [1] https://lore.kernel.org/lkml/e2d3f5a6-0a36-f19d-8f19-748197c3308d@intel.com/


    \
     
     \ /
      Last update: 2023-03-27 01:09    [W:6.659 / U:0.028 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site