Hello Laurent, On 04/07/2016 11:13 AM, Laurent Vivier wrote: > > > On 05/04/2016 04:17, David Gibson wrote: >> From: Cédric Le Goater <c...@fr.ibm.com> >> >> From: Benjamin Herrenschmidt <b...@kernel.crashing.org> >> >> This patch fixes the current AIL implementation for POWER8. The >> interrupt vector address can be calculated directly from LPCR when the >> exception is handled. The excp_prefix update becomes useless and we >> can cleanup the H_SET_MODE hcall. > > I know it's a little bit late to comment this patch but: > > what about the initialization of the NIP in ppc_cpu_reset()? > > env->nip = env->hreset_vector | env->excp_prefix; > > on POWER8 "env->excp_prefix" is always 0, but LPCR can have an AIL defined?
yes. env->spr[SPR_LPCR] still has the previous value at that time and it is reseted right below in the same routine. The cpu should restart in a valid state after that and later on, use the H_SET_MODE hcall from the guest kernel to set the AIL bits back in LPCR. It looks fine to me but I might be missing something. Thanks, C. > > Laurent >> >> Signed-off-by: Benjamin Herrenschmidt <b...@kernel.crashing.org> >> [clg: Removed LPES0/1 handling for HV vs. !HV >> Fixed LPCR_ILE case for POWERPC_EXCP_POWER8 ] >> Signed-off-by: Cédric Le Goater <c...@fr.ibm.com> >> [dwg: This was written as a cleanup, but it also fixes a real bug >> where setting an alternative interrupt location would not be >> correctly migrated] >> Signed-off-by: David Gibson <da...@gibson.dropbear.id.au> >> --- >> hw/ppc/spapr_hcall.c | 16 +-------------- >> include/hw/ppc/spapr.h | 5 ----- >> target-ppc/cpu.h | 10 +++++++++ >> target-ppc/excp_helper.c | 49 >> +++++++++++++++++++++++++++++++++++++++++++-- >> target-ppc/translate_init.c | 2 +- >> 5 files changed, 59 insertions(+), 23 deletions(-) >> >> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c >> index 2dcb676..8f40602 100644 >> --- a/hw/ppc/spapr_hcall.c >> +++ b/hw/ppc/spapr_hcall.c >> @@ -824,7 +824,6 @@ static target_ulong >> h_set_mode_resource_addr_trans_mode(PowerPCCPU *cpu, >> { >> CPUState *cs; >> PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu); >> - target_ulong prefix; >> >> if (!(pcc->insns_flags2 & PPC2_ISA207S)) { >> return H_P2; >> @@ -836,25 +835,12 @@ static target_ulong >> h_set_mode_resource_addr_trans_mode(PowerPCCPU *cpu, >> return H_P4; >> } >> >> - switch (mflags) { >> - case H_SET_MODE_ADDR_TRANS_NONE: >> - prefix = 0; >> - break; >> - case H_SET_MODE_ADDR_TRANS_0001_8000: >> - prefix = 0x18000; >> - break; >> - case H_SET_MODE_ADDR_TRANS_C000_0000_0000_4000: >> - prefix = 0xC000000000004000ULL; >> - break; >> - default: >> + if (mflags == AIL_RESERVED) { >> return H_UNSUPPORTED_FLAG; >> } >> >> CPU_FOREACH(cs) { >> - CPUPPCState *env = &POWERPC_CPU(cpu)->env; >> - >> set_spr(cs, SPR_LPCR, mflags << LPCR_AIL_SHIFT, LPCR_AIL); >> - env->excp_prefix = prefix; >> } >> >> return H_SUCCESS; >> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h >> index 098d85d..815d5ee 100644 >> --- a/include/hw/ppc/spapr.h >> +++ b/include/hw/ppc/spapr.h >> @@ -204,11 +204,6 @@ struct sPAPRMachineState { >> #define H_SET_MODE_ENDIAN_BIG 0 >> #define H_SET_MODE_ENDIAN_LITTLE 1 >> >> -/* Flags for H_SET_MODE_RESOURCE_ADDR_TRANS_MODE */ >> -#define H_SET_MODE_ADDR_TRANS_NONE 0 >> -#define H_SET_MODE_ADDR_TRANS_0001_8000 2 >> -#define H_SET_MODE_ADDR_TRANS_C000_0000_0000_4000 3 >> - >> /* VASI States */ >> #define H_VASI_INVALID 0 >> #define H_VASI_ENABLED 1 >> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h >> index 676081e..9d4e43c 100644 >> --- a/target-ppc/cpu.h >> +++ b/target-ppc/cpu.h >> @@ -167,6 +167,8 @@ enum powerpc_excp_t { >> POWERPC_EXCP_970, >> /* POWER7 exception model */ >> POWERPC_EXCP_POWER7, >> + /* POWER8 exception model */ >> + POWERPC_EXCP_POWER8, >> #endif /* defined(TARGET_PPC64) */ >> }; >> >> @@ -2277,6 +2279,14 @@ enum { >> HMER_XSCOM_STATUS_LSH = (63 - 23), >> }; >> >> +/* Alternate Interrupt Location (AIL) */ >> +enum { >> + AIL_NONE = 0, >> + AIL_RESERVED = 1, >> + AIL_0001_8000 = 2, >> + AIL_C000_0000_0000_4000 = 3, >> +}; >> + >> >> /*****************************************************************************/ >> >> static inline target_ulong cpu_read_xer(CPUPPCState *env) >> diff --git a/target-ppc/excp_helper.c b/target-ppc/excp_helper.c >> index c890853..ca4ffe8 100644 >> --- a/target-ppc/excp_helper.c >> +++ b/target-ppc/excp_helper.c >> @@ -77,7 +77,7 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int >> excp_model, int excp) >> CPUPPCState *env = &cpu->env; >> target_ulong msr, new_msr, vector; >> int srr0, srr1, asrr0, asrr1; >> - int lpes0, lpes1, lev; >> + int lpes0, lpes1, lev, ail; >> >> if (0) { >> /* XXX: find a suitable condition to enable the hypervisor mode */ >> @@ -108,6 +108,25 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int >> excp_model, int excp) >> asrr0 = -1; >> asrr1 = -1; >> >> + /* Exception targetting modifiers >> + * >> + * 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) { >> + if (excp_model == POWERPC_EXCP_POWER8) { >> + ail = (env->spr[SPR_LPCR] & LPCR_AIL) >> LPCR_AIL_SHIFT; >> + } else { >> + ail = 0; >> + } >> + } else >> +#endif /* defined(TARGET_PPC64) */ >> + { >> + ail = 0; >> + } >> + >> switch (excp) { >> case POWERPC_EXCP_NONE: >> /* Should never happen */ >> @@ -146,6 +165,7 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int >> excp_model, int excp) >> /* XXX: find a suitable condition to enable the hypervisor mode >> */ >> new_msr |= (target_ulong)MSR_HVB; >> } >> + ail = 0; >> >> /* machine check exceptions don't have ME set */ >> new_msr &= ~((target_ulong)1 << MSR_ME); >> @@ -344,6 +364,7 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int >> excp_model, int excp) >> /* XXX: find a suitable condition to enable the hypervisor mode >> */ >> new_msr |= (target_ulong)MSR_HVB; >> } >> + ail = 0; >> goto store_next; >> case POWERPC_EXCP_DSEG: /* Data segment exception >> */ >> if (lpes1 == 0) { >> @@ -630,7 +651,8 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int >> excp_model, int excp) >> } >> >> #ifdef TARGET_PPC64 >> - if (excp_model == POWERPC_EXCP_POWER7) { >> + if (excp_model == POWERPC_EXCP_POWER7 || >> + excp_model == POWERPC_EXCP_POWER8) { >> if (env->spr[SPR_LPCR] & LPCR_ILE) { >> new_msr |= (target_ulong)1 << MSR_LE; >> } >> @@ -650,6 +672,29 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int >> excp_model, int excp) >> excp); >> } >> vector |= env->excp_prefix; >> + >> + /* AIL only works if there is no HV transition and we are running with >> + * translations enabled >> + */ >> + if (!((msr >> MSR_IR) & 1) || !((msr >> MSR_DR) & 1)) { >> + ail = 0; >> + } >> + /* Handle AIL */ >> + if (ail) { >> + new_msr |= (1 << MSR_IR) | (1 << MSR_DR); >> + switch(ail) { >> + case AIL_0001_8000: >> + vector |= 0x18000; >> + break; >> + case AIL_C000_0000_0000_4000: >> + vector |= 0xc000000000004000ull; >> + break; >> + default: >> + cpu_abort(cs, "Invalid AIL combination %d\n", ail); >> + break; >> + } >> + } >> + >> #if defined(TARGET_PPC64) >> if (excp_model == POWERPC_EXCP_BOOKE) { >> if (env->spr[SPR_BOOKE_EPCR] & EPCR_ICM) { >> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c >> index 0a33597..f515725 100644 >> --- a/target-ppc/translate_init.c >> +++ b/target-ppc/translate_init.c >> @@ -8487,7 +8487,7 @@ POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data) >> pcc->handle_mmu_fault = ppc_hash64_handle_mmu_fault; >> pcc->sps = &POWER7_POWER8_sps; >> #endif >> - pcc->excp_model = POWERPC_EXCP_POWER7; >> + pcc->excp_model = POWERPC_EXCP_POWER8; >> pcc->bus_model = PPC_FLAGS_INPUT_POWER7; >> pcc->bfd_mach = bfd_mach_ppc64; >> pcc->flags = POWERPC_FLAG_VRE | POWERPC_FLAG_SE | >> >