arm pullreq for rc1. All minor bugfixes, except for the sve-default-vector-length patches, which are somewhere between a bugfix and a new feature. thanks -- PMM The following changes since commit c08ccd1b53f488ac86c1f65cf7623dc91acc249a: Merge remote-tracking branch 'remotes/rth-gitlab/tags/pull-tcg-20210726' into staging (2021-07-27 08:35:01 +0100) are available in the Git repository at: https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20210727 for you to fetch changes up to e229a179a503f2aee43a76888cf12fbdfe8a3749: hw: aspeed_gpio: Fix memory size (2021-07-27 11:00:00 +0100) ---------------------------------------------------------------- target-arm queue: * hw/arm/smmuv3: Check 31st bit to see if CD is valid * qemu-options.hx: Fix formatting of -machine memory-backend option * hw: aspeed_gpio: Fix memory size * hw/arm/nseries: Display hexadecimal value with '0x' prefix * Add sve-default-vector-length cpu property * docs: Update path that mentions deprecated.rst * hw/intc/armv7m_nvic: for v8.1M VECTPENDING hides S exceptions from NS * hw/intc/armv7m_nvic: Correct size of ICSR.VECTPENDING * hw/intc/armv7m_nvic: ISCR.ISRPENDING is set for non-enabled pending interrupts * target/arm: Report M-profile alignment faults correctly to the guest * target/arm: Add missing 'return's after calling v7m_exception_taken() * target/arm: Enforce that M-profile SP low 2 bits are always zero ---------------------------------------------------------------- Joe Komlodi (1): hw/arm/smmuv3: Check 31st bit to see if CD is valid Joel Stanley (1): hw: aspeed_gpio: Fix memory size Mao Zhongyi (1): docs: Update path that mentions deprecated.rst Peter Maydell (7): qemu-options.hx: Fix formatting of -machine memory-backend option target/arm: Enforce that M-profile SP low 2 bits are always zero target/arm: Add missing 'return's after calling v7m_exception_taken() target/arm: Report M-profile alignment faults correctly to the guest hw/intc/armv7m_nvic: ISCR.ISRPENDING is set for non-enabled pending interrupts hw/intc/armv7m_nvic: Correct size of ICSR.VECTPENDING hw/intc/armv7m_nvic: for v8.1M VECTPENDING hides S exceptions from NS Philippe Mathieu-Daudé (1): hw/arm/nseries: Display hexadecimal value with '0x' prefix Richard Henderson (3): target/arm: Correctly bound length in sve_zcr_get_valid_len target/arm: Export aarch64_sve_zcr_get_valid_len target/arm: Add sve-default-vector-length cpu property docs/system/arm/cpu-features.rst | 15 ++++++++++ configure | 2 +- hw/arm/smmuv3-internal.h | 2 +- target/arm/cpu.h | 5 ++++ target/arm/internals.h | 10 +++++++ hw/arm/nseries.c | 2 +- hw/gpio/aspeed_gpio.c | 3 +- hw/intc/armv7m_nvic.c | 40 +++++++++++++++++++-------- target/arm/cpu.c | 14 ++++++++-- target/arm/cpu64.c | 60 ++++++++++++++++++++++++++++++++++++++++ target/arm/gdbstub.c | 4 +++ target/arm/helper.c | 8 ++++-- target/arm/m_helper.c | 24 ++++++++++++---- target/arm/translate.c | 3 ++ target/i386/cpu.c | 2 +- MAINTAINERS | 2 +- qemu-options.hx | 30 +++++++++++--------- 17 files changed, 183 insertions(+), 43 deletions(-) From: Joe Komlodi <joe.komlodi@xilinx.com> The bit to see if a CD is valid is the last bit of the first word of the CD. Signed-off-by: Joe Komlodi <joe.komlodi@xilinx.com> Message-id: 1626728232-134665-2-git-send-email-joe.komlodi@xilinx.com Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/smmuv3-internal.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/arm/smmuv3-internal.h b/hw/arm/smmuv3-internal.h index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/smmuv3-internal.h +++ b/hw/arm/smmuv3-internal.h @@ -XXX,XX +XXX,XX @@ static inline int pa_range(STE *ste) /* CD fields */ -#define CD_VALID(x) extract32((x)->word[0], 30, 1) +#define CD_VALID(x) extract32((x)->word[0], 31, 1) #define CD_ASID(x) extract32((x)->word[1], 16, 16) #define CD_TTB(x, sel) \ ({ \ -- 2.20.1 The documentation of the -machine memory-backend has some minor formatting errors: * Misindentation of the initial line meant that the whole option section is incorrectly indented in the HTML output compared to the other -machine options * The examples weren't indented, which meant that they were formatted as plain run-on text including outputting the "::" as text. * The a) b) list has no rst-format markup so it is rendered as a single run-on paragraph Fix the formatting. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Igor Mammedov <imammedo@redhat.com> Message-id: 20210719105257.3599-1-peter.maydell@linaro.org --- qemu-options.hx | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/qemu-options.hx b/qemu-options.hx index XXXXXXX..XXXXXXX 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -XXX,XX +XXX,XX @@ SRST Enables or disables ACPI Heterogeneous Memory Attribute Table (HMAT) support. The default is off. - ``memory-backend='id'`` + ``memory-backend='id'`` An alternative to legacy ``-mem-path`` and ``mem-prealloc`` options. Allows to use a memory backend as main RAM. For example: :: - -object memory-backend-file,id=pc.ram,size=512M,mem-path=/hugetlbfs,prealloc=on,share=on - -machine memory-backend=pc.ram - -m 512M + + -object memory-backend-file,id=pc.ram,size=512M,mem-path=/hugetlbfs,prealloc=on,share=on + -machine memory-backend=pc.ram + -m 512M Migration compatibility note: - a) as backend id one shall use value of 'default-ram-id', advertised by - machine type (available via ``query-machines`` QMP command), if migration - to/from old QEMU (<5.0) is expected. - b) for machine types 4.0 and older, user shall - use ``x-use-canonical-path-for-ramblock-id=off`` backend option - if migration to/from old QEMU (<5.0) is expected. + + * as backend id one shall use value of 'default-ram-id', advertised by + machine type (available via ``query-machines`` QMP command), if migration + to/from old QEMU (<5.0) is expected. + * for machine types 4.0 and older, user shall + use ``x-use-canonical-path-for-ramblock-id=off`` backend option + if migration to/from old QEMU (<5.0) is expected. + For example: :: - -object memory-backend-ram,id=pc.ram,size=512M,x-use-canonical-path-for-ramblock-id=off - -machine memory-backend=pc.ram - -m 512M + + -object memory-backend-ram,id=pc.ram,size=512M,x-use-canonical-path-for-ramblock-id=off + -machine memory-backend=pc.ram + -m 512M ERST HXCOMM Deprecated by -machine -- 2.20.1 For M-profile, unlike A-profile, the low 2 bits of SP are defined to be RES0H, which is to say that they must be hardwired to zero so that guest attempts to write non-zero values to them are ignored. Implement this behaviour by masking out the low bits: * for writes to r13 by the gdbstub * for writes to any of the various flavours of SP via MSR * for writes to r13 via store_reg() in generated code Note that all the direct uses of cpu_R[] in translate.c are in places where the register is definitely not r13 (usually because that has been checked for as an UNDEFINED or UNPREDICTABLE case and handled as UNDEF). All the other writes to regs[13] in C code are either: * A-profile only code * writes of values we can guarantee to be aligned, such as - writes of previous-SP-value plus or minus a 4-aligned constant - writes of the value in an SP limit register (which we already enforce to be aligned) Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20210723162146.5167-2-peter.maydell@linaro.org --- target/arm/gdbstub.c | 4 ++++ target/arm/m_helper.c | 14 ++++++++------ target/arm/translate.c | 3 +++ 3 files changed, 15 insertions(+), 6 deletions(-) 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 @@ int arm_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n) if (n < 16) { /* Core integer register. */ + if (n == 13 && arm_feature(env, ARM_FEATURE_M)) { + /* M profile SP low bits are always 0 */ + tmp &= ~3; + } env->regs[n] = tmp; return 4; } 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 @@ void HELPER(v7m_msr)(CPUARMState *env, uint32_t maskreg, uint32_t val) if (!env->v7m.secure) { return; } - env->v7m.other_ss_msp = val; + env->v7m.other_ss_msp = val & ~3; return; case 0x89: /* PSP_NS */ if (!env->v7m.secure) { return; } - env->v7m.other_ss_psp = val; + env->v7m.other_ss_psp = val & ~3; return; case 0x8a: /* MSPLIM_NS */ if (!env->v7m.secure) { @@ -XXX,XX +XXX,XX @@ void HELPER(v7m_msr)(CPUARMState *env, uint32_t maskreg, uint32_t val) limit = is_psp ? env->v7m.psplim[false] : env->v7m.msplim[false]; + val &= ~0x3; + if (val < limit) { raise_exception_ra(env, EXCP_STKOF, 0, 1, GETPC()); } @@ -XXX,XX +XXX,XX @@ void HELPER(v7m_msr)(CPUARMState *env, uint32_t maskreg, uint32_t val) break; case 8: /* MSP */ if (v7m_using_psp(env)) { - env->v7m.other_sp = val; + env->v7m.other_sp = val & ~3; } else { - env->regs[13] = val; + env->regs[13] = val & ~3; } break; case 9: /* PSP */ if (v7m_using_psp(env)) { - env->regs[13] = val; + env->regs[13] = val & ~3; } else { - env->v7m.other_sp = val; + env->v7m.other_sp = val & ~3; } break; case 10: /* MSPLIM */ 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 @@ void store_reg(DisasContext *s, int reg, TCGv_i32 var) */ tcg_gen_andi_i32(var, var, s->thumb ? ~1 : ~3); s->base.is_jmp = DISAS_JUMP; + } else if (reg == 13 && arm_dc_feature(s, ARM_FEATURE_M)) { + /* For M-profile SP bits [1:0] are always zero */ + tcg_gen_andi_i32(var, var, ~3); } tcg_gen_mov_i32(cpu_R[reg], var); tcg_temp_free_i32(var); -- 2.20.1 In do_v7m_exception_exit(), we perform various checks as part of performing the exception return. If one of these checks fails, the architecture requires that we take an appropriate exception on the existing stackframe. We implement this by calling v7m_exception_taken() to set up to take the new exception, and then immediately returning from do_v7m_exception_exit() without proceeding any further with the unstack-and-exception-return process. In a couple of checks that are new in v8.1M, we forgot the "return" statement, with the effect that if bad code in the guest tripped over these checks we would set up to take a UsageFault exception but then blunder on trying to also unstack and return from the original exception, with the probable result that the guest would crash. Add the missing return statements. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20210723162146.5167-3-peter.maydell@linaro.org --- target/arm/m_helper.c | 2 ++ 1 file changed, 2 insertions(+) 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 void do_v7m_exception_exit(ARMCPU *cpu) qemu_log_mask(CPU_LOG_INT, "...taking UsageFault on existing " "stackframe: NSACR prevents clearing FPU registers\n"); v7m_exception_taken(cpu, excret, true, false); + return; } else if (!cpacr_pass) { armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_USAGE, exc_secure); @@ -XXX,XX +XXX,XX @@ static void do_v7m_exception_exit(ARMCPU *cpu) qemu_log_mask(CPU_LOG_INT, "...taking UsageFault on existing " "stackframe: CPACR prevents clearing FPU registers\n"); v7m_exception_taken(cpu, excret, true, false); + return; } } /* Clear s0..s15, FPSCR and VPR */ -- 2.20.1 For M-profile, we weren't reporting alignment faults triggered by the generic TCG code correctly to the guest. These get passed into arm_v7m_cpu_do_interrupt() as an EXCP_DATA_ABORT with an A-profile style exception.fsr value of 1. We didn't check for this, and so they fell through into the default of "assume this is an MPU fault" and were reported to the guest as a data access violation MPU fault. Report these alignment faults as UsageFaults which set the UNALIGNED bit in the UFSR. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20210723162146.5167-4-peter.maydell@linaro.org --- target/arm/m_helper.c | 8 ++++++++ 1 file changed, 8 insertions(+) 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 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs) env->v7m.sfsr |= R_V7M_SFSR_LSERR_MASK; break; case EXCP_UNALIGNED: + /* Unaligned faults reported by M-profile aware code */ armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_USAGE, env->v7m.secure); env->v7m.cfsr[env->v7m.secure] |= R_V7M_CFSR_UNALIGNED_MASK; break; @@ -XXX,XX +XXX,XX @@ void arm_v7m_cpu_do_interrupt(CPUState *cs) } armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_BUS, false); break; + case 0x1: /* Alignment fault reported by generic code */ + qemu_log_mask(CPU_LOG_INT, + "...really UsageFault with UFSR.UNALIGNED\n"); + env->v7m.cfsr[env->v7m.secure] |= R_V7M_CFSR_UNALIGNED_MASK; + armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_USAGE, + env->v7m.secure); + break; default: /* * All other FSR values are either MPU faults or "can't happen -- 2.20.1 The ISCR.ISRPENDING bit is set when an external interrupt is pending. This is true whether that external interrupt is enabled or not. This means that we can't use 's->vectpending == 0' as a shortcut to "ISRPENDING is zero", because s->vectpending indicates only the highest priority pending enabled interrupt. Remove the incorrect optimization so that if there is no pending enabled interrupt we fall through to scanning through the whole interrupt array. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20210723162146.5167-5-peter.maydell@linaro.org --- hw/intc/armv7m_nvic.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index XXXXXXX..XXXXXXX 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -XXX,XX +XXX,XX @@ static bool nvic_isrpending(NVICState *s) { int irq; - /* We can shortcut if the highest priority pending interrupt - * happens to be external or if there is nothing pending. + /* + * We can shortcut if the highest priority pending interrupt + * happens to be external; if not we need to check the whole + * vectors[] array. */ if (s->vectpending > NVIC_FIRST_IRQ) { return true; } - if (s->vectpending == 0) { - return false; - } for (irq = NVIC_FIRST_IRQ; irq < s->num_irq; irq++) { if (s->vectors[irq].pending) { -- 2.20.1 The VECTPENDING field in the ICSR is 9 bits wide, in bits [20:12] of the register. We were incorrectly masking it to 8 bits, so it would report the wrong value if the pending exception was greater than 256. Fix the bug. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20210723162146.5167-6-peter.maydell@linaro.org --- hw/intc/armv7m_nvic.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index XXXXXXX..XXXXXXX 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -XXX,XX +XXX,XX @@ static uint32_t nvic_readl(NVICState *s, uint32_t offset, MemTxAttrs attrs) /* VECTACTIVE */ val = cpu->env.v7m.exception; /* VECTPENDING */ - val |= (s->vectpending & 0xff) << 12; + val |= (s->vectpending & 0x1ff) << 12; /* ISRPENDING - set if any external IRQ is pending */ if (nvic_isrpending(s)) { val |= (1 << 22); -- 2.20.1 In Arm v8.1M the VECTPENDING field in the ICSR has new behaviour: if the register is accessed NonSecure and the highest priority pending enabled exception (that would be returned in the VECTPENDING field) targets Secure, then the VECTPENDING field must read 1 rather than the exception number of the pending exception. Implement this. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20210723162146.5167-7-peter.maydell@linaro.org --- hw/intc/armv7m_nvic.c | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index XXXXXXX..XXXXXXX 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -XXX,XX +XXX,XX @@ void armv7m_nvic_acknowledge_irq(void *opaque) nvic_irq_update(s); } +static bool vectpending_targets_secure(NVICState *s) +{ + /* Return true if s->vectpending targets Secure state */ + if (s->vectpending_is_s_banked) { + return true; + } + return !exc_is_banked(s->vectpending) && + exc_targets_secure(s, s->vectpending); +} + void armv7m_nvic_get_pending_irq_info(void *opaque, int *pirq, bool *ptargets_secure) { @@ -XXX,XX +XXX,XX @@ void armv7m_nvic_get_pending_irq_info(void *opaque, assert(pending > ARMV7M_EXCP_RESET && pending < s->num_irq); - if (s->vectpending_is_s_banked) { - targets_secure = true; - } else { - targets_secure = !exc_is_banked(pending) && - exc_targets_secure(s, pending); - } + targets_secure = vectpending_targets_secure(s); trace_nvic_get_pending_irq_info(pending, targets_secure); @@ -XXX,XX +XXX,XX @@ static uint32_t nvic_readl(NVICState *s, uint32_t offset, MemTxAttrs attrs) /* VECTACTIVE */ val = cpu->env.v7m.exception; /* VECTPENDING */ - val |= (s->vectpending & 0x1ff) << 12; + if (s->vectpending) { + /* + * From v8.1M VECTPENDING must read as 1 if accessed as + * NonSecure and the highest priority pending and enabled + * exception targets Secure. + */ + int vp = s->vectpending; + if (!attrs.secure && arm_feature(&cpu->env, ARM_FEATURE_V8_1M) && + vectpending_targets_secure(s)) { + vp = 1; + } + val |= (vp & 0x1ff) << 12; + } /* ISRPENDING - set if any external IRQ is pending */ if (nvic_isrpending(s)) { val |= (1 << 22); -- 2.20.1 From: Mao Zhongyi <maozhongyi@cmss.chinamobile.com> Missed in commit f3478392 "docs: Move deprecation, build and license info out of system/" Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 20210723065828.1336760-1-maozhongyi@cmss.chinamobile.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- configure | 2 +- target/i386/cpu.c | 2 +- MAINTAINERS | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/configure b/configure index XXXXXXX..XXXXXXX 100755 --- a/configure +++ b/configure @@ -XXX,XX +XXX,XX @@ fi if test -n "${deprecated_features}"; then echo "Warning, deprecated features enabled." - echo "Please see docs/system/deprecated.rst" + echo "Please see docs/about/deprecated.rst" echo " features: ${deprecated_features}" fi diff --git a/target/i386/cpu.c b/target/i386/cpu.c index XXXXXXX..XXXXXXX 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -XXX,XX +XXX,XX @@ static const X86CPUDefinition builtin_x86_defs[] = { * none", but this is just for compatibility while libvirt isn't * adapted to resolve CPU model versions before creating VMs. * See "Runnability guarantee of CPU models" at - * docs/system/deprecated.rst. + * docs/about/deprecated.rst. */ X86CPUVersion default_cpu_version = 1; diff --git a/MAINTAINERS b/MAINTAINERS index XXXXXXX..XXXXXXX 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -XXX,XX +XXX,XX @@ F: contrib/gitdm/* Incompatible changes R: libvir-list@redhat.com -F: docs/system/deprecated.rst +F: docs/about/deprecated.rst Build System ------------ -- 2.20.1 From: Richard Henderson <richard.henderson@linaro.org> Currently, our only caller is sve_zcr_len_for_el, which has already masked the length extracted from ZCR_ELx, so the masking done here is a nop. But we will shortly have uses from other locations, where the length will be unmasked. Saturate the length to ARM_MAX_VQ instead of truncating to the low 4 bits. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 20210723203344.968563-2-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/helper.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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 uint32_t sve_zcr_get_valid_len(ARMCPU *cpu, uint32_t start_len) { uint32_t end_len; - end_len = start_len &= 0xf; + start_len = MIN(start_len, ARM_MAX_VQ - 1); + end_len = start_len; + if (!test_bit(start_len, cpu->sve_vq_map)) { end_len = find_last_bit(cpu->sve_vq_map, start_len); assert(end_len < start_len); -- 2.20.1 From: Richard Henderson <richard.henderson@linaro.org> Rename from sve_zcr_get_valid_len and make accessible from outside of helper.c. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 20210723203344.968563-3-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/internals.h | 10 ++++++++++ target/arm/helper.c | 4 ++-- 2 files changed, 12 insertions(+), 2 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 arm_translate_init(void); void arm_cpu_synchronize_from_tb(CPUState *cs, const TranslationBlock *tb); #endif /* CONFIG_TCG */ +/** + * aarch64_sve_zcr_get_valid_len: + * @cpu: cpu context + * @start_len: maximum len to consider + * + * Return the maximum supported sve vector length <= @start_len. + * Note that both @start_len and the return value are in units + * of ZCR_ELx.LEN, so the vector bit length is (x + 1) * 128. + */ +uint32_t aarch64_sve_zcr_get_valid_len(ARMCPU *cpu, uint32_t start_len); enum arm_fprounding { FPROUNDING_TIEEVEN, 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 @@ int sve_exception_el(CPUARMState *env, int el) return 0; } -static uint32_t sve_zcr_get_valid_len(ARMCPU *cpu, uint32_t start_len) +uint32_t aarch64_sve_zcr_get_valid_len(ARMCPU *cpu, uint32_t start_len) { uint32_t end_len; @@ -XXX,XX +XXX,XX @@ uint32_t sve_zcr_len_for_el(CPUARMState *env, int el) zcr_len = MIN(zcr_len, 0xf & (uint32_t)env->vfp.zcr_el[3]); } - return sve_zcr_get_valid_len(cpu, zcr_len); + return aarch64_sve_zcr_get_valid_len(cpu, zcr_len); } static void zcr_write(CPUARMState *env, const ARMCPRegInfo *ri, -- 2.20.1 From: Richard Henderson <richard.henderson@linaro.org> Mirror the behavour of /proc/sys/abi/sve_default_vector_length under the real linux kernel. We have no way of passing along a real default across exec like the kernel can, but this is a decent way of adjusting the startup vector length of a process. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/482 Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 20210723203344.968563-4-richard.henderson@linaro.org [PMM: tweaked docs formatting, document -1 special-case, added fixup patch from RTH mentioning QEMU's maximum veclen.] Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- docs/system/arm/cpu-features.rst | 15 ++++++++ target/arm/cpu.h | 5 +++ target/arm/cpu.c | 14 ++++++-- target/arm/cpu64.c | 60 ++++++++++++++++++++++++++++++++ 4 files changed, 92 insertions(+), 2 deletions(-) diff --git a/docs/system/arm/cpu-features.rst b/docs/system/arm/cpu-features.rst index XXXXXXX..XXXXXXX 100644 --- a/docs/system/arm/cpu-features.rst +++ b/docs/system/arm/cpu-features.rst @@ -XXX,XX +XXX,XX @@ verbose command lines. However, the recommended way to select vector lengths is to explicitly enable each desired length. Therefore only example's (1), (4), and (6) exhibit recommended uses of the properties. +SVE User-mode Default Vector Length Property +-------------------------------------------- + +For qemu-aarch64, the cpu property ``sve-default-vector-length=N`` is +defined to mirror the Linux kernel parameter file +``/proc/sys/abi/sve_default_vector_length``. The default length, ``N``, +is in units of bytes and must be between 16 and 8192. +If not specified, the default vector length is 64. + +If the default length is larger than the maximum vector length enabled, +the actual vector length will be reduced. Note that the maximum vector +length supported by QEMU is 256. + +If this property is set to ``-1`` then the default vector length +is set to the maximum possible length. 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 @@ struct ARMCPU { /* Used to set the maximum vector length the cpu will support. */ uint32_t sve_max_vq; +#ifdef CONFIG_USER_ONLY + /* Used to set the default vector length at process start. */ + uint32_t sve_default_vq; +#endif + /* * In sve_vq_map each set bit is a supported vector length of * (bit-number + 1) * 16 bytes, i.e. each bit number + 1 is the vector 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) env->cp15.cpacr_el1 = deposit64(env->cp15.cpacr_el1, 16, 2, 3); /* with reasonable vector length */ if (cpu_isar_feature(aa64_sve, cpu)) { - env->vfp.zcr_el[1] = MIN(cpu->sve_max_vq - 1, 3); + env->vfp.zcr_el[1] = + aarch64_sve_zcr_get_valid_len(cpu, cpu->sve_default_vq - 1); } /* * Enable TBI0 but not TBI1. @@ -XXX,XX +XXX,XX @@ static void arm_cpu_initfn(Object *obj) QLIST_INIT(&cpu->pre_el_change_hooks); QLIST_INIT(&cpu->el_change_hooks); -#ifndef CONFIG_USER_ONLY +#ifdef CONFIG_USER_ONLY +# ifdef TARGET_AARCH64 + /* + * The linux kernel defaults to 512-bit vectors, when sve is supported. + * See documentation for /proc/sys/abi/sve_default_vector_length, and + * our corresponding sve-default-vector-length cpu property. + */ + cpu->sve_default_vq = 4; +# endif +#else /* Our inbound IRQ and FIQ lines */ if (kvm_enabled()) { /* VIRQ and VFIQ are unused with KVM but we add them to maintain 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 void cpu_arm_set_sve(Object *obj, bool value, Error **errp) cpu->isar.id_aa64pfr0 = t; } +#ifdef CONFIG_USER_ONLY +/* Mirror linux /proc/sys/abi/sve_default_vector_length. */ +static void cpu_arm_set_sve_default_vec_len(Object *obj, Visitor *v, + const char *name, void *opaque, + Error **errp) +{ + ARMCPU *cpu = ARM_CPU(obj); + int32_t default_len, default_vq, remainder; + + if (!visit_type_int32(v, name, &default_len, errp)) { + return; + } + + /* Undocumented, but the kernel allows -1 to indicate "maximum". */ + if (default_len == -1) { + cpu->sve_default_vq = ARM_MAX_VQ; + return; + } + + default_vq = default_len / 16; + remainder = default_len % 16; + + /* + * Note that the 512 max comes from include/uapi/asm/sve_context.h + * and is the maximum architectural width of ZCR_ELx.LEN. + */ + if (remainder || default_vq < 1 || default_vq > 512) { + error_setg(errp, "cannot set sve-default-vector-length"); + if (remainder) { + error_append_hint(errp, "Vector length not a multiple of 16\n"); + } else if (default_vq < 1) { + error_append_hint(errp, "Vector length smaller than 16\n"); + } else { + error_append_hint(errp, "Vector length larger than %d\n", + 512 * 16); + } + return; + } + + cpu->sve_default_vq = default_vq; +} + +static void cpu_arm_get_sve_default_vec_len(Object *obj, Visitor *v, + const char *name, void *opaque, + Error **errp) +{ + ARMCPU *cpu = ARM_CPU(obj); + int32_t value = cpu->sve_default_vq * 16; + + visit_type_int32(v, name, &value, errp); +} +#endif + void aarch64_add_sve_properties(Object *obj) { uint32_t vq; @@ -XXX,XX +XXX,XX @@ void aarch64_add_sve_properties(Object *obj) object_property_add(obj, name, "bool", cpu_arm_get_sve_vq, cpu_arm_set_sve_vq, NULL, NULL); } + +#ifdef CONFIG_USER_ONLY + /* Mirror linux /proc/sys/abi/sve_default_vector_length. */ + object_property_add(obj, "sve-default-vector-length", "int32", + cpu_arm_get_sve_default_vec_len, + cpu_arm_set_sve_default_vec_len, NULL, NULL); +#endif } void arm_cpu_pauth_finalize(ARMCPU *cpu, Error **errp) -- 2.20.1 From: Philippe Mathieu-Daudé <f4bug@amsat.org> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20210726150953.1218690-1-f4bug@amsat.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/nseries.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/arm/nseries.c b/hw/arm/nseries.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/nseries.c +++ b/hw/arm/nseries.c @@ -XXX,XX +XXX,XX @@ static uint32_t mipid_txrx(void *opaque, uint32_t cmd, int len) default: bad_cmd: qemu_log_mask(LOG_GUEST_ERROR, - "%s: unknown command %02x\n", __func__, s->cmd); + "%s: unknown command 0x%02x\n", __func__, s->cmd); break; } -- 2.20.1 From: Joel Stanley <joel@jms.id.au> The macro used to calculate the maximum memory size of the MMIO region had a mistake, causing all GPIO models to create a mapping of 0x9D8. The intent was to have it be 0x9D8 - 0x800. This extra size doesn't matter on ast2400 and ast2500, which have a 4KB region set aside for the GPIO controller. On the ast2600 the 3.3V and 1.8V GPIO controllers are 2KB apart, so the regions would overlap. Worse was the 1.8V controller would map over the top of the following peripheral, which happens to be the RTC. The mmio region used by each device is a maximum of 2KB, so avoid the calculations and hard code this as the maximum. Fixes: 36d737ee82b2 ("hw/gpio: Add in AST2600 specific implementation") Signed-off-by: Joel Stanley <joel@jms.id.au> Reviewed-by: Rashmica Gupta <rashmica.g@gmail.com> Reviewed-by: Cédric Le Goater <clg@kaod.org> Message-id: 20210713065854.134634-2-joel@jms.id.au [PMM: fix autocorrect error in commit message] Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/gpio/aspeed_gpio.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/hw/gpio/aspeed_gpio.c b/hw/gpio/aspeed_gpio.c index XXXXXXX..XXXXXXX 100644 --- a/hw/gpio/aspeed_gpio.c +++ b/hw/gpio/aspeed_gpio.c @@ -XXX,XX +XXX,XX @@ #define GPIO_1_8V_MEM_SIZE 0x9D8 #define GPIO_1_8V_REG_ARRAY_SIZE ((GPIO_1_8V_MEM_SIZE - \ GPIO_1_8V_REG_OFFSET) >> 2) -#define GPIO_MAX_MEM_SIZE MAX(GPIO_3_6V_MEM_SIZE, GPIO_1_8V_MEM_SIZE) static int aspeed_evaluate_irq(GPIOSets *regs, int gpio_prev_high, int gpio) { @@ -XXX,XX +XXX,XX @@ static void aspeed_gpio_realize(DeviceState *dev, Error **errp) } memory_region_init_io(&s->iomem, OBJECT(s), &aspeed_gpio_ops, s, - TYPE_ASPEED_GPIO, GPIO_MAX_MEM_SIZE); + TYPE_ASPEED_GPIO, 0x800); sysbus_init_mmio(sbd, &s->iomem); } -- 2.20.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