Re: [Qemu-ppc] [PATCH RFC] spapr: ignore interrupts during reset state

[Nikunj A Dadhania]

David Gibson <address@hidden> writes:

> On Fri, Jun 09, 2017 at 10:32:25AM +0530, Nikunj A Dadhania wrote:
>> David Gibson <address@hidden> 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:

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