Re: [PATCH 5/7] ath11k: Use VFIO MSI cache when available

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] ath11k/pci: guest MSI data unconditionally overwrites host VFIO cache
- [Medium] ath11k/pci: early return bypasses ath11k_pci_msi_disable()
- [Low] PCIe: linear scan of extended config space risks side effects
--

commit fb991301d679c8e835b3eb00875de8e25f6d571d
Author: Jose Ignacio Tornos Martinez <jtornosm@redhat.com>

ath11k: Use VFIO MSI cache when available

This patch adds support for reading cached host MSI values from the PCI
extended configuration space when running in a VM via VFIO. This allows the
firmware to receive physical host MSI addresses rather than guest IOVA
addresses.

> diff --git a/drivers/net/wireless/ath/ath11k/pci.c b/drivers/net/wireless/ath/ath11k/pci.c
> index c372676b097f5..99a3a709831c1 100644
> --- a/drivers/net/wireless/ath/ath11k/pci.c
> +++ b/drivers/net/wireless/ath/ath11k/pci.c
[ ... ]
> @@ -488,6 +492,35 @@ static int ath11k_pci_alloc_msi(struct ath11k_pci *ab_pci)
>  	}
>  	ath11k_info(ab, "MSI vectors: %d\n", num_vectors);
>  
> +	/*
> +	 * Try to get host MSI address from VFIO cache.
> +	 * When running on a VM, the ath11k firmware requires physical host
> +	 * MSI addresses and cannot handle guest IOVA addresses.
> +	 * qcom-vfio-pci writes host MSI values to extended config space with
> +	 * a magic signature "QMSI" for discovery. Search for it.
> +	 */
> +	msi_offset = qcom_vfio_find_msi_cache(pci_dev);

[Severity: Low]
Does this unconditional linear scan of the extended configuration space risk
unintended hardware side effects?

Calling qcom_vfio_find_msi_cache() loops over the entire extended
configuration space reading 4 bytes at a time instead of using the standard
PCIe capability linked-list traversal.

This could trigger side effects if there are clear-on-read registers present
in the configuration space.

> +	if (msi_offset) {
> +		pci_read_config_dword(pci_dev,
> +				      msi_offset + QCOM_VFIO_MSI_ADDR_LO_OFFSET,
> +				      &msi_addr_lo);
> +		pci_read_config_dword(pci_dev,
> +				      msi_offset + QCOM_VFIO_MSI_ADDR_HI_OFFSET,
> +				      &msi_addr_hi);
> +		pci_read_config_word(pci_dev,
> +				     msi_offset + QCOM_VFIO_MSI_DATA_OFFSET,
> +				     &msi_data);
> +
> +		if (msi_addr_lo && msi_addr_lo != 0xFFFFFFFF) {
> +			ab->pci.msi.addr_lo = msi_addr_lo;
> +			ab->pci.msi.addr_hi = msi_addr_hi;
> +			ab->pci.msi.ep_base_data = msi_data;

[Severity: High]
Will this host physical MSI data be unconditionally overwritten later during
the probe sequence?

After ath11k_pci_alloc_msi() correctly sets ab->pci.msi.ep_base_data from the
VFIO cache, the probe sequence in ath11k_pci_probe() continues.

Later, ath11k_pci_probe() calls ath11k_pci_config_msi_data() which does this:

ath11k_pci_config_msi_data() {
        ...
        ab_pci->ab->pci.msi.ep_base_data = msi_desc->msg.data;
}

This appears to overwrite the host MSI data retrieved from the VFIO cache
with the guest virtual MSI data, which could break interrupt delivery in VMs.

> +			ath11k_info(ab, "VFIO MSI cache at offset 0x%x: addr_lo=0x%x addr_hi=0x%x data=%d\n",
> +				    msi_offset, msi_addr_lo, msi_addr_hi, msi_data);
> +			return 0;

[Severity: Medium]
Does returning early here skip ath11k_pci_msi_disable() and leave the device
in an inconsistent initialization state compared to the bare-metal path?

By bypassing the ath11k_pci_msi_disable(ab_pci) call just below, the device
is left with MSI enabled prematurely in the PCI configuration space.

> +		}
> +	}
> +
>  	ath11k_pci_msi_disable(ab_pci);
>  
>  	msi_desc = irq_get_msi_desc(ab_pci->pdev->irq);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260930140833.576941-1-jtornosm@redhat.com?part=5