Re: [PATCH v2 1/2] vfio/pci: Fix racy bitfields and tighten struct layout

[David Laight]

On Mon, 11 May 2026 16:16:02 -0600
Alex Williamson <alex.williamson@xxxxxxxxxx> wrote:

> Bitfield operations are not atomic, they use a read-modify-write
> pattern, therefore we should be careful not to pack bitfields that
> can be concurrently updated into the same storage unit.
> 
> The split fields (virq_disabled, bardirty, pm_intx_masked,
> pm_runtime_engaged, sriov_pwr_active) are mutated post-init from
> contexts that don't serialize against the other writers in the same
> storage unit, so a bitfield RMW could drop an adjacent field's
> update.  The remaining bitfields are touched only during probe or
> close where no concurrent writer exists, so they stay packed.
> 
> While reordering, place virq_disabled and bardirty earlier to fill
> an existing alignment hole.
> 
> Fixes: 9cd0f6d5cbb6 ("vfio/pci: Use bitfield for struct vfio_pci_core_device flags")
> Cc: stable@xxxxxxxxxxxxxxx
> Assisted-by: Claude:claude-opus-4-7
> Signed-off-by: Alex Williamson <alex.williamson@xxxxxxxxxx>
> ---
>  include/linux/vfio_pci_core.h | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/include/linux/vfio_pci_core.h b/include/linux/vfio_pci_core.h
> index 2ebba746c18f..24e8db5b1c0d 100644
> --- a/include/linux/vfio_pci_core.h
> +++ b/include/linux/vfio_pci_core.h
> @@ -101,6 +101,8 @@ struct vfio_pci_core_device {
>  	const struct vfio_pci_device_ops *pci_ops;
>  	void __iomem		*barmap[PCI_STD_NUM_BARS];
>  	bool			bar_mmap_supported[PCI_STD_NUM_BARS];
> +	bool			virq_disabled;
> +	bool			bardirty;

I'd put those two after the :1 fields to avoid an extra hole.

-- David

>  	u8			*pci_config_map;
>  	u8			*vconfig;
>  	struct perm_bits	*msi_perm;
> @@ -117,16 +119,14 @@ struct vfio_pci_core_device {
>  	u32			rbar[7];
>  	bool			has_dyn_msix:1;
>  	bool			pci_2_3:1;
> -	bool			virq_disabled:1;
>  	bool			reset_works:1;
>  	bool			extended_caps:1;
> -	bool			bardirty:1;
>  	bool			has_vga:1;
>  	bool			needs_reset:1;
>  	bool			nointx:1;
>  	bool			needs_pm_restore:1;
> -	bool			pm_intx_masked:1;
> -	bool			pm_runtime_engaged:1;
> +	bool			pm_intx_masked;
> +	bool			pm_runtime_engaged;
>  	struct pci_saved_state	*pci_saved_state;
>  	struct pci_saved_state	*pm_save;
>  	int			ioeventfds_nr;