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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] ath12k: ath12k_pci_config_msi_data() overwrites cached host MSI data
- [Medium] ath12k: qcom_vfio_find_msi_cache() linearly scans PCIe config space
--

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

ath12k: Use VFIO MSI cache when available

This patch reads physical host MSI addresses from a VFIO cache exposed in
the PCI extended configuration space, allowing the ath12k firmware to
handle interrupt configuration in virtualized environments.

> diff --git a/drivers/net/wireless/ath/ath12k/pci.c b/drivers/net/wireless/ath/ath12k/pci.c
> index 65712486d3127..832d3cf652d9d 100644
> --- a/drivers/net/wireless/ath/ath12k/pci.c
> +++ b/drivers/net/wireless/ath/ath12k/pci.c
[ ... ]
> @@ -812,6 +816,39 @@ static int ath12k_pci_msi_alloc(struct ath12k_pci *ab_pci)
>  
>  	ath12k_info(ab, "MSI vectors: %d\n", num_vectors);
>  
> +	/*
> +	 * Try to get host MSI address from VFIO cache.
> +	 * When running on a VM, the ath12k 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(ab_pci->pdev);

[Severity: Medium]
Does this call perform an unsafe and unstandardized linear scan of the PCIe
extended configuration space?

Looking at qcom_vfio_find_msi_cache() in include/linux/qcom_vfio.h:

static inline int qcom_vfio_find_msi_cache(struct pci_dev *pdev)
{
	...
	for (offset = 0x100; offset <= 0xf00; offset += 4) {
		if (pci_read_config_dword(pdev, offset, &magic))
			continue;
		...

Performing a linear dword-by-dword read of the entire PCIe extended
configuration space violates PCIe specifications. Reading unimplemented space
can generate Unsupported Request (UR) errors, potentially triggering Advanced
Error Reporting (AER) events or system machine checks on certain platforms.

Could this traversal use the standard PCIe linked list of Extended
Capabilities (e.g., standard DVSEC or VSEC headers)?

> +	if (msi_offset) {
> +		pci_read_config_dword(ab_pci->pdev,
> +				      msi_offset + QCOM_VFIO_MSI_ADDR_LO_OFFSET,
> +				      &msi_addr_lo);
> +		pci_read_config_dword(ab_pci->pdev,
> +				      msi_offset + QCOM_VFIO_MSI_ADDR_HI_OFFSET,
> +				      &msi_addr_hi);
> +		pci_read_config_word(ab_pci->pdev,
> +				     msi_offset + QCOM_VFIO_MSI_DATA_OFFSET,
> +				     &msi_data);
> +
> +		if (msi_addr_lo && msi_addr_lo != 0xFFFFFFFF) {
> +			ab_pci->msi_addr_hi = msi_addr_hi;
> +			ab_pci->msi_addr_lo = msi_addr_lo;
> +			ab_pci->msi_ep_base_data = msi_data;

[Severity: High]
Does this cached host MSI data get overwritten shortly after it is saved here?

When the driver probes in a VM, ath12k_pci_probe() unconditionally calls
ath12k_pci_config_msi_data(). In ath12k_pci_config_msi_data(), it directly
overwrites msi_ep_base_data with the guest's virtual MSI data:

static int ath12k_pci_config_msi_data(struct ath12k_pci *ab_pci)
{
	...
	ab_pci->msi_ep_base_data = msi_desc->msg.data;
	...
}

Will this defeat the VFIO cache mechanism by replacing the host MSI data with
the guest's virtual MSI data, preventing the device from receiving hardware
interrupts in a VM?

> +			ab_pci->msi_addr_cached = true;
> +
> +			ath12k_info(ab,
> +				    "using host MSI from VFIO 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;
> +		}
> +	}

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