On 11/28/18 10:35 AM, Greg Kurz wrote: > 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.
That 'smt' routine is black magic to me and I would not dare touching it. C. > >>> --- >>> 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); >>> >> >