On 07/04/2016 12:27, Cédric Le Goater wrote: > 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.
What I mean is if we want to keep the previous behavior we should have something like: env->nip = env->hreset_vector | env->excp_prefix; #if defined(TARGET_PPC64) switch((env->spr[SPR_LPCR] & LPCR_AIL) >> LPCR_AIL_SHIFT) { case AIL_0001_8000: env->nip |= 0x18000; break; case AIL_C000_0000_0000_4000: env->nip |= 0xc000000000004000ull; break; } #endif But I don't know how behaves really a POWER8. Laurent > 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 | >>> >> >