On 12.02.2025 16:38, Roger Pau Monne wrote:
> --- a/xen/arch/x86/dom0_build.c
> +++ b/xen/arch/x86/dom0_build.c
> @@ -555,10 +555,6 @@ int __init dom0_setup_permissions(struct domain *d)
> if ( !rangeset_contains_singleton(mmio_ro_ranges, mfn) )
> rc |= iomem_deny_access(d, mfn, mfn);
> }
> - /* MSI range. */
> - rc |= iomem_deny_access(d, paddr_to_pfn(MSI_ADDR_BASE_LO),
> - paddr_to_pfn(MSI_ADDR_BASE_LO +
> - MSI_ADDR_DEST_ID_MASK));
For this one, yes, I can see the LAPIC counterpart a few lines up from here
(as the description says). In arch_iommu_hwdom_init(), however, I can't.
Where's that?
> --- a/xen/drivers/passthrough/x86/iommu.c
> +++ b/xen/drivers/passthrough/x86/iommu.c
> @@ -475,11 +475,6 @@ void __hwdom_init arch_iommu_hwdom_init(struct domain *d)
> if ( rc )
> panic("IOMMU failed to remove Xen ranges: %d\n", rc);
>
> - /* Remove any overlap with the Interrupt Address Range. */
> - rc = rangeset_remove_range(map, 0xfee00, 0xfeeff);
> - if ( rc )
> - panic("IOMMU failed to remove Interrupt Address Range: %d\n", rc);
Besides being puzzled by the use of literal numbers here, why was this
necessary in the first place? Or in other words, why do we not respect the
domain's ->iomem_caps here (irrespective of iommu_hwdom_{inclusive,reserved}),
thus making sure we don't allow access to anything dom0_setup_permissions()
denies? There is iomem_access_permitted() checking in identity_map() for PV,
but no equivalent for PVH that I could spot. If that was checked somewhere, my
question on the earlier hunk would then also be addressed, of course.
Further, with the expectation for the UCSI region to eventually be marked
ACPI_NVS, how's that going to help here? The loop over the E820 map a few
lines up from here doesn't make any mappings for E820_{ACPI,NVS}. (later) Oh,
pvh_setup_acpi() does map them, and it running after iommu_hwdom_init() the
mappings should be made in both page tables (if not shared). Which gets me to
a tangential question though: Am I failing to spot where we avoid, for the
shared page tables case, doing all the work arch_iommu_hwdom_init() does?
Jan
