Re: [PATCH v7 09/12] PCI: liveupdate: Inherit ARI Forwarding Enable on preserved bridges

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Data race on the `ari_enabled` bitfield of the bridge device.
--

--- Patch [9]: [PATCH v7 09/12] PCI: liveupdate: Inherit ARI Forwarding Enable on preserved bridges ---
commit dba41feb80b848815c664a662c635fefb4ca7403
Author: David Matlack <dmatlack@google.com>

PCI: liveupdate: Inherit ARI Forwarding Enable on preserved bridges

This commit inherits the ARI Forwarding Enable on preserved bridges during a
Live Update. It updates the pci_dev->ari_enabled flag so preserved devices
can be identified with the expanded 8-bit function number.

> diff --git a/drivers/pci/liveupdate.c b/drivers/pci/liveupdate.c
> index a95bfe5eff770..74a11e520f0d3 100644
> --- a/drivers/pci/liveupdate.c
> +++ b/drivers/pci/liveupdate.c

[ ... ]

> @@ -816,6 +820,20 @@ int pci_liveupdate_enable_acs(struct pci_dev *dev)
>  	return 0;
>  }
>  
> +int pci_liveupdate_configure_ari(struct pci_dev *dev)
> +{
> +	u16 val;
> +
> +	guard(rwsem_read)(&pci_liveupdate.rwsem);
> +
> +	if (!dev->liveupdate.incoming)
> +		return -EINVAL;
> +
> +	pcie_capability_read_word(dev, PCI_EXP_DEVCTL2, &val);
> +	dev->ari_enabled = !!(val & PCI_EXP_DEVCTL2_ARI);

[Severity: Medium]
Can this bitfield modification race with concurrent updates to other
bitfields in the same word?

In pci_liveupdate_configure_ari(), dev->ari_enabled is updated. Since
bitfield modifications compile to read-modify-write operations on the
underlying word, concurrent updates to other flags in the same word (like
is_busmaster, is_managed, or msi_enabled) could be silently lost.

The pci_liveupdate.rwsem protects liveupdate structures but does not appear
to provide mutual exclusion for the pci_dev bitfields.

> +	return 0;
> +}

[ ... ]

> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 739ecaab2e76f..e0c133b66a359 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c

[ ... ]

> @@ -3539,6 +3539,12 @@ void pci_configure_ari(struct pci_dev *dev)
>  	if (!(cap & PCI_EXP_DEVCAP2_ARI))
>  		return;
>  
> +	if (!pci_liveupdate_configure_ari(bridge))
> +		return;
> +
> +	if (pcie_ari_disabled)
> +		return;
> +
>  	if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ARI)) {
>  		pcie_capability_set_word(bridge, PCI_EXP_DEVCTL2,
>  					 PCI_EXP_DEVCTL2_ARI);
>  		bridge->ari_enabled = 1;

[Severity: Medium]
This isn't a bug introduced by this patch, but this preexisting fallback
path in pci_configure_ari() also modifies the bridge->ari_enabled bitfield.

Could this also be susceptible to the same read-modify-write data race
against other flags in the same word?

>  	} else {
>  		pcie_capability_clear_word(bridge, PCI_EXP_DEVCTL2,

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260710212616.1351130-1-dmatlack@google.com?part=9