* [PATCH 1/2] target/ppc: Introduce ppc_interrupts_little_endian()
2021-06-22 14:09 [PATCH 0/2] target/ppc: Drop PowerPCCPUClass::interrupts_big_endian() Greg Kurz
@ 2021-06-22 14:09 ` Greg Kurz
2021-06-22 18:52 ` Fabiano Rosas
2021-06-22 14:09 ` [PATCH 2/2] target/ppc: Drop PowerPCCPUClass::interrupts_big_endian() Greg Kurz
2021-06-24 1:25 ` [PATCH 0/2] " David Gibson
2 siblings, 1 reply; 6+ messages in thread
From: Greg Kurz @ 2021-06-22 14:09 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-ppc, Greg Kurz, David Gibson
PowerPC CPUs use big endian by default but starting with POWER7,
server grade CPUs use the ILE bit of the LPCR special purpose
register to decide on the endianness to use when handling
interrupts. This gives a clue to QEMU on the endianness the
guest kernel is running, which is needed when generating an
ELF dump of the guest or when delivering an FWNMI machine
check interrupt.
Commit 382d2db62bcb ("target-ppc: Introduce callback for interrupt
endianness") added a class method to PowerPCCPUClass to modelize
this : default implementation returns a fixed "big endian" value,
while POWER7 and newer do the LPCR_ILE check. This is suboptimal
as it forces to implement the method for every new CPU family, and
it is very unlikely that this will ever be different than what we
have today.
We basically only have three cases to consider:
a) CPU doesn't have an LPCR => big endian
b) CPU has an LPCR but doesn't support the ILE bit => big endian
c) CPU has an LPCR and supports the ILE bit => little or big endian
Instead of class methods, introduce an inline helper that checks the
ILE bit in the LPCR_MASK to decide on the outcome. The new helper
words little endian instead of big endian. This allows to drop a !
operator in ppc_cpu_do_fwnmi_machine_check().
Signed-off-by: Greg Kurz <groug@kaod.org>
---
target/ppc/cpu.h | 15 +++++++++++++++
target/ppc/arch_dump.c | 8 +++-----
target/ppc/excp_helper.c | 3 +--
3 files changed, 19 insertions(+), 7 deletions(-)
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index b4de0db7ff5c..93d308ac8f2d 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -2643,6 +2643,21 @@ static inline bool ppc_has_spr(PowerPCCPU *cpu, int spr)
return cpu->env.spr_cb[spr].name != NULL;
}
+static inline bool ppc_interrupts_little_endian(PowerPCCPU *cpu)
+{
+ PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
+
+ /*
+ * Only models that have an LPCR and know about LPCR_ILE can do little
+ * endian.
+ */
+ if (pcc->lpcr_mask & LPCR_ILE) {
+ return !!(cpu->env.spr[SPR_LPCR] & LPCR_ILE);
+ }
+
+ return false;
+}
+
void dump_mmu(CPUPPCState *env);
void ppc_maybe_bswap_register(CPUPPCState *env, uint8_t *mem_buf, int len);
diff --git a/target/ppc/arch_dump.c b/target/ppc/arch_dump.c
index 9210e61ef463..bb392f6d8885 100644
--- a/target/ppc/arch_dump.c
+++ b/target/ppc/arch_dump.c
@@ -227,22 +227,20 @@ int cpu_get_dump_info(ArchDumpInfo *info,
const struct GuestPhysBlockList *guest_phys_blocks)
{
PowerPCCPU *cpu;
- PowerPCCPUClass *pcc;
if (first_cpu == NULL) {
return -1;
}
cpu = POWERPC_CPU(first_cpu);
- pcc = POWERPC_CPU_GET_CLASS(cpu);
info->d_machine = PPC_ELF_MACHINE;
info->d_class = ELFCLASS;
- if ((*pcc->interrupts_big_endian)(cpu)) {
- info->d_endian = ELFDATA2MSB;
- } else {
+ if (ppc_interrupts_little_endian(cpu)) {
info->d_endian = ELFDATA2LSB;
+ } else {
+ info->d_endian = ELFDATA2MSB;
}
/* 64KB is the max page size for pseries kernel */
if (strncmp(object_get_typename(qdev_get_machine()),
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index fd147e2a3766..a79a0ed465e5 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -1099,7 +1099,6 @@ void ppc_cpu_do_fwnmi_machine_check(CPUState *cs, target_ulong vector)
{
PowerPCCPU *cpu = POWERPC_CPU(cs);
CPUPPCState *env = &cpu->env;
- PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
target_ulong msr = 0;
/*
@@ -1108,7 +1107,7 @@ void ppc_cpu_do_fwnmi_machine_check(CPUState *cs, target_ulong vector)
*/
msr = (1ULL << MSR_ME);
msr |= env->msr & (1ULL << MSR_SF);
- if (!(*pcc->interrupts_big_endian)(cpu)) {
+ if (ppc_interrupts_little_endian(cpu)) {
msr |= (1ULL << MSR_LE);
}
--
2.31.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 1/2] target/ppc: Introduce ppc_interrupts_little_endian()
2021-06-22 14:09 ` [PATCH 1/2] target/ppc: Introduce ppc_interrupts_little_endian() Greg Kurz
@ 2021-06-22 18:52 ` Fabiano Rosas
0 siblings, 0 replies; 6+ messages in thread
From: Fabiano Rosas @ 2021-06-22 18:52 UTC (permalink / raw)
To: Greg Kurz, qemu-devel; +Cc: qemu-ppc, Greg Kurz, David Gibson
Greg Kurz <groug@kaod.org> writes:
> PowerPC CPUs use big endian by default but starting with POWER7,
> server grade CPUs use the ILE bit of the LPCR special purpose
> register to decide on the endianness to use when handling
> interrupts. This gives a clue to QEMU on the endianness the
> guest kernel is running, which is needed when generating an
> ELF dump of the guest or when delivering an FWNMI machine
> check interrupt.
>
> Commit 382d2db62bcb ("target-ppc: Introduce callback for interrupt
> endianness") added a class method to PowerPCCPUClass to modelize
> this : default implementation returns a fixed "big endian" value,
> while POWER7 and newer do the LPCR_ILE check. This is suboptimal
> as it forces to implement the method for every new CPU family, and
> it is very unlikely that this will ever be different than what we
> have today.
>
> We basically only have three cases to consider:
> a) CPU doesn't have an LPCR => big endian
> b) CPU has an LPCR but doesn't support the ILE bit => big endian
> c) CPU has an LPCR and supports the ILE bit => little or big endian
>
> Instead of class methods, introduce an inline helper that checks the
> ILE bit in the LPCR_MASK to decide on the outcome. The new helper
> words little endian instead of big endian. This allows to drop a !
> operator in ppc_cpu_do_fwnmi_machine_check().
>
> Signed-off-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Fabiano Rosas <farosas@linux.ibm.com>
> ---
> target/ppc/cpu.h | 15 +++++++++++++++
> target/ppc/arch_dump.c | 8 +++-----
> target/ppc/excp_helper.c | 3 +--
> 3 files changed, 19 insertions(+), 7 deletions(-)
>
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index b4de0db7ff5c..93d308ac8f2d 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -2643,6 +2643,21 @@ static inline bool ppc_has_spr(PowerPCCPU *cpu, int spr)
> return cpu->env.spr_cb[spr].name != NULL;
> }
>
> +static inline bool ppc_interrupts_little_endian(PowerPCCPU *cpu)
> +{
> + PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
> +
> + /*
> + * Only models that have an LPCR and know about LPCR_ILE can do little
> + * endian.
> + */
> + if (pcc->lpcr_mask & LPCR_ILE) {
> + return !!(cpu->env.spr[SPR_LPCR] & LPCR_ILE);
> + }
> +
> + return false;
> +}
> +
> void dump_mmu(CPUPPCState *env);
>
> void ppc_maybe_bswap_register(CPUPPCState *env, uint8_t *mem_buf, int len);
> diff --git a/target/ppc/arch_dump.c b/target/ppc/arch_dump.c
> index 9210e61ef463..bb392f6d8885 100644
> --- a/target/ppc/arch_dump.c
> +++ b/target/ppc/arch_dump.c
> @@ -227,22 +227,20 @@ int cpu_get_dump_info(ArchDumpInfo *info,
> const struct GuestPhysBlockList *guest_phys_blocks)
> {
> PowerPCCPU *cpu;
> - PowerPCCPUClass *pcc;
>
> if (first_cpu == NULL) {
> return -1;
> }
>
> cpu = POWERPC_CPU(first_cpu);
> - pcc = POWERPC_CPU_GET_CLASS(cpu);
>
> info->d_machine = PPC_ELF_MACHINE;
> info->d_class = ELFCLASS;
>
> - if ((*pcc->interrupts_big_endian)(cpu)) {
> - info->d_endian = ELFDATA2MSB;
> - } else {
> + if (ppc_interrupts_little_endian(cpu)) {
> info->d_endian = ELFDATA2LSB;
> + } else {
> + info->d_endian = ELFDATA2MSB;
> }
> /* 64KB is the max page size for pseries kernel */
> if (strncmp(object_get_typename(qdev_get_machine()),
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index fd147e2a3766..a79a0ed465e5 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -1099,7 +1099,6 @@ void ppc_cpu_do_fwnmi_machine_check(CPUState *cs, target_ulong vector)
> {
> PowerPCCPU *cpu = POWERPC_CPU(cs);
> CPUPPCState *env = &cpu->env;
> - PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
> target_ulong msr = 0;
>
> /*
> @@ -1108,7 +1107,7 @@ void ppc_cpu_do_fwnmi_machine_check(CPUState *cs, target_ulong vector)
> */
> msr = (1ULL << MSR_ME);
> msr |= env->msr & (1ULL << MSR_SF);
> - if (!(*pcc->interrupts_big_endian)(cpu)) {
> + if (ppc_interrupts_little_endian(cpu)) {
> msr |= (1ULL << MSR_LE);
> }
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] target/ppc: Drop PowerPCCPUClass::interrupts_big_endian()
2021-06-22 14:09 [PATCH 0/2] target/ppc: Drop PowerPCCPUClass::interrupts_big_endian() Greg Kurz
2021-06-22 14:09 ` [PATCH 1/2] target/ppc: Introduce ppc_interrupts_little_endian() Greg Kurz
@ 2021-06-22 14:09 ` Greg Kurz
2021-06-22 18:53 ` Fabiano Rosas
2021-06-24 1:25 ` [PATCH 0/2] " David Gibson
2 siblings, 1 reply; 6+ messages in thread
From: Greg Kurz @ 2021-06-22 14:09 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-ppc, Greg Kurz, David Gibson
This isn't used anymore.
Signed-off-by: Greg Kurz <groug@kaod.org>
---
target/ppc/cpu-qom.h | 1 -
target/ppc/cpu_init.c | 17 -----------------
2 files changed, 18 deletions(-)
diff --git a/target/ppc/cpu-qom.h b/target/ppc/cpu-qom.h
index 06b6571bc9d5..7b424e3cb0bc 100644
--- a/target/ppc/cpu-qom.h
+++ b/target/ppc/cpu-qom.h
@@ -199,7 +199,6 @@ struct PowerPCCPUClass {
void (*init_proc)(CPUPPCState *env);
int (*check_pow)(CPUPPCState *env);
int (*handle_mmu_fault)(PowerPCCPU *cpu, vaddr eaddr, int rwx, int mmu_idx);
- bool (*interrupts_big_endian)(PowerPCCPU *cpu);
};
#ifndef CONFIG_USER_ONLY
diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index d0411e7302a2..1a22aef874b1 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -2666,18 +2666,6 @@ static int check_pow_hid0_74xx(CPUPPCState *env)
return 0;
}
-static bool ppc_cpu_interrupts_big_endian_always(PowerPCCPU *cpu)
-{
- return true;
-}
-
-#ifdef TARGET_PPC64
-static bool ppc_cpu_interrupts_big_endian_lpcr(PowerPCCPU *cpu)
-{
- return !(cpu->env.spr[SPR_LPCR] & LPCR_ILE);
-}
-#endif
-
/*****************************************************************************/
/* PowerPC implementations definitions */
@@ -7740,7 +7728,6 @@ POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data)
POWERPC_FLAG_VSX;
pcc->l1_dcache_size = 0x8000;
pcc->l1_icache_size = 0x8000;
- pcc->interrupts_big_endian = ppc_cpu_interrupts_big_endian_lpcr;
}
static void init_proc_POWER8(CPUPPCState *env)
@@ -7918,7 +7905,6 @@ POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data)
POWERPC_FLAG_VSX | POWERPC_FLAG_TM;
pcc->l1_dcache_size = 0x8000;
pcc->l1_icache_size = 0x8000;
- pcc->interrupts_big_endian = ppc_cpu_interrupts_big_endian_lpcr;
}
#ifdef CONFIG_SOFTMMU
@@ -8136,7 +8122,6 @@ POWERPC_FAMILY(POWER9)(ObjectClass *oc, void *data)
POWERPC_FLAG_VSX | POWERPC_FLAG_TM | POWERPC_FLAG_SCV;
pcc->l1_dcache_size = 0x8000;
pcc->l1_icache_size = 0x8000;
- pcc->interrupts_big_endian = ppc_cpu_interrupts_big_endian_lpcr;
}
#ifdef CONFIG_SOFTMMU
@@ -8347,7 +8332,6 @@ POWERPC_FAMILY(POWER10)(ObjectClass *oc, void *data)
POWERPC_FLAG_VSX | POWERPC_FLAG_TM | POWERPC_FLAG_SCV;
pcc->l1_dcache_size = 0x8000;
pcc->l1_icache_size = 0x8000;
- pcc->interrupts_big_endian = ppc_cpu_interrupts_big_endian_lpcr;
}
#if !defined(CONFIG_USER_ONLY)
@@ -9094,7 +9078,6 @@ static void ppc_cpu_class_init(ObjectClass *oc, void *data)
device_class_set_parent_unrealize(dc, ppc_cpu_unrealize,
&pcc->parent_unrealize);
pcc->pvr_match = ppc_pvr_match_default;
- pcc->interrupts_big_endian = ppc_cpu_interrupts_big_endian_always;
device_class_set_props(dc, ppc_cpu_properties);
device_class_set_parent_reset(dc, ppc_cpu_reset, &pcc->parent_reset);
--
2.31.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 2/2] target/ppc: Drop PowerPCCPUClass::interrupts_big_endian()
2021-06-22 14:09 ` [PATCH 2/2] target/ppc: Drop PowerPCCPUClass::interrupts_big_endian() Greg Kurz
@ 2021-06-22 18:53 ` Fabiano Rosas
0 siblings, 0 replies; 6+ messages in thread
From: Fabiano Rosas @ 2021-06-22 18:53 UTC (permalink / raw)
To: Greg Kurz, qemu-devel; +Cc: qemu-ppc, Greg Kurz, David Gibson
Greg Kurz <groug@kaod.org> writes:
> This isn't used anymore.
>
> Signed-off-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Fabiano Rosas <farosas@linux.ibm.com>
> ---
> target/ppc/cpu-qom.h | 1 -
> target/ppc/cpu_init.c | 17 -----------------
> 2 files changed, 18 deletions(-)
>
> diff --git a/target/ppc/cpu-qom.h b/target/ppc/cpu-qom.h
> index 06b6571bc9d5..7b424e3cb0bc 100644
> --- a/target/ppc/cpu-qom.h
> +++ b/target/ppc/cpu-qom.h
> @@ -199,7 +199,6 @@ struct PowerPCCPUClass {
> void (*init_proc)(CPUPPCState *env);
> int (*check_pow)(CPUPPCState *env);
> int (*handle_mmu_fault)(PowerPCCPU *cpu, vaddr eaddr, int rwx, int mmu_idx);
> - bool (*interrupts_big_endian)(PowerPCCPU *cpu);
> };
>
> #ifndef CONFIG_USER_ONLY
> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
> index d0411e7302a2..1a22aef874b1 100644
> --- a/target/ppc/cpu_init.c
> +++ b/target/ppc/cpu_init.c
> @@ -2666,18 +2666,6 @@ static int check_pow_hid0_74xx(CPUPPCState *env)
> return 0;
> }
>
> -static bool ppc_cpu_interrupts_big_endian_always(PowerPCCPU *cpu)
> -{
> - return true;
> -}
> -
> -#ifdef TARGET_PPC64
> -static bool ppc_cpu_interrupts_big_endian_lpcr(PowerPCCPU *cpu)
> -{
> - return !(cpu->env.spr[SPR_LPCR] & LPCR_ILE);
> -}
> -#endif
> -
> /*****************************************************************************/
> /* PowerPC implementations definitions */
>
> @@ -7740,7 +7728,6 @@ POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data)
> POWERPC_FLAG_VSX;
> pcc->l1_dcache_size = 0x8000;
> pcc->l1_icache_size = 0x8000;
> - pcc->interrupts_big_endian = ppc_cpu_interrupts_big_endian_lpcr;
> }
>
> static void init_proc_POWER8(CPUPPCState *env)
> @@ -7918,7 +7905,6 @@ POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data)
> POWERPC_FLAG_VSX | POWERPC_FLAG_TM;
> pcc->l1_dcache_size = 0x8000;
> pcc->l1_icache_size = 0x8000;
> - pcc->interrupts_big_endian = ppc_cpu_interrupts_big_endian_lpcr;
> }
>
> #ifdef CONFIG_SOFTMMU
> @@ -8136,7 +8122,6 @@ POWERPC_FAMILY(POWER9)(ObjectClass *oc, void *data)
> POWERPC_FLAG_VSX | POWERPC_FLAG_TM | POWERPC_FLAG_SCV;
> pcc->l1_dcache_size = 0x8000;
> pcc->l1_icache_size = 0x8000;
> - pcc->interrupts_big_endian = ppc_cpu_interrupts_big_endian_lpcr;
> }
>
> #ifdef CONFIG_SOFTMMU
> @@ -8347,7 +8332,6 @@ POWERPC_FAMILY(POWER10)(ObjectClass *oc, void *data)
> POWERPC_FLAG_VSX | POWERPC_FLAG_TM | POWERPC_FLAG_SCV;
> pcc->l1_dcache_size = 0x8000;
> pcc->l1_icache_size = 0x8000;
> - pcc->interrupts_big_endian = ppc_cpu_interrupts_big_endian_lpcr;
> }
>
> #if !defined(CONFIG_USER_ONLY)
> @@ -9094,7 +9078,6 @@ static void ppc_cpu_class_init(ObjectClass *oc, void *data)
> device_class_set_parent_unrealize(dc, ppc_cpu_unrealize,
> &pcc->parent_unrealize);
> pcc->pvr_match = ppc_pvr_match_default;
> - pcc->interrupts_big_endian = ppc_cpu_interrupts_big_endian_always;
> device_class_set_props(dc, ppc_cpu_properties);
>
> device_class_set_parent_reset(dc, ppc_cpu_reset, &pcc->parent_reset);
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 0/2] target/ppc: Drop PowerPCCPUClass::interrupts_big_endian()
2021-06-22 14:09 [PATCH 0/2] target/ppc: Drop PowerPCCPUClass::interrupts_big_endian() Greg Kurz
2021-06-22 14:09 ` [PATCH 1/2] target/ppc: Introduce ppc_interrupts_little_endian() Greg Kurz
2021-06-22 14:09 ` [PATCH 2/2] target/ppc: Drop PowerPCCPUClass::interrupts_big_endian() Greg Kurz
@ 2021-06-24 1:25 ` David Gibson
2 siblings, 0 replies; 6+ messages in thread
From: David Gibson @ 2021-06-24 1:25 UTC (permalink / raw)
To: Greg Kurz; +Cc: qemu-ppc, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 873 bytes --]
On Tue, Jun 22, 2021 at 04:09:24PM +0200, Greg Kurz wrote:
> Class method is overkill and cause code duplication in CPU setup functions.
> Switch to a more lightweight solution with a unique inline helper.
LGTM, applied to ppc-for-6.1, thanks.
>
> Greg Kurz (2):
> target/ppc: Introduce ppc_interrupts_little_endian()
> target/ppc: Drop PowerPCCPUClass::interrupts_big_endian()
>
> target/ppc/cpu-qom.h | 1 -
> target/ppc/cpu.h | 15 +++++++++++++++
> target/ppc/arch_dump.c | 8 +++-----
> target/ppc/cpu_init.c | 17 -----------------
> target/ppc/excp_helper.c | 3 +--
> 5 files changed, 19 insertions(+), 25 deletions(-)
>
--
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] 6+ messages in thread