From: "Cédric Le Goater" <clg@kaod.org>
To: Nicholas Piggin <npiggin@gmail.com>, <qemu-ppc@nongnu.org>
Cc: "Fabiano Rosas" <farosas@linux.ibm.com>,
"Cédric Le Goater" <clg@fr.ibm.com>,
qemu-devel@nongnu.org,
"David Gibson" <david@gibson.dropbear.id.au>
Subject: Re: [EXTERNAL] [RFC PATCH 1/2] target/ppc: rework AIL logic in interrupt delivery
Date: Wed, 14 Apr 2021 17:24:27 +0200 [thread overview]
Message-ID: <eaada833-6df9-a056-b137-9b5fd6643992@kaod.org> (raw)
In-Reply-To: <20210414032343.1720010-2-npiggin@gmail.com>
On 4/14/21 5:23 AM, Nicholas Piggin wrote:
> The AIL logic is becoming unmanageable spread all over powerpc_excp(),
> and it is slated to get even worse with POWER10 support.
>
> Move it all to a new helper function.
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Tested-by: Cédric Le Goater <clg@kaod.org>
Thanks for the effort and the documentation. One minor comment below,
C.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> hw/ppc/spapr_hcall.c | 3 +-
> target/ppc/cpu.h | 8 --
> target/ppc/excp_helper.c | 161 ++++++++++++++++++++------------
> target/ppc/translate_init.c.inc | 2 +-
> 4 files changed, 104 insertions(+), 70 deletions(-)
>
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 7b5cd3553c..2fbe04a689 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -1395,7 +1395,8 @@ static target_ulong h_set_mode_resource_addr_trans_mode(PowerPCCPU *cpu,
> return H_P4;
> }
>
> - if (mflags == AIL_RESERVED) {
> + if (mflags == 1) {
> + /* AIL=1 is reserved */
> return H_UNSUPPORTED_FLAG;
> }
>
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index e73416da68..5200a16d23 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -2375,14 +2375,6 @@ enum {
> HMER_XSCOM_STATUS_MASK = PPC_BITMASK(21, 23),
> };
>
> -/* Alternate Interrupt Location (AIL) */
> -enum {
> - AIL_NONE = 0,
> - AIL_RESERVED = 1,
> - AIL_0001_8000 = 2,
> - AIL_C000_0000_0000_4000 = 3,
> -};
I kind of like these. No big deal.
> -
> /*****************************************************************************/
>
> #define is_isa300(ctx) (!!(ctx->insns_flags2 & PPC2_ISA300))
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index b8881c0f85..9ff316767c 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -136,25 +136,107 @@ static int powerpc_reset_wakeup(CPUState *cs, CPUPPCState *env, int excp,
> return POWERPC_EXCP_RESET;
> }
>
> -static uint64_t ppc_excp_vector_offset(CPUState *cs, int ail)
> +/*
> + * AIL - Alternate Interrupt Location, a mode that allows interrupts to be
> + * taken with the MMU on, and which uses an alternate location (e.g., so the
> + * kernel/hv can map the vectors there with an effective address).
> + *
> + * An interrupt is considered to be taken "with AIL" or "AIL applies" if they
> + * are delivered in this way. AIL requires the LPCR to be set to enable this
> + * mode, and a number of conditions have to be true for AIL to apply.
> + *
> + * First of all, SRESET, MCE, and HMI are always delivered without AIL,
> + * because they are specifically want to be in real mode (e.g., MCE might
> + * be signaling a SLB multi-hit which requires SLB flush before the MMU can
> + * be enabled).
> + *
> + * After that, behaviour depends on the current MSR[IR], MSR[DR], MSR[HV], and
> + * whether or not the interrupt changes MSR[HV] from 0 to 1, and the current
> + * radix mode (LPCR[HR]).
> + *
> + * POWER8, POWER9 with LPCR[HR]=0
> + * | LPCR[AIL] | MSR[IR||DR] | MSR[HV] | new MSR[HV] | AIL |
> + * +-----------+-------------+---------+-------------+-----+
> + * | a | 00/01/10 | x | x | 0 |
> + * | a | 11 | 0 | 1 | 0 |
> + * | a | 11 | 1 | 1 | a |
> + * | a | 11 | 0 | 0 | a |
> + * +-------------------------------------------------------+
> + *
> + * POWER9 with LPCR[HR]=1
> + * | LPCR[AIL] | MSR[IR||DR] | MSR[HV] | new MSR[HV] | AIL |
> + * +-----------+-------------+---------+-------------+-----+
> + * | a | 00/01/10 | x | x | 0 |
> + * | a | 11 | x | x | a |
> + * +-------------------------------------------------------+
> + *
> + * The difference with POWER9 being that MSR[HV] 0->1 interrupts can be
> + * sent to the hypervisor in AIL mode if the guest is radix (LPCR[HR]=1).
> + */
> +static inline void ppc_excp_apply_ail(PowerPCCPU *cpu, int excp_model, int excp,
> + target_ulong msr,
> + target_ulong *new_msr,
> + target_ulong *vector)
> {
> - uint64_t offset = 0;
> +#if defined(TARGET_PPC64)
> + CPUPPCState *env = &cpu->env;
> + bool mmu_all_on = ((msr >> MSR_IR) & 1) && ((msr >> MSR_DR) & 1);
> + bool hv_escalation = !(msr & MSR_HVB) && (*new_msr & MSR_HVB);
> + int ail = 0;
> +
> + if (excp == POWERPC_EXCP_MCHECK ||
> + excp == POWERPC_EXCP_RESET ||
> + excp == POWERPC_EXCP_HV_MAINT) {
> + /* SRESET, MCE, HMI never apply AIL */
> + return;
> + }
>
> - switch (ail) {
> - case AIL_NONE:
> - break;
> - case AIL_0001_8000:
> - offset = 0x18000;
> - break;
> - case AIL_C000_0000_0000_4000:
> - offset = 0xc000000000004000ull;
> - break;
> - default:
> - cpu_abort(cs, "Invalid AIL combination %d\n", ail);
Could we keep this abort ?
> - break;
> + if (excp_model == POWERPC_EXCP_POWER8 ||
> + excp_model == POWERPC_EXCP_POWER9) {
> + if (!mmu_all_on) {
> + /* AIL only works if MSR[IR] and MSR[DR] are both enabled. */
> + return;
> + }
> + if (hv_escalation && !(env->spr[SPR_LPCR] & LPCR_HR)) {
> + /*
> + * AIL does not work if there is a MSR[HV] 0->1 transition and the
> + * partition is in HPT mode. For radix guests, such interrupts are
> + * allowed to be delivered to the hypervisor in ail mode.
> + */
> + return;
> + }
> +
> + ail = (env->spr[SPR_LPCR] & LPCR_AIL) >> LPCR_AIL_SHIFT;
> + if (ail != 2 && ail != 3) {
> + /* AIL=1 is reserved */
> + return;
> + }
> + } else {
> + /* Other processors do not support AIL */
> + return;
> }
>
> - return offset;
> + /*
> + * AIL applies, so the new MSR gets IR and DR set, and an offset applied
> + * to the new IP.
> + */
> + *new_msr |= (1 << MSR_IR) | (1 << MSR_DR);
> +
> + if (excp != POWERPC_EXCP_SYSCALL_VECTORED) {
> + if (ail == 2) {
> + *vector |= 0x0000000000018000ull;
> + } else if (ail == 3) {
> + *vector |= 0xc000000000004000ull;
> + }
> + } else {
> + /* scv AIL is a little different */
> + if (ail == 3) {
> + /* Un-apply the base offset */
> + *vector &= ~0x0000000000017000ull;
> + *vector |= 0xc000000000003000ull;
> + }
> + }
> +#endif
> }
>
> static inline void powerpc_set_excp_state(PowerPCCPU *cpu,
> @@ -197,7 +279,7 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
> CPUState *cs = CPU(cpu);
> CPUPPCState *env = &cpu->env;
> target_ulong msr, new_msr, vector;
> - int srr0, srr1, asrr0, asrr1, lev = -1, ail;
> + int srr0, srr1, asrr0, asrr1, lev = -1;
> bool lpes0;
>
> qemu_log_mask(CPU_LOG_INT, "Raise exception at " TARGET_FMT_lx
> @@ -238,25 +320,16 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
> *
> * On anything else, we behave as if LPES0 is 1
> * (externals don't alter MSR:HV)
> - *
> - * AIL is initialized here but can be cleared by
> - * selected exceptions
> */
> #if defined(TARGET_PPC64)
> if (excp_model == POWERPC_EXCP_POWER7 ||
> excp_model == POWERPC_EXCP_POWER8 ||
> excp_model == POWERPC_EXCP_POWER9) {
> lpes0 = !!(env->spr[SPR_LPCR] & LPCR_LPES0);
> - if (excp_model != POWERPC_EXCP_POWER7) {
> - ail = (env->spr[SPR_LPCR] & LPCR_AIL) >> LPCR_AIL_SHIFT;
> - } else {
> - ail = 0;
> - }
> } else
> #endif /* defined(TARGET_PPC64) */
> {
> lpes0 = true;
> - ail = 0;
> }
>
> /*
> @@ -315,7 +388,6 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
> */
> new_msr |= (target_ulong)MSR_HVB;
> }
> - ail = 0;
>
> /* machine check exceptions don't have ME set */
> new_msr &= ~((target_ulong)1 << MSR_ME);
> @@ -519,7 +591,6 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
> "exception %d with no HV support\n", excp);
> }
> }
> - ail = 0;
> break;
> case POWERPC_EXCP_DSEG: /* Data segment exception */
> case POWERPC_EXCP_ISEG: /* Instruction segment exception */
> @@ -790,24 +861,6 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
> }
> #endif
>
> - /*
> - * AIL only works if MSR[IR] and MSR[DR] are both enabled.
> - */
> - if (!((msr >> MSR_IR) & 1) || !((msr >> MSR_DR) & 1)) {
> - ail = 0;
> - }
> -
> - /*
> - * AIL does not work if there is a MSR[HV] 0->1 transition and the
> - * partition is in HPT mode. For radix guests, such interrupts are
> - * allowed to be delivered to the hypervisor in ail mode.
> - */
> - if ((new_msr & MSR_HVB) && !(msr & MSR_HVB)) {
> - if (!(env->spr[SPR_LPCR] & LPCR_HR)) {
> - ail = 0;
> - }
> - }
> -
> vector = env->excp_vectors[excp];
> if (vector == (target_ulong)-1ULL) {
> cpu_abort(cs, "Raised an exception without defined vector %d\n",
> @@ -848,23 +901,8 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
> /* Save MSR */
> env->spr[srr1] = msr;
>
> - /* Handle AIL */
> - if (ail) {
> - new_msr |= (1 << MSR_IR) | (1 << MSR_DR);
> - vector |= ppc_excp_vector_offset(cs, ail);
> - }
> -
> #if defined(TARGET_PPC64)
> } else {
> - /* scv AIL is a little different */
> - if (ail) {
> - new_msr |= (1 << MSR_IR) | (1 << MSR_DR);
> - }
> - if (ail == AIL_C000_0000_0000_4000) {
> - vector |= 0xc000000000003000ull;
> - } else {
> - vector |= 0x0000000000017000ull;
> - }
> vector += lev * 0x20;
>
> env->lr = env->nip;
> @@ -872,6 +910,9 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
> #endif
> }
>
> + /* This can update new_msr and vector if AIL applies */
> + ppc_excp_apply_ail(cpu, excp_model, excp, msr, &new_msr, &vector);
> +
> powerpc_set_excp_state(cpu, vector, new_msr);
> }
>
> diff --git a/target/ppc/translate_init.c.inc b/target/ppc/translate_init.c.inc
> index 70f9b9b150..a82d9ed647 100644
> --- a/target/ppc/translate_init.c.inc
> +++ b/target/ppc/translate_init.c.inc
> @@ -3457,7 +3457,7 @@ static void init_excp_POWER9(CPUPPCState *env)
>
> #if !defined(CONFIG_USER_ONLY)
> env->excp_vectors[POWERPC_EXCP_HVIRT] = 0x00000EA0;
> - env->excp_vectors[POWERPC_EXCP_SYSCALL_VECTORED] = 0x00000000;
> + env->excp_vectors[POWERPC_EXCP_SYSCALL_VECTORED] = 0x00017000;
> #endif
> }
>
next prev parent reply other threads:[~2021-04-14 15:26 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-14 3:23 [RFC PATCH 0/2] ppc: rework AIL logic, add POWER10 exception model Nicholas Piggin
2021-04-14 3:23 ` [RFC PATCH 1/2] target/ppc: rework AIL logic in interrupt delivery Nicholas Piggin
2021-04-14 15:24 ` Cédric Le Goater [this message]
2021-04-15 5:25 ` [EXTERNAL] " Nicholas Piggin
2021-04-14 3:23 ` [RFC PATCH 2/2] target/ppc: Add POWER10 exception model Nicholas Piggin
2021-04-14 15:54 ` [EXTERNAL] " Cédric Le Goater
2021-04-15 5:28 ` Nicholas Piggin
2021-04-15 6:50 ` Cédric Le Goater
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=eaada833-6df9-a056-b137-9b5fd6643992@kaod.org \
--to=clg@kaod.org \
--cc=clg@fr.ibm.com \
--cc=david@gibson.dropbear.id.au \
--cc=farosas@linux.ibm.com \
--cc=npiggin@gmail.com \
--cc=qemu-devel@nongnu.org \
--cc=qemu-ppc@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).