Re: [PATCH] x86/cpu-policy: Hide x2APIC from PV guests

From: Andrew Cooper <andrew.cooper3@citrix.com>
To: "Roger Pau Monné" <roger.pau@citrix.com>
Cc: Xen-devel <xen-devel@lists.xenproject.org>,
	Jan Beulich <JBeulich@suse.com>, Wei Liu <wl@xen.org>
Subject: Re: [PATCH] x86/cpu-policy: Hide x2APIC from PV guests
Date: Fri, 1 Mar 2024 15:29:08 +0000	[thread overview]
Message-ID: <c1187c2f-cc1f-49e8-b303-8ab75be9effb@citrix.com> (raw)
In-Reply-To: <ZeHGAJfw0HLnkB0p@macbook>

On 01/03/2024 12:11 pm, Roger Pau Monné wrote:
> On Thu, Feb 29, 2024 at 10:14:48PM +0000, Andrew Cooper wrote:
>> PV guests can't write to MSR_APIC_BASE (in order to set EXTD), nor can they
>> access any of the x2APIC MSR range.  Therefore they mustn't see the x2APIC
>> CPUID bit saying that they can.
>>
>> Right now, the host x2APIC flag filters into PV guests, meaning that PV guests
>> generally see x2APIC except on Zen1-and-older AMD systems.
>>
>> Linux works around this by explicitly hiding the bit itself, and filtering
>> EXTD out of MSR_APIC_BASE reads.  NetBSD behaves more in the spirit of PV
>> guests, and entirely ignores the APIC when built as a PV guest.
>>
>> Change the annotation from !A to !S.  This has a consequence of stripping it
>> out of both PV featuremasks.  However, as existing guests may have seen the
>> bit, set it back into the PV Max policy; a VM which saw the bit and is alive
>> enough to migrate will have ignored it one way or another.
>>
>> Hiding x2APIC does also change the contents of leaf 0xb, but as the
>> information is nonsense to begin with, this is likely an improvement on the
>> status quo.  The blind reporting of APIC_ID = vCPU_ID * 2 isn't interlinked
>> with the host's topology structure, and the APIC_IDs are useless without an
>> MADT to start with.
> AFAICT the APIC ID in CPUID leaf 0x1 %ebx is only reported to HVM
> guests, not PV ones (see the dynamic section of guest_cpuid()).

"reported to" is complicated for PV with CPUID Masking, because host
details will leak in for both the xAPIC and x2APIC IDs.

> This paragraph might need adjusting then, as it reads to me as if PV
> guests could also expect APIC_ID == vCPU_ID * 2.
>
> With this change no x{,2}APIC ID gets exposed in CPUID for PV guests
> unless it's for compat reasons for guests that already saw the x2APIC
> feature.

I'll see if I can find some better wording.
>> Dom0 is the only PV VM to get an MADT but it's the host
>> one, meaning the two sets of APIC_IDs are from different address spaces.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks.

~Andrew