Re: [PATCH v20 02/21] s390x/cpu topology: add topology entries on CPU hotplug

[Cédric Le Goater]

On 5/3/23 09:21, Pierre Morel wrote:
> On 5/2/23 14:30, Cédric Le Goater wrote:
> > On 4/25/23 18:14, Pierre Morel wrote:
> > > The topology information are attributes of the CPU and are
> > > specified during the CPU device creation.
> [...]
> > > +
> > > +/**
> > > + * s390_topology_init:
> > > + * @ms: the machine state where the machine topology is defined
> > > + *
> > > + * Keep track of the machine topology.
> > > + *
> > > + * Allocate an array to keep the count of cores per socket.
> > > + * The index of the array starts at socket 0 from book 0 and
> > > + * drawer 0 up to the maximum allowed by the machine topology.
> > > + */
> > > +static void s390_topology_init(MachineState *ms)
> > > +{
> > > +    CpuTopology *smp = &ms->smp;
> > > +
> > > +    s390_topology.smp = smp;
> >
> > I am not sure the 'smp' shortcut is necessary. 'MachineState *ms' is
> > always available where 'CpuTopology *smp' is used. so it could be
> > computed from a local variable AFAICT. It would reduce the risk of
> > 'smp' being NULL in some (future) code path.
> >
> > Thanks,
> >
> > C.
>
>
> So I will use directly current_machine->smp

There are a couple of place where 'MachineState *ms' can be used directly.
In the other places, routines will need an extra 'CpuTopology *smp'
parameter. If it gets too ugly, current_machine->smp might a better
alternative.

Thanks,

C.