On Mon, Jan 28, 2019 at 10:46:15AM +0100, Cédric Le Goater wrote: > From: Benjamin Herrenschmidt <b...@kernel.crashing.org> > > When issuing a power management instruction, we set MSR:EE > to force ppc_hw_interrupt() into calling powerpc_excp() > to deal with the fact that on P7 and P8, the system reset > caused by the wakeup needs to be generated regardless of > the MSR:EE value (using LPCR only). > > This however means that the OS will see a bogus SRR1:EE > value which is a problem. It also prevents properly > implementing P9 STOP "light". > > So fix this by instead putting some logic in ppc_hw_interrupt() > to decide whether to deliver or not by taking into account the > fact that we are waking up from sleep. > > The LPCR isn't checked as this is done in the has_work() test. > > Signed-off-by: Benjamin Herrenschmidt <b...@kernel.crashing.org> > Signed-off-by: Cédric Le Goater <c...@kaod.org>
Reviewed-by: David Gibson <da...@gibson.dropbear.id.au>
> ---
> target/ppc/excp_helper.c | 27 +++++++++++++++------------
> 1 file changed, 15 insertions(+), 12 deletions(-)
>
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index 8407e0ade938..7c7c8d1b9dc6 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -748,6 +748,7 @@ void ppc_cpu_do_interrupt(CPUState *cs)
> static void ppc_hw_interrupt(CPUPPCState *env)
> {
> PowerPCCPU *cpu = ppc_env_get_cpu(env);
> + bool async_deliver;
>
> /* External reset */
> if (env->pending_interrupts & (1 << PPC_INTERRUPT_RESET)) {
> @@ -769,11 +770,20 @@ static void ppc_hw_interrupt(CPUPPCState *env)
> return;
> }
> #endif
> +
> + /*
> + * For interrupts that gate on MSR:EE, we need to do something a
> + * bit more subtle, as we need to let them through even when EE is
> + * clear when coming out of some power management states (in order
> + * for them to become a 0x100).
> + */
> + async_deliver = (msr_ee != 0) || env->in_pm_state;
> +
> /* Hypervisor decrementer exception */
> if (env->pending_interrupts & (1 << PPC_INTERRUPT_HDECR)) {
> /* LPCR will be clear when not supported so this will work */
> bool hdice = !!(env->spr[SPR_LPCR] & LPCR_HDICE);
> - if ((msr_ee != 0 || msr_hv == 0) && hdice) {
> + if ((async_deliver || msr_hv == 0) && hdice) {
> /* HDEC clears on delivery */
> env->pending_interrupts &= ~(1 << PPC_INTERRUPT_HDECR);
> powerpc_excp(cpu, env->excp_model, POWERPC_EXCP_HDECR);
> @@ -783,7 +793,7 @@ static void ppc_hw_interrupt(CPUPPCState *env)
> /* Extermal interrupt can ignore MSR:EE under some circumstances */
> if (env->pending_interrupts & (1 << PPC_INTERRUPT_EXT)) {
> bool lpes0 = !!(env->spr[SPR_LPCR] & LPCR_LPES0);
> - if (msr_ee != 0 || (env->has_hv_mode && msr_hv == 0 && !lpes0)) {
> + if (async_deliver || (env->has_hv_mode && msr_hv == 0 && !lpes0)) {
> powerpc_excp(cpu, env->excp_model, POWERPC_EXCP_EXTERNAL);
> return;
> }
> @@ -795,7 +805,7 @@ static void ppc_hw_interrupt(CPUPPCState *env)
> return;
> }
> }
> - if (msr_ee != 0) {
> + if (async_deliver != 0) {
> /* Watchdog timer on embedded PowerPC */
> if (env->pending_interrupts & (1 << PPC_INTERRUPT_WDT)) {
> env->pending_interrupts &= ~(1 << PPC_INTERRUPT_WDT);
> @@ -943,21 +953,14 @@ void helper_pminsn(CPUPPCState *env, powerpc_pm_insn_t
> insn)
>
> cs = CPU(ppc_env_get_cpu(env));
> cs->halted = 1;
> - env->in_pm_state = true;
>
> /* The architecture specifies that HDEC interrupts are
> * discarded in PM states
> */
> env->pending_interrupts &= ~(1 << PPC_INTERRUPT_HDECR);
>
> - /* Technically, nap doesn't set EE, but if we don't set it
> - * then ppc_hw_interrupt() won't deliver. We could add some
> - * other tests there based on LPCR but it's simpler to just
> - * whack EE in. It will be cleared by the 0x100 at wakeup
> - * anyway. It will still be observable by the guest in SRR1
> - * but this doesn't seem to be a problem.
> - */
> - env->msr |= (1ull << MSR_EE);
> + /* Condition for waking up at 0x100 */
> + env->in_pm_state = true;
> }
> #endif /* defined(TARGET_PPC64) */
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
