Re: [PATCH v3 2/6] xen/x86: Add initial x2APIC ID to the per-vLAPIC save area

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 29/05/2024 3:32 pm, Alejandro Vallejo wrote:
> diff --git a/xen/lib/x86/policy.c b/xen/lib/x86/policy.c
> index f033d22785be..b70b22d55fcf 100644
> --- a/xen/lib/x86/policy.c
> +++ b/xen/lib/x86/policy.c
> @@ -2,6 +2,17 @@
>  
>  #include <xen/lib/x86/cpu-policy.h>
>  
> +uint32_t x86_x2apic_id_from_vcpu_id(const struct cpu_policy *p, uint32_t id)
> +{
> +    /*
> +     * TODO: Derive x2APIC ID from the topology information inside `p`
> +     *       rather than from the vCPU ID alone. This bodge is a temporary
> +     *       measure until all infra is in place to retrieve or derive the
> +     *       initial x2APIC ID from migrated domains.
> +     */
> +    return id * 2;
> +}
> +

I'm afraid it's nonsensical to try and derive x2APIC ID from a
policy+vcpu_id.

Take a step back, and think the data through.

A VM has:
* A unique APIC_ID for each vCPU
* Info in CPUID describing how to decompose the APIC_ID into topology

Right now, because this is all completely broken, we have:
* Hardcoded APIC_ID = vCPU_ID * 2
* Total nonsense in CPUID


When constructing a VM, the toolstack (given suitable admin
guidance/defaults) *must* choose both:
* The APIC_ID themselves
* The CPUID topo data to match

i.e. this series should be editing the toolstack's call to
xc_domain_hvm_setcontext().

It's not, because AFAICT you're depending on the migration compatibility
logic and inserting a new hardcoded assumption about symmetry of the layout.


The data flows we need are:

(New) create:
* Toolstack chooses both parts of topo information
* Xen needs a default, which reasonably can be APIC_ID=vCPU_ID when the
rest of the data flow has been cleaned up.  But this is needs to be
explicit in vcpu_create() and without reference to the policy.

And to be clear, it's fine for now for the toolstack to choose a
symmetric layout and pick appropriate APIC_IDs+CPUID for this, but it
needs to be the toolstack making this decision, not Xen inventing state
out of thin air based on the toolstack only giving half the information.

(New) migrate:
* Data from the stream, exactly as presented

(Compat) migrate:
* Synthesize the missing xapic_id field in LAPIC_REGs as APIC_ID=vCPU_ID
* 2.

I'm pretty sure this will be a net reduction in complexity in this
series.  It definitely reduces the Xen complexity.

~Andrew