linux-kernel - Re: [RFC PATCH 8/8] vfio/pci: Clear VFIO_IRQ_INFO

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <61296e93-6268-05cd-e876-680e07645a16@intel.com>
Date:   Fri, 17 Mar 2023 15:21:09 -0700
From:   Reinette Chatre <reinette.chatre@...el.com>
To:     Alex Williamson <alex.williamson@...hat.com>
CC:     <jgg@...dia.com>, <yishaih@...dia.com>,
        <shameerali.kolothum.thodi@...wei.com>, <kevin.tian@...el.com>,
        <tglx@...utronix.de>, <darwi@...utronix.de>, <kvm@...r.kernel.org>,
        <dave.jiang@...el.com>, <jing2.liu@...el.com>,
        <ashok.raj@...el.com>, <fenghua.yu@...el.com>,
        <tom.zanussi@...ux.intel.com>, <linux-kernel@...r.kernel.org>
Subject: Re: [RFC PATCH 8/8] vfio/pci: Clear VFIO_IRQ_INFO_NORESIZE for MSI-X

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@...el.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@...el.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.

Reinette