Re: [PATCH v5 6/9] PCI: hv: Export hv_build_devid_type_pci() and change return type

Thank you for your contribution! Sashiko AI review found 2 potential issue(=
s) to consider:

New issues:
- [Medium] Incomplete API export for cross-architecture code

Pre-existing issues:
- [High] Resource leak of multi-vector MSI IRQs during teardown
--

--- Patch [6]: [PATCH v5 6/9] PCI: hv: Export hv_build_devid_type_pci() and=
 change return type ---
commit 2b1777d94c87d493032cb98e1c21a4774a30a00a
Author: Mukesh R <[email protected]>

PCI: hv: Export hv_build_devid_type_pci() and change return type

On Hyper-V, most hypercalls related to PCI passthru to map/unmap regions,
interrupts, etc need a device ID as a parameter. This device ID refers
to that specific device during the lifetime of passthru. So,
make hv_build_devid_type_pci() public and change return type to u64 to
enforce it's size.

> diff --git a/arch/x86/hyperv/irqdomain.c b/arch/x86/hyperv/irqdomain.c
> index b3ad50a874dca..8780573a43326 100644
> --- a/arch/x86/hyperv/irqdomain.c
> +++ b/arch/x86/hyperv/irqdomain.c
[ ... ]
> @@ -172,8 +172,9 @@ static union hv_device_id hv_build_devid_type_pci(str=
uct pci_dev *pdev)
>  	}
> =20
>  out:
> -	return hv_devid;
> +	return hv_devid.as_uint64;
>  }
> +EXPORT_SYMBOL_GPL(hv_build_devid_type_pci);

[Severity: Medium]
Will exporting and implementing this API solely in an x86-specific file cau=
se
build failures for ARM64?

The commit message mentions making this public for PCI passthrough hypercal=
ls,
but the mshv driver supports both x86 and ARM64. It appears cross-architect=
ure
builds will lack this implementation when they try to utilize the newly
exported API.

[ ... ]
> diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyp=
erv.h
> index f64393e853ee3..8ebbd1cb7c8ce 100644
> --- a/arch/x86/include/asm/mshyperv.h
> +++ b/arch/x86/include/asm/mshyperv.h
> @@ -248,6 +248,12 @@ void hv_crash_asm_end(void);
>  static inline void hv_root_crash_init(void) {}
>  #endif  /* CONFIG_MSHV_ROOT && CONFIG_CRASH_DUMP */
> =20
> +#ifdef CONFIG_PCI_MSI
> +u64 hv_build_devid_type_pci(struct pci_dev *pdev);
> +#else
> +static inline u64 hv_build_devid_type_pci(struct pci_dev *pdev) { return=
 0; }
> +#endif
> +

[Severity: Medium]
Similarly, should this declaration be placed in a shared or cross-architect=
ure
header so that ARM64 drivers can safely include it without breaking?

While reviewing this file, I also noticed an issue in hv_msi_domain_free().

[Severity: High]
This is a pre-existing issue, but does this loop fail to tear down the
remaining vectors for multi-vector MSI IRQs?

arch/x86/hyperv/irqdomain.c:hv_msi_domain_free() {
    ...
	for (int i =3D 0; i < nr_irqs; ++i) {
		struct irq_data *irqd =3D irq_domain_get_irq_data(d, virq);
    ...
}

Because it looks up the base virq on every iteration instead of virq + i,
does this continuously leak hypervisor-level interrupt resources and chip_d=
ata
memory allocations during a device unbind?

--=20
Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260731223427.2554=
[email protected]?part=3D6