[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [PATCH 1/2] spapr/rtas: disable the decrementer interrupt
From: |
Cédric Le Goater |
Subject: |
Re: [Qemu-ppc] [PATCH 1/2] spapr/rtas: disable the decrementer interrupt when a CPU is unplugged |
Date: |
Fri, 6 Oct 2017 23:15:31 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 |
On 10/06/2017 11:07 AM, David Gibson wrote:
> On Thu, Oct 05, 2017 at 06:49:58PM +0200, Cédric Le Goater wrote:
>> When a CPU is stopped with the 'stop-self' RTAS call, its state
>> 'halted' is switched to 1 and, in this case, the MSR is not taken into
>> account anymore in the cpu_has_work() routine. Only the pending
>> hardware interrupts are checked with their LPCR:PECE* enablement bit.
>>
>> If the DECR timer fires after 'stop-self' is called and before the CPU
>> 'stop' state is reached, the nearly-dead CPU will have some work to do
>> and the guest will crash. This case happens very frequently with the
>> not yet upstream P9 XIVE exploitation mode. In XICS mode, the DECR is
>> occasionally fired but after 'stop' state, so no work is to be done
>> and the guest survives.
>>
>> I suspect there is a race between the QEMU mainloop triggering the
>> timers and the TCG CPU thread but I could not quite identify the root
>> cause. To be safe, let's disable the decrementer interrupt in the LPCR
>> when the CPU is halted and reenable it when the CPU is restarted.
>>
>> Signed-off-by: Cédric Le Goater <address@hidden>
>> ---
>> hw/ppc/spapr_rtas.c | 16 ++++++++++++++++
>> 1 file changed, 16 insertions(+)
>>
>> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
>> index cdf0b607a0a0..2389220c9738 100644
>> --- a/hw/ppc/spapr_rtas.c
>> +++ b/hw/ppc/spapr_rtas.c
>> @@ -174,6 +174,15 @@ static void rtas_start_cpu(PowerPCCPU *cpu_,
>> sPAPRMachineState *spapr,
>> kvm_cpu_synchronize_state(cs);
>>
>> env->msr = (1ULL << MSR_SF) | (1ULL << MSR_ME);
>> +
>> + /* Enable DECR interrupt */
>> + if (env->mmu_model == POWERPC_MMU_3_00) {
>
> Hm. Checking mmu_model doesn't seem right to me. I mean, it'll get
> the right answer in practice, but the LPCR programming has nothing
> whatsoever to do with the MMU.
>
> I think explicitly checking if cpu_ is a POWER9 instance with
> object_dynamic_cast would be a better option.
OK. So I guess we should change the switch statement in cpu_ppc_set_papr()
also.
C.
>
>> + env->spr[SPR_LPCR] |= LPCR_DEE;
>> + } else {
>> + /* P7 and P8 both have same bit for DECR */
>> + env->spr[SPR_LPCR] |= LPCR_P8_PECE3;
>> + }
>> +
>> env->nip = start;
>> env->gpr[3] = r3;
>> cs->halted = 0;
>> @@ -210,6 +219,13 @@ static void rtas_stop_self(PowerPCCPU *cpu,
>> sPAPRMachineState *spapr,
>> * no need to bother with specific bits, we just clear it.
>> */
>> env->msr = 0;
>> +
>> + if (env->mmu_model == POWERPC_MMU_3_00) {
>> + env->spr[SPR_LPCR] &= ~LPCR_DEE;
>> + } else {
>> + /* P7 and P8 both have same bit for DECR */
>> + env->spr[SPR_LPCR] &= ~LPCR_P8_PECE3;
>> + }
>> }
>>
>> static inline int sysparm_st(target_ulong addr, target_ulong len,
>
[Qemu-ppc] [PATCH 2/2] spapr/rtas: do not reset the MSR in stop-self command, Cédric Le Goater, 2017/10/05
Re: [Qemu-ppc] [PATCH 0/2] disable the decrementer interrupt when a CPU is unplugged, Nikunj A Dadhania, 2017/10/06