Re: [PATCH v3 5/8] ath11k: use KVM MSI domain for VM passthrough

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] wifi: ath11k: hard dependency on pci-kvm-msi breaks module load on baremetal
- [Low] wifi: ath11k: vmlinux link failure when PCI_KVM_MSI_DOMAIN=m and ATH11K_PCI=y
--

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

ath11k: use KVM MSI domain for VM passthrough

When CONFIG_PCI_KVM_MSI_DOMAIN is enabled, this patch uses
kvm_pci_msi_wait_ready() and kvm_pci_msi_get_address() in
ath11k_pci_power_up() to obtain the host physical MSI address for VM
passthrough, and adds a pre-softdep on pci-kvm-msi.

> diff --git a/drivers/net/wireless/ath/ath11k/pci.c b/drivers/net/wireless/ath/ath11k/pci.c
> index 7e29ebaf97731..31889e5caa8d7 100644
> --- a/drivers/net/wireless/ath/ath11k/pci.c
> +++ b/drivers/net/wireless/ath/ath11k/pci.c
> @@ -7,6 +7,9 @@
>  #include <linux/module.h>
>  #include <linux/msi.h>
>  #include <linux/pci.h>
> +#if IS_ENABLED(CONFIG_PCI_KVM_MSI_DOMAIN)
> +#include <linux/pci-kvm-msi.h>
> +#endif
>  #include <linux/of.h>
>  #include <linux/time.h>
>  #include <linux/vmalloc.h>
> @@ -855,6 +858,13 @@ static int ath11k_pci_power_up(struct ath11k_base *ab)
>  
>  	ath11k_pci_msi_enable(ab_pci);
>  
> +#if IS_ENABLED(CONFIG_PCI_KVM_MSI_DOMAIN)
> +	if (kvm_pci_msi_wait_ready(ab_pci->pdev, 500) > 0)
> +		kvm_pci_msi_get_address(ab_pci->pdev, &ab->pci.msi.addr_lo,
> +					&ab->pci.msi.addr_hi,
> +					&ab->pci.msi.ep_base_data);
> +#endif

[Severity: Low]
Will vmlinux fail to link when CONFIG_ATH11K_PCI=y and
CONFIG_PCI_KVM_MSI_DOMAIN=m?

In drivers/pci/controller/Kconfig, PCI_KVM_MSI_DOMAIN is defined as a
tristate option:

config PCI_KVM_MSI_DOMAIN
	tristate "KVM PCI passthrough MSI domain"
	depends on X86_64 && KVM_GUEST && PCI_MSI
	select IRQ_MSI_LIB

And in drivers/net/wireless/ath/ath11k/Kconfig, ATH11K_PCI is also tristate
without a dependency on PCI_KVM_MSI_DOMAIN:

config ATH11K_PCI
	tristate "Atheros ath11k PCI support"
	depends on ATH11K && PCI

When CONFIG_ATH11K_PCI=y and CONFIG_PCI_KVM_MSI_DOMAIN=m,
IS_ENABLED(CONFIG_PCI_KVM_MSI_DOMAIN) evaluates to true in
drivers/net/wireless/ath/ath11k/pci.c, so ath11k_pci_power_up() emits direct
calls from built-in code to kvm_pci_msi_wait_ready() and
kvm_pci_msi_get_address(), which are only exported by the loadable module
pci-kvm-msi.ko.

Should ATH11K_PCI add a dependency such as:

	depends on PCI_KVM_MSI_DOMAIN || !PCI_KVM_MSI_DOMAIN

or should this be guarded with IS_REACHABLE(CONFIG_PCI_KVM_MSI_DOMAIN)?

> +
>  	ret = ath11k_mhi_start(ab_pci);
>  	if (ret) {
>  		ath11k_err(ab, "failed to start mhi: %d\n", ret);
> @@ -1350,6 +1360,9 @@ module_exit(ath11k_pci_exit);
>  
>  MODULE_DESCRIPTION("Driver support for Qualcomm Technologies PCIe 802.11ax WLAN devices");
>  MODULE_LICENSE("Dual BSD/GPL");
> +#if IS_ENABLED(CONFIG_PCI_KVM_MSI_DOMAIN)
> +MODULE_SOFTDEP("pre: pci-kvm-msi");
> +#endif

[Severity: High]
Does this prevent ath11k_pci.ko from loading on baremetal or with devices
not listed in kvm_msi_devids when both CONFIG_ATH11K_PCI=m and
CONFIG_PCI_KVM_MSI_DOMAIN=m?

Because ath11k_pci_power_up() in drivers/net/wireless/ath/ath11k/pci.c
directly calls kvm_pci_msi_wait_ready() and kvm_pci_msi_get_address(),
ath11k_pci.ko has a hard ELF symbol dependency on pci-kvm-msi.ko.
MODULE_SOFTDEP("pre: pci-kvm-msi") only instructs modprobe to attempt
loading pci-kvm-msi before ath11k_pci, and does not make symbol resolution
optional.

In drivers/pci/controller/pci-kvm-msi.c, kvm_pci_msi_init() refuses to load
the module on baremetal or when no matching device in kvm_msi_devids is
found (which also omits ath11k devices such as QCA6390 0x1101 and QCN9074
0x1104):

static int __init kvm_pci_msi_init(void)
{
	struct pci_dev *pdev = NULL;
	struct pci_dev *first = NULL;
	int ret;

	if (!kvm_para_available())
		return -ENODEV;

	for_each_pci_dev(pdev) {
		if (pci_match_id(kvm_msi_devids, pdev)) {
			first = pdev;
			break;
		}
	}

	if (!first) {
		pr_info("kvm-pci-msi: no matching devices found\n");
		return -ENODEV;
	}

When kvm_pci_msi_init() returns -ENODEV, pci-kvm-msi.ko is not loaded, so
the kernel module loader cannot resolve kvm_pci_msi_wait_ready and
kvm_pci_msi_get_address when loading ath11k_pci.ko.

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