Re: [PATCH v14 03/11] target/s390x/cpu topology: handle STSI(15) and build the SYSIB

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

On 1/18/23 12:54, Nina Schoetterl-Glausch wrote:
> On Wed, 2023-01-18 at 11:26 +0100, Thomas Huth wrote:
>> On 17/01/2023 17.56, Pierre Morel wrote:
>>>
>>>
>>> On 1/10/23 15:29, Thomas Huth wrote:
>>>> On 05/01/2023 15.53, Pierre Morel wrote:
>>>>> On interception of STSI(15.1.x) the System Information Block
>>>>> (SYSIB) is built from the list of pre-ordered topology entries.
>>>>>
>>>>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>>>>> ---
>> ...
>>>>> +typedef struct SysIBTl_container {
>>>>> +        uint8_t nl;
>>>>> +        uint8_t reserved[6];
>>>>> +        uint8_t id;
>>>>> +} QEMU_PACKED QEMU_ALIGNED(8) SysIBTl_container;
>>>>> +QEMU_BUILD_BUG_ON(sizeof(SysIBTl_container) != 8);
>>>>> +
>>>>> +/* CPU type Topology List Entry */
>>>>> +typedef struct SysIBTl_cpu {
>>>>> +        uint8_t nl;
>>>>> +        uint8_t reserved0[3];
>>>>> +        uint8_t reserved1:5;
>>>>> +        uint8_t dedicated:1;
>>>>> +        uint8_t polarity:2;
>>>>
>>>> Hmmm, yet another bitfield...
>>>
>>> Yes, this is the firmware interface.
>>> If it makes problem I can use masks and logic arithmetic
>>
>> It depends ... if we are sure that this will ever only be used with KVM on
>> real s390x hardware, then bitfields are OK. If we think that this is
>> something that could be implemented in TCG, too, I'd scratch the bitfields
>> and use logic arithmetic instead...
> 
> Is there something like linux' bitfield.h in qemu?
> In this case it's only two fields, and not too complicated, but I imagine it could
> get quite ugly to do it manually in other cases.
>>
>> I'm not too experienced with this CPU topology stuff, but it sounds like it
>> could be implemented in TCG without too much efforts one day, too, so I'd
>> rather go with the logic arithmetic immediately instead if it is not too
>> annoying for you right now.

No problem, I changed all fields to uint8_t and it fits in a uint64_t so 
I have only a single bit and a single mask to define.

>>
>>>>> diff --git a/target/s390x/kvm/cpu_topology.c
>>>>> b/target/s390x/kvm/cpu_topology.c
>>>>> new file mode 100644
>>>>> index 0000000000..3831a3264c
>>>>> --- /dev/null
>>>>> +++ b/target/s390x/kvm/cpu_topology.c
>>>>> @@ -0,0 +1,136 @@
>>>>> +/*
>>>>> + * QEMU S390x CPU Topology
>>>>> + *
>>>>> + * Copyright IBM Corp. 2022
>>>>
>>>> Happy new year?
>>>
>>> So after Nina's comment what do I do?
>>> let it be 22 because I started last year or update because what is important
>>> is when it comes into mainline?
>>
>> Honestly, I don't have a really good clue either... But keeping 2022 is
>> certainly fine for me, too.

OK then I keep 2022

Regards,

Pierre


-- 
Pierre Morel
IBM Lab Boeblingen