Re: [PATCH] x86/hvm: Add APIC IDs to the per-vLAPIC save area

[Jan Beulich <jbeulich@xxxxxxxx>]

On 18.02.2025 15:22, Alejandro Vallejo wrote:
> Today, Xen hardcodes apic_id = vcpu_id * 2, but this is unwise and
> interferes with providing accurate topology information to the guest.
> 
> Introduce a new x2apic_id field into hvm_hw_lapic.  This is immutable
> state from the guest's point of view, but it will allow the toolstack to
> eventually configure the value, and for the value to move on migrate.
> 
> For backwards compatibility, the patch rebuilds the old-style APIC IDs
> from migration streams lacking them when they aren't present.

Nit: "when they aren't present" looks to duplicate "lacking them"?

> Signed-off-by: Alejandro Vallejo <alejandro.vallejo@xxxxxxxxx>
> ---
> I've split this one from the rest of the topology series as it's independent
> and entangled with another patch from Andrew.

Albeit I think meanwhile we've settled that the entangling isn't quite as
problematic.

> @@ -1621,6 +1624,14 @@ static int cf_check lapic_load_hidden(struct domain 
> *d, hvm_domain_context_t *h)
>          return -EINVAL;
>      }
>  
> +    /*
> +     * Xen 4.20 and earlier had no x2APIC ID in the migration stream and
> +     * hard-coded "vcpu_id * 2". Default back to this if we have a
> +     * zero-extended record.
> +     */
> +    if ( h->size <= offsetof(struct hvm_hw_lapic, x2apic_id) )
> +        s->hw.x2apic_id = v->vcpu_id * 2;

While we better wouldn't get to see such input, it is in principle possible
to have an input stream with, say, half the field. Imo the condition ought
to be such that we'd make the adjustment when less than the full field is
available.

Jan