On Wed, 28 Nov 2018 13:57:14 +1100 David Gibson <da...@gibson.dropbear.id.au> wrote:
> On Fri, Nov 16, 2018 at 11:57:05AM +0100, Cédric Le Goater wrote:
> > We will need to use xics_max_server_number() to create the sPAPRXive
> > object modeling the interrupt controller of the machine which is
> > created before the CPUs.
> >
> > Signed-off-by: Cédric Le Goater <c...@kaod.org>
>
> My only concern here is that this moves the spapr_set_vsmt_mode()
> before some of the sanity checks in spapr_init_cpus(). Are we certain
> there are no edge cases that could cause badness?
>
The early checks in spapr_init_cpus() filter out topologies that would
result in partially filled cores. They're only related to the rest of
the code that creates the boot CPUs. Before commit 1a5008fc17,
spapr_set_vsmt_mode() was even being called before spapr_init_cpus().
The rationale to move it there was to ensure it is called before the
first user of spapr->vsmt, which happens to be a call to
xics_max_server_number().
Now that xics_max_server_number() needs to be called even earlier, I think a
better change is to have xics_max_server_number() to call spapr_set_vsmt_mode()
if spapr->vsmt isn't set.
> > ---
> > hw/ppc/spapr.c | 10 +++++-----
> > 1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 7afd1a175bf2..50cb9f9f4a02 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -2466,11 +2466,6 @@ static void spapr_init_cpus(sPAPRMachineState *spapr)
> > boot_cores_nr = possible_cpus->len;
> > }
> >
> > - /* VSMT must be set in order to be able to compute VCPU ids, ie to
> > - * call xics_max_server_number() or spapr_vcpu_id().
> > - */
> > - spapr_set_vsmt_mode(spapr, &error_fatal);
> > -
> > if (smc->pre_2_10_has_unused_icps) {
> > int i;
> >
> > @@ -2593,6 +2588,11 @@ static void spapr_machine_init(MachineState *machine)
> > /* Setup a load limit for the ramdisk leaving room for SLOF and FDT */
> > load_limit = MIN(spapr->rma_size, RTAS_MAX_ADDR) - FW_OVERHEAD;
> >
> > + /* VSMT must be set in order to be able to compute VCPU ids, ie to
> > + * call xics_max_server_number() or spapr_vcpu_id().
> > + */
> > + spapr_set_vsmt_mode(spapr, &error_fatal);
> > +
> > /* Set up Interrupt Controller before we create the VCPUs */
> > smc->irq->init(spapr, &error_fatal);
> >
>
pgpHArCZq30mG.pgp
Description: OpenPGP digital signature
