| Message ID | 20240719010707.1319675-7-richard.henderson@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fixes for user-only munmap races | expand |
Hi, On Fri, Jul 19, 2024 at 11:07:01AM +1000, Richard Henderson wrote: > We can determine at translation time whether the insn is or > is not dbczl. We must retain a runtime check against the > HID5 register, but we can move that to a separate function > that never affects other ppc models. > > Reviewed-by: Nicholas Piggin <npiggin@gmail.com> > Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> I see an odd failure when trying to boot ppc64 images in qemu v9.1.0 and v9.1.1. Starting network: udhcpc: started, v1.36.1 malloc(): corrupted top size Aborted /usr/share/udhcpc/default.script: line 41: can't create : nonexistent directory I bisected the problem to this patch; bisect log is attached. I also found that the problem has been fixed in mainline qemu. Bisect points to commit: f168808d7d10 ("accel/tcg: Add TCGCPUOps.tlb_fill_align") as the fix. I attached this bisect log as well. Reverting this patch isn't easy due to several follow-up commits. Applying commit f168808d7d10 plus several preceding commits on top of v9.1.1 seems to fix the problem. The commits I applied are da335fe12a5d include/exec/memop: Move get_alignment_bits from tcg.h c5809eee452b include/exec/memop: Rename get_alignment_bits e5b063e81fd2 include/exec/memop: Introduce memop_atomicity_bits f168808d7d10 accel/tcg: Add TCGCPUOps.tlb_fill_align Obviously I have no idea if this is even remotely correct, so please take this report as purely informational in case someone else observes a similar problem. Thanks, Guenter --- Bug introduced: # bad: [fd1952d814da738ed107e05583b3e02ac11e88ff] Update version for v9.1.0 release # good: [c25df57ae8f9fe1c72eee2dab37d76d904ac382e] Update version for 9.0.0 release git bisect start 'v9.1.0' 'v9.0.0' # good: [2529ea2d561ea9fe359fb19ebdcfeb8b6cddd219] hw/acpi/ich9: Remove dead code related to 'acpi_memory_hotplug' git bisect good 2529ea2d561ea9fe359fb19ebdcfeb8b6cddd219 # good: [76e375fc3c538bd6e4232314f693b56536a50b73] docs/qapidoc: add QMP highlighting to annotated qmp-example blocks git bisect good 76e375fc3c538bd6e4232314f693b56536a50b73 # bad: [60d30cff8472c0bf05a40b0f55221fb4efb768e2] target/ppc: Move SPR indirect registers into PnvCore git bisect bad 60d30cff8472c0bf05a40b0f55221fb4efb768e2 # bad: [6c635326425091e164b563a7ce96408ef74ff2ec] vfio/{iommufd,container}: Remove caps::aw_bits git bisect bad 6c635326425091e164b563a7ce96408ef74ff2ec # good: [23fa74974d8c96bc95cbecc0d4e2d90f984939f6] Merge tag 'pull-target-arm-20240718' of https://git.linaro.org/people/pmaydell/qemu-arm into staging git bisect good 23fa74974d8c96bc95cbecc0d4e2d90f984939f6 # good: [c135d5eaafe7aa2533da663d8e5a34a424b71eb9] tests/tcg/aarch64: Fix test-mte.py git bisect good c135d5eaafe7aa2533da663d8e5a34a424b71eb9 # good: [6af69d02706c821797802cfd56acdac13a7c9422] Merge tag 'nvme-next-pull-request' of https://gitlab.com/birkelund/qemu into staging git bisect good 6af69d02706c821797802cfd56acdac13a7c9422 # bad: [71bce0e1fb1a866dde4a4b6016fc18b09f317338] Merge tag 'pull-tcg-20240723' of https://gitlab.com/rth7680/qemu into staging git bisect bad 71bce0e1fb1a866dde4a4b6016fc18b09f317338 # bad: [62fe57c6d23fe8136d281f0e37ec8a9fab08b60a] target/ppc: Split out helper_dbczl for 970 git bisect bad 62fe57c6d23fe8136d281f0e37ec8a9fab08b60a # good: [3b9991e35c08be7fd6b84090b2114ff1bfd44d3f] target/arm: Use set/clear_helper_retaddr in SVE and SME helpers git bisect good 3b9991e35c08be7fd6b84090b2114ff1bfd44d3f # good: [521a80d895ec8ef0200dcac9b9b19e60b0cc1d1a] target/ppc: Hoist dcbz_size out of dcbz_common git bisect good 521a80d895ec8ef0200dcac9b9b19e60b0cc1d1a # first bad commit: [62fe57c6d23fe8136d281f0e37ec8a9fab08b60a] target/ppc: Split out helper_dbczl for 970 --- Bug fixed: # pass: [72b0b80714066a435502b67cdb55a7868ba0487d] Merge tag 'for-upstream' of https://gitlab.com/bonzini/qemu into staging # fail: [fd1952d814da738ed107e05583b3e02ac11e88ff] Update version for v9.1.0 release git bisect start '--term-bad=pass' '--term-good=fail' 'HEAD' 'v9.1.0' # fail: [4ae7d11b70a840eec7aa27269093b15d04ebc84e] Merge tag 'pull-tcg-20240922' of https://gitlab.com/rth7680/qemu into staging git bisect fail 4ae7d11b70a840eec7aa27269093b15d04ebc84e # fail: [b5ab62b3c0050612c7f9b0b4baeb44ebab42775a] Merge tag 'for-upstream' of https://gitlab.com/bonzini/qemu into staging git bisect fail b5ab62b3c0050612c7f9b0b4baeb44ebab42775a # pass: [1bfb726112ea4fda07c988f08df32d1eebb9abec] ui/pixman: generalize shared_image_destroy git bisect pass 1bfb726112ea4fda07c988f08df32d1eebb9abec # fail: [7e3b6d8063f245d27eecce5aabe624b5785f2a77] Merge tag 'crypto-fixes-pull-request' of https://gitlab.com/berrange/qemu into staging git bisect fail 7e3b6d8063f245d27eecce5aabe624b5785f2a77 # pass: [e530581ee06573fcf48c7f7a6c3f8ec6e5809243] target/arm: Fix alignment fault priority in get_phys_addr_lpae git bisect pass e530581ee06573fcf48c7f7a6c3f8ec6e5809243 # pass: [795592fef7d5d66a67b95a7f45cc1a84883db6a8] accel/tcg: Use the alignment test in tlb_fill_align git bisect pass 795592fef7d5d66a67b95a7f45cc1a84883db6a8 # fail: [9d08a70ddc08e9b6ecf870fd232531c78fe0b208] tests/tcg: Run test-proc-mappings.py on i386 git bisect fail 9d08a70ddc08e9b6ecf870fd232531c78fe0b208 # fail: [da335fe12a5da71a33d7afc2075a341f26213f53] include/exec/memop: Move get_alignment_bits from tcg.h git bisect fail da335fe12a5da71a33d7afc2075a341f26213f53 # fail: [e5b063e81fd2b30aad1e9128238871c71b62a666] include/exec/memop: Introduce memop_atomicity_bits git bisect fail e5b063e81fd2b30aad1e9128238871c71b62a666 # pass: [f168808d7d100ed96c52c4438c4ddb557bd086bf] accel/tcg: Add TCGCPUOps.tlb_fill_align git bisect pass f168808d7d100ed96c52c4438c4ddb557bd086bf # first pass commit: [f168808d7d100ed96c52c4438c4ddb557bd086bf] accel/tcg: Add TCGCPUOps.tlb_fill_align
On Sun, 20 Oct 2024, Guenter Roeck wrote: > Hi, > > On Fri, Jul 19, 2024 at 11:07:01AM +1000, Richard Henderson wrote: >> We can determine at translation time whether the insn is or >> is not dbczl. We must retain a runtime check against the >> HID5 register, but we can move that to a separate function >> that never affects other ppc models. >> >> Reviewed-by: Nicholas Piggin <npiggin@gmail.com> >> Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu> >> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > > I see an odd failure when trying to boot ppc64 images in qemu v9.1.0 and > v9.1.1. > > Starting network: udhcpc: started, v1.36.1 > malloc(): corrupted top size > Aborted Do you have more info on what reproduces this? What is the QEMU command and what guest code is running? This looks like something may be using dcbzl on something else than a 970 or if this is emulating a 970 then I don't see why this stopped working. Looking at the reproducer may help. Regards, BALATON Zoltan > /usr/share/udhcpc/default.script: line 41: can't create : nonexistent directory > > I bisected the problem to this patch; bisect log is attached. > I also found that the problem has been fixed in mainline qemu. Bisect > points to commit: f168808d7d10 ("accel/tcg: Add TCGCPUOps.tlb_fill_align") > as the fix. I attached this bisect log as well. > > Reverting this patch isn't easy due to several follow-up commits. Applying > commit f168808d7d10 plus several preceding commits on top of v9.1.1 seems > to fix the problem. The commits I applied are > > da335fe12a5d include/exec/memop: Move get_alignment_bits from tcg.h > c5809eee452b include/exec/memop: Rename get_alignment_bits > e5b063e81fd2 include/exec/memop: Introduce memop_atomicity_bits > f168808d7d10 accel/tcg: Add TCGCPUOps.tlb_fill_align > > Obviously I have no idea if this is even remotely correct, so please take > this report as purely informational in case someone else observes a similar > problem. > > Thanks, > Guenter > > --- > Bug introduced: > > # bad: [fd1952d814da738ed107e05583b3e02ac11e88ff] Update version for v9.1.0 release > # good: [c25df57ae8f9fe1c72eee2dab37d76d904ac382e] Update version for 9.0.0 release > git bisect start 'v9.1.0' 'v9.0.0' > # good: [2529ea2d561ea9fe359fb19ebdcfeb8b6cddd219] hw/acpi/ich9: Remove dead code related to 'acpi_memory_hotplug' > git bisect good 2529ea2d561ea9fe359fb19ebdcfeb8b6cddd219 > # good: [76e375fc3c538bd6e4232314f693b56536a50b73] docs/qapidoc: add QMP highlighting to annotated qmp-example blocks > git bisect good 76e375fc3c538bd6e4232314f693b56536a50b73 > # bad: [60d30cff8472c0bf05a40b0f55221fb4efb768e2] target/ppc: Move SPR indirect registers into PnvCore > git bisect bad 60d30cff8472c0bf05a40b0f55221fb4efb768e2 > # bad: [6c635326425091e164b563a7ce96408ef74ff2ec] vfio/{iommufd,container}: Remove caps::aw_bits > git bisect bad 6c635326425091e164b563a7ce96408ef74ff2ec > # good: [23fa74974d8c96bc95cbecc0d4e2d90f984939f6] Merge tag 'pull-target-arm-20240718' of https://git.linaro.org/people/pmaydell/qemu-arm into staging > git bisect good 23fa74974d8c96bc95cbecc0d4e2d90f984939f6 > # good: [c135d5eaafe7aa2533da663d8e5a34a424b71eb9] tests/tcg/aarch64: Fix test-mte.py > git bisect good c135d5eaafe7aa2533da663d8e5a34a424b71eb9 > # good: [6af69d02706c821797802cfd56acdac13a7c9422] Merge tag 'nvme-next-pull-request' of https://gitlab.com/birkelund/qemu into staging > git bisect good 6af69d02706c821797802cfd56acdac13a7c9422 > # bad: [71bce0e1fb1a866dde4a4b6016fc18b09f317338] Merge tag 'pull-tcg-20240723' of https://gitlab.com/rth7680/qemu into staging > git bisect bad 71bce0e1fb1a866dde4a4b6016fc18b09f317338 > # bad: [62fe57c6d23fe8136d281f0e37ec8a9fab08b60a] target/ppc: Split out helper_dbczl for 970 > git bisect bad 62fe57c6d23fe8136d281f0e37ec8a9fab08b60a > # good: [3b9991e35c08be7fd6b84090b2114ff1bfd44d3f] target/arm: Use set/clear_helper_retaddr in SVE and SME helpers > git bisect good 3b9991e35c08be7fd6b84090b2114ff1bfd44d3f > # good: [521a80d895ec8ef0200dcac9b9b19e60b0cc1d1a] target/ppc: Hoist dcbz_size out of dcbz_common > git bisect good 521a80d895ec8ef0200dcac9b9b19e60b0cc1d1a > # first bad commit: [62fe57c6d23fe8136d281f0e37ec8a9fab08b60a] target/ppc: Split out helper_dbczl for 970 > > --- > Bug fixed: > > # pass: [72b0b80714066a435502b67cdb55a7868ba0487d] Merge tag 'for-upstream' of https://gitlab.com/bonzini/qemu into staging > # fail: [fd1952d814da738ed107e05583b3e02ac11e88ff] Update version for v9.1.0 release > git bisect start '--term-bad=pass' '--term-good=fail' 'HEAD' 'v9.1.0' > # fail: [4ae7d11b70a840eec7aa27269093b15d04ebc84e] Merge tag 'pull-tcg-20240922' of https://gitlab.com/rth7680/qemu into staging > git bisect fail 4ae7d11b70a840eec7aa27269093b15d04ebc84e > # fail: [b5ab62b3c0050612c7f9b0b4baeb44ebab42775a] Merge tag 'for-upstream' of https://gitlab.com/bonzini/qemu into staging > git bisect fail b5ab62b3c0050612c7f9b0b4baeb44ebab42775a > # pass: [1bfb726112ea4fda07c988f08df32d1eebb9abec] ui/pixman: generalize shared_image_destroy > git bisect pass 1bfb726112ea4fda07c988f08df32d1eebb9abec > # fail: [7e3b6d8063f245d27eecce5aabe624b5785f2a77] Merge tag 'crypto-fixes-pull-request' of https://gitlab.com/berrange/qemu into staging > git bisect fail 7e3b6d8063f245d27eecce5aabe624b5785f2a77 > # pass: [e530581ee06573fcf48c7f7a6c3f8ec6e5809243] target/arm: Fix alignment fault priority in get_phys_addr_lpae > git bisect pass e530581ee06573fcf48c7f7a6c3f8ec6e5809243 > # pass: [795592fef7d5d66a67b95a7f45cc1a84883db6a8] accel/tcg: Use the alignment test in tlb_fill_align > git bisect pass 795592fef7d5d66a67b95a7f45cc1a84883db6a8 > # fail: [9d08a70ddc08e9b6ecf870fd232531c78fe0b208] tests/tcg: Run test-proc-mappings.py on i386 > git bisect fail 9d08a70ddc08e9b6ecf870fd232531c78fe0b208 > # fail: [da335fe12a5da71a33d7afc2075a341f26213f53] include/exec/memop: Move get_alignment_bits from tcg.h > git bisect fail da335fe12a5da71a33d7afc2075a341f26213f53 > # fail: [e5b063e81fd2b30aad1e9128238871c71b62a666] include/exec/memop: Introduce memop_atomicity_bits > git bisect fail e5b063e81fd2b30aad1e9128238871c71b62a666 > # pass: [f168808d7d100ed96c52c4438c4ddb557bd086bf] accel/tcg: Add TCGCPUOps.tlb_fill_align > git bisect pass f168808d7d100ed96c52c4438c4ddb557bd086bf > # first pass commit: [f168808d7d100ed96c52c4438c4ddb557bd086bf] accel/tcg: Add TCGCPUOps.tlb_fill_align > >
diff --git a/target/ppc/helper.h b/target/ppc/helper.h index 76b8f25c77..afc56855ff 100644 --- a/target/ppc/helper.h +++ b/target/ppc/helper.h @@ -46,8 +46,11 @@ DEF_HELPER_FLAGS_3(stmw, TCG_CALL_NO_WG, void, env, tl, i32) DEF_HELPER_4(lsw, void, env, tl, i32, i32) DEF_HELPER_5(lswx, void, env, tl, i32, i32, i32) DEF_HELPER_FLAGS_4(stsw, TCG_CALL_NO_WG, void, env, tl, i32, i32) -DEF_HELPER_FLAGS_3(dcbz, TCG_CALL_NO_WG, void, env, tl, i32) -DEF_HELPER_FLAGS_3(dcbzep, TCG_CALL_NO_WG, void, env, tl, i32) +DEF_HELPER_FLAGS_2(dcbz, TCG_CALL_NO_WG, void, env, tl) +DEF_HELPER_FLAGS_2(dcbzep, TCG_CALL_NO_WG, void, env, tl) +#ifdef TARGET_PPC64 +DEF_HELPER_FLAGS_2(dcbzl, TCG_CALL_NO_WG, void, env, tl) +#endif DEF_HELPER_FLAGS_2(icbi, TCG_CALL_NO_WG, void, env, tl) DEF_HELPER_FLAGS_2(icbiep, TCG_CALL_NO_WG, void, env, tl) DEF_HELPER_5(lscbx, tl, env, tl, i32, i32, i32) diff --git a/target/ppc/mem_helper.c b/target/ppc/mem_helper.c index 5067919ff8..d4957efd6e 100644 --- a/target/ppc/mem_helper.c +++ b/target/ppc/mem_helper.c @@ -296,26 +296,34 @@ static void dcbz_common(CPUPPCState *env, target_ulong addr, } } -void helper_dcbz(CPUPPCState *env, target_ulong addr, uint32_t opcode) +void helper_dcbz(CPUPPCState *env, target_ulong addr) { - int dcbz_size = env->dcache_line_size; - -#if defined(TARGET_PPC64) - /* Check for dcbz vs dcbzl on 970 */ - if (env->excp_model == POWERPC_EXCP_970 && - !(opcode & 0x00200000) && ((env->spr[SPR_970_HID5] >> 7) & 0x3) == 1) { - dcbz_size = 32; - } -#endif - - dcbz_common(env, addr, dcbz_size, ppc_env_mmu_index(env, false), GETPC()); + dcbz_common(env, addr, env->dcache_line_size, + ppc_env_mmu_index(env, false), GETPC()); } -void helper_dcbzep(CPUPPCState *env, target_ulong addr, uint32_t opcode) +void helper_dcbzep(CPUPPCState *env, target_ulong addr) { dcbz_common(env, addr, env->dcache_line_size, PPC_TLB_EPID_STORE, GETPC()); } +#ifdef TARGET_PPC64 +void helper_dcbzl(CPUPPCState *env, target_ulong addr) +{ + int dcbz_size = env->dcache_line_size; + + /* + * The translator checked for POWERPC_EXCP_970. + * All that's left is to check HID5. + */ + if (((env->spr[SPR_970_HID5] >> 7) & 0x3) == 1) { + dcbz_size = 32; + } + + dcbz_common(env, addr, dcbz_size, ppc_env_mmu_index(env, false), GETPC()); +} +#endif + void helper_icbi(CPUPPCState *env, target_ulong addr) { addr &= ~(env->dcache_line_size - 1); diff --git a/target/ppc/translate.c b/target/ppc/translate.c index 0bc16d7251..9e472ab7ef 100644 --- a/target/ppc/translate.c +++ b/target/ppc/translate.c @@ -178,6 +178,7 @@ struct DisasContext { /* Translation flags */ MemOp default_tcg_memop_mask; #if defined(TARGET_PPC64) + powerpc_excp_t excp_model; bool sf_mode; bool has_cfar; bool has_bhrb; @@ -4445,27 +4446,29 @@ static void gen_dcblc(DisasContext *ctx) /* dcbz */ static void gen_dcbz(DisasContext *ctx) { - TCGv tcgv_addr; - TCGv_i32 tcgv_op; + TCGv tcgv_addr = tcg_temp_new(); gen_set_access_type(ctx, ACCESS_CACHE); - tcgv_addr = tcg_temp_new(); - tcgv_op = tcg_constant_i32(ctx->opcode & 0x03FF000); gen_addr_reg_index(ctx, tcgv_addr); - gen_helper_dcbz(tcg_env, tcgv_addr, tcgv_op); + +#ifdef TARGET_PPC64 + if (ctx->excp_model == POWERPC_EXCP_970 && !(ctx->opcode & 0x00200000)) { + gen_helper_dcbzl(tcg_env, tcgv_addr); + return; + } +#endif + + gen_helper_dcbz(tcg_env, tcgv_addr); } /* dcbzep */ static void gen_dcbzep(DisasContext *ctx) { - TCGv tcgv_addr; - TCGv_i32 tcgv_op; + TCGv tcgv_addr = tcg_temp_new(); gen_set_access_type(ctx, ACCESS_CACHE); - tcgv_addr = tcg_temp_new(); - tcgv_op = tcg_constant_i32(ctx->opcode & 0x03FF000); gen_addr_reg_index(ctx, tcgv_addr); - gen_helper_dcbzep(tcg_env, tcgv_addr, tcgv_op); + gen_helper_dcbzep(tcg_env, tcgv_addr); } /* dst / dstt */ @@ -6486,6 +6489,7 @@ static void ppc_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs) ctx->default_tcg_memop_mask = ctx->le_mode ? MO_LE : MO_BE; ctx->flags = env->flags; #if defined(TARGET_PPC64) + ctx->excp_model = env->excp_model; ctx->sf_mode = (hflags >> HFLAGS_64) & 1; ctx->has_cfar = !!(env->flags & POWERPC_FLAG_CFAR); ctx->has_bhrb = !!(env->flags & POWERPC_FLAG_BHRB);