From cce84fc9193e2566b4f7f9de5a13e7ed360d44d9 Mon Sep 17 00:00:00 2001 From: Frederic Barrat <fbarrat@linux.ibm.com> Date: Thu, 1 Jun 2023 14:13:27 +0200 Subject: [PATCH 01/29] pnv/xive2: Add definition for TCTXT Config register MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add basic read/write support for the TCTXT Config register on P10. qemu doesn't do anything with it yet, but it avoids logging a guest error when skiboot configures the fused-core state: qemu-system-ppc64 -machine powernv10 ... -d guest_errors ... [ 0.131670000,5] XIVE: [ IC 00 ] Initializing XIVE block ID 0... XIVE[0] - TCTXT: invalid read @140 XIVE[0] - TCTXT: invalid write @140 Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com> Reviewed-by: Cédric Le Goater <clg@kaod.org> Message-Id: <20230601121331.487207-2-fbarrat@linux.ibm.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- hw/intc/pnv_xive2.c | 8 +++++++- hw/intc/pnv_xive2_regs.h | 4 ++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/hw/intc/pnv_xive2.c b/hw/intc/pnv_xive2.c index 7176d70234..889e409929 100644 --- a/hw/intc/pnv_xive2.c +++ b/hw/intc/pnv_xive2.c @@ -1265,6 +1265,9 @@ static uint64_t pnv_xive2_ic_tctxt_read(void *opaque, hwaddr offset, case TCTXT_EN1_RESET: val = xive->tctxt_regs[TCTXT_EN1 >> 3]; break; + case TCTXT_CFG: + val = xive->tctxt_regs[reg]; + break; default: xive2_error(xive, "TCTXT: invalid read @%"HWADDR_PRIx, offset); } @@ -1276,6 +1279,7 @@ static void pnv_xive2_ic_tctxt_write(void *opaque, hwaddr offset, uint64_t val, unsigned size) { PnvXive2 *xive = PNV_XIVE2(opaque); + uint32_t reg = offset >> 3; switch (offset) { /* @@ -1297,7 +1301,9 @@ static void pnv_xive2_ic_tctxt_write(void *opaque, hwaddr offset, case TCTXT_EN1_RESET: xive->tctxt_regs[TCTXT_EN1 >> 3] &= ~val; break; - + case TCTXT_CFG: + xive->tctxt_regs[reg] = val; + break; default: xive2_error(xive, "TCTXT: invalid write @%"HWADDR_PRIx, offset); return; diff --git a/hw/intc/pnv_xive2_regs.h b/hw/intc/pnv_xive2_regs.h index 0c096e4adb..8f1e0a1fde 100644 --- a/hw/intc/pnv_xive2_regs.h +++ b/hw/intc/pnv_xive2_regs.h @@ -405,6 +405,10 @@ #define X_TCTXT_EN1_RESET 0x307 #define TCTXT_EN1_RESET 0x038 +/* TCTXT Config register */ +#define X_TCTXT_CFG 0x328 +#define TCTXT_CFG 0x140 + /* * VSD Tables */ From 32af01f83a763ccbba39c1cbc424e1b724d233df Mon Sep 17 00:00:00 2001 From: Frederic Barrat <fbarrat@linux.ibm.com> Date: Thu, 1 Jun 2023 14:13:28 +0200 Subject: [PATCH 02/29] pnv/xive2: Add definition for the ESB cache configuration register MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add basic read/write support for the ESB cache configuration register on P10. We don't model the ESB cache in qemu so reading/writing the register won't do anything, but it avoids logging a guest error when skiboot configures it: qemu-system-ppc64 -machine powernv10 ... -d guest_errors ... XIVE[0] - VC: invalid read @240 XIVE[0] - VC: invalid write @240 Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com> Reviewed-by: Cédric Le Goater <clg@kaod.org> Message-Id: <20230601121331.487207-3-fbarrat@linux.ibm.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- hw/intc/pnv_xive2.c | 7 +++++++ hw/intc/pnv_xive2_regs.h | 4 ++++ 2 files changed, 11 insertions(+) diff --git a/hw/intc/pnv_xive2.c b/hw/intc/pnv_xive2.c index 889e409929..a75ff270ac 100644 --- a/hw/intc/pnv_xive2.c +++ b/hw/intc/pnv_xive2.c @@ -955,6 +955,10 @@ static uint64_t pnv_xive2_ic_vc_read(void *opaque, hwaddr offset, val = xive->vc_regs[reg]; break; + case VC_ESBC_CFG: + val = xive->vc_regs[reg]; + break; + /* * EAS cache updates (not modeled) */ @@ -1046,6 +1050,9 @@ static void pnv_xive2_ic_vc_write(void *opaque, hwaddr offset, /* ESB update */ break; + case VC_ESBC_CFG: + break; + /* * EAS cache updates (not modeled) */ diff --git a/hw/intc/pnv_xive2_regs.h b/hw/intc/pnv_xive2_regs.h index 8f1e0a1fde..7165dc8704 100644 --- a/hw/intc/pnv_xive2_regs.h +++ b/hw/intc/pnv_xive2_regs.h @@ -232,6 +232,10 @@ #define VC_ESBC_FLUSH_POLL_BLOCK_ID_MASK PPC_BITMASK(32, 35) #define VC_ESBC_FLUSH_POLL_OFFSET_MASK PPC_BITMASK(36, 63) /* 28-bit */ +/* ESBC configuration */ +#define X_VC_ESBC_CFG 0x148 +#define VC_ESBC_CFG 0x240 + /* EASC flush control register */ #define X_VC_EASC_FLUSH_CTRL 0x160 #define VC_EASC_FLUSH_CTRL 0x300 From f0fc1c29a8163ce383d3bcb3aac0964747d2d8b1 Mon Sep 17 00:00:00 2001 From: Frederic Barrat <fbarrat@linux.ibm.com> Date: Thu, 1 Jun 2023 14:13:29 +0200 Subject: [PATCH 03/29] pnv/xive2: Allow writes to the Physical Thread Enable registers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix what was probably a silly mistake and allow to write the Physical Thread enable registers 0 and 1. Skiboot prefers to use the ENx_SET variant so it went unnoticed, but there's no reason to discard a write to the full register, it is Read-Write. Fixes: da71b7e3ed45 ("ppc/pnv: Add a XIVE2 controller to the POWER10 chip") Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com> Reviewed-by: Cédric Le Goater <clg@kaod.org> Message-Id: <20230601121331.487207-4-fbarrat@linux.ibm.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- hw/intc/pnv_xive2.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/intc/pnv_xive2.c b/hw/intc/pnv_xive2.c index a75ff270ac..132f82a035 100644 --- a/hw/intc/pnv_xive2.c +++ b/hw/intc/pnv_xive2.c @@ -1294,6 +1294,7 @@ static void pnv_xive2_ic_tctxt_write(void *opaque, hwaddr offset, */ case TCTXT_EN0: /* Physical Thread Enable */ case TCTXT_EN1: /* Physical Thread Enable (fused core) */ + xive->tctxt_regs[reg] = val; break; case TCTXT_EN0_SET: From afca92071fc12402a8dee1ad68f66f22dd4b9872 Mon Sep 17 00:00:00 2001 From: Frederic Barrat <fbarrat@linux.ibm.com> Date: Thu, 1 Jun 2023 14:13:30 +0200 Subject: [PATCH 04/29] pnv/xive2: Introduce macros to manipulate TIMA addresses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TIMA addresses are somewhat special and are split in several bit fields with different meanings. This patch describes it and introduce macros to more easily access the various fields. Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com> Reviewed-by: Cédric Le Goater <clg@kaod.org> Message-Id: <20230601121331.487207-5-fbarrat@linux.ibm.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- hw/intc/xive.c | 14 +++++++------- include/hw/ppc/xive_regs.h | 16 ++++++++++++++++ 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/hw/intc/xive.c b/hw/intc/xive.c index a986b96843..ebe399bc09 100644 --- a/hw/intc/xive.c +++ b/hw/intc/xive.c @@ -249,7 +249,7 @@ static const uint8_t *xive_tm_views[] = { static uint64_t xive_tm_mask(hwaddr offset, unsigned size, bool write) { uint8_t page_offset = (offset >> TM_SHIFT) & 0x3; - uint8_t reg_offset = offset & 0x3F; + uint8_t reg_offset = offset & TM_REG_OFFSET; uint8_t reg_mask = write ? 0x1 : 0x2; uint64_t mask = 0x0; int i; @@ -266,8 +266,8 @@ static uint64_t xive_tm_mask(hwaddr offset, unsigned size, bool write) static void xive_tm_raw_write(XiveTCTX *tctx, hwaddr offset, uint64_t value, unsigned size) { - uint8_t ring_offset = offset & 0x30; - uint8_t reg_offset = offset & 0x3F; + uint8_t ring_offset = offset & TM_RING_OFFSET; + uint8_t reg_offset = offset & TM_REG_OFFSET; uint64_t mask = xive_tm_mask(offset, size, true); int i; @@ -296,8 +296,8 @@ static void xive_tm_raw_write(XiveTCTX *tctx, hwaddr offset, uint64_t value, static uint64_t xive_tm_raw_read(XiveTCTX *tctx, hwaddr offset, unsigned size) { - uint8_t ring_offset = offset & 0x30; - uint8_t reg_offset = offset & 0x3F; + uint8_t ring_offset = offset & TM_RING_OFFSET; + uint8_t reg_offset = offset & TM_REG_OFFSET; uint64_t mask = xive_tm_mask(offset, size, false); uint64_t ret; int i; @@ -534,7 +534,7 @@ void xive_tctx_tm_write(XivePresenter *xptr, XiveTCTX *tctx, hwaddr offset, /* * First, check for special operations in the 2K region */ - if (offset & 0x800) { + if (offset & TM_SPECIAL_OP) { xto = xive_tm_find_op(offset, size, true); if (!xto) { qemu_log_mask(LOG_GUEST_ERROR, "XIVE: invalid write access at TIMA " @@ -573,7 +573,7 @@ uint64_t xive_tctx_tm_read(XivePresenter *xptr, XiveTCTX *tctx, hwaddr offset, /* * First, check for special operations in the 2K region */ - if (offset & 0x800) { + if (offset & TM_SPECIAL_OP) { xto = xive_tm_find_op(offset, size, false); if (!xto) { qemu_log_mask(LOG_GUEST_ERROR, "XIVE: invalid read access to TIMA" diff --git a/include/hw/ppc/xive_regs.h b/include/hw/ppc/xive_regs.h index b7fde2354e..4a3c9badd3 100644 --- a/include/hw/ppc/xive_regs.h +++ b/include/hw/ppc/xive_regs.h @@ -48,6 +48,22 @@ #define TM_SHIFT 16 +/* + * TIMA addresses are 12-bits (4k page). + * The MSB indicates a special op with side effect, which can be + * refined with bit 10 (see below). + * The registers, logically grouped in 4 rings (a quad-word each), are + * defined on the 6 LSBs (offset below 0x40) + * In between, we can add a cache line index from 0...3 (ie, 0, 0x80, + * 0x100, 0x180) to select a specific snooper. Those 'snoop port + * address' bits should be dropped when processing the operations as + * they are all equivalent. + */ +#define TM_ADDRESS_MASK 0xC3F +#define TM_SPECIAL_OP 0x800 +#define TM_RING_OFFSET 0x30 +#define TM_REG_OFFSET 0x3F + /* TM register offsets */ #define TM_QW0_USER 0x000 /* All rings */ #define TM_QW1_OS 0x010 /* Ring 0..2 */ From 6f2cbd133d44547fe71879271adaadf11f20965b Mon Sep 17 00:00:00 2001 From: Frederic Barrat <fbarrat@linux.ibm.com> Date: Thu, 1 Jun 2023 14:13:31 +0200 Subject: [PATCH 05/29] pnv/xive2: Handle TIMA access through all ports MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Thread Interrupt Management Area (TIMA) can be accessed through 4 ports, targeted by the address. The base address of a TIMA is using port 0 and the other ports are 0x80 apart. Using one port or another can be useful to balance the load on the snoop buses. With skiboot and linux, we currently use port 0, but as it tends to be busy, another hypervisor is using port 1 for TIMA access. The port address bits fall in between the special op indication bits (the 2 MSBs) and the register offset bits (the 6 LSBs). They are "don't care" for the hardware when processing a TIMA operation. This patch filters out those port address bits so that a TIMA operation can be triggered using any port. It is also true for indirect access (through the IC BAR) and it's actually nothing new, it was already the case on P9. Which helps here, as the TIMA handling code is common between P9 (xive) and P10 (xive2). Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com> Reviewed-by: Cédric Le Goater <clg@kaod.org> Message-Id: <20230601121331.487207-6-fbarrat@linux.ibm.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- hw/intc/pnv_xive2.c | 4 ++++ hw/intc/xive.c | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/hw/intc/pnv_xive2.c b/hw/intc/pnv_xive2.c index 132f82a035..e5a028c1e6 100644 --- a/hw/intc/pnv_xive2.c +++ b/hw/intc/pnv_xive2.c @@ -1662,6 +1662,8 @@ static void pnv_xive2_tm_write(void *opaque, hwaddr offset, bool gen1_tima_os = xive->cq_regs[CQ_XIVE_CFG >> 3] & CQ_XIVE_CFG_GEN1_TIMA_OS; + offset &= TM_ADDRESS_MASK; + /* TODO: should we switch the TM ops table instead ? */ if (!gen1_tima_os && offset == HV_PUSH_OS_CTX_OFFSET) { xive2_tm_push_os_ctx(xptr, tctx, offset, value, size); @@ -1681,6 +1683,8 @@ static uint64_t pnv_xive2_tm_read(void *opaque, hwaddr offset, unsigned size) bool gen1_tima_os = xive->cq_regs[CQ_XIVE_CFG >> 3] & CQ_XIVE_CFG_GEN1_TIMA_OS; + offset &= TM_ADDRESS_MASK; + /* TODO: should we switch the TM ops table instead ? */ if (!gen1_tima_os && offset == HV_PULL_OS_CTX_OFFSET) { return xive2_tm_pull_os_ctx(xptr, tctx, offset, size); diff --git a/hw/intc/xive.c b/hw/intc/xive.c index ebe399bc09..5204c14b87 100644 --- a/hw/intc/xive.c +++ b/hw/intc/xive.c @@ -500,7 +500,7 @@ static const XiveTmOp xive_tm_operations[] = { static const XiveTmOp *xive_tm_find_op(hwaddr offset, unsigned size, bool write) { uint8_t page_offset = (offset >> TM_SHIFT) & 0x3; - uint32_t op_offset = offset & 0xFFF; + uint32_t op_offset = offset & TM_ADDRESS_MASK; int i; for (i = 0; i < ARRAY_SIZE(xive_tm_operations); i++) { From 6c242e79b876b3570b8fd2f10f2a502467758e56 Mon Sep 17 00:00:00 2001 From: Nicholas Piggin <npiggin@gmail.com> Date: Tue, 30 May 2023 23:21:27 +1000 Subject: [PATCH 06/29] target/ppc: Fix nested-hv HEAI delivery ppc hypervisors turn HEAI interrupts into program interrupts injected into the guest that executed the illegal instruction, if the hypervisor doesn't handle it some other way. The nested-hv implementation failed to account for this HEAI->program conversion. The virtual hypervisor wants to see the HEAI when running a nested guest, so that interrupt type can be returned to its KVM caller. Fixes: 7cebc5db2eba6 ("target/ppc: Introduce a vhyp framework for nested HV support") Cc: balaton@eik.bme.hu Reviewed-by: Fabiano Rosas <farosas@suse.de> Signed-off-by: Nicholas Piggin <npiggin@gmail.com> Message-Id: <20230530132127.385001-1-npiggin@gmail.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- target/ppc/excp_helper.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c index fea9221501..9ffcfe788a 100644 --- a/target/ppc/excp_helper.c +++ b/target/ppc/excp_helper.c @@ -1358,9 +1358,12 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp) /* * We don't want to generate a Hypervisor Emulation Assistance - * Interrupt if we don't have HVB in msr_mask (PAPR mode). + * Interrupt if we don't have HVB in msr_mask (PAPR mode), + * unless running a nested-hv guest, in which case the L1 + * kernel wants the interrupt. */ - if (excp == POWERPC_EXCP_HV_EMU && !(env->msr_mask & MSR_HVB)) { + if (excp == POWERPC_EXCP_HV_EMU && !(env->msr_mask & MSR_HVB) && + !books_vhyp_handles_hv_excp(cpu)) { excp = POWERPC_EXCP_PROGRAM; } From 34b4313070b5f5613fb8198806f57614826b0aac Mon Sep 17 00:00:00 2001 From: Frederic Barrat <fbarrat@linux.ibm.com> Date: Wed, 31 May 2023 17:05:37 +0200 Subject: [PATCH 07/29] pnv/xive2: Quiet down some error messages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When dumping the END and NVP tables ("info pic" from the HMP) on the P10 model, we're likely to be flooded with error messages such as: XIVE[0] - VST: invalid NVPT entry f33800 !? The error is printed when finding an empty VSD in an indirect table (thus END and NVP tables with skiboot), which is going to happen when dumping the xive state. So let's tune down those messages. They can be re-enabled easily with a macro if needed. Those errors were already hidden on xive/P9, for the same reason. Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com> Reviewed-by: Cédric Le Goater <clg@kaod.org> Message-Id: <20230531150537.369350-1-fbarrat@linux.ibm.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- hw/intc/pnv_xive2.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/hw/intc/pnv_xive2.c b/hw/intc/pnv_xive2.c index e5a028c1e6..ec1edeb385 100644 --- a/hw/intc/pnv_xive2.c +++ b/hw/intc/pnv_xive2.c @@ -163,7 +163,9 @@ static uint64_t pnv_xive2_vst_addr_indirect(PnvXive2 *xive, uint32_t type, ldq_be_dma(&address_space_memory, vsd_addr, &vsd, MEMTXATTRS_UNSPECIFIED); if (!(vsd & VSD_ADDRESS_MASK)) { +#ifdef XIVE2_DEBUG xive2_error(xive, "VST: invalid %s entry %x !?", info->name, idx); +#endif return 0; } @@ -185,7 +187,9 @@ static uint64_t pnv_xive2_vst_addr_indirect(PnvXive2 *xive, uint32_t type, MEMTXATTRS_UNSPECIFIED); if (!(vsd & VSD_ADDRESS_MASK)) { +#ifdef XIVE2_DEBUG xive2_error(xive, "VST: invalid %s entry %x !?", info->name, idx); +#endif return 0; } From 6494d2c1fd4ebc37b575130399a97a1fcfff1afc Mon Sep 17 00:00:00 2001 From: Nicholas Piggin <npiggin@gmail.com> Date: Tue, 30 May 2023 23:04:47 +1000 Subject: [PATCH 08/29] target/ppc: Fix PMU hflags calculation Some of the PMU hflags bits can go out of synch, for example a store to MMCR0 with PMCjCE=1 fails to update hflags correctly and results in hflags mismatch: qemu: fatal: TCG hflags mismatch (current:0x2408003d rebuilt:0x240a003d) This can be reproduced by running perf on a recent machine. Some of the fragility here is the duplication of PMU hflags calculations. This change consolidates that in a single place to update pmu-related hflags, to be called after a well defined state changes. The post-load PMU update is pulled out of the MSR update because it does not depend on the MSR value. Fixes: 8b3d1c49a9f0 ("target/ppc: Add new PMC HFLAGS") Signed-off-by: Nicholas Piggin <npiggin@gmail.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> Message-Id: <20230530130447.372617-1-npiggin@gmail.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- target/ppc/cpu_init.c | 2 +- target/ppc/helper_regs.c | 73 ++++++++++++++++++++++++++++++---------- target/ppc/helper_regs.h | 1 + target/ppc/machine.c | 8 ++--- target/ppc/power8-pmu.c | 38 ++++++++++++--------- target/ppc/power8-pmu.h | 4 +-- 6 files changed, 85 insertions(+), 41 deletions(-) diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c index 05bf73296b..398f2d9966 100644 --- a/target/ppc/cpu_init.c +++ b/target/ppc/cpu_init.c @@ -7083,7 +7083,7 @@ static void ppc_cpu_reset_hold(Object *obj) if (env->mmu_model != POWERPC_MMU_REAL) { ppc_tlb_invalidate_all(env); } - pmu_update_summaries(env); + pmu_mmcr01_updated(env); } /* clean any pending stop state */ diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c index fb351c303f..bc7e9d7eda 100644 --- a/target/ppc/helper_regs.c +++ b/target/ppc/helper_regs.c @@ -47,6 +47,48 @@ void hreg_swap_gpr_tgpr(CPUPPCState *env) env->tgpr[3] = tmp; } +static uint32_t hreg_compute_pmu_hflags_value(CPUPPCState *env) +{ + uint32_t hflags = 0; + +#if defined(TARGET_PPC64) + if (env->spr[SPR_POWER_MMCR0] & MMCR0_PMCC0) { + hflags |= 1 << HFLAGS_PMCC0; + } + if (env->spr[SPR_POWER_MMCR0] & MMCR0_PMCC1) { + hflags |= 1 << HFLAGS_PMCC1; + } + if (env->spr[SPR_POWER_MMCR0] & MMCR0_PMCjCE) { + hflags |= 1 << HFLAGS_PMCJCE; + } + +#ifndef CONFIG_USER_ONLY + if (env->pmc_ins_cnt) { + hflags |= 1 << HFLAGS_INSN_CNT; + } + if (env->pmc_ins_cnt & 0x1e) { + hflags |= 1 << HFLAGS_PMC_OTHER; + } +#endif +#endif + + return hflags; +} + +/* Mask of all PMU hflags */ +static uint32_t hreg_compute_pmu_hflags_mask(CPUPPCState *env) +{ + uint32_t hflags_mask = 0; +#if defined(TARGET_PPC64) + hflags_mask |= 1 << HFLAGS_PMCC0; + hflags_mask |= 1 << HFLAGS_PMCC1; + hflags_mask |= 1 << HFLAGS_PMCJCE; + hflags_mask |= 1 << HFLAGS_INSN_CNT; + hflags_mask |= 1 << HFLAGS_PMC_OTHER; +#endif + return hflags_mask; +} + static uint32_t hreg_compute_hflags_value(CPUPPCState *env) { target_ulong msr = env->msr; @@ -104,30 +146,12 @@ static uint32_t hreg_compute_hflags_value(CPUPPCState *env) if (env->spr[SPR_LPCR] & LPCR_HR) { hflags |= 1 << HFLAGS_HR; } - if (env->spr[SPR_POWER_MMCR0] & MMCR0_PMCC0) { - hflags |= 1 << HFLAGS_PMCC0; - } - if (env->spr[SPR_POWER_MMCR0] & MMCR0_PMCC1) { - hflags |= 1 << HFLAGS_PMCC1; - } - if (env->spr[SPR_POWER_MMCR0] & MMCR0_PMCjCE) { - hflags |= 1 << HFLAGS_PMCJCE; - } #ifndef CONFIG_USER_ONLY if (!env->has_hv_mode || (msr & (1ull << MSR_HV))) { hflags |= 1 << HFLAGS_HV; } -#if defined(TARGET_PPC64) - if (env->pmc_ins_cnt) { - hflags |= 1 << HFLAGS_INSN_CNT; - } - if (env->pmc_ins_cnt & 0x1e) { - hflags |= 1 << HFLAGS_PMC_OTHER; - } -#endif - /* * This is our encoding for server processors. The architecture * specifies that there is no such thing as userspace with @@ -172,6 +196,8 @@ static uint32_t hreg_compute_hflags_value(CPUPPCState *env) hflags |= dmmu_idx << HFLAGS_DMMU_IDX; #endif + hflags |= hreg_compute_pmu_hflags_value(env); + return hflags | (msr & msr_mask); } @@ -180,6 +206,17 @@ void hreg_compute_hflags(CPUPPCState *env) env->hflags = hreg_compute_hflags_value(env); } +/* + * This can be used as a lighter-weight alternative to hreg_compute_hflags + * when PMU MMCR0 or pmc_ins_cnt changes. pmc_ins_cnt is changed by + * pmu_update_summaries. + */ +void hreg_update_pmu_hflags(CPUPPCState *env) +{ + env->hflags &= ~hreg_compute_pmu_hflags_mask(env); + env->hflags |= hreg_compute_pmu_hflags_value(env); +} + #ifdef CONFIG_DEBUG_TCG void cpu_get_tb_cpu_state(CPUPPCState *env, target_ulong *pc, target_ulong *cs_base, uint32_t *flags) diff --git a/target/ppc/helper_regs.h b/target/ppc/helper_regs.h index 42f26870b9..8196c1346d 100644 --- a/target/ppc/helper_regs.h +++ b/target/ppc/helper_regs.h @@ -22,6 +22,7 @@ void hreg_swap_gpr_tgpr(CPUPPCState *env); void hreg_compute_hflags(CPUPPCState *env); +void hreg_update_pmu_hflags(CPUPPCState *env); void cpu_interrupt_exittb(CPUState *cs); int hreg_store_msr(CPUPPCState *env, target_ulong value, int alter_hv); diff --git a/target/ppc/machine.c b/target/ppc/machine.c index be6eb3d968..134b16c625 100644 --- a/target/ppc/machine.c +++ b/target/ppc/machine.c @@ -21,10 +21,6 @@ static void post_load_update_msr(CPUPPCState *env) */ env->msr ^= env->msr_mask & ~((1ULL << MSR_TGPR) | MSR_HVB); ppc_store_msr(env, msr); - - if (tcg_enabled()) { - pmu_update_summaries(env); - } } static int get_avr(QEMUFile *f, void *pv, size_t size, @@ -317,6 +313,10 @@ static int cpu_post_load(void *opaque, int version_id) post_load_update_msr(env); + if (tcg_enabled()) { + pmu_mmcr01_updated(env); + } + return 0; } diff --git a/target/ppc/power8-pmu.c b/target/ppc/power8-pmu.c index 64a64865d7..c4c331c6b5 100644 --- a/target/ppc/power8-pmu.c +++ b/target/ppc/power8-pmu.c @@ -31,7 +31,11 @@ static bool pmc_has_overflow_enabled(CPUPPCState *env, int sprn) return env->spr[SPR_POWER_MMCR0] & MMCR0_PMCjCE; } -void pmu_update_summaries(CPUPPCState *env) +/* + * Called after MMCR0 or MMCR1 changes to update pmc_ins_cnt and pmc_cyc_cnt. + * hflags must subsequently be updated. + */ +static void pmu_update_summaries(CPUPPCState *env) { target_ulong mmcr0 = env->spr[SPR_POWER_MMCR0]; target_ulong mmcr1 = env->spr[SPR_POWER_MMCR1]; @@ -39,7 +43,7 @@ void pmu_update_summaries(CPUPPCState *env) int cyc_cnt = 0; if (mmcr0 & MMCR0_FC) { - goto hflags_calc; + goto out; } if (!(mmcr0 & MMCR0_FC14) && mmcr1 != 0) { @@ -73,10 +77,19 @@ void pmu_update_summaries(CPUPPCState *env) ins_cnt |= !(mmcr0 & MMCR0_FC56) << 5; cyc_cnt |= !(mmcr0 & MMCR0_FC56) << 6; - hflags_calc: + out: env->pmc_ins_cnt = ins_cnt; env->pmc_cyc_cnt = cyc_cnt; - env->hflags = deposit32(env->hflags, HFLAGS_INSN_CNT, 1, ins_cnt != 0); +} + +void pmu_mmcr01_updated(CPUPPCState *env) +{ + pmu_update_summaries(env); + hreg_update_pmu_hflags(env); + /* + * Should this update overflow timers (if mmcr0 is updated) so they + * get set in cpu_post_load? + */ } static bool pmu_increment_insns(CPUPPCState *env, uint32_t num_insns) @@ -234,18 +247,11 @@ static void pmu_delete_timers(CPUPPCState *env) void helper_store_mmcr0(CPUPPCState *env, target_ulong value) { - bool hflags_pmcc0 = (value & MMCR0_PMCC0) != 0; - bool hflags_pmcc1 = (value & MMCR0_PMCC1) != 0; - pmu_update_cycles(env); env->spr[SPR_POWER_MMCR0] = value; - /* MMCR0 writes can change HFLAGS_PMCC[01] and HFLAGS_INSN_CNT */ - env->hflags = deposit32(env->hflags, HFLAGS_PMCC0, 1, hflags_pmcc0); - env->hflags = deposit32(env->hflags, HFLAGS_PMCC1, 1, hflags_pmcc1); - - pmu_update_summaries(env); + pmu_mmcr01_updated(env); /* Update cycle overflow timers with the current MMCR0 state */ pmu_update_overflow_timers(env); @@ -257,8 +263,7 @@ void helper_store_mmcr1(CPUPPCState *env, uint64_t value) env->spr[SPR_POWER_MMCR1] = value; - /* MMCR1 writes can change HFLAGS_INSN_CNT */ - pmu_update_summaries(env); + pmu_mmcr01_updated(env); } target_ulong helper_read_pmc(CPUPPCState *env, uint32_t sprn) @@ -287,8 +292,8 @@ static void fire_PMC_interrupt(PowerPCCPU *cpu) env->spr[SPR_POWER_MMCR0] &= ~MMCR0_FCECE; env->spr[SPR_POWER_MMCR0] |= MMCR0_FC; - /* Changing MMCR0_FC requires a new HFLAGS_INSN_CNT calc */ - pmu_update_summaries(env); + /* Changing MMCR0_FC requires summaries and hflags update */ + pmu_mmcr01_updated(env); /* * Delete all pending timers if we need to freeze @@ -299,6 +304,7 @@ static void fire_PMC_interrupt(PowerPCCPU *cpu) } if (env->spr[SPR_POWER_MMCR0] & MMCR0_PMAE) { + /* These MMCR0 bits do not require summaries or hflags update. */ env->spr[SPR_POWER_MMCR0] &= ~MMCR0_PMAE; env->spr[SPR_POWER_MMCR0] |= MMCR0_PMAO; } diff --git a/target/ppc/power8-pmu.h b/target/ppc/power8-pmu.h index c0093e2219..775e640053 100644 --- a/target/ppc/power8-pmu.h +++ b/target/ppc/power8-pmu.h @@ -18,10 +18,10 @@ #define PMC_COUNTER_NEGATIVE_VAL 0x80000000UL void cpu_ppc_pmu_init(CPUPPCState *env); -void pmu_update_summaries(CPUPPCState *env); +void pmu_mmcr01_updated(CPUPPCState *env); #else static inline void cpu_ppc_pmu_init(CPUPPCState *env) { } -static inline void pmu_update_summaries(CPUPPCState *env) { } +static inline void pmu_mmcr01_updated(CPUPPCState *env) { } #endif #endif From 82ce3d5614b2ef3a00b92ecbff074d5476eff285 Mon Sep 17 00:00:00 2001 From: Nicholas Piggin <npiggin@gmail.com> Date: Tue, 30 May 2023 23:43:12 +1000 Subject: [PATCH 09/29] target/ppc: PMU do not clear MMCR0[FCECE] on performance monitor alert FCECE does not get cleared according to the ISA v3.1B. Signed-off-by: Nicholas Piggin <npiggin@gmail.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> Message-Id: <20230530134313.387252-1-npiggin@gmail.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- target/ppc/power8-pmu.c | 1 - 1 file changed, 1 deletion(-) diff --git a/target/ppc/power8-pmu.c b/target/ppc/power8-pmu.c index c4c331c6b5..af065115f2 100644 --- a/target/ppc/power8-pmu.c +++ b/target/ppc/power8-pmu.c @@ -289,7 +289,6 @@ static void fire_PMC_interrupt(PowerPCCPU *cpu) pmu_update_cycles(env); if (env->spr[SPR_POWER_MMCR0] & MMCR0_FCECE) { - env->spr[SPR_POWER_MMCR0] &= ~MMCR0_FCECE; env->spr[SPR_POWER_MMCR0] |= MMCR0_FC; /* Changing MMCR0_FC requires summaries and hflags update */ From 2e9855555ebbbd4b22418b6bffce1f0f14234141 Mon Sep 17 00:00:00 2001 From: Nicholas Piggin <npiggin@gmail.com> Date: Tue, 30 May 2023 23:07:14 +1000 Subject: [PATCH 10/29] target/ppc: Fix msgclrp interrupt type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit msgclrp matches msgsndp and should clear PPC_INTERRUPT_DOORBELL. Signed-off-by: Nicholas Piggin <npiggin@gmail.com> Reviewed-by: Cédric Le Goater <clg@kaod.org> Message-Id: <20230530130714.373215-1-npiggin@gmail.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- target/ppc/excp_helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c index 9ffcfe788a..de6ad121d2 100644 --- a/target/ppc/excp_helper.c +++ b/target/ppc/excp_helper.c @@ -3071,7 +3071,7 @@ void helper_book3s_msgclrp(CPUPPCState *env, target_ulong rb) return; } - ppc_set_irq(env_archcpu(env), PPC_INTERRUPT_HDOORBELL, 0); + ppc_set_irq(env_archcpu(env), PPC_INTERRUPT_DOORBELL, 0); } /* From fd7abfab662a2bd8bbf63ad52d4b58243cd9c409 Mon Sep 17 00:00:00 2001 From: Nicholas Piggin <npiggin@gmail.com> Date: Tue, 30 May 2023 23:05:26 +1000 Subject: [PATCH 11/29] target/ppc: Support directed privileged doorbell interrupt (SDOOR) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BookS msgsndp instruction to self or DPDES register can cause SDOOR interrupts which crash QEMU with exception not implemented. Linux does not use msgsndp in SMT1, and KVM only uses DPDES to cause doorbells when emulating a SMT guest (which is not the default), so this has gone unnoticed. Signed-off-by: Nicholas Piggin <npiggin@gmail.com> Reviewed-by: Cédric Le Goater <clg@kaod.org> Message-Id: <20230530130526.372701-1-npiggin@gmail.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- target/ppc/excp_helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c index de6ad121d2..befa9aab7f 100644 --- a/target/ppc/excp_helper.c +++ b/target/ppc/excp_helper.c @@ -1542,6 +1542,7 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp) case POWERPC_EXCP_DSEG: /* Data segment exception */ case POWERPC_EXCP_ISEG: /* Instruction segment exception */ case POWERPC_EXCP_TRACE: /* Trace exception */ + case POWERPC_EXCP_SDOOR: /* Doorbell interrupt */ break; case POWERPC_EXCP_HISI: /* Hypervisor instruction storage exception */ msr |= env->error_code; @@ -1587,7 +1588,6 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp) case POWERPC_EXCP_PERFM: /* Embedded performance monitor interrupt */ case POWERPC_EXCP_VPUA: /* Vector assist exception */ case POWERPC_EXCP_MAINT: /* Maintenance exception */ - case POWERPC_EXCP_SDOOR: /* Doorbell interrupt */ case POWERPC_EXCP_HV_MAINT: /* Hypervisor Maintenance exception */ cpu_abort(cs, "%s exception not implemented\n", powerpc_excp_name(excp)); From c29b070418c79fa8e09d17e4b52f3ffddd393764 Mon Sep 17 00:00:00 2001 From: Nicholas Piggin <npiggin@gmail.com> Date: Tue, 30 May 2023 23:43:13 +1000 Subject: [PATCH 12/29] target/ppc: PMU implement PERFM interrupts The PMU raises a performance monitor exception (causing an interrupt when MSR[EE]=1) when MMCR0[PMAO] is set, and lowers it when clear. Wire this up and implement the interrupt delivery for books. Linux perf record can now collect PMI-driven samples. fire_PMC_interrupt is renamed to perfm_alert, which matches a bit closer to the new terminology used in the ISA and distinguishes the alert condition (e.g., counter overflow) from the PERFM (or EBB) interrupts. Signed-off-by: Nicholas Piggin <npiggin@gmail.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> Message-Id: <20230530134313.387252-2-npiggin@gmail.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- target/ppc/excp_helper.c | 2 +- target/ppc/power8-pmu.c | 21 ++++++++++++++------- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c index befa9aab7f..8b95410c36 100644 --- a/target/ppc/excp_helper.c +++ b/target/ppc/excp_helper.c @@ -1543,6 +1543,7 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp) case POWERPC_EXCP_ISEG: /* Instruction segment exception */ case POWERPC_EXCP_TRACE: /* Trace exception */ case POWERPC_EXCP_SDOOR: /* Doorbell interrupt */ + case POWERPC_EXCP_PERFM: /* Performance monitor interrupt */ break; case POWERPC_EXCP_HISI: /* Hypervisor instruction storage exception */ msr |= env->error_code; @@ -1585,7 +1586,6 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp) */ return; case POWERPC_EXCP_THERM: /* Thermal interrupt */ - case POWERPC_EXCP_PERFM: /* Embedded performance monitor interrupt */ case POWERPC_EXCP_VPUA: /* Vector assist exception */ case POWERPC_EXCP_MAINT: /* Maintenance exception */ case POWERPC_EXCP_HV_MAINT: /* Hypervisor Maintenance exception */ diff --git a/target/ppc/power8-pmu.c b/target/ppc/power8-pmu.c index af065115f2..7bb4bf81f7 100644 --- a/target/ppc/power8-pmu.c +++ b/target/ppc/power8-pmu.c @@ -84,8 +84,17 @@ static void pmu_update_summaries(CPUPPCState *env) void pmu_mmcr01_updated(CPUPPCState *env) { + PowerPCCPU *cpu = env_archcpu(env); + pmu_update_summaries(env); hreg_update_pmu_hflags(env); + + if (env->spr[SPR_POWER_MMCR0] & MMCR0_PMAO) { + ppc_set_irq(cpu, PPC_INTERRUPT_PERFM, 1); + } else { + ppc_set_irq(cpu, PPC_INTERRUPT_PERFM, 0); + } + /* * Should this update overflow timers (if mmcr0 is updated) so they * get set in cpu_post_load? @@ -282,7 +291,7 @@ void helper_store_pmc(CPUPPCState *env, uint32_t sprn, uint64_t value) pmc_update_overflow_timer(env, sprn); } -static void fire_PMC_interrupt(PowerPCCPU *cpu) +static void perfm_alert(PowerPCCPU *cpu) { CPUPPCState *env = &cpu->env; @@ -306,6 +315,7 @@ static void fire_PMC_interrupt(PowerPCCPU *cpu) /* These MMCR0 bits do not require summaries or hflags update. */ env->spr[SPR_POWER_MMCR0] &= ~MMCR0_PMAE; env->spr[SPR_POWER_MMCR0] |= MMCR0_PMAO; + ppc_set_irq(cpu, PPC_INTERRUPT_PERFM, 1); } raise_ebb_perfm_exception(env); @@ -314,20 +324,17 @@ static void fire_PMC_interrupt(PowerPCCPU *cpu) void helper_handle_pmc5_overflow(CPUPPCState *env) { env->spr[SPR_POWER_PMC5] = PMC_COUNTER_NEGATIVE_VAL; - fire_PMC_interrupt(env_archcpu(env)); + perfm_alert(env_archcpu(env)); } /* This helper assumes that the PMC is running. */ void helper_insns_inc(CPUPPCState *env, uint32_t num_insns) { bool overflow_triggered; - PowerPCCPU *cpu; overflow_triggered = pmu_increment_insns(env, num_insns); - if (overflow_triggered) { - cpu = env_archcpu(env); - fire_PMC_interrupt(cpu); + perfm_alert(env_archcpu(env)); } } @@ -335,7 +342,7 @@ static void cpu_ppc_pmu_timer_cb(void *opaque) { PowerPCCPU *cpu = opaque; - fire_PMC_interrupt(cpu); + perfm_alert(cpu); } void cpu_ppc_pmu_init(CPUPPCState *env) From 728fbfb57be5f4a3089a01f5c6ed72e0618b49ae Mon Sep 17 00:00:00 2001 From: BALATON Zoltan <balaton@eik.bme.hu> Date: Tue, 30 May 2023 15:28:07 +0200 Subject: [PATCH 13/29] target/ppc: Remove single use function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The get_physical_address() function is a trivial wrapper of get_physical_address_wtlb() that is only used once. Remove it and call get_physical_address_wtlb() directly instead. Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> Reviewed-by: Cédric Le Goater <clg@kaod.org> Message-Id: <302697d63d26caebefaeee1e45352145ebd0318a.1685448535.git.balaton@eik.bme.hu> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- target/ppc/mmu_helper.c | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c index 64e30435f5..c0c71a68ff 100644 --- a/target/ppc/mmu_helper.c +++ b/target/ppc/mmu_helper.c @@ -168,15 +168,6 @@ static void booke206_flush_tlb(CPUPPCState *env, int flags, tlb_flush(env_cpu(env)); } -static int get_physical_address(CPUPPCState *env, mmu_ctx_t *ctx, - target_ulong eaddr, MMUAccessType access_type, - int type) -{ - return get_physical_address_wtlb(env, ctx, eaddr, access_type, type, 0); -} - - - /*****************************************************************************/ /* BATs management */ #if !defined(FLUSH_ALL_TLBS) @@ -643,7 +634,7 @@ target_ulong helper_rac(CPUPPCState *env, target_ulong addr) */ nb_BATs = env->nb_BATs; env->nb_BATs = 0; - if (get_physical_address(env, &ctx, addr, 0, ACCESS_INT) == 0) { + if (get_physical_address_wtlb(env, &ctx, addr, 0, ACCESS_INT, 0) == 0) { ret = ctx.raddr; } env->nb_BATs = nb_BATs; From 62860c5feaf4eb53acafd10b238374d3ef21229a Mon Sep 17 00:00:00 2001 From: BALATON Zoltan <balaton@eik.bme.hu> Date: Tue, 30 May 2023 15:28:08 +0200 Subject: [PATCH 14/29] target/ppc: Remove "ext" parameter of ppcemb_tlb_check() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is only used by one caller so simplify function by removing this parameter and move the operation to the single place where it's used. Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> Reviewed-by: Cédric Le Goater <clg@kaod.org> Message-Id: <b21f11ae20e8a8c2e8b5d943f2bff12b5356005a.1685448535.git.balaton@eik.bme.hu> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- target/ppc/cpu.h | 3 +-- target/ppc/mmu_common.c | 21 +++++++++------------ target/ppc/mmu_helper.c | 2 +- 3 files changed, 11 insertions(+), 15 deletions(-) diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h index 10c4ffa148..557e02e697 100644 --- a/target/ppc/cpu.h +++ b/target/ppc/cpu.h @@ -1429,8 +1429,7 @@ int ppcmas_tlb_check(CPUPPCState *env, ppcmas_tlb_t *tlb, uint32_t pid); int ppcemb_tlb_check(CPUPPCState *env, ppcemb_tlb_t *tlb, hwaddr *raddrp, - target_ulong address, uint32_t pid, int ext, - int i); + target_ulong address, uint32_t pid, int i); hwaddr booke206_tlb_to_page_size(CPUPPCState *env, ppcmas_tlb_t *tlb); #endif diff --git a/target/ppc/mmu_common.c b/target/ppc/mmu_common.c index 7235a4befe..21a353c51a 100644 --- a/target/ppc/mmu_common.c +++ b/target/ppc/mmu_common.c @@ -491,8 +491,7 @@ static int get_segment_6xx_tlb(CPUPPCState *env, mmu_ctx_t *ctx, /* Generic TLB check function for embedded PowerPC implementations */ int ppcemb_tlb_check(CPUPPCState *env, ppcemb_tlb_t *tlb, hwaddr *raddrp, - target_ulong address, uint32_t pid, int ext, - int i) + target_ulong address, uint32_t pid, int i) { target_ulong mask; @@ -514,11 +513,6 @@ int ppcemb_tlb_check(CPUPPCState *env, ppcemb_tlb_t *tlb, return -1; } *raddrp = (tlb->RPN & mask) | (address & ~mask); - if (ext) { - /* Extend the physical address to 36 bits */ - *raddrp |= (uint64_t)(tlb->RPN & 0xF) << 32; - } - return 0; } @@ -536,7 +530,7 @@ static int mmu40x_get_physical_address(CPUPPCState *env, mmu_ctx_t *ctx, for (i = 0; i < env->nb_tlb; i++) { tlb = &env->tlb.tlbe[i]; if (ppcemb_tlb_check(env, tlb, &raddr, address, - env->spr[SPR_40x_PID], 0, i) < 0) { + env->spr[SPR_40x_PID], i) < 0) { continue; } zsel = (tlb->attr >> 4) & 0xF; @@ -598,20 +592,23 @@ static int mmubooke_check_tlb(CPUPPCState *env, ppcemb_tlb_t *tlb, int prot2; if (ppcemb_tlb_check(env, tlb, raddr, address, - env->spr[SPR_BOOKE_PID], - !env->nb_pids, i) >= 0) { + env->spr[SPR_BOOKE_PID], i) >= 0) { + if (!env->nb_pids) { + /* Extend the physical address to 36 bits */ + *raddr |= (uint64_t)(tlb->RPN & 0xF) << 32; + } goto found_tlb; } if (env->spr[SPR_BOOKE_PID1] && ppcemb_tlb_check(env, tlb, raddr, address, - env->spr[SPR_BOOKE_PID1], 0, i) >= 0) { + env->spr[SPR_BOOKE_PID1], i) >= 0) { goto found_tlb; } if (env->spr[SPR_BOOKE_PID2] && ppcemb_tlb_check(env, tlb, raddr, address, - env->spr[SPR_BOOKE_PID2], 0, i) >= 0) { + env->spr[SPR_BOOKE_PID2], i) >= 0) { goto found_tlb; } diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c index c0c71a68ff..e7275eaec1 100644 --- a/target/ppc/mmu_helper.c +++ b/target/ppc/mmu_helper.c @@ -124,7 +124,7 @@ static int ppcemb_tlb_search(CPUPPCState *env, target_ulong address, ret = -1; for (i = 0; i < env->nb_tlb; i++) { tlb = &env->tlb.tlbe[i]; - if (ppcemb_tlb_check(env, tlb, &raddr, address, pid, 0, i) == 0) { + if (ppcemb_tlb_check(env, tlb, &raddr, address, pid, i) == 0) { ret = i; break; } From 753441c8898e5de41e6051f9d1244e98849bb866 Mon Sep 17 00:00:00 2001 From: BALATON Zoltan <balaton@eik.bme.hu> Date: Tue, 30 May 2023 15:28:09 +0200 Subject: [PATCH 15/29] target/ppc: Move ppcemb_tlb_search() to mmu_common.c MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This function is the only reason why ppcemb_tlb_check() is not static to mmu_common.c but it also better fits in mmu_common.c so move it there. Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> Reviewed-by: Cédric Le Goater <clg@kaod.org> Message-Id: <b64fd712a773558dea9b84945c57785546c0ae2e.1685448535.git.balaton@eik.bme.hu> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- target/ppc/cpu.h | 4 +--- target/ppc/mmu_common.c | 22 +++++++++++++++++++++- target/ppc/mmu_helper.c | 21 --------------------- 3 files changed, 22 insertions(+), 25 deletions(-) diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h index 557e02e697..8001582d52 100644 --- a/target/ppc/cpu.h +++ b/target/ppc/cpu.h @@ -1427,9 +1427,7 @@ void cpu_ppc_set_vhyp(PowerPCCPU *cpu, PPCVirtualHypervisor *vhyp); int ppcmas_tlb_check(CPUPPCState *env, ppcmas_tlb_t *tlb, hwaddr *raddrp, target_ulong address, uint32_t pid); -int ppcemb_tlb_check(CPUPPCState *env, ppcemb_tlb_t *tlb, - hwaddr *raddrp, - target_ulong address, uint32_t pid, int i); +int ppcemb_tlb_search(CPUPPCState *env, target_ulong address, uint32_t pid); hwaddr booke206_tlb_to_page_size(CPUPPCState *env, ppcmas_tlb_t *tlb); #endif diff --git a/target/ppc/mmu_common.c b/target/ppc/mmu_common.c index 21a353c51a..845eee4c6f 100644 --- a/target/ppc/mmu_common.c +++ b/target/ppc/mmu_common.c @@ -489,7 +489,7 @@ static int get_segment_6xx_tlb(CPUPPCState *env, mmu_ctx_t *ctx, } /* Generic TLB check function for embedded PowerPC implementations */ -int ppcemb_tlb_check(CPUPPCState *env, ppcemb_tlb_t *tlb, +static int ppcemb_tlb_check(CPUPPCState *env, ppcemb_tlb_t *tlb, hwaddr *raddrp, target_ulong address, uint32_t pid, int i) { @@ -516,6 +516,26 @@ int ppcemb_tlb_check(CPUPPCState *env, ppcemb_tlb_t *tlb, return 0; } +/* Generic TLB search function for PowerPC embedded implementations */ +int ppcemb_tlb_search(CPUPPCState *env, target_ulong address, uint32_t pid) +{ + ppcemb_tlb_t *tlb; + hwaddr raddr; + int i, ret; + + /* Default return value is no match */ + ret = -1; + for (i = 0; i < env->nb_tlb; i++) { + tlb = &env->tlb.tlbe[i]; + if (ppcemb_tlb_check(env, tlb, &raddr, address, pid, i) == 0) { + ret = i; + break; + } + } + + return ret; +} + static int mmu40x_get_physical_address(CPUPPCState *env, mmu_ctx_t *ctx, target_ulong address, MMUAccessType access_type) diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c index e7275eaec1..d3ea7588f9 100644 --- a/target/ppc/mmu_helper.c +++ b/target/ppc/mmu_helper.c @@ -112,27 +112,6 @@ static void ppc6xx_tlb_store(CPUPPCState *env, target_ulong EPN, int way, env->last_way = way; } -/* Generic TLB search function for PowerPC embedded implementations */ -static int ppcemb_tlb_search(CPUPPCState *env, target_ulong address, - uint32_t pid) -{ - ppcemb_tlb_t *tlb; - hwaddr raddr; - int i, ret; - - /* Default return value is no match */ - ret = -1; - for (i = 0; i < env->nb_tlb; i++) { - tlb = &env->tlb.tlbe[i]; - if (ppcemb_tlb_check(env, tlb, &raddr, address, pid, i) == 0) { - ret = i; - break; - } - } - - return ret; -} - /* Helpers specific to PowerPC 40x implementations */ static inline void ppc4xx_tlb_invalidate_all(CPUPPCState *env) { From a1fa47fad12ea104b2d3f7f8b39f102bc16df539 Mon Sep 17 00:00:00 2001 From: BALATON Zoltan <balaton@eik.bme.hu> Date: Tue, 30 May 2023 15:28:10 +0200 Subject: [PATCH 16/29] target/ppc: Remove some unneded line breaks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Make lines shorter and fix indentation in some functions prototypes. Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> Reviewed-by: Cédric Le Goater <clg@kaod.org> Message-Id: <70952ba2d82141db1cf5cfcf4b227402be575874.1685448535.git.balaton@eik.bme.hu> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- target/ppc/cpu.h | 8 +++----- target/ppc/mmu_common.c | 8 +++----- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h index 8001582d52..c7c2a5534c 100644 --- a/target/ppc/cpu.h +++ b/target/ppc/cpu.h @@ -1424,12 +1424,10 @@ void store_booke_tsr(CPUPPCState *env, target_ulong val); void ppc_tlb_invalidate_all(CPUPPCState *env); void ppc_tlb_invalidate_one(CPUPPCState *env, target_ulong addr); void cpu_ppc_set_vhyp(PowerPCCPU *cpu, PPCVirtualHypervisor *vhyp); -int ppcmas_tlb_check(CPUPPCState *env, ppcmas_tlb_t *tlb, - hwaddr *raddrp, target_ulong address, - uint32_t pid); +int ppcmas_tlb_check(CPUPPCState *env, ppcmas_tlb_t *tlb, hwaddr *raddrp, + target_ulong address, uint32_t pid); int ppcemb_tlb_search(CPUPPCState *env, target_ulong address, uint32_t pid); -hwaddr booke206_tlb_to_page_size(CPUPPCState *env, - ppcmas_tlb_t *tlb); +hwaddr booke206_tlb_to_page_size(CPUPPCState *env, ppcmas_tlb_t *tlb); #endif void ppc_store_fpscr(CPUPPCState *env, target_ulong val); diff --git a/target/ppc/mmu_common.c b/target/ppc/mmu_common.c index 845eee4c6f..a84bc7de88 100644 --- a/target/ppc/mmu_common.c +++ b/target/ppc/mmu_common.c @@ -694,8 +694,7 @@ static int mmubooke_get_physical_address(CPUPPCState *env, mmu_ctx_t *ctx, return ret; } -hwaddr booke206_tlb_to_page_size(CPUPPCState *env, - ppcmas_tlb_t *tlb) +hwaddr booke206_tlb_to_page_size(CPUPPCState *env, ppcmas_tlb_t *tlb) { int tlbm_size; @@ -705,9 +704,8 @@ hwaddr booke206_tlb_to_page_size(CPUPPCState *env, } /* TLB check function for MAS based SoftTLBs */ -int ppcmas_tlb_check(CPUPPCState *env, ppcmas_tlb_t *tlb, - hwaddr *raddrp, target_ulong address, - uint32_t pid) +int ppcmas_tlb_check(CPUPPCState *env, ppcmas_tlb_t *tlb, hwaddr *raddrp, + target_ulong address, uint32_t pid) { hwaddr mask; uint32_t tlb_pid; From bb60364c202d0447fab78653b660a092b11378e4 Mon Sep 17 00:00:00 2001 From: BALATON Zoltan <balaton@eik.bme.hu> Date: Tue, 30 May 2023 15:28:11 +0200 Subject: [PATCH 17/29] target/ppc: Simplify ppcemb_tlb_search() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit No nead to store return value and break from loop when we can return directly. Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> Reviewed-by: Cédric Le Goater <clg@kaod.org> Message-Id: <d470118c3adcbd41b1a91779f6bb7cbdb2b0d346.1685448535.git.balaton@eik.bme.hu> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- target/ppc/mmu_common.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/target/ppc/mmu_common.c b/target/ppc/mmu_common.c index a84bc7de88..ff7f987546 100644 --- a/target/ppc/mmu_common.c +++ b/target/ppc/mmu_common.c @@ -521,19 +521,15 @@ int ppcemb_tlb_search(CPUPPCState *env, target_ulong address, uint32_t pid) { ppcemb_tlb_t *tlb; hwaddr raddr; - int i, ret; + int i; - /* Default return value is no match */ - ret = -1; for (i = 0; i < env->nb_tlb; i++) { tlb = &env->tlb.tlbe[i]; if (ppcemb_tlb_check(env, tlb, &raddr, address, pid, i) == 0) { - ret = i; - break; + return i; } } - - return ret; + return -1; } static int mmu40x_get_physical_address(CPUPPCState *env, mmu_ctx_t *ctx, From 2b23daa8ebaf184e89bd04eed21817620d600cc7 Mon Sep 17 00:00:00 2001 From: BALATON Zoltan <balaton@eik.bme.hu> Date: Tue, 30 May 2023 15:28:12 +0200 Subject: [PATCH 18/29] target/ppc: Change ppcemb_tlb_check() to return bool MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> Reviewed-by: Cédric Le Goater <clg@kaod.org> Message-Id: <bacd1bcbe99c07930c29a9815915da9ac75f6920.1685448535.git.balaton@eik.bme.hu> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- target/ppc/mmu_common.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/target/ppc/mmu_common.c b/target/ppc/mmu_common.c index ff7f987546..bd7d7d5257 100644 --- a/target/ppc/mmu_common.c +++ b/target/ppc/mmu_common.c @@ -489,15 +489,15 @@ static int get_segment_6xx_tlb(CPUPPCState *env, mmu_ctx_t *ctx, } /* Generic TLB check function for embedded PowerPC implementations */ -static int ppcemb_tlb_check(CPUPPCState *env, ppcemb_tlb_t *tlb, - hwaddr *raddrp, - target_ulong address, uint32_t pid, int i) +static bool ppcemb_tlb_check(CPUPPCState *env, ppcemb_tlb_t *tlb, + hwaddr *raddrp, + target_ulong address, uint32_t pid, int i) { target_ulong mask; /* Check valid flag */ if (!(tlb->prot & PAGE_VALID)) { - return -1; + return false; } mask = ~(tlb->size - 1); qemu_log_mask(CPU_LOG_MMU, "%s: TLB %d address " TARGET_FMT_lx @@ -506,14 +506,14 @@ static int ppcemb_tlb_check(CPUPPCState *env, ppcemb_tlb_t *tlb, mask, (uint32_t)tlb->PID, tlb->prot); /* Check PID */ if (tlb->PID != 0 && tlb->PID != pid) { - return -1; + return false; } /* Check effective address */ if ((address & mask) != tlb->EPN) { - return -1; + return false; } *raddrp = (tlb->RPN & mask) | (address & ~mask); - return 0; + return true; } /* Generic TLB search function for PowerPC embedded implementations */ @@ -525,7 +525,7 @@ int ppcemb_tlb_search(CPUPPCState *env, target_ulong address, uint32_t pid) for (i = 0; i < env->nb_tlb; i++) { tlb = &env->tlb.tlbe[i]; - if (ppcemb_tlb_check(env, tlb, &raddr, address, pid, i) == 0) { + if (ppcemb_tlb_check(env, tlb, &raddr, address, pid, i)) { return i; } } @@ -545,8 +545,8 @@ static int mmu40x_get_physical_address(CPUPPCState *env, mmu_ctx_t *ctx, pr = FIELD_EX64(env->msr, MSR, PR); for (i = 0; i < env->nb_tlb; i++) { tlb = &env->tlb.tlbe[i]; - if (ppcemb_tlb_check(env, tlb, &raddr, address, - env->spr[SPR_40x_PID], i) < 0) { + if (!ppcemb_tlb_check(env, tlb, &raddr, address, + env->spr[SPR_40x_PID], i)) { continue; } zsel = (tlb->attr >> 4) & 0xF; @@ -608,7 +608,7 @@ static int mmubooke_check_tlb(CPUPPCState *env, ppcemb_tlb_t *tlb, int prot2; if (ppcemb_tlb_check(env, tlb, raddr, address, - env->spr[SPR_BOOKE_PID], i) >= 0) { + env->spr[SPR_BOOKE_PID], i)) { if (!env->nb_pids) { /* Extend the physical address to 36 bits */ *raddr |= (uint64_t)(tlb->RPN & 0xF) << 32; @@ -618,13 +618,13 @@ static int mmubooke_check_tlb(CPUPPCState *env, ppcemb_tlb_t *tlb, if (env->spr[SPR_BOOKE_PID1] && ppcemb_tlb_check(env, tlb, raddr, address, - env->spr[SPR_BOOKE_PID1], i) >= 0) { + env->spr[SPR_BOOKE_PID1], i)) { goto found_tlb; } if (env->spr[SPR_BOOKE_PID2] && ppcemb_tlb_check(env, tlb, raddr, address, - env->spr[SPR_BOOKE_PID2], i) >= 0) { + env->spr[SPR_BOOKE_PID2], i)) { goto found_tlb; } From a5436bc6ed4d385690546783f1799ea927f5ebd2 Mon Sep 17 00:00:00 2001 From: BALATON Zoltan <balaton@eik.bme.hu> Date: Tue, 30 May 2023 15:28:13 +0200 Subject: [PATCH 19/29] target/ppc: Eliminate goto in mmubooke_check_tlb() Move out checking PID registers into a separate function which makes mmubooke_check_tlb() simpler and avoids using goto. Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> Message-Id: <bd84d5f38af0ba2983ccd5c07635db49267c828f.1685448535.git.balaton@eik.bme.hu> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- target/ppc/mmu_common.c | 50 +++++++++++++++++++++-------------------- 1 file changed, 26 insertions(+), 24 deletions(-) diff --git a/target/ppc/mmu_common.c b/target/ppc/mmu_common.c index bd7d7d5257..ae1db6e348 100644 --- a/target/ppc/mmu_common.c +++ b/target/ppc/mmu_common.c @@ -601,38 +601,40 @@ static int mmu40x_get_physical_address(CPUPPCState *env, mmu_ctx_t *ctx, return ret; } +static bool mmubooke_check_pid(CPUPPCState *env, ppcemb_tlb_t *tlb, + hwaddr *raddr, target_ulong addr, int i) +{ + if (ppcemb_tlb_check(env, tlb, raddr, addr, env->spr[SPR_BOOKE_PID], i)) { + if (!env->nb_pids) { + /* Extend the physical address to 36 bits */ + *raddr |= (uint64_t)(tlb->RPN & 0xF) << 32; + } + return true; + } else if (!env->nb_pids) { + return false; + } + if (env->spr[SPR_BOOKE_PID1] && + ppcemb_tlb_check(env, tlb, raddr, addr, env->spr[SPR_BOOKE_PID1], i)) { + return true; + } + if (env->spr[SPR_BOOKE_PID2] && + ppcemb_tlb_check(env, tlb, raddr, addr, env->spr[SPR_BOOKE_PID2], i)) { + return true; + } + return false; +} + static int mmubooke_check_tlb(CPUPPCState *env, ppcemb_tlb_t *tlb, hwaddr *raddr, int *prot, target_ulong address, MMUAccessType access_type, int i) { int prot2; - if (ppcemb_tlb_check(env, tlb, raddr, address, - env->spr[SPR_BOOKE_PID], i)) { - if (!env->nb_pids) { - /* Extend the physical address to 36 bits */ - *raddr |= (uint64_t)(tlb->RPN & 0xF) << 32; - } - goto found_tlb; + if (!mmubooke_check_pid(env, tlb, raddr, address, i)) { + qemu_log_mask(CPU_LOG_MMU, "%s: TLB entry not found\n", __func__); + return -1; } - if (env->spr[SPR_BOOKE_PID1] && - ppcemb_tlb_check(env, tlb, raddr, address, - env->spr[SPR_BOOKE_PID1], i)) { - goto found_tlb; - } - - if (env->spr[SPR_BOOKE_PID2] && - ppcemb_tlb_check(env, tlb, raddr, address, - env->spr[SPR_BOOKE_PID2], i)) { - goto found_tlb; - } - - qemu_log_mask(CPU_LOG_MMU, "%s: TLB entry not found\n", __func__); - return -1; - -found_tlb: - if (FIELD_EX64(env->msr, MSR, PR)) { prot2 = tlb->prot & 0xF; } else { From e025e8f5a8a7e32409bb4c7c509d752486113188 Mon Sep 17 00:00:00 2001 From: Nicholas Piggin <npiggin@gmail.com> Date: Mon, 5 Jun 2023 12:54:42 +1000 Subject: [PATCH 20/29] target/ppc: Fix lqarx to set cpu_reserve lqarx does not set cpu_reserve, which causes stqcx. to never succeed. Cc: qemu-stable@nongnu.org Fixes: 94bf2658676 ("target/ppc: Use atomic load for LQ and LQARX") Fixes: 57b38ffd0c6 ("target/ppc: Use tcg_gen_qemu_{ld,st}_i128 for LQARX, LQ, STQ") Signed-off-by: Nicholas Piggin <npiggin@gmail.com> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-Id: <20230605025445.161932-1-npiggin@gmail.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- target/ppc/translate.c | 1 + 1 file changed, 1 insertion(+) diff --git a/target/ppc/translate.c b/target/ppc/translate.c index 37fd431870..452439b729 100644 --- a/target/ppc/translate.c +++ b/target/ppc/translate.c @@ -3765,6 +3765,7 @@ static void gen_lqarx(DisasContext *ctx) tcg_gen_qemu_ld_i128(t16, EA, ctx->mem_idx, DEF_MEMOP(MO_128 | MO_ALIGN)); tcg_gen_extr_i128_i64(lo, hi, t16); + tcg_gen_mov_tl(cpu_reserve, EA); tcg_gen_st_tl(hi, cpu_env, offsetof(CPUPPCState, reserve_val)); tcg_gen_st_tl(lo, cpu_env, offsetof(CPUPPCState, reserve_val2)); } From 392d328abe7534a6e852151740913e2cd50426bc Mon Sep 17 00:00:00 2001 From: Nicholas Piggin <npiggin@gmail.com> Date: Mon, 5 Jun 2023 12:54:43 +1000 Subject: [PATCH 21/29] target/ppc: Ensure stcx size matches larx Differently-sized larx/stcx. pairs can succeed if the starting address matches. Add a check to require the size of stcx. exactly match the larx that established the reservation. Use the term "reserve_length" for this state, which matches the terminology used in the ISA. Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Nicholas Piggin <npiggin@gmail.com> Message-Id: <20230605025445.161932-2-npiggin@gmail.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- target/ppc/cpu.h | 5 +++-- target/ppc/cpu_init.c | 4 ++-- target/ppc/translate.c | 9 +++++++++ 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h index c7c2a5534c..20508bac5e 100644 --- a/target/ppc/cpu.h +++ b/target/ppc/cpu.h @@ -1114,8 +1114,9 @@ struct CPUArchState { target_ulong ov32; target_ulong ca32; - target_ulong reserve_addr; /* Reservation address */ - target_ulong reserve_val; /* Reservation value */ + target_ulong reserve_addr; /* Reservation address */ + target_ulong reserve_length; /* Reservation larx op size (bytes) */ + target_ulong reserve_val; /* Reservation value */ target_ulong reserve_val2; /* These are used in supervisor mode only */ diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c index 398f2d9966..d4ef074afb 100644 --- a/target/ppc/cpu_init.c +++ b/target/ppc/cpu_init.c @@ -7392,8 +7392,8 @@ void ppc_cpu_dump_state(CPUState *cs, FILE *f, int flags) } qemu_fprintf(f, " %c%c", a, env->crf[i] & 0x01 ? 'O' : ' '); } - qemu_fprintf(f, " ] RES " TARGET_FMT_lx "\n", - env->reserve_addr); + qemu_fprintf(f, " ] RES %03x@" TARGET_FMT_lx "\n", + (int)env->reserve_length, env->reserve_addr); if (flags & CPU_DUMP_FPU) { for (i = 0; i < 32; i++) { diff --git a/target/ppc/translate.c b/target/ppc/translate.c index 452439b729..cf0bd79b8c 100644 --- a/target/ppc/translate.c +++ b/target/ppc/translate.c @@ -75,6 +75,7 @@ static TCGv cpu_cfar; #endif static TCGv cpu_xer, cpu_so, cpu_ov, cpu_ca, cpu_ov32, cpu_ca32; static TCGv cpu_reserve; +static TCGv cpu_reserve_length; static TCGv cpu_reserve_val; static TCGv cpu_reserve_val2; static TCGv cpu_fpscr; @@ -143,6 +144,10 @@ void ppc_translate_init(void) cpu_reserve = tcg_global_mem_new(cpu_env, offsetof(CPUPPCState, reserve_addr), "reserve_addr"); + cpu_reserve_length = tcg_global_mem_new(cpu_env, + offsetof(CPUPPCState, + reserve_length), + "reserve_length"); cpu_reserve_val = tcg_global_mem_new(cpu_env, offsetof(CPUPPCState, reserve_val), "reserve_val"); @@ -3469,6 +3474,7 @@ static void gen_load_locked(DisasContext *ctx, MemOp memop) gen_addr_reg_index(ctx, t0); tcg_gen_qemu_ld_tl(gpr, t0, ctx->mem_idx, memop | MO_ALIGN); tcg_gen_mov_tl(cpu_reserve, t0); + tcg_gen_movi_tl(cpu_reserve_length, memop_size(memop)); tcg_gen_mov_tl(cpu_reserve_val, gpr); tcg_gen_mb(TCG_MO_ALL | TCG_BAR_LDAQ); } @@ -3700,6 +3706,7 @@ static void gen_conditional_store(DisasContext *ctx, MemOp memop) gen_set_access_type(ctx, ACCESS_RES); gen_addr_reg_index(ctx, t0); tcg_gen_brcond_tl(TCG_COND_NE, t0, cpu_reserve, l1); + tcg_gen_brcondi_tl(TCG_COND_NE, cpu_reserve_length, memop_size(memop), l1); t0 = tcg_temp_new(); tcg_gen_atomic_cmpxchg_tl(t0, cpu_reserve, cpu_reserve_val, @@ -3766,6 +3773,7 @@ static void gen_lqarx(DisasContext *ctx) tcg_gen_extr_i128_i64(lo, hi, t16); tcg_gen_mov_tl(cpu_reserve, EA); + tcg_gen_movi_tl(cpu_reserve_length, 16); tcg_gen_st_tl(hi, cpu_env, offsetof(CPUPPCState, reserve_val)); tcg_gen_st_tl(lo, cpu_env, offsetof(CPUPPCState, reserve_val2)); } @@ -3791,6 +3799,7 @@ static void gen_stqcx_(DisasContext *ctx) gen_addr_reg_index(ctx, EA); tcg_gen_brcond_tl(TCG_COND_NE, EA, cpu_reserve, lab_fail); + tcg_gen_brcondi_tl(TCG_COND_NE, cpu_reserve_length, 16, lab_fail); cmp = tcg_temp_new_i128(); val = tcg_temp_new_i128(); From 2c901dca1863515bd71c3f351610f0698cb8f0b4 Mon Sep 17 00:00:00 2001 From: Nicholas Piggin <npiggin@gmail.com> Date: Mon, 5 Jun 2023 12:54:44 +1000 Subject: [PATCH 22/29] target/ppc: Remove larx/stcx. memory barrier semantics larx and stcx. are not defined to order any memory operations. Remove the barriers. Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Nicholas Piggin <npiggin@gmail.com> Message-Id: <20230605025445.161932-3-npiggin@gmail.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- target/ppc/translate.c | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/target/ppc/translate.c b/target/ppc/translate.c index cf0bd79b8c..cb4764476d 100644 --- a/target/ppc/translate.c +++ b/target/ppc/translate.c @@ -3476,7 +3476,6 @@ static void gen_load_locked(DisasContext *ctx, MemOp memop) tcg_gen_mov_tl(cpu_reserve, t0); tcg_gen_movi_tl(cpu_reserve_length, memop_size(memop)); tcg_gen_mov_tl(cpu_reserve_val, gpr); - tcg_gen_mb(TCG_MO_ALL | TCG_BAR_LDAQ); } #define LARX(name, memop) \ @@ -3720,11 +3719,6 @@ static void gen_conditional_store(DisasContext *ctx, MemOp memop) gen_set_label(l1); - /* - * Address mismatch implies failure. But we still need to provide - * the memory barrier semantics of the instruction. - */ - tcg_gen_mb(TCG_MO_ALL | TCG_BAR_STRL); tcg_gen_trunc_tl_i32(cpu_crf[0], cpu_so); gen_set_label(l2); @@ -3828,11 +3822,6 @@ static void gen_stqcx_(DisasContext *ctx) tcg_gen_br(lab_over); gen_set_label(lab_fail); - /* - * Address mismatch implies failure. But we still need to provide - * the memory barrier semantics of the instruction. - */ - tcg_gen_mb(TCG_MO_ALL | TCG_BAR_STRL); tcg_gen_trunc_tl_i32(cpu_crf[0], cpu_so); gen_set_label(lab_over); From 21ee07e7737a2dbba226a8d8192246e51855910d Mon Sep 17 00:00:00 2001 From: Nicholas Piggin <npiggin@gmail.com> Date: Mon, 5 Jun 2023 12:54:45 +1000 Subject: [PATCH 23/29] target/ppc: Rework store conditional to avoid branch Rework store conditional to avoid a branch in the success case. Change some of the variable names and layout while here so gen_conditional_store more closely matches gen_stqcx_. Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Nicholas Piggin <npiggin@gmail.com> Message-Id: <20230605025445.161932-4-npiggin@gmail.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- target/ppc/translate.c | 67 ++++++++++++++++++++---------------------- 1 file changed, 32 insertions(+), 35 deletions(-) diff --git a/target/ppc/translate.c b/target/ppc/translate.c index cb4764476d..b591f2e496 100644 --- a/target/ppc/translate.c +++ b/target/ppc/translate.c @@ -3697,31 +3697,32 @@ static void gen_stdat(DisasContext *ctx) static void gen_conditional_store(DisasContext *ctx, MemOp memop) { - TCGLabel *l1 = gen_new_label(); - TCGLabel *l2 = gen_new_label(); - TCGv t0 = tcg_temp_new(); - int reg = rS(ctx->opcode); - - gen_set_access_type(ctx, ACCESS_RES); - gen_addr_reg_index(ctx, t0); - tcg_gen_brcond_tl(TCG_COND_NE, t0, cpu_reserve, l1); - tcg_gen_brcondi_tl(TCG_COND_NE, cpu_reserve_length, memop_size(memop), l1); + TCGLabel *lfail; + TCGv EA; + TCGv cr0; + TCGv t0; + int rs = rS(ctx->opcode); + lfail = gen_new_label(); + EA = tcg_temp_new(); + cr0 = tcg_temp_new(); t0 = tcg_temp_new(); + + tcg_gen_mov_tl(cr0, cpu_so); + gen_set_access_type(ctx, ACCESS_RES); + gen_addr_reg_index(ctx, EA); + tcg_gen_brcond_tl(TCG_COND_NE, EA, cpu_reserve, lfail); + tcg_gen_brcondi_tl(TCG_COND_NE, cpu_reserve_length, memop_size(memop), lfail); + tcg_gen_atomic_cmpxchg_tl(t0, cpu_reserve, cpu_reserve_val, - cpu_gpr[reg], ctx->mem_idx, + cpu_gpr[rs], ctx->mem_idx, DEF_MEMOP(memop) | MO_ALIGN); tcg_gen_setcond_tl(TCG_COND_EQ, t0, t0, cpu_reserve_val); tcg_gen_shli_tl(t0, t0, CRF_EQ_BIT); - tcg_gen_or_tl(t0, t0, cpu_so); - tcg_gen_trunc_tl_i32(cpu_crf[0], t0); - tcg_gen_br(l2); + tcg_gen_or_tl(cr0, cr0, t0); - gen_set_label(l1); - - tcg_gen_trunc_tl_i32(cpu_crf[0], cpu_so); - - gen_set_label(l2); + gen_set_label(lfail); + tcg_gen_trunc_tl_i32(cpu_crf[0], cr0); tcg_gen_movi_tl(cpu_reserve, -1); } @@ -3775,25 +3776,26 @@ static void gen_lqarx(DisasContext *ctx) /* stqcx. */ static void gen_stqcx_(DisasContext *ctx) { - TCGLabel *lab_fail, *lab_over; - int rs = rS(ctx->opcode); + TCGLabel *lfail; TCGv EA, t0, t1; + TCGv cr0; TCGv_i128 cmp, val; + int rs = rS(ctx->opcode); if (unlikely(rs & 1)) { gen_inval_exception(ctx, POWERPC_EXCP_INVAL_INVAL); return; } - lab_fail = gen_new_label(); - lab_over = gen_new_label(); - - gen_set_access_type(ctx, ACCESS_RES); + lfail = gen_new_label(); EA = tcg_temp_new(); - gen_addr_reg_index(ctx, EA); + cr0 = tcg_temp_new(); - tcg_gen_brcond_tl(TCG_COND_NE, EA, cpu_reserve, lab_fail); - tcg_gen_brcondi_tl(TCG_COND_NE, cpu_reserve_length, 16, lab_fail); + tcg_gen_mov_tl(cr0, cpu_so); + gen_set_access_type(ctx, ACCESS_RES); + gen_addr_reg_index(ctx, EA); + tcg_gen_brcond_tl(TCG_COND_NE, EA, cpu_reserve, lfail); + tcg_gen_brcondi_tl(TCG_COND_NE, cpu_reserve_length, 16, lfail); cmp = tcg_temp_new_i128(); val = tcg_temp_new_i128(); @@ -3816,15 +3818,10 @@ static void gen_stqcx_(DisasContext *ctx) tcg_gen_setcondi_tl(TCG_COND_EQ, t0, t0, 0); tcg_gen_shli_tl(t0, t0, CRF_EQ_BIT); - tcg_gen_or_tl(t0, t0, cpu_so); - tcg_gen_trunc_tl_i32(cpu_crf[0], t0); + tcg_gen_or_tl(cr0, cr0, t0); - tcg_gen_br(lab_over); - gen_set_label(lab_fail); - - tcg_gen_trunc_tl_i32(cpu_crf[0], cpu_so); - - gen_set_label(lab_over); + gen_set_label(lfail); + tcg_gen_trunc_tl_i32(cpu_crf[0], cr0); tcg_gen_movi_tl(cpu_reserve, -1); } #endif /* defined(TARGET_PPC64) */ From 09d2db9f46e38e2da990df8ad914d735d764251a Mon Sep 17 00:00:00 2001 From: Nicholas Piggin <npiggin@gmail.com> Date: Tue, 30 May 2023 23:12:12 +1000 Subject: [PATCH 24/29] target/ppc: Fix decrementer time underflow and infinite timer loop It is possible to store a very large value to the decrementer that it does not raise the decrementer exception so the timer is scheduled, but the next time value wraps and is treated as in the past. This can occur if (u64)-1 is stored on a zero-triggered exception, or (u64)-1 is stored twice on an underflow-triggered exception, for example. If such a value is set in DECAR, it gets stored to the decrementer by the timer function, which then immediately causes another timer, which hangs QEMU. Clamp the decrementer to the implemented width, and use that as the value for the timer calculation, effectively preventing this overflow. Reported-by: sdicaro@DDCI.com Signed-off-by: Nicholas Piggin <npiggin@gmail.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> Message-Id: <20230530131214.373524-1-npiggin@gmail.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- hw/ppc/ppc.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c index 4e816c68c7..d80b0adc6c 100644 --- a/hw/ppc/ppc.c +++ b/hw/ppc/ppc.c @@ -798,6 +798,8 @@ static void __cpu_ppc_store_decr(PowerPCCPU *cpu, uint64_t *nextp, int64_t signed_decr; /* Truncate value to decr_width and sign extend for simplicity */ + value = extract64(value, 0, nr_bits); + decr = extract64(decr, 0, nr_bits); signed_value = sextract64(value, 0, nr_bits); signed_decr = sextract64(decr, 0, nr_bits); From 17dd1354c1d1aba9caf4af01e11aa7dbe128474f Mon Sep 17 00:00:00 2001 From: Nicholas Piggin <npiggin@gmail.com> Date: Tue, 30 May 2023 23:12:13 +1000 Subject: [PATCH 25/29] target/ppc: Decrementer fix BookE semantics The decrementer store function has logic that short-cuts the timer if a very small value is stored (0, 1, or 2) and raises an interrupt directly. There are two problem with this on BookE. First is that BookE says a decrementer interrupt should not be raised on a store of 0, only of a decrement from 1. Second is that raising the irq directly will bypass the auto-reload logic in the booke decr timer function, breaking autoreload when 1 or 2 is stored. Fix this by removing that small-value special case. It makes this tricky logic even more difficult to reason about, and it hardly matters for performance. Cc: sdicaro@DDCI.com Signed-off-by: Nicholas Piggin <npiggin@gmail.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> Message-Id: <20230530131214.373524-2-npiggin@gmail.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- hw/ppc/ppc.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c index d80b0adc6c..1b1220c423 100644 --- a/hw/ppc/ppc.c +++ b/hw/ppc/ppc.c @@ -811,11 +811,7 @@ static void __cpu_ppc_store_decr(PowerPCCPU *cpu, uint64_t *nextp, } /* - * Going from 2 -> 1, 1 -> 0 or 0 -> -1 is the event to generate a DEC - * interrupt. - * - * If we get a really small DEC value, we can assume that by the time we - * handled it we should inject an interrupt already. + * Going from 1 -> 0 or 0 -> -1 is the event to generate a DEC interrupt. * * On MSB level based DEC implementations the MSB always means the interrupt * is pending, so raise it on those. @@ -823,8 +819,7 @@ static void __cpu_ppc_store_decr(PowerPCCPU *cpu, uint64_t *nextp, * On MSB edge based DEC implementations the MSB going from 0 -> 1 triggers * an edge interrupt, so raise it here too. */ - if ((value < 3) || - ((tb_env->flags & PPC_DECR_UNDERFLOW_LEVEL) && signed_value < 0) || + if (((tb_env->flags & PPC_DECR_UNDERFLOW_LEVEL) && signed_value < 0) || ((tb_env->flags & PPC_DECR_UNDERFLOW_TRIGGERED) && signed_value < 0 && signed_decr >= 0)) { (*raise_excp)(cpu); From 8e67403a2cdde2dd43b43683f4478f3c8ed07cf4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= <philmd@linaro.org> Date: Tue, 23 May 2023 08:15:46 +0200 Subject: [PATCH 26/29] hw/ppc/openpic: Do not open-code ROUND_UP() macro MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit While reviewing, the ROUND_UP() macro is easier to figure out. Besides, the comment confirms we want to round up here. Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> Message-Id: <20230523061546.49031-1-philmd@linaro.org> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- include/hw/ppc/openpic.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/hw/ppc/openpic.h b/include/hw/ppc/openpic.h index ebdaf8a493..bae8dafe16 100644 --- a/include/hw/ppc/openpic.h +++ b/include/hw/ppc/openpic.h @@ -55,7 +55,7 @@ typedef enum IRQType { * Round up to the nearest 64 IRQs so that the queue length * won't change when moving between 32 and 64 bit hosts. */ -#define IRQQUEUE_SIZE_BITS ((OPENPIC_MAX_IRQ + 63) & ~63) +#define IRQQUEUE_SIZE_BITS ROUND_UP(OPENPIC_MAX_IRQ, 64) typedef struct IRQQueue { unsigned long *queue; From 12cae32fe1a18a4e89472e7f92b06a4eb1beb671 Mon Sep 17 00:00:00 2001 From: Thomas Huth <thuth@redhat.com> Date: Tue, 6 Jun 2023 21:28:02 +0200 Subject: [PATCH 27/29] tests/avocado/tuxrun_baselines: Fix ppc64 tests for binaries without slirp MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ppc64 tuxrun tests are currently failing if "slirp" has been disabled in the binary since they are using "-netdev user" now. We have to skip the test if this network backend is missing. Fixes: 6ee3624236 ("improve code coverage for ppc64") Signed-off-by: Thomas Huth <thuth@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> Acked-by: Alex Bennée <alex.bennee@linaro.org> Message-Id: <20230606192802.666000-1-thuth@redhat.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- tests/avocado/tuxrun_baselines.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/avocado/tuxrun_baselines.py b/tests/avocado/tuxrun_baselines.py index 3a46e7a745..e12250eabb 100644 --- a/tests/avocado/tuxrun_baselines.py +++ b/tests/avocado/tuxrun_baselines.py @@ -184,6 +184,7 @@ class TuxRunBaselineTest(QemuSystemTest): def ppc64_common_tuxrun(self, sums, prefix): # add device args to command line. + self.require_netdev('user') self.vm.add_args('-netdev', 'user,id=vnet,hostfwd=:127.0.0.1:0-:22', '-device', 'virtio-net,netdev=vnet') self.vm.add_args('-netdev', '{"type":"user","id":"hostnet0"}', From 8a15ccee4d59b74be551dc7956d78bde2a52764a Mon Sep 17 00:00:00 2001 From: BALATON Zoltan <balaton@eik.bme.hu> Date: Wed, 7 Jun 2023 00:02:00 +0200 Subject: [PATCH 28/29] target/ppc: Implement gathering irq statistics MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Count exceptions which can be queried with info irq monitor command. Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Message-Id: <20230606220200.7EBCC74635C@zero.eik.bme.hu> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- target/ppc/cpu.h | 1 + target/ppc/cpu_init.c | 18 ++++++++++++++++++ target/ppc/excp_helper.c | 1 + 3 files changed, 20 insertions(+) diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h index 20508bac5e..0ee2adc105 100644 --- a/target/ppc/cpu.h +++ b/target/ppc/cpu.h @@ -1195,6 +1195,7 @@ struct CPUArchState { int error_code; uint32_t pending_interrupts; #if !defined(CONFIG_USER_ONLY) + uint64_t excp_stats[POWERPC_EXCP_NB]; /* * This is the IRQ controller, which is implementation dependent and only * relevant when emulating a complete machine. Note that this isn't used diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c index d4ef074afb..9f97222655 100644 --- a/target/ppc/cpu_init.c +++ b/target/ppc/cpu_init.c @@ -48,6 +48,7 @@ #ifndef CONFIG_USER_ONLY #include "hw/boards.h" +#include "hw/intc/intc.h" #endif /* #define PPC_DEBUG_SPR */ @@ -7123,6 +7124,16 @@ static bool ppc_cpu_is_big_endian(CPUState *cs) return !FIELD_EX64(env->msr, MSR, LE); } +static bool ppc_get_irq_stats(InterruptStatsProvider *obj, + uint64_t **irq_counts, unsigned int *nb_irqs) +{ + CPUPPCState *env = &POWERPC_CPU(obj)->env; + + *irq_counts = env->excp_stats; + *nb_irqs = ARRAY_SIZE(env->excp_stats); + return true; +} + #ifdef CONFIG_TCG static void ppc_cpu_exec_enter(CPUState *cs) { @@ -7286,6 +7297,7 @@ static void ppc_cpu_class_init(ObjectClass *oc, void *data) cc->gdb_write_register = ppc_cpu_gdb_write_register; #ifndef CONFIG_USER_ONLY cc->sysemu_ops = &ppc_sysemu_ops; + INTERRUPT_STATS_PROVIDER_CLASS(oc)->get_statistics = ppc_get_irq_stats; #endif cc->gdb_num_core_regs = 71; @@ -7323,6 +7335,12 @@ static const TypeInfo ppc_cpu_type_info = { .abstract = true, .class_size = sizeof(PowerPCCPUClass), .class_init = ppc_cpu_class_init, +#ifndef CONFIG_USER_ONLY + .interfaces = (InterfaceInfo[]) { + { TYPE_INTERRUPT_STATS_PROVIDER }, + { } + }, +#endif }; #ifndef CONFIG_USER_ONLY diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c index 8b95410c36..12d8a7257b 100644 --- a/target/ppc/excp_helper.c +++ b/target/ppc/excp_helper.c @@ -1655,6 +1655,7 @@ static void powerpc_excp(PowerPCCPU *cpu, int excp) qemu_log_mask(CPU_LOG_INT, "Raise exception at " TARGET_FMT_lx " => %s (%d) error=%02x\n", env->nip, powerpc_excp_name(excp), excp, env->error_code); + env->excp_stats[excp]++; switch (env->excp_model) { case POWERPC_EXCP_40x: From 9ec08f3569be3bc8bfd4d9b8b0445b9136910661 Mon Sep 17 00:00:00 2001 From: Thomas Huth <thuth@redhat.com> Date: Tue, 30 May 2023 12:20:41 +0200 Subject: [PATCH 29/29] hw/ppc/Kconfig: MAC_NEWWORLD should always select USB_OHCI_PCI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The PowerMacs have an OHCI controller soldered on the motherboard, so this should always be enabled for the "mac99" machine. This fixes the problem that QEMU aborts when the user tries to run the "mac99" machine with a build that has been compiled with the "--without-default-devices" configure switch. Signed-off-by: Thomas Huth <thuth@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> Message-Id: <20230530102041.55527-1-thuth@redhat.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- hw/ppc/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig index a689d9b219..5dfbf47ef5 100644 --- a/hw/ppc/Kconfig +++ b/hw/ppc/Kconfig @@ -115,6 +115,7 @@ config MAC_NEWWORLD select MAC_PMU select UNIN_PCI select FW_CFG_PPC + select USB_OHCI_PCI config E500 bool