From d7d9c6071e6dc5d466b229457fc4ad34e101dccd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Tue, 19 Mar 2024 06:10:20 +0100 Subject: target/ppc/mmu-radix64: Use correct string format in walk_tree() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 'mask', 'nlb' and 'base_addr' are all uin64_t types. Use the corresponding PRIx64 format. Fixes: d2066bc50d ("target/ppc: Check page dir/table base alignment") Signed-off-by: Philippe Mathieu-Daudé Signed-off-by: Nicholas Piggin --- target/ppc/mmu-radix64.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'target') diff --git a/target/ppc/mmu-radix64.c b/target/ppc/mmu-radix64.c index 5823e039e6..690dff7a49 100644 --- a/target/ppc/mmu-radix64.c +++ b/target/ppc/mmu-radix64.c @@ -300,8 +300,8 @@ static int ppc_radix64_next_level(AddressSpace *as, vaddr eaddr, if (nlb & mask) { qemu_log_mask(LOG_GUEST_ERROR, - "%s: misaligned page dir/table base: 0x"TARGET_FMT_lx - " page dir size: 0x"TARGET_FMT_lx"\n", + "%s: misaligned page dir/table base: 0x%" PRIx64 + " page dir size: 0x%" PRIx64 "\n", __func__, nlb, mask + 1); nlb &= ~mask; } @@ -324,8 +324,8 @@ static int ppc_radix64_walk_tree(AddressSpace *as, vaddr eaddr, if (base_addr & mask) { qemu_log_mask(LOG_GUEST_ERROR, - "%s: misaligned page dir base: 0x"TARGET_FMT_lx - " page dir size: 0x"TARGET_FMT_lx"\n", + "%s: misaligned page dir base: 0x%" PRIx64 + " page dir size: 0x%" PRIx64 "\n", __func__, base_addr, mask + 1); base_addr &= ~mask; } -- cgit v1.2.3 From 978897a572e975faad912a473815a668a43d9f1f Mon Sep 17 00:00:00 2001 From: Benjamin Gray Date: Wed, 20 Mar 2024 12:50:24 +1100 Subject: target/ppc: Restore [H]DEXCR to 64-bits The DEXCR emulation was recently changed to a 32-bit register, possibly because it does have a 32-bit read-only view. It is a full 64-bit SPR though, so use the corresponding 64-bit write functions. Fixes: fbda88f7abdee ("target/ppc: Fix width of some 32-bit SPRs") Reviewed-by: Nicholas Piggin Signed-off-by: Benjamin Gray Signed-off-by: Nicholas Piggin --- target/ppc/cpu_init.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'target') diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c index 7e65f08147..22fdea093b 100644 --- a/target/ppc/cpu_init.c +++ b/target/ppc/cpu_init.c @@ -5820,7 +5820,7 @@ static void register_power10_dexcr_sprs(CPUPPCState *env) { spr_register(env, SPR_DEXCR, "DEXCR", SPR_NOACCESS, SPR_NOACCESS, - &spr_read_generic, &spr_write_generic32, + &spr_read_generic, &spr_write_generic, 0); spr_register(env, SPR_UDEXCR, "UDEXCR", @@ -5831,7 +5831,7 @@ static void register_power10_dexcr_sprs(CPUPPCState *env) spr_register_hv(env, SPR_HDEXCR, "HDEXCR", SPR_NOACCESS, SPR_NOACCESS, SPR_NOACCESS, SPR_NOACCESS, - &spr_read_generic, &spr_write_generic32, + &spr_read_generic, &spr_write_generic, 0); spr_register(env, SPR_UHDEXCR, "UHDEXCR", -- cgit v1.2.3 From ed399ade3c85adf82fe507339560693e83a27020 Mon Sep 17 00:00:00 2001 From: Benjamin Gray Date: Wed, 20 Mar 2024 12:50:25 +1100 Subject: target/ppc: Fix GDB register indexing on secondary CPUs The GDB server protocol assigns an arbitrary numbering of the SPRs. We track this correspondence on each SPR with gdb_id, using it to resolve any SPR requests GDB makes. Early on we generate an XML representation of the SPRs to give GDB, including this numbering. However the XML is cached globally, and we skip setting the SPR gdb_id values on subsequent threads if we detect it is cached. This causes QEMU to fail to resolve SPR requests against secondary CPUs because it cannot find the matching gdb_id value on that thread's SPRs. This is a minimal fix to first assign the gdb_id values, then return early if the XML is cached. Otherwise we generate the XML using the now already initialised gdb_id values. Fixes: 1b53948ff8f7 ("target/ppc: Use GDBFeature for dynamic XML") Reviewed-by: Nicholas Piggin Signed-off-by: Benjamin Gray Signed-off-by: Nicholas Piggin --- target/ppc/gdbstub.c | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-) (limited to 'target') diff --git a/target/ppc/gdbstub.c b/target/ppc/gdbstub.c index 3f1e61bdb7..3b28d4e21c 100644 --- a/target/ppc/gdbstub.c +++ b/target/ppc/gdbstub.c @@ -305,14 +305,6 @@ static void gdb_gen_spr_feature(CPUState *cs) unsigned int num_regs = 0; int i; - if (pcc->gdb_spr.xml) { - return; - } - - gdb_feature_builder_init(&builder, &pcc->gdb_spr, - "org.qemu.power.spr", "power-spr.xml", - cs->gdb_num_regs); - for (i = 0; i < ARRAY_SIZE(env->spr_cb); i++) { ppc_spr_t *spr = &env->spr_cb[i]; @@ -320,9 +312,6 @@ static void gdb_gen_spr_feature(CPUState *cs) continue; } - gdb_feature_builder_append_reg(&builder, g_ascii_strdown(spr->name, -1), - TARGET_LONG_BITS, num_regs, - "int", "spr"); /* * GDB identifies registers based on the order they are * presented in the XML. These ids will not match QEMU's @@ -335,6 +324,26 @@ static void gdb_gen_spr_feature(CPUState *cs) num_regs++; } + if (pcc->gdb_spr.xml) { + return; + } + + gdb_feature_builder_init(&builder, &pcc->gdb_spr, + "org.qemu.power.spr", "power-spr.xml", + cs->gdb_num_regs); + + for (i = 0; i < ARRAY_SIZE(env->spr_cb); i++) { + ppc_spr_t *spr = &env->spr_cb[i]; + + if (!spr->name) { + continue; + } + + gdb_feature_builder_append_reg(&builder, g_ascii_strdown(spr->name, -1), + TARGET_LONG_BITS, spr->gdb_id, + "int", "spr"); + } + gdb_feature_builder_end(&builder); } #endif -- cgit v1.2.3 From 434531619fecd5ff9a08e548ed87b2b73c29cf3e Mon Sep 17 00:00:00 2001 From: Nicholas Piggin Date: Tue, 19 Mar 2024 16:01:46 +1000 Subject: target/ppc: Do not clear MSR[ME] on MCE interrupts to supervisor Hardware clears the MSR[ME] bit when delivering a machine check interrupt, so that is what QEMU does. The spapr environment runs in supervisor mode though, and receives machine check interrupts after they are processed by the hypervisor, and MSR[ME] must always be enabled in supervisor mode (otherwise it could checkstop the system). So MSR[ME] must not be cleared when delivering machine checks to the supervisor. The fix to prevent supervisor mode from modifying MSR[ME] also prevented it from re-enabling the incorrectly cleared MSR[ME] bit when returning from handling the interrupt. Before that fix, the problem was not very noticable with well-behaved code. So the Fixes tag is not strictly correct, but practically they go together. Found by kvm-unit-tests machine check tests (not yet upstream). Fixes: 678b6f1af75ef ("target/ppc: Prevent supervisor from modifying MSR[ME]") Reviewed-by: Harsh Prateek Bora Signed-off-by: Nicholas Piggin --- target/ppc/excp_helper.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'target') diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c index 80f584f933..674c05a2ce 100644 --- a/target/ppc/excp_helper.c +++ b/target/ppc/excp_helper.c @@ -1345,9 +1345,10 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp) * clear (e.g., see FWNMI in PAPR). */ new_msr |= (target_ulong)MSR_HVB; + + /* HV machine check exceptions don't have ME set */ + new_msr &= ~((target_ulong)1 << MSR_ME); } - /* machine check exceptions don't have ME set */ - new_msr &= ~((target_ulong)1 << MSR_ME); msr |= env->error_code; break; -- cgit v1.2.3