From cb9fb64d0789a3ec47eb0d7549026e353e98b8c1 Mon Sep 17 00:00:00 2001 From: Mahesh Salgaonkar Date: Wed, 18 Mar 2020 13:04:20 +0530 Subject: [PATCH 1/7] ppc/spapr: Set the effective address provided flag in mc error log. Per PAPR, it is expected to set effective address provided flag in sub_err_type member of mc extended error log (i.e rtas_event_log_v6_mc.sub_err_type). This somehow got missed in original fwnmi-mce patch series. The current code just updates the effective address but does not set the flag to indicate that it is available. Hence guest fails to extract effective address from mce rtas log. This patch fixes that. Without this patch guest MCE logs fails print DAR value: [ 11.933608] Disabling lock debugging due to kernel taint [ 11.933773] MCE: CPU0: machine check (Severe) Host TLB Multihit [Recovered] [ 11.933979] MCE: CPU0: NIP: [c000000000090b34] radix__flush_tlb_range_psize+0x194/0xf00 [ 11.934223] MCE: CPU0: Initiator CPU [ 11.934341] MCE: CPU0: Unknown After the change: [ 22.454149] Disabling lock debugging due to kernel taint [ 22.454316] MCE: CPU0: machine check (Severe) Host TLB Multihit DAR: deadbeefdeadbeef [Recovered] [ 22.454605] MCE: CPU0: NIP: [c0000000003e5804] kmem_cache_alloc+0x84/0x330 [ 22.454820] MCE: CPU0: Initiator CPU [ 22.454944] MCE: CPU0: Unknown Signed-off-by: Mahesh Salgaonkar Message-Id: <158451653844.22972.17999316676230071087.stgit@jupiter> Reviewed-by: Nicholas Piggin Signed-off-by: David Gibson --- hw/ppc/spapr_events.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c index 323fcef4aa..a4a540f43d 100644 --- a/hw/ppc/spapr_events.c +++ b/hw/ppc/spapr_events.c @@ -243,6 +243,14 @@ struct rtas_event_log_v6_mc { #define RTAS_LOG_V6_MC_TLB_PARITY 1 #define RTAS_LOG_V6_MC_TLB_MULTIHIT 2 #define RTAS_LOG_V6_MC_TLB_INDETERMINATE 3 +/* + * Per PAPR, + * For UE error type, set bit 1 of sub_err_type to indicate effective addr is + * provided. For other error types (SLB/ERAT/TLB), set bit 0 to indicate + * same. + */ +#define RTAS_LOG_V6_MC_UE_EA_ADDR_PROVIDED 0x40 +#define RTAS_LOG_V6_MC_EA_ADDR_PROVIDED 0x80 uint8_t reserved_1[6]; uint64_t effective_address; uint64_t logical_address; @@ -726,6 +734,22 @@ void spapr_hotplug_req_remove_by_count_indexed(SpaprDrcType drc_type, RTAS_LOG_V6_HP_ACTION_REMOVE, drc_type, &drc_id); } +static void spapr_mc_set_ea_provided_flag(struct mc_extended_log *ext_elog) +{ + switch (ext_elog->mc.error_type) { + case RTAS_LOG_V6_MC_TYPE_UE: + ext_elog->mc.sub_err_type |= RTAS_LOG_V6_MC_UE_EA_ADDR_PROVIDED; + break; + case RTAS_LOG_V6_MC_TYPE_SLB: + case RTAS_LOG_V6_MC_TYPE_ERAT: + case RTAS_LOG_V6_MC_TYPE_TLB: + ext_elog->mc.sub_err_type |= RTAS_LOG_V6_MC_EA_ADDR_PROVIDED; + break; + default: + break; + } +} + static uint32_t spapr_mce_get_elog_type(PowerPCCPU *cpu, bool recovered, struct mc_extended_log *ext_elog) { @@ -751,6 +775,7 @@ static uint32_t spapr_mce_get_elog_type(PowerPCCPU *cpu, bool recovered, ext_elog->mc.sub_err_type = mc_derror_table[i].error_subtype; if (mc_derror_table[i].dar_valid) { ext_elog->mc.effective_address = cpu_to_be64(env->spr[SPR_DAR]); + spapr_mc_set_ea_provided_flag(ext_elog); } summary |= mc_derror_table[i].initiator @@ -769,6 +794,7 @@ static uint32_t spapr_mce_get_elog_type(PowerPCCPU *cpu, bool recovered, ext_elog->mc.sub_err_type = mc_ierror_table[i].error_subtype; if (mc_ierror_table[i].nip_valid) { ext_elog->mc.effective_address = cpu_to_be64(env->nip); + spapr_mc_set_ea_provided_flag(ext_elog); } summary |= mc_ierror_table[i].initiator From f9e3e1a35e8fd63d61fae58bd98d24d7defa9316 Mon Sep 17 00:00:00 2001 From: Nicholas Piggin Date: Wed, 18 Mar 2020 14:41:34 +1000 Subject: [PATCH 2/7] target/ppc: Fix slbia TLB invalidation gap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit slbia must invalidate TLBs even if it does not remove a valid SLB entry, because slbmte can overwrite valid entries without removing their TLBs. As the architecture says, slbia invalidates all lookaside information, not conditionally based on if it removed valid entries. It does not seem possible for POWER8 or earlier Linux kernels to hit this bug because it never changes its kernel SLB translations, and it should always have valid entries if any accesses are made to userspace regions. However other operating systems which may modify SLB entry 0 or do more fancy things with segments might be affected. When POWER9 slbia support is added in the next patch, this becomes a real problem because some new slbia variants don't invalidate all non-zero entries. Signed-off-by: Nicholas Piggin Message-Id: <20200318044135.851716-1-npiggin@gmail.com> Reviewed-by: Cédric Le Goater Reviewed-by: Greg Kurz Signed-off-by: David Gibson --- target/ppc/mmu-hash64.c | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c index 34f6009b1e..373d44de74 100644 --- a/target/ppc/mmu-hash64.c +++ b/target/ppc/mmu-hash64.c @@ -100,20 +100,29 @@ void helper_slbia(CPUPPCState *env) PowerPCCPU *cpu = env_archcpu(env); int n; + /* + * slbia must always flush all TLB (which is equivalent to ERAT in ppc + * architecture). Matching on SLB_ESID_V is not good enough, because slbmte + * can overwrite a valid SLB without flushing its lookaside information. + * + * It would be possible to keep the TLB in synch with the SLB by flushing + * when a valid entry is overwritten by slbmte, and therefore slbia would + * not have to flush unless it evicts a valid SLB entry. However it is + * expected that slbmte is more common than slbia, and slbia is usually + * going to evict valid SLB entries, so that tradeoff is unlikely to be a + * good one. + */ + /* XXX: Warning: slbia never invalidates the first segment */ for (n = 1; n < cpu->hash64_opts->slb_size; n++) { ppc_slb_t *slb = &env->slb[n]; if (slb->esid & SLB_ESID_V) { slb->esid &= ~SLB_ESID_V; - /* - * XXX: given the fact that segment size is 256 MB or 1TB, - * and we still don't have a tlb_flush_mask(env, n, mask) - * in QEMU, we just invalidate all TLBs - */ - env->tlb_need_flush |= TLB_NEED_LOCAL_FLUSH; } } + + env->tlb_need_flush |= TLB_NEED_LOCAL_FLUSH; } static void __helper_slbie(CPUPPCState *env, target_ulong addr, From 0418bf78fe8e61e619782940e77ca2d18b8c2d35 Mon Sep 17 00:00:00 2001 From: Nicholas Piggin Date: Thu, 19 Mar 2020 16:44:39 +1000 Subject: [PATCH 3/7] target/ppc: Fix ISA v3.0 (POWER9) slbia implementation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The new ISA v3.0 slbia variants have not been implemented for TCG, which can lead to crashing when a POWER9 machine boots Linux using the hash MMU, for example ("disable_radix" kernel command line). Add them. Signed-off-by: Nicholas Piggin Message-Id: <20200319064439.1020571-1-npiggin@gmail.com> Reviewed-by: Cédric Le Goater [dwg: Fixed compile error for USER_ONLY builds] Signed-off-by: David Gibson --- target/ppc/helper.h | 2 +- target/ppc/mmu-hash64.c | 56 +++++++++++++++++++++++++++++++++++------ target/ppc/translate.c | 5 +++- 3 files changed, 54 insertions(+), 9 deletions(-) diff --git a/target/ppc/helper.h b/target/ppc/helper.h index cfb4c07085..a95c010391 100644 --- a/target/ppc/helper.h +++ b/target/ppc/helper.h @@ -614,7 +614,7 @@ DEF_HELPER_FLAGS_3(store_slb, TCG_CALL_NO_RWG, void, env, tl, tl) DEF_HELPER_2(load_slb_esid, tl, env, tl) DEF_HELPER_2(load_slb_vsid, tl, env, tl) DEF_HELPER_2(find_slb_vsid, tl, env, tl) -DEF_HELPER_FLAGS_1(slbia, TCG_CALL_NO_RWG, void, env) +DEF_HELPER_FLAGS_2(slbia, TCG_CALL_NO_RWG, void, env, i32) DEF_HELPER_FLAGS_2(slbie, TCG_CALL_NO_RWG, void, env, tl) DEF_HELPER_FLAGS_2(slbieg, TCG_CALL_NO_RWG, void, env, tl) #endif diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c index 373d44de74..e5baabf0e1 100644 --- a/target/ppc/mmu-hash64.c +++ b/target/ppc/mmu-hash64.c @@ -95,9 +95,10 @@ void dump_slb(PowerPCCPU *cpu) } } -void helper_slbia(CPUPPCState *env) +void helper_slbia(CPUPPCState *env, uint32_t ih) { PowerPCCPU *cpu = env_archcpu(env); + int starting_entry; int n; /* @@ -111,18 +112,59 @@ void helper_slbia(CPUPPCState *env) * expected that slbmte is more common than slbia, and slbia is usually * going to evict valid SLB entries, so that tradeoff is unlikely to be a * good one. + * + * ISA v2.05 introduced IH field with values 0,1,2,6. These all invalidate + * the same SLB entries (everything but entry 0), but differ in what + * "lookaside information" is invalidated. TCG can ignore this and flush + * everything. + * + * ISA v3.0 introduced additional values 3,4,7, which change what SLBs are + * invalidated. */ - /* XXX: Warning: slbia never invalidates the first segment */ - for (n = 1; n < cpu->hash64_opts->slb_size; n++) { - ppc_slb_t *slb = &env->slb[n]; + env->tlb_need_flush |= TLB_NEED_LOCAL_FLUSH; - if (slb->esid & SLB_ESID_V) { - slb->esid &= ~SLB_ESID_V; + starting_entry = 1; /* default for IH=0,1,2,6 */ + + if (env->mmu_model == POWERPC_MMU_3_00) { + switch (ih) { + case 0x7: + /* invalidate no SLBs, but all lookaside information */ + return; + + case 0x3: + case 0x4: + /* also considers SLB entry 0 */ + starting_entry = 0; + break; + + case 0x5: + /* treat undefined values as ih==0, and warn */ + qemu_log_mask(LOG_GUEST_ERROR, + "slbia undefined IH field %u.\n", ih); + break; + + default: + /* 0,1,2,6 */ + break; } } - env->tlb_need_flush |= TLB_NEED_LOCAL_FLUSH; + for (n = starting_entry; n < cpu->hash64_opts->slb_size; n++) { + ppc_slb_t *slb = &env->slb[n]; + + if (!(slb->esid & SLB_ESID_V)) { + continue; + } + if (env->mmu_model == POWERPC_MMU_3_00) { + if (ih == 0x3 && (slb->vsid & SLB_VSID_C) == 0) { + /* preserves entries with a class value of 0 */ + continue; + } + } + + slb->esid &= ~SLB_ESID_V; + } } static void __helper_slbie(CPUPPCState *env, target_ulong addr, diff --git a/target/ppc/translate.c b/target/ppc/translate.c index 127c82a24e..b207fb5386 100644 --- a/target/ppc/translate.c +++ b/target/ppc/translate.c @@ -4997,9 +4997,12 @@ static void gen_slbia(DisasContext *ctx) #if defined(CONFIG_USER_ONLY) GEN_PRIV; #else + uint32_t ih = (ctx->opcode >> 21) & 0x7; + TCGv_i32 t0 = tcg_const_i32(ih); + CHK_SV; - gen_helper_slbia(cpu_env); + gen_helper_slbia(cpu_env, t0); #endif /* defined(CONFIG_USER_ONLY) */ } From feb39b62288cb780561ca83b84ca82dceb3cedb9 Mon Sep 17 00:00:00 2001 From: Vincent Fazio Date: Thu, 19 Mar 2020 08:32:44 -0500 Subject: [PATCH 4/7] target/ppc: don't byte swap ELFv2 signal handler Previously, the signal handler would be byte swapped if the target and host CPU used different endianness. This would cause a SIGSEGV when attempting to translate the opcode pointed to by the swapped address. Thread 1 "qemu-ppc64" received signal SIGSEGV, Segmentation fault. 0x00000000600a9257 in ldl_he_p (ptr=0x4c2c061000000000) at qemu/include/qemu/bswap.h:351 351 __builtin_memcpy(&r, ptr, sizeof(r)); #0 0x00000000600a9257 in ldl_he_p (ptr=0x4c2c061000000000) at qemu/include/qemu/bswap.h:351 #1 0x00000000600a92fe in ldl_be_p (ptr=0x4c2c061000000000) at qemu/include/qemu/bswap.h:449 #2 0x00000000600c0790 in translator_ldl_swap at qemu/include/exec/translator.h:201 #3 0x000000006011c1ab in ppc_tr_translate_insn at qemu/target/ppc/translate.c:7856 #4 0x000000006005ae70 in translator_loop at qemu/accel/tcg/translator.c:102 The signal handler will be byte swapped as a result of the __get_user() call in sigaction() if it is necessary, no additional swap is required. Signed-off-by: Vincent Fazio Reviewed-by: Laurent Vivier Reviewed-by: Richard Henderson Message-Id: <20200319133244.8818-1-vfazio@xes-inc.com> Signed-off-by: David Gibson --- linux-user/ppc/signal.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/linux-user/ppc/signal.c b/linux-user/ppc/signal.c index 0c4e7ba54c..ecd99736b7 100644 --- a/linux-user/ppc/signal.c +++ b/linux-user/ppc/signal.c @@ -567,10 +567,8 @@ void setup_rt_frame(int sig, struct target_sigaction *ka, env->nip = tswapl(handler->entry); env->gpr[2] = tswapl(handler->toc); } else { - /* ELFv2 PPC64 function pointers are entry points, but R12 - * must also be set */ - env->nip = tswapl((target_ulong) ka->_sa_handler); - env->gpr[12] = env->nip; + /* ELFv2 PPC64 function pointers are entry points. R12 must also be set. */ + env->gpr[12] = env->nip = ka->_sa_handler; } #else env->nip = (target_ulong) ka->_sa_handler; From ce05fa0fcc9e23d16b2ff079cb3cb6aceaccbc28 Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Sat, 21 Mar 2020 18:34:22 +0100 Subject: [PATCH 5/7] spapr: Fix memory leak in h_client_architecture_support() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is the only error path that needs to free the previously allocated ov1. Reported-by: Coverity (CID 1421924) Fixes: cbd0d7f36322 "spapr: Fail CAS if option vector table cannot be parsed" Signed-off-by: Greg Kurz Message-Id: <158481206205.336182.16106097429336044843.stgit@bahia.lan> Signed-off-by: David Gibson Reviewed-by: Philippe Mathieu-Daudé --- hw/ppc/spapr_hcall.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c index 40c86e91eb..0d50fc9117 100644 --- a/hw/ppc/spapr_hcall.c +++ b/hw/ppc/spapr_hcall.c @@ -1726,6 +1726,7 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu, } ov5_guest = spapr_ovec_parse_vector(ov_table, 5); if (!ov5_guest) { + spapr_ovec_cleanup(ov1_guest); warn_report("guest didn't provide option vector 5"); return H_PARAMETER; } From 235352ee6e73d7716d20e706d484fd45c232ec09 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Sun, 22 Mar 2020 19:22:58 +0000 Subject: [PATCH 6/7] hw/ppc: Take QEMU lock when calling ppc_dcr_read/write() The ppc_dcr_read() and ppc_dcr_write() functions call into callbacks in device code, so we need to hold the QEMU iothread lock while calling them. This is the case already for the callsites in kvmppc_handle_dcr_read/write(), but we must also take the lock when calling the helpers from TCG. This fixes a bug where attempting to initialise the PPC405EP SDRAM will cause an assertion when sdram_map_bcr() attempts to remap memory regions. Reported-by: Amit Lazar Signed-off-by: Peter Maydell Message-Id: <20200322192258.14039-1-peter.maydell@linaro.org> Signed-off-by: David Gibson --- target/ppc/timebase_helper.c | 40 +++++++++++++++++++++++------------- 1 file changed, 26 insertions(+), 14 deletions(-) diff --git a/target/ppc/timebase_helper.c b/target/ppc/timebase_helper.c index 703bd9ed18..d16360ab66 100644 --- a/target/ppc/timebase_helper.c +++ b/target/ppc/timebase_helper.c @@ -21,6 +21,7 @@ #include "exec/helper-proto.h" #include "exec/exec-all.h" #include "qemu/log.h" +#include "qemu/main-loop.h" /*****************************************************************************/ /* SPR accesses */ @@ -167,13 +168,19 @@ target_ulong helper_load_dcr(CPUPPCState *env, target_ulong dcrn) raise_exception_err_ra(env, POWERPC_EXCP_PROGRAM, POWERPC_EXCP_INVAL | POWERPC_EXCP_INVAL_INVAL, GETPC()); - } else if (unlikely(ppc_dcr_read(env->dcr_env, - (uint32_t)dcrn, &val) != 0)) { - qemu_log_mask(LOG_GUEST_ERROR, "DCR read error %d %03x\n", - (uint32_t)dcrn, (uint32_t)dcrn); - raise_exception_err_ra(env, POWERPC_EXCP_PROGRAM, - POWERPC_EXCP_INVAL | - POWERPC_EXCP_PRIV_REG, GETPC()); + } else { + int ret; + + qemu_mutex_lock_iothread(); + ret = ppc_dcr_read(env->dcr_env, (uint32_t)dcrn, &val); + qemu_mutex_unlock_iothread(); + if (unlikely(ret != 0)) { + qemu_log_mask(LOG_GUEST_ERROR, "DCR read error %d %03x\n", + (uint32_t)dcrn, (uint32_t)dcrn); + raise_exception_err_ra(env, POWERPC_EXCP_PROGRAM, + POWERPC_EXCP_INVAL | + POWERPC_EXCP_PRIV_REG, GETPC()); + } } return val; } @@ -185,12 +192,17 @@ void helper_store_dcr(CPUPPCState *env, target_ulong dcrn, target_ulong val) raise_exception_err_ra(env, POWERPC_EXCP_PROGRAM, POWERPC_EXCP_INVAL | POWERPC_EXCP_INVAL_INVAL, GETPC()); - } else if (unlikely(ppc_dcr_write(env->dcr_env, (uint32_t)dcrn, - (uint32_t)val) != 0)) { - qemu_log_mask(LOG_GUEST_ERROR, "DCR write error %d %03x\n", - (uint32_t)dcrn, (uint32_t)dcrn); - raise_exception_err_ra(env, POWERPC_EXCP_PROGRAM, - POWERPC_EXCP_INVAL | - POWERPC_EXCP_PRIV_REG, GETPC()); + } else { + int ret; + qemu_mutex_lock_iothread(); + ret = ppc_dcr_write(env->dcr_env, (uint32_t)dcrn, (uint32_t)val); + qemu_mutex_unlock_iothread(); + if (unlikely(ret != 0)) { + qemu_log_mask(LOG_GUEST_ERROR, "DCR write error %d %03x\n", + (uint32_t)dcrn, (uint32_t)dcrn); + raise_exception_err_ra(env, POWERPC_EXCP_PROGRAM, + POWERPC_EXCP_INVAL | + POWERPC_EXCP_PRIV_REG, GETPC()); + } } } From 1583794b9b36911df116cc726750dadbeeac506a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Fri, 20 Mar 2020 16:57:40 +0100 Subject: [PATCH 7/7] ppc/ppc405_boards: Remove unnecessary NULL check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This code is inside the "if (dinfo)" condition, so testing again here whether it is NULL is unnecessary. Fixes: dd59bcae7 (Don't size flash memory to match backing image) Reported-by: Coverity (CID 1421917) Suggested-by: Peter Maydell Signed-off-by: Philippe Mathieu-Daudé Message-Id: <20200320155740.5342-1-philmd@redhat.com> Reviewed-by: Markus Armbruster Signed-off-by: David Gibson --- hw/ppc/ppc405_boards.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c index e6bffb9e1a..6198ec1035 100644 --- a/hw/ppc/ppc405_boards.c +++ b/hw/ppc/ppc405_boards.c @@ -191,7 +191,7 @@ static void ref405ep_init(MachineState *machine) bios_size = 8 * MiB; pflash_cfi02_register((uint32_t)(-bios_size), "ef405ep.bios", bios_size, - dinfo ? blk_by_legacy_dinfo(dinfo) : NULL, + blk_by_legacy_dinfo(dinfo), 64 * KiB, 1, 2, 0x0001, 0x22DA, 0x0000, 0x0000, 0x555, 0x2AA, 1); @@ -459,7 +459,7 @@ static void taihu_405ep_init(MachineState *machine) bios_size = 2 * MiB; pflash_cfi02_register(0xFFE00000, "taihu_405ep.bios", bios_size, - dinfo ? blk_by_legacy_dinfo(dinfo) : NULL, + blk_by_legacy_dinfo(dinfo), 64 * KiB, 1, 4, 0x0001, 0x22DA, 0x0000, 0x0000, 0x555, 0x2AA, 1); @@ -494,7 +494,7 @@ static void taihu_405ep_init(MachineState *machine) if (dinfo) { bios_size = 32 * MiB; pflash_cfi02_register(0xfc000000, "taihu_405ep.flash", bios_size, - dinfo ? blk_by_legacy_dinfo(dinfo) : NULL, + blk_by_legacy_dinfo(dinfo), 64 * KiB, 1, 4, 0x0001, 0x22DA, 0x0000, 0x0000, 0x555, 0x2AA, 1);