target/ppc/mem_helper.c | 15 ++++++++++----- target/ppc/translate.c | 20 +++++++++----------- 2 files changed, 19 insertions(+), 16 deletions(-)
dcbz was broken with the refactoring introduced in the External PID patch. The
GETPC() call is moved directly to the helper in this patch, to report correct PC
address. The issue did not always manifest: if the compiler decided to inline
the call, the PC wound up being correct. So in order to reproduce, use a debug
build.
A problem in dcbzep, which did not perform external PID access in its slow path
was also fixed.
dcbtst opcode is now fixed, it was swapped with dcbtstep.
I also misunderstood the instruction registration mechanism and the instructions
were not truly limited to BookE 2.06. The PPC_CACHE/PPC_INTEGER type mask was
changed to PPC_NONE.
Fixes: ea8073c10d ("target/ppc: add external PID support")
Signed-off-by: Roman Kapl <rka@sysgo.com>
---
This fixes the sandalfoot image boot. And thanks to PMM for spotting the GETPC
issue.
target/ppc/mem_helper.c | 15 ++++++++++-----
target/ppc/translate.c | 20 +++++++++-----------
2 files changed, 19 insertions(+), 16 deletions(-)
diff --git a/target/ppc/mem_helper.c b/target/ppc/mem_helper.c
index 0c39141ab7..9d140dc4a3 100644
--- a/target/ppc/mem_helper.c
+++ b/target/ppc/mem_helper.c
@@ -141,12 +141,13 @@ void helper_stsw(CPUPPCState *env, target_ulong addr, uint32_t nb,
}
}
-static void helper_dcbz_common(CPUPPCState *env, target_ulong addr,
- uint32_t opcode, int mmu_idx)
+static void dcbz_common(CPUPPCState *env, target_ulong addr,
+ uint32_t opcode, bool epid, uintptr_t pc)
{
target_ulong mask, dcbz_size = env->dcache_line_size;
uint32_t i;
void *haddr;
+ int mmu_idx = epid ? PPC_TLB_EPID_STORE : env->dmmu_idx;
#if defined(TARGET_PPC64)
/* Check for dcbz vs dcbzl on 970 */
@@ -172,19 +173,23 @@ static void helper_dcbz_common(CPUPPCState *env, target_ulong addr,
} else {
/* Slow path */
for (i = 0; i < dcbz_size; i += 8) {
- cpu_stq_data_ra(env, addr + i, 0, GETPC());
+ if (epid) {
+ cpu_stq_eps_ra(env, addr + i, 0, pc);
+ } else {
+ cpu_stq_data_ra(env, addr + i, 0, pc);
+ }
}
}
}
void helper_dcbz(CPUPPCState *env, target_ulong addr, uint32_t opcode)
{
- helper_dcbz_common(env, addr, opcode, env->dmmu_idx);
+ dcbz_common(env, addr, opcode, false, GETPC());
}
void helper_dcbzep(CPUPPCState *env, target_ulong addr, uint32_t opcode)
{
- helper_dcbz_common(env, addr, opcode, PPC_TLB_EPID_STORE);
+ dcbz_common(env, addr, opcode, true, GETPC());
}
void helper_icbi(CPUPPCState *env, target_ulong addr)
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 41322fb9c4..3e279bcbc9 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -6881,24 +6881,22 @@ GEN_HANDLER_E(mcrxrx, 0x1F, 0x00, 0x12, 0x007FF801, PPC_NONE, PPC2_ISA300),
GEN_HANDLER(mtmsr, 0x1F, 0x12, 0x04, 0x001EF801, PPC_MISC),
GEN_HANDLER(mtspr, 0x1F, 0x13, 0x0E, 0x00000000, PPC_MISC),
GEN_HANDLER(dcbf, 0x1F, 0x16, 0x02, 0x03C00001, PPC_CACHE),
-GEN_HANDLER_E(dcbfep, 0x1F, 0x1F, 0x03, 0x03C00001, PPC_CACHE, PPC2_BOOKE206),
+GEN_HANDLER_E(dcbfep, 0x1F, 0x1F, 0x03, 0x03C00001, PPC_NONE, PPC2_BOOKE206),
GEN_HANDLER(dcbi, 0x1F, 0x16, 0x0E, 0x03E00001, PPC_CACHE),
GEN_HANDLER(dcbst, 0x1F, 0x16, 0x01, 0x03E00001, PPC_CACHE),
-GEN_HANDLER_E(dcbstep, 0x1F, 0x1F, 0x01, 0x03E00001, PPC_CACHE, PPC2_BOOKE206),
+GEN_HANDLER_E(dcbstep, 0x1F, 0x1F, 0x01, 0x03E00001, PPC_NONE, PPC2_BOOKE206),
GEN_HANDLER(dcbt, 0x1F, 0x16, 0x08, 0x00000001, PPC_CACHE),
-GEN_HANDLER_E(dcbtep, 0x1F, 0x1F, 0x09, 0x00000001, PPC_CACHE, PPC2_BOOKE206),
-GEN_HANDLER(dcbtst, 0x1F, 0x1F, 0x07, 0x00000001, PPC_CACHE),
-GEN_HANDLER_E(dcbtstep, 0x1F, 0x16, 0x07, 0x00000001, PPC_CACHE, PPC2_BOOKE206),
+GEN_HANDLER_E(dcbtep, 0x1F, 0x1F, 0x09, 0x00000001, PPC_NONE, PPC2_BOOKE206),
+GEN_HANDLER(dcbtst, 0x1F, 0x16, 0x07, 0x00000001, PPC_CACHE),
+GEN_HANDLER_E(dcbtstep, 0x1F, 0x1F, 0x07, 0x00000001, PPC_NONE, PPC2_BOOKE206),
GEN_HANDLER_E(dcbtls, 0x1F, 0x06, 0x05, 0x02000001, PPC_BOOKE, PPC2_BOOKE206),
GEN_HANDLER(dcbz, 0x1F, 0x16, 0x1F, 0x03C00001, PPC_CACHE_DCBZ),
-GEN_HANDLER_E(dcbzep, 0x1F, 0x1F, 0x1F, 0x03C00001,
- PPC_CACHE_DCBZ, PPC2_BOOKE206),
+GEN_HANDLER_E(dcbzep, 0x1F, 0x1F, 0x1F, 0x03C00001, PPC_NONE, PPC2_BOOKE206),
GEN_HANDLER(dst, 0x1F, 0x16, 0x0A, 0x01800001, PPC_ALTIVEC),
GEN_HANDLER(dstst, 0x1F, 0x16, 0x0B, 0x01800001, PPC_ALTIVEC),
GEN_HANDLER(dss, 0x1F, 0x16, 0x19, 0x019FF801, PPC_ALTIVEC),
GEN_HANDLER(icbi, 0x1F, 0x16, 0x1E, 0x03E00001, PPC_CACHE_ICBI),
-GEN_HANDLER_E(icbiep, 0x1F, 0x1F, 0x1E, 0x03E00001,
- PPC_CACHE_ICBI, PPC2_BOOKE206),
+GEN_HANDLER_E(icbiep, 0x1F, 0x1F, 0x1E, 0x03E00001, PPC_NONE, PPC2_BOOKE206),
GEN_HANDLER(dcba, 0x1F, 0x16, 0x17, 0x03E00001, PPC_CACHE_DCBA),
GEN_HANDLER(mfsr, 0x1F, 0x13, 0x12, 0x0010F801, PPC_SEGMENT),
GEN_HANDLER(mfsrin, 0x1F, 0x13, 0x14, 0x001F0001, PPC_SEGMENT),
@@ -7205,7 +7203,7 @@ GEN_LDX(lwbr, ld32ur, 0x16, 0x10, PPC_INTEGER)
#undef GEN_LDEPX
#define GEN_LDEPX(name, ldop, opc2, opc3) \
GEN_HANDLER_E(name##epx, 0x1F, opc2, opc3, \
- 0x00000001, PPC_INTEGER, PPC2_BOOKE206),
+ 0x00000001, PPC_NONE, PPC2_BOOKE206),
GEN_LDEPX(lb, DEF_MEMOP(MO_UB), 0x1F, 0x02)
GEN_LDEPX(lh, DEF_MEMOP(MO_UW), 0x1F, 0x08)
@@ -7251,7 +7249,7 @@ GEN_STX(stwbr, st32r, 0x16, 0x14, PPC_INTEGER)
#undef GEN_STEPX
#define GEN_STEPX(name, ldop, opc2, opc3) \
GEN_HANDLER_E(name##epx, 0x1F, opc2, opc3, \
- 0x00000001, PPC_INTEGER, PPC2_BOOKE206),
+ 0x00000001, PPC_NONE, PPC2_BOOKE206),
GEN_STEPX(stb, DEF_MEMOP(MO_UB), 0x1F, 0x06)
GEN_STEPX(sth, DEF_MEMOP(MO_UW), 0x1F, 0x0C)
--
2.11.0
Hi Roman,
On 9/20/18 11:34 AM, Roman Kapl wrote:
> dcbz was broken with the refactoring introduced in the External PID patch. The
> GETPC() call is moved directly to the helper in this patch, to report correct PC
> address. The issue did not always manifest: if the compiler decided to inline
> the call, the PC wound up being correct. So in order to reproduce, use a debug
> build.
Can you split this patch in 4 please? (Easier to bisect or cherry-pick).
> A problem in dcbzep, which did not perform external PID access in its slow path
> was also fixed.
>
> dcbtst opcode is now fixed, it was swapped with dcbtstep.
>
> I also misunderstood the instruction registration mechanism and the instructions
> were not truly limited to BookE 2.06. The PPC_CACHE/PPC_INTEGER type mask was
> changed to PPC_NONE.
>
> Fixes: ea8073c10d ("target/ppc: add external PID support")
> Signed-off-by: Roman Kapl <rka@sysgo.com>
> ---
>
> This fixes the sandalfoot image boot. And thanks to PMM for spotting the GETPC
> issue.
>
> target/ppc/mem_helper.c | 15 ++++++++++-----
> target/ppc/translate.c | 20 +++++++++-----------
> 2 files changed, 19 insertions(+), 16 deletions(-)
>
> diff --git a/target/ppc/mem_helper.c b/target/ppc/mem_helper.c
> index 0c39141ab7..9d140dc4a3 100644
> --- a/target/ppc/mem_helper.c
> +++ b/target/ppc/mem_helper.c
> @@ -141,12 +141,13 @@ void helper_stsw(CPUPPCState *env, target_ulong addr, uint32_t nb,
> }
> }
>
> -static void helper_dcbz_common(CPUPPCState *env, target_ulong addr,
> - uint32_t opcode, int mmu_idx)
> +static void dcbz_common(CPUPPCState *env, target_ulong addr,
> + uint32_t opcode, bool epid, uintptr_t pc)
> {
> target_ulong mask, dcbz_size = env->dcache_line_size;
> uint32_t i;
> void *haddr;
> + int mmu_idx = epid ? PPC_TLB_EPID_STORE : env->dmmu_idx;
>
> #if defined(TARGET_PPC64)
> /* Check for dcbz vs dcbzl on 970 */
> @@ -172,19 +173,23 @@ static void helper_dcbz_common(CPUPPCState *env, target_ulong addr,
> } else {
> /* Slow path */
> for (i = 0; i < dcbz_size; i += 8) {
> - cpu_stq_data_ra(env, addr + i, 0, GETPC());
> + if (epid) {
> + cpu_stq_eps_ra(env, addr + i, 0, pc);
> + } else {
> + cpu_stq_data_ra(env, addr + i, 0, pc);
> + }
> }
> }
> }
>
> void helper_dcbz(CPUPPCState *env, target_ulong addr, uint32_t opcode)
> {
> - helper_dcbz_common(env, addr, opcode, env->dmmu_idx);
> + dcbz_common(env, addr, opcode, false, GETPC());
> }
>
> void helper_dcbzep(CPUPPCState *env, target_ulong addr, uint32_t opcode)
> {
> - helper_dcbz_common(env, addr, opcode, PPC_TLB_EPID_STORE);
> + dcbz_common(env, addr, opcode, true, GETPC());
> }
>
> void helper_icbi(CPUPPCState *env, target_ulong addr)
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index 41322fb9c4..3e279bcbc9 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -6881,24 +6881,22 @@ GEN_HANDLER_E(mcrxrx, 0x1F, 0x00, 0x12, 0x007FF801, PPC_NONE, PPC2_ISA300),
> GEN_HANDLER(mtmsr, 0x1F, 0x12, 0x04, 0x001EF801, PPC_MISC),
> GEN_HANDLER(mtspr, 0x1F, 0x13, 0x0E, 0x00000000, PPC_MISC),
> GEN_HANDLER(dcbf, 0x1F, 0x16, 0x02, 0x03C00001, PPC_CACHE),
> -GEN_HANDLER_E(dcbfep, 0x1F, 0x1F, 0x03, 0x03C00001, PPC_CACHE, PPC2_BOOKE206),
> +GEN_HANDLER_E(dcbfep, 0x1F, 0x1F, 0x03, 0x03C00001, PPC_NONE, PPC2_BOOKE206),
> GEN_HANDLER(dcbi, 0x1F, 0x16, 0x0E, 0x03E00001, PPC_CACHE),
> GEN_HANDLER(dcbst, 0x1F, 0x16, 0x01, 0x03E00001, PPC_CACHE),
> -GEN_HANDLER_E(dcbstep, 0x1F, 0x1F, 0x01, 0x03E00001, PPC_CACHE, PPC2_BOOKE206),
> +GEN_HANDLER_E(dcbstep, 0x1F, 0x1F, 0x01, 0x03E00001, PPC_NONE, PPC2_BOOKE206),
> GEN_HANDLER(dcbt, 0x1F, 0x16, 0x08, 0x00000001, PPC_CACHE),
> -GEN_HANDLER_E(dcbtep, 0x1F, 0x1F, 0x09, 0x00000001, PPC_CACHE, PPC2_BOOKE206),
> -GEN_HANDLER(dcbtst, 0x1F, 0x1F, 0x07, 0x00000001, PPC_CACHE),
> -GEN_HANDLER_E(dcbtstep, 0x1F, 0x16, 0x07, 0x00000001, PPC_CACHE, PPC2_BOOKE206),
> +GEN_HANDLER_E(dcbtep, 0x1F, 0x1F, 0x09, 0x00000001, PPC_NONE, PPC2_BOOKE206),
> +GEN_HANDLER(dcbtst, 0x1F, 0x16, 0x07, 0x00000001, PPC_CACHE),
> +GEN_HANDLER_E(dcbtstep, 0x1F, 0x1F, 0x07, 0x00000001, PPC_NONE, PPC2_BOOKE206),
> GEN_HANDLER_E(dcbtls, 0x1F, 0x06, 0x05, 0x02000001, PPC_BOOKE, PPC2_BOOKE206),
> GEN_HANDLER(dcbz, 0x1F, 0x16, 0x1F, 0x03C00001, PPC_CACHE_DCBZ),
> -GEN_HANDLER_E(dcbzep, 0x1F, 0x1F, 0x1F, 0x03C00001,
> - PPC_CACHE_DCBZ, PPC2_BOOKE206),
> +GEN_HANDLER_E(dcbzep, 0x1F, 0x1F, 0x1F, 0x03C00001, PPC_NONE, PPC2_BOOKE206),
> GEN_HANDLER(dst, 0x1F, 0x16, 0x0A, 0x01800001, PPC_ALTIVEC),
> GEN_HANDLER(dstst, 0x1F, 0x16, 0x0B, 0x01800001, PPC_ALTIVEC),
> GEN_HANDLER(dss, 0x1F, 0x16, 0x19, 0x019FF801, PPC_ALTIVEC),
> GEN_HANDLER(icbi, 0x1F, 0x16, 0x1E, 0x03E00001, PPC_CACHE_ICBI),
> -GEN_HANDLER_E(icbiep, 0x1F, 0x1F, 0x1E, 0x03E00001,
> - PPC_CACHE_ICBI, PPC2_BOOKE206),
> +GEN_HANDLER_E(icbiep, 0x1F, 0x1F, 0x1E, 0x03E00001, PPC_NONE, PPC2_BOOKE206),
> GEN_HANDLER(dcba, 0x1F, 0x16, 0x17, 0x03E00001, PPC_CACHE_DCBA),
> GEN_HANDLER(mfsr, 0x1F, 0x13, 0x12, 0x0010F801, PPC_SEGMENT),
> GEN_HANDLER(mfsrin, 0x1F, 0x13, 0x14, 0x001F0001, PPC_SEGMENT),
> @@ -7205,7 +7203,7 @@ GEN_LDX(lwbr, ld32ur, 0x16, 0x10, PPC_INTEGER)
> #undef GEN_LDEPX
> #define GEN_LDEPX(name, ldop, opc2, opc3) \
> GEN_HANDLER_E(name##epx, 0x1F, opc2, opc3, \
> - 0x00000001, PPC_INTEGER, PPC2_BOOKE206),
> + 0x00000001, PPC_NONE, PPC2_BOOKE206),
>
> GEN_LDEPX(lb, DEF_MEMOP(MO_UB), 0x1F, 0x02)
> GEN_LDEPX(lh, DEF_MEMOP(MO_UW), 0x1F, 0x08)
> @@ -7251,7 +7249,7 @@ GEN_STX(stwbr, st32r, 0x16, 0x14, PPC_INTEGER)
> #undef GEN_STEPX
> #define GEN_STEPX(name, ldop, opc2, opc3) \
> GEN_HANDLER_E(name##epx, 0x1F, opc2, opc3, \
> - 0x00000001, PPC_INTEGER, PPC2_BOOKE206),
> + 0x00000001, PPC_NONE, PPC2_BOOKE206),
>
> GEN_STEPX(stb, DEF_MEMOP(MO_UB), 0x1F, 0x06)
> GEN_STEPX(sth, DEF_MEMOP(MO_UW), 0x1F, 0x0C)
>
On 20/09/2018 10:34, Roman Kapl wrote:
> dcbz was broken with the refactoring introduced in the External PID patch. The
> GETPC() call is moved directly to the helper in this patch, to report correct PC
> address. The issue did not always manifest: if the compiler decided to inline
> the call, the PC wound up being correct. So in order to reproduce, use a debug
> build.
>
> A problem in dcbzep, which did not perform external PID access in its slow path
> was also fixed.
>
> dcbtst opcode is now fixed, it was swapped with dcbtstep.
>
> I also misunderstood the instruction registration mechanism and the instructions
> were not truly limited to BookE 2.06. The PPC_CACHE/PPC_INTEGER type mask was
> changed to PPC_NONE.
>
> Fixes: ea8073c10d ("target/ppc: add external PID support")
> Signed-off-by: Roman Kapl <rka@sysgo.com>
> ---
>
> This fixes the sandalfoot image boot. And thanks to PMM for spotting the GETPC
> issue.
>
> target/ppc/mem_helper.c | 15 ++++++++++-----
> target/ppc/translate.c | 20 +++++++++-----------
> 2 files changed, 19 insertions(+), 16 deletions(-)
>
> diff --git a/target/ppc/mem_helper.c b/target/ppc/mem_helper.c
> index 0c39141ab7..9d140dc4a3 100644
> --- a/target/ppc/mem_helper.c
> +++ b/target/ppc/mem_helper.c
> @@ -141,12 +141,13 @@ void helper_stsw(CPUPPCState *env, target_ulong addr, uint32_t nb,
> }
> }
>
> -static void helper_dcbz_common(CPUPPCState *env, target_ulong addr,
> - uint32_t opcode, int mmu_idx)
> +static void dcbz_common(CPUPPCState *env, target_ulong addr,
> + uint32_t opcode, bool epid, uintptr_t pc)
> {
> target_ulong mask, dcbz_size = env->dcache_line_size;
> uint32_t i;
> void *haddr;
> + int mmu_idx = epid ? PPC_TLB_EPID_STORE : env->dmmu_idx;
>
> #if defined(TARGET_PPC64)
> /* Check for dcbz vs dcbzl on 970 */
> @@ -172,19 +173,23 @@ static void helper_dcbz_common(CPUPPCState *env, target_ulong addr,
> } else {
> /* Slow path */
> for (i = 0; i < dcbz_size; i += 8) {
> - cpu_stq_data_ra(env, addr + i, 0, GETPC());
> + if (epid) {
> + cpu_stq_eps_ra(env, addr + i, 0, pc);
> + } else {
> + cpu_stq_data_ra(env, addr + i, 0, pc);
> + }
> }
> }
> }
>
> void helper_dcbz(CPUPPCState *env, target_ulong addr, uint32_t opcode)
> {
> - helper_dcbz_common(env, addr, opcode, env->dmmu_idx);
> + dcbz_common(env, addr, opcode, false, GETPC());
> }
>
> void helper_dcbzep(CPUPPCState *env, target_ulong addr, uint32_t opcode)
> {
> - helper_dcbz_common(env, addr, opcode, PPC_TLB_EPID_STORE);
> + dcbz_common(env, addr, opcode, true, GETPC());
> }
>
> void helper_icbi(CPUPPCState *env, target_ulong addr)
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index 41322fb9c4..3e279bcbc9 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -6881,24 +6881,22 @@ GEN_HANDLER_E(mcrxrx, 0x1F, 0x00, 0x12, 0x007FF801, PPC_NONE, PPC2_ISA300),
> GEN_HANDLER(mtmsr, 0x1F, 0x12, 0x04, 0x001EF801, PPC_MISC),
> GEN_HANDLER(mtspr, 0x1F, 0x13, 0x0E, 0x00000000, PPC_MISC),
> GEN_HANDLER(dcbf, 0x1F, 0x16, 0x02, 0x03C00001, PPC_CACHE),
> -GEN_HANDLER_E(dcbfep, 0x1F, 0x1F, 0x03, 0x03C00001, PPC_CACHE, PPC2_BOOKE206),
> +GEN_HANDLER_E(dcbfep, 0x1F, 0x1F, 0x03, 0x03C00001, PPC_NONE, PPC2_BOOKE206),
> GEN_HANDLER(dcbi, 0x1F, 0x16, 0x0E, 0x03E00001, PPC_CACHE),
> GEN_HANDLER(dcbst, 0x1F, 0x16, 0x01, 0x03E00001, PPC_CACHE),
> -GEN_HANDLER_E(dcbstep, 0x1F, 0x1F, 0x01, 0x03E00001, PPC_CACHE, PPC2_BOOKE206),
> +GEN_HANDLER_E(dcbstep, 0x1F, 0x1F, 0x01, 0x03E00001, PPC_NONE, PPC2_BOOKE206),
> GEN_HANDLER(dcbt, 0x1F, 0x16, 0x08, 0x00000001, PPC_CACHE),
> -GEN_HANDLER_E(dcbtep, 0x1F, 0x1F, 0x09, 0x00000001, PPC_CACHE, PPC2_BOOKE206),
> -GEN_HANDLER(dcbtst, 0x1F, 0x1F, 0x07, 0x00000001, PPC_CACHE),
> -GEN_HANDLER_E(dcbtstep, 0x1F, 0x16, 0x07, 0x00000001, PPC_CACHE, PPC2_BOOKE206),
> +GEN_HANDLER_E(dcbtep, 0x1F, 0x1F, 0x09, 0x00000001, PPC_NONE, PPC2_BOOKE206),
> +GEN_HANDLER(dcbtst, 0x1F, 0x16, 0x07, 0x00000001, PPC_CACHE),
> +GEN_HANDLER_E(dcbtstep, 0x1F, 0x1F, 0x07, 0x00000001, PPC_NONE, PPC2_BOOKE206),
> GEN_HANDLER_E(dcbtls, 0x1F, 0x06, 0x05, 0x02000001, PPC_BOOKE, PPC2_BOOKE206),
> GEN_HANDLER(dcbz, 0x1F, 0x16, 0x1F, 0x03C00001, PPC_CACHE_DCBZ),
> -GEN_HANDLER_E(dcbzep, 0x1F, 0x1F, 0x1F, 0x03C00001,
> - PPC_CACHE_DCBZ, PPC2_BOOKE206),
> +GEN_HANDLER_E(dcbzep, 0x1F, 0x1F, 0x1F, 0x03C00001, PPC_NONE, PPC2_BOOKE206),
> GEN_HANDLER(dst, 0x1F, 0x16, 0x0A, 0x01800001, PPC_ALTIVEC),
> GEN_HANDLER(dstst, 0x1F, 0x16, 0x0B, 0x01800001, PPC_ALTIVEC),
> GEN_HANDLER(dss, 0x1F, 0x16, 0x19, 0x019FF801, PPC_ALTIVEC),
> GEN_HANDLER(icbi, 0x1F, 0x16, 0x1E, 0x03E00001, PPC_CACHE_ICBI),
> -GEN_HANDLER_E(icbiep, 0x1F, 0x1F, 0x1E, 0x03E00001,
> - PPC_CACHE_ICBI, PPC2_BOOKE206),
> +GEN_HANDLER_E(icbiep, 0x1F, 0x1F, 0x1E, 0x03E00001, PPC_NONE, PPC2_BOOKE206),
> GEN_HANDLER(dcba, 0x1F, 0x16, 0x17, 0x03E00001, PPC_CACHE_DCBA),
> GEN_HANDLER(mfsr, 0x1F, 0x13, 0x12, 0x0010F801, PPC_SEGMENT),
> GEN_HANDLER(mfsrin, 0x1F, 0x13, 0x14, 0x001F0001, PPC_SEGMENT),
> @@ -7205,7 +7203,7 @@ GEN_LDX(lwbr, ld32ur, 0x16, 0x10, PPC_INTEGER)
> #undef GEN_LDEPX
> #define GEN_LDEPX(name, ldop, opc2, opc3) \
> GEN_HANDLER_E(name##epx, 0x1F, opc2, opc3, \
> - 0x00000001, PPC_INTEGER, PPC2_BOOKE206),
> + 0x00000001, PPC_NONE, PPC2_BOOKE206),
>
> GEN_LDEPX(lb, DEF_MEMOP(MO_UB), 0x1F, 0x02)
> GEN_LDEPX(lh, DEF_MEMOP(MO_UW), 0x1F, 0x08)
> @@ -7251,7 +7249,7 @@ GEN_STX(stwbr, st32r, 0x16, 0x14, PPC_INTEGER)
> #undef GEN_STEPX
> #define GEN_STEPX(name, ldop, opc2, opc3) \
> GEN_HANDLER_E(name##epx, 0x1F, opc2, opc3, \
> - 0x00000001, PPC_INTEGER, PPC2_BOOKE206),
> + 0x00000001, PPC_NONE, PPC2_BOOKE206),
>
> GEN_STEPX(stb, DEF_MEMOP(MO_UB), 0x1F, 0x06)
> GEN_STEPX(sth, DEF_MEMOP(MO_UW), 0x1F, 0x0C)
>
Given that the previous (broken) version of this patch is already queued in
ppc-for-3.1, would it be better to send a v2 whereby these changes are squashed into
the original?
ATB,
Mark.
Hi, > > Given that the previous (broken) version of this patch is already queued in > ppc-for-3.1, would it be better to send a v2 whereby these changes are squashed into > the original? > I don't know if David Gibson will want to modify history in his branch -- maybe other people depend on it? And anyway, Phillipe wants it even more granular. I don't have any strong opinions about it, David can either squash it or tell me to split it, I just need some consensus. I thought that splitting this was overdoing it a bit and does not have that many advantages given how close the commits are. Thanks, Roman Kapl
On 9/20/18 12:31 PM, Roman Kapl wrote:
>> Given that the previous (broken) version of this patch is already
>> queued in
>> ppc-for-3.1, would it be better to send a v2 whereby these changes are
>> squashed into
>> the original?
>>
>
> I don't know if David Gibson will want to modify history in his branch
> -- maybe other people depend on it?
>
> And anyway, Phillipe wants it even more granular.
>
> I don't have any strong opinions about it, David can either squash it or
> tell me to split it, I just need some consensus. I thought that
> splitting this was overdoing it a bit and does not have that many
> advantages given how close the commits are.
> Fixes: ea8073c10d ("target/ppc: add external PID support")
Your previous patch is now d12a22c5c7 in David Gibson's tree, so I think
here squashing your fixes (once this patch got reviewed) is the best option.
Regards,
Phil.
On 09/20/2018 01:13 PM, Philippe Mathieu-Daudé wrote:
>
>> Fixes: ea8073c10d ("target/ppc: add external PID support")
>
> Your previous patch is now d12a22c5c7 in David Gibson's tree, so I think
> here squashing your fixes (once this patch got reviewed) is the best option.
You are right -- I've referenced the wrong tree. At least this needs to
be fixed if it won't be squashed.
Thanks, Roman Kapl
On Thu, Sep 20, 2018 at 01:17:59PM +0200, Roman Kapl wrote:
> On 09/20/2018 01:13 PM, Philippe Mathieu-Daudé wrote:
> >
> > > Fixes: ea8073c10d ("target/ppc: add external PID support")
> >
> > Your previous patch is now d12a22c5c7 in David Gibson's tree, so I think
> > here squashing your fixes (once this patch got reviewed) is the best option.
>
> You are right -- I've referenced the wrong tree. At least this needs to be
> fixed if it won't be squashed.
I've squashed it into the original patch.
For future reference, my ppc-for-3.1 branch is a rebasing tree, so
SHAs in there can't be relied on to remain stable.
--
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
On Fri, Sep 21, 2018 at 10:55:56AM +1000, David Gibson wrote:
> On Thu, Sep 20, 2018 at 01:17:59PM +0200, Roman Kapl wrote:
> > On 09/20/2018 01:13 PM, Philippe Mathieu-Daudé wrote:
> > >
> > > > Fixes: ea8073c10d ("target/ppc: add external PID support")
> > >
> > > Your previous patch is now d12a22c5c7 in David Gibson's tree, so I think
> > > here squashing your fixes (once this patch got reviewed) is the best option.
> >
> > You are right -- I've referenced the wrong tree. At least this needs to be
> > fixed if it won't be squashed.
>
> I've squashed it into the original patch.
Well, I had squashed it, but it turns out the "corrected" version
breaks compile for the ppc64abi32-linux-user target. Please remember
to do an all-targets build before sending patches.
For now I've pulled the whole thing from my tree, please send a v2
with the fixes folded in.
> For future reference, my ppc-for-3.1 branch is a rebasing tree, so
> SHAs in there can't be relied on to remain stable.
--
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
On 20 September 2018 at 02:34, Roman Kapl <rka@sysgo.com> wrote:
> dcbz was broken with the refactoring introduced in the External PID patch. The
> GETPC() call is moved directly to the helper in this patch, to report correct PC
> address. The issue did not always manifest: if the compiler decided to inline
> the call, the PC wound up being correct. So in order to reproduce, use a debug
> build.
>
> A problem in dcbzep, which did not perform external PID access in its slow path
> was also fixed.
>
> dcbtst opcode is now fixed, it was swapped with dcbtstep.
>
> I also misunderstood the instruction registration mechanism and the instructions
> were not truly limited to BookE 2.06. The PPC_CACHE/PPC_INTEGER type mask was
> changed to PPC_NONE.
>
> Fixes: ea8073c10d ("target/ppc: add external PID support")
> Signed-off-by: Roman Kapl <rka@sysgo.com>
> ---
>
> This fixes the sandalfoot image boot. And thanks to PMM for spotting the GETPC
> issue.
>
> target/ppc/mem_helper.c | 15 ++++++++++-----
> target/ppc/translate.c | 20 +++++++++-----------
> 2 files changed, 19 insertions(+), 16 deletions(-)
>
> diff --git a/target/ppc/mem_helper.c b/target/ppc/mem_helper.c
> index 0c39141ab7..9d140dc4a3 100644
> --- a/target/ppc/mem_helper.c
> +++ b/target/ppc/mem_helper.c
> @@ -141,12 +141,13 @@ void helper_stsw(CPUPPCState *env, target_ulong addr, uint32_t nb,
> }
> }
>
> -static void helper_dcbz_common(CPUPPCState *env, target_ulong addr,
> - uint32_t opcode, int mmu_idx)
> +static void dcbz_common(CPUPPCState *env, target_ulong addr,
> + uint32_t opcode, bool epid, uintptr_t pc)
"retaddr" is the more usual name for this parameter rather than "pc".
thanks
-- PMM
© 2016 - 2025 Red Hat, Inc.