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

[Nina Schoetterl-Glausch]

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