Two small bugfixes, plus most of RTH's refactoring of cpregs handling. -- PMM The following changes since commit 1fba9dc71a170b3a05b9d3272dd8ecfe7f26e215: Merge tag 'pull-request-2022-05-04' of https://gitlab.com/thuth/qemu into staging (2022-05-04 08:07:02 -0700) are available in the Git repository at: https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20220505 for you to fetch changes up to 99a50d1a67c602126fc2b3a4812d3000eba9bf34: target/arm: read access to performance counters from EL0 (2022-05-05 09:36:22 +0100) ---------------------------------------------------------------- target-arm queue: * Enable read access to performance counters from EL0 * Enable SCTLR_EL1.BT0 for aarch64-linux-user * Refactoring of cpreg handling ---------------------------------------------------------------- Alex Zuepke (1): target/arm: read access to performance counters from EL0 Richard Henderson (22): target/arm: Enable SCTLR_EL1.BT0 for aarch64-linux-user target/arm: Split out cpregs.h target/arm: Reorg CPAccessResult and access_check_cp_reg target/arm: Replace sentinels with ARRAY_SIZE in cpregs.h target/arm: Make some more cpreg data static const target/arm: Reorg ARMCPRegInfo type field bits target/arm: Avoid bare abort() or assert(0) target/arm: Change cpreg access permissions to enum target/arm: Name CPState type target/arm: Name CPSecureState type target/arm: Drop always-true test in define_arm_vh_e2h_redirects_aliases target/arm: Store cpregs key in the hash table directly target/arm: Merge allocation of the cpreg and its name target/arm: Hoist computation of key in add_cpreg_to_hashtable target/arm: Consolidate cpreg updates in add_cpreg_to_hashtable target/arm: Use bool for is64 and ns in add_cpreg_to_hashtable target/arm: Hoist isbanked computation in add_cpreg_to_hashtable target/arm: Perform override check early in add_cpreg_to_hashtable target/arm: Reformat comments in add_cpreg_to_hashtable target/arm: Remove HOST_BIG_ENDIAN ifdef in add_cpreg_to_hashtable target/arm: Add isar predicates for FEAT_Debugv8p2 target/arm: Add isar_feature_{aa64,any}_ras target/arm/cpregs.h | 453 ++++++++++++++++++++++++++++++++++++++ target/arm/cpu.h | 393 +++------------------------------ hw/arm/pxa2xx.c | 2 +- hw/arm/pxa2xx_pic.c | 2 +- hw/intc/arm_gicv3_cpuif.c | 6 +- hw/intc/arm_gicv3_kvm.c | 3 +- target/arm/cpu.c | 25 +-- target/arm/cpu64.c | 2 +- target/arm/cpu_tcg.c | 5 +- target/arm/gdbstub.c | 5 +- target/arm/helper.c | 358 +++++++++++++----------------- target/arm/hvf/hvf.c | 2 +- target/arm/kvm-stub.c | 4 +- target/arm/kvm.c | 4 +- target/arm/machine.c | 4 +- target/arm/op_helper.c | 57 ++--- target/arm/translate-a64.c | 14 +- target/arm/translate-neon.c | 2 +- target/arm/translate.c | 13 +- tests/tcg/aarch64/bti-3.c | 42 ++++ tests/tcg/aarch64/Makefile.target | 6 +- 21 files changed, 738 insertions(+), 664 deletions(-) create mode 100644 target/arm/cpregs.h create mode 100644 tests/tcg/aarch64/bti-3.c From: Richard Henderson <richard.henderson@linaro.org> This controls whether the PACI{A,B}SP instructions trap with BTYPE=3 (indirect branch from register other than x16/x17). The linux kernel sets this in bti_enable(). Resolves: https://gitlab.com/qemu-project/qemu/-/issues/998 Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 20220427042312.294300-1-richard.henderson@linaro.org [PMM: remove stray change to makefile comment] Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/cpu.c | 2 ++ tests/tcg/aarch64/bti-3.c | 42 +++++++++++++++++++++++++++++++ tests/tcg/aarch64/Makefile.target | 6 ++--- 3 files changed, 47 insertions(+), 3 deletions(-) create mode 100644 tests/tcg/aarch64/bti-3.c diff --git a/target/arm/cpu.c b/target/arm/cpu.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu.c +++ b/target/arm/cpu.c @@ -XXX,XX +XXX,XX @@ static void arm_cpu_reset(DeviceState *dev) /* Enable all PAC keys. */ env->cp15.sctlr_el[1] |= (SCTLR_EnIA | SCTLR_EnIB | SCTLR_EnDA | SCTLR_EnDB); + /* Trap on btype=3 for PACIxSP. */ + env->cp15.sctlr_el[1] |= SCTLR_BT0; /* and to the FP/Neon instructions */ env->cp15.cpacr_el1 = deposit64(env->cp15.cpacr_el1, 20, 2, 3); /* and to the SVE instructions */ diff --git a/tests/tcg/aarch64/bti-3.c b/tests/tcg/aarch64/bti-3.c new file mode 100644 index XXXXXXX..XXXXXXX --- /dev/null +++ b/tests/tcg/aarch64/bti-3.c @@ -XXX,XX +XXX,XX @@ +/* + * BTI vs PACIASP + */ + +#include "bti-crt.inc.c" + +static void skip2_sigill(int sig, siginfo_t *info, ucontext_t *uc) +{ + uc->uc_mcontext.pc += 8; + uc->uc_mcontext.pstate = 1; +} + +#define BTYPE_1() \ + asm("mov %0,#1; adr x16, 1f; br x16; 1: hint #25; mov %0,#0" \ + : "=r"(skipped) : : "x16", "x30") + +#define BTYPE_2() \ + asm("mov %0,#1; adr x16, 1f; blr x16; 1: hint #25; mov %0,#0" \ + : "=r"(skipped) : : "x16", "x30") + +#define BTYPE_3() \ + asm("mov %0,#1; adr x15, 1f; br x15; 1: hint #25; mov %0,#0" \ + : "=r"(skipped) : : "x15", "x30") + +#define TEST(WHICH, EXPECT) \ + do { WHICH(); fail += skipped ^ EXPECT; } while (0) + +int main() +{ + int fail = 0; + int skipped; + + /* Signal-like with SA_SIGINFO. */ + signal_info(SIGILL, skip2_sigill); + + /* With SCTLR_EL1.BT0 set, PACIASP is not compatible with type=3. */ + TEST(BTYPE_1, 0); + TEST(BTYPE_2, 0); + TEST(BTYPE_3, 1); + + return fail; +} diff --git a/tests/tcg/aarch64/Makefile.target b/tests/tcg/aarch64/Makefile.target index XXXXXXX..XXXXXXX 100644 --- a/tests/tcg/aarch64/Makefile.target +++ b/tests/tcg/aarch64/Makefile.target @@ -XXX,XX +XXX,XX @@ endif # BTI Tests # bti-1 tests the elf notes, so we require special compiler support. ifneq ($(CROSS_CC_HAS_ARMV8_BTI),) -AARCH64_TESTS += bti-1 -bti-1: CFLAGS += -mbranch-protection=standard -bti-1: LDFLAGS += -nostdlib +AARCH64_TESTS += bti-1 bti-3 +bti-1 bti-3: CFLAGS += -mbranch-protection=standard +bti-1 bti-3: LDFLAGS += -nostdlib endif # bti-2 tests PROT_BTI, so no special compiler support required. AARCH64_TESTS += bti-2 -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Move ARMCPRegInfo and all related declarations to a new internal header, out of the public cpu.h. Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220501055028.646596-2-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/cpregs.h | 413 +++++++++++++++++++++++++++++++++++++ target/arm/cpu.h | 368 --------------------------------- hw/arm/pxa2xx.c | 1 + hw/arm/pxa2xx_pic.c | 1 + hw/intc/arm_gicv3_cpuif.c | 1 + hw/intc/arm_gicv3_kvm.c | 2 + target/arm/cpu.c | 1 + target/arm/cpu64.c | 1 + target/arm/cpu_tcg.c | 1 + target/arm/gdbstub.c | 3 +- target/arm/helper.c | 1 + target/arm/op_helper.c | 1 + target/arm/translate-a64.c | 4 +- target/arm/translate.c | 3 +- 14 files changed, 427 insertions(+), 374 deletions(-) create mode 100644 target/arm/cpregs.h diff --git a/target/arm/cpregs.h b/target/arm/cpregs.h new file mode 100644 index XXXXXXX..XXXXXXX --- /dev/null +++ b/target/arm/cpregs.h @@ -XXX,XX +XXX,XX @@ +/* + * QEMU ARM CP Register access and descriptions + * + * Copyright (c) 2022 Linaro Ltd + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, see + * <http://www.gnu.org/licenses/gpl-2.0.html> + */ + +#ifndef TARGET_ARM_CPREGS_H +#define TARGET_ARM_CPREGS_H + +/* + * ARMCPRegInfo type field bits. If the SPECIAL bit is set this is a + * special-behaviour cp reg and bits [11..8] indicate what behaviour + * it has. Otherwise it is a simple cp reg, where CONST indicates that + * TCG can assume the value to be constant (ie load at translate time) + * and 64BIT indicates a 64 bit wide coprocessor register. SUPPRESS_TB_END + * indicates that the TB should not be ended after a write to this register + * (the default is that the TB ends after cp writes). OVERRIDE permits + * a register definition to override a previous definition for the + * same (cp, is64, crn, crm, opc1, opc2) tuple: either the new or the + * old must have the OVERRIDE bit set. + * ALIAS indicates that this register is an alias view of some underlying + * state which is also visible via another register, and that the other + * register is handling migration and reset; registers marked ALIAS will not be + * migrated but may have their state set by syncing of register state from KVM. + * NO_RAW indicates that this register has no underlying state and does not + * support raw access for state saving/loading; it will not be used for either + * migration or KVM state synchronization. (Typically this is for "registers" + * which are actually used as instructions for cache maintenance and so on.) + * IO indicates that this register does I/O and therefore its accesses + * need to be marked with gen_io_start() and also end the TB. In particular, + * registers which implement clocks or timers require this. + * RAISES_EXC is for when the read or write hook might raise an exception; + * the generated code will synchronize the CPU state before calling the hook + * so that it is safe for the hook to call raise_exception(). + * NEWEL is for writes to registers that might change the exception + * level - typically on older ARM chips. For those cases we need to + * re-read the new el when recomputing the translation flags. + */ +#define ARM_CP_SPECIAL 0x0001 +#define ARM_CP_CONST 0x0002 +#define ARM_CP_64BIT 0x0004 +#define ARM_CP_SUPPRESS_TB_END 0x0008 +#define ARM_CP_OVERRIDE 0x0010 +#define ARM_CP_ALIAS 0x0020 +#define ARM_CP_IO 0x0040 +#define ARM_CP_NO_RAW 0x0080 +#define ARM_CP_NOP (ARM_CP_SPECIAL | 0x0100) +#define ARM_CP_WFI (ARM_CP_SPECIAL | 0x0200) +#define ARM_CP_NZCV (ARM_CP_SPECIAL | 0x0300) +#define ARM_CP_CURRENTEL (ARM_CP_SPECIAL | 0x0400) +#define ARM_CP_DC_ZVA (ARM_CP_SPECIAL | 0x0500) +#define ARM_CP_DC_GVA (ARM_CP_SPECIAL | 0x0600) +#define ARM_CP_DC_GZVA (ARM_CP_SPECIAL | 0x0700) +#define ARM_LAST_SPECIAL ARM_CP_DC_GZVA +#define ARM_CP_FPU 0x1000 +#define ARM_CP_SVE 0x2000 +#define ARM_CP_NO_GDB 0x4000 +#define ARM_CP_RAISES_EXC 0x8000 +#define ARM_CP_NEWEL 0x10000 +/* Used only as a terminator for ARMCPRegInfo lists */ +#define ARM_CP_SENTINEL 0xfffff +/* Mask of only the flag bits in a type field */ +#define ARM_CP_FLAG_MASK 0x1f0ff + +/* + * Valid values for ARMCPRegInfo state field, indicating which of + * the AArch32 and AArch64 execution states this register is visible in. + * If the reginfo doesn't explicitly specify then it is AArch32 only. + * If the reginfo is declared to be visible in both states then a second + * reginfo is synthesised for the AArch32 view of the AArch64 register, + * such that the AArch32 view is the lower 32 bits of the AArch64 one. + * Note that we rely on the values of these enums as we iterate through + * the various states in some places. + */ +enum { + ARM_CP_STATE_AA32 = 0, + ARM_CP_STATE_AA64 = 1, + ARM_CP_STATE_BOTH = 2, +}; + +/* + * ARM CP register secure state flags. These flags identify security state + * attributes for a given CP register entry. + * The existence of both or neither secure and non-secure flags indicates that + * the register has both a secure and non-secure hash entry. A single one of + * these flags causes the register to only be hashed for the specified + * security state. + * Although definitions may have any combination of the S/NS bits, each + * registered entry will only have one to identify whether the entry is secure + * or non-secure. + */ +enum { + ARM_CP_SECSTATE_S = (1 << 0), /* bit[0]: Secure state register */ + ARM_CP_SECSTATE_NS = (1 << 1), /* bit[1]: Non-secure state register */ +}; + +/* + * Return true if cptype is a valid type field. This is used to try to + * catch errors where the sentinel has been accidentally left off the end + * of a list of registers. + */ +static inline bool cptype_valid(int cptype) +{ + return ((cptype & ~ARM_CP_FLAG_MASK) == 0) + || ((cptype & ARM_CP_SPECIAL) && + ((cptype & ~ARM_CP_FLAG_MASK) <= ARM_LAST_SPECIAL)); +} + +/* + * Access rights: + * We define bits for Read and Write access for what rev C of the v7-AR ARM ARM + * defines as PL0 (user), PL1 (fiq/irq/svc/abt/und/sys, ie privileged), and + * PL2 (hyp). The other level which has Read and Write bits is Secure PL1 + * (ie any of the privileged modes in Secure state, or Monitor mode). + * If a register is accessible in one privilege level it's always accessible + * in higher privilege levels too. Since "Secure PL1" also follows this rule + * (ie anything visible in PL2 is visible in S-PL1, some things are only + * visible in S-PL1) but "Secure PL1" is a bit of a mouthful, we bend the + * terminology a little and call this PL3. + * In AArch64 things are somewhat simpler as the PLx bits line up exactly + * with the ELx exception levels. + * + * If access permissions for a register are more complex than can be + * described with these bits, then use a laxer set of restrictions, and + * do the more restrictive/complex check inside a helper function. + */ +#define PL3_R 0x80 +#define PL3_W 0x40 +#define PL2_R (0x20 | PL3_R) +#define PL2_W (0x10 | PL3_W) +#define PL1_R (0x08 | PL2_R) +#define PL1_W (0x04 | PL2_W) +#define PL0_R (0x02 | PL1_R) +#define PL0_W (0x01 | PL1_W) + +/* + * For user-mode some registers are accessible to EL0 via a kernel + * trap-and-emulate ABI. In this case we define the read permissions + * as actually being PL0_R. However some bits of any given register + * may still be masked. + */ +#ifdef CONFIG_USER_ONLY +#define PL0U_R PL0_R +#else +#define PL0U_R PL1_R +#endif + +#define PL3_RW (PL3_R | PL3_W) +#define PL2_RW (PL2_R | PL2_W) +#define PL1_RW (PL1_R | PL1_W) +#define PL0_RW (PL0_R | PL0_W) + +typedef enum CPAccessResult { + /* Access is permitted */ + CP_ACCESS_OK = 0, + /* + * Access fails due to a configurable trap or enable which would + * result in a categorized exception syndrome giving information about + * the failing instruction (ie syndrome category 0x3, 0x4, 0x5, 0x6, + * 0xc or 0x18). The exception is taken to the usual target EL (EL1 or + * PL1 if in EL0, otherwise to the current EL). + */ + CP_ACCESS_TRAP = 1, + /* + * Access fails and results in an exception syndrome 0x0 ("uncategorized"). + * Note that this is not a catch-all case -- the set of cases which may + * result in this failure is specifically defined by the architecture. + */ + CP_ACCESS_TRAP_UNCATEGORIZED = 2, + /* As CP_ACCESS_TRAP, but for traps directly to EL2 or EL3 */ + CP_ACCESS_TRAP_EL2 = 3, + CP_ACCESS_TRAP_EL3 = 4, + /* As CP_ACCESS_UNCATEGORIZED, but for traps directly to EL2 or EL3 */ + CP_ACCESS_TRAP_UNCATEGORIZED_EL2 = 5, + CP_ACCESS_TRAP_UNCATEGORIZED_EL3 = 6, +} CPAccessResult; + +typedef struct ARMCPRegInfo ARMCPRegInfo; + +/* + * Access functions for coprocessor registers. These cannot fail and + * may not raise exceptions. + */ +typedef uint64_t CPReadFn(CPUARMState *env, const ARMCPRegInfo *opaque); +typedef void CPWriteFn(CPUARMState *env, const ARMCPRegInfo *opaque, + uint64_t value); +/* Access permission check functions for coprocessor registers. */ +typedef CPAccessResult CPAccessFn(CPUARMState *env, + const ARMCPRegInfo *opaque, + bool isread); +/* Hook function for register reset */ +typedef void CPResetFn(CPUARMState *env, const ARMCPRegInfo *opaque); + +#define CP_ANY 0xff + +/* Definition of an ARM coprocessor register */ +struct ARMCPRegInfo { + /* Name of register (useful mainly for debugging, need not be unique) */ + const char *name; + /* + * Location of register: coprocessor number and (crn,crm,opc1,opc2) + * tuple. Any of crm, opc1 and opc2 may be CP_ANY to indicate a + * 'wildcard' field -- any value of that field in the MRC/MCR insn + * will be decoded to this register. The register read and write + * callbacks will be passed an ARMCPRegInfo with the crn/crm/opc1/opc2 + * used by the program, so it is possible to register a wildcard and + * then behave differently on read/write if necessary. + * For 64 bit registers, only crm and opc1 are relevant; crn and opc2 + * must both be zero. + * For AArch64-visible registers, opc0 is also used. + * Since there are no "coprocessors" in AArch64, cp is purely used as a + * way to distinguish (for KVM's benefit) guest-visible system registers + * from demuxed ones provided to preserve the "no side effects on + * KVM register read/write from QEMU" semantics. cp==0x13 is guest + * visible (to match KVM's encoding); cp==0 will be converted to + * cp==0x13 when the ARMCPRegInfo is registered, for convenience. + */ + uint8_t cp; + uint8_t crn; + uint8_t crm; + uint8_t opc0; + uint8_t opc1; + uint8_t opc2; + /* Execution state in which this register is visible: ARM_CP_STATE_* */ + int state; + /* Register type: ARM_CP_* bits/values */ + int type; + /* Access rights: PL*_[RW] */ + int access; + /* Security state: ARM_CP_SECSTATE_* bits/values */ + int secure; + /* + * The opaque pointer passed to define_arm_cp_regs_with_opaque() when + * this register was defined: can be used to hand data through to the + * register read/write functions, since they are passed the ARMCPRegInfo*. + */ + void *opaque; + /* + * Value of this register, if it is ARM_CP_CONST. Otherwise, if + * fieldoffset is non-zero, the reset value of the register. + */ + uint64_t resetvalue; + /* + * Offset of the field in CPUARMState for this register. + * This is not needed if either: + * 1. type is ARM_CP_CONST or one of the ARM_CP_SPECIALs + * 2. both readfn and writefn are specified + */ + ptrdiff_t fieldoffset; /* offsetof(CPUARMState, field) */ + + /* + * Offsets of the secure and non-secure fields in CPUARMState for the + * register if it is banked. These fields are only used during the static + * registration of a register. During hashing the bank associated + * with a given security state is copied to fieldoffset which is used from + * there on out. + * + * It is expected that register definitions use either fieldoffset or + * bank_fieldoffsets in the definition but not both. It is also expected + * that both bank offsets are set when defining a banked register. This + * use indicates that a register is banked. + */ + ptrdiff_t bank_fieldoffsets[2]; + + /* + * Function for making any access checks for this register in addition to + * those specified by the 'access' permissions bits. If NULL, no extra + * checks required. The access check is performed at runtime, not at + * translate time. + */ + CPAccessFn *accessfn; + /* + * Function for handling reads of this register. If NULL, then reads + * will be done by loading from the offset into CPUARMState specified + * by fieldoffset. + */ + CPReadFn *readfn; + /* + * Function for handling writes of this register. If NULL, then writes + * will be done by writing to the offset into CPUARMState specified + * by fieldoffset. + */ + CPWriteFn *writefn; + /* + * Function for doing a "raw" read; used when we need to copy + * coprocessor state to the kernel for KVM or out for + * migration. This only needs to be provided if there is also a + * readfn and it has side effects (for instance clear-on-read bits). + */ + CPReadFn *raw_readfn; + /* + * Function for doing a "raw" write; used when we need to copy KVM + * kernel coprocessor state into userspace, or for inbound + * migration. This only needs to be provided if there is also a + * writefn and it masks out "unwritable" bits or has write-one-to-clear + * or similar behaviour. + */ + CPWriteFn *raw_writefn; + /* + * Function for resetting the register. If NULL, then reset will be done + * by writing resetvalue to the field specified in fieldoffset. If + * fieldoffset is 0 then no reset will be done. + */ + CPResetFn *resetfn; + + /* + * "Original" writefn and readfn. + * For ARMv8.1-VHE register aliases, we overwrite the read/write + * accessor functions of various EL1/EL0 to perform the runtime + * check for which sysreg should actually be modified, and then + * forwards the operation. Before overwriting the accessors, + * the original function is copied here, so that accesses that + * really do go to the EL1/EL0 version proceed normally. + * (The corresponding EL2 register is linked via opaque.) + */ + CPReadFn *orig_readfn; + CPWriteFn *orig_writefn; +}; + +/* + * Macros which are lvalues for the field in CPUARMState for the + * ARMCPRegInfo *ri. + */ +#define CPREG_FIELD32(env, ri) \ + (*(uint32_t *)((char *)(env) + (ri)->fieldoffset)) +#define CPREG_FIELD64(env, ri) \ + (*(uint64_t *)((char *)(env) + (ri)->fieldoffset)) + +#define REGINFO_SENTINEL { .type = ARM_CP_SENTINEL } + +void define_arm_cp_regs_with_opaque(ARMCPU *cpu, + const ARMCPRegInfo *regs, void *opaque); +void define_one_arm_cp_reg_with_opaque(ARMCPU *cpu, + const ARMCPRegInfo *regs, void *opaque); +static inline void define_arm_cp_regs(ARMCPU *cpu, const ARMCPRegInfo *regs) +{ + define_arm_cp_regs_with_opaque(cpu, regs, 0); +} +static inline void define_one_arm_cp_reg(ARMCPU *cpu, const ARMCPRegInfo *regs) +{ + define_one_arm_cp_reg_with_opaque(cpu, regs, 0); +} +const ARMCPRegInfo *get_arm_cp_reginfo(GHashTable *cpregs, uint32_t encoded_cp); + +/* + * Definition of an ARM co-processor register as viewed from + * userspace. This is used for presenting sanitised versions of + * registers to userspace when emulating the Linux AArch64 CPU + * ID/feature ABI (advertised as HWCAP_CPUID). + */ +typedef struct ARMCPRegUserSpaceInfo { + /* Name of register */ + const char *name; + + /* Is the name actually a glob pattern */ + bool is_glob; + + /* Only some bits are exported to user space */ + uint64_t exported_bits; + + /* Fixed bits are applied after the mask */ + uint64_t fixed_bits; +} ARMCPRegUserSpaceInfo; + +#define REGUSERINFO_SENTINEL { .name = NULL } + +void modify_arm_cp_regs(ARMCPRegInfo *regs, const ARMCPRegUserSpaceInfo *mods); + +/* CPWriteFn that can be used to implement writes-ignored behaviour */ +void arm_cp_write_ignore(CPUARMState *env, const ARMCPRegInfo *ri, + uint64_t value); +/* CPReadFn that can be used for read-as-zero behaviour */ +uint64_t arm_cp_read_zero(CPUARMState *env, const ARMCPRegInfo *ri); + +/* + * CPResetFn that does nothing, for use if no reset is required even + * if fieldoffset is non zero. + */ +void arm_cp_reset_ignore(CPUARMState *env, const ARMCPRegInfo *opaque); + +/* + * Return true if this reginfo struct's field in the cpu state struct + * is 64 bits wide. + */ +static inline bool cpreg_field_is_64bit(const ARMCPRegInfo *ri) +{ + return (ri->state == ARM_CP_STATE_AA64) || (ri->type & ARM_CP_64BIT); +} + +static inline bool cp_access_ok(int current_el, + const ARMCPRegInfo *ri, int isread) +{ + return (ri->access >> ((current_el * 2) + isread)) & 1; +} + +/* Raw read of a coprocessor register (as needed for migration, etc) */ +uint64_t read_raw_cp_reg(CPUARMState *env, const ARMCPRegInfo *ri); + +#endif /* TARGET_ARM_CPREGS_H */ diff --git a/target/arm/cpu.h b/target/arm/cpu.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -XXX,XX +XXX,XX @@ static inline uint64_t cpreg_to_kvm_id(uint32_t cpregid) return kvmid; } -/* ARMCPRegInfo type field bits. If the SPECIAL bit is set this is a - * special-behaviour cp reg and bits [11..8] indicate what behaviour - * it has. Otherwise it is a simple cp reg, where CONST indicates that - * TCG can assume the value to be constant (ie load at translate time) - * and 64BIT indicates a 64 bit wide coprocessor register. SUPPRESS_TB_END - * indicates that the TB should not be ended after a write to this register - * (the default is that the TB ends after cp writes). OVERRIDE permits - * a register definition to override a previous definition for the - * same (cp, is64, crn, crm, opc1, opc2) tuple: either the new or the - * old must have the OVERRIDE bit set. - * ALIAS indicates that this register is an alias view of some underlying - * state which is also visible via another register, and that the other - * register is handling migration and reset; registers marked ALIAS will not be - * migrated but may have their state set by syncing of register state from KVM. - * NO_RAW indicates that this register has no underlying state and does not - * support raw access for state saving/loading; it will not be used for either - * migration or KVM state synchronization. (Typically this is for "registers" - * which are actually used as instructions for cache maintenance and so on.) - * IO indicates that this register does I/O and therefore its accesses - * need to be marked with gen_io_start() and also end the TB. In particular, - * registers which implement clocks or timers require this. - * RAISES_EXC is for when the read or write hook might raise an exception; - * the generated code will synchronize the CPU state before calling the hook - * so that it is safe for the hook to call raise_exception(). - * NEWEL is for writes to registers that might change the exception - * level - typically on older ARM chips. For those cases we need to - * re-read the new el when recomputing the translation flags. - */ -#define ARM_CP_SPECIAL 0x0001 -#define ARM_CP_CONST 0x0002 -#define ARM_CP_64BIT 0x0004 -#define ARM_CP_SUPPRESS_TB_END 0x0008 -#define ARM_CP_OVERRIDE 0x0010 -#define ARM_CP_ALIAS 0x0020 -#define ARM_CP_IO 0x0040 -#define ARM_CP_NO_RAW 0x0080 -#define ARM_CP_NOP (ARM_CP_SPECIAL | 0x0100) -#define ARM_CP_WFI (ARM_CP_SPECIAL | 0x0200) -#define ARM_CP_NZCV (ARM_CP_SPECIAL | 0x0300) -#define ARM_CP_CURRENTEL (ARM_CP_SPECIAL | 0x0400) -#define ARM_CP_DC_ZVA (ARM_CP_SPECIAL | 0x0500) -#define ARM_CP_DC_GVA (ARM_CP_SPECIAL | 0x0600) -#define ARM_CP_DC_GZVA (ARM_CP_SPECIAL | 0x0700) -#define ARM_LAST_SPECIAL ARM_CP_DC_GZVA -#define ARM_CP_FPU 0x1000 -#define ARM_CP_SVE 0x2000 -#define ARM_CP_NO_GDB 0x4000 -#define ARM_CP_RAISES_EXC 0x8000 -#define ARM_CP_NEWEL 0x10000 -/* Used only as a terminator for ARMCPRegInfo lists */ -#define ARM_CP_SENTINEL 0xfffff -/* Mask of only the flag bits in a type field */ -#define ARM_CP_FLAG_MASK 0x1f0ff - -/* Valid values for ARMCPRegInfo state field, indicating which of - * the AArch32 and AArch64 execution states this register is visible in. - * If the reginfo doesn't explicitly specify then it is AArch32 only. - * If the reginfo is declared to be visible in both states then a second - * reginfo is synthesised for the AArch32 view of the AArch64 register, - * such that the AArch32 view is the lower 32 bits of the AArch64 one. - * Note that we rely on the values of these enums as we iterate through - * the various states in some places. - */ -enum { - ARM_CP_STATE_AA32 = 0, - ARM_CP_STATE_AA64 = 1, - ARM_CP_STATE_BOTH = 2, -}; - -/* ARM CP register secure state flags. These flags identify security state - * attributes for a given CP register entry. - * The existence of both or neither secure and non-secure flags indicates that - * the register has both a secure and non-secure hash entry. A single one of - * these flags causes the register to only be hashed for the specified - * security state. - * Although definitions may have any combination of the S/NS bits, each - * registered entry will only have one to identify whether the entry is secure - * or non-secure. - */ -enum { - ARM_CP_SECSTATE_S = (1 << 0), /* bit[0]: Secure state register */ - ARM_CP_SECSTATE_NS = (1 << 1), /* bit[1]: Non-secure state register */ -}; - -/* Return true if cptype is a valid type field. This is used to try to - * catch errors where the sentinel has been accidentally left off the end - * of a list of registers. - */ -static inline bool cptype_valid(int cptype) -{ - return ((cptype & ~ARM_CP_FLAG_MASK) == 0) - || ((cptype & ARM_CP_SPECIAL) && - ((cptype & ~ARM_CP_FLAG_MASK) <= ARM_LAST_SPECIAL)); -} - -/* Access rights: - * We define bits for Read and Write access for what rev C of the v7-AR ARM ARM - * defines as PL0 (user), PL1 (fiq/irq/svc/abt/und/sys, ie privileged), and - * PL2 (hyp). The other level which has Read and Write bits is Secure PL1 - * (ie any of the privileged modes in Secure state, or Monitor mode). - * If a register is accessible in one privilege level it's always accessible - * in higher privilege levels too. Since "Secure PL1" also follows this rule - * (ie anything visible in PL2 is visible in S-PL1, some things are only - * visible in S-PL1) but "Secure PL1" is a bit of a mouthful, we bend the - * terminology a little and call this PL3. - * In AArch64 things are somewhat simpler as the PLx bits line up exactly - * with the ELx exception levels. - * - * If access permissions for a register are more complex than can be - * described with these bits, then use a laxer set of restrictions, and - * do the more restrictive/complex check inside a helper function. - */ -#define PL3_R 0x80 -#define PL3_W 0x40 -#define PL2_R (0x20 | PL3_R) -#define PL2_W (0x10 | PL3_W) -#define PL1_R (0x08 | PL2_R) -#define PL1_W (0x04 | PL2_W) -#define PL0_R (0x02 | PL1_R) -#define PL0_W (0x01 | PL1_W) - -/* - * For user-mode some registers are accessible to EL0 via a kernel - * trap-and-emulate ABI. In this case we define the read permissions - * as actually being PL0_R. However some bits of any given register - * may still be masked. - */ -#ifdef CONFIG_USER_ONLY -#define PL0U_R PL0_R -#else -#define PL0U_R PL1_R -#endif - -#define PL3_RW (PL3_R | PL3_W) -#define PL2_RW (PL2_R | PL2_W) -#define PL1_RW (PL1_R | PL1_W) -#define PL0_RW (PL0_R | PL0_W) - /* Return the highest implemented Exception Level */ static inline int arm_highest_el(CPUARMState *env) { @@ -XXX,XX +XXX,XX @@ static inline int arm_current_el(CPUARMState *env) } } -typedef struct ARMCPRegInfo ARMCPRegInfo; - -typedef enum CPAccessResult { - /* Access is permitted */ - CP_ACCESS_OK = 0, - /* Access fails due to a configurable trap or enable which would - * result in a categorized exception syndrome giving information about - * the failing instruction (ie syndrome category 0x3, 0x4, 0x5, 0x6, - * 0xc or 0x18). The exception is taken to the usual target EL (EL1 or - * PL1 if in EL0, otherwise to the current EL). - */ - CP_ACCESS_TRAP = 1, - /* Access fails and results in an exception syndrome 0x0 ("uncategorized"). - * Note that this is not a catch-all case -- the set of cases which may - * result in this failure is specifically defined by the architecture. - */ - CP_ACCESS_TRAP_UNCATEGORIZED = 2, - /* As CP_ACCESS_TRAP, but for traps directly to EL2 or EL3 */ - CP_ACCESS_TRAP_EL2 = 3, - CP_ACCESS_TRAP_EL3 = 4, - /* As CP_ACCESS_UNCATEGORIZED, but for traps directly to EL2 or EL3 */ - CP_ACCESS_TRAP_UNCATEGORIZED_EL2 = 5, - CP_ACCESS_TRAP_UNCATEGORIZED_EL3 = 6, -} CPAccessResult; - -/* Access functions for coprocessor registers. These cannot fail and - * may not raise exceptions. - */ -typedef uint64_t CPReadFn(CPUARMState *env, const ARMCPRegInfo *opaque); -typedef void CPWriteFn(CPUARMState *env, const ARMCPRegInfo *opaque, - uint64_t value); -/* Access permission check functions for coprocessor registers. */ -typedef CPAccessResult CPAccessFn(CPUARMState *env, - const ARMCPRegInfo *opaque, - bool isread); -/* Hook function for register reset */ -typedef void CPResetFn(CPUARMState *env, const ARMCPRegInfo *opaque); - -#define CP_ANY 0xff - -/* Definition of an ARM coprocessor register */ -struct ARMCPRegInfo { - /* Name of register (useful mainly for debugging, need not be unique) */ - const char *name; - /* Location of register: coprocessor number and (crn,crm,opc1,opc2) - * tuple. Any of crm, opc1 and opc2 may be CP_ANY to indicate a - * 'wildcard' field -- any value of that field in the MRC/MCR insn - * will be decoded to this register. The register read and write - * callbacks will be passed an ARMCPRegInfo with the crn/crm/opc1/opc2 - * used by the program, so it is possible to register a wildcard and - * then behave differently on read/write if necessary. - * For 64 bit registers, only crm and opc1 are relevant; crn and opc2 - * must both be zero. - * For AArch64-visible registers, opc0 is also used. - * Since there are no "coprocessors" in AArch64, cp is purely used as a - * way to distinguish (for KVM's benefit) guest-visible system registers - * from demuxed ones provided to preserve the "no side effects on - * KVM register read/write from QEMU" semantics. cp==0x13 is guest - * visible (to match KVM's encoding); cp==0 will be converted to - * cp==0x13 when the ARMCPRegInfo is registered, for convenience. - */ - uint8_t cp; - uint8_t crn; - uint8_t crm; - uint8_t opc0; - uint8_t opc1; - uint8_t opc2; - /* Execution state in which this register is visible: ARM_CP_STATE_* */ - int state; - /* Register type: ARM_CP_* bits/values */ - int type; - /* Access rights: PL*_[RW] */ - int access; - /* Security state: ARM_CP_SECSTATE_* bits/values */ - int secure; - /* The opaque pointer passed to define_arm_cp_regs_with_opaque() when - * this register was defined: can be used to hand data through to the - * register read/write functions, since they are passed the ARMCPRegInfo*. - */ - void *opaque; - /* Value of this register, if it is ARM_CP_CONST. Otherwise, if - * fieldoffset is non-zero, the reset value of the register. - */ - uint64_t resetvalue; - /* Offset of the field in CPUARMState for this register. - * - * This is not needed if either: - * 1. type is ARM_CP_CONST or one of the ARM_CP_SPECIALs - * 2. both readfn and writefn are specified - */ - ptrdiff_t fieldoffset; /* offsetof(CPUARMState, field) */ - - /* Offsets of the secure and non-secure fields in CPUARMState for the - * register if it is banked. These fields are only used during the static - * registration of a register. During hashing the bank associated - * with a given security state is copied to fieldoffset which is used from - * there on out. - * - * It is expected that register definitions use either fieldoffset or - * bank_fieldoffsets in the definition but not both. It is also expected - * that both bank offsets are set when defining a banked register. This - * use indicates that a register is banked. - */ - ptrdiff_t bank_fieldoffsets[2]; - - /* Function for making any access checks for this register in addition to - * those specified by the 'access' permissions bits. If NULL, no extra - * checks required. The access check is performed at runtime, not at - * translate time. - */ - CPAccessFn *accessfn; - /* Function for handling reads of this register. If NULL, then reads - * will be done by loading from the offset into CPUARMState specified - * by fieldoffset. - */ - CPReadFn *readfn; - /* Function for handling writes of this register. If NULL, then writes - * will be done by writing to the offset into CPUARMState specified - * by fieldoffset. - */ - CPWriteFn *writefn; - /* Function for doing a "raw" read; used when we need to copy - * coprocessor state to the kernel for KVM or out for - * migration. This only needs to be provided if there is also a - * readfn and it has side effects (for instance clear-on-read bits). - */ - CPReadFn *raw_readfn; - /* Function for doing a "raw" write; used when we need to copy KVM - * kernel coprocessor state into userspace, or for inbound - * migration. This only needs to be provided if there is also a - * writefn and it masks out "unwritable" bits or has write-one-to-clear - * or similar behaviour. - */ - CPWriteFn *raw_writefn; - /* Function for resetting the register. If NULL, then reset will be done - * by writing resetvalue to the field specified in fieldoffset. If - * fieldoffset is 0 then no reset will be done. - */ - CPResetFn *resetfn; - - /* - * "Original" writefn and readfn. - * For ARMv8.1-VHE register aliases, we overwrite the read/write - * accessor functions of various EL1/EL0 to perform the runtime - * check for which sysreg should actually be modified, and then - * forwards the operation. Before overwriting the accessors, - * the original function is copied here, so that accesses that - * really do go to the EL1/EL0 version proceed normally. - * (The corresponding EL2 register is linked via opaque.) - */ - CPReadFn *orig_readfn; - CPWriteFn *orig_writefn; -}; - -/* Macros which are lvalues for the field in CPUARMState for the - * ARMCPRegInfo *ri. - */ -#define CPREG_FIELD32(env, ri) \ - (*(uint32_t *)((char *)(env) + (ri)->fieldoffset)) -#define CPREG_FIELD64(env, ri) \ - (*(uint64_t *)((char *)(env) + (ri)->fieldoffset)) - -#define REGINFO_SENTINEL { .type = ARM_CP_SENTINEL } - -void define_arm_cp_regs_with_opaque(ARMCPU *cpu, - const ARMCPRegInfo *regs, void *opaque); -void define_one_arm_cp_reg_with_opaque(ARMCPU *cpu, - const ARMCPRegInfo *regs, void *opaque); -static inline void define_arm_cp_regs(ARMCPU *cpu, const ARMCPRegInfo *regs) -{ - define_arm_cp_regs_with_opaque(cpu, regs, 0); -} -static inline void define_one_arm_cp_reg(ARMCPU *cpu, const ARMCPRegInfo *regs) -{ - define_one_arm_cp_reg_with_opaque(cpu, regs, 0); -} -const ARMCPRegInfo *get_arm_cp_reginfo(GHashTable *cpregs, uint32_t encoded_cp); - -/* - * Definition of an ARM co-processor register as viewed from - * userspace. This is used for presenting sanitised versions of - * registers to userspace when emulating the Linux AArch64 CPU - * ID/feature ABI (advertised as HWCAP_CPUID). - */ -typedef struct ARMCPRegUserSpaceInfo { - /* Name of register */ - const char *name; - - /* Is the name actually a glob pattern */ - bool is_glob; - - /* Only some bits are exported to user space */ - uint64_t exported_bits; - - /* Fixed bits are applied after the mask */ - uint64_t fixed_bits; -} ARMCPRegUserSpaceInfo; - -#define REGUSERINFO_SENTINEL { .name = NULL } - -void modify_arm_cp_regs(ARMCPRegInfo *regs, const ARMCPRegUserSpaceInfo *mods); - -/* CPWriteFn that can be used to implement writes-ignored behaviour */ -void arm_cp_write_ignore(CPUARMState *env, const ARMCPRegInfo *ri, - uint64_t value); -/* CPReadFn that can be used for read-as-zero behaviour */ -uint64_t arm_cp_read_zero(CPUARMState *env, const ARMCPRegInfo *ri); - -/* CPResetFn that does nothing, for use if no reset is required even - * if fieldoffset is non zero. - */ -void arm_cp_reset_ignore(CPUARMState *env, const ARMCPRegInfo *opaque); - -/* Return true if this reginfo struct's field in the cpu state struct - * is 64 bits wide. - */ -static inline bool cpreg_field_is_64bit(const ARMCPRegInfo *ri) -{ - return (ri->state == ARM_CP_STATE_AA64) || (ri->type & ARM_CP_64BIT); -} - -static inline bool cp_access_ok(int current_el, - const ARMCPRegInfo *ri, int isread) -{ - return (ri->access >> ((current_el * 2) + isread)) & 1; -} - -/* Raw read of a coprocessor register (as needed for migration, etc) */ -uint64_t read_raw_cp_reg(CPUARMState *env, const ARMCPRegInfo *ri); - /** * write_list_to_cpustate * @cpu: ARMCPU diff --git a/hw/arm/pxa2xx.c b/hw/arm/pxa2xx.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/pxa2xx.c +++ b/hw/arm/pxa2xx.c @@ -XXX,XX +XXX,XX @@ #include "qemu/cutils.h" #include "qemu/log.h" #include "qom/object.h" +#include "target/arm/cpregs.h" static struct { hwaddr io_base; diff --git a/hw/arm/pxa2xx_pic.c b/hw/arm/pxa2xx_pic.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/pxa2xx_pic.c +++ b/hw/arm/pxa2xx_pic.c @@ -XXX,XX +XXX,XX @@ #include "hw/sysbus.h" #include "migration/vmstate.h" #include "qom/object.h" +#include "target/arm/cpregs.h" #define ICIP 0x00 /* Interrupt Controller IRQ Pending register */ #define ICMR 0x04 /* Interrupt Controller Mask register */ diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c index XXXXXXX..XXXXXXX 100644 --- a/hw/intc/arm_gicv3_cpuif.c +++ b/hw/intc/arm_gicv3_cpuif.c @@ -XXX,XX +XXX,XX @@ #include "gicv3_internal.h" #include "hw/irq.h" #include "cpu.h" +#include "target/arm/cpregs.h" /* * Special case return value from hppvi_index(); must be larger than diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c index XXXXXXX..XXXXXXX 100644 --- a/hw/intc/arm_gicv3_kvm.c +++ b/hw/intc/arm_gicv3_kvm.c @@ -XXX,XX +XXX,XX @@ #include "vgic_common.h" #include "migration/blocker.h" #include "qom/object.h" +#include "target/arm/cpregs.h" + #ifdef DEBUG_GICV3_KVM #define DPRINTF(fmt, ...) \ diff --git a/target/arm/cpu.c b/target/arm/cpu.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu.c +++ b/target/arm/cpu.c @@ -XXX,XX +XXX,XX @@ #include "kvm_arm.h" #include "disas/capstone.h" #include "fpu/softfloat.h" +#include "cpregs.h" static void arm_cpu_set_pc(CPUState *cs, vaddr value) { diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu64.c +++ b/target/arm/cpu64.c @@ -XXX,XX +XXX,XX @@ #include "hvf_arm.h" #include "qapi/visitor.h" #include "hw/qdev-properties.h" +#include "cpregs.h" #ifndef CONFIG_USER_ONLY diff --git a/target/arm/cpu_tcg.c b/target/arm/cpu_tcg.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu_tcg.c +++ b/target/arm/cpu_tcg.c @@ -XXX,XX +XXX,XX @@ #if !defined(CONFIG_USER_ONLY) #include "hw/boards.h" #endif +#include "cpregs.h" /* CPU models. These are not needed for the AArch64 linux-user build. */ #if !defined(CONFIG_USER_ONLY) || !defined(TARGET_AARCH64) diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/gdbstub.c +++ b/target/arm/gdbstub.c @@ -XXX,XX +XXX,XX @@ */ #include "qemu/osdep.h" #include "cpu.h" -#include "internals.h" #include "exec/gdbstub.h" +#include "internals.h" +#include "cpregs.h" typedef struct RegisterSysregXmlParam { CPUState *cs; diff --git a/target/arm/helper.c b/target/arm/helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -XXX,XX +XXX,XX @@ #include "exec/cpu_ldst.h" #include "semihosting/common-semi.h" #endif +#include "cpregs.h" #define ARM_CPU_FREQ 1000000000 /* FIXME: 1 GHz, should be configurable */ #define PMCR_NUM_COUNTERS 4 /* QEMU IMPDEF choice */ diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/op_helper.c +++ b/target/arm/op_helper.c @@ -XXX,XX +XXX,XX @@ #include "internals.h" #include "exec/exec-all.h" #include "exec/cpu_ldst.h" +#include "cpregs.h" #define SIGNBIT (uint32_t)0x80000000 #define SIGNBIT64 ((uint64_t)1 << 63) diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-a64.c +++ b/target/arm/translate-a64.c @@ -XXX,XX +XXX,XX @@ #include "translate.h" #include "internals.h" #include "qemu/host-utils.h" - #include "semihosting/semihost.h" #include "exec/gen-icount.h" - #include "exec/helper-proto.h" #include "exec/helper-gen.h" #include "exec/log.h" - +#include "cpregs.h" #include "translate-a64.h" #include "qemu/atomic128.h" diff --git a/target/arm/translate.c b/target/arm/translate.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -XXX,XX +XXX,XX @@ #include "qemu/bitops.h" #include "arm_ldst.h" #include "semihosting/semihost.h" - #include "exec/helper-proto.h" #include "exec/helper-gen.h" - #include "exec/log.h" +#include "cpregs.h" #define ENABLE_ARCH_4T arm_dc_feature(s, ARM_FEATURE_V4T) -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Rearrange the values of the enumerators of CPAccessResult so that we may directly extract the target el. For the two special cases in access_check_cp_reg, use CPAccessResult. Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220501055028.646596-3-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/cpregs.h | 26 ++++++++++++-------- target/arm/op_helper.c | 56 +++++++++++++++++++++--------------------- 2 files changed, 44 insertions(+), 38 deletions(-) diff --git a/target/arm/cpregs.h b/target/arm/cpregs.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpregs.h +++ b/target/arm/cpregs.h @@ -XXX,XX +XXX,XX @@ static inline bool cptype_valid(int cptype) typedef enum CPAccessResult { /* Access is permitted */ CP_ACCESS_OK = 0, + + /* + * Combined with one of the following, the low 2 bits indicate the + * target exception level. If 0, the exception is taken to the usual + * target EL (EL1 or PL1 if in EL0, otherwise to the current EL). + */ + CP_ACCESS_EL_MASK = 3, + /* * Access fails due to a configurable trap or enable which would * result in a categorized exception syndrome giving information about * the failing instruction (ie syndrome category 0x3, 0x4, 0x5, 0x6, - * 0xc or 0x18). The exception is taken to the usual target EL (EL1 or - * PL1 if in EL0, otherwise to the current EL). + * 0xc or 0x18). */ - CP_ACCESS_TRAP = 1, + CP_ACCESS_TRAP = (1 << 2), + CP_ACCESS_TRAP_EL2 = CP_ACCESS_TRAP | 2, + CP_ACCESS_TRAP_EL3 = CP_ACCESS_TRAP | 3, + /* * Access fails and results in an exception syndrome 0x0 ("uncategorized"). * Note that this is not a catch-all case -- the set of cases which may * result in this failure is specifically defined by the architecture. */ - CP_ACCESS_TRAP_UNCATEGORIZED = 2, - /* As CP_ACCESS_TRAP, but for traps directly to EL2 or EL3 */ - CP_ACCESS_TRAP_EL2 = 3, - CP_ACCESS_TRAP_EL3 = 4, - /* As CP_ACCESS_UNCATEGORIZED, but for traps directly to EL2 or EL3 */ - CP_ACCESS_TRAP_UNCATEGORIZED_EL2 = 5, - CP_ACCESS_TRAP_UNCATEGORIZED_EL3 = 6, + CP_ACCESS_TRAP_UNCATEGORIZED = (2 << 2), + CP_ACCESS_TRAP_UNCATEGORIZED_EL2 = CP_ACCESS_TRAP_UNCATEGORIZED | 2, + CP_ACCESS_TRAP_UNCATEGORIZED_EL3 = CP_ACCESS_TRAP_UNCATEGORIZED | 3, } CPAccessResult; typedef struct ARMCPRegInfo ARMCPRegInfo; diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/op_helper.c +++ b/target/arm/op_helper.c @@ -XXX,XX +XXX,XX @@ void HELPER(access_check_cp_reg)(CPUARMState *env, void *rip, uint32_t syndrome, uint32_t isread) { const ARMCPRegInfo *ri = rip; + CPAccessResult res = CP_ACCESS_OK; int target_el; if (arm_feature(env, ARM_FEATURE_XSCALE) && ri->cp < 14 && extract32(env->cp15.c15_cpar, ri->cp, 1) == 0) { - raise_exception(env, EXCP_UDEF, syndrome, exception_target_el(env)); + res = CP_ACCESS_TRAP; + goto fail; } /* @@ -XXX,XX +XXX,XX @@ void HELPER(access_check_cp_reg)(CPUARMState *env, void *rip, uint32_t syndrome, mask &= ~((1 << 4) | (1 << 14)); if (env->cp15.hstr_el2 & mask) { - target_el = 2; - goto exept; + res = CP_ACCESS_TRAP_EL2; + goto fail; } } - if (!ri->accessfn) { + if (ri->accessfn) { + res = ri->accessfn(env, ri, isread); + } + if (likely(res == CP_ACCESS_OK)) { return; } - switch (ri->accessfn(env, ri, isread)) { - case CP_ACCESS_OK: - return; + fail: + switch (res & ~CP_ACCESS_EL_MASK) { case CP_ACCESS_TRAP: - target_el = exception_target_el(env); - break; - case CP_ACCESS_TRAP_EL2: - /* Requesting a trap to EL2 when we're in EL3 is - * a bug in the access function. - */ - assert(arm_current_el(env) != 3); - target_el = 2; - break; - case CP_ACCESS_TRAP_EL3: - target_el = 3; break; case CP_ACCESS_TRAP_UNCATEGORIZED: - target_el = exception_target_el(env); - syndrome = syn_uncategorized(); - break; - case CP_ACCESS_TRAP_UNCATEGORIZED_EL2: - target_el = 2; - syndrome = syn_uncategorized(); - break; - case CP_ACCESS_TRAP_UNCATEGORIZED_EL3: - target_el = 3; syndrome = syn_uncategorized(); break; default: g_assert_not_reached(); } -exept: + target_el = res & CP_ACCESS_EL_MASK; + switch (target_el) { + case 0: + target_el = exception_target_el(env); + break; + case 2: + assert(arm_current_el(env) != 3); + assert(arm_is_el2_enabled(env)); + break; + case 3: + assert(arm_feature(env, ARM_FEATURE_EL3)); + break; + default: + /* No "direct" traps to EL1 */ + g_assert_not_reached(); + } + raise_exception(env, EXCP_UDEF, syndrome, target_el); } -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Remove a possible source of error by removing REGINFO_SENTINEL and using ARRAY_SIZE (convinently hidden inside a macro) to find the end of the set of regs being registered or modified. The space saved by not having the extra array element reduces the executable's .data.rel.ro section by about 9k. Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220501055028.646596-4-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/cpregs.h | 53 +++++++++--------- hw/arm/pxa2xx.c | 1 - hw/arm/pxa2xx_pic.c | 1 - hw/intc/arm_gicv3_cpuif.c | 5 -- hw/intc/arm_gicv3_kvm.c | 1 - target/arm/cpu64.c | 1 - target/arm/cpu_tcg.c | 4 -- target/arm/helper.c | 111 ++++++++------------------------------ 8 files changed, 48 insertions(+), 129 deletions(-) diff --git a/target/arm/cpregs.h b/target/arm/cpregs.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpregs.h +++ b/target/arm/cpregs.h @@ -XXX,XX +XXX,XX @@ #define ARM_CP_NO_GDB 0x4000 #define ARM_CP_RAISES_EXC 0x8000 #define ARM_CP_NEWEL 0x10000 -/* Used only as a terminator for ARMCPRegInfo lists */ -#define ARM_CP_SENTINEL 0xfffff /* Mask of only the flag bits in a type field */ #define ARM_CP_FLAG_MASK 0x1f0ff @@ -XXX,XX +XXX,XX @@ enum { ARM_CP_SECSTATE_NS = (1 << 1), /* bit[1]: Non-secure state register */ }; -/* - * Return true if cptype is a valid type field. This is used to try to - * catch errors where the sentinel has been accidentally left off the end - * of a list of registers. - */ -static inline bool cptype_valid(int cptype) -{ - return ((cptype & ~ARM_CP_FLAG_MASK) == 0) - || ((cptype & ARM_CP_SPECIAL) && - ((cptype & ~ARM_CP_FLAG_MASK) <= ARM_LAST_SPECIAL)); -} - /* * Access rights: * We define bits for Read and Write access for what rev C of the v7-AR ARM ARM @@ -XXX,XX +XXX,XX @@ struct ARMCPRegInfo { #define CPREG_FIELD64(env, ri) \ (*(uint64_t *)((char *)(env) + (ri)->fieldoffset)) -#define REGINFO_SENTINEL { .type = ARM_CP_SENTINEL } +void define_one_arm_cp_reg_with_opaque(ARMCPU *cpu, const ARMCPRegInfo *reg, + void *opaque); -void define_arm_cp_regs_with_opaque(ARMCPU *cpu, - const ARMCPRegInfo *regs, void *opaque); -void define_one_arm_cp_reg_with_opaque(ARMCPU *cpu, - const ARMCPRegInfo *regs, void *opaque); -static inline void define_arm_cp_regs(ARMCPU *cpu, const ARMCPRegInfo *regs) -{ - define_arm_cp_regs_with_opaque(cpu, regs, 0); -} static inline void define_one_arm_cp_reg(ARMCPU *cpu, const ARMCPRegInfo *regs) { - define_one_arm_cp_reg_with_opaque(cpu, regs, 0); + define_one_arm_cp_reg_with_opaque(cpu, regs, NULL); } + +void define_arm_cp_regs_with_opaque_len(ARMCPU *cpu, const ARMCPRegInfo *regs, + void *opaque, size_t len); + +#define define_arm_cp_regs_with_opaque(CPU, REGS, OPAQUE) \ + do { \ + QEMU_BUILD_BUG_ON(ARRAY_SIZE(REGS) == 0); \ + define_arm_cp_regs_with_opaque_len(CPU, REGS, OPAQUE, \ + ARRAY_SIZE(REGS)); \ + } while (0) + +#define define_arm_cp_regs(CPU, REGS) \ + define_arm_cp_regs_with_opaque(CPU, REGS, NULL) + const ARMCPRegInfo *get_arm_cp_reginfo(GHashTable *cpregs, uint32_t encoded_cp); /* @@ -XXX,XX +XXX,XX @@ typedef struct ARMCPRegUserSpaceInfo { uint64_t fixed_bits; } ARMCPRegUserSpaceInfo; -#define REGUSERINFO_SENTINEL { .name = NULL } +void modify_arm_cp_regs_with_len(ARMCPRegInfo *regs, size_t regs_len, + const ARMCPRegUserSpaceInfo *mods, + size_t mods_len); -void modify_arm_cp_regs(ARMCPRegInfo *regs, const ARMCPRegUserSpaceInfo *mods); +#define modify_arm_cp_regs(REGS, MODS) \ + do { \ + QEMU_BUILD_BUG_ON(ARRAY_SIZE(REGS) == 0); \ + QEMU_BUILD_BUG_ON(ARRAY_SIZE(MODS) == 0); \ + modify_arm_cp_regs_with_len(REGS, ARRAY_SIZE(REGS), \ + MODS, ARRAY_SIZE(MODS)); \ + } while (0) /* CPWriteFn that can be used to implement writes-ignored behaviour */ void arm_cp_write_ignore(CPUARMState *env, const ARMCPRegInfo *ri, diff --git a/hw/arm/pxa2xx.c b/hw/arm/pxa2xx.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/pxa2xx.c +++ b/hw/arm/pxa2xx.c @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo pxa_cp_reginfo[] = { { .name = "PWRMODE", .cp = 14, .crn = 7, .crm = 0, .opc1 = 0, .opc2 = 0, .access = PL1_RW, .type = ARM_CP_IO, .readfn = arm_cp_read_zero, .writefn = pxa2xx_pwrmode_write }, - REGINFO_SENTINEL }; static void pxa2xx_setup_cp14(PXA2xxState *s) diff --git a/hw/arm/pxa2xx_pic.c b/hw/arm/pxa2xx_pic.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/pxa2xx_pic.c +++ b/hw/arm/pxa2xx_pic.c @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo pxa_pic_cp_reginfo[] = { REGINFO_FOR_PIC_CP("ICLR2", 8), REGINFO_FOR_PIC_CP("ICFP2", 9), REGINFO_FOR_PIC_CP("ICPR2", 0xa), - REGINFO_SENTINEL }; static const MemoryRegionOps pxa2xx_pic_ops = { diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c index XXXXXXX..XXXXXXX 100644 --- a/hw/intc/arm_gicv3_cpuif.c +++ b/hw/intc/arm_gicv3_cpuif.c @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo gicv3_cpuif_reginfo[] = { .readfn = icc_igrpen1_el3_read, .writefn = icc_igrpen1_el3_write, }, - REGINFO_SENTINEL }; static uint64_t ich_ap_read(CPUARMState *env, const ARMCPRegInfo *ri) @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo gicv3_cpuif_hcr_reginfo[] = { .readfn = ich_vmcr_read, .writefn = ich_vmcr_write, }, - REGINFO_SENTINEL }; static const ARMCPRegInfo gicv3_cpuif_ich_apxr1_reginfo[] = { @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo gicv3_cpuif_ich_apxr1_reginfo[] = { .readfn = ich_ap_read, .writefn = ich_ap_write, }, - REGINFO_SENTINEL }; static const ARMCPRegInfo gicv3_cpuif_ich_apxr23_reginfo[] = { @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo gicv3_cpuif_ich_apxr23_reginfo[] = { .readfn = ich_ap_read, .writefn = ich_ap_write, }, - REGINFO_SENTINEL }; static void gicv3_cpuif_el_change_hook(ARMCPU *cpu, void *opaque) @@ -XXX,XX +XXX,XX @@ void gicv3_init_cpuif(GICv3State *s) .readfn = ich_lr_read, .writefn = ich_lr_write, }, - REGINFO_SENTINEL }; define_arm_cp_regs(cpu, lr_regset); } diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c index XXXXXXX..XXXXXXX 100644 --- a/hw/intc/arm_gicv3_kvm.c +++ b/hw/intc/arm_gicv3_kvm.c @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo gicv3_cpuif_reginfo[] = { */ .resetfn = arm_gicv3_icc_reset, }, - REGINFO_SENTINEL }; /** diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu64.c +++ b/target/arm/cpu64.c @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo cortex_a72_a57_a53_cp_reginfo[] = { { .name = "L2MERRSR", .cp = 15, .opc1 = 3, .crm = 15, .access = PL1_RW, .type = ARM_CP_CONST | ARM_CP_64BIT, .resetvalue = 0 }, - REGINFO_SENTINEL }; static void aarch64_a57_initfn(Object *obj) diff --git a/target/arm/cpu_tcg.c b/target/arm/cpu_tcg.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu_tcg.c +++ b/target/arm/cpu_tcg.c @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo cortexa8_cp_reginfo[] = { .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, { .name = "L2AUXCR", .cp = 15, .crn = 9, .crm = 0, .opc1 = 1, .opc2 = 2, .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, - REGINFO_SENTINEL }; static void cortex_a8_initfn(Object *obj) @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo cortexa9_cp_reginfo[] = { .access = PL1_RW, .resetvalue = 0, .type = ARM_CP_CONST }, { .name = "TLB_ATTR", .cp = 15, .crn = 15, .crm = 7, .opc1 = 5, .opc2 = 2, .access = PL1_RW, .resetvalue = 0, .type = ARM_CP_CONST }, - REGINFO_SENTINEL }; static void cortex_a9_initfn(Object *obj) @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo cortexa15_cp_reginfo[] = { #endif { .name = "L2ECTLR", .cp = 15, .crn = 9, .crm = 0, .opc1 = 1, .opc2 = 3, .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, - REGINFO_SENTINEL }; static void cortex_a7_initfn(Object *obj) @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo cortexr5_cp_reginfo[] = { .access = PL1_RW, .type = ARM_CP_CONST }, { .name = "DCACHE_INVAL", .cp = 15, .opc1 = 0, .crn = 15, .crm = 5, .opc2 = 0, .access = PL1_W, .type = ARM_CP_NOP }, - REGINFO_SENTINEL }; static void cortex_r5_initfn(Object *obj) diff --git a/target/arm/helper.c b/target/arm/helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo cp_reginfo[] = { .secure = ARM_CP_SECSTATE_S, .fieldoffset = offsetof(CPUARMState, cp15.contextidr_s), .resetvalue = 0, .writefn = contextidr_write, .raw_writefn = raw_write, }, - REGINFO_SENTINEL }; static const ARMCPRegInfo not_v8_cp_reginfo[] = { @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo not_v8_cp_reginfo[] = { { .name = "CACHEMAINT", .cp = 15, .crn = 7, .crm = CP_ANY, .opc1 = 0, .opc2 = CP_ANY, .access = PL1_W, .type = ARM_CP_NOP | ARM_CP_OVERRIDE }, - REGINFO_SENTINEL }; static const ARMCPRegInfo not_v6_cp_reginfo[] = { @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo not_v6_cp_reginfo[] = { */ { .name = "WFI_v5", .cp = 15, .crn = 7, .crm = 8, .opc1 = 0, .opc2 = 2, .access = PL1_W, .type = ARM_CP_WFI }, - REGINFO_SENTINEL }; static const ARMCPRegInfo not_v7_cp_reginfo[] = { @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo not_v7_cp_reginfo[] = { .opc1 = 0, .opc2 = 0, .access = PL1_RW, .type = ARM_CP_NOP }, { .name = "NMRR", .cp = 15, .crn = 10, .crm = 2, .opc1 = 0, .opc2 = 1, .access = PL1_RW, .type = ARM_CP_NOP }, - REGINFO_SENTINEL }; static void cpacr_write(CPUARMState *env, const ARMCPRegInfo *ri, @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo v6_cp_reginfo[] = { .crn = 1, .crm = 0, .opc1 = 0, .opc2 = 2, .accessfn = cpacr_access, .access = PL1_RW, .fieldoffset = offsetof(CPUARMState, cp15.cpacr_el1), .resetfn = cpacr_reset, .writefn = cpacr_write, .readfn = cpacr_read }, - REGINFO_SENTINEL }; typedef struct pm_event { @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo v7_cp_reginfo[] = { { .name = "TLBIMVAA", .cp = 15, .opc1 = 0, .crn = 8, .crm = 7, .opc2 = 3, .type = ARM_CP_NO_RAW, .access = PL1_W, .accessfn = access_ttlb, .writefn = tlbimvaa_write }, - REGINFO_SENTINEL }; static const ARMCPRegInfo v7mp_cp_reginfo[] = { @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo v7mp_cp_reginfo[] = { { .name = "TLBIMVAAIS", .cp = 15, .opc1 = 0, .crn = 8, .crm = 3, .opc2 = 3, .type = ARM_CP_NO_RAW, .access = PL1_W, .accessfn = access_ttlb, .writefn = tlbimvaa_is_write }, - REGINFO_SENTINEL }; static const ARMCPRegInfo pmovsset_cp_reginfo[] = { @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo pmovsset_cp_reginfo[] = { .fieldoffset = offsetof(CPUARMState, cp15.c9_pmovsr), .writefn = pmovsset_write, .raw_writefn = raw_write }, - REGINFO_SENTINEL }; static void teecr_write(CPUARMState *env, const ARMCPRegInfo *ri, @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo t2ee_cp_reginfo[] = { { .name = "TEEHBR", .cp = 14, .crn = 1, .crm = 0, .opc1 = 6, .opc2 = 0, .access = PL0_RW, .fieldoffset = offsetof(CPUARMState, teehbr), .accessfn = teehbr_access, .resetvalue = 0 }, - REGINFO_SENTINEL }; static const ARMCPRegInfo v6k_cp_reginfo[] = { @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo v6k_cp_reginfo[] = { .bank_fieldoffsets = { offsetoflow32(CPUARMState, cp15.tpidrprw_s), offsetoflow32(CPUARMState, cp15.tpidrprw_ns) }, .resetvalue = 0 }, - REGINFO_SENTINEL }; #ifndef CONFIG_USER_ONLY @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo generic_timer_cp_reginfo[] = { .fieldoffset = offsetof(CPUARMState, cp15.c14_timer[GTIMER_SEC].cval), .writefn = gt_sec_cval_write, .raw_writefn = raw_write, }, - REGINFO_SENTINEL }; static CPAccessResult e2h_access(CPUARMState *env, const ARMCPRegInfo *ri, @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo generic_timer_cp_reginfo[] = { .access = PL0_R, .type = ARM_CP_NO_RAW | ARM_CP_IO, .readfn = gt_virt_cnt_read, }, - REGINFO_SENTINEL }; #endif @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo vapa_cp_reginfo[] = { .access = PL1_W, .accessfn = ats_access, .writefn = ats_write, .type = ARM_CP_NO_RAW | ARM_CP_RAISES_EXC }, #endif - REGINFO_SENTINEL }; /* Return basic MPU access permission bits. */ @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo pmsav7_cp_reginfo[] = { .fieldoffset = offsetof(CPUARMState, pmsav7.rnr[M_REG_NS]), .writefn = pmsav7_rgnr_write, .resetfn = arm_cp_reset_ignore }, - REGINFO_SENTINEL }; static const ARMCPRegInfo pmsav5_cp_reginfo[] = { @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo pmsav5_cp_reginfo[] = { { .name = "946_PRBS7", .cp = 15, .crn = 6, .crm = 7, .opc1 = 0, .opc2 = CP_ANY, .access = PL1_RW, .resetvalue = 0, .fieldoffset = offsetof(CPUARMState, cp15.c6_region[7]) }, - REGINFO_SENTINEL }; static void vmsa_ttbcr_raw_write(CPUARMState *env, const ARMCPRegInfo *ri, @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo vmsa_pmsa_cp_reginfo[] = { .access = PL1_RW, .accessfn = access_tvm_trvm, .fieldoffset = offsetof(CPUARMState, cp15.far_el[1]), .resetvalue = 0, }, - REGINFO_SENTINEL }; static const ARMCPRegInfo vmsa_cp_reginfo[] = { @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo vmsa_cp_reginfo[] = { /* No offsetoflow32 -- pass the entire TCR to writefn/raw_writefn. */ .bank_fieldoffsets = { offsetof(CPUARMState, cp15.tcr_el[3]), offsetof(CPUARMState, cp15.tcr_el[1])} }, - REGINFO_SENTINEL }; /* Note that unlike TTBCR, writing to TTBCR2 does not require flushing @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo omap_cp_reginfo[] = { { .name = "C9", .cp = 15, .crn = 9, .crm = CP_ANY, .opc1 = CP_ANY, .opc2 = CP_ANY, .access = PL1_RW, .type = ARM_CP_CONST | ARM_CP_OVERRIDE, .resetvalue = 0 }, - REGINFO_SENTINEL }; static void xscale_cpar_write(CPUARMState *env, const ARMCPRegInfo *ri, @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo xscale_cp_reginfo[] = { { .name = "XSCALE_UNLOCK_DCACHE", .cp = 15, .opc1 = 0, .crn = 9, .crm = 2, .opc2 = 1, .access = PL1_W, .type = ARM_CP_NOP }, - REGINFO_SENTINEL }; static const ARMCPRegInfo dummy_c15_cp_reginfo[] = { @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo dummy_c15_cp_reginfo[] = { .access = PL1_RW, .type = ARM_CP_CONST | ARM_CP_NO_RAW | ARM_CP_OVERRIDE, .resetvalue = 0 }, - REGINFO_SENTINEL }; static const ARMCPRegInfo cache_dirty_status_cp_reginfo[] = { @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo cache_dirty_status_cp_reginfo[] = { { .name = "CDSR", .cp = 15, .crn = 7, .crm = 10, .opc1 = 0, .opc2 = 6, .access = PL1_R, .type = ARM_CP_CONST | ARM_CP_NO_RAW, .resetvalue = 0 }, - REGINFO_SENTINEL }; static const ARMCPRegInfo cache_block_ops_cp_reginfo[] = { @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo cache_block_ops_cp_reginfo[] = { .access = PL0_W, .type = ARM_CP_NOP|ARM_CP_64BIT }, { .name = "CIDCR", .cp = 15, .crm = 14, .opc1 = 0, .access = PL1_W, .type = ARM_CP_NOP|ARM_CP_64BIT }, - REGINFO_SENTINEL }; static const ARMCPRegInfo cache_test_clean_cp_reginfo[] = { @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo cache_test_clean_cp_reginfo[] = { { .name = "TCI_DCACHE", .cp = 15, .crn = 7, .crm = 14, .opc1 = 0, .opc2 = 3, .access = PL0_R, .type = ARM_CP_CONST | ARM_CP_NO_RAW, .resetvalue = (1 << 30) }, - REGINFO_SENTINEL }; static const ARMCPRegInfo strongarm_cp_reginfo[] = { @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo strongarm_cp_reginfo[] = { .crm = CP_ANY, .opc1 = CP_ANY, .opc2 = CP_ANY, .access = PL1_RW, .resetvalue = 0, .type = ARM_CP_CONST | ARM_CP_OVERRIDE | ARM_CP_NO_RAW }, - REGINFO_SENTINEL }; static uint64_t midr_read(CPUARMState *env, const ARMCPRegInfo *ri) @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo lpae_cp_reginfo[] = { .bank_fieldoffsets = { offsetof(CPUARMState, cp15.ttbr1_s), offsetof(CPUARMState, cp15.ttbr1_ns) }, .writefn = vmsa_ttbr_write, }, - REGINFO_SENTINEL }; static uint64_t aa64_fpcr_read(CPUARMState *env, const ARMCPRegInfo *ri) @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo v8_cp_reginfo[] = { .access = PL1_RW, .accessfn = access_trap_aa32s_el1, .writefn = sdcr_write, .fieldoffset = offsetoflow32(CPUARMState, cp15.mdcr_el3) }, - REGINFO_SENTINEL }; /* Used to describe the behaviour of EL2 regs when EL2 does not exist. */ @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo el3_no_el2_cp_reginfo[] = { .type = ARM_CP_CONST, .cp = 15, .opc1 = 4, .crn = 6, .crm = 0, .opc2 = 2, .access = PL2_RW, .resetvalue = 0 }, - REGINFO_SENTINEL }; /* Ditto, but for registers which exist in ARMv8 but not v7 */ @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo el3_no_el2_v8_cp_reginfo[] = { .cp = 15, .opc1 = 4, .crn = 1, .crm = 1, .opc2 = 4, .access = PL2_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, - REGINFO_SENTINEL }; static void do_hcr_write(CPUARMState *env, uint64_t value, uint64_t valid_mask) @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo el2_cp_reginfo[] = { .cp = 15, .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 1, .opc2 = 3, .access = PL2_RW, .fieldoffset = offsetof(CPUARMState, cp15.hstr_el2) }, - REGINFO_SENTINEL }; static const ARMCPRegInfo el2_v8_cp_reginfo[] = { @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo el2_v8_cp_reginfo[] = { .access = PL2_RW, .fieldoffset = offsetofhigh32(CPUARMState, cp15.hcr_el2), .writefn = hcr_writehigh }, - REGINFO_SENTINEL }; static CPAccessResult sel2_access(CPUARMState *env, const ARMCPRegInfo *ri, @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo el2_sec_cp_reginfo[] = { .opc0 = 3, .opc1 = 4, .crn = 2, .crm = 6, .opc2 = 2, .access = PL2_RW, .accessfn = sel2_access, .fieldoffset = offsetof(CPUARMState, cp15.vstcr_el2) }, - REGINFO_SENTINEL }; static CPAccessResult nsacr_access(CPUARMState *env, const ARMCPRegInfo *ri, @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo el3_cp_reginfo[] = { .opc0 = 1, .opc1 = 6, .crn = 8, .crm = 7, .opc2 = 5, .access = PL3_W, .type = ARM_CP_NO_RAW, .writefn = tlbi_aa64_vae3_write }, - REGINFO_SENTINEL }; #ifndef CONFIG_USER_ONLY @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo debug_cp_reginfo[] = { .cp = 14, .opc0 = 2, .opc1 = 0, .crn = 0, .crm = 2, .opc2 = 0, .access = PL1_RW, .accessfn = access_tda, .type = ARM_CP_NOP }, - REGINFO_SENTINEL }; static const ARMCPRegInfo debug_lpae_cp_reginfo[] = { @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo debug_lpae_cp_reginfo[] = { .access = PL0_R, .type = ARM_CP_CONST|ARM_CP_64BIT, .resetvalue = 0 }, { .name = "DBGDSAR", .cp = 14, .crm = 2, .opc1 = 0, .access = PL0_R, .type = ARM_CP_CONST|ARM_CP_64BIT, .resetvalue = 0 }, - REGINFO_SENTINEL }; /* Return the exception level to which exceptions should be taken @@ -XXX,XX +XXX,XX @@ static void define_debug_regs(ARMCPU *cpu) .fieldoffset = offsetof(CPUARMState, cp15.dbgbcr[i]), .writefn = dbgbcr_write, .raw_writefn = raw_write }, - REGINFO_SENTINEL }; define_arm_cp_regs(cpu, dbgregs); } @@ -XXX,XX +XXX,XX @@ static void define_debug_regs(ARMCPU *cpu) .fieldoffset = offsetof(CPUARMState, cp15.dbgwcr[i]), .writefn = dbgwcr_write, .raw_writefn = raw_write }, - REGINFO_SENTINEL }; define_arm_cp_regs(cpu, dbgregs); } @@ -XXX,XX +XXX,XX @@ static void define_pmu_regs(ARMCPU *cpu) .type = ARM_CP_IO, .readfn = pmevtyper_readfn, .writefn = pmevtyper_writefn, .raw_writefn = pmevtyper_rawwrite }, - REGINFO_SENTINEL }; define_arm_cp_regs(cpu, pmev_regs); g_free(pmevcntr_name); @@ -XXX,XX +XXX,XX @@ static void define_pmu_regs(ARMCPU *cpu) .cp = 15, .opc1 = 0, .crn = 9, .crm = 14, .opc2 = 5, .access = PL0_R, .accessfn = pmreg_access, .type = ARM_CP_CONST, .resetvalue = extract64(cpu->pmceid1, 32, 32) }, - REGINFO_SENTINEL }; define_arm_cp_regs(cpu, v81_pmu_regs); } @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo lor_reginfo[] = { .opc0 = 3, .opc1 = 0, .crn = 10, .crm = 4, .opc2 = 7, .access = PL1_R, .accessfn = access_lor_ns, .type = ARM_CP_CONST, .resetvalue = 0 }, - REGINFO_SENTINEL }; #ifdef TARGET_AARCH64 @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo pauth_reginfo[] = { .opc0 = 3, .opc1 = 0, .crn = 2, .crm = 1, .opc2 = 3, .access = PL1_RW, .accessfn = access_pauth, .fieldoffset = offsetof(CPUARMState, keys.apib.hi) }, - REGINFO_SENTINEL }; static const ARMCPRegInfo tlbirange_reginfo[] = { @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo tlbirange_reginfo[] = { .opc0 = 1, .opc1 = 6, .crn = 8, .crm = 6, .opc2 = 5, .access = PL3_W, .type = ARM_CP_NO_RAW, .writefn = tlbi_aa64_rvae3_write }, - REGINFO_SENTINEL }; static const ARMCPRegInfo tlbios_reginfo[] = { @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo tlbios_reginfo[] = { .opc0 = 1, .opc1 = 6, .crn = 8, .crm = 1, .opc2 = 5, .access = PL3_W, .type = ARM_CP_NO_RAW, .writefn = tlbi_aa64_vae3is_write }, - REGINFO_SENTINEL }; static uint64_t rndr_readfn(CPUARMState *env, const ARMCPRegInfo *ri) @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo rndr_reginfo[] = { .type = ARM_CP_NO_RAW | ARM_CP_SUPPRESS_TB_END | ARM_CP_IO, .opc0 = 3, .opc1 = 3, .crn = 2, .crm = 4, .opc2 = 1, .access = PL0_R, .readfn = rndr_readfn }, - REGINFO_SENTINEL }; #ifndef CONFIG_USER_ONLY @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo dcpop_reg[] = { .opc0 = 1, .opc1 = 3, .crn = 7, .crm = 12, .opc2 = 1, .access = PL0_W, .type = ARM_CP_NO_RAW | ARM_CP_SUPPRESS_TB_END, .accessfn = aa64_cacheop_poc_access, .writefn = dccvap_writefn }, - REGINFO_SENTINEL }; static const ARMCPRegInfo dcpodp_reg[] = { @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo dcpodp_reg[] = { .opc0 = 1, .opc1 = 3, .crn = 7, .crm = 13, .opc2 = 1, .access = PL0_W, .type = ARM_CP_NO_RAW | ARM_CP_SUPPRESS_TB_END, .accessfn = aa64_cacheop_poc_access, .writefn = dccvap_writefn }, - REGINFO_SENTINEL }; #endif /*CONFIG_USER_ONLY*/ @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo mte_reginfo[] = { { .name = "DC_CIGDSW", .state = ARM_CP_STATE_AA64, .opc0 = 1, .opc1 = 0, .crn = 7, .crm = 14, .opc2 = 6, .type = ARM_CP_NOP, .access = PL1_W, .accessfn = access_tsw }, - REGINFO_SENTINEL }; static const ARMCPRegInfo mte_tco_ro_reginfo[] = { { .name = "TCO", .state = ARM_CP_STATE_AA64, .opc0 = 3, .opc1 = 3, .crn = 4, .crm = 2, .opc2 = 7, .type = ARM_CP_CONST, .access = PL0_RW, }, - REGINFO_SENTINEL }; static const ARMCPRegInfo mte_el0_cacheop_reginfo[] = { @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo mte_el0_cacheop_reginfo[] = { .accessfn = aa64_zva_access, #endif }, - REGINFO_SENTINEL }; #endif @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo predinv_reginfo[] = { { .name = "CPPRCTX", .state = ARM_CP_STATE_AA32, .cp = 15, .opc1 = 0, .crn = 7, .crm = 3, .opc2 = 7, .type = ARM_CP_NOP, .access = PL0_W, .accessfn = access_predinv }, - REGINFO_SENTINEL }; static uint64_t ccsidr2_read(CPUARMState *env, const ARMCPRegInfo *ri) @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo ccsidr2_reginfo[] = { .access = PL1_R, .accessfn = access_aa64_tid2, .readfn = ccsidr2_read, .type = ARM_CP_NO_RAW }, - REGINFO_SENTINEL }; static CPAccessResult access_aa64_tid3(CPUARMState *env, const ARMCPRegInfo *ri, @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo jazelle_regs[] = { .cp = 14, .crn = 2, .crm = 0, .opc1 = 7, .opc2 = 0, .accessfn = access_joscr_jmcr, .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, - REGINFO_SENTINEL }; static const ARMCPRegInfo vhe_reginfo[] = { @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo vhe_reginfo[] = { .access = PL2_RW, .accessfn = e2h_access, .writefn = gt_virt_cval_write, .raw_writefn = raw_write }, #endif - REGINFO_SENTINEL }; #ifndef CONFIG_USER_ONLY @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo ats1e1_reginfo[] = { .opc0 = 1, .opc1 = 0, .crn = 7, .crm = 9, .opc2 = 1, .access = PL1_W, .type = ARM_CP_NO_RAW | ARM_CP_RAISES_EXC, .writefn = ats_write64 }, - REGINFO_SENTINEL }; static const ARMCPRegInfo ats1cp_reginfo[] = { @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo ats1cp_reginfo[] = { .cp = 15, .opc1 = 0, .crn = 7, .crm = 9, .opc2 = 1, .access = PL1_W, .type = ARM_CP_NO_RAW | ARM_CP_RAISES_EXC, .writefn = ats_write }, - REGINFO_SENTINEL }; #endif @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo actlr2_hactlr2_reginfo[] = { .cp = 15, .opc1 = 4, .crn = 1, .crm = 0, .opc2 = 3, .access = PL2_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, - REGINFO_SENTINEL }; void register_cp_regs_for_features(ARMCPU *cpu) @@ -XXX,XX +XXX,XX @@ void register_cp_regs_for_features(ARMCPU *cpu) .access = PL1_R, .type = ARM_CP_CONST, .accessfn = access_aa32_tid3, .resetvalue = cpu->isar.id_isar6 }, - REGINFO_SENTINEL }; define_arm_cp_regs(cpu, v6_idregs); define_arm_cp_regs(cpu, v6_cp_reginfo); @@ -XXX,XX +XXX,XX @@ void register_cp_regs_for_features(ARMCPU *cpu) .opc0 = 3, .opc1 = 3, .crn = 9, .crm = 12, .opc2 = 7, .access = PL0_R, .accessfn = pmreg_access, .type = ARM_CP_CONST, .resetvalue = cpu->pmceid1 }, - REGINFO_SENTINEL }; #ifdef CONFIG_USER_ONLY ARMCPRegUserSpaceInfo v8_user_idregs[] = { @@ -XXX,XX +XXX,XX @@ void register_cp_regs_for_features(ARMCPU *cpu) .exported_bits = 0x000000f0ffffffff }, { .name = "ID_AA64ISAR*_EL1_RESERVED", .is_glob = true }, - REGUSERINFO_SENTINEL }; modify_arm_cp_regs(v8_idregs, v8_user_idregs); #endif @@ -XXX,XX +XXX,XX @@ void register_cp_regs_for_features(ARMCPU *cpu) .access = PL2_RW, .resetvalue = vmpidr_def, .fieldoffset = offsetof(CPUARMState, cp15.vmpidr_el2) }, - REGINFO_SENTINEL }; define_arm_cp_regs(cpu, vpidr_regs); define_arm_cp_regs(cpu, el2_cp_reginfo); @@ -XXX,XX +XXX,XX @@ void register_cp_regs_for_features(ARMCPU *cpu) .access = PL2_RW, .accessfn = access_el3_aa32ns, .type = ARM_CP_NO_RAW, .writefn = arm_cp_write_ignore, .readfn = mpidr_read }, - REGINFO_SENTINEL }; define_arm_cp_regs(cpu, vpidr_regs); define_arm_cp_regs(cpu, el3_no_el2_cp_reginfo); @@ -XXX,XX +XXX,XX @@ void register_cp_regs_for_features(ARMCPU *cpu) .raw_writefn = raw_write, .writefn = sctlr_write, .fieldoffset = offsetof(CPUARMState, cp15.sctlr_el[3]), .resetvalue = cpu->reset_sctlr }, - REGINFO_SENTINEL }; define_arm_cp_regs(cpu, el3_regs); @@ -XXX,XX +XXX,XX @@ void register_cp_regs_for_features(ARMCPU *cpu) { .name = "DUMMY", .cp = 15, .crn = 0, .crm = 7, .opc1 = 0, .opc2 = CP_ANY, .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = 0 }, - REGINFO_SENTINEL }; ARMCPRegInfo id_v8_midr_cp_reginfo[] = { { .name = "MIDR_EL1", .state = ARM_CP_STATE_BOTH, @@ -XXX,XX +XXX,XX @@ void register_cp_regs_for_features(ARMCPU *cpu) .access = PL1_R, .accessfn = access_aa64_tid1, .type = ARM_CP_CONST, .resetvalue = cpu->revidr }, - REGINFO_SENTINEL }; ARMCPRegInfo id_cp_reginfo[] = { /* These are common to v8 and pre-v8 */ @@ -XXX,XX +XXX,XX @@ void register_cp_regs_for_features(ARMCPU *cpu) .access = PL1_R, .accessfn = access_aa32_tid1, .type = ARM_CP_CONST, .resetvalue = 0 }, - REGINFO_SENTINEL }; /* TLBTR is specific to VMSA */ ARMCPRegInfo id_tlbtr_reginfo = { @@ -XXX,XX +XXX,XX @@ void register_cp_regs_for_features(ARMCPU *cpu) { .name = "MIDR_EL1", .exported_bits = 0x00000000ffffffff }, { .name = "REVIDR_EL1" }, - REGUSERINFO_SENTINEL }; modify_arm_cp_regs(id_v8_midr_cp_reginfo, id_v8_user_midr_cp_reginfo); #endif if (arm_feature(env, ARM_FEATURE_OMAPCP) || arm_feature(env, ARM_FEATURE_STRONGARM)) { - ARMCPRegInfo *r; + size_t i; /* Register the blanket "writes ignored" value first to cover the * whole space. Then update the specific ID registers to allow write * access, so that they ignore writes rather than causing them to * UNDEF. */ define_one_arm_cp_reg(cpu, &crn0_wi_reginfo); - for (r = id_pre_v8_midr_cp_reginfo; - r->type != ARM_CP_SENTINEL; r++) { - r->access = PL1_RW; + for (i = 0; i < ARRAY_SIZE(id_pre_v8_midr_cp_reginfo); ++i) { + id_pre_v8_midr_cp_reginfo[i].access = PL1_RW; } - for (r = id_cp_reginfo; r->type != ARM_CP_SENTINEL; r++) { - r->access = PL1_RW; + for (i = 0; i < ARRAY_SIZE(id_cp_reginfo); ++i) { + id_cp_reginfo[i].access = PL1_RW; } id_mpuir_reginfo.access = PL1_RW; id_tlbtr_reginfo.access = PL1_RW; @@ -XXX,XX +XXX,XX @@ void register_cp_regs_for_features(ARMCPU *cpu) { .name = "MPIDR_EL1", .state = ARM_CP_STATE_BOTH, .opc0 = 3, .crn = 0, .crm = 0, .opc1 = 0, .opc2 = 5, .access = PL1_R, .readfn = mpidr_read, .type = ARM_CP_NO_RAW }, - REGINFO_SENTINEL }; #ifdef CONFIG_USER_ONLY ARMCPRegUserSpaceInfo mpidr_user_cp_reginfo[] = { { .name = "MPIDR_EL1", .fixed_bits = 0x0000000080000000 }, - REGUSERINFO_SENTINEL }; modify_arm_cp_regs(mpidr_cp_reginfo, mpidr_user_cp_reginfo); #endif @@ -XXX,XX +XXX,XX @@ void register_cp_regs_for_features(ARMCPU *cpu) .opc0 = 3, .opc1 = 6, .crn = 1, .crm = 0, .opc2 = 1, .access = PL3_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, - REGINFO_SENTINEL }; define_arm_cp_regs(cpu, auxcr_reginfo); if (cpu_isar_feature(aa32_ac2, cpu)) { @@ -XXX,XX +XXX,XX @@ void register_cp_regs_for_features(ARMCPU *cpu) .type = ARM_CP_CONST, .opc0 = 3, .opc1 = 1, .crn = 15, .crm = 3, .opc2 = 0, .access = PL1_R, .resetvalue = cpu->reset_cbar }, - REGINFO_SENTINEL }; /* We don't implement a r/w 64 bit CBAR currently */ assert(arm_feature(env, ARM_FEATURE_CBAR_RO)); @@ -XXX,XX +XXX,XX @@ void register_cp_regs_for_features(ARMCPU *cpu) .bank_fieldoffsets = { offsetof(CPUARMState, cp15.vbar_s), offsetof(CPUARMState, cp15.vbar_ns) }, .resetvalue = 0 }, - REGINFO_SENTINEL }; define_arm_cp_regs(cpu, vbar_cp_reginfo); } @@ -XXX,XX +XXX,XX @@ void define_one_arm_cp_reg_with_opaque(ARMCPU *cpu, r->writefn); } } - /* Bad type field probably means missing sentinel at end of reg list */ - assert(cptype_valid(r->type)); + for (crm = crmmin; crm <= crmmax; crm++) { for (opc1 = opc1min; opc1 <= opc1max; opc1++) { for (opc2 = opc2min; opc2 <= opc2max; opc2++) { @@ -XXX,XX +XXX,XX @@ void define_one_arm_cp_reg_with_opaque(ARMCPU *cpu, } } -void define_arm_cp_regs_with_opaque(ARMCPU *cpu, - const ARMCPRegInfo *regs, void *opaque) +/* Define a whole list of registers */ +void define_arm_cp_regs_with_opaque_len(ARMCPU *cpu, const ARMCPRegInfo *regs, + void *opaque, size_t len) { - /* Define a whole list of registers */ - const ARMCPRegInfo *r; - for (r = regs; r->type != ARM_CP_SENTINEL; r++) { - define_one_arm_cp_reg_with_opaque(cpu, r, opaque); + size_t i; + for (i = 0; i < len; ++i) { + define_one_arm_cp_reg_with_opaque(cpu, regs + i, opaque); } } @@ -XXX,XX +XXX,XX @@ void define_arm_cp_regs_with_opaque(ARMCPU *cpu, * user-space cannot alter any values and dynamic values pertaining to * execution state are hidden from user space view anyway. */ -void modify_arm_cp_regs(ARMCPRegInfo *regs, const ARMCPRegUserSpaceInfo *mods) +void modify_arm_cp_regs_with_len(ARMCPRegInfo *regs, size_t regs_len, + const ARMCPRegUserSpaceInfo *mods, + size_t mods_len) { - const ARMCPRegUserSpaceInfo *m; - ARMCPRegInfo *r; - - for (m = mods; m->name; m++) { + for (size_t mi = 0; mi < mods_len; ++mi) { + const ARMCPRegUserSpaceInfo *m = mods + mi; GPatternSpec *pat = NULL; + if (m->is_glob) { pat = g_pattern_spec_new(m->name); } - for (r = regs; r->type != ARM_CP_SENTINEL; r++) { + for (size_t ri = 0; ri < regs_len; ++ri) { + ARMCPRegInfo *r = regs + ri; + if (pat && g_pattern_match_string(pat, r->name)) { r->type = ARM_CP_CONST; r->access = PL0U_R; -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> These particular data structures are not modified at runtime. Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220501055028.646596-5-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/helper.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/target/arm/helper.c b/target/arm/helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -XXX,XX +XXX,XX @@ void register_cp_regs_for_features(ARMCPU *cpu) .resetvalue = cpu->pmceid1 }, }; #ifdef CONFIG_USER_ONLY - ARMCPRegUserSpaceInfo v8_user_idregs[] = { + static const ARMCPRegUserSpaceInfo v8_user_idregs[] = { { .name = "ID_AA64PFR0_EL1", .exported_bits = 0x000f000f00ff0000, .fixed_bits = 0x0000000000000011 }, @@ -XXX,XX +XXX,XX @@ void register_cp_regs_for_features(ARMCPU *cpu) */ if (arm_feature(env, ARM_FEATURE_EL3)) { if (arm_feature(env, ARM_FEATURE_AARCH64)) { - ARMCPRegInfo nsacr = { + static const ARMCPRegInfo nsacr = { .name = "NSACR", .type = ARM_CP_CONST, .cp = 15, .opc1 = 0, .crn = 1, .crm = 1, .opc2 = 2, .access = PL1_RW, .accessfn = nsacr_access, @@ -XXX,XX +XXX,XX @@ void register_cp_regs_for_features(ARMCPU *cpu) }; define_one_arm_cp_reg(cpu, &nsacr); } else { - ARMCPRegInfo nsacr = { + static const ARMCPRegInfo nsacr = { .name = "NSACR", .cp = 15, .opc1 = 0, .crn = 1, .crm = 1, .opc2 = 2, .access = PL3_RW | PL1_R, @@ -XXX,XX +XXX,XX @@ void register_cp_regs_for_features(ARMCPU *cpu) } } else { if (arm_feature(env, ARM_FEATURE_V8)) { - ARMCPRegInfo nsacr = { + static const ARMCPRegInfo nsacr = { .name = "NSACR", .type = ARM_CP_CONST, .cp = 15, .opc1 = 0, .crn = 1, .crm = 1, .opc2 = 2, .access = PL1_R, @@ -XXX,XX +XXX,XX @@ void register_cp_regs_for_features(ARMCPU *cpu) .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = cpu->pmsav7_dregion << 8 }; - ARMCPRegInfo crn0_wi_reginfo = { + static const ARMCPRegInfo crn0_wi_reginfo = { .name = "CRN0_WI", .cp = 15, .crn = 0, .crm = CP_ANY, .opc1 = CP_ANY, .opc2 = CP_ANY, .access = PL1_W, .type = ARM_CP_NOP | ARM_CP_OVERRIDE }; #ifdef CONFIG_USER_ONLY - ARMCPRegUserSpaceInfo id_v8_user_midr_cp_reginfo[] = { + static const ARMCPRegUserSpaceInfo id_v8_user_midr_cp_reginfo[] = { { .name = "MIDR_EL1", .exported_bits = 0x00000000ffffffff }, { .name = "REVIDR_EL1" }, @@ -XXX,XX +XXX,XX @@ void register_cp_regs_for_features(ARMCPU *cpu) .access = PL1_R, .readfn = mpidr_read, .type = ARM_CP_NO_RAW }, }; #ifdef CONFIG_USER_ONLY - ARMCPRegUserSpaceInfo mpidr_user_cp_reginfo[] = { + static const ARMCPRegUserSpaceInfo mpidr_user_cp_reginfo[] = { { .name = "MPIDR_EL1", .fixed_bits = 0x0000000080000000 }, }; @@ -XXX,XX +XXX,XX @@ void register_cp_regs_for_features(ARMCPU *cpu) } if (arm_feature(env, ARM_FEATURE_VBAR)) { - ARMCPRegInfo vbar_cp_reginfo[] = { + static const ARMCPRegInfo vbar_cp_reginfo[] = { { .name = "VBAR", .state = ARM_CP_STATE_BOTH, .opc0 = 3, .crn = 12, .crm = 0, .opc1 = 0, .opc2 = 0, .access = PL1_RW, .writefn = vbar_write, -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Instead of defining ARM_CP_FLAG_MASK to remove flags, define ARM_CP_SPECIAL_MASK to isolate special cases. Sort the specials to the low bits. Use an enum. Split the large comment block so as to document each value separately. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 20220501055028.646596-6-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/cpregs.h | 130 +++++++++++++++++++++++-------------- target/arm/cpu.c | 4 +- target/arm/helper.c | 4 +- target/arm/translate-a64.c | 6 +- target/arm/translate.c | 6 +- 5 files changed, 92 insertions(+), 58 deletions(-) diff --git a/target/arm/cpregs.h b/target/arm/cpregs.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpregs.h +++ b/target/arm/cpregs.h @@ -XXX,XX +XXX,XX @@ #define TARGET_ARM_CPREGS_H /* - * ARMCPRegInfo type field bits. If the SPECIAL bit is set this is a - * special-behaviour cp reg and bits [11..8] indicate what behaviour - * it has. Otherwise it is a simple cp reg, where CONST indicates that - * TCG can assume the value to be constant (ie load at translate time) - * and 64BIT indicates a 64 bit wide coprocessor register. SUPPRESS_TB_END - * indicates that the TB should not be ended after a write to this register - * (the default is that the TB ends after cp writes). OVERRIDE permits - * a register definition to override a previous definition for the - * same (cp, is64, crn, crm, opc1, opc2) tuple: either the new or the - * old must have the OVERRIDE bit set. - * ALIAS indicates that this register is an alias view of some underlying - * state which is also visible via another register, and that the other - * register is handling migration and reset; registers marked ALIAS will not be - * migrated but may have their state set by syncing of register state from KVM. - * NO_RAW indicates that this register has no underlying state and does not - * support raw access for state saving/loading; it will not be used for either - * migration or KVM state synchronization. (Typically this is for "registers" - * which are actually used as instructions for cache maintenance and so on.) - * IO indicates that this register does I/O and therefore its accesses - * need to be marked with gen_io_start() and also end the TB. In particular, - * registers which implement clocks or timers require this. - * RAISES_EXC is for when the read or write hook might raise an exception; - * the generated code will synchronize the CPU state before calling the hook - * so that it is safe for the hook to call raise_exception(). - * NEWEL is for writes to registers that might change the exception - * level - typically on older ARM chips. For those cases we need to - * re-read the new el when recomputing the translation flags. + * ARMCPRegInfo type field bits: */ -#define ARM_CP_SPECIAL 0x0001 -#define ARM_CP_CONST 0x0002 -#define ARM_CP_64BIT 0x0004 -#define ARM_CP_SUPPRESS_TB_END 0x0008 -#define ARM_CP_OVERRIDE 0x0010 -#define ARM_CP_ALIAS 0x0020 -#define ARM_CP_IO 0x0040 -#define ARM_CP_NO_RAW 0x0080 -#define ARM_CP_NOP (ARM_CP_SPECIAL | 0x0100) -#define ARM_CP_WFI (ARM_CP_SPECIAL | 0x0200) -#define ARM_CP_NZCV (ARM_CP_SPECIAL | 0x0300) -#define ARM_CP_CURRENTEL (ARM_CP_SPECIAL | 0x0400) -#define ARM_CP_DC_ZVA (ARM_CP_SPECIAL | 0x0500) -#define ARM_CP_DC_GVA (ARM_CP_SPECIAL | 0x0600) -#define ARM_CP_DC_GZVA (ARM_CP_SPECIAL | 0x0700) -#define ARM_LAST_SPECIAL ARM_CP_DC_GZVA -#define ARM_CP_FPU 0x1000 -#define ARM_CP_SVE 0x2000 -#define ARM_CP_NO_GDB 0x4000 -#define ARM_CP_RAISES_EXC 0x8000 -#define ARM_CP_NEWEL 0x10000 -/* Mask of only the flag bits in a type field */ -#define ARM_CP_FLAG_MASK 0x1f0ff +enum { + /* + * Register must be handled specially during translation. + * The method is one of the values below: + */ + ARM_CP_SPECIAL_MASK = 0x000f, + /* Special: no change to PE state: writes ignored, reads ignored. */ + ARM_CP_NOP = 0x0001, + /* Special: sysreg is WFI, for v5 and v6. */ + ARM_CP_WFI = 0x0002, + /* Special: sysreg is NZCV. */ + ARM_CP_NZCV = 0x0003, + /* Special: sysreg is CURRENTEL. */ + ARM_CP_CURRENTEL = 0x0004, + /* Special: sysreg is DC ZVA or similar. */ + ARM_CP_DC_ZVA = 0x0005, + ARM_CP_DC_GVA = 0x0006, + ARM_CP_DC_GZVA = 0x0007, + + /* Flag: reads produce resetvalue; writes ignored. */ + ARM_CP_CONST = 1 << 4, + /* Flag: For ARM_CP_STATE_AA32, sysreg is 64-bit. */ + ARM_CP_64BIT = 1 << 5, + /* + * Flag: TB should not be ended after a write to this register + * (the default is that the TB ends after cp writes). + */ + ARM_CP_SUPPRESS_TB_END = 1 << 6, + /* + * Flag: Permit a register definition to override a previous definition + * for the same (cp, is64, crn, crm, opc1, opc2) tuple: either the new + * or the old must have the ARM_CP_OVERRIDE bit set. + */ + ARM_CP_OVERRIDE = 1 << 7, + /* + * Flag: Register is an alias view of some underlying state which is also + * visible via another register, and that the other register is handling + * migration and reset; registers marked ARM_CP_ALIAS will not be migrated + * but may have their state set by syncing of register state from KVM. + */ + ARM_CP_ALIAS = 1 << 8, + /* + * Flag: Register does I/O and therefore its accesses need to be marked + * with gen_io_start() and also end the TB. In particular, registers which + * implement clocks or timers require this. + */ + ARM_CP_IO = 1 << 9, + /* + * Flag: Register has no underlying state and does not support raw access + * for state saving/loading; it will not be used for either migration or + * KVM state synchronization. Typically this is for "registers" which are + * actually used as instructions for cache maintenance and so on. + */ + ARM_CP_NO_RAW = 1 << 10, + /* + * Flag: The read or write hook might raise an exception; the generated + * code will synchronize the CPU state before calling the hook so that it + * is safe for the hook to call raise_exception(). + */ + ARM_CP_RAISES_EXC = 1 << 11, + /* + * Flag: Writes to the sysreg might change the exception level - typically + * on older ARM chips. For those cases we need to re-read the new el when + * recomputing the translation flags. + */ + ARM_CP_NEWEL = 1 << 12, + /* + * Flag: Access check for this sysreg is identical to accessing FPU state + * from an instruction: use translation fp_access_check(). + */ + ARM_CP_FPU = 1 << 13, + /* + * Flag: Access check for this sysreg is identical to accessing SVE state + * from an instruction: use translation sve_access_check(). + */ + ARM_CP_SVE = 1 << 14, + /* Flag: Do not expose in gdb sysreg xml. */ + ARM_CP_NO_GDB = 1 << 15, +}; /* * Valid values for ARMCPRegInfo state field, indicating which of diff --git a/target/arm/cpu.c b/target/arm/cpu.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu.c +++ b/target/arm/cpu.c @@ -XXX,XX +XXX,XX @@ static void cp_reg_reset(gpointer key, gpointer value, gpointer opaque) ARMCPRegInfo *ri = value; ARMCPU *cpu = opaque; - if (ri->type & (ARM_CP_SPECIAL | ARM_CP_ALIAS)) { + if (ri->type & (ARM_CP_SPECIAL_MASK | ARM_CP_ALIAS)) { return; } @@ -XXX,XX +XXX,XX @@ static void cp_reg_check_reset(gpointer key, gpointer value, gpointer opaque) ARMCPU *cpu = opaque; uint64_t oldvalue, newvalue; - if (ri->type & (ARM_CP_SPECIAL | ARM_CP_ALIAS | ARM_CP_NO_RAW)) { + if (ri->type & (ARM_CP_SPECIAL_MASK | ARM_CP_ALIAS | ARM_CP_NO_RAW)) { return; } diff --git a/target/arm/helper.c b/target/arm/helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -XXX,XX +XXX,XX @@ static void add_cpreg_to_hashtable(ARMCPU *cpu, const ARMCPRegInfo *r, * multiple times. Special registers (ie NOP/WFI) are * never migratable and not even raw-accessible. */ - if ((r->type & ARM_CP_SPECIAL)) { + if (r->type & ARM_CP_SPECIAL_MASK) { r2->type |= ARM_CP_NO_RAW; } if (((r->crm == CP_ANY) && crm != 0) || @@ -XXX,XX +XXX,XX @@ void define_one_arm_cp_reg_with_opaque(ARMCPU *cpu, /* Check that the register definition has enough info to handle * reads and writes if they are permitted. */ - if (!(r->type & (ARM_CP_SPECIAL|ARM_CP_CONST))) { + if (!(r->type & (ARM_CP_SPECIAL_MASK | ARM_CP_CONST))) { if (r->access & PL3_R) { assert((r->fieldoffset || (r->bank_fieldoffsets[0] && r->bank_fieldoffsets[1])) || diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-a64.c +++ b/target/arm/translate-a64.c @@ -XXX,XX +XXX,XX @@ static void handle_sys(DisasContext *s, uint32_t insn, bool isread, } /* Handle special cases first */ - switch (ri->type & ~(ARM_CP_FLAG_MASK & ~ARM_CP_SPECIAL)) { + switch (ri->type & ARM_CP_SPECIAL_MASK) { + case 0: + break; case ARM_CP_NOP: return; case ARM_CP_NZCV: @@ -XXX,XX +XXX,XX @@ static void handle_sys(DisasContext *s, uint32_t insn, bool isread, } return; default: - break; + g_assert_not_reached(); } if ((ri->type & ARM_CP_FPU) && !fp_access_check(s)) { return; diff --git a/target/arm/translate.c b/target/arm/translate.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -XXX,XX +XXX,XX @@ static void do_coproc_insn(DisasContext *s, int cpnum, int is64, } /* Handle special cases first */ - switch (ri->type & ~(ARM_CP_FLAG_MASK & ~ARM_CP_SPECIAL)) { + switch (ri->type & ARM_CP_SPECIAL_MASK) { + case 0: + break; case ARM_CP_NOP: return; case ARM_CP_WFI: @@ -XXX,XX +XXX,XX @@ static void do_coproc_insn(DisasContext *s, int cpnum, int is64, s->base.is_jmp = DISAS_WFI; return; default: - break; + g_assert_not_reached(); } if ((tb_cflags(s->base.tb) & CF_USE_ICOUNT) && (ri->type & ARM_CP_IO)) { -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Standardize on g_assert_not_reached() for "should not happen". Retain abort() when preceeded by fprintf or error_report. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 20220501055028.646596-7-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/helper.c | 7 +++---- target/arm/hvf/hvf.c | 2 +- target/arm/kvm-stub.c | 4 ++-- target/arm/kvm.c | 4 ++-- target/arm/machine.c | 4 ++-- target/arm/translate-a64.c | 4 ++-- target/arm/translate-neon.c | 2 +- target/arm/translate.c | 4 ++-- 8 files changed, 15 insertions(+), 16 deletions(-) diff --git a/target/arm/helper.c b/target/arm/helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -XXX,XX +XXX,XX @@ void define_one_arm_cp_reg_with_opaque(ARMCPU *cpu, break; default: /* broken reginfo with out-of-range opc1 */ - assert(false); - break; + g_assert_not_reached(); } /* assert our permissions are not too lax (stricter is fine) */ assert((r->access & ~mask) == 0); @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_v5(CPUARMState *env, uint32_t address, break; default: /* Never happens, but compiler isn't smart enough to tell. */ - abort(); + g_assert_not_reached(); } } *prot = ap_to_rw_prot(env, mmu_idx, ap, domain_prot); @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_v6(CPUARMState *env, uint32_t address, break; default: /* Never happens, but compiler isn't smart enough to tell. */ - abort(); + g_assert_not_reached(); } } if (domain_prot == 3) { diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/hvf/hvf.c +++ b/target/arm/hvf/hvf.c @@ -XXX,XX +XXX,XX @@ int hvf_vcpu_exec(CPUState *cpu) /* we got kicked, no exit to process */ return 0; default: - assert(0); + g_assert_not_reached(); } hvf_sync_vtimer(cpu); diff --git a/target/arm/kvm-stub.c b/target/arm/kvm-stub.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/kvm-stub.c +++ b/target/arm/kvm-stub.c @@ -XXX,XX +XXX,XX @@ bool write_kvmstate_to_list(ARMCPU *cpu) { - abort(); + g_assert_not_reached(); } bool write_list_to_kvmstate(ARMCPU *cpu, int level) { - abort(); + g_assert_not_reached(); } diff --git a/target/arm/kvm.c b/target/arm/kvm.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/kvm.c +++ b/target/arm/kvm.c @@ -XXX,XX +XXX,XX @@ bool write_kvmstate_to_list(ARMCPU *cpu) ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &r); break; default: - abort(); + g_assert_not_reached(); } if (ret) { ok = false; @@ -XXX,XX +XXX,XX @@ bool write_list_to_kvmstate(ARMCPU *cpu, int level) r.addr = (uintptr_t)(cpu->cpreg_values + i); break; default: - abort(); + g_assert_not_reached(); } ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &r); if (ret) { diff --git a/target/arm/machine.c b/target/arm/machine.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/machine.c +++ b/target/arm/machine.c @@ -XXX,XX +XXX,XX @@ static int cpu_pre_save(void *opaque) if (kvm_enabled()) { if (!write_kvmstate_to_list(cpu)) { /* This should never fail */ - abort(); + g_assert_not_reached(); } /* @@ -XXX,XX +XXX,XX @@ static int cpu_pre_save(void *opaque) } else { if (!write_cpustate_to_list(cpu, false)) { /* This should never fail. */ - abort(); + g_assert_not_reached(); } } diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-a64.c +++ b/target/arm/translate-a64.c @@ -XXX,XX +XXX,XX @@ static void handle_fp_1src_half(DisasContext *s, int opcode, int rd, int rn) gen_helper_advsimd_rinth(tcg_res, tcg_op, fpst); break; default: - abort(); + g_assert_not_reached(); } write_fp_sreg(s, rd, tcg_res); @@ -XXX,XX +XXX,XX @@ static void handle_fp_fcvt(DisasContext *s, int opcode, break; } default: - abort(); + g_assert_not_reached(); } } diff --git a/target/arm/translate-neon.c b/target/arm/translate-neon.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-neon.c +++ b/target/arm/translate-neon.c @@ -XXX,XX +XXX,XX @@ static bool trans_VLDST_single(DisasContext *s, arg_VLDST_single *a) } break; default: - abort(); + g_assert_not_reached(); } if ((vd + a->stride * (nregs - 1)) > 31) { /* diff --git a/target/arm/translate.c b/target/arm/translate.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -XXX,XX +XXX,XX @@ static void gen_srs(DisasContext *s, offset = 4; break; default: - abort(); + g_assert_not_reached(); } tcg_gen_addi_i32(addr, addr, offset); tmp = load_reg(s, 14); @@ -XXX,XX +XXX,XX @@ static void gen_srs(DisasContext *s, offset = 0; break; default: - abort(); + g_assert_not_reached(); } tcg_gen_addi_i32(addr, addr, offset); gen_helper_set_r13_banked(cpu_env, tcg_constant_i32(mode), addr); -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Create a typedef as well, and use it in ARMCPRegInfo. This won't be perfect for debugging, but it'll nicely display the most common cases. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220501055028.646596-8-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/cpregs.h | 44 +++++++++++++++++++++++--------------------- target/arm/helper.c | 2 +- 2 files changed, 24 insertions(+), 22 deletions(-) diff --git a/target/arm/cpregs.h b/target/arm/cpregs.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpregs.h +++ b/target/arm/cpregs.h @@ -XXX,XX +XXX,XX @@ enum { * described with these bits, then use a laxer set of restrictions, and * do the more restrictive/complex check inside a helper function. */ -#define PL3_R 0x80 -#define PL3_W 0x40 -#define PL2_R (0x20 | PL3_R) -#define PL2_W (0x10 | PL3_W) -#define PL1_R (0x08 | PL2_R) -#define PL1_W (0x04 | PL2_W) -#define PL0_R (0x02 | PL1_R) -#define PL0_W (0x01 | PL1_W) +typedef enum { + PL3_R = 0x80, + PL3_W = 0x40, + PL2_R = 0x20 | PL3_R, + PL2_W = 0x10 | PL3_W, + PL1_R = 0x08 | PL2_R, + PL1_W = 0x04 | PL2_W, + PL0_R = 0x02 | PL1_R, + PL0_W = 0x01 | PL1_W, -/* - * For user-mode some registers are accessible to EL0 via a kernel - * trap-and-emulate ABI. In this case we define the read permissions - * as actually being PL0_R. However some bits of any given register - * may still be masked. - */ + /* + * For user-mode some registers are accessible to EL0 via a kernel + * trap-and-emulate ABI. In this case we define the read permissions + * as actually being PL0_R. However some bits of any given register + * may still be masked. + */ #ifdef CONFIG_USER_ONLY -#define PL0U_R PL0_R + PL0U_R = PL0_R, #else -#define PL0U_R PL1_R + PL0U_R = PL1_R, #endif -#define PL3_RW (PL3_R | PL3_W) -#define PL2_RW (PL2_R | PL2_W) -#define PL1_RW (PL1_R | PL1_W) -#define PL0_RW (PL0_R | PL0_W) + PL3_RW = PL3_R | PL3_W, + PL2_RW = PL2_R | PL2_W, + PL1_RW = PL1_R | PL1_W, + PL0_RW = PL0_R | PL0_W, +} CPAccessRights; typedef enum CPAccessResult { /* Access is permitted */ @@ -XXX,XX +XXX,XX @@ struct ARMCPRegInfo { /* Register type: ARM_CP_* bits/values */ int type; /* Access rights: PL*_[RW] */ - int access; + CPAccessRights access; /* Security state: ARM_CP_SECSTATE_* bits/values */ int secure; /* diff --git a/target/arm/helper.c b/target/arm/helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -XXX,XX +XXX,XX @@ void define_one_arm_cp_reg_with_opaque(ARMCPU *cpu, * to encompass the generic architectural permission check. */ if (r->state != ARM_CP_STATE_AA32) { - int mask = 0; + CPAccessRights mask; switch (r->opc1) { case 0: /* min_EL EL1, but some accessible to EL0 via kernel ABI */ -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Give this enum a name and use in ARMCPRegInfo, add_cpreg_to_hashtable and define_one_arm_cp_reg_with_opaque. Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220501055028.646596-9-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/cpregs.h | 6 +++--- target/arm/helper.c | 6 ++++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/target/arm/cpregs.h b/target/arm/cpregs.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpregs.h +++ b/target/arm/cpregs.h @@ -XXX,XX +XXX,XX @@ enum { * Note that we rely on the values of these enums as we iterate through * the various states in some places. */ -enum { +typedef enum { ARM_CP_STATE_AA32 = 0, ARM_CP_STATE_AA64 = 1, ARM_CP_STATE_BOTH = 2, -}; +} CPState; /* * ARM CP register secure state flags. These flags identify security state @@ -XXX,XX +XXX,XX @@ struct ARMCPRegInfo { uint8_t opc1; uint8_t opc2; /* Execution state in which this register is visible: ARM_CP_STATE_* */ - int state; + CPState state; /* Register type: ARM_CP_* bits/values */ int type; /* Access rights: PL*_[RW] */ diff --git a/target/arm/helper.c b/target/arm/helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -XXX,XX +XXX,XX @@ CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp) } static void add_cpreg_to_hashtable(ARMCPU *cpu, const ARMCPRegInfo *r, - void *opaque, int state, int secstate, + void *opaque, CPState state, int secstate, int crm, int opc1, int opc2, const char *name) { @@ -XXX,XX +XXX,XX @@ void define_one_arm_cp_reg_with_opaque(ARMCPU *cpu, * bits; the ARM_CP_64BIT* flag applies only to the AArch32 view of * the register, if any. */ - int crm, opc1, opc2, state; + int crm, opc1, opc2; int crmmin = (r->crm == CP_ANY) ? 0 : r->crm; int crmmax = (r->crm == CP_ANY) ? 15 : r->crm; int opc1min = (r->opc1 == CP_ANY) ? 0 : r->opc1; int opc1max = (r->opc1 == CP_ANY) ? 7 : r->opc1; int opc2min = (r->opc2 == CP_ANY) ? 0 : r->opc2; int opc2max = (r->opc2 == CP_ANY) ? 7 : r->opc2; + CPState state; + /* 64 bit registers have only CRm and Opc1 fields */ assert(!((r->type & ARM_CP_64BIT) && (r->opc2 || r->crn))); /* op0 only exists in the AArch64 encodings */ -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Give this enum a name and use in ARMCPRegInfo and add_cpreg_to_hashtable. Add the enumerator ARM_CP_SECSTATE_BOTH to clarify how 0 is handled in define_one_arm_cp_reg_with_opaque. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220501055028.646596-10-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/cpregs.h | 7 ++++--- target/arm/helper.c | 7 +++++-- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/target/arm/cpregs.h b/target/arm/cpregs.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpregs.h +++ b/target/arm/cpregs.h @@ -XXX,XX +XXX,XX @@ typedef enum { * registered entry will only have one to identify whether the entry is secure * or non-secure. */ -enum { +typedef enum { + ARM_CP_SECSTATE_BOTH = 0, /* define one cpreg for each secstate */ ARM_CP_SECSTATE_S = (1 << 0), /* bit[0]: Secure state register */ ARM_CP_SECSTATE_NS = (1 << 1), /* bit[1]: Non-secure state register */ -}; +} CPSecureState; /* * Access rights: @@ -XXX,XX +XXX,XX @@ struct ARMCPRegInfo { /* Access rights: PL*_[RW] */ CPAccessRights access; /* Security state: ARM_CP_SECSTATE_* bits/values */ - int secure; + CPSecureState secure; /* * The opaque pointer passed to define_arm_cp_regs_with_opaque() when * this register was defined: can be used to hand data through to the diff --git a/target/arm/helper.c b/target/arm/helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -XXX,XX +XXX,XX @@ CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp) } static void add_cpreg_to_hashtable(ARMCPU *cpu, const ARMCPRegInfo *r, - void *opaque, CPState state, int secstate, + void *opaque, CPState state, + CPSecureState secstate, int crm, int opc1, int opc2, const char *name) { @@ -XXX,XX +XXX,XX @@ void define_one_arm_cp_reg_with_opaque(ARMCPU *cpu, r->secure, crm, opc1, opc2, r->name); break; - default: + case ARM_CP_SECSTATE_BOTH: name = g_strdup_printf("%s_S", r->name); add_cpreg_to_hashtable(cpu, r, opaque, state, ARM_CP_SECSTATE_S, @@ -XXX,XX +XXX,XX @@ void define_one_arm_cp_reg_with_opaque(ARMCPU *cpu, ARM_CP_SECSTATE_NS, crm, opc1, opc2, r->name); break; + default: + g_assert_not_reached(); } } else { /* AArch64 registers get mapped to non-secure instance -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> The new_key field is always non-zero -- drop the if. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 20220501055028.646596-11-richard.henderson@linaro.org [PMM: reinstated dropped PL3_RW mask] Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/helper.c | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/target/arm/helper.c b/target/arm/helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -XXX,XX +XXX,XX @@ static void define_arm_vh_e2h_redirects_aliases(ARMCPU *cpu) for (i = 0; i < ARRAY_SIZE(aliases); i++) { const struct E2HAlias *a = &aliases[i]; - ARMCPRegInfo *src_reg, *dst_reg; + ARMCPRegInfo *src_reg, *dst_reg, *new_reg; + uint32_t *new_key; + bool ok; if (a->feature && !a->feature(&cpu->isar)) { continue; @@ -XXX,XX +XXX,XX @@ static void define_arm_vh_e2h_redirects_aliases(ARMCPU *cpu) g_assert(src_reg->opaque == NULL); /* Create alias before redirection so we dup the right data. */ - if (a->new_key) { - ARMCPRegInfo *new_reg = g_memdup(src_reg, sizeof(ARMCPRegInfo)); - uint32_t *new_key = g_memdup(&a->new_key, sizeof(uint32_t)); - bool ok; + new_reg = g_memdup(src_reg, sizeof(ARMCPRegInfo)); + new_key = g_memdup(&a->new_key, sizeof(uint32_t)); - new_reg->name = a->new_name; - new_reg->type |= ARM_CP_ALIAS; - /* Remove PL1/PL0 access, leaving PL2/PL3 R/W in place. */ - new_reg->access &= PL2_RW | PL3_RW; + new_reg->name = a->new_name; + new_reg->type |= ARM_CP_ALIAS; + /* Remove PL1/PL0 access, leaving PL2/PL3 R/W in place. */ + new_reg->access &= PL2_RW | PL3_RW; - ok = g_hash_table_insert(cpu->cp_regs, new_key, new_reg); - g_assert(ok); - } + ok = g_hash_table_insert(cpu->cp_regs, new_key, new_reg); + g_assert(ok); src_reg->opaque = dst_reg; src_reg->orig_readfn = src_reg->readfn ?: raw_read; -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Cast the uint32_t key into a gpointer directly, which allows us to avoid allocating storage for each key. Use g_hash_table_lookup when we already have a gpointer (e.g. for callbacks like count_cpreg), or when using get_arm_cp_reginfo would require casting away const. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 20220501055028.646596-12-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/cpu.c | 4 ++-- target/arm/gdbstub.c | 2 +- target/arm/helper.c | 41 ++++++++++++++++++----------------------- 3 files changed, 21 insertions(+), 26 deletions(-) diff --git a/target/arm/cpu.c b/target/arm/cpu.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu.c +++ b/target/arm/cpu.c @@ -XXX,XX +XXX,XX @@ static void arm_cpu_initfn(Object *obj) ARMCPU *cpu = ARM_CPU(obj); cpu_set_cpustate_pointers(cpu); - cpu->cp_regs = g_hash_table_new_full(g_int_hash, g_int_equal, - g_free, cpreg_hashtable_data_destroy); + cpu->cp_regs = g_hash_table_new_full(g_direct_hash, g_direct_equal, + NULL, cpreg_hashtable_data_destroy); QLIST_INIT(&cpu->pre_el_change_hooks); QLIST_INIT(&cpu->el_change_hooks); diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/gdbstub.c +++ b/target/arm/gdbstub.c @@ -XXX,XX +XXX,XX @@ static void arm_gen_one_xml_sysreg_tag(GString *s, DynamicGDBXMLInfo *dyn_xml, static void arm_register_sysreg_for_xml(gpointer key, gpointer value, gpointer p) { - uint32_t ri_key = *(uint32_t *)key; + uint32_t ri_key = (uintptr_t)key; ARMCPRegInfo *ri = value; RegisterSysregXmlParam *param = (RegisterSysregXmlParam *)p; GString *s = param->s; diff --git a/target/arm/helper.c b/target/arm/helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -XXX,XX +XXX,XX @@ bool write_list_to_cpustate(ARMCPU *cpu) static void add_cpreg_to_list(gpointer key, gpointer opaque) { ARMCPU *cpu = opaque; - uint64_t regidx; - const ARMCPRegInfo *ri; - - regidx = *(uint32_t *)key; - ri = get_arm_cp_reginfo(cpu->cp_regs, regidx); + uint32_t regidx = (uintptr_t)key; + const ARMCPRegInfo *ri = get_arm_cp_reginfo(cpu->cp_regs, regidx); if (!(ri->type & (ARM_CP_NO_RAW|ARM_CP_ALIAS))) { cpu->cpreg_indexes[cpu->cpreg_array_len] = cpreg_to_kvm_id(regidx); @@ -XXX,XX +XXX,XX @@ static void add_cpreg_to_list(gpointer key, gpointer opaque) static void count_cpreg(gpointer key, gpointer opaque) { ARMCPU *cpu = opaque; - uint64_t regidx; const ARMCPRegInfo *ri; - regidx = *(uint32_t *)key; - ri = get_arm_cp_reginfo(cpu->cp_regs, regidx); + ri = g_hash_table_lookup(cpu->cp_regs, key); if (!(ri->type & (ARM_CP_NO_RAW|ARM_CP_ALIAS))) { cpu->cpreg_array_len++; @@ -XXX,XX +XXX,XX @@ static void count_cpreg(gpointer key, gpointer opaque) static gint cpreg_key_compare(gconstpointer a, gconstpointer b) { - uint64_t aidx = cpreg_to_kvm_id(*(uint32_t *)a); - uint64_t bidx = cpreg_to_kvm_id(*(uint32_t *)b); + uint64_t aidx = cpreg_to_kvm_id((uintptr_t)a); + uint64_t bidx = cpreg_to_kvm_id((uintptr_t)b); if (aidx > bidx) { return 1; @@ -XXX,XX +XXX,XX @@ static void define_arm_vh_e2h_redirects_aliases(ARMCPU *cpu) for (i = 0; i < ARRAY_SIZE(aliases); i++) { const struct E2HAlias *a = &aliases[i]; ARMCPRegInfo *src_reg, *dst_reg, *new_reg; - uint32_t *new_key; bool ok; if (a->feature && !a->feature(&cpu->isar)) { continue; } - src_reg = g_hash_table_lookup(cpu->cp_regs, &a->src_key); - dst_reg = g_hash_table_lookup(cpu->cp_regs, &a->dst_key); + src_reg = g_hash_table_lookup(cpu->cp_regs, + (gpointer)(uintptr_t)a->src_key); + dst_reg = g_hash_table_lookup(cpu->cp_regs, + (gpointer)(uintptr_t)a->dst_key); g_assert(src_reg != NULL); g_assert(dst_reg != NULL); @@ -XXX,XX +XXX,XX @@ static void define_arm_vh_e2h_redirects_aliases(ARMCPU *cpu) /* Create alias before redirection so we dup the right data. */ new_reg = g_memdup(src_reg, sizeof(ARMCPRegInfo)); - new_key = g_memdup(&a->new_key, sizeof(uint32_t)); new_reg->name = a->new_name; new_reg->type |= ARM_CP_ALIAS; /* Remove PL1/PL0 access, leaving PL2/PL3 R/W in place. */ new_reg->access &= PL2_RW | PL3_RW; - ok = g_hash_table_insert(cpu->cp_regs, new_key, new_reg); + ok = g_hash_table_insert(cpu->cp_regs, + (gpointer)(uintptr_t)a->new_key, new_reg); g_assert(ok); src_reg->opaque = dst_reg; @@ -XXX,XX +XXX,XX @@ static void add_cpreg_to_hashtable(ARMCPU *cpu, const ARMCPRegInfo *r, /* Private utility function for define_one_arm_cp_reg_with_opaque(): * add a single reginfo struct to the hash table. */ - uint32_t *key = g_new(uint32_t, 1); + uint32_t key; ARMCPRegInfo *r2 = g_memdup(r, sizeof(ARMCPRegInfo)); int is64 = (r->type & ARM_CP_64BIT) ? 1 : 0; int ns = (secstate & ARM_CP_SECSTATE_NS) ? 1 : 0; @@ -XXX,XX +XXX,XX @@ static void add_cpreg_to_hashtable(ARMCPU *cpu, const ARMCPRegInfo *r, if (r->cp == 0 || r->state == ARM_CP_STATE_BOTH) { r2->cp = CP_REG_ARM64_SYSREG_CP; } - *key = ENCODE_AA64_CP_REG(r2->cp, r2->crn, crm, - r2->opc0, opc1, opc2); + key = ENCODE_AA64_CP_REG(r2->cp, r2->crn, crm, + r2->opc0, opc1, opc2); } else { - *key = ENCODE_CP_REG(r2->cp, is64, ns, r2->crn, crm, opc1, opc2); + key = ENCODE_CP_REG(r2->cp, is64, ns, r2->crn, crm, opc1, opc2); } if (opaque) { r2->opaque = opaque; @@ -XXX,XX +XXX,XX @@ static void add_cpreg_to_hashtable(ARMCPU *cpu, const ARMCPRegInfo *r, * requested. */ if (!(r->type & ARM_CP_OVERRIDE)) { - ARMCPRegInfo *oldreg; - oldreg = g_hash_table_lookup(cpu->cp_regs, key); + const ARMCPRegInfo *oldreg = get_arm_cp_reginfo(cpu->cp_regs, key); if (oldreg && !(oldreg->type & ARM_CP_OVERRIDE)) { fprintf(stderr, "Register redefined: cp=%d %d bit " "crn=%d crm=%d opc1=%d opc2=%d, " @@ -XXX,XX +XXX,XX @@ static void add_cpreg_to_hashtable(ARMCPU *cpu, const ARMCPRegInfo *r, g_assert_not_reached(); } } - g_hash_table_insert(cpu->cp_regs, key, r2); + g_hash_table_insert(cpu->cp_regs, (gpointer)(uintptr_t)key, r2); } @@ -XXX,XX +XXX,XX @@ void modify_arm_cp_regs_with_len(ARMCPRegInfo *regs, size_t regs_len, const ARMCPRegInfo *get_arm_cp_reginfo(GHashTable *cpregs, uint32_t encoded_cp) { - return g_hash_table_lookup(cpregs, &encoded_cp); + return g_hash_table_lookup(cpregs, (gpointer)(uintptr_t)encoded_cp); } void arm_cp_write_ignore(CPUARMState *env, const ARMCPRegInfo *ri, -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Simplify freeing cp_regs hash table entries by using a single allocation for the entire value. This fixes a theoretical bug if we were to ever free the entire hash table, because we've been installing string literal constants into the cpreg structure in define_arm_vh_e2h_redirects_aliases. However, at present we only free entries created for AArch32 wildcard cpregs which get overwritten by more specific cpregs, so this bug is never exposed. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 20220501055028.646596-13-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/cpu.c | 16 +--------------- target/arm/helper.c | 10 ++++++++-- 2 files changed, 9 insertions(+), 17 deletions(-) diff --git a/target/arm/cpu.c b/target/arm/cpu.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu.c +++ b/target/arm/cpu.c @@ -XXX,XX +XXX,XX @@ uint64_t arm_cpu_mp_affinity(int idx, uint8_t clustersz) return (Aff1 << ARM_AFF1_SHIFT) | Aff0; } -static void cpreg_hashtable_data_destroy(gpointer data) -{ - /* - * Destroy function for cpu->cp_regs hashtable data entries. - * We must free the name string because it was g_strdup()ed in - * add_cpreg_to_hashtable(). It's OK to cast away the 'const' - * from r->name because we know we definitely allocated it. - */ - ARMCPRegInfo *r = data; - - g_free((void *)r->name); - g_free(r); -} - static void arm_cpu_initfn(Object *obj) { ARMCPU *cpu = ARM_CPU(obj); cpu_set_cpustate_pointers(cpu); cpu->cp_regs = g_hash_table_new_full(g_direct_hash, g_direct_equal, - NULL, cpreg_hashtable_data_destroy); + NULL, g_free); QLIST_INIT(&cpu->pre_el_change_hooks); QLIST_INIT(&cpu->el_change_hooks); diff --git a/target/arm/helper.c b/target/arm/helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -XXX,XX +XXX,XX @@ static void add_cpreg_to_hashtable(ARMCPU *cpu, const ARMCPRegInfo *r, * add a single reginfo struct to the hash table. */ uint32_t key; - ARMCPRegInfo *r2 = g_memdup(r, sizeof(ARMCPRegInfo)); + ARMCPRegInfo *r2; int is64 = (r->type & ARM_CP_64BIT) ? 1 : 0; int ns = (secstate & ARM_CP_SECSTATE_NS) ? 1 : 0; + size_t name_len; + + /* Combine cpreg and name into one allocation. */ + name_len = strlen(name) + 1; + r2 = g_malloc(sizeof(*r2) + name_len); + *r2 = *r; + r2->name = memcpy(r2 + 1, name, name_len); - r2->name = g_strdup(name); /* Reset the secure state to the specific incoming state. This is * necessary as the register may have been defined with both states. */ -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Move the computation of key to the top of the function. Hoist the resolution of cp as well, as an input to the computation of key. This will be required by a subsequent patch. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 20220501055028.646596-14-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/helper.c | 49 +++++++++++++++++++++++++-------------------- 1 file changed, 27 insertions(+), 22 deletions(-) diff --git a/target/arm/helper.c b/target/arm/helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -XXX,XX +XXX,XX @@ static void add_cpreg_to_hashtable(ARMCPU *cpu, const ARMCPRegInfo *r, ARMCPRegInfo *r2; int is64 = (r->type & ARM_CP_64BIT) ? 1 : 0; int ns = (secstate & ARM_CP_SECSTATE_NS) ? 1 : 0; + int cp = r->cp; size_t name_len; + switch (state) { + case ARM_CP_STATE_AA32: + /* We assume it is a cp15 register if the .cp field is left unset. */ + if (cp == 0 && r->state == ARM_CP_STATE_BOTH) { + cp = 15; + } + key = ENCODE_CP_REG(cp, is64, ns, r->crn, crm, opc1, opc2); + break; + case ARM_CP_STATE_AA64: + /* + * To allow abbreviation of ARMCPRegInfo definitions, we treat + * cp == 0 as equivalent to the value for "standard guest-visible + * sysreg". STATE_BOTH definitions are also always "standard sysreg" + * in their AArch64 view (the .cp value may be non-zero for the + * benefit of the AArch32 view). + */ + if (cp == 0 || r->state == ARM_CP_STATE_BOTH) { + cp = CP_REG_ARM64_SYSREG_CP; + } + key = ENCODE_AA64_CP_REG(cp, r->crn, crm, r->opc0, opc1, opc2); + break; + default: + g_assert_not_reached(); + } + /* Combine cpreg and name into one allocation. */ name_len = strlen(name) + 1; r2 = g_malloc(sizeof(*r2) + name_len); @@ -XXX,XX +XXX,XX @@ static void add_cpreg_to_hashtable(ARMCPU *cpu, const ARMCPRegInfo *r, } if (r->state == ARM_CP_STATE_BOTH) { - /* We assume it is a cp15 register if the .cp field is left unset. - */ - if (r2->cp == 0) { - r2->cp = 15; - } - #if HOST_BIG_ENDIAN if (r2->fieldoffset) { r2->fieldoffset += sizeof(uint32_t); @@ -XXX,XX +XXX,XX @@ static void add_cpreg_to_hashtable(ARMCPU *cpu, const ARMCPRegInfo *r, #endif } } - if (state == ARM_CP_STATE_AA64) { - /* To allow abbreviation of ARMCPRegInfo - * definitions, we treat cp == 0 as equivalent to - * the value for "standard guest-visible sysreg". - * STATE_BOTH definitions are also always "standard - * sysreg" in their AArch64 view (the .cp value may - * be non-zero for the benefit of the AArch32 view). - */ - if (r->cp == 0 || r->state == ARM_CP_STATE_BOTH) { - r2->cp = CP_REG_ARM64_SYSREG_CP; - } - key = ENCODE_AA64_CP_REG(r2->cp, r2->crn, crm, - r2->opc0, opc1, opc2); - } else { - key = ENCODE_CP_REG(r2->cp, is64, ns, r2->crn, crm, opc1, opc2); - } if (opaque) { r2->opaque = opaque; } @@ -XXX,XX +XXX,XX @@ static void add_cpreg_to_hashtable(ARMCPU *cpu, const ARMCPRegInfo *r, /* Make sure reginfo passed to helpers for wildcarded regs * has the correct crm/opc1/opc2 for this reg, not CP_ANY: */ + r2->cp = cp; r2->crm = crm; r2->opc1 = opc1; r2->opc2 = opc2; -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Put most of the value writeback to the same place, and improve the comment that goes with them. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 20220501055028.646596-15-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/helper.c | 28 ++++++++++++---------------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/target/arm/helper.c b/target/arm/helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -XXX,XX +XXX,XX @@ static void add_cpreg_to_hashtable(ARMCPU *cpu, const ARMCPRegInfo *r, *r2 = *r; r2->name = memcpy(r2 + 1, name, name_len); - /* Reset the secure state to the specific incoming state. This is - * necessary as the register may have been defined with both states. + /* + * Update fields to match the instantiation, overwiting wildcards + * such as CP_ANY, ARM_CP_STATE_BOTH, or ARM_CP_SECSTATE_BOTH. */ + r2->cp = cp; + r2->crm = crm; + r2->opc1 = opc1; + r2->opc2 = opc2; + r2->state = state; r2->secure = secstate; + if (opaque) { + r2->opaque = opaque; + } if (r->bank_fieldoffsets[0] && r->bank_fieldoffsets[1]) { /* Register is banked (using both entries in array). @@ -XXX,XX +XXX,XX @@ static void add_cpreg_to_hashtable(ARMCPU *cpu, const ARMCPRegInfo *r, #endif } } - if (opaque) { - r2->opaque = opaque; - } - /* reginfo passed to helpers is correct for the actual access, - * and is never ARM_CP_STATE_BOTH: - */ - r2->state = state; - /* Make sure reginfo passed to helpers for wildcarded regs - * has the correct crm/opc1/opc2 for this reg, not CP_ANY: - */ - r2->cp = cp; - r2->crm = crm; - r2->opc1 = opc1; - r2->opc2 = opc2; + /* By convention, for wildcarded registers only the first * entry is used for migration; the others are marked as * ALIAS so we don't try to transfer the register -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Bool is a more appropriate type for these variables. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 20220501055028.646596-16-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/helper.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/target/arm/helper.c b/target/arm/helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -XXX,XX +XXX,XX @@ static void add_cpreg_to_hashtable(ARMCPU *cpu, const ARMCPRegInfo *r, */ uint32_t key; ARMCPRegInfo *r2; - int is64 = (r->type & ARM_CP_64BIT) ? 1 : 0; - int ns = (secstate & ARM_CP_SECSTATE_NS) ? 1 : 0; + bool is64 = r->type & ARM_CP_64BIT; + bool ns = secstate & ARM_CP_SECSTATE_NS; int cp = r->cp; size_t name_len; -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Computing isbanked only once makes the code a bit easier to read. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 20220501055028.646596-17-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/helper.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/target/arm/helper.c b/target/arm/helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -XXX,XX +XXX,XX @@ static void add_cpreg_to_hashtable(ARMCPU *cpu, const ARMCPRegInfo *r, bool is64 = r->type & ARM_CP_64BIT; bool ns = secstate & ARM_CP_SECSTATE_NS; int cp = r->cp; + bool isbanked; size_t name_len; switch (state) { @@ -XXX,XX +XXX,XX @@ static void add_cpreg_to_hashtable(ARMCPU *cpu, const ARMCPRegInfo *r, r2->opaque = opaque; } - if (r->bank_fieldoffsets[0] && r->bank_fieldoffsets[1]) { + isbanked = r->bank_fieldoffsets[0] && r->bank_fieldoffsets[1]; + if (isbanked) { /* Register is banked (using both entries in array). * Overwriting fieldoffset as the array is only used to define * banked registers but later only fieldoffset is used. @@ -XXX,XX +XXX,XX @@ static void add_cpreg_to_hashtable(ARMCPU *cpu, const ARMCPRegInfo *r, } if (state == ARM_CP_STATE_AA32) { - if (r->bank_fieldoffsets[0] && r->bank_fieldoffsets[1]) { + if (isbanked) { /* If the register is banked then we don't need to migrate or * reset the 32-bit instance in certain cases: * -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Perform the override check early, so that it is still done even when we decide to discard an unreachable cpreg. Use assert not printf+abort. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 20220501055028.646596-18-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/helper.c | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/target/arm/helper.c b/target/arm/helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -XXX,XX +XXX,XX @@ static void add_cpreg_to_hashtable(ARMCPU *cpu, const ARMCPRegInfo *r, g_assert_not_reached(); } + /* Overriding of an existing definition must be explicitly requested. */ + if (!(r->type & ARM_CP_OVERRIDE)) { + const ARMCPRegInfo *oldreg = get_arm_cp_reginfo(cpu->cp_regs, key); + if (oldreg) { + assert(oldreg->type & ARM_CP_OVERRIDE); + } + } + /* Combine cpreg and name into one allocation. */ name_len = strlen(name) + 1; r2 = g_malloc(sizeof(*r2) + name_len); @@ -XXX,XX +XXX,XX @@ static void add_cpreg_to_hashtable(ARMCPU *cpu, const ARMCPRegInfo *r, assert(!raw_accessors_invalid(r2)); } - /* Overriding of an existing definition must be explicitly - * requested. - */ - if (!(r->type & ARM_CP_OVERRIDE)) { - const ARMCPRegInfo *oldreg = get_arm_cp_reginfo(cpu->cp_regs, key); - if (oldreg && !(oldreg->type & ARM_CP_OVERRIDE)) { - fprintf(stderr, "Register redefined: cp=%d %d bit " - "crn=%d crm=%d opc1=%d opc2=%d, " - "was %s, now %s\n", r2->cp, 32 + 32 * is64, - r2->crn, r2->crm, r2->opc1, r2->opc2, - oldreg->name, r2->name); - g_assert_not_reached(); - } - } g_hash_table_insert(cpu->cp_regs, (gpointer)(uintptr_t)key, r2); } -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Put the block comments into the current coding style. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 20220501055028.646596-19-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/helper.c | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/target/arm/helper.c b/target/arm/helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -XXX,XX +XXX,XX @@ CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp) return cpu_list; } +/* + * Private utility function for define_one_arm_cp_reg_with_opaque(): + * add a single reginfo struct to the hash table. + */ static void add_cpreg_to_hashtable(ARMCPU *cpu, const ARMCPRegInfo *r, void *opaque, CPState state, CPSecureState secstate, int crm, int opc1, int opc2, const char *name) { - /* Private utility function for define_one_arm_cp_reg_with_opaque(): - * add a single reginfo struct to the hash table. - */ uint32_t key; ARMCPRegInfo *r2; bool is64 = r->type & ARM_CP_64BIT; @@ -XXX,XX +XXX,XX @@ static void add_cpreg_to_hashtable(ARMCPU *cpu, const ARMCPRegInfo *r, isbanked = r->bank_fieldoffsets[0] && r->bank_fieldoffsets[1]; if (isbanked) { - /* Register is banked (using both entries in array). + /* + * Register is banked (using both entries in array). * Overwriting fieldoffset as the array is only used to define * banked registers but later only fieldoffset is used. */ @@ -XXX,XX +XXX,XX @@ static void add_cpreg_to_hashtable(ARMCPU *cpu, const ARMCPRegInfo *r, if (state == ARM_CP_STATE_AA32) { if (isbanked) { - /* If the register is banked then we don't need to migrate or + /* + * If the register is banked then we don't need to migrate or * reset the 32-bit instance in certain cases: * * 1) If the register has both 32-bit and 64-bit instances then we @@ -XXX,XX +XXX,XX @@ static void add_cpreg_to_hashtable(ARMCPU *cpu, const ARMCPRegInfo *r, r2->type |= ARM_CP_ALIAS; } } else if ((secstate != r->secure) && !ns) { - /* The register is not banked so we only want to allow migration of - * the non-secure instance. + /* + * The register is not banked so we only want to allow migration + * of the non-secure instance. */ r2->type |= ARM_CP_ALIAS; } @@ -XXX,XX +XXX,XX @@ static void add_cpreg_to_hashtable(ARMCPU *cpu, const ARMCPRegInfo *r, } } - /* By convention, for wildcarded registers only the first + /* + * By convention, for wildcarded registers only the first * entry is used for migration; the others are marked as * ALIAS so we don't try to transfer the register * multiple times. Special registers (ie NOP/WFI) are @@ -XXX,XX +XXX,XX @@ static void add_cpreg_to_hashtable(ARMCPU *cpu, const ARMCPRegInfo *r, r2->type |= ARM_CP_ALIAS | ARM_CP_NO_GDB; } - /* Check that raw accesses are either forbidden or handled. Note that + /* + * Check that raw accesses are either forbidden or handled. Note that * we can't assert this earlier because the setup of fieldoffset for * banked registers has to be done first. */ -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Since e03b56863d2bc, our host endian indicator is unconditionally set, which means that we can use a normal C condition. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 20220501055028.646596-20-richard.henderson@linaro.org [PMM: quote correct git hash in commit message] Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/helper.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/target/arm/helper.c b/target/arm/helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -XXX,XX +XXX,XX @@ static void add_cpreg_to_hashtable(ARMCPU *cpu, const ARMCPRegInfo *r, r2->type |= ARM_CP_ALIAS; } - if (r->state == ARM_CP_STATE_BOTH) { -#if HOST_BIG_ENDIAN - if (r2->fieldoffset) { - r2->fieldoffset += sizeof(uint32_t); - } -#endif + if (HOST_BIG_ENDIAN && + r->state == ARM_CP_STATE_BOTH && r2->fieldoffset) { + r2->fieldoffset += sizeof(uint32_t); } } -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220501055028.646596-24-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/cpu.h | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/target/arm/cpu.h b/target/arm/cpu.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -XXX,XX +XXX,XX @@ static inline bool isar_feature_aa32_ssbs(const ARMISARegisters *id) return FIELD_EX32(id->id_pfr2, ID_PFR2, SSBS) != 0; } +static inline bool isar_feature_aa32_debugv8p2(const ARMISARegisters *id) +{ + return FIELD_EX32(id->id_dfr0, ID_DFR0, COPDBG) >= 8; +} + /* * 64-bit feature tests via id registers. */ @@ -XXX,XX +XXX,XX @@ static inline bool isar_feature_aa64_ssbs(const ARMISARegisters *id) return FIELD_EX64(id->id_aa64pfr1, ID_AA64PFR1, SSBS) != 0; } +static inline bool isar_feature_aa64_debugv8p2(const ARMISARegisters *id) +{ + return FIELD_EX64(id->id_aa64dfr0, ID_AA64DFR0, DEBUGVER) >= 8; +} + static inline bool isar_feature_aa64_sve2(const ARMISARegisters *id) { return FIELD_EX64(id->id_aa64zfr0, ID_AA64ZFR0, SVEVER) != 0; @@ -XXX,XX +XXX,XX @@ static inline bool isar_feature_any_tts2uxn(const ARMISARegisters *id) return isar_feature_aa64_tts2uxn(id) || isar_feature_aa32_tts2uxn(id); } +static inline bool isar_feature_any_debugv8p2(const ARMISARegisters *id) +{ + return isar_feature_aa64_debugv8p2(id) || isar_feature_aa32_debugv8p2(id); +} + /* * Forward to the above feature tests given an ARMCPU pointer. */ -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Add the aa64 predicate for detecting RAS support from id registers. We already have the aa32 version from the M-profile work. Add the 'any' predicate for testing both aa64 and aa32. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220501055028.646596-34-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/cpu.h | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/target/arm/cpu.h b/target/arm/cpu.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -XXX,XX +XXX,XX @@ static inline bool isar_feature_aa64_aa32_el1(const ARMISARegisters *id) return FIELD_EX64(id->id_aa64pfr0, ID_AA64PFR0, EL1) >= 2; } +static inline bool isar_feature_aa64_ras(const ARMISARegisters *id) +{ + return FIELD_EX64(id->id_aa64pfr0, ID_AA64PFR0, RAS) != 0; +} + static inline bool isar_feature_aa64_sve(const ARMISARegisters *id) { return FIELD_EX64(id->id_aa64pfr0, ID_AA64PFR0, SVE) != 0; @@ -XXX,XX +XXX,XX @@ static inline bool isar_feature_any_debugv8p2(const ARMISARegisters *id) return isar_feature_aa64_debugv8p2(id) || isar_feature_aa32_debugv8p2(id); } +static inline bool isar_feature_any_ras(const ARMISARegisters *id) +{ + return isar_feature_aa64_ras(id) || isar_feature_aa32_ras(id); +} + /* * Forward to the above feature tests given an ARMCPU pointer. */ -- 2.25.1 From: Alex Zuepke <alex.zuepke@tum.de> The ARMv8 manual defines that PMUSERENR_EL0.ER enables read-access to both PMXEVCNTR_EL0 and PMEVCNTR<n>_EL0 registers, however, we only use it for PMXEVCNTR_EL0. Extend to PMEVCNTR<n>_EL0 as well. Signed-off-by: Alex Zuepke <alex.zuepke@tum.de> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220428132717.84190-1-alex.zuepke@tum.de Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/helper.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/target/arm/helper.c b/target/arm/helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -XXX,XX +XXX,XX @@ static void define_pmu_regs(ARMCPU *cpu) .crm = 8 | (3 & (i >> 3)), .opc1 = 0, .opc2 = i & 7, .access = PL0_RW, .type = ARM_CP_IO | ARM_CP_ALIAS, .readfn = pmevcntr_readfn, .writefn = pmevcntr_writefn, - .accessfn = pmreg_access }, + .accessfn = pmreg_access_xevcntr }, { .name = pmevcntr_el0_name, .state = ARM_CP_STATE_AA64, .opc0 = 3, .opc1 = 3, .crn = 14, .crm = 8 | (3 & (i >> 3)), - .opc2 = i & 7, .access = PL0_RW, .accessfn = pmreg_access, + .opc2 = i & 7, .access = PL0_RW, .accessfn = pmreg_access_xevcntr, .type = ARM_CP_IO, .readfn = pmevcntr_readfn, .writefn = pmevcntr_writefn, .raw_readfn = pmevcntr_rawread, -- 2.25.1 Hi; this pullreq contains mainly a chunk of RTH's refactoring of the Arm pagetable walk code, plus a series from me fixing configure checkpatch warnings, and some old patches to various files all over the tree getting rid of dynamic stack allocation. thanks -- PMM The following changes since commit 6338c30111d596d955e6bc933a82184a0b910c43: Merge tag 'm68k-for-7.2-pull-request' of https://github.com/vivier/qemu-m68k into staging (2022-09-21 13:12:36 -0400) are available in the Git repository at: https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20220922 for you to fetch changes up to b3b5472db0ab7a53499441c1fe1dedec05b1e285: configure: Avoid use of 'local' as it is non-POSIX (2022-09-22 16:38:29 +0100) ---------------------------------------------------------------- target-arm queue: * hw/net/can: fix Xilinx ZynqMP CAN RX FIFO logic * Fix alignment for Neon VLD4.32 * Refactoring of page-table-walk code * hw/acpi: Add ospm_status hook implementation for acpi-ged * hw/net/lan9118: Signal TSFL_INT flag when TX FIFO reaches specified level * chardev/baum: avoid variable-length arrays * io/channel-websock: avoid variable-length arrays * hw/net/e1000e_core: Use definition to avoid dynamic stack allocation * hw/ppc/pnv: Avoid dynamic stack allocation * hw/intc/xics: Avoid dynamic stack allocation * hw/i386/multiboot: Avoid dynamic stack allocation * hw/usb/hcd-ohci: Use definition to avoid dynamic stack allocation * ui/curses: Avoid dynamic stack allocation * tests/unit/test-vmstate: Avoid dynamic stack allocation * configure: fix various shellcheck-spotted issues and nits ---------------------------------------------------------------- Anton Kochkov (1): hw/net/can: fix Xilinx ZynqMP CAN RX FIFO logic Clément Chigot (1): target/arm: Fix alignment for VLD4.32 Keqian Zhu (1): hw/acpi: Add ospm_status hook implementation for acpi-ged Lucas Dietrich (1): hw/net/lan9118: Signal TSFL_INT flag when TX FIFO reaches specified level Peter Maydell (7): configure: Remove unused python_version variable configure: Remove unused meson_args variable configure: Add missing quoting for some easy cases configure: Add './' on front of glob of */config-devices.mak.d configure: Remove use of backtick `...` syntax configure: Check mkdir result directly, not via $? configure: Avoid use of 'local' as it is non-POSIX Philippe Mathieu-Daudé (11): chardev/baum: Replace magic values by X_MAX / Y_MAX definitions chardev/baum: Use definitions to avoid dynamic stack allocation chardev/baum: Avoid dynamic stack allocation io/channel-websock: Replace strlen(const_str) by sizeof(const_str) - 1 hw/net/e1000e_core: Use definition to avoid dynamic stack allocation hw/ppc/pnv: Avoid dynamic stack allocation hw/intc/xics: Avoid dynamic stack allocation hw/i386/multiboot: Avoid dynamic stack allocation hw/usb/hcd-ohci: Use definition to avoid dynamic stack allocation ui/curses: Avoid dynamic stack allocation tests/unit/test-vmstate: Avoid dynamic stack allocation Richard Henderson (17): target/arm: Create GetPhysAddrResult target/arm: Use GetPhysAddrResult in get_phys_addr_lpae target/arm: Use GetPhysAddrResult in get_phys_addr_v6 target/arm: Use GetPhysAddrResult in get_phys_addr_v5 target/arm: Use GetPhysAddrResult in get_phys_addr_pmsav5 target/arm: Use GetPhysAddrResult in get_phys_addr_pmsav7 target/arm: Use GetPhysAddrResult in get_phys_addr_pmsav8 target/arm: Use GetPhysAddrResult in pmsav8_mpu_lookup target/arm: Remove is_subpage argument to pmsav8_mpu_lookup target/arm: Add is_secure parameter to v8m_security_lookup target/arm: Add secure parameter to pmsav8_mpu_lookup target/arm: Add is_secure parameter to get_phys_addr_v5 target/arm: Add is_secure parameter to get_phys_addr_v6 target/arm: Add secure parameter to get_phys_addr_pmsav8 target/arm: Add is_secure parameter to pmsav7_use_background_region target/arm: Add secure parameter to get_phys_addr_pmsav7 target/arm: Add is_secure parameter to get_phys_addr_pmsav5 configure | 82 +++++----- target/arm/internals.h | 26 +-- chardev/baum.c | 22 ++- hw/acpi/generic_event_device.c | 8 + hw/i386/multiboot.c | 5 +- hw/intc/xics.c | 2 +- hw/net/can/xlnx-zynqmp-can.c | 32 ++-- hw/net/e1000e_core.c | 7 +- hw/net/lan9118.c | 8 + hw/ppc/pnv.c | 4 +- hw/ppc/spapr.c | 8 +- hw/ppc/spapr_pci_nvlink2.c | 2 +- hw/usb/hcd-ohci.c | 7 +- io/channel-websock.c | 2 +- target/arm/helper.c | 27 ++- target/arm/m_helper.c | 78 ++++----- target/arm/ptw.c | 364 +++++++++++++++++++---------------------- target/arm/tlb_helper.c | 22 +-- target/arm/translate-neon.c | 6 +- tests/unit/test-vmstate.c | 7 +- ui/curses.c | 2 +- 21 files changed, 347 insertions(+), 374 deletions(-) From: Anton Kochkov <anton.kochkov@proton.me> For consistency, function "update_rx_fifo()" should use the RX FIFO register field names, not the TX FIFO ones, even if they refer to the same bit positions in the register. Signed-off-by: Anton Kochkov <anton.kochkov@proton.me> Reviewed-by: Francisco Iglesias <frasse.iglesias@gmail.com> Message-id: 20220817141754.2105981-1-anton.kochkov@proton.me Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1123 [PMM: tweaked commit message] Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/net/can/xlnx-zynqmp-can.c | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/hw/net/can/xlnx-zynqmp-can.c b/hw/net/can/xlnx-zynqmp-can.c index XXXXXXX..XXXXXXX 100644 --- a/hw/net/can/xlnx-zynqmp-can.c +++ b/hw/net/can/xlnx-zynqmp-can.c @@ -XXX,XX +XXX,XX @@ static void update_rx_fifo(XlnxZynqMPCANState *s, const qemu_can_frame *frame) timestamp)); /* First 32 bit of the data. */ - fifo32_push(&s->rx_fifo, deposit32(0, R_TXFIFO_DATA1_DB3_SHIFT, - R_TXFIFO_DATA1_DB3_LENGTH, + fifo32_push(&s->rx_fifo, deposit32(0, R_RXFIFO_DATA1_DB3_SHIFT, + R_RXFIFO_DATA1_DB3_LENGTH, frame->data[0]) | - deposit32(0, R_TXFIFO_DATA1_DB2_SHIFT, - R_TXFIFO_DATA1_DB2_LENGTH, + deposit32(0, R_RXFIFO_DATA1_DB2_SHIFT, + R_RXFIFO_DATA1_DB2_LENGTH, frame->data[1]) | - deposit32(0, R_TXFIFO_DATA1_DB1_SHIFT, - R_TXFIFO_DATA1_DB1_LENGTH, + deposit32(0, R_RXFIFO_DATA1_DB1_SHIFT, + R_RXFIFO_DATA1_DB1_LENGTH, frame->data[2]) | - deposit32(0, R_TXFIFO_DATA1_DB0_SHIFT, - R_TXFIFO_DATA1_DB0_LENGTH, + deposit32(0, R_RXFIFO_DATA1_DB0_SHIFT, + R_RXFIFO_DATA1_DB0_LENGTH, frame->data[3])); /* Last 32 bit of the data. */ - fifo32_push(&s->rx_fifo, deposit32(0, R_TXFIFO_DATA2_DB7_SHIFT, - R_TXFIFO_DATA2_DB7_LENGTH, + fifo32_push(&s->rx_fifo, deposit32(0, R_RXFIFO_DATA2_DB7_SHIFT, + R_RXFIFO_DATA2_DB7_LENGTH, frame->data[4]) | - deposit32(0, R_TXFIFO_DATA2_DB6_SHIFT, - R_TXFIFO_DATA2_DB6_LENGTH, + deposit32(0, R_RXFIFO_DATA2_DB6_SHIFT, + R_RXFIFO_DATA2_DB6_LENGTH, frame->data[5]) | - deposit32(0, R_TXFIFO_DATA2_DB5_SHIFT, - R_TXFIFO_DATA2_DB5_LENGTH, + deposit32(0, R_RXFIFO_DATA2_DB5_SHIFT, + R_RXFIFO_DATA2_DB5_LENGTH, frame->data[6]) | - deposit32(0, R_TXFIFO_DATA2_DB4_SHIFT, - R_TXFIFO_DATA2_DB4_LENGTH, + deposit32(0, R_RXFIFO_DATA2_DB4_SHIFT, + R_RXFIFO_DATA2_DB4_LENGTH, frame->data[7])); ARRAY_FIELD_DP32(s->regs, INTERRUPT_STATUS_REGISTER, RXOK, 1); -- 2.25.1 From: Clément Chigot <chigot@adacore.com> When requested, the alignment for VLD4.32 is 8 and not 16. See ARM documentation about VLD4 encoding: ebytes = 1 << UInt(size); if size == '10' then alignment = if a == '0' then 1 else 8; else alignment = if a == '0' then 1 else 4*ebytes; Signed-off-by: Clément Chigot <chigot@adacore.com> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220914105058.2787404-1-chigot@adacore.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/translate-neon.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/target/arm/translate-neon.c b/target/arm/translate-neon.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-neon.c +++ b/target/arm/translate-neon.c @@ -XXX,XX +XXX,XX @@ static bool trans_VLD_all_lanes(DisasContext *s, arg_VLD_all_lanes *a) case 3: return false; case 4: - align = pow2_align(size + 2); + if (size == 2) { + align = pow2_align(3); + } else { + align = pow2_align(size + 2); + } break; default: g_assert_not_reached(); -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Combine 5 output pointer arguments from get_phys_addr into a single struct. Adjust all callers. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220822152741.1617527-2-richard.henderson@linaro.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/internals.h | 13 ++++- target/arm/helper.c | 27 ++++----- target/arm/m_helper.c | 52 ++++++----------- target/arm/ptw.c | 120 +++++++++++++++++++++------------------- target/arm/tlb_helper.c | 22 +++----- 5 files changed, 109 insertions(+), 125 deletions(-) diff --git a/target/arm/internals.h b/target/arm/internals.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/internals.h +++ b/target/arm/internals.h @@ -XXX,XX +XXX,XX @@ typedef struct ARMCacheAttrs { bool is_s2_format:1; } ARMCacheAttrs; +/* Fields that are valid upon success. */ +typedef struct GetPhysAddrResult { + hwaddr phys; + target_ulong page_size; + int prot; + MemTxAttrs attrs; + ARMCacheAttrs cacheattrs; +} GetPhysAddrResult; + bool get_phys_addr(CPUARMState *env, target_ulong address, MMUAccessType access_type, ARMMMUIdx mmu_idx, - hwaddr *phys_ptr, MemTxAttrs *attrs, int *prot, - target_ulong *page_size, - ARMMMUFaultInfo *fi, ARMCacheAttrs *cacheattrs) + GetPhysAddrResult *result, ARMMMUFaultInfo *fi) __attribute__((nonnull)); void arm_log_exception(CPUState *cs); diff --git a/target/arm/helper.c b/target/arm/helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -XXX,XX +XXX,XX @@ static CPAccessResult ats_access(CPUARMState *env, const ARMCPRegInfo *ri, static uint64_t do_ats_write(CPUARMState *env, uint64_t value, MMUAccessType access_type, ARMMMUIdx mmu_idx) { - hwaddr phys_addr; - target_ulong page_size; - int prot; bool ret; uint64_t par64; bool format64 = false; - MemTxAttrs attrs = {}; ARMMMUFaultInfo fi = {}; - ARMCacheAttrs cacheattrs = {}; + GetPhysAddrResult res = {}; - ret = get_phys_addr(env, value, access_type, mmu_idx, &phys_addr, &attrs, - &prot, &page_size, &fi, &cacheattrs); + ret = get_phys_addr(env, value, access_type, mmu_idx, &res, &fi); /* * ATS operations only do S1 or S1+S2 translations, so we never * have to deal with the ARMCacheAttrs format for S2 only. */ - assert(!cacheattrs.is_s2_format); + assert(!res.cacheattrs.is_s2_format); if (ret) { /* @@ -XXX,XX +XXX,XX @@ static uint64_t do_ats_write(CPUARMState *env, uint64_t value, /* Create a 64-bit PAR */ par64 = (1 << 11); /* LPAE bit always set */ if (!ret) { - par64 |= phys_addr & ~0xfffULL; - if (!attrs.secure) { + par64 |= res.phys & ~0xfffULL; + if (!res.attrs.secure) { par64 |= (1 << 9); /* NS */ } - par64 |= (uint64_t)cacheattrs.attrs << 56; /* ATTR */ - par64 |= cacheattrs.shareability << 7; /* SH */ + par64 |= (uint64_t)res.cacheattrs.attrs << 56; /* ATTR */ + par64 |= res.cacheattrs.shareability << 7; /* SH */ } else { uint32_t fsr = arm_fi_to_lfsc(&fi); @@ -XXX,XX +XXX,XX @@ static uint64_t do_ats_write(CPUARMState *env, uint64_t value, */ if (!ret) { /* We do not set any attribute bits in the PAR */ - if (page_size == (1 << 24) + if (res.page_size == (1 << 24) && arm_feature(env, ARM_FEATURE_V7)) { - par64 = (phys_addr & 0xff000000) | (1 << 1); + par64 = (res.phys & 0xff000000) | (1 << 1); } else { - par64 = phys_addr & 0xfffff000; + par64 = res.phys & 0xfffff000; } - if (!attrs.secure) { + if (!res.attrs.secure) { par64 |= (1 << 9); /* NS */ } } else { diff --git a/target/arm/m_helper.c b/target/arm/m_helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/m_helper.c +++ b/target/arm/m_helper.c @@ -XXX,XX +XXX,XX @@ static bool v7m_stack_write(ARMCPU *cpu, uint32_t addr, uint32_t value, { CPUState *cs = CPU(cpu); CPUARMState *env = &cpu->env; - MemTxAttrs attrs = {}; MemTxResult txres; - target_ulong page_size; - hwaddr physaddr; - int prot; + GetPhysAddrResult res = {}; ARMMMUFaultInfo fi = {}; - ARMCacheAttrs cacheattrs = {}; bool secure = mmu_idx & ARM_MMU_IDX_M_S; int exc; bool exc_secure; - if (get_phys_addr(env, addr, MMU_DATA_STORE, mmu_idx, &physaddr, - &attrs, &prot, &page_size, &fi, &cacheattrs)) { + if (get_phys_addr(env, addr, MMU_DATA_STORE, mmu_idx, &res, &fi)) { /* MPU/SAU lookup failed */ if (fi.type == ARMFault_QEMU_SFault) { if (mode == STACK_LAZYFP) { @@ -XXX,XX +XXX,XX @@ static bool v7m_stack_write(ARMCPU *cpu, uint32_t addr, uint32_t value, } goto pend_fault; } - address_space_stl_le(arm_addressspace(cs, attrs), physaddr, value, - attrs, &txres); + address_space_stl_le(arm_addressspace(cs, res.attrs), res.phys, value, + res.attrs, &txres); if (txres != MEMTX_OK) { /* BusFault trying to write the data */ if (mode == STACK_LAZYFP) { @@ -XXX,XX +XXX,XX @@ static bool v7m_stack_read(ARMCPU *cpu, uint32_t *dest, uint32_t addr, { CPUState *cs = CPU(cpu); CPUARMState *env = &cpu->env; - MemTxAttrs attrs = {}; MemTxResult txres; - target_ulong page_size; - hwaddr physaddr; - int prot; + GetPhysAddrResult res = {}; ARMMMUFaultInfo fi = {}; - ARMCacheAttrs cacheattrs = {}; bool secure = mmu_idx & ARM_MMU_IDX_M_S; int exc; bool exc_secure; uint32_t value; - if (get_phys_addr(env, addr, MMU_DATA_LOAD, mmu_idx, &physaddr, - &attrs, &prot, &page_size, &fi, &cacheattrs)) { + if (get_phys_addr(env, addr, MMU_DATA_LOAD, mmu_idx, &res, &fi)) { /* MPU/SAU lookup failed */ if (fi.type == ARMFault_QEMU_SFault) { qemu_log_mask(CPU_LOG_INT, @@ -XXX,XX +XXX,XX @@ static bool v7m_stack_read(ARMCPU *cpu, uint32_t *dest, uint32_t addr, goto pend_fault; } - value = address_space_ldl(arm_addressspace(cs, attrs), physaddr, - attrs, &txres); + value = address_space_ldl(arm_addressspace(cs, res.attrs), res.phys, + res.attrs, &txres); if (txres != MEMTX_OK) { /* BusFault trying to read the data */ qemu_log_mask(CPU_LOG_INT, "...BusFault with BFSR.UNSTKERR\n"); @@ -XXX,XX +XXX,XX @@ static bool v7m_read_half_insn(ARMCPU *cpu, ARMMMUIdx mmu_idx, CPUState *cs = CPU(cpu); CPUARMState *env = &cpu->env; V8M_SAttributes sattrs = {}; - MemTxAttrs attrs = {}; + GetPhysAddrResult res = {}; ARMMMUFaultInfo fi = {}; - ARMCacheAttrs cacheattrs = {}; MemTxResult txres; - target_ulong page_size; - hwaddr physaddr; - int prot; v8m_security_lookup(env, addr, MMU_INST_FETCH, mmu_idx, &sattrs); if (!sattrs.nsc || sattrs.ns) { @@ -XXX,XX +XXX,XX @@ static bool v7m_read_half_insn(ARMCPU *cpu, ARMMMUIdx mmu_idx, "...really SecureFault with SFSR.INVEP\n"); return false; } - if (get_phys_addr(env, addr, MMU_INST_FETCH, mmu_idx, &physaddr, - &attrs, &prot, &page_size, &fi, &cacheattrs)) { + if (get_phys_addr(env, addr, MMU_INST_FETCH, mmu_idx, &res, &fi)) { /* the MPU lookup failed */ env->v7m.cfsr[env->v7m.secure] |= R_V7M_CFSR_IACCVIOL_MASK; armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_MEM, env->v7m.secure); qemu_log_mask(CPU_LOG_INT, "...really MemManage with CFSR.IACCVIOL\n"); return false; } - *insn = address_space_lduw_le(arm_addressspace(cs, attrs), physaddr, - attrs, &txres); + *insn = address_space_lduw_le(arm_addressspace(cs, res.attrs), res.phys, + res.attrs, &txres); if (txres != MEMTX_OK) { env->v7m.cfsr[M_REG_NS] |= R_V7M_CFSR_IBUSERR_MASK; armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_BUS, false); @@ -XXX,XX +XXX,XX @@ static bool v7m_read_sg_stack_word(ARMCPU *cpu, ARMMMUIdx mmu_idx, */ CPUState *cs = CPU(cpu); CPUARMState *env = &cpu->env; - MemTxAttrs attrs = {}; MemTxResult txres; - target_ulong page_size; - hwaddr physaddr; - int prot; + GetPhysAddrResult res = {}; ARMMMUFaultInfo fi = {}; - ARMCacheAttrs cacheattrs = {}; uint32_t value; - if (get_phys_addr(env, addr, MMU_DATA_LOAD, mmu_idx, &physaddr, - &attrs, &prot, &page_size, &fi, &cacheattrs)) { + if (get_phys_addr(env, addr, MMU_DATA_LOAD, mmu_idx, &res, &fi)) { /* MPU/SAU lookup failed */ if (fi.type == ARMFault_QEMU_SFault) { qemu_log_mask(CPU_LOG_INT, @@ -XXX,XX +XXX,XX @@ static bool v7m_read_sg_stack_word(ARMCPU *cpu, ARMMMUIdx mmu_idx, } return false; } - value = address_space_ldl(arm_addressspace(cs, attrs), physaddr, - attrs, &txres); + value = address_space_ldl(arm_addressspace(cs, res.attrs), res.phys, + res.attrs, &txres); if (txres != MEMTX_OK) { /* BusFault trying to read the data */ qemu_log_mask(CPU_LOG_INT, diff --git a/target/arm/ptw.c b/target/arm/ptw.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/ptw.c +++ b/target/arm/ptw.c @@ -XXX,XX +XXX,XX @@ static ARMCacheAttrs combine_cacheattrs(CPUARMState *env, * @address: virtual address to get physical address for * @access_type: 0 for read, 1 for write, 2 for execute * @mmu_idx: MMU index indicating required translation regime - * @phys_ptr: set to the physical address corresponding to the virtual address - * @attrs: set to the memory transaction attributes to use - * @prot: set to the permissions for the page containing phys_ptr - * @page_size: set to the size of the page containing phys_ptr + * @result: set on translation success. * @fi: set to fault info if the translation fails - * @cacheattrs: (if non-NULL) set to the cacheability/shareability attributes */ bool get_phys_addr(CPUARMState *env, target_ulong address, MMUAccessType access_type, ARMMMUIdx mmu_idx, - hwaddr *phys_ptr, MemTxAttrs *attrs, int *prot, - target_ulong *page_size, - ARMMMUFaultInfo *fi, ARMCacheAttrs *cacheattrs) + GetPhysAddrResult *result, ARMMMUFaultInfo *fi) { ARMMMUIdx s1_mmu_idx = stage_1_mmu_idx(mmu_idx); @@ -XXX,XX +XXX,XX @@ bool get_phys_addr(CPUARMState *env, target_ulong address, */ if (arm_feature(env, ARM_FEATURE_EL2)) { hwaddr ipa; - int s2_prot; + int s1_prot; int ret; bool ipa_secure; - ARMCacheAttrs cacheattrs2 = {}; + ARMCacheAttrs cacheattrs1; ARMMMUIdx s2_mmu_idx; bool is_el0; - ret = get_phys_addr(env, address, access_type, s1_mmu_idx, &ipa, - attrs, prot, page_size, fi, cacheattrs); + ret = get_phys_addr(env, address, access_type, s1_mmu_idx, + result, fi); /* If S1 fails or S2 is disabled, return early. */ if (ret || regime_translation_disabled(env, ARMMMUIdx_Stage2)) { - *phys_ptr = ipa; return ret; } - ipa_secure = attrs->secure; + ipa = result->phys; + ipa_secure = result->attrs.secure; if (arm_is_secure_below_el3(env)) { if (ipa_secure) { - attrs->secure = !(env->cp15.vstcr_el2 & VSTCR_SW); + result->attrs.secure = !(env->cp15.vstcr_el2 & VSTCR_SW); } else { - attrs->secure = !(env->cp15.vtcr_el2 & VTCR_NSW); + result->attrs.secure = !(env->cp15.vtcr_el2 & VTCR_NSW); } } else { assert(!ipa_secure); } - s2_mmu_idx = attrs->secure ? ARMMMUIdx_Stage2_S : ARMMMUIdx_Stage2; + s2_mmu_idx = (result->attrs.secure + ? ARMMMUIdx_Stage2_S : ARMMMUIdx_Stage2); is_el0 = mmu_idx == ARMMMUIdx_E10_0 || mmu_idx == ARMMMUIdx_SE10_0; - /* S1 is done. Now do S2 translation. */ + /* + * S1 is done, now do S2 translation. + * Save the stage1 results so that we may merge + * prot and cacheattrs later. + */ + s1_prot = result->prot; + cacheattrs1 = result->cacheattrs; + memset(result, 0, sizeof(*result)); + ret = get_phys_addr_lpae(env, ipa, access_type, s2_mmu_idx, is_el0, - phys_ptr, attrs, &s2_prot, - page_size, fi, &cacheattrs2); + &result->phys, &result->attrs, + &result->prot, &result->page_size, + fi, &result->cacheattrs); fi->s2addr = ipa; + /* Combine the S1 and S2 perms. */ - *prot &= s2_prot; + result->prot &= s1_prot; /* If S2 fails, return early. */ if (ret) { @@ -XXX,XX +XXX,XX @@ bool get_phys_addr(CPUARMState *env, target_ulong address, * Outer Write-Back Read-Allocate Write-Allocate. * Do not overwrite Tagged within attrs. */ - if (cacheattrs->attrs != 0xf0) { - cacheattrs->attrs = 0xff; + if (cacheattrs1.attrs != 0xf0) { + cacheattrs1.attrs = 0xff; } - cacheattrs->shareability = 0; + cacheattrs1.shareability = 0; } - *cacheattrs = combine_cacheattrs(env, *cacheattrs, cacheattrs2); + result->cacheattrs = combine_cacheattrs(env, cacheattrs1, + result->cacheattrs); /* Check if IPA translates to secure or non-secure PA space. */ if (arm_is_secure_below_el3(env)) { if (ipa_secure) { - attrs->secure = + result->attrs.secure = !(env->cp15.vstcr_el2 & (VSTCR_SA | VSTCR_SW)); } else { - attrs->secure = + result->attrs.secure = !((env->cp15.vtcr_el2 & (VTCR_NSA | VTCR_NSW)) || (env->cp15.vstcr_el2 & (VSTCR_SA | VSTCR_SW))); } @@ -XXX,XX +XXX,XX @@ bool get_phys_addr(CPUARMState *env, target_ulong address, * cannot upgrade an non-secure translation regime's attributes * to secure. */ - attrs->secure = regime_is_secure(env, mmu_idx); - attrs->user = regime_is_user(env, mmu_idx); + result->attrs.secure = regime_is_secure(env, mmu_idx); + result->attrs.user = regime_is_user(env, mmu_idx); /* * Fast Context Switch Extension. This doesn't exist at all in v8. @@ -XXX,XX +XXX,XX @@ bool get_phys_addr(CPUARMState *env, target_ulong address, if (arm_feature(env, ARM_FEATURE_PMSA)) { bool ret; - *page_size = TARGET_PAGE_SIZE; + result->page_size = TARGET_PAGE_SIZE; if (arm_feature(env, ARM_FEATURE_V8)) { /* PMSAv8 */ ret = get_phys_addr_pmsav8(env, address, access_type, mmu_idx, - phys_ptr, attrs, prot, page_size, fi); + &result->phys, &result->attrs, + &result->prot, &result->page_size, fi); } else if (arm_feature(env, ARM_FEATURE_V7)) { /* PMSAv7 */ ret = get_phys_addr_pmsav7(env, address, access_type, mmu_idx, - phys_ptr, prot, page_size, fi); + &result->phys, &result->prot, + &result->page_size, fi); } else { /* Pre-v7 MPU */ ret = get_phys_addr_pmsav5(env, address, access_type, mmu_idx, - phys_ptr, prot, fi); + &result->phys, &result->prot, fi); } qemu_log_mask(CPU_LOG_MMU, "PMSA MPU lookup for %s at 0x%08" PRIx32 " mmu_idx %u -> %s (prot %c%c%c)\n", @@ -XXX,XX +XXX,XX @@ bool get_phys_addr(CPUARMState *env, target_ulong address, (access_type == MMU_DATA_STORE ? "writing" : "execute"), (uint32_t)address, mmu_idx, ret ? "Miss" : "Hit", - *prot & PAGE_READ ? 'r' : '-', - *prot & PAGE_WRITE ? 'w' : '-', - *prot & PAGE_EXEC ? 'x' : '-'); + result->prot & PAGE_READ ? 'r' : '-', + result->prot & PAGE_WRITE ? 'w' : '-', + result->prot & PAGE_EXEC ? 'x' : '-'); return ret; } @@ -XXX,XX +XXX,XX @@ bool get_phys_addr(CPUARMState *env, target_ulong address, address = extract64(address, 0, 52); } } - *phys_ptr = address; - *prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC; - *page_size = TARGET_PAGE_SIZE; + result->phys = address; + result->prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC; + result->page_size = TARGET_PAGE_SIZE; /* Fill in cacheattr a-la AArch64.TranslateAddressS1Off. */ hcr = arm_hcr_el2_eff(env); - cacheattrs->shareability = 0; - cacheattrs->is_s2_format = false; + result->cacheattrs.shareability = 0; + result->cacheattrs.is_s2_format = false; if (hcr & HCR_DC) { if (hcr & HCR_DCT) { memattr = 0xf0; /* Tagged, Normal, WB, RWA */ @@ -XXX,XX +XXX,XX @@ bool get_phys_addr(CPUARMState *env, target_ulong address, } else { memattr = 0x44; /* Normal, NC, No */ } - cacheattrs->shareability = 2; /* outer sharable */ + result->cacheattrs.shareability = 2; /* outer sharable */ } else { memattr = 0x00; /* Device, nGnRnE */ } - cacheattrs->attrs = memattr; + result->cacheattrs.attrs = memattr; return 0; } if (regime_using_lpae_format(env, mmu_idx)) { return get_phys_addr_lpae(env, address, access_type, mmu_idx, false, - phys_ptr, attrs, prot, page_size, - fi, cacheattrs); + &result->phys, &result->attrs, + &result->prot, &result->page_size, + fi, &result->cacheattrs); } else if (regime_sctlr(env, mmu_idx) & SCTLR_XP) { return get_phys_addr_v6(env, address, access_type, mmu_idx, - phys_ptr, attrs, prot, page_size, fi); + &result->phys, &result->attrs, + &result->prot, &result->page_size, fi); } else { return get_phys_addr_v5(env, address, access_type, mmu_idx, - phys_ptr, prot, page_size, fi); + &result->phys, &result->prot, + &result->page_size, fi); } } @@ -XXX,XX +XXX,XX @@ hwaddr arm_cpu_get_phys_page_attrs_debug(CPUState *cs, vaddr addr, { ARMCPU *cpu = ARM_CPU(cs); CPUARMState *env = &cpu->env; - hwaddr phys_addr; - target_ulong page_size; - int prot; - bool ret; + GetPhysAddrResult res = {}; ARMMMUFaultInfo fi = {}; ARMMMUIdx mmu_idx = arm_mmu_idx(env); - ARMCacheAttrs cacheattrs = {}; + bool ret; - *attrs = (MemTxAttrs) {}; - - ret = get_phys_addr(env, addr, MMU_DATA_LOAD, mmu_idx, &phys_addr, - attrs, &prot, &page_size, &fi, &cacheattrs); + ret = get_phys_addr(env, addr, MMU_DATA_LOAD, mmu_idx, &res, &fi); + *attrs = res.attrs; if (ret) { return -1; } - return phys_addr; + return res.phys; } diff --git a/target/arm/tlb_helper.c b/target/arm/tlb_helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/tlb_helper.c +++ b/target/arm/tlb_helper.c @@ -XXX,XX +XXX,XX @@ bool arm_cpu_tlb_fill(CPUState *cs, vaddr address, int size, { ARMCPU *cpu = ARM_CPU(cs); ARMMMUFaultInfo fi = {}; - hwaddr phys_addr; - target_ulong page_size; - int prot, ret; - MemTxAttrs attrs = {}; - ARMCacheAttrs cacheattrs = {}; + GetPhysAddrResult res = {}; + int ret; /* * Walk the page table and (if the mapping exists) add the page @@ -XXX,XX +XXX,XX @@ bool arm_cpu_tlb_fill(CPUState *cs, vaddr address, int size, */ ret = get_phys_addr(&cpu->env, address, access_type, core_to_arm_mmu_idx(&cpu->env, mmu_idx), - &phys_addr, &attrs, &prot, &page_size, - &fi, &cacheattrs); + &res, &fi); if (likely(!ret)) { /* * Map a single [sub]page. Regions smaller than our declared * target page size are handled specially, so for those we * pass in the exact addresses. */ - if (page_size >= TARGET_PAGE_SIZE) { - phys_addr &= TARGET_PAGE_MASK; + if (res.page_size >= TARGET_PAGE_SIZE) { + res.phys &= TARGET_PAGE_MASK; address &= TARGET_PAGE_MASK; } /* Notice and record tagged memory. */ - if (cpu_isar_feature(aa64_mte, cpu) && cacheattrs.attrs == 0xf0) { - arm_tlb_mte_tagged(&attrs) = true; + if (cpu_isar_feature(aa64_mte, cpu) && res.cacheattrs.attrs == 0xf0) { + arm_tlb_mte_tagged(&res.attrs) = true; } - tlb_set_page_with_attrs(cs, address, phys_addr, attrs, - prot, mmu_idx, page_size); + tlb_set_page_with_attrs(cs, address, res.phys, res.attrs, + res.prot, mmu_idx, res.page_size); return true; } else if (probe) { return false; -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220822152741.1617527-4-richard.henderson@linaro.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/ptw.c | 69 ++++++++++++++++++------------------------------ 1 file changed, 26 insertions(+), 43 deletions(-) diff --git a/target/arm/ptw.c b/target/arm/ptw.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/ptw.c +++ b/target/arm/ptw.c @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_lpae(CPUARMState *env, uint64_t address, MMUAccessType access_type, ARMMMUIdx mmu_idx, - bool s1_is_el0, hwaddr *phys_ptr, - MemTxAttrs *txattrs, int *prot, - target_ulong *page_size_ptr, - ARMMMUFaultInfo *fi, ARMCacheAttrs *cacheattrs) + bool s1_is_el0, GetPhysAddrResult *result, + ARMMMUFaultInfo *fi) __attribute__((nonnull)); /* This mapping is common between ID_AA64MMFR0.PARANGE and TCR_ELx.{I}PS. */ @@ -XXX,XX +XXX,XX @@ static hwaddr S1_ptw_translate(CPUARMState *env, ARMMMUIdx mmu_idx, { if (arm_mmu_idx_is_stage1_of_2(mmu_idx) && !regime_translation_disabled(env, ARMMMUIdx_Stage2)) { - target_ulong s2size; - hwaddr s2pa; - int s2prot; - int ret; ARMMMUIdx s2_mmu_idx = *is_secure ? ARMMMUIdx_Stage2_S : ARMMMUIdx_Stage2; - ARMCacheAttrs cacheattrs = {}; - MemTxAttrs txattrs = {}; + GetPhysAddrResult s2 = {}; + int ret; ret = get_phys_addr_lpae(env, addr, MMU_DATA_LOAD, s2_mmu_idx, false, - &s2pa, &txattrs, &s2prot, &s2size, fi, - &cacheattrs); + &s2, fi); if (ret) { assert(fi->type != ARMFault_None); fi->s2addr = addr; @@ -XXX,XX +XXX,XX @@ static hwaddr S1_ptw_translate(CPUARMState *env, ARMMMUIdx mmu_idx, return ~0; } if ((arm_hcr_el2_eff(env) & HCR_PTW) && - ptw_attrs_are_device(env, cacheattrs)) { + ptw_attrs_are_device(env, s2.cacheattrs)) { /* * PTW set and S1 walk touched S2 Device memory: * generate Permission fault. @@ -XXX,XX +XXX,XX @@ static hwaddr S1_ptw_translate(CPUARMState *env, ARMMMUIdx mmu_idx, assert(!*is_secure); } - addr = s2pa; + addr = s2.phys; } return addr; } @@ -XXX,XX +XXX,XX @@ static bool check_s2_mmu_setup(ARMCPU *cpu, bool is_aa64, int level, * table walk), must be true if this is stage 2 of a stage 1+2 * walk for an EL0 access. If @mmu_idx is anything else, * @s1_is_el0 is ignored. - * @phys_ptr: set to the physical address corresponding to the virtual address - * @attrs: set to the memory transaction attributes to use - * @prot: set to the permissions for the page containing phys_ptr - * @page_size_ptr: set to the size of the page containing phys_ptr + * @result: set on translation success, * @fi: set to fault info if the translation fails - * @cacheattrs: (if non-NULL) set to the cacheability/shareability attributes */ static bool get_phys_addr_lpae(CPUARMState *env, uint64_t address, MMUAccessType access_type, ARMMMUIdx mmu_idx, - bool s1_is_el0, hwaddr *phys_ptr, - MemTxAttrs *txattrs, int *prot, - target_ulong *page_size_ptr, - ARMMMUFaultInfo *fi, ARMCacheAttrs *cacheattrs) + bool s1_is_el0, GetPhysAddrResult *result, + ARMMMUFaultInfo *fi) { ARMCPU *cpu = env_archcpu(env); /* Read an LPAE long-descriptor translation table. */ @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_lpae(CPUARMState *env, uint64_t address, if (mmu_idx == ARMMMUIdx_Stage2 || mmu_idx == ARMMMUIdx_Stage2_S) { ns = mmu_idx == ARMMMUIdx_Stage2; xn = extract32(attrs, 11, 2); - *prot = get_S2prot(env, ap, xn, s1_is_el0); + result->prot = get_S2prot(env, ap, xn, s1_is_el0); } else { ns = extract32(attrs, 3, 1); xn = extract32(attrs, 12, 1); pxn = extract32(attrs, 11, 1); - *prot = get_S1prot(env, mmu_idx, aarch64, ap, ns, xn, pxn); + result->prot = get_S1prot(env, mmu_idx, aarch64, ap, ns, xn, pxn); } fault_type = ARMFault_Permission; - if (!(*prot & (1 << access_type))) { + if (!(result->prot & (1 << access_type))) { goto do_fault; } @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_lpae(CPUARMState *env, uint64_t address, * the CPU doesn't support TZ or this is a non-secure translation * regime, because the attribute will already be non-secure. */ - txattrs->secure = false; + result->attrs.secure = false; } /* When in aarch64 mode, and BTI is enabled, remember GP in the IOTLB. */ if (aarch64 && guarded && cpu_isar_feature(aa64_bti, cpu)) { - arm_tlb_bti_gp(txattrs) = true; + arm_tlb_bti_gp(&result->attrs) = true; } if (mmu_idx == ARMMMUIdx_Stage2 || mmu_idx == ARMMMUIdx_Stage2_S) { - cacheattrs->is_s2_format = true; - cacheattrs->attrs = extract32(attrs, 0, 4); + result->cacheattrs.is_s2_format = true; + result->cacheattrs.attrs = extract32(attrs, 0, 4); } else { /* Index into MAIR registers for cache attributes */ uint8_t attrindx = extract32(attrs, 0, 3); uint64_t mair = env->cp15.mair_el[regime_el(env, mmu_idx)]; assert(attrindx <= 7); - cacheattrs->is_s2_format = false; - cacheattrs->attrs = extract64(mair, attrindx * 8, 8); + result->cacheattrs.is_s2_format = false; + result->cacheattrs.attrs = extract64(mair, attrindx * 8, 8); } /* @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_lpae(CPUARMState *env, uint64_t address, * that case comes from TCR_ELx, which we extracted earlier. */ if (param.ds) { - cacheattrs->shareability = param.sh; + result->cacheattrs.shareability = param.sh; } else { - cacheattrs->shareability = extract32(attrs, 6, 2); + result->cacheattrs.shareability = extract32(attrs, 6, 2); } - *phys_ptr = descaddr; - *page_size_ptr = page_size; + result->phys = descaddr; + result->page_size = page_size; return false; do_fault: @@ -XXX,XX +XXX,XX @@ bool get_phys_addr(CPUARMState *env, target_ulong address, cacheattrs1 = result->cacheattrs; memset(result, 0, sizeof(*result)); - ret = get_phys_addr_lpae(env, ipa, access_type, s2_mmu_idx, is_el0, - &result->phys, &result->attrs, - &result->prot, &result->page_size, - fi, &result->cacheattrs); + ret = get_phys_addr_lpae(env, ipa, access_type, s2_mmu_idx, + is_el0, result, fi); fi->s2addr = ipa; /* Combine the S1 and S2 perms. */ @@ -XXX,XX +XXX,XX @@ bool get_phys_addr(CPUARMState *env, target_ulong address, if (regime_using_lpae_format(env, mmu_idx)) { return get_phys_addr_lpae(env, address, access_type, mmu_idx, false, - &result->phys, &result->attrs, - &result->prot, &result->page_size, - fi, &result->cacheattrs); + result, fi); } else if (regime_sctlr(env, mmu_idx) & SCTLR_XP) { return get_phys_addr_v6(env, address, access_type, mmu_idx, &result->phys, &result->attrs, -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220822152741.1617527-5-richard.henderson@linaro.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/ptw.c | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/target/arm/ptw.c b/target/arm/ptw.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/ptw.c +++ b/target/arm/ptw.c @@ -XXX,XX +XXX,XX @@ do_fault: static bool get_phys_addr_v6(CPUARMState *env, uint32_t address, MMUAccessType access_type, ARMMMUIdx mmu_idx, - hwaddr *phys_ptr, MemTxAttrs *attrs, int *prot, - target_ulong *page_size, ARMMMUFaultInfo *fi) + GetPhysAddrResult *result, ARMMMUFaultInfo *fi) { ARMCPU *cpu = env_archcpu(env); int level = 1; @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_v6(CPUARMState *env, uint32_t address, phys_addr = (desc & 0xff000000) | (address & 0x00ffffff); phys_addr |= (uint64_t)extract32(desc, 20, 4) << 32; phys_addr |= (uint64_t)extract32(desc, 5, 4) << 36; - *page_size = 0x1000000; + result->page_size = 0x1000000; } else { /* Section. */ phys_addr = (desc & 0xfff00000) | (address & 0x000fffff); - *page_size = 0x100000; + result->page_size = 0x100000; } ap = ((desc >> 10) & 3) | ((desc >> 13) & 4); xn = desc & (1 << 4); @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_v6(CPUARMState *env, uint32_t address, case 1: /* 64k page. */ phys_addr = (desc & 0xffff0000) | (address & 0xffff); xn = desc & (1 << 15); - *page_size = 0x10000; + result->page_size = 0x10000; break; case 2: case 3: /* 4k page. */ phys_addr = (desc & 0xfffff000) | (address & 0xfff); xn = desc & 1; - *page_size = 0x1000; + result->page_size = 0x1000; break; default: /* Never happens, but compiler isn't smart enough to tell. */ @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_v6(CPUARMState *env, uint32_t address, } } if (domain_prot == 3) { - *prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC; + result->prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC; } else { if (pxn && !regime_is_user(env, mmu_idx)) { xn = 1; @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_v6(CPUARMState *env, uint32_t address, fi->type = ARMFault_AccessFlag; goto do_fault; } - *prot = simple_ap_to_rw_prot(env, mmu_idx, ap >> 1); + result->prot = simple_ap_to_rw_prot(env, mmu_idx, ap >> 1); } else { - *prot = ap_to_rw_prot(env, mmu_idx, ap, domain_prot); + result->prot = ap_to_rw_prot(env, mmu_idx, ap, domain_prot); } - if (*prot && !xn) { - *prot |= PAGE_EXEC; + if (result->prot && !xn) { + result->prot |= PAGE_EXEC; } - if (!(*prot & (1 << access_type))) { + if (!(result->prot & (1 << access_type))) { /* Access permission fault. */ fi->type = ARMFault_Permission; goto do_fault; @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_v6(CPUARMState *env, uint32_t address, * the CPU doesn't support TZ or this is a non-secure translation * regime, because the attribute will already be non-secure. */ - attrs->secure = false; + result->attrs.secure = false; } - *phys_ptr = phys_addr; + result->phys = phys_addr; return false; do_fault: fi->domain = domain; @@ -XXX,XX +XXX,XX @@ bool get_phys_addr(CPUARMState *env, target_ulong address, result, fi); } else if (regime_sctlr(env, mmu_idx) & SCTLR_XP) { return get_phys_addr_v6(env, address, access_type, mmu_idx, - &result->phys, &result->attrs, - &result->prot, &result->page_size, fi); + result, fi); } else { return get_phys_addr_v5(env, address, access_type, mmu_idx, &result->phys, &result->prot, -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220822152741.1617527-6-richard.henderson@linaro.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/ptw.c | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/target/arm/ptw.c b/target/arm/ptw.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/ptw.c +++ b/target/arm/ptw.c @@ -XXX,XX +XXX,XX @@ static int simple_ap_to_rw_prot(CPUARMState *env, ARMMMUIdx mmu_idx, int ap) static bool get_phys_addr_v5(CPUARMState *env, uint32_t address, MMUAccessType access_type, ARMMMUIdx mmu_idx, - hwaddr *phys_ptr, int *prot, - target_ulong *page_size, - ARMMMUFaultInfo *fi) + GetPhysAddrResult *result, ARMMMUFaultInfo *fi) { int level = 1; uint32_t table; @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_v5(CPUARMState *env, uint32_t address, /* 1Mb section. */ phys_addr = (desc & 0xfff00000) | (address & 0x000fffff); ap = (desc >> 10) & 3; - *page_size = 1024 * 1024; + result->page_size = 1024 * 1024; } else { /* Lookup l2 entry. */ if (type == 1) { @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_v5(CPUARMState *env, uint32_t address, case 1: /* 64k page. */ phys_addr = (desc & 0xffff0000) | (address & 0xffff); ap = (desc >> (4 + ((address >> 13) & 6))) & 3; - *page_size = 0x10000; + result->page_size = 0x10000; break; case 2: /* 4k page. */ phys_addr = (desc & 0xfffff000) | (address & 0xfff); ap = (desc >> (4 + ((address >> 9) & 6))) & 3; - *page_size = 0x1000; + result->page_size = 0x1000; break; case 3: /* 1k page, or ARMv6/XScale "extended small (4k) page" */ if (type == 1) { @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_v5(CPUARMState *env, uint32_t address, if (arm_feature(env, ARM_FEATURE_XSCALE) || arm_feature(env, ARM_FEATURE_V6)) { phys_addr = (desc & 0xfffff000) | (address & 0xfff); - *page_size = 0x1000; + result->page_size = 0x1000; } else { /* * UNPREDICTABLE in ARMv5; we choose to take a @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_v5(CPUARMState *env, uint32_t address, } } else { phys_addr = (desc & 0xfffffc00) | (address & 0x3ff); - *page_size = 0x400; + result->page_size = 0x400; } ap = (desc >> 4) & 3; break; @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_v5(CPUARMState *env, uint32_t address, g_assert_not_reached(); } } - *prot = ap_to_rw_prot(env, mmu_idx, ap, domain_prot); - *prot |= *prot ? PAGE_EXEC : 0; - if (!(*prot & (1 << access_type))) { + result->prot = ap_to_rw_prot(env, mmu_idx, ap, domain_prot); + result->prot |= result->prot ? PAGE_EXEC : 0; + if (!(result->prot & (1 << access_type))) { /* Access permission fault. */ fi->type = ARMFault_Permission; goto do_fault; } - *phys_ptr = phys_addr; + result->phys = phys_addr; return false; do_fault: fi->domain = domain; @@ -XXX,XX +XXX,XX @@ bool get_phys_addr(CPUARMState *env, target_ulong address, result, fi); } else { return get_phys_addr_v5(env, address, access_type, mmu_idx, - &result->phys, &result->prot, - &result->page_size, fi); + result, fi); } } -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220822152741.1617527-7-richard.henderson@linaro.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/ptw.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/target/arm/ptw.c b/target/arm/ptw.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/ptw.c +++ b/target/arm/ptw.c @@ -XXX,XX +XXX,XX @@ do_fault: static bool get_phys_addr_pmsav5(CPUARMState *env, uint32_t address, MMUAccessType access_type, ARMMMUIdx mmu_idx, - hwaddr *phys_ptr, int *prot, + GetPhysAddrResult *result, ARMMMUFaultInfo *fi) { int n; @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_pmsav5(CPUARMState *env, uint32_t address, if (regime_translation_disabled(env, mmu_idx)) { /* MPU disabled. */ - *phys_ptr = address; - *prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC; + result->phys = address; + result->prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC; return false; } - *phys_ptr = address; + result->phys = address; for (n = 7; n >= 0; n--) { base = env->cp15.c6_region[n]; if ((base & 1) == 0) { @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_pmsav5(CPUARMState *env, uint32_t address, fi->level = 1; return true; } - *prot = PAGE_READ | PAGE_WRITE; + result->prot = PAGE_READ | PAGE_WRITE; break; case 2: - *prot = PAGE_READ; + result->prot = PAGE_READ; if (!is_user) { - *prot |= PAGE_WRITE; + result->prot |= PAGE_WRITE; } break; case 3: - *prot = PAGE_READ | PAGE_WRITE; + result->prot = PAGE_READ | PAGE_WRITE; break; case 5: if (is_user) { @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_pmsav5(CPUARMState *env, uint32_t address, fi->level = 1; return true; } - *prot = PAGE_READ; + result->prot = PAGE_READ; break; case 6: - *prot = PAGE_READ; + result->prot = PAGE_READ; break; default: /* Bad permission. */ @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_pmsav5(CPUARMState *env, uint32_t address, fi->level = 1; return true; } - *prot |= PAGE_EXEC; + result->prot |= PAGE_EXEC; return false; } @@ -XXX,XX +XXX,XX @@ bool get_phys_addr(CPUARMState *env, target_ulong address, } else { /* Pre-v7 MPU */ ret = get_phys_addr_pmsav5(env, address, access_type, mmu_idx, - &result->phys, &result->prot, fi); + result, fi); } qemu_log_mask(CPU_LOG_MMU, "PMSA MPU lookup for %s at 0x%08" PRIx32 " mmu_idx %u -> %s (prot %c%c%c)\n", -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220822152741.1617527-8-richard.henderson@linaro.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/ptw.c | 36 +++++++++++++++++------------------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/target/arm/ptw.c b/target/arm/ptw.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/ptw.c +++ b/target/arm/ptw.c @@ -XXX,XX +XXX,XX @@ static bool pmsav7_use_background_region(ARMCPU *cpu, ARMMMUIdx mmu_idx, static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address, MMUAccessType access_type, ARMMMUIdx mmu_idx, - hwaddr *phys_ptr, int *prot, - target_ulong *page_size, + GetPhysAddrResult *result, ARMMMUFaultInfo *fi) { ARMCPU *cpu = env_archcpu(env); int n; bool is_user = regime_is_user(env, mmu_idx); - *phys_ptr = address; - *page_size = TARGET_PAGE_SIZE; - *prot = 0; + result->phys = address; + result->page_size = TARGET_PAGE_SIZE; + result->prot = 0; if (regime_translation_disabled(env, mmu_idx) || m_is_ppb_region(env, address)) { @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address, * which always does a direct read using address_space_ldl(), rather * than going via this function, so we don't need to check that here. */ - get_phys_addr_pmsav7_default(env, mmu_idx, address, prot); + get_phys_addr_pmsav7_default(env, mmu_idx, address, &result->prot); } else { /* MPU enabled */ for (n = (int)cpu->pmsav7_dregion - 1; n >= 0; n--) { /* region search */ @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address, if (ranges_overlap(base, rmask, address & TARGET_PAGE_MASK, TARGET_PAGE_SIZE)) { - *page_size = 1; + result->page_size = 1; } continue; } @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address, continue; } if (rsize < TARGET_PAGE_BITS) { - *page_size = 1 << rsize; + result->page_size = 1 << rsize; } break; } @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address, fi->type = ARMFault_Background; return true; } - get_phys_addr_pmsav7_default(env, mmu_idx, address, prot); + get_phys_addr_pmsav7_default(env, mmu_idx, address, &result->prot); } else { /* a MPU hit! */ uint32_t ap = extract32(env->pmsav7.dracr[n], 8, 3); uint32_t xn = extract32(env->pmsav7.dracr[n], 12, 1); @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address, case 5: break; /* no access */ case 3: - *prot |= PAGE_WRITE; + result->prot |= PAGE_WRITE; /* fall through */ case 2: case 6: - *prot |= PAGE_READ | PAGE_EXEC; + result->prot |= PAGE_READ | PAGE_EXEC; break; case 7: /* for v7M, same as 6; for R profile a reserved value */ if (arm_feature(env, ARM_FEATURE_M)) { - *prot |= PAGE_READ | PAGE_EXEC; + result->prot |= PAGE_READ | PAGE_EXEC; break; } /* fall through */ @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address, case 1: case 2: case 3: - *prot |= PAGE_WRITE; + result->prot |= PAGE_WRITE; /* fall through */ case 5: case 6: - *prot |= PAGE_READ | PAGE_EXEC; + result->prot |= PAGE_READ | PAGE_EXEC; break; case 7: /* for v7M, same as 6; for R profile a reserved value */ if (arm_feature(env, ARM_FEATURE_M)) { - *prot |= PAGE_READ | PAGE_EXEC; + result->prot |= PAGE_READ | PAGE_EXEC; break; } /* fall through */ @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address, /* execute never */ if (xn) { - *prot &= ~PAGE_EXEC; + result->prot &= ~PAGE_EXEC; } } } fi->type = ARMFault_Permission; fi->level = 1; - return !(*prot & (1 << access_type)); + return !(result->prot & (1 << access_type)); } bool pmsav8_mpu_lookup(CPUARMState *env, uint32_t address, @@ -XXX,XX +XXX,XX @@ bool get_phys_addr(CPUARMState *env, target_ulong address, } else if (arm_feature(env, ARM_FEATURE_V7)) { /* PMSAv7 */ ret = get_phys_addr_pmsav7(env, address, access_type, mmu_idx, - &result->phys, &result->prot, - &result->page_size, fi); + result, fi); } else { /* Pre-v7 MPU */ ret = get_phys_addr_pmsav5(env, address, access_type, mmu_idx, -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220822152741.1617527-9-richard.henderson@linaro.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/ptw.c | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/target/arm/ptw.c b/target/arm/ptw.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/ptw.c +++ b/target/arm/ptw.c @@ -XXX,XX +XXX,XX @@ void v8m_security_lookup(CPUARMState *env, uint32_t address, static bool get_phys_addr_pmsav8(CPUARMState *env, uint32_t address, MMUAccessType access_type, ARMMMUIdx mmu_idx, - hwaddr *phys_ptr, MemTxAttrs *txattrs, - int *prot, target_ulong *page_size, + GetPhysAddrResult *result, ARMMMUFaultInfo *fi) { uint32_t secure = regime_is_secure(env, mmu_idx); @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_pmsav8(CPUARMState *env, uint32_t address, } else { fi->type = ARMFault_QEMU_SFault; } - *page_size = sattrs.subpage ? 1 : TARGET_PAGE_SIZE; - *phys_ptr = address; - *prot = 0; + result->page_size = sattrs.subpage ? 1 : TARGET_PAGE_SIZE; + result->phys = address; + result->prot = 0; return true; } } else { @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_pmsav8(CPUARMState *env, uint32_t address, * might downgrade a secure access to nonsecure. */ if (sattrs.ns) { - txattrs->secure = false; + result->attrs.secure = false; } else if (!secure) { /* * NS access to S memory must fault. @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_pmsav8(CPUARMState *env, uint32_t address, * for M_FAKE_FSR_SFAULT in arm_v7m_cpu_do_interrupt(). */ fi->type = ARMFault_QEMU_SFault; - *page_size = sattrs.subpage ? 1 : TARGET_PAGE_SIZE; - *phys_ptr = address; - *prot = 0; + result->page_size = sattrs.subpage ? 1 : TARGET_PAGE_SIZE; + result->phys = address; + result->prot = 0; return true; } } } - ret = pmsav8_mpu_lookup(env, address, access_type, mmu_idx, phys_ptr, - txattrs, prot, &mpu_is_subpage, fi, NULL); - *page_size = sattrs.subpage || mpu_is_subpage ? 1 : TARGET_PAGE_SIZE; + ret = pmsav8_mpu_lookup(env, address, access_type, mmu_idx, + &result->phys, &result->attrs, &result->prot, + &mpu_is_subpage, fi, NULL); + result->page_size = + sattrs.subpage || mpu_is_subpage ? 1 : TARGET_PAGE_SIZE; return ret; } @@ -XXX,XX +XXX,XX @@ bool get_phys_addr(CPUARMState *env, target_ulong address, if (arm_feature(env, ARM_FEATURE_V8)) { /* PMSAv8 */ ret = get_phys_addr_pmsav8(env, address, access_type, mmu_idx, - &result->phys, &result->attrs, - &result->prot, &result->page_size, fi); + result, fi); } else if (arm_feature(env, ARM_FEATURE_V7)) { /* PMSAv7 */ ret = get_phys_addr_pmsav7(env, address, access_type, mmu_idx, -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220822152741.1617527-10-richard.henderson@linaro.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/internals.h | 11 +++++------ target/arm/m_helper.c | 16 +++++++--------- target/arm/ptw.c | 20 +++++++++----------- 3 files changed, 21 insertions(+), 26 deletions(-) diff --git a/target/arm/internals.h b/target/arm/internals.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/internals.h +++ b/target/arm/internals.h @@ -XXX,XX +XXX,XX @@ void v8m_security_lookup(CPUARMState *env, uint32_t address, MMUAccessType access_type, ARMMMUIdx mmu_idx, V8M_SAttributes *sattrs); -bool pmsav8_mpu_lookup(CPUARMState *env, uint32_t address, - MMUAccessType access_type, ARMMMUIdx mmu_idx, - hwaddr *phys_ptr, MemTxAttrs *txattrs, - int *prot, bool *is_subpage, - ARMMMUFaultInfo *fi, uint32_t *mregion); - /* Cacheability and shareability attributes for a memory access */ typedef struct ARMCacheAttrs { /* @@ -XXX,XX +XXX,XX @@ bool get_phys_addr(CPUARMState *env, target_ulong address, GetPhysAddrResult *result, ARMMMUFaultInfo *fi) __attribute__((nonnull)); +bool pmsav8_mpu_lookup(CPUARMState *env, uint32_t address, + MMUAccessType access_type, ARMMMUIdx mmu_idx, + GetPhysAddrResult *result, bool *is_subpage, + ARMMMUFaultInfo *fi, uint32_t *mregion); + void arm_log_exception(CPUState *cs); #endif /* !CONFIG_USER_ONLY */ diff --git a/target/arm/m_helper.c b/target/arm/m_helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/m_helper.c +++ b/target/arm/m_helper.c @@ -XXX,XX +XXX,XX @@ uint32_t HELPER(v7m_tt)(CPUARMState *env, uint32_t addr, uint32_t op) V8M_SAttributes sattrs = {}; uint32_t tt_resp; bool r, rw, nsr, nsrw, mrvalid; - int prot; - ARMMMUFaultInfo fi = {}; - MemTxAttrs attrs = {}; - hwaddr phys_addr; ARMMMUIdx mmu_idx; uint32_t mregion; bool targetpriv; bool targetsec = env->v7m.secure; - bool is_subpage; /* * Work out what the security state and privilege level we're @@ -XXX,XX +XXX,XX @@ uint32_t HELPER(v7m_tt)(CPUARMState *env, uint32_t addr, uint32_t op) * inspecting the other MPU state. */ if (arm_current_el(env) != 0 || alt) { + GetPhysAddrResult res = {}; + ARMMMUFaultInfo fi = {}; + bool is_subpage; + /* We can ignore the return value as prot is always set */ pmsav8_mpu_lookup(env, addr, MMU_DATA_LOAD, mmu_idx, - &phys_addr, &attrs, &prot, &is_subpage, - &fi, &mregion); + &res, &is_subpage, &fi, &mregion); if (mregion == -1) { mrvalid = false; mregion = 0; } else { mrvalid = true; } - r = prot & PAGE_READ; - rw = prot & PAGE_WRITE; + r = res.prot & PAGE_READ; + rw = res.prot & PAGE_WRITE; } else { r = false; rw = false; diff --git a/target/arm/ptw.c b/target/arm/ptw.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/ptw.c +++ b/target/arm/ptw.c @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address, bool pmsav8_mpu_lookup(CPUARMState *env, uint32_t address, MMUAccessType access_type, ARMMMUIdx mmu_idx, - hwaddr *phys_ptr, MemTxAttrs *txattrs, - int *prot, bool *is_subpage, + GetPhysAddrResult *result, bool *is_subpage, ARMMMUFaultInfo *fi, uint32_t *mregion) { /* @@ -XXX,XX +XXX,XX @@ bool pmsav8_mpu_lookup(CPUARMState *env, uint32_t address, uint32_t addr_page_limit = addr_page_base + (TARGET_PAGE_SIZE - 1); *is_subpage = false; - *phys_ptr = address; - *prot = 0; + result->phys = address; + result->prot = 0; if (mregion) { *mregion = -1; } @@ -XXX,XX +XXX,XX @@ bool pmsav8_mpu_lookup(CPUARMState *env, uint32_t address, if (matchregion == -1) { /* hit using the background region */ - get_phys_addr_pmsav7_default(env, mmu_idx, address, prot); + get_phys_addr_pmsav7_default(env, mmu_idx, address, &result->prot); } else { uint32_t ap = extract32(env->pmsav8.rbar[secure][matchregion], 1, 2); uint32_t xn = extract32(env->pmsav8.rbar[secure][matchregion], 0, 1); @@ -XXX,XX +XXX,XX @@ bool pmsav8_mpu_lookup(CPUARMState *env, uint32_t address, xn = 1; } - *prot = simple_ap_to_rw_prot(env, mmu_idx, ap); - if (*prot && !xn && !(pxn && !is_user)) { - *prot |= PAGE_EXEC; + result->prot = simple_ap_to_rw_prot(env, mmu_idx, ap); + if (result->prot && !xn && !(pxn && !is_user)) { + result->prot |= PAGE_EXEC; } /* * We don't need to look the attribute up in the MAIR0/MAIR1 @@ -XXX,XX +XXX,XX @@ bool pmsav8_mpu_lookup(CPUARMState *env, uint32_t address, fi->type = ARMFault_Permission; fi->level = 1; - return !(*prot & (1 << access_type)); + return !(result->prot & (1 << access_type)); } static bool v8m_is_sau_exempt(CPUARMState *env, @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_pmsav8(CPUARMState *env, uint32_t address, } ret = pmsav8_mpu_lookup(env, address, access_type, mmu_idx, - &result->phys, &result->attrs, &result->prot, - &mpu_is_subpage, fi, NULL); + result, &mpu_is_subpage, fi, NULL); result->page_size = sattrs.subpage || mpu_is_subpage ? 1 : TARGET_PAGE_SIZE; return ret; -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> This can be made redundant with result->page_size, by moving the basic set of page_size from get_phys_addr_pmsav8. We still need to overwrite page_size when v8m_security_lookup signals a subpage. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220822152741.1617527-11-richard.henderson@linaro.org [PMM: Update a comment that used to refer to is_subpage] Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/internals.h | 4 ++-- target/arm/m_helper.c | 3 +-- target/arm/ptw.c | 23 ++++++++++++----------- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/target/arm/internals.h b/target/arm/internals.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/internals.h +++ b/target/arm/internals.h @@ -XXX,XX +XXX,XX @@ bool get_phys_addr(CPUARMState *env, target_ulong address, bool pmsav8_mpu_lookup(CPUARMState *env, uint32_t address, MMUAccessType access_type, ARMMMUIdx mmu_idx, - GetPhysAddrResult *result, bool *is_subpage, - ARMMMUFaultInfo *fi, uint32_t *mregion); + GetPhysAddrResult *result, ARMMMUFaultInfo *fi, + uint32_t *mregion); void arm_log_exception(CPUState *cs); diff --git a/target/arm/m_helper.c b/target/arm/m_helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/m_helper.c +++ b/target/arm/m_helper.c @@ -XXX,XX +XXX,XX @@ uint32_t HELPER(v7m_tt)(CPUARMState *env, uint32_t addr, uint32_t op) if (arm_current_el(env) != 0 || alt) { GetPhysAddrResult res = {}; ARMMMUFaultInfo fi = {}; - bool is_subpage; /* We can ignore the return value as prot is always set */ pmsav8_mpu_lookup(env, addr, MMU_DATA_LOAD, mmu_idx, - &res, &is_subpage, &fi, &mregion); + &res, &fi, &mregion); if (mregion == -1) { mrvalid = false; mregion = 0; diff --git a/target/arm/ptw.c b/target/arm/ptw.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/ptw.c +++ b/target/arm/ptw.c @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address, bool pmsav8_mpu_lookup(CPUARMState *env, uint32_t address, MMUAccessType access_type, ARMMMUIdx mmu_idx, - GetPhysAddrResult *result, bool *is_subpage, - ARMMMUFaultInfo *fi, uint32_t *mregion) + GetPhysAddrResult *result, ARMMMUFaultInfo *fi, + uint32_t *mregion) { /* * Perform a PMSAv8 MPU lookup (without also doing the SAU check @@ -XXX,XX +XXX,XX @@ bool pmsav8_mpu_lookup(CPUARMState *env, uint32_t address, * mregion is (if not NULL) set to the region number which matched, * or -1 if no region number is returned (MPU off, address did not * hit a region, address hit in multiple regions). - * We set is_subpage to true if the region hit doesn't cover the - * entire TARGET_PAGE the address is within. + * If the region hit doesn't cover the entire TARGET_PAGE the address + * is within, then we set the result page_size to 1 to force the + * memory system to use a subpage. */ ARMCPU *cpu = env_archcpu(env); bool is_user = regime_is_user(env, mmu_idx); @@ -XXX,XX +XXX,XX @@ bool pmsav8_mpu_lookup(CPUARMState *env, uint32_t address, uint32_t addr_page_base = address & TARGET_PAGE_MASK; uint32_t addr_page_limit = addr_page_base + (TARGET_PAGE_SIZE - 1); - *is_subpage = false; + result->page_size = TARGET_PAGE_SIZE; result->phys = address; result->prot = 0; if (mregion) { @@ -XXX,XX +XXX,XX @@ bool pmsav8_mpu_lookup(CPUARMState *env, uint32_t address, ranges_overlap(base, limit - base + 1, addr_page_base, TARGET_PAGE_SIZE)) { - *is_subpage = true; + result->page_size = 1; } continue; } if (base > addr_page_base || limit < addr_page_limit) { - *is_subpage = true; + result->page_size = 1; } if (matchregion != -1) { @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_pmsav8(CPUARMState *env, uint32_t address, uint32_t secure = regime_is_secure(env, mmu_idx); V8M_SAttributes sattrs = {}; bool ret; - bool mpu_is_subpage; if (arm_feature(env, ARM_FEATURE_M_SECURITY)) { v8m_security_lookup(env, address, access_type, mmu_idx, &sattrs); @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_pmsav8(CPUARMState *env, uint32_t address, } ret = pmsav8_mpu_lookup(env, address, access_type, mmu_idx, - result, &mpu_is_subpage, fi, NULL); - result->page_size = - sattrs.subpage || mpu_is_subpage ? 1 : TARGET_PAGE_SIZE; + result, fi, NULL); + if (sattrs.subpage) { + result->page_size = 1; + } return ret; } -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Remove the use of regime_is_secure from v8m_security_lookup, passing the new parameter to the lookup instead. Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220822152741.1617527-12-richard.henderson@linaro.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/internals.h | 2 +- target/arm/m_helper.c | 9 ++++++--- target/arm/ptw.c | 9 +++++---- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/target/arm/internals.h b/target/arm/internals.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/internals.h +++ b/target/arm/internals.h @@ -XXX,XX +XXX,XX @@ typedef struct V8M_SAttributes { void v8m_security_lookup(CPUARMState *env, uint32_t address, MMUAccessType access_type, ARMMMUIdx mmu_idx, - V8M_SAttributes *sattrs); + bool secure, V8M_SAttributes *sattrs); /* Cacheability and shareability attributes for a memory access */ typedef struct ARMCacheAttrs { diff --git a/target/arm/m_helper.c b/target/arm/m_helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/m_helper.c +++ b/target/arm/m_helper.c @@ -XXX,XX +XXX,XX @@ static bool arm_v7m_load_vector(ARMCPU *cpu, int exc, bool targets_secure, if (arm_feature(env, ARM_FEATURE_M_SECURITY)) { V8M_SAttributes sattrs = {}; - v8m_security_lookup(env, addr, MMU_DATA_LOAD, mmu_idx, &sattrs); + v8m_security_lookup(env, addr, MMU_DATA_LOAD, mmu_idx, + targets_secure, &sattrs); if (sattrs.ns) { attrs.secure = false; } else if (!targets_secure) { @@ -XXX,XX +XXX,XX @@ static bool v7m_read_half_insn(ARMCPU *cpu, ARMMMUIdx mmu_idx, ARMMMUFaultInfo fi = {}; MemTxResult txres; - v8m_security_lookup(env, addr, MMU_INST_FETCH, mmu_idx, &sattrs); + v8m_security_lookup(env, addr, MMU_INST_FETCH, mmu_idx, + regime_is_secure(env, mmu_idx), &sattrs); if (!sattrs.nsc || sattrs.ns) { /* * This must be the second half of the insn, and it straddles a @@ -XXX,XX +XXX,XX @@ uint32_t HELPER(v7m_tt)(CPUARMState *env, uint32_t addr, uint32_t op) } if (env->v7m.secure) { - v8m_security_lookup(env, addr, MMU_DATA_LOAD, mmu_idx, &sattrs); + v8m_security_lookup(env, addr, MMU_DATA_LOAD, mmu_idx, + targetsec, &sattrs); nsr = sattrs.ns && r; nsrw = sattrs.ns && rw; } else { diff --git a/target/arm/ptw.c b/target/arm/ptw.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/ptw.c +++ b/target/arm/ptw.c @@ -XXX,XX +XXX,XX @@ static bool v8m_is_sau_exempt(CPUARMState *env, } void v8m_security_lookup(CPUARMState *env, uint32_t address, - MMUAccessType access_type, ARMMMUIdx mmu_idx, - V8M_SAttributes *sattrs) + MMUAccessType access_type, ARMMMUIdx mmu_idx, + bool is_secure, V8M_SAttributes *sattrs) { /* * Look up the security attributes for this address. Compare the @@ -XXX,XX +XXX,XX @@ void v8m_security_lookup(CPUARMState *env, uint32_t address, } if (idau_exempt || v8m_is_sau_exempt(env, address, access_type)) { - sattrs->ns = !regime_is_secure(env, mmu_idx); + sattrs->ns = !is_secure; return; } @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_pmsav8(CPUARMState *env, uint32_t address, bool ret; if (arm_feature(env, ARM_FEATURE_M_SECURITY)) { - v8m_security_lookup(env, address, access_type, mmu_idx, &sattrs); + v8m_security_lookup(env, address, access_type, mmu_idx, + secure, &sattrs); if (access_type == MMU_INST_FETCH) { /* * Instruction fetches always use the MMU bank and the -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Remove the use of regime_is_secure from pmsav8_mpu_lookup, passing the new parameter to the lookup instead. Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220822152741.1617527-13-richard.henderson@linaro.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/internals.h | 4 ++-- target/arm/m_helper.c | 2 +- target/arm/ptw.c | 7 +++---- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/target/arm/internals.h b/target/arm/internals.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/internals.h +++ b/target/arm/internals.h @@ -XXX,XX +XXX,XX @@ bool get_phys_addr(CPUARMState *env, target_ulong address, bool pmsav8_mpu_lookup(CPUARMState *env, uint32_t address, MMUAccessType access_type, ARMMMUIdx mmu_idx, - GetPhysAddrResult *result, ARMMMUFaultInfo *fi, - uint32_t *mregion); + bool is_secure, GetPhysAddrResult *result, + ARMMMUFaultInfo *fi, uint32_t *mregion); void arm_log_exception(CPUState *cs); diff --git a/target/arm/m_helper.c b/target/arm/m_helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/m_helper.c +++ b/target/arm/m_helper.c @@ -XXX,XX +XXX,XX @@ uint32_t HELPER(v7m_tt)(CPUARMState *env, uint32_t addr, uint32_t op) ARMMMUFaultInfo fi = {}; /* We can ignore the return value as prot is always set */ - pmsav8_mpu_lookup(env, addr, MMU_DATA_LOAD, mmu_idx, + pmsav8_mpu_lookup(env, addr, MMU_DATA_LOAD, mmu_idx, targetsec, &res, &fi, &mregion); if (mregion == -1) { mrvalid = false; diff --git a/target/arm/ptw.c b/target/arm/ptw.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/ptw.c +++ b/target/arm/ptw.c @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address, bool pmsav8_mpu_lookup(CPUARMState *env, uint32_t address, MMUAccessType access_type, ARMMMUIdx mmu_idx, - GetPhysAddrResult *result, ARMMMUFaultInfo *fi, - uint32_t *mregion) + bool secure, GetPhysAddrResult *result, + ARMMMUFaultInfo *fi, uint32_t *mregion) { /* * Perform a PMSAv8 MPU lookup (without also doing the SAU check @@ -XXX,XX +XXX,XX @@ bool pmsav8_mpu_lookup(CPUARMState *env, uint32_t address, */ ARMCPU *cpu = env_archcpu(env); bool is_user = regime_is_user(env, mmu_idx); - uint32_t secure = regime_is_secure(env, mmu_idx); int n; int matchregion = -1; bool hit = false; @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_pmsav8(CPUARMState *env, uint32_t address, } } - ret = pmsav8_mpu_lookup(env, address, access_type, mmu_idx, + ret = pmsav8_mpu_lookup(env, address, access_type, mmu_idx, secure, result, fi, NULL); if (sattrs.subpage) { result->page_size = 1; -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Remove the use of regime_is_secure from get_phys_addr_v5, passing the new parameter to the lookup instead. Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> [PMM: Folded in definition of local is_secure in get_phys_addr(), since I dropped the earlier patch that would have provided it] Message-id: 20220822152741.1617527-14-richard.henderson@linaro.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/ptw.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/target/arm/ptw.c b/target/arm/ptw.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/ptw.c +++ b/target/arm/ptw.c @@ -XXX,XX +XXX,XX @@ static int simple_ap_to_rw_prot(CPUARMState *env, ARMMMUIdx mmu_idx, int ap) static bool get_phys_addr_v5(CPUARMState *env, uint32_t address, MMUAccessType access_type, ARMMMUIdx mmu_idx, - GetPhysAddrResult *result, ARMMMUFaultInfo *fi) + bool is_secure, GetPhysAddrResult *result, + ARMMMUFaultInfo *fi) { int level = 1; uint32_t table; @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_v5(CPUARMState *env, uint32_t address, fi->type = ARMFault_Translation; goto do_fault; } - desc = arm_ldl_ptw(env, table, regime_is_secure(env, mmu_idx), - mmu_idx, fi); + desc = arm_ldl_ptw(env, table, is_secure, mmu_idx, fi); if (fi->type != ARMFault_None) { goto do_fault; } @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_v5(CPUARMState *env, uint32_t address, /* Fine pagetable. */ table = (desc & 0xfffff000) | ((address >> 8) & 0xffc); } - desc = arm_ldl_ptw(env, table, regime_is_secure(env, mmu_idx), - mmu_idx, fi); + desc = arm_ldl_ptw(env, table, is_secure, mmu_idx, fi); if (fi->type != ARMFault_None) { goto do_fault; } @@ -XXX,XX +XXX,XX @@ bool get_phys_addr(CPUARMState *env, target_ulong address, GetPhysAddrResult *result, ARMMMUFaultInfo *fi) { ARMMMUIdx s1_mmu_idx = stage_1_mmu_idx(mmu_idx); + bool is_secure = regime_is_secure(env, mmu_idx); if (mmu_idx != s1_mmu_idx) { /* @@ -XXX,XX +XXX,XX @@ bool get_phys_addr(CPUARMState *env, target_ulong address, * cannot upgrade an non-secure translation regime's attributes * to secure. */ - result->attrs.secure = regime_is_secure(env, mmu_idx); + result->attrs.secure = is_secure; result->attrs.user = regime_is_user(env, mmu_idx); /* @@ -XXX,XX +XXX,XX @@ bool get_phys_addr(CPUARMState *env, target_ulong address, result, fi); } else { return get_phys_addr_v5(env, address, access_type, mmu_idx, - result, fi); + is_secure, result, fi); } } -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Remove the use of regime_is_secure from get_phys_addr_v6, passing the new parameter to the lookup instead. Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220822152741.1617527-15-richard.henderson@linaro.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/ptw.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/target/arm/ptw.c b/target/arm/ptw.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/ptw.c +++ b/target/arm/ptw.c @@ -XXX,XX +XXX,XX @@ do_fault: static bool get_phys_addr_v6(CPUARMState *env, uint32_t address, MMUAccessType access_type, ARMMMUIdx mmu_idx, - GetPhysAddrResult *result, ARMMMUFaultInfo *fi) + bool is_secure, GetPhysAddrResult *result, + ARMMMUFaultInfo *fi) { ARMCPU *cpu = env_archcpu(env); int level = 1; @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_v6(CPUARMState *env, uint32_t address, fi->type = ARMFault_Translation; goto do_fault; } - desc = arm_ldl_ptw(env, table, regime_is_secure(env, mmu_idx), - mmu_idx, fi); + desc = arm_ldl_ptw(env, table, is_secure, mmu_idx, fi); if (fi->type != ARMFault_None) { goto do_fault; } @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_v6(CPUARMState *env, uint32_t address, ns = extract32(desc, 3, 1); /* Lookup l2 entry. */ table = (desc & 0xfffffc00) | ((address >> 10) & 0x3fc); - desc = arm_ldl_ptw(env, table, regime_is_secure(env, mmu_idx), - mmu_idx, fi); + desc = arm_ldl_ptw(env, table, is_secure, mmu_idx, fi); if (fi->type != ARMFault_None) { goto do_fault; } @@ -XXX,XX +XXX,XX @@ bool get_phys_addr(CPUARMState *env, target_ulong address, result, fi); } else if (regime_sctlr(env, mmu_idx) & SCTLR_XP) { return get_phys_addr_v6(env, address, access_type, mmu_idx, - result, fi); + is_secure, result, fi); } else { return get_phys_addr_v5(env, address, access_type, mmu_idx, is_secure, result, fi); -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Remove the use of regime_is_secure from get_phys_addr_pmsav8. Since we already had a local variable named secure, use that. Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220822152741.1617527-16-richard.henderson@linaro.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/ptw.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/target/arm/ptw.c b/target/arm/ptw.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/ptw.c +++ b/target/arm/ptw.c @@ -XXX,XX +XXX,XX @@ void v8m_security_lookup(CPUARMState *env, uint32_t address, static bool get_phys_addr_pmsav8(CPUARMState *env, uint32_t address, MMUAccessType access_type, ARMMMUIdx mmu_idx, - GetPhysAddrResult *result, + bool secure, GetPhysAddrResult *result, ARMMMUFaultInfo *fi) { - uint32_t secure = regime_is_secure(env, mmu_idx); V8M_SAttributes sattrs = {}; bool ret; @@ -XXX,XX +XXX,XX @@ bool get_phys_addr(CPUARMState *env, target_ulong address, if (arm_feature(env, ARM_FEATURE_V8)) { /* PMSAv8 */ ret = get_phys_addr_pmsav8(env, address, access_type, mmu_idx, - result, fi); + is_secure, result, fi); } else if (arm_feature(env, ARM_FEATURE_V7)) { /* PMSAv7 */ ret = get_phys_addr_pmsav7(env, address, access_type, mmu_idx, -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Remove the use of regime_is_secure from pmsav7_use_background_region, using the new parameter instead. Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220822152741.1617527-17-richard.henderson@linaro.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/ptw.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/target/arm/ptw.c b/target/arm/ptw.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/ptw.c +++ b/target/arm/ptw.c @@ -XXX,XX +XXX,XX @@ static bool m_is_system_region(CPUARMState *env, uint32_t address) } static bool pmsav7_use_background_region(ARMCPU *cpu, ARMMMUIdx mmu_idx, - bool is_user) + bool is_secure, bool is_user) { /* * Return true if we should use the default memory map as a @@ -XXX,XX +XXX,XX @@ static bool pmsav7_use_background_region(ARMCPU *cpu, ARMMMUIdx mmu_idx, } if (arm_feature(env, ARM_FEATURE_M)) { - return env->v7m.mpu_ctrl[regime_is_secure(env, mmu_idx)] - & R_V7M_MPU_CTRL_PRIVDEFENA_MASK; + return env->v7m.mpu_ctrl[is_secure] & R_V7M_MPU_CTRL_PRIVDEFENA_MASK; } else { return regime_sctlr(env, mmu_idx) & SCTLR_BR; } @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address, { ARMCPU *cpu = env_archcpu(env); int n; + bool secure = regime_is_secure(env, mmu_idx); bool is_user = regime_is_user(env, mmu_idx); result->phys = address; @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address, } if (n == -1) { /* no hits */ - if (!pmsav7_use_background_region(cpu, mmu_idx, is_user)) { + if (!pmsav7_use_background_region(cpu, mmu_idx, secure, is_user)) { /* background fault */ fi->type = ARMFault_Background; return true; @@ -XXX,XX +XXX,XX @@ bool pmsav8_mpu_lookup(CPUARMState *env, uint32_t address, } else if (m_is_ppb_region(env, address)) { hit = true; } else { - if (pmsav7_use_background_region(cpu, mmu_idx, is_user)) { + if (pmsav7_use_background_region(cpu, mmu_idx, secure, is_user)) { hit = true; } -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Remove the use of regime_is_secure from get_phys_addr_pmsav7, using the new parameter instead. Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220822152741.1617527-19-richard.henderson@linaro.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/ptw.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/target/arm/ptw.c b/target/arm/ptw.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/ptw.c +++ b/target/arm/ptw.c @@ -XXX,XX +XXX,XX @@ static bool pmsav7_use_background_region(ARMCPU *cpu, ARMMMUIdx mmu_idx, static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address, MMUAccessType access_type, ARMMMUIdx mmu_idx, - GetPhysAddrResult *result, + bool secure, GetPhysAddrResult *result, ARMMMUFaultInfo *fi) { ARMCPU *cpu = env_archcpu(env); int n; - bool secure = regime_is_secure(env, mmu_idx); bool is_user = regime_is_user(env, mmu_idx); result->phys = address; @@ -XXX,XX +XXX,XX @@ bool get_phys_addr(CPUARMState *env, target_ulong address, } else if (arm_feature(env, ARM_FEATURE_V7)) { /* PMSAv7 */ ret = get_phys_addr_pmsav7(env, address, access_type, mmu_idx, - result, fi); + is_secure, result, fi); } else { /* Pre-v7 MPU */ ret = get_phys_addr_pmsav5(env, address, access_type, mmu_idx, -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Remove the use of regime_is_secure from get_phys_addr_pmsav5. Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220822152741.1617527-21-richard.henderson@linaro.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/ptw.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/target/arm/ptw.c b/target/arm/ptw.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/ptw.c +++ b/target/arm/ptw.c @@ -XXX,XX +XXX,XX @@ do_fault: static bool get_phys_addr_pmsav5(CPUARMState *env, uint32_t address, MMUAccessType access_type, ARMMMUIdx mmu_idx, - GetPhysAddrResult *result, + bool is_secure, GetPhysAddrResult *result, ARMMMUFaultInfo *fi) { int n; @@ -XXX,XX +XXX,XX @@ bool get_phys_addr(CPUARMState *env, target_ulong address, } else { /* Pre-v7 MPU */ ret = get_phys_addr_pmsav5(env, address, access_type, mmu_idx, - result, fi); + is_secure, result, fi); } qemu_log_mask(CPU_LOG_MMU, "PMSA MPU lookup for %s at 0x%08" PRIx32 " mmu_idx %u -> %s (prot %c%c%c)\n", -- 2.25.1 From: Keqian Zhu <zhukeqian1@huawei.com> Setup an ARM virtual machine of machine virt and execute qmp "query-acpi-ospm-status" causes segmentation fault with following dumpstack: #1 0x0000aaaaab64235c in qmp_query_acpi_ospm_status (errp=errp@entry=0xfffffffff030) at ../monitor/qmp-cmds.c:312 #2 0x0000aaaaabfc4e20 in qmp_marshal_query_acpi_ospm_status (args=<optimized out>, ret=0xffffea4ffe90, errp=0xffffea4ffe88) at qapi/qapi-commands-acpi.c:63 #3 0x0000aaaaabff8ba0 in do_qmp_dispatch_bh (opaque=0xffffea4ffe98) at ../qapi/qmp-dispatch.c:128 #4 0x0000aaaaac02e594 in aio_bh_call (bh=0xffffe0004d80) at ../util/async.c:150 #5 aio_bh_poll (ctx=ctx@entry=0xaaaaad0f6040) at ../util/async.c:178 #6 0x0000aaaaac00bd40 in aio_dispatch (ctx=ctx@entry=0xaaaaad0f6040) at ../util/aio-posix.c:421 #7 0x0000aaaaac02e010 in aio_ctx_dispatch (source=0xaaaaad0f6040, callback=<optimized out>, user_data=<optimized out>) at ../util/async.c:320 #8 0x0000fffff76f6884 in g_main_context_dispatch () at /usr/lib64/libglib-2.0.so.0 #9 0x0000aaaaac0452d4 in glib_pollfds_poll () at ../util/main-loop.c:297 #10 os_host_main_loop_wait (timeout=0) at ../util/main-loop.c:320 #11 main_loop_wait (nonblocking=nonblocking@entry=0) at ../util/main-loop.c:596 #12 0x0000aaaaab5c9e50 in qemu_main_loop () at ../softmmu/runstate.c:734 #13 0x0000aaaaab185370 in qemu_main (argc=argc@entry=47, argv=argv@entry=0xfffffffff518, envp=envp@entry=0x0) at ../softmmu/main.c:38 #14 0x0000aaaaab16f99c in main (argc=47, argv=0xfffffffff518) at ../softmmu/main.c:47 Fixes: ebb62075021a ("hw/acpi: Add ACPI Generic Event Device Support") Signed-off-by: Keqian Zhu <zhukeqian1@huawei.com> Reviewed-by: Igor Mammedov <imammedo@redhat.com> Message-id: 20220816094957.31700-1-zhukeqian1@huawei.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/acpi/generic_event_device.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c index XXXXXXX..XXXXXXX 100644 --- a/hw/acpi/generic_event_device.c +++ b/hw/acpi/generic_event_device.c @@ -XXX,XX +XXX,XX @@ static void acpi_ged_unplug_cb(HotplugHandler *hotplug_dev, } } +static void acpi_ged_ospm_status(AcpiDeviceIf *adev, ACPIOSTInfoList ***list) +{ + AcpiGedState *s = ACPI_GED(adev); + + acpi_memory_ospm_status(&s->memhp_state, list); +} + static void acpi_ged_send_event(AcpiDeviceIf *adev, AcpiEventStatusBits ev) { AcpiGedState *s = ACPI_GED(adev); @@ -XXX,XX +XXX,XX @@ static void acpi_ged_class_init(ObjectClass *class, void *data) hc->unplug_request = acpi_ged_unplug_request_cb; hc->unplug = acpi_ged_unplug_cb; + adevc->ospm_status = acpi_ged_ospm_status; adevc->send_event = acpi_ged_send_event; } -- 2.25.1 From: Lucas Dietrich <ld.adecy@gmail.com> The LAN9118 allows the guest to specify a level for both the TX and RX FIFOs at which an interrupt will be generated. We implement the RSFL_INT interrupt for the RX FIFO but are missing the handling of the equivalent TSFL_INT for the TX FIFO. Add the missing test to set the interrupt if the TX FIFO has exceeded the guest-specified level. This flag is required for Micrium lan911x ethernet driver to work. Signed-off-by: Lucas Dietrich <ld.adecy@gmail.com> [PMM: Tweaked commit message and comment] Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/net/lan9118.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/hw/net/lan9118.c b/hw/net/lan9118.c index XXXXXXX..XXXXXXX 100644 --- a/hw/net/lan9118.c +++ b/hw/net/lan9118.c @@ -XXX,XX +XXX,XX @@ static void do_tx_packet(lan9118_state *s) n = (s->tx_status_fifo_head + s->tx_status_fifo_used) & 511; s->tx_status_fifo[n] = status; s->tx_status_fifo_used++; + + /* + * Generate TSFL interrupt if TX FIFO level exceeds the level + * specified in the FIFO_INT TX Status Level field. + */ + if (s->tx_status_fifo_used > ((s->fifo_int >> 16) & 0xff)) { + s->int_sts |= TSFL_INT; + } if (s->tx_status_fifo_used == 512) { s->int_sts |= TSFF_INT; /* TODO: Stop transmission. */ -- 2.25.1 From: Philippe Mathieu-Daudé <philmd@redhat.com> Replace '84' magic value by the X_MAX definition, and '1' by Y_MAX. Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 20220819153931.3147384-2-peter.maydell@linaro.org --- chardev/baum.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/chardev/baum.c b/chardev/baum.c index XXXXXXX..XXXXXXX 100644 --- a/chardev/baum.c +++ b/chardev/baum.c @@ -XXX,XX +XXX,XX @@ #define BUF_SIZE 256 +#define X_MAX 84 +#define Y_MAX 1 + struct BaumChardev { Chardev parent; @@ -XXX,XX +XXX,XX @@ static int baum_deferred_init(BaumChardev *baum) brlapi_perror("baum: brlapi__getDisplaySize"); return 0; } - if (baum->y > 1) { - baum->y = 1; + if (baum->y > Y_MAX) { + baum->y = Y_MAX; } - if (baum->x > 84) { - baum->x = 84; + if (baum->x > X_MAX) { + baum->x = X_MAX; } con = qemu_console_lookup_by_index(0); -- 2.25.1 From: Philippe Mathieu-Daudé <philmd@redhat.com> We know 'x * y' will be at most 'X_MAX * Y_MAX' (which is not a big value, it is actually 84). Instead of having the compiler use variable-length array, declare an array able to hold the maximum 'x * y'. Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 20220819153931.3147384-3-peter.maydell@linaro.org --- chardev/baum.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/chardev/baum.c b/chardev/baum.c index XXXXXXX..XXXXXXX 100644 --- a/chardev/baum.c +++ b/chardev/baum.c @@ -XXX,XX +XXX,XX @@ static int baum_eat_packet(BaumChardev *baum, const uint8_t *buf, int len) switch (req) { case BAUM_REQ_DisplayData: { - uint8_t cells[baum->x * baum->y], c; - uint8_t text[baum->x * baum->y]; - uint8_t zero[baum->x * baum->y]; + uint8_t cells[X_MAX * Y_MAX], c; + uint8_t text[X_MAX * Y_MAX]; + uint8_t zero[X_MAX * Y_MAX]; int cursor = BRLAPI_CURSOR_OFF; int i; @@ -XXX,XX +XXX,XX @@ static int baum_eat_packet(BaumChardev *baum, const uint8_t *buf, int len) } timer_del(baum->cellCount_timer); - memset(zero, 0, sizeof(zero)); + memset(zero, 0, baum->x * baum->y); brlapi_writeArguments_t wa = { .displayNumber = BRLAPI_DISPLAY_DEFAULT, -- 2.25.1 From: Philippe Mathieu-Daudé <philmd@redhat.com> Use autofree heap allocation instead of variable-length array on the stack. Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 20220819153931.3147384-4-peter.maydell@linaro.org --- chardev/baum.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/chardev/baum.c b/chardev/baum.c index XXXXXXX..XXXXXXX 100644 --- a/chardev/baum.c +++ b/chardev/baum.c @@ -XXX,XX +XXX,XX @@ static void baum_chr_accept_input(struct Chardev *chr) static void baum_write_packet(BaumChardev *baum, const uint8_t *buf, int len) { Chardev *chr = CHARDEV(baum); - uint8_t io_buf[1 + 2 * len], *cur = io_buf; + g_autofree uint8_t *io_buf = g_malloc(1 + 2 * len); + uint8_t *cur = io_buf; int room; *cur++ = ESC; while (len--) -- 2.25.1 From: Philippe Mathieu-Daudé <philmd@redhat.com> The combined_key[... QIO_CHANNEL_WEBSOCK_GUID_LEN ...] array in qio_channel_websock_handshake_send_res_ok() expands to a call to strlen(QIO_CHANNEL_WEBSOCK_GUID), and the compiler doesn't realize the string is const, so consider combined_key[] being a variable-length array. To remove the variable-length array, we provide it a hint to the compiler by using sizeof() - 1 instead of strlen(). Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 20220819153931.3147384-5-peter.maydell@linaro.org --- io/channel-websock.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/io/channel-websock.c b/io/channel-websock.c index XXXXXXX..XXXXXXX 100644 --- a/io/channel-websock.c +++ b/io/channel-websock.c @@ -XXX,XX +XXX,XX @@ #define QIO_CHANNEL_WEBSOCK_CLIENT_KEY_LEN 24 #define QIO_CHANNEL_WEBSOCK_GUID "258EAFA5-E914-47DA-95CA-C5AB0DC85B11" -#define QIO_CHANNEL_WEBSOCK_GUID_LEN strlen(QIO_CHANNEL_WEBSOCK_GUID) +#define QIO_CHANNEL_WEBSOCK_GUID_LEN (sizeof(QIO_CHANNEL_WEBSOCK_GUID) - 1) #define QIO_CHANNEL_WEBSOCK_HEADER_PROTOCOL "sec-websocket-protocol" #define QIO_CHANNEL_WEBSOCK_HEADER_VERSION "sec-websocket-version" -- 2.25.1 From: Philippe Mathieu-Daudé <philmd@redhat.com> The compiler isn't clever enough to figure 'min_buf_size' is a constant, so help it by using a definitions instead. Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> Acked-by: Jason Wang <jasowang@redhat.com> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 20220819153931.3147384-6-peter.maydell@linaro.org --- hw/net/e1000e_core.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/hw/net/e1000e_core.c b/hw/net/e1000e_core.c index XXXXXXX..XXXXXXX 100644 --- a/hw/net/e1000e_core.c +++ b/hw/net/e1000e_core.c @@ -XXX,XX +XXX,XX @@ e1000e_rx_fix_l4_csum(E1000ECore *core, struct NetRxPkt *pkt) } } +/* Min. octets in an ethernet frame sans FCS */ +#define MIN_BUF_SIZE 60 + ssize_t e1000e_receive_iov(E1000ECore *core, const struct iovec *iov, int iovcnt) { static const int maximum_ethernet_hdr_len = (14 + 4); - /* Min. octets in an ethernet frame sans FCS */ - static const int min_buf_size = 60; uint32_t n = 0; - uint8_t min_buf[min_buf_size]; + uint8_t min_buf[MIN_BUF_SIZE]; struct iovec min_iov; uint8_t *filter_buf; size_t size, orig_size; -- 2.25.1 From: Philippe Mathieu-Daudé <philmd@redhat.com> Use autofree heap allocation instead of variable-length array on the stack. Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> Acked-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> Message-id: 20220819153931.3147384-7-peter.maydell@linaro.org --- hw/ppc/pnv.c | 4 ++-- hw/ppc/spapr.c | 8 ++++---- hw/ppc/spapr_pci_nvlink2.c | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c index XXXXXXX..XXXXXXX 100644 --- a/hw/ppc/pnv.c +++ b/hw/ppc/pnv.c @@ -XXX,XX +XXX,XX @@ static void pnv_dt_core(PnvChip *chip, PnvCore *pc, void *fdt) int smt_threads = CPU_CORE(pc)->nr_threads; CPUPPCState *env = &cpu->env; PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cs); - uint32_t servers_prop[smt_threads]; + g_autofree uint32_t *servers_prop = g_new(uint32_t, smt_threads); int i; uint32_t segs[] = {cpu_to_be32(28), cpu_to_be32(40), 0xffffffff, 0xffffffff}; @@ -XXX,XX +XXX,XX @@ static void pnv_dt_core(PnvChip *chip, PnvCore *pc, void *fdt) servers_prop[i] = cpu_to_be32(pc->pir + i); } _FDT((fdt_setprop(fdt, offset, "ibm,ppc-interrupt-server#s", - servers_prop, sizeof(servers_prop)))); + servers_prop, sizeof(*servers_prop) * smt_threads))); } static void pnv_dt_icp(PnvChip *chip, void *fdt, uint32_t pir, diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index XXXXXXX..XXXXXXX 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -XXX,XX +XXX,XX @@ static int spapr_fixup_cpu_smt_dt(void *fdt, int offset, PowerPCCPU *cpu, int smt_threads) { int i, ret = 0; - uint32_t servers_prop[smt_threads]; - uint32_t gservers_prop[smt_threads * 2]; + g_autofree uint32_t *servers_prop = g_new(uint32_t, smt_threads); + g_autofree uint32_t *gservers_prop = g_new(uint32_t, smt_threads * 2); int index = spapr_get_vcpu_id(cpu); if (cpu->compat_pvr) { @@ -XXX,XX +XXX,XX @@ static int spapr_fixup_cpu_smt_dt(void *fdt, int offset, PowerPCCPU *cpu, gservers_prop[i*2 + 1] = 0; } ret = fdt_setprop(fdt, offset, "ibm,ppc-interrupt-server#s", - servers_prop, sizeof(servers_prop)); + servers_prop, sizeof(*servers_prop) * smt_threads); if (ret < 0) { return ret; } ret = fdt_setprop(fdt, offset, "ibm,ppc-interrupt-gserver#s", - gservers_prop, sizeof(gservers_prop)); + gservers_prop, sizeof(*gservers_prop) * smt_threads * 2); return ret; } diff --git a/hw/ppc/spapr_pci_nvlink2.c b/hw/ppc/spapr_pci_nvlink2.c index XXXXXXX..XXXXXXX 100644 --- a/hw/ppc/spapr_pci_nvlink2.c +++ b/hw/ppc/spapr_pci_nvlink2.c @@ -XXX,XX +XXX,XX @@ void spapr_phb_nvgpu_populate_pcidev_dt(PCIDevice *dev, void *fdt, int offset, continue; } if (dev == nvslot->gpdev) { - uint32_t npus[nvslot->linknum]; + g_autofree uint32_t *npus = g_new(uint32_t, nvslot->linknum); for (j = 0; j < nvslot->linknum; ++j) { PCIDevice *npdev = nvslot->links[j].npdev; -- 2.25.1 From: Philippe Mathieu-Daudé <philmd@redhat.com> Use autofree heap allocation instead of variable-length array on the stack. Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> Acked-by: David Gibson <david@gibson.dropbear.id.au> Reviewed-by: Greg Kurz <groug@kaod.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 20220819153931.3147384-8-peter.maydell@linaro.org --- hw/intc/xics.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/intc/xics.c b/hw/intc/xics.c index XXXXXXX..XXXXXXX 100644 --- a/hw/intc/xics.c +++ b/hw/intc/xics.c @@ -XXX,XX +XXX,XX @@ static void ics_reset_irq(ICSIRQState *irq) static void ics_reset(DeviceState *dev) { ICSState *ics = ICS(dev); + g_autofree uint8_t *flags = g_malloc(ics->nr_irqs); int i; - uint8_t flags[ics->nr_irqs]; for (i = 0; i < ics->nr_irqs; i++) { flags[i] = ics->irqs[i].flags; -- 2.25.1 From: Philippe Mathieu-Daudé <philmd@redhat.com> Use autofree heap allocation instead of variable-length array on the stack. Replace the snprintf() call by g_strdup_printf(). Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 20220819153931.3147384-9-peter.maydell@linaro.org --- hw/i386/multiboot.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/hw/i386/multiboot.c b/hw/i386/multiboot.c index XXXXXXX..XXXXXXX 100644 --- a/hw/i386/multiboot.c +++ b/hw/i386/multiboot.c @@ -XXX,XX +XXX,XX @@ int load_multiboot(X86MachineState *x86ms, uint8_t *mb_bootinfo_data; uint32_t cmdline_len; GList *mods = NULL; + g_autofree char *kcmdline = NULL; /* Ok, let's see if it is a multiboot image. The header is 12x32bit long, so the latest entry may be 8192 - 48. */ @@ -XXX,XX +XXX,XX @@ int load_multiboot(X86MachineState *x86ms, } /* Commandline support */ - char kcmdline[strlen(kernel_filename) + strlen(kernel_cmdline) + 2]; - snprintf(kcmdline, sizeof(kcmdline), "%s %s", - kernel_filename, kernel_cmdline); + kcmdline = g_strdup_printf("%s %s", kernel_filename, kernel_cmdline); stl_p(bootinfo + MBI_CMDLINE, mb_add_cmdline(&mbs, kcmdline)); stl_p(bootinfo + MBI_BOOTLOADER, mb_add_bootloader(&mbs, bootloader_name)); -- 2.25.1 From: Philippe Mathieu-Daudé <philmd@redhat.com> The compiler isn't clever enough to figure 'width' is a constant, so help it by using a definitions instead. Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 20220819153931.3147384-10-peter.maydell@linaro.org --- hw/usb/hcd-ohci.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c index XXXXXXX..XXXXXXX 100644 --- a/hw/usb/hcd-ohci.c +++ b/hw/usb/hcd-ohci.c @@ -XXX,XX +XXX,XX @@ static int ohci_service_iso_td(OHCIState *ohci, struct ohci_ed *ed) return 1; } +#define HEX_CHAR_PER_LINE 16 + static void ohci_td_pkt(const char *msg, const uint8_t *buf, size_t len) { bool print16; bool printall; - const int width = 16; int i; - char tmp[3 * width + 1]; + char tmp[3 * HEX_CHAR_PER_LINE + 1]; char *p = tmp; print16 = !!trace_event_get_state_backends(TRACE_USB_OHCI_TD_PKT_SHORT); @@ -XXX,XX +XXX,XX @@ static void ohci_td_pkt(const char *msg, const uint8_t *buf, size_t len) } for (i = 0; ; i++) { - if (i && (!(i % width) || (i == len))) { + if (i && (!(i % HEX_CHAR_PER_LINE) || (i == len))) { if (!printall) { trace_usb_ohci_td_pkt_short(msg, tmp); break; -- 2.25.1 From: Philippe Mathieu-Daudé <philmd@redhat.com> Use autofree heap allocation instead of variable-length array on the stack. Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 20220819153931.3147384-11-peter.maydell@linaro.org --- ui/curses.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/curses.c b/ui/curses.c index XXXXXXX..XXXXXXX 100644 --- a/ui/curses.c +++ b/ui/curses.c @@ -XXX,XX +XXX,XX @@ static void curses_update(DisplayChangeListener *dcl, int x, int y, int w, int h) { console_ch_t *line; - cchar_t curses_line[width]; + g_autofree cchar_t *curses_line = g_new(cchar_t, width); wchar_t wch[CCHARW_MAX]; attr_t attrs; short colors; -- 2.25.1 From: Philippe Mathieu-Daudé <philmd@redhat.com> Use autofree heap allocation instead of variable-length array on the stack. Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 20220819153931.3147384-12-peter.maydell@linaro.org --- tests/unit/test-vmstate.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/unit/test-vmstate.c b/tests/unit/test-vmstate.c index XXXXXXX..XXXXXXX 100644 --- a/tests/unit/test-vmstate.c +++ b/tests/unit/test-vmstate.c @@ -XXX,XX +XXX,XX @@ static void save_buffer(const uint8_t *buf, size_t buf_size) static void compare_vmstate(const uint8_t *wire, size_t size) { QEMUFile *f = open_test_file(false); - uint8_t result[size]; + g_autofree uint8_t *result = g_malloc(size); /* read back as binary */ - g_assert_cmpint(qemu_get_buffer(f, result, sizeof(result)), ==, - sizeof(result)); + g_assert_cmpint(qemu_get_buffer(f, result, size), ==, size); g_assert(!qemu_file_get_error(f)); /* Compare that what is on the file is the same that what we expected to be there */ - SUCCESS(memcmp(result, wire, sizeof(result))); + SUCCESS(memcmp(result, wire, size)); /* Must reach EOF */ qemu_get_byte(f); -- 2.25.1 Shellcheck correctly reports that we set python_version and never use it. This is a leftover from commit f9332757898a7: we used to use python_version purely to as part of the summary information printed at the end of a configure run, and that commit changed to printing the information from meson (which looks up the python version itself). Remove the unused variable. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20220825150703.4074125-2-peter.maydell@linaro.org --- configure | 3 --- 1 file changed, 3 deletions(-) diff --git a/configure b/configure index XXXXXXX..XXXXXXX 100755 --- a/configure +++ b/configure @@ -XXX,XX +XXX,XX @@ if ! $python -c 'import sys; sys.exit(sys.version_info < (3,6))'; then "Use --python=/path/to/python to specify a supported Python." fi -# Preserve python version since some functionality is dependent on it -python_version=$($python -c 'import sys; print("%d.%d.%d" % (sys.version_info[0], sys.version_info[1], sys.version_info[2]))' 2>/dev/null) - # Suppress writing compiled files python="$python -B" -- 2.25.1 The meson_args variable was added in commit 3b4da13293482134b, but was not used in that commit and isn't used today. Delete the unnecessary assignment. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20220825150703.4074125-3-peter.maydell@linaro.org --- configure | 1 - 1 file changed, 1 deletion(-) diff --git a/configure b/configure index XXXXXXX..XXXXXXX 100755 --- a/configure +++ b/configure @@ -XXX,XX +XXX,XX @@ pie="" coroutine="" plugins="$default_feature" meson="" -meson_args="" ninja="" bindir="bin" skip_meson=no -- 2.25.1 This commit adds quotes in some places which: * are spotted by shellcheck * are obviously incorrect * are easy to fix just by adding the quotes It doesn't attempt fix all of the places shellcheck finds errors, or even all the ones which are easy to fix. It's just a random sampling which is hopefully easy to review and which cuts down the size of the problem for next time somebody wants to try to look at shellcheck errors. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20220825150703.4074125-4-peter.maydell@linaro.org --- configure | 64 +++++++++++++++++++++++++++---------------------------- 1 file changed, 32 insertions(+), 32 deletions(-) diff --git a/configure b/configure index XXXXXXX..XXXXXXX 100755 --- a/configure +++ b/configure @@ -XXX,XX +XXX,XX @@ GNUmakefile: ; EOF cd build - exec $source_path/configure "$@" + exec "$source_path/configure" "$@" fi # Temporary directory used for files created while @@ -XXX,XX +XXX,XX @@ meson_option_build_array() { printf ']\n' } -. $source_path/scripts/meson-buildoptions.sh +. "$source_path/scripts/meson-buildoptions.sh" meson_options= meson_option_add() { @@ -XXX,XX +XXX,XX @@ for opt do case "$opt" in --help|-h) show_help=yes ;; - --version|-V) exec cat $source_path/VERSION + --version|-V) exec cat "$source_path/VERSION" ;; --prefix=*) prefix="$optarg" ;; @@ -XXX,XX +XXX,XX @@ default_target_list="" mak_wilds="" if [ "$linux_user" != no ]; then - if [ "$targetos" = linux ] && [ -d $source_path/linux-user/include/host/$cpu ]; then + if [ "$targetos" = linux ] && [ -d "$source_path/linux-user/include/host/$cpu" ]; then linux_user=yes elif [ "$linux_user" = yes ]; then error_exit "linux-user not supported on this architecture" @@ -XXX,XX +XXX,XX @@ if [ "$bsd_user" != no ]; then if [ "$bsd_user" = "" ]; then test $targetos = freebsd && bsd_user=yes fi - if [ "$bsd_user" = yes ] && ! [ -d $source_path/bsd-user/$targetos ]; then + if [ "$bsd_user" = yes ] && ! [ -d "$source_path/bsd-user/$targetos" ]; then error_exit "bsd-user not supported on this host OS" fi fi @@ -XXX,XX +XXX,XX @@ python="$python -B" if test -z "$meson"; then if test "$explicit_python" = no && has meson && version_ge "$(meson --version)" 0.59.3; then meson=meson - elif test $git_submodules_action != 'ignore' ; then + elif test "$git_submodules_action" != 'ignore' ; then meson=git elif test -e "${source_path}/meson/meson.py" ; then meson=internal @@ -XXX,XX +XXX,XX @@ esac container="no" if test $use_containers = "yes"; then if has "docker" || has "podman"; then - container=$($python $source_path/tests/docker/docker.py probe) + container=$($python "$source_path"/tests/docker/docker.py probe) fi fi @@ -XXX,XX +XXX,XX @@ if test "$QEMU_GA_DISTRO" = ""; then QEMU_GA_DISTRO=Linux fi if test "$QEMU_GA_VERSION" = ""; then - QEMU_GA_VERSION=$(cat $source_path/VERSION) + QEMU_GA_VERSION=$(cat "$source_path"/VERSION) fi @@ -XXX,XX +XXX,XX @@ fi for target in $target_list; do target_dir="$target" target_name=$(echo $target | cut -d '-' -f 1)$EXESUF - mkdir -p $target_dir + mkdir -p "$target_dir" case $target in *-user) symlink "../qemu-$target_name" "$target_dir/qemu-$target_name" ;; *) symlink "../qemu-system-$target_name" "$target_dir/qemu-system-$target_name" ;; @@ -XXX,XX +XXX,XX @@ for target in $target_list; do config_target_mak=tests/tcg/config-$target.mak echo "# Automatically generated by configure - do not modify" > $config_target_mak - echo "TARGET_NAME=$arch" >> $config_target_mak + echo "TARGET_NAME=$arch" >> "$config_target_mak" case $target in xtensa*-linux-user) # the toolchain is not complete with headers, only build softmmu tests continue ;; *-softmmu) - test -f $source_path/tests/tcg/$arch/Makefile.softmmu-target || continue + test -f "$source_path/tests/tcg/$arch/Makefile.softmmu-target" || continue qemu="qemu-system-$arch" ;; *-linux-user|*-bsd-user) @@ -XXX,XX +XXX,XX @@ for target in $target_list; do # compilers is a requirememt for adding a new test that needs a # compiler feature. - echo "BUILD_STATIC=$build_static" >> $config_target_mak - write_target_makefile >> $config_target_mak + echo "BUILD_STATIC=$build_static" >> "$config_target_mak" + write_target_makefile >> "$config_target_mak" case $target in aarch64-*) if do_compiler "$target_cc" $target_cflags \ -march=armv8.1-a+sve -o $TMPE $TMPC; then - echo "CROSS_CC_HAS_SVE=y" >> $config_target_mak + echo "CROSS_CC_HAS_SVE=y" >> "$config_target_mak" fi if do_compiler "$target_cc" $target_cflags \ -march=armv8.1-a+sve2 -o $TMPE $TMPC; then - echo "CROSS_CC_HAS_SVE2=y" >> $config_target_mak + echo "CROSS_CC_HAS_SVE2=y" >> "$config_target_mak" fi if do_compiler "$target_cc" $target_cflags \ -march=armv8.3-a -o $TMPE $TMPC; then - echo "CROSS_CC_HAS_ARMV8_3=y" >> $config_target_mak + echo "CROSS_CC_HAS_ARMV8_3=y" >> "$config_target_mak" fi if do_compiler "$target_cc" $target_cflags \ -mbranch-protection=standard -o $TMPE $TMPC; then - echo "CROSS_CC_HAS_ARMV8_BTI=y" >> $config_target_mak + echo "CROSS_CC_HAS_ARMV8_BTI=y" >> "$config_target_mak" fi if do_compiler "$target_cc" $target_cflags \ -march=armv8.5-a+memtag -o $TMPE $TMPC; then - echo "CROSS_CC_HAS_ARMV8_MTE=y" >> $config_target_mak + echo "CROSS_CC_HAS_ARMV8_MTE=y" >> "$config_target_mak" fi ;; ppc*) if do_compiler "$target_cc" $target_cflags \ -mpower8-vector -o $TMPE $TMPC; then - echo "CROSS_CC_HAS_POWER8_VECTOR=y" >> $config_target_mak + echo "CROSS_CC_HAS_POWER8_VECTOR=y" >> "$config_target_mak" fi if do_compiler "$target_cc" $target_cflags \ -mpower10 -o $TMPE $TMPC; then - echo "CROSS_CC_HAS_POWER10=y" >> $config_target_mak + echo "CROSS_CC_HAS_POWER10=y" >> "$config_target_mak" fi ;; i386-linux-user) if do_compiler "$target_cc" $target_cflags \ -Werror -fno-pie -o $TMPE $TMPC; then - echo "CROSS_CC_HAS_I386_NOPIE=y" >> $config_target_mak + echo "CROSS_CC_HAS_I386_NOPIE=y" >> "$config_target_mak" fi ;; esac elif test -n "$container_image"; then echo "build-tcg-tests-$target: docker-image-$container_image" >> $makefile - echo "BUILD_STATIC=y" >> $config_target_mak - write_container_target_makefile >> $config_target_mak + echo "BUILD_STATIC=y" >> "$config_target_mak" + write_container_target_makefile >> "$config_target_mak" case $target in aarch64-*) - echo "CROSS_CC_HAS_SVE=y" >> $config_target_mak - echo "CROSS_CC_HAS_SVE2=y" >> $config_target_mak - echo "CROSS_CC_HAS_ARMV8_3=y" >> $config_target_mak - echo "CROSS_CC_HAS_ARMV8_BTI=y" >> $config_target_mak - echo "CROSS_CC_HAS_ARMV8_MTE=y" >> $config_target_mak + echo "CROSS_CC_HAS_SVE=y" >> "$config_target_mak" + echo "CROSS_CC_HAS_SVE2=y" >> "$config_target_mak" + echo "CROSS_CC_HAS_ARMV8_3=y" >> "$config_target_mak" + echo "CROSS_CC_HAS_ARMV8_BTI=y" >> "$config_target_mak" + echo "CROSS_CC_HAS_ARMV8_MTE=y" >> "$config_target_mak" ;; ppc*) - echo "CROSS_CC_HAS_POWER8_VECTOR=y" >> $config_target_mak - echo "CROSS_CC_HAS_POWER10=y" >> $config_target_mak + echo "CROSS_CC_HAS_POWER8_VECTOR=y" >> "$config_target_mak" + echo "CROSS_CC_HAS_POWER10=y" >> "$config_target_mak" ;; i386-linux-user) - echo "CROSS_CC_HAS_I386_NOPIE=y" >> $config_target_mak + echo "CROSS_CC_HAS_I386_NOPIE=y" >> "$config_target_mak" ;; esac got_cross_cc=yes fi if test $got_cross_cc = yes; then mkdir -p tests/tcg/$target - echo "QEMU=$PWD/$qemu" >> $config_target_mak + echo "QEMU=$PWD/$qemu" >> "$config_target_mak" echo "run-tcg-tests-$target: $qemu\$(EXESUF)" >> $makefile tcg_tests_targets="$tcg_tests_targets $target" fi -- 2.25.1 Shellcheck warns that in rm -f */config-devices.mak.d the glob might expand to something with a '-' in it, which would then be misinterpreted as an option to rm. Fix this by adding './'. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20220825150703.4074125-5-peter.maydell@linaro.org --- configure | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure b/configure index XXXXXXX..XXXXXXX 100755 --- a/configure +++ b/configure @@ -XXX,XX +XXX,XX @@ exit 0 fi # Remove old dependency files to make sure that they get properly regenerated -rm -f */config-devices.mak.d +rm -f ./*/config-devices.mak.d if test -z "$python" then -- 2.25.1 There's only one place in configure where we use `...` to execute a command and capture the result. Switch to $() to match the rest of the script. This silences a shellcheck warning. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20220825150703.4074125-6-peter.maydell@linaro.org --- configure | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure b/configure index XXXXXXX..XXXXXXX 100755 --- a/configure +++ b/configure @@ -XXX,XX +XXX,XX @@ LINKS="$LINKS python" LINKS="$LINKS contrib/plugins/Makefile " for f in $LINKS ; do if [ -e "$source_path/$f" ]; then - mkdir -p `dirname ./$f` + mkdir -p "$(dirname ./"$f")" symlink "$source_path/$f" "$f" fi done -- 2.25.1 Shellcheck warns that we have one place where we run a command and then check if it failed using $?; this is better written to simply check the command in the 'if' statement directly. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20220825150703.4074125-7-peter.maydell@linaro.org --- configure | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/configure b/configure index XXXXXXX..XXXXXXX 100755 --- a/configure +++ b/configure @@ -XXX,XX +XXX,XX @@ fi # it when configure exits.) TMPDIR1="config-temp" rm -rf "${TMPDIR1}" -mkdir -p "${TMPDIR1}" -if [ $? -ne 0 ]; then +if ! mkdir -p "${TMPDIR1}"; then echo "ERROR: failed to create temporary directory" exit 1 fi -- 2.25.1 We use the non-POSIX 'local' keyword in just two places in configure; rewrite to avoid it. In do_compiler(), just drop the 'local' keyword. The variable 'compiler' is only used elsewhere in the do_compiler_werror() function, which already uses the variable as a normal non-local one. In probe_target_compiler(), $try and $t are both local; make them normal variables and use a more obviously distinct variable name for $t. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20220825150703.4074125-8-peter.maydell@linaro.org --- configure | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/configure b/configure index XXXXXXX..XXXXXXX 100755 --- a/configure +++ b/configure @@ -XXX,XX +XXX,XX @@ error_exit() { do_compiler() { # Run the compiler, capturing its output to the log. First argument # is compiler binary to execute. - local compiler="$1" + compiler="$1" shift if test -n "$BASH_VERSION"; then eval ' echo >>config.log " @@ -XXX,XX +XXX,XX @@ probe_target_compiler() { : ${container_cross_strip:=${container_cross_prefix}strip} done - local t try try=cross case "$target_arch:$cpu" in aarch64_be:aarch64 | \ @@ -XXX,XX +XXX,XX @@ probe_target_compiler() { try='native cross' ;; esac eval "target_cflags=\${cross_cc_cflags_$target_arch}" - for t in $try; do - case $t in + for thistry in $try; do + case $thistry in native) target_cc=$cc target_ccas=$ccas -- 2.25.1