Re: [PATCH V3 08/10] vfio/pci: Probe and store ability to support dynamic MSI-X

[Reinette Chatre]

Hi Jason,

On 4/24/2023 10:43 AM, Jason Gunthorpe wrote:
> On Wed, Apr 19, 2023 at 11:11:48AM -0700, Reinette Chatre wrote:
>> On 4/18/2023 3:38 PM, Alex Williamson wrote:
>>> On Tue, 18 Apr 2023 10:29:19 -0700
>>> Reinette Chatre <reinette.chatre@xxxxxxxxx> wrote:

...

>> diff --git a/include/linux/vfio_pci_core.h b/include/linux/vfio_pci_core.h
>> index 4f070f2d6fde..d730d78754a2 100644
>> --- a/include/linux/vfio_pci_core.h
>> +++ b/include/linux/vfio_pci_core.h
>> @@ -67,8 +67,8 @@ struct vfio_pci_core_device {
>>  	u8			msix_bar;
>>  	u16			msix_size;
>>  	u32			msix_offset;
>> -	bool			has_dyn_msix;
>>  	u32			rbar[7];
>> +	bool			has_dyn_msix;
>>  	bool			pci_2_3;
>>  	bool			virq_disabled;
>>  	bool			reset_works;
> 
> Also, Linus on record as strongly disliking these lists of bools

This looks like an example:
https://lkml.org/lkml/2017/11/21/384

> 
> If they don't need read_once/etc stuff then use a list of bitfields

I do not see any direct usage of read_once in the driver, but it is not
clear to me what falls under the "etc" umbrella. Do you consider all
the bools in struct vfio_pci_core_device to be candidates for 
transition?

> 
> bool abc:1;
> bool xyz:1;
> 

I think a base type of unsigned int since it appears to be the custom
and (if I understand correctly) was preferred at the time Linus wrote
the message I found.

Looking ahead there seems be be a bigger task here. A quick search
revealed a few other instances of vfio using "bool" in a struct. It
does not all qualify for your "lists of bools" comment, but they
may need a closer look because of the "please don't use "bool" in
structures at all" comment made by Linus in the email I found.

vfio_device::iommufd_attached
vfio_container::noiommu
vfio_platform_irq::masked
vfio_platform_device::reset_required
vfio_iommu::v2
vfio_iommu::nesting
vfio_iommu::dirty_page_tracking
vfio_dma::iommu_mapped
vfio_dma::lock_cap
vfio_dma::vaddr_invalid
vfio_iommu_group::pinned_page_dirty_scope
tce_container::enabled
tce_container::v2
tce_container::def_window_pending

Reinette