* [PATCH v2 1/7] target/ppc: use locally stored msr and avoid indirect access
2024-05-23 5:14 [PATCH v2 0/7] target/ppc: misc ppc improvements/optimizations Harsh Prateek Bora
@ 2024-05-23 5:14 ` Harsh Prateek Bora
2024-07-04 7:36 ` Nicholas Piggin
2024-07-04 7:42 ` Nicholas Piggin
2024-05-23 5:14 ` [PATCH v2 2/7] target/ppc: optimize hreg_compute_pmu_hflags_value Harsh Prateek Bora
` (5 subsequent siblings)
6 siblings, 2 replies; 16+ messages in thread
From: Harsh Prateek Bora @ 2024-05-23 5:14 UTC (permalink / raw)
To: qemu-ppc, qemu-devel; +Cc: npiggin, balaton, danielhb413
hreg_compute_hflags_value already stores msr locally to be used in most
of the logic in the routine however some instances are still using
env->msr which is unnecessary. Use locally stored value as available.
Signed-off-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
---
target/ppc/helper_regs.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c
index 25258986e3..945fa1a596 100644
--- a/target/ppc/helper_regs.c
+++ b/target/ppc/helper_regs.c
@@ -106,10 +106,10 @@ static uint32_t hreg_compute_hflags_value(CPUPPCState *env)
if (ppc_flags & POWERPC_FLAG_DE) {
target_ulong dbcr0 = env->spr[SPR_BOOKE_DBCR0];
- if ((dbcr0 & DBCR0_ICMP) && FIELD_EX64(env->msr, MSR, DE)) {
+ if ((dbcr0 & DBCR0_ICMP) && FIELD_EX64(msr, MSR, DE)) {
hflags |= 1 << HFLAGS_SE;
}
- if ((dbcr0 & DBCR0_BRT) && FIELD_EX64(env->msr, MSR, DE)) {
+ if ((dbcr0 & DBCR0_BRT) && FIELD_EX64(msr, MSR, DE)) {
hflags |= 1 << HFLAGS_BE;
}
} else {
--
2.39.3
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v2 1/7] target/ppc: use locally stored msr and avoid indirect access
2024-05-23 5:14 ` [PATCH v2 1/7] target/ppc: use locally stored msr and avoid indirect access Harsh Prateek Bora
@ 2024-07-04 7:36 ` Nicholas Piggin
2024-07-04 7:42 ` Nicholas Piggin
1 sibling, 0 replies; 16+ messages in thread
From: Nicholas Piggin @ 2024-07-04 7:36 UTC (permalink / raw)
To: Harsh Prateek Bora, qemu-ppc, qemu-devel; +Cc: balaton, danielhb413
On Thu May 23, 2024 at 3:14 PM AEST, Harsh Prateek Bora wrote:
> hreg_compute_hflags_value already stores msr locally to be used in most
> of the logic in the routine however some instances are still using
> env->msr which is unnecessary. Use locally stored value as available.
>
> Signed-off-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> target/ppc/helper_regs.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c
> index 25258986e3..945fa1a596 100644
> --- a/target/ppc/helper_regs.c
> +++ b/target/ppc/helper_regs.c
> @@ -106,10 +106,10 @@ static uint32_t hreg_compute_hflags_value(CPUPPCState *env)
>
> if (ppc_flags & POWERPC_FLAG_DE) {
> target_ulong dbcr0 = env->spr[SPR_BOOKE_DBCR0];
> - if ((dbcr0 & DBCR0_ICMP) && FIELD_EX64(env->msr, MSR, DE)) {
> + if ((dbcr0 & DBCR0_ICMP) && FIELD_EX64(msr, MSR, DE)) {
> hflags |= 1 << HFLAGS_SE;
> }
> - if ((dbcr0 & DBCR0_BRT) && FIELD_EX64(env->msr, MSR, DE)) {
> + if ((dbcr0 & DBCR0_BRT) && FIELD_EX64(msr, MSR, DE)) {
> hflags |= 1 << HFLAGS_BE;
> }
> } else {
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v2 1/7] target/ppc: use locally stored msr and avoid indirect access
2024-05-23 5:14 ` [PATCH v2 1/7] target/ppc: use locally stored msr and avoid indirect access Harsh Prateek Bora
2024-07-04 7:36 ` Nicholas Piggin
@ 2024-07-04 7:42 ` Nicholas Piggin
1 sibling, 0 replies; 16+ messages in thread
From: Nicholas Piggin @ 2024-07-04 7:42 UTC (permalink / raw)
To: Harsh Prateek Bora, qemu-ppc, qemu-devel; +Cc: balaton, danielhb413
On Thu May 23, 2024 at 3:14 PM AEST, Harsh Prateek Bora wrote:
> hreg_compute_hflags_value already stores msr locally to be used in most
> of the logic in the routine however some instances are still using
> env->msr which is unnecessary. Use locally stored value as available.
BTW hreg_store_msr uses env->msr a bunch of times. Would a local
variable improve that too?
Thanks,
Nick
>
> Signed-off-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
> ---
> target/ppc/helper_regs.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c
> index 25258986e3..945fa1a596 100644
> --- a/target/ppc/helper_regs.c
> +++ b/target/ppc/helper_regs.c
> @@ -106,10 +106,10 @@ static uint32_t hreg_compute_hflags_value(CPUPPCState *env)
>
> if (ppc_flags & POWERPC_FLAG_DE) {
> target_ulong dbcr0 = env->spr[SPR_BOOKE_DBCR0];
> - if ((dbcr0 & DBCR0_ICMP) && FIELD_EX64(env->msr, MSR, DE)) {
> + if ((dbcr0 & DBCR0_ICMP) && FIELD_EX64(msr, MSR, DE)) {
> hflags |= 1 << HFLAGS_SE;
> }
> - if ((dbcr0 & DBCR0_BRT) && FIELD_EX64(env->msr, MSR, DE)) {
> + if ((dbcr0 & DBCR0_BRT) && FIELD_EX64(msr, MSR, DE)) {
> hflags |= 1 << HFLAGS_BE;
> }
> } else {
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 2/7] target/ppc: optimize hreg_compute_pmu_hflags_value
2024-05-23 5:14 [PATCH v2 0/7] target/ppc: misc ppc improvements/optimizations Harsh Prateek Bora
2024-05-23 5:14 ` [PATCH v2 1/7] target/ppc: use locally stored msr and avoid indirect access Harsh Prateek Bora
@ 2024-05-23 5:14 ` Harsh Prateek Bora
2024-07-04 7:37 ` Nicholas Piggin
2024-05-23 5:14 ` [PATCH v2 3/7] " Harsh Prateek Bora
` (4 subsequent siblings)
6 siblings, 1 reply; 16+ messages in thread
From: Harsh Prateek Bora @ 2024-05-23 5:14 UTC (permalink / raw)
To: qemu-ppc, qemu-devel; +Cc: npiggin, balaton, danielhb413
Cache env->spr[SPR_POWER_MMCR0] in a local variable as used in multiple
conditions to avoid multiple indirect accesses.
Signed-off-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
---
target/ppc/helper_regs.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c
index 945fa1a596..d09dcacd5e 100644
--- a/target/ppc/helper_regs.c
+++ b/target/ppc/helper_regs.c
@@ -50,15 +50,16 @@ void hreg_swap_gpr_tgpr(CPUPPCState *env)
static uint32_t hreg_compute_pmu_hflags_value(CPUPPCState *env)
{
uint32_t hflags = 0;
-
#if defined(TARGET_PPC64)
- if (env->spr[SPR_POWER_MMCR0] & MMCR0_PMCC0) {
+ target_ulong mmcr0 = env->spr[SPR_POWER_MMCR0];
+
+ if (mmcr0 & MMCR0_PMCC0) {
hflags |= 1 << HFLAGS_PMCC0;
}
- if (env->spr[SPR_POWER_MMCR0] & MMCR0_PMCC1) {
+ if (mmcr0 & MMCR0_PMCC1) {
hflags |= 1 << HFLAGS_PMCC1;
}
- if (env->spr[SPR_POWER_MMCR0] & MMCR0_PMCjCE) {
+ if (mmcr0 & MMCR0_PMCjCE) {
hflags |= 1 << HFLAGS_PMCJCE;
}
--
2.39.3
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v2 2/7] target/ppc: optimize hreg_compute_pmu_hflags_value
2024-05-23 5:14 ` [PATCH v2 2/7] target/ppc: optimize hreg_compute_pmu_hflags_value Harsh Prateek Bora
@ 2024-07-04 7:37 ` Nicholas Piggin
0 siblings, 0 replies; 16+ messages in thread
From: Nicholas Piggin @ 2024-07-04 7:37 UTC (permalink / raw)
To: Harsh Prateek Bora, qemu-ppc, qemu-devel; +Cc: balaton, danielhb413
On Thu May 23, 2024 at 3:14 PM AEST, Harsh Prateek Bora wrote:
> Cache env->spr[SPR_POWER_MMCR0] in a local variable as used in multiple
> conditions to avoid multiple indirect accesses.
>
> Signed-off-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
Compiler might cache it in a reg, but anyway I like it.
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> target/ppc/helper_regs.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c
> index 945fa1a596..d09dcacd5e 100644
> --- a/target/ppc/helper_regs.c
> +++ b/target/ppc/helper_regs.c
> @@ -50,15 +50,16 @@ void hreg_swap_gpr_tgpr(CPUPPCState *env)
> static uint32_t hreg_compute_pmu_hflags_value(CPUPPCState *env)
> {
> uint32_t hflags = 0;
> -
> #if defined(TARGET_PPC64)
> - if (env->spr[SPR_POWER_MMCR0] & MMCR0_PMCC0) {
> + target_ulong mmcr0 = env->spr[SPR_POWER_MMCR0];
> +
> + if (mmcr0 & MMCR0_PMCC0) {
> hflags |= 1 << HFLAGS_PMCC0;
> }
> - if (env->spr[SPR_POWER_MMCR0] & MMCR0_PMCC1) {
> + if (mmcr0 & MMCR0_PMCC1) {
> hflags |= 1 << HFLAGS_PMCC1;
> }
> - if (env->spr[SPR_POWER_MMCR0] & MMCR0_PMCjCE) {
> + if (mmcr0 & MMCR0_PMCjCE) {
> hflags |= 1 << HFLAGS_PMCJCE;
> }
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 3/7] target/ppc: optimize hreg_compute_pmu_hflags_value
2024-05-23 5:14 [PATCH v2 0/7] target/ppc: misc ppc improvements/optimizations Harsh Prateek Bora
2024-05-23 5:14 ` [PATCH v2 1/7] target/ppc: use locally stored msr and avoid indirect access Harsh Prateek Bora
2024-05-23 5:14 ` [PATCH v2 2/7] target/ppc: optimize hreg_compute_pmu_hflags_value Harsh Prateek Bora
@ 2024-05-23 5:14 ` Harsh Prateek Bora
2024-07-04 7:38 ` Nicholas Piggin
2024-05-23 5:14 ` [PATCH v2 4/7] target/ppc: optimize p9 exception handling routines Harsh Prateek Bora
` (3 subsequent siblings)
6 siblings, 1 reply; 16+ messages in thread
From: Harsh Prateek Bora @ 2024-05-23 5:14 UTC (permalink / raw)
To: qemu-ppc, qemu-devel; +Cc: npiggin, balaton, danielhb413
The second if-condition can be true only if the first one above is true.
Enclose the latter into the former to avoid un-necessary check if first
condition fails.
Signed-off-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>
---
target/ppc/helper_regs.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c
index d09dcacd5e..261a8ba79f 100644
--- a/target/ppc/helper_regs.c
+++ b/target/ppc/helper_regs.c
@@ -66,9 +66,9 @@ static uint32_t hreg_compute_pmu_hflags_value(CPUPPCState *env)
#ifndef CONFIG_USER_ONLY
if (env->pmc_ins_cnt) {
hflags |= 1 << HFLAGS_INSN_CNT;
- }
- if (env->pmc_ins_cnt & 0x1e) {
- hflags |= 1 << HFLAGS_PMC_OTHER;
+ if (env->pmc_ins_cnt & 0x1e) {
+ hflags |= 1 << HFLAGS_PMC_OTHER;
+ }
}
#endif
#endif
--
2.39.3
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v2 3/7] target/ppc: optimize hreg_compute_pmu_hflags_value
2024-05-23 5:14 ` [PATCH v2 3/7] " Harsh Prateek Bora
@ 2024-07-04 7:38 ` Nicholas Piggin
0 siblings, 0 replies; 16+ messages in thread
From: Nicholas Piggin @ 2024-07-04 7:38 UTC (permalink / raw)
To: Harsh Prateek Bora, qemu-ppc, qemu-devel; +Cc: balaton, danielhb413
On Thu May 23, 2024 at 3:14 PM AEST, Harsh Prateek Bora wrote:
> The second if-condition can be true only if the first one above is true.
> Enclose the latter into the former to avoid un-necessary check if first
> condition fails.
>
> Signed-off-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
> Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>
Ditto for this it's possible compiler can transform it, but I
like the code.
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> target/ppc/helper_regs.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c
> index d09dcacd5e..261a8ba79f 100644
> --- a/target/ppc/helper_regs.c
> +++ b/target/ppc/helper_regs.c
> @@ -66,9 +66,9 @@ static uint32_t hreg_compute_pmu_hflags_value(CPUPPCState *env)
> #ifndef CONFIG_USER_ONLY
> if (env->pmc_ins_cnt) {
> hflags |= 1 << HFLAGS_INSN_CNT;
> - }
> - if (env->pmc_ins_cnt & 0x1e) {
> - hflags |= 1 << HFLAGS_PMC_OTHER;
> + if (env->pmc_ins_cnt & 0x1e) {
> + hflags |= 1 << HFLAGS_PMC_OTHER;
> + }
> }
> #endif
> #endif
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 4/7] target/ppc: optimize p9 exception handling routines
2024-05-23 5:14 [PATCH v2 0/7] target/ppc: misc ppc improvements/optimizations Harsh Prateek Bora
` (2 preceding siblings ...)
2024-05-23 5:14 ` [PATCH v2 3/7] " Harsh Prateek Bora
@ 2024-05-23 5:14 ` Harsh Prateek Bora
2024-07-04 7:40 ` Nicholas Piggin
2024-05-23 5:14 ` [PATCH v2 5/7] target/ppc: optimize p9 exception handling routines for lpcr Harsh Prateek Bora
` (2 subsequent siblings)
6 siblings, 1 reply; 16+ messages in thread
From: Harsh Prateek Bora @ 2024-05-23 5:14 UTC (permalink / raw)
To: qemu-ppc, qemu-devel; +Cc: npiggin, balaton, danielhb413
Currently, p9 exception handling has multiple if-condition checks where
it does an indirect access to pending_interrupts via env. Pass the
value during entry to avoid multiple indirect accesses.
Signed-off-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
---
target/ppc/excp_helper.c | 47 +++++++++++++++++++++-------------------
1 file changed, 25 insertions(+), 22 deletions(-)
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 0712098cf7..704eddac63 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -1842,10 +1842,12 @@ static int p8_next_unmasked_interrupt(CPUPPCState *env)
PPC_INTERRUPT_WDT | PPC_INTERRUPT_CDOORBELL | PPC_INTERRUPT_FIT | \
PPC_INTERRUPT_PIT | PPC_INTERRUPT_THERM)
-static int p9_interrupt_powersave(CPUPPCState *env)
+static int p9_interrupt_powersave(CPUPPCState *env,
+ uint32_t pending_interrupts)
{
+
/* External Exception */
- if ((env->pending_interrupts & PPC_INTERRUPT_EXT) &&
+ if ((pending_interrupts & PPC_INTERRUPT_EXT) &&
(env->spr[SPR_LPCR] & LPCR_EEE)) {
bool heic = !!(env->spr[SPR_LPCR] & LPCR_HEIC);
if (!heic || !FIELD_EX64_HV(env->msr) ||
@@ -1854,48 +1856,49 @@ static int p9_interrupt_powersave(CPUPPCState *env)
}
}
/* Decrementer Exception */
- if ((env->pending_interrupts & PPC_INTERRUPT_DECR) &&
+ if ((pending_interrupts & PPC_INTERRUPT_DECR) &&
(env->spr[SPR_LPCR] & LPCR_DEE)) {
return PPC_INTERRUPT_DECR;
}
/* Machine Check or Hypervisor Maintenance Exception */
if (env->spr[SPR_LPCR] & LPCR_OEE) {
- if (env->pending_interrupts & PPC_INTERRUPT_MCK) {
+ if (pending_interrupts & PPC_INTERRUPT_MCK) {
return PPC_INTERRUPT_MCK;
}
- if (env->pending_interrupts & PPC_INTERRUPT_HMI) {
+ if (pending_interrupts & PPC_INTERRUPT_HMI) {
return PPC_INTERRUPT_HMI;
}
}
/* Privileged Doorbell Exception */
- if ((env->pending_interrupts & PPC_INTERRUPT_DOORBELL) &&
+ if ((pending_interrupts & PPC_INTERRUPT_DOORBELL) &&
(env->spr[SPR_LPCR] & LPCR_PDEE)) {
return PPC_INTERRUPT_DOORBELL;
}
/* Hypervisor Doorbell Exception */
- if ((env->pending_interrupts & PPC_INTERRUPT_HDOORBELL) &&
+ if ((pending_interrupts & PPC_INTERRUPT_HDOORBELL) &&
(env->spr[SPR_LPCR] & LPCR_HDEE)) {
return PPC_INTERRUPT_HDOORBELL;
}
/* Hypervisor virtualization exception */
- if ((env->pending_interrupts & PPC_INTERRUPT_HVIRT) &&
+ if ((pending_interrupts & PPC_INTERRUPT_HVIRT) &&
(env->spr[SPR_LPCR] & LPCR_HVEE)) {
return PPC_INTERRUPT_HVIRT;
}
- if (env->pending_interrupts & PPC_INTERRUPT_RESET) {
+ if (pending_interrupts & PPC_INTERRUPT_RESET) {
return PPC_INTERRUPT_RESET;
}
return 0;
}
-static int p9_next_unmasked_interrupt(CPUPPCState *env)
+static int p9_next_unmasked_interrupt(CPUPPCState *env,
+ uint32_t pending_interrupts)
{
CPUState *cs = env_cpu(env);
/* 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);
+ assert((pending_interrupts & P9_UNUSED_INTERRUPTS) == 0);
if (cs->halted) {
if (env->spr[SPR_PSSCR] & PSSCR_EC) {
@@ -1903,7 +1906,7 @@ static int p9_next_unmasked_interrupt(CPUPPCState *env)
* When PSSCR[EC] is set, LPCR[PECE] controls which interrupts can
* wakeup the processor
*/
- return p9_interrupt_powersave(env);
+ return p9_interrupt_powersave(env, pending_interrupts);
} else {
/*
* When it's clear, any system-caused exception exits power-saving
@@ -1914,12 +1917,12 @@ static int p9_next_unmasked_interrupt(CPUPPCState *env)
}
/* Machine check exception */
- if (env->pending_interrupts & PPC_INTERRUPT_MCK) {
+ if (pending_interrupts & PPC_INTERRUPT_MCK) {
return PPC_INTERRUPT_MCK;
}
/* Hypervisor decrementer exception */
- if (env->pending_interrupts & PPC_INTERRUPT_HDECR) {
+ if (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 ((msr_ee || !FIELD_EX64_HV(env->msr)) && hdice) {
@@ -1929,7 +1932,7 @@ static int p9_next_unmasked_interrupt(CPUPPCState *env)
}
/* Hypervisor virtualization interrupt */
- if (env->pending_interrupts & PPC_INTERRUPT_HVIRT) {
+ if (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 ((msr_ee || !FIELD_EX64_HV(env->msr)) && hvice) {
@@ -1938,7 +1941,7 @@ static int p9_next_unmasked_interrupt(CPUPPCState *env)
}
/* External interrupt can ignore MSR:EE under some circumstances */
- if (env->pending_interrupts & PPC_INTERRUPT_EXT) {
+ if (pending_interrupts & PPC_INTERRUPT_EXT) {
bool lpes0 = !!(env->spr[SPR_LPCR] & LPCR_LPES0);
bool heic = !!(env->spr[SPR_LPCR] & LPCR_HEIC);
/* HEIC blocks delivery to the hypervisor */
@@ -1950,20 +1953,20 @@ static int p9_next_unmasked_interrupt(CPUPPCState *env)
}
if (msr_ee != 0) {
/* Decrementer exception */
- if (env->pending_interrupts & PPC_INTERRUPT_DECR) {
+ if (pending_interrupts & PPC_INTERRUPT_DECR) {
return PPC_INTERRUPT_DECR;
}
- if (env->pending_interrupts & PPC_INTERRUPT_DOORBELL) {
+ if (pending_interrupts & PPC_INTERRUPT_DOORBELL) {
return PPC_INTERRUPT_DOORBELL;
}
- if (env->pending_interrupts & PPC_INTERRUPT_HDOORBELL) {
+ if (pending_interrupts & PPC_INTERRUPT_HDOORBELL) {
return PPC_INTERRUPT_HDOORBELL;
}
- if (env->pending_interrupts & PPC_INTERRUPT_PERFM) {
+ if (pending_interrupts & PPC_INTERRUPT_PERFM) {
return PPC_INTERRUPT_PERFM;
}
/* EBB exception */
- if (env->pending_interrupts & PPC_INTERRUPT_EBB) {
+ if (pending_interrupts & PPC_INTERRUPT_EBB) {
/*
* EBB exception must be taken in problem state and
* with BESCR_GE set.
@@ -1989,7 +1992,7 @@ static int ppc_next_unmasked_interrupt(CPUPPCState *env)
return p8_next_unmasked_interrupt(env);
case POWERPC_EXCP_POWER9:
case POWERPC_EXCP_POWER10:
- return p9_next_unmasked_interrupt(env);
+ return p9_next_unmasked_interrupt(env, env->pending_interrupts);
default:
break;
}
--
2.39.3
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v2 4/7] target/ppc: optimize p9 exception handling routines
2024-05-23 5:14 ` [PATCH v2 4/7] target/ppc: optimize p9 exception handling routines Harsh Prateek Bora
@ 2024-07-04 7:40 ` Nicholas Piggin
0 siblings, 0 replies; 16+ messages in thread
From: Nicholas Piggin @ 2024-07-04 7:40 UTC (permalink / raw)
To: Harsh Prateek Bora, qemu-ppc, qemu-devel; +Cc: balaton, danielhb413
On Thu May 23, 2024 at 3:14 PM AEST, Harsh Prateek Bora wrote:
> Currently, p9 exception handling has multiple if-condition checks where
> it does an indirect access to pending_interrupts via env. Pass the
> value during entry to avoid multiple indirect accesses.
Does code change? I don't mind, would like all CPU funtions done
the same way if we're going to do this though.
Thanks,
Nick
>
> Signed-off-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
> ---
> target/ppc/excp_helper.c | 47 +++++++++++++++++++++-------------------
> 1 file changed, 25 insertions(+), 22 deletions(-)
>
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index 0712098cf7..704eddac63 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -1842,10 +1842,12 @@ static int p8_next_unmasked_interrupt(CPUPPCState *env)
> PPC_INTERRUPT_WDT | PPC_INTERRUPT_CDOORBELL | PPC_INTERRUPT_FIT | \
> PPC_INTERRUPT_PIT | PPC_INTERRUPT_THERM)
>
> -static int p9_interrupt_powersave(CPUPPCState *env)
> +static int p9_interrupt_powersave(CPUPPCState *env,
> + uint32_t pending_interrupts)
> {
> +
> /* External Exception */
> - if ((env->pending_interrupts & PPC_INTERRUPT_EXT) &&
> + if ((pending_interrupts & PPC_INTERRUPT_EXT) &&
> (env->spr[SPR_LPCR] & LPCR_EEE)) {
> bool heic = !!(env->spr[SPR_LPCR] & LPCR_HEIC);
> if (!heic || !FIELD_EX64_HV(env->msr) ||
> @@ -1854,48 +1856,49 @@ static int p9_interrupt_powersave(CPUPPCState *env)
> }
> }
> /* Decrementer Exception */
> - if ((env->pending_interrupts & PPC_INTERRUPT_DECR) &&
> + if ((pending_interrupts & PPC_INTERRUPT_DECR) &&
> (env->spr[SPR_LPCR] & LPCR_DEE)) {
> return PPC_INTERRUPT_DECR;
> }
> /* Machine Check or Hypervisor Maintenance Exception */
> if (env->spr[SPR_LPCR] & LPCR_OEE) {
> - if (env->pending_interrupts & PPC_INTERRUPT_MCK) {
> + if (pending_interrupts & PPC_INTERRUPT_MCK) {
> return PPC_INTERRUPT_MCK;
> }
> - if (env->pending_interrupts & PPC_INTERRUPT_HMI) {
> + if (pending_interrupts & PPC_INTERRUPT_HMI) {
> return PPC_INTERRUPT_HMI;
> }
> }
> /* Privileged Doorbell Exception */
> - if ((env->pending_interrupts & PPC_INTERRUPT_DOORBELL) &&
> + if ((pending_interrupts & PPC_INTERRUPT_DOORBELL) &&
> (env->spr[SPR_LPCR] & LPCR_PDEE)) {
> return PPC_INTERRUPT_DOORBELL;
> }
> /* Hypervisor Doorbell Exception */
> - if ((env->pending_interrupts & PPC_INTERRUPT_HDOORBELL) &&
> + if ((pending_interrupts & PPC_INTERRUPT_HDOORBELL) &&
> (env->spr[SPR_LPCR] & LPCR_HDEE)) {
> return PPC_INTERRUPT_HDOORBELL;
> }
> /* Hypervisor virtualization exception */
> - if ((env->pending_interrupts & PPC_INTERRUPT_HVIRT) &&
> + if ((pending_interrupts & PPC_INTERRUPT_HVIRT) &&
> (env->spr[SPR_LPCR] & LPCR_HVEE)) {
> return PPC_INTERRUPT_HVIRT;
> }
> - if (env->pending_interrupts & PPC_INTERRUPT_RESET) {
> + if (pending_interrupts & PPC_INTERRUPT_RESET) {
> return PPC_INTERRUPT_RESET;
> }
> return 0;
> }
>
> -static int p9_next_unmasked_interrupt(CPUPPCState *env)
> +static int p9_next_unmasked_interrupt(CPUPPCState *env,
> + uint32_t pending_interrupts)
> {
> CPUState *cs = env_cpu(env);
>
> /* 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);
> + assert((pending_interrupts & P9_UNUSED_INTERRUPTS) == 0);
>
> if (cs->halted) {
> if (env->spr[SPR_PSSCR] & PSSCR_EC) {
> @@ -1903,7 +1906,7 @@ static int p9_next_unmasked_interrupt(CPUPPCState *env)
> * When PSSCR[EC] is set, LPCR[PECE] controls which interrupts can
> * wakeup the processor
> */
> - return p9_interrupt_powersave(env);
> + return p9_interrupt_powersave(env, pending_interrupts);
> } else {
> /*
> * When it's clear, any system-caused exception exits power-saving
> @@ -1914,12 +1917,12 @@ static int p9_next_unmasked_interrupt(CPUPPCState *env)
> }
>
> /* Machine check exception */
> - if (env->pending_interrupts & PPC_INTERRUPT_MCK) {
> + if (pending_interrupts & PPC_INTERRUPT_MCK) {
> return PPC_INTERRUPT_MCK;
> }
>
> /* Hypervisor decrementer exception */
> - if (env->pending_interrupts & PPC_INTERRUPT_HDECR) {
> + if (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 ((msr_ee || !FIELD_EX64_HV(env->msr)) && hdice) {
> @@ -1929,7 +1932,7 @@ static int p9_next_unmasked_interrupt(CPUPPCState *env)
> }
>
> /* Hypervisor virtualization interrupt */
> - if (env->pending_interrupts & PPC_INTERRUPT_HVIRT) {
> + if (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 ((msr_ee || !FIELD_EX64_HV(env->msr)) && hvice) {
> @@ -1938,7 +1941,7 @@ static int p9_next_unmasked_interrupt(CPUPPCState *env)
> }
>
> /* External interrupt can ignore MSR:EE under some circumstances */
> - if (env->pending_interrupts & PPC_INTERRUPT_EXT) {
> + if (pending_interrupts & PPC_INTERRUPT_EXT) {
> bool lpes0 = !!(env->spr[SPR_LPCR] & LPCR_LPES0);
> bool heic = !!(env->spr[SPR_LPCR] & LPCR_HEIC);
> /* HEIC blocks delivery to the hypervisor */
> @@ -1950,20 +1953,20 @@ static int p9_next_unmasked_interrupt(CPUPPCState *env)
> }
> if (msr_ee != 0) {
> /* Decrementer exception */
> - if (env->pending_interrupts & PPC_INTERRUPT_DECR) {
> + if (pending_interrupts & PPC_INTERRUPT_DECR) {
> return PPC_INTERRUPT_DECR;
> }
> - if (env->pending_interrupts & PPC_INTERRUPT_DOORBELL) {
> + if (pending_interrupts & PPC_INTERRUPT_DOORBELL) {
> return PPC_INTERRUPT_DOORBELL;
> }
> - if (env->pending_interrupts & PPC_INTERRUPT_HDOORBELL) {
> + if (pending_interrupts & PPC_INTERRUPT_HDOORBELL) {
> return PPC_INTERRUPT_HDOORBELL;
> }
> - if (env->pending_interrupts & PPC_INTERRUPT_PERFM) {
> + if (pending_interrupts & PPC_INTERRUPT_PERFM) {
> return PPC_INTERRUPT_PERFM;
> }
> /* EBB exception */
> - if (env->pending_interrupts & PPC_INTERRUPT_EBB) {
> + if (pending_interrupts & PPC_INTERRUPT_EBB) {
> /*
> * EBB exception must be taken in problem state and
> * with BESCR_GE set.
> @@ -1989,7 +1992,7 @@ static int ppc_next_unmasked_interrupt(CPUPPCState *env)
> return p8_next_unmasked_interrupt(env);
> case POWERPC_EXCP_POWER9:
> case POWERPC_EXCP_POWER10:
> - return p9_next_unmasked_interrupt(env);
> + return p9_next_unmasked_interrupt(env, env->pending_interrupts);
> default:
> break;
> }
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 5/7] target/ppc: optimize p9 exception handling routines for lpcr
2024-05-23 5:14 [PATCH v2 0/7] target/ppc: misc ppc improvements/optimizations Harsh Prateek Bora
` (3 preceding siblings ...)
2024-05-23 5:14 ` [PATCH v2 4/7] target/ppc: optimize p9 exception handling routines Harsh Prateek Bora
@ 2024-05-23 5:14 ` Harsh Prateek Bora
2024-07-04 7:43 ` Nicholas Piggin
2024-05-23 5:14 ` [PATCH v2 6/7] target/ppc: reduce duplicate code between init_proc_POWER{9, 10} Harsh Prateek Bora
2024-05-23 5:14 ` [PATCH v2 7/7] target/ppc: redue code duplication across Power9/10 init code Harsh Prateek Bora
6 siblings, 1 reply; 16+ messages in thread
From: Harsh Prateek Bora @ 2024-05-23 5:14 UTC (permalink / raw)
To: qemu-ppc, qemu-devel; +Cc: npiggin, balaton, danielhb413
Like pending_interrupts, env->spr[SPR_LPCR] is being used at multiple
places across p9 exception handlers. Pass the value during entry and
avoid multiple indirect accesses.
Signed-off-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
---
target/ppc/excp_helper.c | 33 ++++++++++++++++++---------------
1 file changed, 18 insertions(+), 15 deletions(-)
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 704eddac63..d3db81e6ae 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -1843,13 +1843,14 @@ static int p8_next_unmasked_interrupt(CPUPPCState *env)
PPC_INTERRUPT_PIT | PPC_INTERRUPT_THERM)
static int p9_interrupt_powersave(CPUPPCState *env,
- uint32_t pending_interrupts)
+ uint32_t pending_interrupts,
+ target_ulong lpcr)
{
/* External Exception */
if ((pending_interrupts & PPC_INTERRUPT_EXT) &&
- (env->spr[SPR_LPCR] & LPCR_EEE)) {
- bool heic = !!(env->spr[SPR_LPCR] & LPCR_HEIC);
+ (lpcr & LPCR_EEE)) {
+ bool heic = !!(lpcr & LPCR_HEIC);
if (!heic || !FIELD_EX64_HV(env->msr) ||
FIELD_EX64(env->msr, MSR, PR)) {
return PPC_INTERRUPT_EXT;
@@ -1857,11 +1858,11 @@ static int p9_interrupt_powersave(CPUPPCState *env,
}
/* Decrementer Exception */
if ((pending_interrupts & PPC_INTERRUPT_DECR) &&
- (env->spr[SPR_LPCR] & LPCR_DEE)) {
+ (lpcr & LPCR_DEE)) {
return PPC_INTERRUPT_DECR;
}
/* Machine Check or Hypervisor Maintenance Exception */
- if (env->spr[SPR_LPCR] & LPCR_OEE) {
+ if (lpcr & LPCR_OEE) {
if (pending_interrupts & PPC_INTERRUPT_MCK) {
return PPC_INTERRUPT_MCK;
}
@@ -1871,17 +1872,17 @@ static int p9_interrupt_powersave(CPUPPCState *env,
}
/* Privileged Doorbell Exception */
if ((pending_interrupts & PPC_INTERRUPT_DOORBELL) &&
- (env->spr[SPR_LPCR] & LPCR_PDEE)) {
+ (lpcr & LPCR_PDEE)) {
return PPC_INTERRUPT_DOORBELL;
}
/* Hypervisor Doorbell Exception */
if ((pending_interrupts & PPC_INTERRUPT_HDOORBELL) &&
- (env->spr[SPR_LPCR] & LPCR_HDEE)) {
+ (lpcr & LPCR_HDEE)) {
return PPC_INTERRUPT_HDOORBELL;
}
/* Hypervisor virtualization exception */
if ((pending_interrupts & PPC_INTERRUPT_HVIRT) &&
- (env->spr[SPR_LPCR] & LPCR_HVEE)) {
+ (lpcr & LPCR_HVEE)) {
return PPC_INTERRUPT_HVIRT;
}
if (pending_interrupts & PPC_INTERRUPT_RESET) {
@@ -1891,7 +1892,8 @@ static int p9_interrupt_powersave(CPUPPCState *env,
}
static int p9_next_unmasked_interrupt(CPUPPCState *env,
- uint32_t pending_interrupts)
+ uint32_t pending_interrupts,
+ target_ulong lpcr)
{
CPUState *cs = env_cpu(env);
@@ -1906,7 +1908,7 @@ static int p9_next_unmasked_interrupt(CPUPPCState *env,
* When PSSCR[EC] is set, LPCR[PECE] controls which interrupts can
* wakeup the processor
*/
- return p9_interrupt_powersave(env, pending_interrupts);
+ return p9_interrupt_powersave(env, pending_interrupts, lpcr);
} else {
/*
* When it's clear, any system-caused exception exits power-saving
@@ -1924,7 +1926,7 @@ static int p9_next_unmasked_interrupt(CPUPPCState *env,
/* Hypervisor decrementer exception */
if (pending_interrupts & PPC_INTERRUPT_HDECR) {
/* LPCR will be clear when not supported so this will work */
- bool hdice = !!(env->spr[SPR_LPCR] & LPCR_HDICE);
+ bool hdice = !!(lpcr & LPCR_HDICE);
if ((msr_ee || !FIELD_EX64_HV(env->msr)) && hdice) {
/* HDEC clears on delivery */
return PPC_INTERRUPT_HDECR;
@@ -1934,7 +1936,7 @@ static int p9_next_unmasked_interrupt(CPUPPCState *env,
/* Hypervisor virtualization interrupt */
if (pending_interrupts & PPC_INTERRUPT_HVIRT) {
/* LPCR will be clear when not supported so this will work */
- bool hvice = !!(env->spr[SPR_LPCR] & LPCR_HVICE);
+ bool hvice = !!(lpcr & LPCR_HVICE);
if ((msr_ee || !FIELD_EX64_HV(env->msr)) && hvice) {
return PPC_INTERRUPT_HVIRT;
}
@@ -1942,8 +1944,8 @@ static int p9_next_unmasked_interrupt(CPUPPCState *env,
/* External interrupt can ignore MSR:EE under some circumstances */
if (pending_interrupts & PPC_INTERRUPT_EXT) {
- bool lpes0 = !!(env->spr[SPR_LPCR] & LPCR_LPES0);
- bool heic = !!(env->spr[SPR_LPCR] & LPCR_HEIC);
+ bool lpes0 = !!(lpcr & LPCR_LPES0);
+ bool heic = !!(lpcr & LPCR_HEIC);
/* HEIC blocks delivery to the hypervisor */
if ((msr_ee && !(heic && FIELD_EX64_HV(env->msr) &&
!FIELD_EX64(env->msr, MSR, PR))) ||
@@ -1992,7 +1994,8 @@ static int ppc_next_unmasked_interrupt(CPUPPCState *env)
return p8_next_unmasked_interrupt(env);
case POWERPC_EXCP_POWER9:
case POWERPC_EXCP_POWER10:
- return p9_next_unmasked_interrupt(env, env->pending_interrupts);
+ return p9_next_unmasked_interrupt(env, env->pending_interrupts,
+ env->spr[SPR_LPCR]);
default:
break;
}
--
2.39.3
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v2 5/7] target/ppc: optimize p9 exception handling routines for lpcr
2024-05-23 5:14 ` [PATCH v2 5/7] target/ppc: optimize p9 exception handling routines for lpcr Harsh Prateek Bora
@ 2024-07-04 7:43 ` Nicholas Piggin
2024-09-13 4:18 ` Harsh Prateek Bora
0 siblings, 1 reply; 16+ messages in thread
From: Nicholas Piggin @ 2024-07-04 7:43 UTC (permalink / raw)
To: Harsh Prateek Bora, qemu-ppc, qemu-devel; +Cc: balaton, danielhb413
On Thu May 23, 2024 at 3:14 PM AEST, Harsh Prateek Bora wrote:
> Like pending_interrupts, env->spr[SPR_LPCR] is being used at multiple
> places across p9 exception handlers. Pass the value during entry and
> avoid multiple indirect accesses.
Ditto for this (does it help code, other CPU functions should
be converted similarly).
Thanks,
Nick
>
> Signed-off-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
> ---
> target/ppc/excp_helper.c | 33 ++++++++++++++++++---------------
> 1 file changed, 18 insertions(+), 15 deletions(-)
>
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index 704eddac63..d3db81e6ae 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -1843,13 +1843,14 @@ static int p8_next_unmasked_interrupt(CPUPPCState *env)
> PPC_INTERRUPT_PIT | PPC_INTERRUPT_THERM)
>
> static int p9_interrupt_powersave(CPUPPCState *env,
> - uint32_t pending_interrupts)
> + uint32_t pending_interrupts,
> + target_ulong lpcr)
> {
>
> /* External Exception */
> if ((pending_interrupts & PPC_INTERRUPT_EXT) &&
> - (env->spr[SPR_LPCR] & LPCR_EEE)) {
> - bool heic = !!(env->spr[SPR_LPCR] & LPCR_HEIC);
> + (lpcr & LPCR_EEE)) {
> + bool heic = !!(lpcr & LPCR_HEIC);
> if (!heic || !FIELD_EX64_HV(env->msr) ||
> FIELD_EX64(env->msr, MSR, PR)) {
> return PPC_INTERRUPT_EXT;
> @@ -1857,11 +1858,11 @@ static int p9_interrupt_powersave(CPUPPCState *env,
> }
> /* Decrementer Exception */
> if ((pending_interrupts & PPC_INTERRUPT_DECR) &&
> - (env->spr[SPR_LPCR] & LPCR_DEE)) {
> + (lpcr & LPCR_DEE)) {
> return PPC_INTERRUPT_DECR;
> }
> /* Machine Check or Hypervisor Maintenance Exception */
> - if (env->spr[SPR_LPCR] & LPCR_OEE) {
> + if (lpcr & LPCR_OEE) {
> if (pending_interrupts & PPC_INTERRUPT_MCK) {
> return PPC_INTERRUPT_MCK;
> }
> @@ -1871,17 +1872,17 @@ static int p9_interrupt_powersave(CPUPPCState *env,
> }
> /* Privileged Doorbell Exception */
> if ((pending_interrupts & PPC_INTERRUPT_DOORBELL) &&
> - (env->spr[SPR_LPCR] & LPCR_PDEE)) {
> + (lpcr & LPCR_PDEE)) {
> return PPC_INTERRUPT_DOORBELL;
> }
> /* Hypervisor Doorbell Exception */
> if ((pending_interrupts & PPC_INTERRUPT_HDOORBELL) &&
> - (env->spr[SPR_LPCR] & LPCR_HDEE)) {
> + (lpcr & LPCR_HDEE)) {
> return PPC_INTERRUPT_HDOORBELL;
> }
> /* Hypervisor virtualization exception */
> if ((pending_interrupts & PPC_INTERRUPT_HVIRT) &&
> - (env->spr[SPR_LPCR] & LPCR_HVEE)) {
> + (lpcr & LPCR_HVEE)) {
> return PPC_INTERRUPT_HVIRT;
> }
> if (pending_interrupts & PPC_INTERRUPT_RESET) {
> @@ -1891,7 +1892,8 @@ static int p9_interrupt_powersave(CPUPPCState *env,
> }
>
> static int p9_next_unmasked_interrupt(CPUPPCState *env,
> - uint32_t pending_interrupts)
> + uint32_t pending_interrupts,
> + target_ulong lpcr)
> {
> CPUState *cs = env_cpu(env);
>
> @@ -1906,7 +1908,7 @@ static int p9_next_unmasked_interrupt(CPUPPCState *env,
> * When PSSCR[EC] is set, LPCR[PECE] controls which interrupts can
> * wakeup the processor
> */
> - return p9_interrupt_powersave(env, pending_interrupts);
> + return p9_interrupt_powersave(env, pending_interrupts, lpcr);
> } else {
> /*
> * When it's clear, any system-caused exception exits power-saving
> @@ -1924,7 +1926,7 @@ static int p9_next_unmasked_interrupt(CPUPPCState *env,
> /* Hypervisor decrementer exception */
> if (pending_interrupts & PPC_INTERRUPT_HDECR) {
> /* LPCR will be clear when not supported so this will work */
> - bool hdice = !!(env->spr[SPR_LPCR] & LPCR_HDICE);
> + bool hdice = !!(lpcr & LPCR_HDICE);
> if ((msr_ee || !FIELD_EX64_HV(env->msr)) && hdice) {
> /* HDEC clears on delivery */
> return PPC_INTERRUPT_HDECR;
> @@ -1934,7 +1936,7 @@ static int p9_next_unmasked_interrupt(CPUPPCState *env,
> /* Hypervisor virtualization interrupt */
> if (pending_interrupts & PPC_INTERRUPT_HVIRT) {
> /* LPCR will be clear when not supported so this will work */
> - bool hvice = !!(env->spr[SPR_LPCR] & LPCR_HVICE);
> + bool hvice = !!(lpcr & LPCR_HVICE);
> if ((msr_ee || !FIELD_EX64_HV(env->msr)) && hvice) {
> return PPC_INTERRUPT_HVIRT;
> }
> @@ -1942,8 +1944,8 @@ static int p9_next_unmasked_interrupt(CPUPPCState *env,
>
> /* External interrupt can ignore MSR:EE under some circumstances */
> if (pending_interrupts & PPC_INTERRUPT_EXT) {
> - bool lpes0 = !!(env->spr[SPR_LPCR] & LPCR_LPES0);
> - bool heic = !!(env->spr[SPR_LPCR] & LPCR_HEIC);
> + bool lpes0 = !!(lpcr & LPCR_LPES0);
> + bool heic = !!(lpcr & LPCR_HEIC);
> /* HEIC blocks delivery to the hypervisor */
> if ((msr_ee && !(heic && FIELD_EX64_HV(env->msr) &&
> !FIELD_EX64(env->msr, MSR, PR))) ||
> @@ -1992,7 +1994,8 @@ static int ppc_next_unmasked_interrupt(CPUPPCState *env)
> return p8_next_unmasked_interrupt(env);
> case POWERPC_EXCP_POWER9:
> case POWERPC_EXCP_POWER10:
> - return p9_next_unmasked_interrupt(env, env->pending_interrupts);
> + return p9_next_unmasked_interrupt(env, env->pending_interrupts,
> + env->spr[SPR_LPCR]);
> default:
> break;
> }
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v2 5/7] target/ppc: optimize p9 exception handling routines for lpcr
2024-07-04 7:43 ` Nicholas Piggin
@ 2024-09-13 4:18 ` Harsh Prateek Bora
0 siblings, 0 replies; 16+ messages in thread
From: Harsh Prateek Bora @ 2024-09-13 4:18 UTC (permalink / raw)
To: Nicholas Piggin, qemu-ppc, qemu-devel; +Cc: balaton, danielhb413
Hi Nick,
On 7/4/24 13:13, Nicholas Piggin wrote:
> On Thu May 23, 2024 at 3:14 PM AEST, Harsh Prateek Bora wrote:
>> Like pending_interrupts, env->spr[SPR_LPCR] is being used at multiple
>> places across p9 exception handlers. Pass the value during entry and
>> avoid multiple indirect accesses.
>
> Ditto for this (does it help code, other CPU functions should
> be converted similarly).
Thanks for your review comments and apologies for a late response.
I have posted v3 with suggested updates and some additional patches.
Patch 7/7 of this series had been picked by Aditya and updated in his
patchset for P11 support so I have excluded that in v3 and just rebased
on top of his v6 series.
v3:
https://lore.kernel.org/qemu-devel/20240913041337.912876-1-harshpb@linux.ibm.com/T/#t
regards,
Harsh
>
> Thanks,
> Nick
>
>>
>> Signed-off-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
>> ---
>> target/ppc/excp_helper.c | 33 ++++++++++++++++++---------------
>> 1 file changed, 18 insertions(+), 15 deletions(-)
>>
>> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
>> index 704eddac63..d3db81e6ae 100644
>> --- a/target/ppc/excp_helper.c
>> +++ b/target/ppc/excp_helper.c
>> @@ -1843,13 +1843,14 @@ static int p8_next_unmasked_interrupt(CPUPPCState *env)
>> PPC_INTERRUPT_PIT | PPC_INTERRUPT_THERM)
>>
>> static int p9_interrupt_powersave(CPUPPCState *env,
>> - uint32_t pending_interrupts)
>> + uint32_t pending_interrupts,
>> + target_ulong lpcr)
>> {
>>
>> /* External Exception */
>> if ((pending_interrupts & PPC_INTERRUPT_EXT) &&
>> - (env->spr[SPR_LPCR] & LPCR_EEE)) {
>> - bool heic = !!(env->spr[SPR_LPCR] & LPCR_HEIC);
>> + (lpcr & LPCR_EEE)) {
>> + bool heic = !!(lpcr & LPCR_HEIC);
>> if (!heic || !FIELD_EX64_HV(env->msr) ||
>> FIELD_EX64(env->msr, MSR, PR)) {
>> return PPC_INTERRUPT_EXT;
>> @@ -1857,11 +1858,11 @@ static int p9_interrupt_powersave(CPUPPCState *env,
>> }
>> /* Decrementer Exception */
>> if ((pending_interrupts & PPC_INTERRUPT_DECR) &&
>> - (env->spr[SPR_LPCR] & LPCR_DEE)) {
>> + (lpcr & LPCR_DEE)) {
>> return PPC_INTERRUPT_DECR;
>> }
>> /* Machine Check or Hypervisor Maintenance Exception */
>> - if (env->spr[SPR_LPCR] & LPCR_OEE) {
>> + if (lpcr & LPCR_OEE) {
>> if (pending_interrupts & PPC_INTERRUPT_MCK) {
>> return PPC_INTERRUPT_MCK;
>> }
>> @@ -1871,17 +1872,17 @@ static int p9_interrupt_powersave(CPUPPCState *env,
>> }
>> /* Privileged Doorbell Exception */
>> if ((pending_interrupts & PPC_INTERRUPT_DOORBELL) &&
>> - (env->spr[SPR_LPCR] & LPCR_PDEE)) {
>> + (lpcr & LPCR_PDEE)) {
>> return PPC_INTERRUPT_DOORBELL;
>> }
>> /* Hypervisor Doorbell Exception */
>> if ((pending_interrupts & PPC_INTERRUPT_HDOORBELL) &&
>> - (env->spr[SPR_LPCR] & LPCR_HDEE)) {
>> + (lpcr & LPCR_HDEE)) {
>> return PPC_INTERRUPT_HDOORBELL;
>> }
>> /* Hypervisor virtualization exception */
>> if ((pending_interrupts & PPC_INTERRUPT_HVIRT) &&
>> - (env->spr[SPR_LPCR] & LPCR_HVEE)) {
>> + (lpcr & LPCR_HVEE)) {
>> return PPC_INTERRUPT_HVIRT;
>> }
>> if (pending_interrupts & PPC_INTERRUPT_RESET) {
>> @@ -1891,7 +1892,8 @@ static int p9_interrupt_powersave(CPUPPCState *env,
>> }
>>
>> static int p9_next_unmasked_interrupt(CPUPPCState *env,
>> - uint32_t pending_interrupts)
>> + uint32_t pending_interrupts,
>> + target_ulong lpcr)
>> {
>> CPUState *cs = env_cpu(env);
>>
>> @@ -1906,7 +1908,7 @@ static int p9_next_unmasked_interrupt(CPUPPCState *env,
>> * When PSSCR[EC] is set, LPCR[PECE] controls which interrupts can
>> * wakeup the processor
>> */
>> - return p9_interrupt_powersave(env, pending_interrupts);
>> + return p9_interrupt_powersave(env, pending_interrupts, lpcr);
>> } else {
>> /*
>> * When it's clear, any system-caused exception exits power-saving
>> @@ -1924,7 +1926,7 @@ static int p9_next_unmasked_interrupt(CPUPPCState *env,
>> /* Hypervisor decrementer exception */
>> if (pending_interrupts & PPC_INTERRUPT_HDECR) {
>> /* LPCR will be clear when not supported so this will work */
>> - bool hdice = !!(env->spr[SPR_LPCR] & LPCR_HDICE);
>> + bool hdice = !!(lpcr & LPCR_HDICE);
>> if ((msr_ee || !FIELD_EX64_HV(env->msr)) && hdice) {
>> /* HDEC clears on delivery */
>> return PPC_INTERRUPT_HDECR;
>> @@ -1934,7 +1936,7 @@ static int p9_next_unmasked_interrupt(CPUPPCState *env,
>> /* Hypervisor virtualization interrupt */
>> if (pending_interrupts & PPC_INTERRUPT_HVIRT) {
>> /* LPCR will be clear when not supported so this will work */
>> - bool hvice = !!(env->spr[SPR_LPCR] & LPCR_HVICE);
>> + bool hvice = !!(lpcr & LPCR_HVICE);
>> if ((msr_ee || !FIELD_EX64_HV(env->msr)) && hvice) {
>> return PPC_INTERRUPT_HVIRT;
>> }
>> @@ -1942,8 +1944,8 @@ static int p9_next_unmasked_interrupt(CPUPPCState *env,
>>
>> /* External interrupt can ignore MSR:EE under some circumstances */
>> if (pending_interrupts & PPC_INTERRUPT_EXT) {
>> - bool lpes0 = !!(env->spr[SPR_LPCR] & LPCR_LPES0);
>> - bool heic = !!(env->spr[SPR_LPCR] & LPCR_HEIC);
>> + bool lpes0 = !!(lpcr & LPCR_LPES0);
>> + bool heic = !!(lpcr & LPCR_HEIC);
>> /* HEIC blocks delivery to the hypervisor */
>> if ((msr_ee && !(heic && FIELD_EX64_HV(env->msr) &&
>> !FIELD_EX64(env->msr, MSR, PR))) ||
>> @@ -1992,7 +1994,8 @@ static int ppc_next_unmasked_interrupt(CPUPPCState *env)
>> return p8_next_unmasked_interrupt(env);
>> case POWERPC_EXCP_POWER9:
>> case POWERPC_EXCP_POWER10:
>> - return p9_next_unmasked_interrupt(env, env->pending_interrupts);
>> + return p9_next_unmasked_interrupt(env, env->pending_interrupts,
>> + env->spr[SPR_LPCR]);
>> default:
>> break;
>> }
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 6/7] target/ppc: reduce duplicate code between init_proc_POWER{9, 10}
2024-05-23 5:14 [PATCH v2 0/7] target/ppc: misc ppc improvements/optimizations Harsh Prateek Bora
` (4 preceding siblings ...)
2024-05-23 5:14 ` [PATCH v2 5/7] target/ppc: optimize p9 exception handling routines for lpcr Harsh Prateek Bora
@ 2024-05-23 5:14 ` Harsh Prateek Bora
2024-05-23 5:14 ` [PATCH v2 7/7] target/ppc: redue code duplication across Power9/10 init code Harsh Prateek Bora
6 siblings, 0 replies; 16+ messages in thread
From: Harsh Prateek Bora @ 2024-05-23 5:14 UTC (permalink / raw)
To: qemu-ppc, qemu-devel; +Cc: npiggin, balaton, danielhb413
Historically, the registration of sprs have been inherited alongwith
every new Power arch support being added leading to a lot of code
duplication. It's time to do necessary cleanups now to avoid further
duplication with newer arch support being added.
Signed-off-by: Harsh Prateek Bora <harshb@linux.ibm.com>
---
target/ppc/cpu_init.c | 43 +++++++++----------------------------------
1 file changed, 9 insertions(+), 34 deletions(-)
diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index 6d82f24c87..5fb9a0583e 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -6307,7 +6307,7 @@ static struct ppc_radix_page_info POWER9_radix_page_info = {
};
#endif /* CONFIG_USER_ONLY */
-static void init_proc_POWER9(CPUPPCState *env)
+static void register_power9_common_sprs(CPUPPCState *env)
{
/* Common Registers */
init_proc_book3s_common(env);
@@ -6326,7 +6326,6 @@ static void init_proc_POWER9(CPUPPCState *env)
register_power5p_ear_sprs(env);
register_power5p_tb_sprs(env);
register_power6_common_sprs(env);
- register_HEIR32_spr(env);
register_power6_dbg_sprs(env);
register_power8_tce_address_control_sprs(env);
register_power8_ids_sprs(env);
@@ -6342,6 +6341,12 @@ static void init_proc_POWER9(CPUPPCState *env)
register_power9_book4_sprs(env);
register_power8_rpr_sprs(env);
register_power9_mmu_sprs(env);
+}
+
+static void init_proc_POWER9(CPUPPCState *env)
+{
+ register_power9_common_sprs(env);
+ register_HEIR32_spr(env);
/* POWER9 Specific registers */
spr_register_kvm(env, SPR_TIDR, "TIDR", NULL, NULL,
@@ -6499,39 +6504,9 @@ static struct ppc_radix_page_info POWER10_radix_page_info = {
static void init_proc_POWER10(CPUPPCState *env)
{
- /* Common Registers */
- init_proc_book3s_common(env);
- register_book3s_207_dbg_sprs(env);
-
- /* Common TCG PMU */
- init_tcg_pmu_power8(env);
-
- /* POWER8 Specific Registers */
- register_book3s_ids_sprs(env);
- register_amr_sprs(env);
- register_iamr_sprs(env);
- register_book3s_purr_sprs(env);
- register_power5p_common_sprs(env);
- register_power5p_lpar_sprs(env);
- register_power5p_ear_sprs(env);
- register_power5p_tb_sprs(env);
- register_power6_common_sprs(env);
+ register_power9_common_sprs(env);
register_HEIR64_spr(env);
- register_power6_dbg_sprs(env);
- register_power8_tce_address_control_sprs(env);
- register_power8_ids_sprs(env);
- register_power8_ebb_sprs(env);
- register_power8_fscr_sprs(env);
- register_power8_pmu_sup_sprs(env);
- register_power8_pmu_user_sprs(env);
- register_power8_tm_sprs(env);
- register_power8_pspb_sprs(env);
- register_power8_dpdes_sprs(env);
- register_vtb_sprs(env);
- register_power8_ic_sprs(env);
- register_power9_book4_sprs(env);
- register_power8_rpr_sprs(env);
- register_power9_mmu_sprs(env);
+
register_power10_hash_sprs(env);
register_power10_dexcr_sprs(env);
register_power10_pmu_sup_sprs(env);
--
2.39.3
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v2 7/7] target/ppc: redue code duplication across Power9/10 init code
2024-05-23 5:14 [PATCH v2 0/7] target/ppc: misc ppc improvements/optimizations Harsh Prateek Bora
` (5 preceding siblings ...)
2024-05-23 5:14 ` [PATCH v2 6/7] target/ppc: reduce duplicate code between init_proc_POWER{9, 10} Harsh Prateek Bora
@ 2024-05-23 5:14 ` Harsh Prateek Bora
2024-07-04 7:49 ` Nicholas Piggin
6 siblings, 1 reply; 16+ messages in thread
From: Harsh Prateek Bora @ 2024-05-23 5:14 UTC (permalink / raw)
To: qemu-ppc, qemu-devel; +Cc: npiggin, balaton, danielhb413
Power9/10 initialization code consists of a lot of logical OR of
various flag bits as supported by respective Power platform during its
initialization, most of which is duplicated and only selected bits are
added or removed as needed with each new platform support being added.
Remove the duplicate code and share using common macros.
Signed-off-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
---
target/ppc/cpu_init.h | 77 ++++++++++++++++++++++++++
target/ppc/cpu_init.c | 123 ++++++------------------------------------
2 files changed, 92 insertions(+), 108 deletions(-)
create mode 100644 target/ppc/cpu_init.h
diff --git a/target/ppc/cpu_init.h b/target/ppc/cpu_init.h
new file mode 100644
index 0000000000..53909987b0
--- /dev/null
+++ b/target/ppc/cpu_init.h
@@ -0,0 +1,77 @@
+#ifndef TARGET_PPC_CPU_INIT_H
+#define TARGET_PPC_CPU_INIT_H
+
+#define POWERPC_FAMILY_POWER9_INSNS_FLAGS \
+ PPC_INSNS_BASE | PPC_ISEL | PPC_STRING | PPC_MFTB | \
+ PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES | \
+ PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE | PPC_FLOAT_FRSQRTES | \
+ PPC_FLOAT_STFIWX | PPC_FLOAT_EXT |PPC_CACHE | PPC_CACHE_ICBI | \
+ PPC_CACHE_DCBZ | PPC_MEM_SYNC | PPC_MEM_EIEIO | PPC_MEM_TLBIE | \
+ PPC_MEM_TLBSYNC | PPC_64B | PPC_64H | PPC_64BX | PPC_ALTIVEC | \
+ PPC_SEGMENT_64B | PPC_SLBI | PPC_POPCNTB | PPC_POPCNTWD | \
+ PPC_CILDST
+
+#define POWERPC_FAMILY_POWER9_INSNS_FLAGS2_COMMON \
+ PPC2_VSX | PPC2_VSX207 | PPC2_DFP | PPC2_DBRX | \
+ PPC2_PERM_ISA206 | PPC2_DIVE_ISA206 | PPC2_ATOMIC_ISA206 | \
+ PPC2_FP_CVT_ISA206 | PPC2_FP_TST_ISA206 | PPC2_BCTAR_ISA207 | \
+ PPC2_LSQ_ISA207 | PPC2_ALTIVEC_207 | PPC2_ISA205 | \
+ PPC2_ISA207S | PPC2_FP_CVT_S64 | PPC2_ISA300 | PPC2_PRCNTL | \
+ PPC2_MEM_LWSYNC | PPC2_BCDA_ISA206
+
+#define POWERPC_FAMILY_POWER9_INSNS_FLAGS2 \
+ POWERPC_FAMILY_POWER9_INSNS_FLAGS2_COMMON | PPC2_TM
+#define POWERPC_FAMILY_POWER10_INSNS_FLAGS2 \
+ POWERPC_FAMILY_POWER9_INSNS_FLAGS2_COMMON | PPC2_ISA310
+
+#define POWERPC_POWER9_COMMON_PCC_MSR_MASK \
+ (1ull << MSR_SF) | \
+ (1ull << MSR_HV) | \
+ (1ull << MSR_VR) | \
+ (1ull << MSR_VSX) | \
+ (1ull << MSR_EE) | \
+ (1ull << MSR_PR) | \
+ (1ull << MSR_FP) | \
+ (1ull << MSR_ME) | \
+ (1ull << MSR_FE0) | \
+ (1ull << MSR_SE) | \
+ (1ull << MSR_DE) | \
+ (1ull << MSR_FE1) | \
+ (1ull << MSR_IR) | \
+ (1ull << MSR_DR) | \
+ (1ull << MSR_PMM) | \
+ (1ull << MSR_RI) | \
+ (1ull << MSR_LE)
+
+#define POWERPC_POWER9_PCC_MSR_MASK \
+ POWERPC_POWER9_COMMON_PCC_MSR_MASK | (1ull << MSR_TM)
+#define POWERPC_POWER10_PCC_MSR_MASK \
+ POWERPC_POWER9_COMMON_PCC_MSR_MASK
+#define POWERPC_POWER9_PCC_PCR_MASK \
+ PCR_COMPAT_2_05 | PCR_COMPAT_2_06 | PCR_COMPAT_2_07
+#define POWERPC_POWER10_PCC_PCR_MASK \
+ POWERPC_POWER9_PCC_PCR_MASK | PCR_COMPAT_3_00
+#define POWERPC_POWER9_PCC_PCR_SUPPORTED \
+ PCR_COMPAT_3_00 | PCR_COMPAT_2_07 | PCR_COMPAT_2_06 | PCR_COMPAT_2_05
+#define POWERPC_POWER10_PCC_PCR_SUPPORTED \
+ POWERPC_POWER9_PCC_PCR_SUPPORTED | PCR_COMPAT_3_10
+#define POWERPC_POWER9_PCC_LPCR_MASK \
+ LPCR_VPM1 | LPCR_ISL | LPCR_KBV | LPCR_DPFD | \
+ (LPCR_PECE_U_MASK & LPCR_HVEE) | LPCR_ILE | LPCR_AIL | \
+ LPCR_UPRT | LPCR_EVIRT | LPCR_ONL | LPCR_HR | LPCR_LD | \
+ (LPCR_PECE_L_MASK & (LPCR_PDEE|LPCR_HDEE|LPCR_EEE|LPCR_DEE|LPCR_OEE)) | \
+ LPCR_MER | LPCR_GTSE | LPCR_TC | LPCR_HEIC | LPCR_LPES0 | LPCR_HVICE | \
+ LPCR_HDICE
+/* DD2 adds an extra HAIL bit */
+#define POWERPC_POWER10_PCC_LPCR_MASK \
+ POWERPC_POWER9_PCC_LPCR_MASK | LPCR_HAIL
+#define POWERPC_POWER9_PCC_FLAGS_COMMON \
+ POWERPC_FLAG_VRE | POWERPC_FLAG_SE | POWERPC_FLAG_BE | \
+ POWERPC_FLAG_PMM | POWERPC_FLAG_BUS_CLK | POWERPC_FLAG_CFAR | \
+ POWERPC_FLAG_VSX | POWERPC_FLAG_SCV
+
+#define POWERPC_POWER9_PCC_FLAGS \
+ POWERPC_POWER9_PCC_FLAGS_COMMON | POWERPC_FLAG_TM
+#define POWERPC_POWER10_PCC_FLAGS POWERPC_POWER9_PCC_FLAGS_COMMON
+
+#endif /* TARGET_PPC_CPU_INIT_H */
diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index 5fb9a0583e..e4f6ad2399 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -51,6 +51,7 @@
#include "kvm_ppc.h"
#endif
+#include "cpu_init.h"
/* #define PPC_DEBUG_SPR */
/* #define USE_APPLE_GDB */
@@ -6412,57 +6413,14 @@ POWERPC_FAMILY(POWER9)(ObjectClass *oc, void *data)
dc->fw_name = "PowerPC,POWER9";
dc->desc = "POWER9";
pcc->pvr_match = ppc_pvr_match_power9;
- pcc->pcr_mask = PCR_COMPAT_2_05 | PCR_COMPAT_2_06 | PCR_COMPAT_2_07;
- pcc->pcr_supported = PCR_COMPAT_3_00 | PCR_COMPAT_2_07 | PCR_COMPAT_2_06 |
- PCR_COMPAT_2_05;
+ pcc->pcr_mask = POWERPC_POWER9_PCC_PCR_MASK;
+ pcc->pcr_supported = POWERPC_POWER9_PCC_PCR_SUPPORTED;
pcc->init_proc = init_proc_POWER9;
pcc->check_pow = check_pow_nocheck;
- pcc->insns_flags = PPC_INSNS_BASE | PPC_ISEL | PPC_STRING | PPC_MFTB |
- PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES |
- PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE |
- PPC_FLOAT_FRSQRTES |
- PPC_FLOAT_STFIWX |
- PPC_FLOAT_EXT |
- PPC_CACHE | PPC_CACHE_ICBI | PPC_CACHE_DCBZ |
- PPC_MEM_SYNC | PPC_MEM_EIEIO |
- PPC_MEM_TLBIE | PPC_MEM_TLBSYNC |
- PPC_64B | PPC_64H | PPC_64BX | PPC_ALTIVEC |
- PPC_SEGMENT_64B | PPC_SLBI |
- PPC_POPCNTB | PPC_POPCNTWD |
- PPC_CILDST;
- pcc->insns_flags2 = PPC2_VSX | PPC2_VSX207 | PPC2_DFP | PPC2_DBRX |
- PPC2_PERM_ISA206 | PPC2_DIVE_ISA206 |
- PPC2_ATOMIC_ISA206 | PPC2_FP_CVT_ISA206 |
- PPC2_FP_TST_ISA206 | PPC2_BCTAR_ISA207 |
- PPC2_LSQ_ISA207 | PPC2_ALTIVEC_207 |
- PPC2_ISA205 | PPC2_ISA207S | PPC2_FP_CVT_S64 |
- PPC2_TM | PPC2_ISA300 | PPC2_PRCNTL | PPC2_MEM_LWSYNC |
- PPC2_BCDA_ISA206;
- pcc->msr_mask = (1ull << MSR_SF) |
- (1ull << MSR_HV) |
- (1ull << MSR_TM) |
- (1ull << MSR_VR) |
- (1ull << MSR_VSX) |
- (1ull << MSR_EE) |
- (1ull << MSR_PR) |
- (1ull << MSR_FP) |
- (1ull << MSR_ME) |
- (1ull << MSR_FE0) |
- (1ull << MSR_SE) |
- (1ull << MSR_DE) |
- (1ull << MSR_FE1) |
- (1ull << MSR_IR) |
- (1ull << MSR_DR) |
- (1ull << MSR_PMM) |
- (1ull << MSR_RI) |
- (1ull << MSR_LE);
- pcc->lpcr_mask = LPCR_VPM1 | LPCR_ISL | LPCR_KBV | LPCR_DPFD |
- (LPCR_PECE_U_MASK & LPCR_HVEE) | LPCR_ILE | LPCR_AIL |
- LPCR_UPRT | LPCR_EVIRT | LPCR_ONL | LPCR_HR | LPCR_LD |
- (LPCR_PECE_L_MASK & (LPCR_PDEE | LPCR_HDEE | LPCR_EEE |
- LPCR_DEE | LPCR_OEE))
- | LPCR_MER | LPCR_GTSE | LPCR_TC |
- LPCR_HEIC | LPCR_LPES0 | LPCR_HVICE | LPCR_HDICE;
+ pcc->insns_flags = POWERPC_FAMILY_POWER9_INSNS_FLAGS;
+ pcc->insns_flags2 = POWERPC_FAMILY_POWER9_INSNS_FLAGS2;
+ pcc->msr_mask = POWERPC_POWER9_PCC_MSR_MASK;
+ pcc->lpcr_mask = POWERPC_POWER9_PCC_LPCR_MASK;
pcc->lpcr_pm = LPCR_PDEE | LPCR_HDEE | LPCR_EEE | LPCR_DEE | LPCR_OEE;
pcc->mmu_model = POWERPC_MMU_3_00;
#if !defined(CONFIG_USER_ONLY)
@@ -6475,10 +6433,7 @@ POWERPC_FAMILY(POWER9)(ObjectClass *oc, void *data)
pcc->excp_model = POWERPC_EXCP_POWER9;
pcc->bus_model = PPC_FLAGS_INPUT_POWER9;
pcc->bfd_mach = bfd_mach_ppc64;
- pcc->flags = POWERPC_FLAG_VRE | POWERPC_FLAG_SE |
- POWERPC_FLAG_BE | POWERPC_FLAG_PMM |
- POWERPC_FLAG_BUS_CLK | POWERPC_FLAG_CFAR |
- POWERPC_FLAG_VSX | POWERPC_FLAG_TM | POWERPC_FLAG_SCV;
+ pcc->flags = POWERPC_POWER9_PCC_FLAGS;
pcc->l1_dcache_size = 0x8000;
pcc->l1_icache_size = 0x8000;
}
@@ -6557,59 +6512,14 @@ POWERPC_FAMILY(POWER10)(ObjectClass *oc, void *data)
dc->fw_name = "PowerPC,POWER10";
dc->desc = "POWER10";
pcc->pvr_match = ppc_pvr_match_power10;
- pcc->pcr_mask = PCR_COMPAT_2_05 | PCR_COMPAT_2_06 | PCR_COMPAT_2_07 |
- PCR_COMPAT_3_00;
- pcc->pcr_supported = PCR_COMPAT_3_10 | PCR_COMPAT_3_00 | PCR_COMPAT_2_07 |
- PCR_COMPAT_2_06 | PCR_COMPAT_2_05;
+ pcc->pcr_mask = POWERPC_POWER10_PCC_PCR_MASK;
+ pcc->pcr_supported = POWERPC_POWER10_PCC_PCR_SUPPORTED;
pcc->init_proc = init_proc_POWER10;
pcc->check_pow = check_pow_nocheck;
- pcc->insns_flags = PPC_INSNS_BASE | PPC_ISEL | PPC_STRING | PPC_MFTB |
- PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES |
- PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE |
- PPC_FLOAT_FRSQRTES |
- PPC_FLOAT_STFIWX |
- PPC_FLOAT_EXT |
- PPC_CACHE | PPC_CACHE_ICBI | PPC_CACHE_DCBZ |
- PPC_MEM_SYNC | PPC_MEM_EIEIO |
- PPC_MEM_TLBIE | PPC_MEM_TLBSYNC |
- PPC_64B | PPC_64H | PPC_64BX | PPC_ALTIVEC |
- PPC_SEGMENT_64B | PPC_SLBI |
- PPC_POPCNTB | PPC_POPCNTWD |
- PPC_CILDST;
- pcc->insns_flags2 = PPC2_VSX | PPC2_VSX207 | PPC2_DFP | PPC2_DBRX |
- PPC2_PERM_ISA206 | PPC2_DIVE_ISA206 |
- PPC2_ATOMIC_ISA206 | PPC2_FP_CVT_ISA206 |
- PPC2_FP_TST_ISA206 | PPC2_BCTAR_ISA207 |
- PPC2_LSQ_ISA207 | PPC2_ALTIVEC_207 |
- PPC2_ISA205 | PPC2_ISA207S | PPC2_FP_CVT_S64 |
- PPC2_ISA300 | PPC2_PRCNTL | PPC2_ISA310 |
- PPC2_MEM_LWSYNC | PPC2_BCDA_ISA206;
- pcc->msr_mask = (1ull << MSR_SF) |
- (1ull << MSR_HV) |
- (1ull << MSR_VR) |
- (1ull << MSR_VSX) |
- (1ull << MSR_EE) |
- (1ull << MSR_PR) |
- (1ull << MSR_FP) |
- (1ull << MSR_ME) |
- (1ull << MSR_FE0) |
- (1ull << MSR_SE) |
- (1ull << MSR_DE) |
- (1ull << MSR_FE1) |
- (1ull << MSR_IR) |
- (1ull << MSR_DR) |
- (1ull << MSR_PMM) |
- (1ull << MSR_RI) |
- (1ull << MSR_LE);
- pcc->lpcr_mask = LPCR_VPM1 | LPCR_ISL | LPCR_KBV | LPCR_DPFD |
- (LPCR_PECE_U_MASK & LPCR_HVEE) | LPCR_ILE | LPCR_AIL |
- LPCR_UPRT | LPCR_EVIRT | LPCR_ONL | LPCR_HR | LPCR_LD |
- (LPCR_PECE_L_MASK & (LPCR_PDEE | LPCR_HDEE | LPCR_EEE |
- LPCR_DEE | LPCR_OEE))
- | LPCR_MER | LPCR_GTSE | LPCR_TC |
- LPCR_HEIC | LPCR_LPES0 | LPCR_HVICE | LPCR_HDICE;
- /* DD2 adds an extra HAIL bit */
- pcc->lpcr_mask |= LPCR_HAIL;
+ pcc->insns_flags = POWERPC_FAMILY_POWER9_INSNS_FLAGS; /* same as P9 */
+ pcc->insns_flags2 = POWERPC_FAMILY_POWER10_INSNS_FLAGS2;
+ pcc->msr_mask = POWERPC_POWER10_PCC_MSR_MASK;
+ pcc->lpcr_mask = POWERPC_POWER10_PCC_LPCR_MASK;
pcc->lpcr_pm = LPCR_PDEE | LPCR_HDEE | LPCR_EEE | LPCR_DEE | LPCR_OEE;
pcc->mmu_model = POWERPC_MMU_3_00;
@@ -6622,10 +6532,7 @@ POWERPC_FAMILY(POWER10)(ObjectClass *oc, void *data)
pcc->excp_model = POWERPC_EXCP_POWER10;
pcc->bus_model = PPC_FLAGS_INPUT_POWER9;
pcc->bfd_mach = bfd_mach_ppc64;
- pcc->flags = POWERPC_FLAG_VRE | POWERPC_FLAG_SE |
- POWERPC_FLAG_BE | POWERPC_FLAG_PMM |
- POWERPC_FLAG_BUS_CLK | POWERPC_FLAG_CFAR |
- POWERPC_FLAG_VSX | POWERPC_FLAG_SCV;
+ pcc->flags = POWERPC_POWER10_PCC_FLAGS;
pcc->l1_dcache_size = 0x8000;
pcc->l1_icache_size = 0x8000;
}
--
2.39.3
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v2 7/7] target/ppc: redue code duplication across Power9/10 init code
2024-05-23 5:14 ` [PATCH v2 7/7] target/ppc: redue code duplication across Power9/10 init code Harsh Prateek Bora
@ 2024-07-04 7:49 ` Nicholas Piggin
0 siblings, 0 replies; 16+ messages in thread
From: Nicholas Piggin @ 2024-07-04 7:49 UTC (permalink / raw)
To: Harsh Prateek Bora, qemu-ppc, qemu-devel; +Cc: balaton, danielhb413
On Thu May 23, 2024 at 3:14 PM AEST, Harsh Prateek Bora wrote:
> Power9/10 initialization code consists of a lot of logical OR of
> various flag bits as supported by respective Power platform during its
> initialization, most of which is duplicated and only selected bits are
> added or removed as needed with each new platform support being added.
> Remove the duplicate code and share using common macros.
This an the previous patch are fiddly to verify but good cleanups I
think. Couple of small things.
>
> Signed-off-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
> ---
> target/ppc/cpu_init.h | 77 ++++++++++++++++++++++++++
> target/ppc/cpu_init.c | 123 ++++++------------------------------------
> 2 files changed, 92 insertions(+), 108 deletions(-)
> create mode 100644 target/ppc/cpu_init.h
>
> diff --git a/target/ppc/cpu_init.h b/target/ppc/cpu_init.h
> new file mode 100644
> index 0000000000..53909987b0
> --- /dev/null
> +++ b/target/ppc/cpu_init.h
> @@ -0,0 +1,77 @@
This should have a SPDX license tag I guess.
I suppose doing a new header for it is the way to go. That cpu_init.c
file is enormous...
> +#ifndef TARGET_PPC_CPU_INIT_H
> +#define TARGET_PPC_CPU_INIT_H
> +
> +#define POWERPC_FAMILY_POWER9_INSNS_FLAGS \
> + PPC_INSNS_BASE | PPC_ISEL | PPC_STRING | PPC_MFTB | \
> + PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES | \
> + PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE | PPC_FLOAT_FRSQRTES | \
> + PPC_FLOAT_STFIWX | PPC_FLOAT_EXT |PPC_CACHE | PPC_CACHE_ICBI | \
> + PPC_CACHE_DCBZ | PPC_MEM_SYNC | PPC_MEM_EIEIO | PPC_MEM_TLBIE | \
> + PPC_MEM_TLBSYNC | PPC_64B | PPC_64H | PPC_64BX | PPC_ALTIVEC | \
> + PPC_SEGMENT_64B | PPC_SLBI | PPC_POPCNTB | PPC_POPCNTWD | \
> + PPC_CILDST
> +
> +#define POWERPC_FAMILY_POWER9_INSNS_FLAGS2_COMMON \
> + PPC2_VSX | PPC2_VSX207 | PPC2_DFP | PPC2_DBRX | \
> + PPC2_PERM_ISA206 | PPC2_DIVE_ISA206 | PPC2_ATOMIC_ISA206 | \
> + PPC2_FP_CVT_ISA206 | PPC2_FP_TST_ISA206 | PPC2_BCTAR_ISA207 | \
> + PPC2_LSQ_ISA207 | PPC2_ALTIVEC_207 | PPC2_ISA205 | \
> + PPC2_ISA207S | PPC2_FP_CVT_S64 | PPC2_ISA300 | PPC2_PRCNTL | \
> + PPC2_MEM_LWSYNC | PPC2_BCDA_ISA206
> +
> +#define POWERPC_FAMILY_POWER9_INSNS_FLAGS2 \
> + POWERPC_FAMILY_POWER9_INSNS_FLAGS2_COMMON | PPC2_TM
> +#define POWERPC_FAMILY_POWER10_INSNS_FLAGS2 \
> + POWERPC_FAMILY_POWER9_INSNS_FLAGS2_COMMON | PPC2_ISA310
> +
> +#define POWERPC_POWER9_COMMON_PCC_MSR_MASK \
> + (1ull << MSR_SF) | \
> + (1ull << MSR_HV) | \
> + (1ull << MSR_VR) | \
> + (1ull << MSR_VSX) | \
> + (1ull << MSR_EE) | \
> + (1ull << MSR_PR) | \
> + (1ull << MSR_FP) | \
> + (1ull << MSR_ME) | \
> + (1ull << MSR_FE0) | \
> + (1ull << MSR_SE) | \
> + (1ull << MSR_DE) | \
> + (1ull << MSR_FE1) | \
> + (1ull << MSR_IR) | \
> + (1ull << MSR_DR) | \
> + (1ull << MSR_PMM) | \
> + (1ull << MSR_RI) | \
> + (1ull << MSR_LE)
> +
> +#define POWERPC_POWER9_PCC_MSR_MASK \
> + POWERPC_POWER9_COMMON_PCC_MSR_MASK | (1ull << MSR_TM)
> +#define POWERPC_POWER10_PCC_MSR_MASK \
> + POWERPC_POWER9_COMMON_PCC_MSR_MASK
> +#define POWERPC_POWER9_PCC_PCR_MASK \
> + PCR_COMPAT_2_05 | PCR_COMPAT_2_06 | PCR_COMPAT_2_07
> +#define POWERPC_POWER10_PCC_PCR_MASK \
> + POWERPC_POWER9_PCC_PCR_MASK | PCR_COMPAT_3_00
> +#define POWERPC_POWER9_PCC_PCR_SUPPORTED \
> + PCR_COMPAT_3_00 | PCR_COMPAT_2_07 | PCR_COMPAT_2_06 | PCR_COMPAT_2_05
> +#define POWERPC_POWER10_PCC_PCR_SUPPORTED \
> + POWERPC_POWER9_PCC_PCR_SUPPORTED | PCR_COMPAT_3_10
> +#define POWERPC_POWER9_PCC_LPCR_MASK \
> + LPCR_VPM1 | LPCR_ISL | LPCR_KBV | LPCR_DPFD | \
> + (LPCR_PECE_U_MASK & LPCR_HVEE) | LPCR_ILE | LPCR_AIL | \
> + LPCR_UPRT | LPCR_EVIRT | LPCR_ONL | LPCR_HR | LPCR_LD | \
> + (LPCR_PECE_L_MASK & (LPCR_PDEE|LPCR_HDEE|LPCR_EEE|LPCR_DEE|LPCR_OEE)) | \
> + LPCR_MER | LPCR_GTSE | LPCR_TC | LPCR_HEIC | LPCR_LPES0 | LPCR_HVICE | \
> + LPCR_HDICE
> +/* DD2 adds an extra HAIL bit */
> +#define POWERPC_POWER10_PCC_LPCR_MASK \
> + POWERPC_POWER9_PCC_LPCR_MASK | LPCR_HAIL
> +#define POWERPC_POWER9_PCC_FLAGS_COMMON \
> + POWERPC_FLAG_VRE | POWERPC_FLAG_SE | POWERPC_FLAG_BE | \
> + POWERPC_FLAG_PMM | POWERPC_FLAG_BUS_CLK | POWERPC_FLAG_CFAR | \
> + POWERPC_FLAG_VSX | POWERPC_FLAG_SCV
> +
> +#define POWERPC_POWER9_PCC_FLAGS \
> + POWERPC_POWER9_PCC_FLAGS_COMMON | POWERPC_FLAG_TM
> +#define POWERPC_POWER10_PCC_FLAGS POWERPC_POWER9_PCC_FLAGS_COMMON
I can't tell if it would be better to add all the POWER9 defines then
all the POWER10 defines or interleave like this. I guess this is okay.
[snip]
> - pcc->lpcr_mask = LPCR_VPM1 | LPCR_ISL | LPCR_KBV | LPCR_DPFD |
> - (LPCR_PECE_U_MASK & LPCR_HVEE) | LPCR_ILE | LPCR_AIL |
> - LPCR_UPRT | LPCR_EVIRT | LPCR_ONL | LPCR_HR | LPCR_LD |
> - (LPCR_PECE_L_MASK & (LPCR_PDEE | LPCR_HDEE | LPCR_EEE |
> - LPCR_DEE | LPCR_OEE))
> - | LPCR_MER | LPCR_GTSE | LPCR_TC |
> - LPCR_HEIC | LPCR_LPES0 | LPCR_HVICE | LPCR_HDICE;
> - /* DD2 adds an extra HAIL bit */
> - pcc->lpcr_mask |= LPCR_HAIL;
> + pcc->insns_flags = POWERPC_FAMILY_POWER9_INSNS_FLAGS; /* same as P9 */
I would add POWER10 defines for all of them even the ones that are
the same, because if you're adding or changing flags around, you then
just have one place to go to which is the cpu_init.h file.
Thanks,
Nick
^ permalink raw reply [flat|nested] 16+ messages in thread