Re: [PATCH v12 2/7] s390x/cpu topology: reporting the CPU topology to the guest

[Pierre Morel <pmorel@linux.ibm.com>]

On 12/7/22 10:12, Cédric Le Goater wrote:
> On 11/29/22 18:42, Pierre Morel wrote:
>> The guest uses the STSI instruction to get information on the
>> CPU topology.
>>
>> Let us implement the STSI instruction for the basis CPU topology
>> level, level 2.
>>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>> ---

...snip...

>> +
>> +#define S390_TOPOLOGY_MAX_MNEST 2
>> +
>> +void insert_stsi_15_1_x(S390CPU *cpu, int sel2, __u64 addr, uint8_t ar)
>> +{
>> +    union {
>> +        char place_holder[S390_TOPOLOGY_SYSIB_SIZE];
>> +        SysIB_151x sysib;
>> +    } buffer QEMU_ALIGNED(8);
>> +    int len;
>> +
>> +    if (s390_is_pv() || !s390_has_topology() ||
>> +        sel2 < 2 || sel2 > S390_TOPOLOGY_MAX_MNEST) {
>> +        setcc(cpu, 3);
>> +        return;
>> +    }
>>
>> +    s390_prepare_topology(S390_CCW_MACHINE(cpu->machine_data));
>> +
>> +    len = setup_stsi(cpu, &buffer.sysib, sel2);
> 
> 
> The S390_CPU_TOPOLOGY object is created by the machine at init time
> and the two above routines are the only users of this object.

This is right at this moment but the object will be used in the next 
patches for implementing reset, patch 3, and migration, patch 4.


> 
> The first loops on all possible CPUs to populate the bitmask array
> 'socket' under S390_CPU_TOPOLOGY and the second uses the result to
> populate the buffer returned to the guest OS.
> 
> I don't understand why the S390_CPU_TOPOLOGY object is needed at all.
> AFAICT, this is just adding extra complexity.

I used an object because I thought it could be cleaner for the 
implementation of reset and migration.

> Is the pachset preparing
> ground for some more features ? 

Yes it is, I removed the books and drawers topology containers from this 
patch series in the version 10 of the patch series to postpone their 
implementation.

The next series on topology implementation will also add, beside the 
implementation of drawers and books, the possibility to modify the 
topology during the life of a guest.

These, book, drawer and the topology modification will need to be migrated.

Is there a good alternative to facilitate the implementation of the 
migration ?

Of course we can put all together inside the CcwMachineState but 
wouldn't the use of a dedicated object make it all cleaner?

Regards,
Pierre

If so, it should be explained in the
> commit log.
> 
> As for now, I see no good justification for S390_CPU_TOPOLOGY and we
> could add support with a simple routine called from insert_stsi_15_1_x().
> 
> Thanks,
> 
> C.

-- 
Pierre Morel
IBM Lab Boeblingen