"Matheus K. Ferst" <matheus.fe...@eldorado.org.br> writes:
> On 30/09/2022 15:38, Fabiano Rosas wrote:
>> Matheus Ferst <matheus.fe...@eldorado.org.br> writes:
>>
>>> Export p9_interrupt_powersave and use it in p9_next_unmasked_interrupt.
>>>
>>> Signed-off-by: Matheus Ferst <matheus.fe...@eldorado.org.br>
>>> ---
>>> Temporarily putting the prototype in internal.h for lack of a better place,
>>> we will un-export p9_interrupt_powersave in future patches.
>>> ---
>>> target/ppc/cpu_init.c | 2 +-
>>> target/ppc/excp_helper.c | 46 ++++++++++++++++++++++++++++------------
>>> target/ppc/internal.h | 4 ++++
>>> 3 files changed, 38 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
>>> index 1f8f6c6ef2..7889158c52 100644
>>> --- a/target/ppc/cpu_init.c
>>> +++ b/target/ppc/cpu_init.c
>>> @@ -6351,7 +6351,7 @@ static bool ppc_pvr_match_power9(PowerPCCPUClass
>>> *pcc, uint32_t pvr, bool best)
>>> return false;
>>> }
>>>
>>> -static int p9_interrupt_powersave(CPUPPCState *env)
>>> +int p9_interrupt_powersave(CPUPPCState *env)
>>> {
>>> /* External Exception */
>>> if ((env->pending_interrupts & PPC_INTERRUPT_EXT) &&
>>> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
>>> index 67e73f30ab..5a0d2c11a2 100644
>>> --- a/target/ppc/excp_helper.c
>>> +++ b/target/ppc/excp_helper.c
>>> @@ -1686,28 +1686,39 @@ void ppc_cpu_do_interrupt(CPUState *cs)
>>>
>>> static int p9_next_unmasked_interrupt(CPUPPCState *env)
>>> {
>>> - bool async_deliver;
>>> + PowerPCCPU *cpu = env_archcpu(env);
>>> + CPUState *cs = CPU(cpu);
>>> + /* Ignore MSR[EE] when coming out of some power management states */
>>> + bool msr_ee = FIELD_EX64(env->msr, MSR, EE) || env->resume_as_sreset;
>>>
>>> assert((env->pending_interrupts & P9_UNUSED_INTERRUPTS) == 0);
>>>
>>> + if (cs->halted) {
>>> + if (env->spr[SPR_PSSCR] & PSSCR_EC) {
>>> + /*
>>> + * When PSSCR[EC] is set, LPCR[PECE] controls which interrupts
>>> can
>>> + * wakeup the processor
>>> + */
>>> + return p9_interrupt_powersave(env);
>>> + } else {
>>> + /*
>>> + * When it's clear, any system-caused exception exits
>>> power-saving
>>> + * mode, even the ones that gate on MSR[EE].
>>> + */
>>> + msr_ee = true;
>>> + }
>>> + }
>>> +
>>> /* Machine check exception */
>>> if (env->pending_interrupts & PPC_INTERRUPT_MCK) {
>>> return PPC_INTERRUPT_MCK;
>>> }
>>>
>>> - /*
>>> - * 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 = FIELD_EX64(env->msr, MSR, EE) || env->resume_as_sreset;
>>> -
>>
>> You could simplify the code below if you bail early here when !msr_ee.
>>
>
> The next interrupts have checks in the form
>
> if (MSR[EE] && some_condition) || (!MSR[HV] && some_other_condition)
>
> so we cannot return yet. We could check twice for these interrupts, e.g.
Ah, ok. Let's leave like it is then.
>
> if ((env->pending_interrupts & PPC_INTERRUPT_EXT) &&
> (env->has_hv_mode && !FIELD_EX64_HV(env->msr) && !lpes0)) {
> return PPC_INTERRUPT_EXT;
> }
>
> /* ... */
>
> if (!msr_ee) {
> return 0;
> }
>
> /* ... */
>
> if ((env->pending_interrupts & PPC_INTERRUPT_EXT) &&
> !(heic && FIELD_EX64_HV(env->msr) && !FIELD_EX64(env->msr, MSR, PR))) {
> return PPC_INTERRUPT_EXT;
> }
>
> But I'm not sure if it'd be better.
>
>>> /* Hypervisor decrementer exception */
>>> if (env->pending_interrupts & PPC_INTERRUPT_HDECR) {
>>> /* LPCR will be clear when not supported so this will work */
>>> bool hdice = !!(env->spr[SPR_LPCR] & LPCR_HDICE);
>>> - if ((async_deliver || !FIELD_EX64_HV(env->msr)) && hdice) {
>>> + if ((msr_ee || !FIELD_EX64_HV(env->msr)) && hdice) {
>>> /* HDEC clears on delivery */
>>> return PPC_INTERRUPT_HDECR;
>>> }
>>> @@ -1717,7 +1728,7 @@ static int p9_next_unmasked_interrupt(CPUPPCState
>>> *env)
>>> if (env->pending_interrupts & PPC_INTERRUPT_HVIRT) {
>>> /* LPCR will be clear when not supported so this will work */
>>> bool hvice = !!(env->spr[SPR_LPCR] & LPCR_HVICE);
>>> - if ((async_deliver || !FIELD_EX64_HV(env->msr)) && hvice) {
>>> + if ((msr_ee || !FIELD_EX64_HV(env->msr)) && hvice) {
>>> return PPC_INTERRUPT_HVIRT;
>>> }
>>> }
>>> @@ -1727,13 +1738,13 @@ static int p9_next_unmasked_interrupt(CPUPPCState
>>> *env)
>>> bool lpes0 = !!(env->spr[SPR_LPCR] & LPCR_LPES0);
>>> bool heic = !!(env->spr[SPR_LPCR] & LPCR_HEIC);
>>> /* HEIC blocks delivery to the hypervisor */
>>> - if ((async_deliver && !(heic && FIELD_EX64_HV(env->msr) &&
>>> + if ((msr_ee && !(heic && FIELD_EX64_HV(env->msr) &&
>>> !FIELD_EX64(env->msr, MSR, PR))) ||
>>> (env->has_hv_mode && !FIELD_EX64_HV(env->msr) && !lpes0)) {
>>> return PPC_INTERRUPT_EXT;
>>> }
>>> }
>>> - if (async_deliver != 0) {
>>> + if (msr_ee != 0) {
>>> /* Decrementer exception */
>>> if (env->pending_interrupts & PPC_INTERRUPT_DECR) {
>>> return PPC_INTERRUPT_DECR;
>>> @@ -1895,6 +1906,15 @@ static void p9_deliver_interrupt(CPUPPCState *env,
>>> int interrupt)
>>> PowerPCCPU *cpu = env_archcpu(env);
>>> CPUState *cs = env_cpu(env);
>>>
>>> + if (cs->halted && !(env->spr[SPR_PSSCR] & PSSCR_EC) &&
>>> + !FIELD_EX64(env->msr, MSR, EE)) {
>>> + /*
>>> + * A pending interrupt took us out of power-saving, but MSR[EE]
>>> says
>>> + * that we should return to NIP+4 instead of delivering it.
>>> + */
>>> + return;
>>
>> How will the NIP be advanced in this case?
>>
>
> It's already incremented by the translation code. ppc_tr_translate_insn
> increments ctx->base.pc_next before calling decode_{insn{32,64},legacy},
> and methods that put the CPU to sleep will use gen_exception_nip with
> this value as the last argument.
>
>>> + }
>>> +
>>> switch (interrupt) {
>>> case PPC_INTERRUPT_MCK: /* Machine check exception */
>>> env->pending_interrupts &= ~PPC_INTERRUPT_MCK;
>>> diff --git a/target/ppc/internal.h b/target/ppc/internal.h
>>> index 337a362205..41e79adfdb 100644
>>> --- a/target/ppc/internal.h
>>> +++ b/target/ppc/internal.h
>>> @@ -306,4 +306,8 @@ static inline int ger_pack_masks(int pmsk, int ymsk,
>>> int xmsk)
>>> return msk;
>>> }
>>>
>>> +#if defined(TARGET_PPC64)
>>> +int p9_interrupt_powersave(CPUPPCState *env);
>>> +#endif
>>> +
>>> #endif /* PPC_INTERNAL_H */
>
> Thanks,
> Matheus K. Ferst
> Instituto de Pesquisas ELDORADO <http://www.eldorado.org.br/>
> Analista de Software
> Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>
