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

[pierre <pierre@imap.linux.ibm.com>]

On 2023-02-23 15:13, Nina Schoetterl-Glausch wrote:
> 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.
>> 
> [...]

Yes right, thanks. May be I keep it in case there is another level one 
of these days.

Regards,
Pierre