Re: [PATCH v3 3/3] x86/dom0: be less restrictive with the Interrupt Address Range

["Roger Pau Monné" <roger.pau@citrix.com>]

On Thu, Feb 20, 2025 at 02:30:38PM +0100, Jan Beulich wrote:
> On 20.02.2025 09:55, Roger Pau Monné wrote:
> > On Thu, Feb 20, 2025 at 09:33:46AM +0100, Jan Beulich wrote:
> >> On 19.02.2025 17:48, Roger Pau Monne wrote:
> >>> Note that the restriction to map the local APIC page is enforced
> >>> separately, and that continues to be present.  Additionally make sure the
> >>> emulated local APIC page is also not mapped, in case dom0 is using it.
> >>
> >> But that's in GFN space, not in MFN one. Why would that matter for iomem_caps?
> > 
> > It's required to avoid arch_iommu_hwdom_init() creating an identity
> > mapping for APIC_DEFAULT_PHYS_BASE, which would prevent the local APIC
> > emulation from being used.
> 
> Hmm, yes, on one hand such a mapping would be created by default, as we
> default to "dom0-iommu=map-reserved". Otoh that mapping would be replaced
> before Dom0 is actually started, via the domain_creation_finished() hook.
> On (modern) VMX that is. So yes, on old VMX and on SVM the slot would need
> to remain unpopulated. Otoh, when the physical LAPICs are elsewhere and
> when the domain is in x2APIC mode, there would be no reason to disallow
> Dom0 access to that page.

Right, but that's now how dom0 is started ATM, as the local APIC is
unconditionally started in xAPIC mode and at APIC_DEFAULT_PHYS_BASE.

I could use vlapic_base_address() against vCPU#0 vlapic, but even in
guest_wrmsr_apic_base() we don't allow moving the local APIC MMIO
region, and hence I assumed it was fine to just use
APIC_DEFAULT_PHYS_BASE here.  Note in pvh_setup_acpi_madt() Xen also
hardcodes the local APIC address to APIC_DEFAULT_PHYS_BASE.

Would you be fine if I expand the comment so it's:

    /* If using an emulated local APIC make sure its MMIO is unpopulated. */
    if ( has_vlapic(d) )
    {
        /* Xen doesn't allow changing the local APIC MMIO window position. */
        mfn = paddr_to_pfn(APIC_DEFAULT_PHYS_BASE);
        rc |= iomem_deny_access(d, mfn, mfn);
    }

> That would apparently mean fiddling with
> iomem_caps once all vCPU-s have entered x2APIC mode.

Urg, that seems ugly.  It would also need undoing if the APICs are
reverted to xAPIC mode?

> With LAPICs not
> normally being elsewhere, question is whether this corner case actually
> needs dealing with. Yet even if not, commentary may want extending, just
> to make clear the case was considered?

As said above, for both HVM and PVH Xen doesn't allow moving the APIC
MMIO window to anything different than APIC_DEFAULT_PHYS_BASE.

Thanks, Roger.