On 07/13/2017 06:38 AM, Nikunj A Dadhania wrote: > David Gibson <da...@gibson.dropbear.id.au> writes: > >> On Fri, Jun 09, 2017 at 10:32:25AM +0530, Nikunj A Dadhania wrote: >>> David Gibson <da...@gibson.dropbear.id.au> writes: >>> >>>> On Thu, Jun 08, 2017 at 12:06:08PM +0530, Nikunj A Dadhania wrote: >>>>> Rebooting a SMP TCG guest is broken for both single/multi threaded TCG. >>>> >>>> Ouch. When exactly did this happen? >>> >>> Broken since long >>> >>>> I know that smp boot used to work under TCG, albeit very slowly. >>> >>> SMP boot works, its the reboot issued from the guest doesn't boot and >>> crashes in SLOF. >> >> Oh, sorry, I misunderstood. >> >>> >>>>> When reset happens, all the CPUs are in halted state. First CPU is >>>>> brought out >>>>> of reset and secondary CPUs would be initialized by the guest kernel >>>>> using a >>>>> rtas call start-cpu. >>>>> >>>>> However, in case of TCG, decrementer interrupts keep on coming and waking >>>>> the >>>>> secondary CPUs up. >>>> >>>> Ok.. how is that happening given that the secondary CPUs should have >>>> MSR[EE] == 0? >>> >>> Basically, the CPU is in halted condition and has_work() does not check >>> for MSR_EE in that case. But I am not sure if checking MSR_EE is >>> sufficient, as the CPU does go to halted state (idle) while running as >>> well. >> >> Ok, but we definitely should be able to fix this without new >> variables. If we can quiesce the secondary CPUs for the first boot, >> we should be able to duplicate that for subsequent boots. > > How about the following, we do not report work until MSR_EE is disabled:
With this fix, I could test the XIVE<->XICS transitions at reboot under TCG. However, the second boot is very slow for some reason. Tested-by: Cédric Le Goater <c...@kaod.org> Thanks, C. > diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c > index 783bf98..2cac98a 100644 > --- a/target/ppc/translate_init.c > +++ b/target/ppc/translate_init.c > @@ -8527,6 +8527,9 @@ static bool cpu_has_work_POWER7(CPUState *cs) > CPUPPCState *env = &cpu->env; > > if (cs->halted) { > + if (!msr_ee) { > + return false; > + } > if (!(cs->interrupt_request & CPU_INTERRUPT_HARD)) { > return false; > } > @@ -8684,6 +8687,9 @@ static bool cpu_has_work_POWER8(CPUState *cs) > CPUPPCState *env = &cpu->env; > > if (cs->halted) { > + if (!msr_ee) { > + return false; > + } > if (!(cs->interrupt_request & CPU_INTERRUPT_HARD)) { > return false; > } > @@ -8865,6 +8871,9 @@ static bool cpu_has_work_POWER9(CPUState *cs) > CPUPPCState *env = &cpu->env; > > if (cs->halted) { > + if (!msr_ee) { > + return false; > + } > if (!(cs->interrupt_request & CPU_INTERRUPT_HARD)) { > return false; > } > > Regards > Nikunj > >