Re: [PATCH 5/9] target/ppc: Use ppc_interrupts_little_endian in powerpc_excp
On 1/4/22 15:11, Fabiano Rosas wrote:
Cédric Le Goater writes:
On 1/3/22 23:07, Fabiano Rosas wrote:
The ppc_interrupts_little_endian function is suitable for determining
the endianness of interrupts for all Book3S CPUs.
(I'm keeping the MSR check for the rest of the CPUs, but it will go
away in the next patch.)
Signed-off-by: Fabiano Rosas
---
target/ppc/excp_helper.c | 21 ++---
1 file changed, 2 insertions(+), 19 deletions(-)
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 0dbadc5d07..5d31940426 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -760,25 +760,8 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp)
* CPU, the HV mode, etc...
*/
#ifdef TARGET_PPC64
-if (excp_model == POWERPC_EXCP_POWER7) {
-if (!(new_msr & MSR_HVB) && (env->spr[SPR_LPCR] & LPCR_ILE)) {
-new_msr |= (target_ulong)1 << MSR_LE;
-}
-} else if (excp_model == POWERPC_EXCP_POWER8) {
-if (new_msr & MSR_HVB) {
-if (env->spr[SPR_HID0] & HID0_HILE) {
-new_msr |= (target_ulong)1 << MSR_LE;
-}
-} else if (env->spr[SPR_LPCR] & LPCR_ILE) {
-new_msr |= (target_ulong)1 << MSR_LE;
-}
-} else if (excp_model == POWERPC_EXCP_POWER9 ||
- excp_model == POWERPC_EXCP_POWER10) {
-if (new_msr & MSR_HVB) {
-if (env->spr[SPR_HID0] & HID0_POWER9_HILE) {
-new_msr |= (target_ulong)1 << MSR_LE;
-}
-} else if (env->spr[SPR_LPCR] & LPCR_ILE) {
+if (excp_model >= POWERPC_EXCP_970) {
why include POWERPC_EXCP_970 ? These CPUs do not support Little Endian.
Because the 970 exception model covers POWER5P as well which has ILE.
we need to untangle this first.
POWERPC_EXCP_970 is checked in dbcz and the HID5 bits are specific to 970.
May be add a POWERPC_EXCP_POWER5P ?
And looking at cpu_init.c, POWER5 uses a bunch of 970 functions. And
POWER7 uses the POWER5 ones. So we kind of have a dependency chain
there. That is why I'm always handling ">= 970" as "books".
This is a mess. We also have is_book3s_arch2x() but it will not apply
here.
C.
Re: [PATCH 5/9] target/ppc: Use ppc_interrupts_little_endian in powerpc_excp
Cédric Le Goater writes:
> On 1/3/22 23:07, Fabiano Rosas wrote:
>> The ppc_interrupts_little_endian function is suitable for determining
>> the endianness of interrupts for all Book3S CPUs.
>>
>> (I'm keeping the MSR check for the rest of the CPUs, but it will go
>> away in the next patch.)
>>
>> Signed-off-by: Fabiano Rosas
>> ---
>> target/ppc/excp_helper.c | 21 ++---
>> 1 file changed, 2 insertions(+), 19 deletions(-)
>>
>> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
>> index 0dbadc5d07..5d31940426 100644
>> --- a/target/ppc/excp_helper.c
>> +++ b/target/ppc/excp_helper.c
>> @@ -760,25 +760,8 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int
>> excp)
>>* CPU, the HV mode, etc...
>>*/
>> #ifdef TARGET_PPC64
>> -if (excp_model == POWERPC_EXCP_POWER7) {
>> -if (!(new_msr & MSR_HVB) && (env->spr[SPR_LPCR] & LPCR_ILE)) {
>> -new_msr |= (target_ulong)1 << MSR_LE;
>> -}
>> -} else if (excp_model == POWERPC_EXCP_POWER8) {
>> -if (new_msr & MSR_HVB) {
>> -if (env->spr[SPR_HID0] & HID0_HILE) {
>> -new_msr |= (target_ulong)1 << MSR_LE;
>> -}
>> -} else if (env->spr[SPR_LPCR] & LPCR_ILE) {
>> -new_msr |= (target_ulong)1 << MSR_LE;
>> -}
>> -} else if (excp_model == POWERPC_EXCP_POWER9 ||
>> - excp_model == POWERPC_EXCP_POWER10) {
>> -if (new_msr & MSR_HVB) {
>> -if (env->spr[SPR_HID0] & HID0_POWER9_HILE) {
>> -new_msr |= (target_ulong)1 << MSR_LE;
>> -}
>> -} else if (env->spr[SPR_LPCR] & LPCR_ILE) {
>> +if (excp_model >= POWERPC_EXCP_970) {
>
> why include POWERPC_EXCP_970 ? These CPUs do not support Little Endian.
>
Because the 970 exception model covers POWER5P as well which has ILE.
And looking at cpu_init.c, POWER5 uses a bunch of 970 functions. And
POWER7 uses the POWER5 ones. So we kind of have a dependency chain
there. That is why I'm always handling ">= 970" as "books".
Re: [PATCH 5/9] target/ppc: Use ppc_interrupts_little_endian in powerpc_excp
On 1/3/22 23:07, Fabiano Rosas wrote:
The ppc_interrupts_little_endian function is suitable for determining
the endianness of interrupts for all Book3S CPUs.
(I'm keeping the MSR check for the rest of the CPUs, but it will go
away in the next patch.)
Signed-off-by: Fabiano Rosas
---
target/ppc/excp_helper.c | 21 ++---
1 file changed, 2 insertions(+), 19 deletions(-)
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 0dbadc5d07..5d31940426 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -760,25 +760,8 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp)
* CPU, the HV mode, etc...
*/
#ifdef TARGET_PPC64
-if (excp_model == POWERPC_EXCP_POWER7) {
-if (!(new_msr & MSR_HVB) && (env->spr[SPR_LPCR] & LPCR_ILE)) {
-new_msr |= (target_ulong)1 << MSR_LE;
-}
-} else if (excp_model == POWERPC_EXCP_POWER8) {
-if (new_msr & MSR_HVB) {
-if (env->spr[SPR_HID0] & HID0_HILE) {
-new_msr |= (target_ulong)1 << MSR_LE;
-}
-} else if (env->spr[SPR_LPCR] & LPCR_ILE) {
-new_msr |= (target_ulong)1 << MSR_LE;
-}
-} else if (excp_model == POWERPC_EXCP_POWER9 ||
- excp_model == POWERPC_EXCP_POWER10) {
-if (new_msr & MSR_HVB) {
-if (env->spr[SPR_HID0] & HID0_POWER9_HILE) {
-new_msr |= (target_ulong)1 << MSR_LE;
-}
-} else if (env->spr[SPR_LPCR] & LPCR_ILE) {
+if (excp_model >= POWERPC_EXCP_970) {
why include POWERPC_EXCP_970 ? These CPUs do not support Little Endian.
Thanks,
C.
+if (ppc_interrupts_little_endian(cpu, !!(new_msr & MSR_HVB))) {
new_msr |= (target_ulong)1 << MSR_LE;
}
} else if (msr_ile) {
[PATCH 5/9] target/ppc: Use ppc_interrupts_little_endian in powerpc_excp
The ppc_interrupts_little_endian function is suitable for determining
the endianness of interrupts for all Book3S CPUs.
(I'm keeping the MSR check for the rest of the CPUs, but it will go
away in the next patch.)
Signed-off-by: Fabiano Rosas
---
target/ppc/excp_helper.c | 21 ++---
1 file changed, 2 insertions(+), 19 deletions(-)
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 0dbadc5d07..5d31940426 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -760,25 +760,8 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp)
* CPU, the HV mode, etc...
*/
#ifdef TARGET_PPC64
-if (excp_model == POWERPC_EXCP_POWER7) {
-if (!(new_msr & MSR_HVB) && (env->spr[SPR_LPCR] & LPCR_ILE)) {
-new_msr |= (target_ulong)1 << MSR_LE;
-}
-} else if (excp_model == POWERPC_EXCP_POWER8) {
-if (new_msr & MSR_HVB) {
-if (env->spr[SPR_HID0] & HID0_HILE) {
-new_msr |= (target_ulong)1 << MSR_LE;
-}
-} else if (env->spr[SPR_LPCR] & LPCR_ILE) {
-new_msr |= (target_ulong)1 << MSR_LE;
-}
-} else if (excp_model == POWERPC_EXCP_POWER9 ||
- excp_model == POWERPC_EXCP_POWER10) {
-if (new_msr & MSR_HVB) {
-if (env->spr[SPR_HID0] & HID0_POWER9_HILE) {
-new_msr |= (target_ulong)1 << MSR_LE;
-}
-} else if (env->spr[SPR_LPCR] & LPCR_ILE) {
+if (excp_model >= POWERPC_EXCP_970) {
+if (ppc_interrupts_little_endian(cpu, !!(new_msr & MSR_HVB))) {
new_msr |= (target_ulong)1 << MSR_LE;
}
} else if (msr_ile) {
--
2.33.1
