Re: [PATCH v2 8/8] xen/x86: Synthesise domain topologies

["Roger Pau Monné" <roger.pau@citrix.com>]

On Fri, May 24, 2024 at 06:16:01PM +0100, Alejandro Vallejo wrote:
> On 24/05/2024 09:58, Roger Pau Monné wrote:
> > On Wed, May 08, 2024 at 01:39:27PM +0100, Alejandro Vallejo wrote:
> > 
> >> +        rc = x86_topo_from_parts(&p->policy, threads_per_core, cores_per_pkg);
> > 
> > I assume this generates the same topology as the current code, or will
> > the population of the leaves be different in some way?
> > 
> 
> The current code does not populate 0xb. This generates a topology
> consistent with the existing INTENDED topology. The actual APIC IDs will
> be different though (because there's no skipping of odd values).
> 
> All the dance in patch 1 was to make this migrate-safe. The x2apic ID is
> stored in the lapic hidden regs so differences with previous behaviour
> don't matter.

What about systems without CPU policy in the migration stream, will
those also get restored as expected?

I think you likely need to check whether 'restore' is set and keep the
old logic in that case?

As otherwise migrated systems without a CPU policy will get the new
topology information instead of the old one?

> IOW, The differences are:
>   * 0xb is exposed, whereas previously it wasn't
>   * APIC IDs are compacted such that new_apicid=old_apicid/2
>   * There's also a cleanup of the murkier paths to put the right core
>     counts in the right leaves (whereas previously it was bonkers)

This needs to be in the commit message IMO.

> > 
> > Note that currently the host policy also gets the topology leaves
> > cleared, is it intended to not clear them anymore after this change?
> > 
> > (as you only clear the leaves for the guest {max,def} policies)
> > 
> > Thanks, Roger.
> 
> It was like that originally in v1, I changed in v2 as part of feedback
> from Jan.

I think that's fine, but this divergence from current behavior of
cleaning the topology for the host policy needs to be mentioned in
the commit message.

Thanks, Roger.