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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 20.02.2025 16:40, Roger Pau Monné wrote:
> 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 will do, I think. Then:
Acked-by: Jan Beulich <jbeulich@xxxxxxxx>

>> 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?

Right.

>> 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.

I was talking about the real one Xen uses.

Jan