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

[Alejandro Vallejo <alejandro.vallejo@xxxxxxxxx>]

On 27/05/2024 09:20, Roger Pau Monné wrote:
> 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?

Bah. I hoped the x2apic ID restoration would mean I could get away with
removing all that junk, but migrations from Xen v.StoneAge do cause
mayhem. And it'd be very bizarre because the new topology leaves would
not reflect the existing x2apic IDs either.

I'll condense that blob of nonsense with the old scheme into a separate
function so we can easily deprecate it in the future.

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

Sure.

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

Sure.

Cheers,
Alejandro