Re: [PATCH v16 02/11] s390x/cpu topology: add topology entries on CPU hotplug

[Nina Schoetterl-Glausch]

On Thu, 2023-02-23 at 15:06 +0100, pierre wrote:
> On 2023-02-23 13:53, Thomas Huth wrote:
> > On 22/02/2023 15.20, Pierre Morel wrote:
> > > The topology information are attributes of the CPU and are
> > > specified during the CPU device creation.
> > ...
> > > diff --git a/include/hw/s390x/cpu-topology.h 
> > > b/include/hw/s390x/cpu-topology.h
> > > index 83f31604cc..fa7f885a9f 100644
> > > --- a/include/hw/s390x/cpu-topology.h
> > > +++ b/include/hw/s390x/cpu-topology.h
> > > @@ -10,6 +10,47 @@
> > >   #ifndef HW_S390X_CPU_TOPOLOGY_H
> > >   #define HW_S390X_CPU_TOPOLOGY_H
> > >   +#include "qemu/queue.h"
> > > +#include "hw/boards.h"
> > > +#include "qapi/qapi-types-machine-target.h"
> > > +
> > >   #define S390_TOPOLOGY_CPU_IFL   0x03
> > >   +typedef struct S390Topology {
> > > +    uint8_t *cores_per_socket;
> > > +    CpuTopology *smp;
> > > +    CpuS390Polarization polarization;
> > > +} S390Topology;
> > > +
> > > +#ifdef CONFIG_KVM
> > > +bool s390_has_topology(void);
> > > +void s390_topology_setup_cpu(MachineState *ms, S390CPU *cpu, Error 
> > > **errp);
> > > +#else
> > > +static inline bool s390_has_topology(void)
> > > +{
> > > +       return false;
> > > +}
> > > +static inline void s390_topology_setup_cpu(MachineState *ms,
> > > +                                           S390CPU *cpu,
> > > +                                           Error **errp) {}
> > > +#endif
> > > +
> > > +extern S390Topology s390_topology;
> > > +int s390_socket_nb(S390CPU *cpu);
> > > +
> > > +static inline int s390_std_socket(int n, CpuTopology *smp)
> > > +{
> > > +    return (n / smp->cores) % smp->sockets;
> > > +}
> > > +
> > > +static inline int s390_std_book(int n, CpuTopology *smp)
> > > +{
> > > +    return (n / (smp->cores * smp->sockets)) % smp->books;
> > > +}
> > > +
> > > +static inline int s390_std_drawer(int n, CpuTopology *smp)
> > > +{
> > > +    return (n / (smp->cores * smp->sockets * smp->books)) % 
> > > smp->books;
> > 
> > Shouldn't that be " % smp->drawers" instead?
> 
> /o\  Yes it is of course.
> thanks.

You can also just drop the modulo, since
n < core * sockets * books * drawers. Not that % drawers does any harm ofc.
> 
[...]