* [PATCH] target/ppc: Do not check for LPCR[HAIL] on power10_v1.0 CPUs @ 2021-05-04 9:59 Cédric Le Goater 2021-05-04 10:49 ` Nicholas Piggin 0 siblings, 1 reply; 4+ messages in thread From: Cédric Le Goater @ 2021-05-04 9:59 UTC (permalink / raw) To: David Gibson Cc: Cédric Le Goater, qemu-ppc, Greg Kurz, Nicholas Piggin, qemu-devel The LPCR[HAIL] bit only applies to POWER10 DD2 CPUs. On POWER10 DD1, the ail value should be extracted using the LPCR_AIL mask like on P9. Cc: Nicholas Piggin <npiggin@gmail.com> Signed-off-by: Cédric Le Goater <clg@kaod.org> --- target/ppc/excp_helper.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c index c0605f84d70f..ba18444f5d1e 100644 --- a/target/ppc/excp_helper.c +++ b/target/ppc/excp_helper.c @@ -198,6 +198,7 @@ static inline void ppc_excp_apply_ail(PowerPCCPU *cpu, int excp_model, int excp, target_ulong *vector) { #if defined(TARGET_PPC64) + PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu); 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); @@ -240,7 +241,7 @@ static inline void ppc_excp_apply_ail(PowerPCCPU *cpu, int excp_model, int excp, return; } - if (*new_msr & MSR_HVB) { + if (*new_msr & MSR_HVB && pcc->lpcr_mask & LPCR_HAIL) { if (!(env->spr[SPR_LPCR] & LPCR_HAIL)) { /* HV interrupts depend on LPCR[HAIL] */ return; -- 2.26.3 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] target/ppc: Do not check for LPCR[HAIL] on power10_v1.0 CPUs 2021-05-04 9:59 [PATCH] target/ppc: Do not check for LPCR[HAIL] on power10_v1.0 CPUs Cédric Le Goater @ 2021-05-04 10:49 ` Nicholas Piggin 2021-05-04 11:54 ` Cédric Le Goater 0 siblings, 1 reply; 4+ messages in thread From: Nicholas Piggin @ 2021-05-04 10:49 UTC (permalink / raw) To: Cédric Le Goater, David Gibson; +Cc: qemu-ppc, Greg Kurz, qemu-devel Excerpts from Cédric Le Goater's message of May 4, 2021 7:59 pm: > The LPCR[HAIL] bit only applies to POWER10 DD2 CPUs. On POWER10 DD1, > the ail value should be extracted using the LPCR_AIL mask like on P9. > > Cc: Nicholas Piggin <npiggin@gmail.com> > Signed-off-by: Cédric Le Goater <clg@kaod.org> Thanks for this, my oversight for not realising the P10 CPU is DD1 (which doesn't have HAIL). I wonder if it could just use the POWER9 excp_model? > --- > target/ppc/excp_helper.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c > index c0605f84d70f..ba18444f5d1e 100644 > --- a/target/ppc/excp_helper.c > +++ b/target/ppc/excp_helper.c > @@ -198,6 +198,7 @@ static inline void ppc_excp_apply_ail(PowerPCCPU *cpu, int excp_model, int excp, > target_ulong *vector) > { > #if defined(TARGET_PPC64) > + PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu); > 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); > @@ -240,7 +241,7 @@ static inline void ppc_excp_apply_ail(PowerPCCPU *cpu, int excp_model, int excp, > return; > } > > - if (*new_msr & MSR_HVB) { > + if (*new_msr & MSR_HVB && pcc->lpcr_mask & LPCR_HAIL) { > if (!(env->spr[SPR_LPCR] & LPCR_HAIL)) { > /* HV interrupts depend on LPCR[HAIL] */ > return; > -- > 2.26.3 > > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] target/ppc: Do not check for LPCR[HAIL] on power10_v1.0 CPUs 2021-05-04 10:49 ` Nicholas Piggin @ 2021-05-04 11:54 ` Cédric Le Goater 2021-05-05 5:00 ` David Gibson 0 siblings, 1 reply; 4+ messages in thread From: Cédric Le Goater @ 2021-05-04 11:54 UTC (permalink / raw) To: Nicholas Piggin, David Gibson; +Cc: qemu-ppc, Greg Kurz, qemu-devel On 5/4/21 12:49 PM, Nicholas Piggin wrote: > Excerpts from Cédric Le Goater's message of May 4, 2021 7:59 pm: >> The LPCR[HAIL] bit only applies to POWER10 DD2 CPUs. On POWER10 DD1, >> the ail value should be extracted using the LPCR_AIL mask like on P9. >> >> Cc: Nicholas Piggin <npiggin@gmail.com> >> Signed-off-by: Cédric Le Goater <clg@kaod.org> > > Thanks for this, my oversight for not realising the P10 CPU is DD1 > (which doesn't have HAIL). > > I wonder if it could just use the POWER9 excp_model? Yes. Why not. It does bring up another problem which is how to define (cleanly) different characteristics for CPUs of the same POWER family. Currently, all P10s are under POWERPC_FAMILY(POWER10). This is a base abstract class and definitions can not depend on the PVR. See below what needs to be done to add a custom LPCR mask for DD2 :/ We could also simply switch P10 to DD2. I would favor that instead of adding complexity. David, what is your opinion on this ? Thank, C. Signed-off-by: Cédric Le Goater <clg@kaod.org> --- target/ppc/cpu-models.c | 13 +++++++++++-- target/ppc/cpu-models.h | 1 + 2 files changed, 12 insertions(+), 2 deletions(-) Index: qemu-powernv-6.1.git/target/ppc/cpu-models.c =================================================================== --- qemu-powernv-6.1.git.orig/target/ppc/cpu-models.c +++ qemu-powernv-6.1.git/target/ppc/cpu-models.c @@ -32,7 +32,7 @@ /* PowerPC CPU definitions */ #define POWERPC_DEF_PREFIX(pvr, svr, type) \ glue(glue(glue(glue(pvr, _), svr), _), type) -#define POWERPC_DEF_SVR(_name, _desc, _pvr, _svr, _type) \ +#define __POWERPC_DEF_SVR(_name, _desc, _pvr, _svr, _type, _lpcr) \ static void \ glue(POWERPC_DEF_PREFIX(_pvr, _svr, _type), _cpu_class_init) \ (ObjectClass *oc, void *data) \ @@ -40,6 +40,7 @@ DeviceClass *dc = DEVICE_CLASS(oc); \ PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc); \ \ + pcc->lpcr_mask |= _lpcr; \ pcc->pvr = _pvr; \ pcc->svr = _svr; \ dc->desc = _desc; \ @@ -63,6 +64,12 @@ type_init( \ glue(POWERPC_DEF_PREFIX(_pvr, _svr, _type), _cpu_register_types)) +#define POWERPC_DEF_SVR(_name, _desc, _pvr, _svr, _type) \ + __POWERPC_DEF_SVR(_name, _desc, _pvr, _svr, _type, 0) + +#define POWERPC_DEF_LPCR(_name, _pvr, _type, _desc, _lpcr) \ + __POWERPC_DEF_SVR(_name, _desc, _pvr, POWERPC_SVR_NONE, _type, _lpcr) + #define POWERPC_DEF(_name, _pvr, _type, _desc) \ POWERPC_DEF_SVR(_name, _desc, _pvr, POWERPC_SVR_NONE, _type) @@ -776,6 +783,8 @@ "POWER9 v2.0") POWERPC_DEF("power10_v1.0", CPU_POWERPC_POWER10_DD1, POWER10, "POWER10 v1.0") + POWERPC_DEF_LPCR("power10_v2.0", CPU_POWERPC_POWER10_DD20, POWER10, + "POWER10 v2.0", LPCR_HAIL) #endif /* defined (TARGET_PPC64) */ /***************************************************************************/ @@ -952,7 +961,7 @@ PowerPCCPUAlias ppc_cpu_aliases[] = { { "power8", "power8_v2.0" }, { "power8nvl", "power8nvl_v1.0" }, { "power9", "power9_v2.0" }, - { "power10", "power10_v1.0" }, + { "power10", "power10_v2.0" }, #endif /* Generic PowerPCs */ Index: qemu-powernv-6.1.git/target/ppc/cpu-models.h =================================================================== --- qemu-powernv-6.1.git.orig/target/ppc/cpu-models.h +++ qemu-powernv-6.1.git/target/ppc/cpu-models.h @@ -375,6 +375,7 @@ enum { CPU_POWERPC_POWER9_DD20 = 0x004E1200, CPU_POWERPC_POWER10_BASE = 0x00800000, CPU_POWERPC_POWER10_DD1 = 0x00800100, + CPU_POWERPC_POWER10_DD20 = 0x00800200, CPU_POWERPC_970_v22 = 0x00390202, CPU_POWERPC_970FX_v10 = 0x00391100, CPU_POWERPC_970FX_v20 = 0x003C0200, ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] target/ppc: Do not check for LPCR[HAIL] on power10_v1.0 CPUs 2021-05-04 11:54 ` Cédric Le Goater @ 2021-05-05 5:00 ` David Gibson 0 siblings, 0 replies; 4+ messages in thread From: David Gibson @ 2021-05-05 5:00 UTC (permalink / raw) To: Cédric Le Goater; +Cc: qemu-ppc, Greg Kurz, Nicholas Piggin, qemu-devel [-- Attachment #1: Type: text/plain, Size: 5211 bytes --] On Tue, May 04, 2021 at 01:54:39PM +0200, Cédric Le Goater wrote: > On 5/4/21 12:49 PM, Nicholas Piggin wrote: > > Excerpts from Cédric Le Goater's message of May 4, 2021 7:59 pm: > >> The LPCR[HAIL] bit only applies to POWER10 DD2 CPUs. On POWER10 DD1, > >> the ail value should be extracted using the LPCR_AIL mask like on P9. > >> > >> Cc: Nicholas Piggin <npiggin@gmail.com> > >> Signed-off-by: Cédric Le Goater <clg@kaod.org> > > > > Thanks for this, my oversight for not realising the P10 CPU is DD1 > > (which doesn't have HAIL). > > > > I wonder if it could just use the POWER9 excp_model? > > Yes. Why not. It does bring up another problem which is how to define > (cleanly) different characteristics for CPUs of the same POWER family. > > Currently, all P10s are under POWERPC_FAMILY(POWER10). This is a base > abstract class and definitions can not depend on the PVR. See below > what needs to be done to add a custom LPCR mask for DD2 :/ > > We could also simply switch P10 to DD2. I would favor that instead of > adding complexity. Definitely. I'm guessing DD1 POWER10s will never be seen outside IBM, so I don't think we want support for them in upstream qemu at all. > > David, what is your opinion on this ? > > Thank, > > C. > > > Signed-off-by: Cédric Le Goater <clg@kaod.org> > --- > target/ppc/cpu-models.c | 13 +++++++++++-- > target/ppc/cpu-models.h | 1 + > 2 files changed, 12 insertions(+), 2 deletions(-) > > Index: qemu-powernv-6.1.git/target/ppc/cpu-models.c > =================================================================== > --- qemu-powernv-6.1.git.orig/target/ppc/cpu-models.c > +++ qemu-powernv-6.1.git/target/ppc/cpu-models.c > @@ -32,7 +32,7 @@ > /* PowerPC CPU definitions */ > #define POWERPC_DEF_PREFIX(pvr, svr, type) \ > glue(glue(glue(glue(pvr, _), svr), _), type) > -#define POWERPC_DEF_SVR(_name, _desc, _pvr, _svr, _type) \ > +#define __POWERPC_DEF_SVR(_name, _desc, _pvr, _svr, _type, _lpcr) \ > static void \ > glue(POWERPC_DEF_PREFIX(_pvr, _svr, _type), _cpu_class_init) \ > (ObjectClass *oc, void *data) \ > @@ -40,6 +40,7 @@ > DeviceClass *dc = DEVICE_CLASS(oc); \ > PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc); \ > \ > + pcc->lpcr_mask |= _lpcr; \ > pcc->pvr = _pvr; \ > pcc->svr = _svr; \ > dc->desc = _desc; \ > @@ -63,6 +64,12 @@ > type_init( \ > glue(POWERPC_DEF_PREFIX(_pvr, _svr, _type), _cpu_register_types)) > > +#define POWERPC_DEF_SVR(_name, _desc, _pvr, _svr, _type) \ > + __POWERPC_DEF_SVR(_name, _desc, _pvr, _svr, _type, 0) > + > +#define POWERPC_DEF_LPCR(_name, _pvr, _type, _desc, _lpcr) \ > + __POWERPC_DEF_SVR(_name, _desc, _pvr, POWERPC_SVR_NONE, _type, _lpcr) > + > #define POWERPC_DEF(_name, _pvr, _type, _desc) \ > POWERPC_DEF_SVR(_name, _desc, _pvr, POWERPC_SVR_NONE, _type) > > @@ -776,6 +783,8 @@ > "POWER9 v2.0") > POWERPC_DEF("power10_v1.0", CPU_POWERPC_POWER10_DD1, POWER10, > "POWER10 v1.0") > + POWERPC_DEF_LPCR("power10_v2.0", CPU_POWERPC_POWER10_DD20, POWER10, > + "POWER10 v2.0", LPCR_HAIL) > #endif /* defined (TARGET_PPC64) */ > > /***************************************************************************/ > @@ -952,7 +961,7 @@ PowerPCCPUAlias ppc_cpu_aliases[] = { > { "power8", "power8_v2.0" }, > { "power8nvl", "power8nvl_v1.0" }, > { "power9", "power9_v2.0" }, > - { "power10", "power10_v1.0" }, > + { "power10", "power10_v2.0" }, > #endif > > /* Generic PowerPCs */ > Index: qemu-powernv-6.1.git/target/ppc/cpu-models.h > =================================================================== > --- qemu-powernv-6.1.git.orig/target/ppc/cpu-models.h > +++ qemu-powernv-6.1.git/target/ppc/cpu-models.h > @@ -375,6 +375,7 @@ enum { > CPU_POWERPC_POWER9_DD20 = 0x004E1200, > CPU_POWERPC_POWER10_BASE = 0x00800000, > CPU_POWERPC_POWER10_DD1 = 0x00800100, > + CPU_POWERPC_POWER10_DD20 = 0x00800200, > CPU_POWERPC_970_v22 = 0x00390202, > CPU_POWERPC_970FX_v10 = 0x00391100, > CPU_POWERPC_970FX_v20 = 0x003C0200, > -- 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 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-05-05 5:04 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-05-04 9:59 [PATCH] target/ppc: Do not check for LPCR[HAIL] on power10_v1.0 CPUs Cédric Le Goater 2021-05-04 10:49 ` Nicholas Piggin 2021-05-04 11:54 ` Cédric Le Goater 2021-05-05 5:00 ` David Gibson
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).