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

From: Jan Beulich <jbeulich@suse.com>
To: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: "Roger Pau Monné" <roger.pau@citrix.com>, "Wei Liu" <wl@xen.org>,
	Xen-devel <xen-devel@lists.xenproject.org>
Subject: Re: [PATCH] x86/cpu-policy: Hide x2APIC from PV guests
Date: Mon, 4 Mar 2024 09:33:11 +0100	[thread overview]
Message-ID: <da6bb1e7-fa8f-4aca-8ba1-36f49f41d38b@suse.com> (raw)
In-Reply-To: <20240229221448.2593171-1-andrew.cooper3@citrix.com>

On 29.02.2024 23:14, 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.

This argumentation could then be used equally for the APIC bit. Why is it
correct to hide x2APIC, but not APIC?

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

Aiui the field will now be seen as zero on all CPUs. Is all CPUs having the
same APIC ID there really better than them all having different ones?

Also while I agree that logically CPUID leaf 0xb EDX is tied to x2APIC being
available as a feature, nothing is said in this regard in the documentation
of that leaf. This in particular leaves open whether on a system with x2APIC
disabled in/by firmware the value would actually be zero. Yet that case could
be considered somewhat similar to the PV case here.

Jan