Re: [PATCH V0 1/3] PCI: hv: Export hv_build_devid_type_pci() and change return type

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] hv: invalid fallback device ID when CONFIG_PCI_MSI=n
--

commit 92e8d721eb5c704c530977f1074dc44ee7895e0a
Author: Mukesh R <mrathor@linux.microsoft.com>

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

This commit makes hv_build_devid_type_pci() public and changes its return
type to u64 to enforce its size. It also adds a fallback stub for when
CONFIG_PCI_MSI is disabled.

> diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.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 */
>  
> +#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; }

[Severity: High]
Does returning 0 here break PCI device isolation and silently misconfigure the
hypervisor IOMMU?

On Hyper-V, a device ID of 0 is not an error code; it is a valid ID
corresponding to HV_DEVICE_TYPE_LOGICAL (type 0) with ID 0. If CONFIG_PCI_MSI
is disabled (which is permitted, as CONFIG_HYPERV_ROOT_IOMMU does not select or
depend on it), the IOMMU driver will silently issue attach/map hypercalls
targeting Logical Device 0 instead of the actual PCI device.

For example, when compiled with CONFIG_HYPERV_ROOT_IOMMU=y and
CONFIG_PCI_MSI=n, during boot and device initialization, hv_iommu_att_dev2dom()
could call this stub and pass the resulting 0 to the
HVCALL_ATTACH_DEVICE_DOMAIN hypercall.

Can the actual implementation of hv_build_devid_type_pci() in
arch/x86/hyperv/irqdomain.c and its dependencies (get_rid_cb, struct rid_data)
be moved outside the CONFIG_PCI_MSI configuration boundary rather than being
masked by a silently failing stub? They use standard PCI APIs and have no
real dependency on MSI.

> +#endif
> +
>  #else /* CONFIG_HYPERV */
>  static inline void hyperv_init(void) {}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260921225028.4007330-1-mrathor@linux.microsoft.com?part=1