Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] Undefined behavior when calculating `aperture_end` if the hypervisor returns `max_iova_width >= 64`. - [High] Unconditional overwrite of `x86_init.iommu.iommu_init` causes hardware IOMMUs (VT-d/AMD) to fail initialization. --
commit fdcb0264dfd6bd1cb7c3c4676b8a796ad8a85e4a Author: Yu Zhang <[email protected]> iommu/hyperv: Add para-virtualized IOMMU support for Hyper-V guest Add a para-virtualized IOMMU driver for Linux guests running on Hyper-V. This driver implements stage-1 IO translation within the guest OS and integrates with the Linux IOMMU core using Hyper-V hypercalls. > diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c > index 55a8b6de2865a..094f9f7ddb728 100644 > --- a/arch/x86/hyperv/hv_init.c > +++ b/arch/x86/hyperv/hv_init.c > @@ -578,6 +578,10 @@ void __init hyperv_init(void) > old_setup_percpu_clockev = x86_init.timers.setup_percpu_clockev; > x86_init.timers.setup_percpu_clockev = hv_stimer_setup_percpu_clockev; > > +#ifdef CONFIG_HYPERV_PVIOMMU > + x86_init.iommu.iommu_init = hv_iommu_init; [Severity: High] The assignment to x86_init.iommu.iommu_init must be conditionally guarded to prevent hardware IOMMU initialization failures. Will this unconditional overwrite break hardware IOMMUs? Initial boot sequence logic and IOMMU detection parameters are validated. If pci_iommu_alloc() previously detected a hardware IOMMU (like VT-d/AMD) and set iommu_detected=1, overwriting the pointer with hv_iommu_init() ensures failure since hv_iommu_init() immediately aborts when iommu_detected==1. > +#endif > + > hv_apic_init(); [ ... ] > diff --git a/drivers/iommu/hyperv/iommu.c b/drivers/iommu/hyperv/iommu.c > new file mode 100644 > index 0000000000000..2541369464040 > --- /dev/null > +++ b/drivers/iommu/hyperv/iommu.c [ ... ] > @@ -509,6 +509,16 @@ static void __init hv_init_iommu_device(struct > hv_iommu_dev *hv_iommu, > + hv_iommu->geometry = (struct iommu_domain_geometry) { > + .aperture_start = 0, > + .aperture_end = (((u64)1) << hv_iommu->max_iova_width) - 1, [Severity: High] The max_iova_width value must be bounds-checked against the maximum 64-bit integer width before performing the bitwise shift. Does this code overflow the 64-bit shift limit? Standard processing applied to the hypercall response. If an untrusted hypervisor responds to HVCALL_GET_IOMMU_CAPABILITIES with max_iova_width >= 64, shifting ((u64)1) directly by that amount results in undefined behavior and unpredictable IOVA aperture sizing. > + .force_aperture = true, > + }; -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=3
