Patches for rc1: nothing major, just some minor bugfixes and code cleanups. -- PMM The following changes since commit f7e1914adad8885a5d4c70239ab90d901ed97e9f: Merge remote-tracking branch 'remotes/alistair/tags/pull-riscv-to-apply-20201109' into staging (2020-11-10 09:24:56 +0000) are available in the Git repository at: https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20201110 for you to fetch changes up to b6c56c8a9a4064ea783f352f43c5df6231a110fa: target/arm/translate-neon.c: Handle VTBL UNDEF case before VFP access check (2020-11-10 11:03:48 +0000) ---------------------------------------------------------------- target-arm queue: * hw/arm/Kconfig: ARM_V7M depends on PTIMER * Minor coding style fixes * docs: add some notes on the sbsa-ref machine * hw/arm/virt: Remove dependency on Cortex-A15 MPCore peripherals * target/arm: Fix neon VTBL/VTBX for len > 1 * hw/arm/armsse: Correct expansion MPC interrupt lines * hw/misc/stm32f2xx_syscfg: Remove extraneous IRQ * hw/arm/nseries: Remove invalid/unnecessary n8x0_uart_setup() * hw/arm/musicpal: Don't connect two qemu_irqs directly to the same input * hw/arm/musicpal: Only use qdev_get_gpio_in() when necessary * hw/arm/nseries: Check return value from load_image_targphys() * tests/qtest/npcm7xx_rng-test: count runs properly * target/arm/translate-neon.c: Handle VTBL UNDEF case before VFP access check ---------------------------------------------------------------- Alex Bennée (1): docs: add some notes on the sbsa-ref machine AlexChen (1): ssi: Fix bad printf format specifiers Andrew Jones (1): hw/arm/Kconfig: ARM_V7M depends on PTIMER Havard Skinnemoen (1): tests/qtest/npcm7xx_rng-test: count runs properly Peter Maydell (2): hw/arm/nseries: Check return value from load_image_targphys() target/arm/translate-neon.c: Handle VTBL UNDEF case before VFP access check Philippe Mathieu-Daudé (6): hw/arm/virt: Remove dependency on Cortex-A15 MPCore peripherals hw/arm/armsse: Correct expansion MPC interrupt lines hw/misc/stm32f2xx_syscfg: Remove extraneous IRQ hw/arm/nseries: Remove invalid/unnecessary n8x0_uart_setup() hw/arm/musicpal: Don't connect two qemu_irqs directly to the same input hw/arm/musicpal: Only use qdev_get_gpio_in() when necessary Richard Henderson (1): target/arm: Fix neon VTBL/VTBX for len > 1 Xinhao Zhang (3): target/arm: add spaces around operator target/arm: Don't use '#' flag of printf format target/arm: add space before the open parenthesis '(' docs/system/arm/sbsa.rst | 32 ++++++++++++++++++++++ docs/system/target-arm.rst | 1 + include/hw/misc/stm32f2xx_syscfg.h | 2 -- target/arm/helper.h | 2 +- hw/arm/armsse.c | 3 +- hw/arm/musicpal.c | 40 +++++++++++++++++---------- hw/arm/nseries.c | 26 ++++++++---------- hw/arm/stm32f205_soc.c | 1 - hw/misc/stm32f2xx_syscfg.c | 2 -- hw/ssi/imx_spi.c | 2 +- hw/ssi/xilinx_spi.c | 2 +- target/arm/arch_dump.c | 8 +++--- target/arm/arm-semi.c | 8 +++--- target/arm/helper.c | 2 +- target/arm/op_helper.c | 23 +++++++++------- target/arm/translate-a64.c | 4 +-- target/arm/translate.c | 2 +- tests/qtest/npcm7xx_rng-test.c | 2 +- hw/arm/Kconfig | 3 +- target/arm/translate-neon.c.inc | 56 ++++++++++++++------------------------ 20 files changed, 123 insertions(+), 98 deletions(-) create mode 100644 docs/system/arm/sbsa.rst From: Andrew Jones <drjones@redhat.com> commit 32bd322a0134 ("hw/timer/armv7m_systick: Rewrite to use ptimers") changed armv7m_systick to build on ptimers. Make sure we have ptimers in the build when building armv7m_systick. Signed-off-by: Andrew Jones <drjones@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Message-id: 20201104103343.30392-1-drjones@redhat.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/Kconfig +++ b/hw/arm/Kconfig @@ -XXX,XX +XXX,XX @@ config ZYNQ config ARM_V7M bool + select PTIMER config ALLWINNER_A10 bool -- 2.20.1 From: AlexChen <alex.chen@huawei.com> We should use printf format specifier "%u" instead of "%d" for argument of type "unsigned int". Reported-by: Euler Robot <euler.robot@huawei.com> Signed-off-by: Alex Chen <alex.chen@huawei.com> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Message-id: 5FA280F5.8060902@huawei.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/ssi/imx_spi.c | 2 +- hw/ssi/xilinx_spi.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/ssi/imx_spi.c b/hw/ssi/imx_spi.c index XXXXXXX..XXXXXXX 100644 --- a/hw/ssi/imx_spi.c +++ b/hw/ssi/imx_spi.c @@ -XXX,XX +XXX,XX @@ static const char *imx_spi_reg_name(uint32_t reg) case ECSPI_MSGDATA: return "ECSPI_MSGDATA"; default: - sprintf(unknown, "%d ?", reg); + sprintf(unknown, "%u ?", reg); return unknown; } } diff --git a/hw/ssi/xilinx_spi.c b/hw/ssi/xilinx_spi.c index XXXXXXX..XXXXXXX 100644 --- a/hw/ssi/xilinx_spi.c +++ b/hw/ssi/xilinx_spi.c @@ -XXX,XX +XXX,XX @@ static void xlx_spi_update_irq(XilinxSPI *s) irq chain unless things really changed. */ if (pending != s->irqline) { s->irqline = pending; - DB_PRINT("irq_change of state %d ISR:%x IER:%X\n", + DB_PRINT("irq_change of state %u ISR:%x IER:%X\n", pending, s->regs[R_IPISR], s->regs[R_IPIER]); qemu_set_irq(s->irq, pending); } -- 2.20.1 From: Xinhao Zhang <zhangxinhao1@huawei.com> Fix code style. Operator needs spaces both sides. Signed-off-by: Xinhao Zhang <zhangxinhao1@huawei.com> Signed-off-by: Kai Deng <dengkai1@huawei.com> Message-id: 20201103114529.638233-1-zhangxinhao1@huawei.com Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/arch_dump.c | 8 ++++---- target/arm/arm-semi.c | 8 ++++---- target/arm/helper.c | 2 +- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/target/arm/arch_dump.c b/target/arm/arch_dump.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/arch_dump.c +++ b/target/arm/arch_dump.c @@ -XXX,XX +XXX,XX @@ static int aarch64_write_elf64_prfpreg(WriteCoreDumpFunction f, for (i = 0; i < 32; ++i) { uint64_t *q = aa64_vfp_qreg(env, i); - note.vfp.vregs[2*i + 0] = cpu_to_dump64(s, q[0]); - note.vfp.vregs[2*i + 1] = cpu_to_dump64(s, q[1]); + note.vfp.vregs[2 * i + 0] = cpu_to_dump64(s, q[0]); + note.vfp.vregs[2 * i + 1] = cpu_to_dump64(s, q[1]); } if (s->dump_info.d_endian == ELFDATA2MSB) { @@ -XXX,XX +XXX,XX @@ static int aarch64_write_elf64_prfpreg(WriteCoreDumpFunction f, */ for (i = 0; i < 32; ++i) { uint64_t tmp = note.vfp.vregs[2*i]; - note.vfp.vregs[2*i] = note.vfp.vregs[2*i+1]; - note.vfp.vregs[2*i+1] = tmp; + note.vfp.vregs[2 * i] = note.vfp.vregs[2 * i + 1]; + note.vfp.vregs[2 * i + 1] = tmp; } } diff --git a/target/arm/arm-semi.c b/target/arm/arm-semi.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/arm-semi.c +++ b/target/arm/arm-semi.c @@ -XXX,XX +XXX,XX @@ target_ulong do_arm_semihosting(CPUARMState *env) if (use_gdb_syscalls()) { arm_semi_open_guestfd = guestfd; ret = arm_gdb_syscall(cpu, arm_semi_open_cb, "open,%s,%x,1a4", arg0, - (int)arg2+1, gdb_open_modeflags[arg1]); + (int)arg2 + 1, gdb_open_modeflags[arg1]); } else { ret = set_swi_errno(env, open(s, open_modeflags[arg1], 0644)); if (ret == (uint32_t)-1) { @@ -XXX,XX +XXX,XX @@ target_ulong do_arm_semihosting(CPUARMState *env) GET_ARG(1); if (use_gdb_syscalls()) { ret = arm_gdb_syscall(cpu, arm_semi_cb, "unlink,%s", - arg0, (int)arg1+1); + arg0, (int)arg1 + 1); } else { s = lock_user_string(arg0); if (!s) { @@ -XXX,XX +XXX,XX @@ target_ulong do_arm_semihosting(CPUARMState *env) GET_ARG(3); if (use_gdb_syscalls()) { return arm_gdb_syscall(cpu, arm_semi_cb, "rename,%s,%s", - arg0, (int)arg1+1, arg2, (int)arg3+1); + arg0, (int)arg1 + 1, arg2, (int)arg3 + 1); } else { char *s2; s = lock_user_string(arg0); @@ -XXX,XX +XXX,XX @@ target_ulong do_arm_semihosting(CPUARMState *env) GET_ARG(1); if (use_gdb_syscalls()) { return arm_gdb_syscall(cpu, arm_semi_cb, "system,%s", - arg0, (int)arg1+1); + arg0, (int)arg1 + 1); } else { s = lock_user_string(arg0); if (!s) { diff --git a/target/arm/helper.c b/target/arm/helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -XXX,XX +XXX,XX @@ uint32_t HELPER(usad8)(uint32_t a, uint32_t b) uint32_t sum; sum = do_usad(a, b); sum += do_usad(a >> 8, b >> 8); - sum += do_usad(a >> 16, b >>16); + sum += do_usad(a >> 16, b >> 16); sum += do_usad(a >> 24, b >> 24); return sum; } -- 2.20.1 From: Xinhao Zhang <zhangxinhao1@huawei.com> Fix code style. Don't use '#' flag of printf format ('%#') in format strings, use '0x' prefix instead Signed-off-by: Xinhao Zhang <zhangxinhao1@huawei.com> Signed-off-by: Kai Deng <dengkai1@huawei.com> Message-id: 20201103114529.638233-2-zhangxinhao1@huawei.com Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/translate-a64.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-a64.c +++ b/target/arm/translate-a64.c @@ -XXX,XX +XXX,XX @@ static void disas_simd_three_reg_same_fp16(DisasContext *s, uint32_t insn) gen_helper_advsimd_acgt_f16(tcg_res, tcg_op1, tcg_op2, fpst); break; default: - fprintf(stderr, "%s: insn %#04x, fpop %#2x @ %#" PRIx64 "\n", + fprintf(stderr, "%s: insn 0x%04x, fpop 0x%2x @ 0x%" PRIx64 "\n", __func__, insn, fpopcode, s->pc_curr); g_assert_not_reached(); } @@ -XXX,XX +XXX,XX @@ static void disas_simd_two_reg_misc_fp16(DisasContext *s, uint32_t insn) case 0x7f: /* FSQRT (vector) */ break; default: - fprintf(stderr, "%s: insn %#04x fpop %#2x\n", __func__, insn, fpop); + fprintf(stderr, "%s: insn 0x%04x fpop 0x%2x\n", __func__, insn, fpop); g_assert_not_reached(); } -- 2.20.1 From: Xinhao Zhang <zhangxinhao1@huawei.com> Fix code style. Space required before the open parenthesis '('. Signed-off-by: Xinhao Zhang <zhangxinhao1@huawei.com> Signed-off-by: Kai Deng <dengkai1@huawei.com> Message-id: 20201103114529.638233-3-zhangxinhao1@huawei.com Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/translate.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target/arm/translate.c b/target/arm/translate.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -XXX,XX +XXX,XX @@ static void arm_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu) - Hardware watchpoints. Hardware breakpoints have already been handled and skip this code. */ - switch(dc->base.is_jmp) { + switch (dc->base.is_jmp) { case DISAS_NEXT: case DISAS_TOO_MANY: gen_goto_tb(dc, 1, dc->base.pc_next); -- 2.20.1 From: Alex Bennée <alex.bennee@linaro.org> We should at least document what this machine is about. Reviewed-by: Graeme Gregory <graeme@nuviainc.com> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Message-id: 20201104165254.24822-1-alex.bennee@linaro.org Cc: Leif Lindholm <leif@nuviainc.com> Cc: Shashi Mallela <shashi.mallela@linaro.org> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> [PMM: fixed filename mismatch] Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- docs/system/arm/sbsa.rst | 32 ++++++++++++++++++++++++++++++++ docs/system/target-arm.rst | 1 + 2 files changed, 33 insertions(+) create mode 100644 docs/system/arm/sbsa.rst diff --git a/docs/system/arm/sbsa.rst b/docs/system/arm/sbsa.rst new file mode 100644 index XXXXXXX..XXXXXXX --- /dev/null +++ b/docs/system/arm/sbsa.rst @@ -XXX,XX +XXX,XX @@ +Arm Server Base System Architecture Reference board (``sbsa-ref``) +================================================================== + +While the `virt` board is a generic board platform that doesn't match +any real hardware the `sbsa-ref` board intends to look like real +hardware. The `Server Base System Architecture +<https://developer.arm.com/documentation/den0029/latest>` defines a +minimum base line of hardware support and importantly how the firmware +reports that to any operating system. It is a static system that +reports a very minimal DT to the firmware for non-discoverable +information about components affected by the qemu command line (i.e. +cpus and memory). As a result it must have a firmware specifically +built to expect a certain hardware layout (as you would in a real +machine). + +It is intended to be a machine for developing firmware and testing +standards compliance with operating systems. + +Supported devices +""""""""""""""""" + +The sbsa-ref board supports: + + - A configurable number of AArch64 CPUs + - GIC version 3 + - System bus AHCI controller + - System bus EHCI controller + - CDROM and hard disc on AHCI bus + - E1000E ethernet card on PCIe bus + - VGA display adaptor on PCIe bus + - A generic SBSA watchdog device + diff --git a/docs/system/target-arm.rst b/docs/system/target-arm.rst index XXXXXXX..XXXXXXX 100644 --- a/docs/system/target-arm.rst +++ b/docs/system/target-arm.rst @@ -XXX,XX +XXX,XX @@ undocumented; you can get a complete list by running arm/mps2 arm/musca arm/realview + arm/sbsa arm/versatile arm/vexpress arm/aspeed -- 2.20.1 From: Philippe Mathieu-Daudé <philmd@redhat.com> When using a Cortex-A15, the Virt machine does not use any MPCore peripherals. Remove the dependency. Fixes: 7951c7b7c05 ("hw/arm: Express dependencies of the virt machine with Kconfig") Reported-by: Miroslav Rezanina <mrezanin@redhat.com> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> Message-id: 20201107114852.271922-1-philmd@redhat.com Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/Kconfig | 1 - 1 file changed, 1 deletion(-) diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/Kconfig +++ b/hw/arm/Kconfig @@ -XXX,XX +XXX,XX @@ config ARM_VIRT imply VFIO_PLATFORM imply VFIO_XGMAC imply TPM_TIS_SYSBUS - select A15MPCORE select ACPI select ARM_SMMUV3 select GPIO_KEY -- 2.20.1 From: Richard Henderson <richard.henderson@linaro.org> The helper function did not get updated when we reorganized the vector register file for SVE. Since then, the neon dregs are non-sequential and cannot be simply indexed. At the same time, make the helper function operate on 64-bit quantities so that we do not have to call it twice. Fixes: c39c2b9043e Reported-by: Ard Biesheuvel <ardb@kernel.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> [PMM: use aa32_vfp_dreg() rather than opencoding] Message-id: 20201105171126.88014-1-richard.henderson@linaro.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/helper.h | 2 +- target/arm/op_helper.c | 23 +++++++++-------- target/arm/translate-neon.c.inc | 44 +++++++++++---------------------- 3 files changed, 29 insertions(+), 40 deletions(-) diff --git a/target/arm/helper.h b/target/arm/helper.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.h +++ b/target/arm/helper.h @@ -XXX,XX +XXX,XX @@ DEF_HELPER_FLAGS_2(rsqrte_f32, TCG_CALL_NO_RWG, f32, f32, ptr) DEF_HELPER_FLAGS_2(rsqrte_f64, TCG_CALL_NO_RWG, f64, f64, ptr) DEF_HELPER_FLAGS_1(recpe_u32, TCG_CALL_NO_RWG, i32, i32) DEF_HELPER_FLAGS_1(rsqrte_u32, TCG_CALL_NO_RWG, i32, i32) -DEF_HELPER_FLAGS_4(neon_tbl, TCG_CALL_NO_RWG, i32, i32, i32, ptr, i32) +DEF_HELPER_FLAGS_4(neon_tbl, TCG_CALL_NO_RWG, i64, env, i32, i64, i64) DEF_HELPER_3(shl_cc, i32, env, i32, i32) DEF_HELPER_3(shr_cc, i32, env, i32, i32) diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/op_helper.c +++ b/target/arm/op_helper.c @@ -XXX,XX +XXX,XX @@ void raise_exception_ra(CPUARMState *env, uint32_t excp, uint32_t syndrome, cpu_loop_exit_restore(cs, ra); } -uint32_t HELPER(neon_tbl)(uint32_t ireg, uint32_t def, void *vn, - uint32_t maxindex) +uint64_t HELPER(neon_tbl)(CPUARMState *env, uint32_t desc, + uint64_t ireg, uint64_t def) { - uint32_t val, shift; - uint64_t *table = vn; + uint64_t tmp, val = 0; + uint32_t maxindex = ((desc & 3) + 1) * 8; + uint32_t base_reg = desc >> 2; + uint32_t shift, index, reg; - val = 0; - for (shift = 0; shift < 32; shift += 8) { - uint32_t index = (ireg >> shift) & 0xff; + for (shift = 0; shift < 64; shift += 8) { + index = (ireg >> shift) & 0xff; if (index < maxindex) { - uint32_t tmp = (table[index >> 3] >> ((index & 7) << 3)) & 0xff; - val |= tmp << shift; + reg = base_reg + (index >> 3); + tmp = *aa32_vfp_dreg(env, reg); + tmp = ((tmp >> ((index & 7) << 3)) & 0xff) << shift; } else { - val |= def & (0xff << shift); + tmp = def & (0xffull << shift); } + val |= tmp; } return val; } diff --git a/target/arm/translate-neon.c.inc b/target/arm/translate-neon.c.inc index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-neon.c.inc +++ b/target/arm/translate-neon.c.inc @@ -XXX,XX +XXX,XX @@ static bool trans_VEXT(DisasContext *s, arg_VEXT *a) static bool trans_VTBL(DisasContext *s, arg_VTBL *a) { - int n; - TCGv_i32 tmp, tmp2, tmp3, tmp4; - TCGv_ptr ptr1; + TCGv_i64 val, def; + TCGv_i32 desc; if (!arm_dc_feature(s, ARM_FEATURE_NEON)) { return false; @@ -XXX,XX +XXX,XX @@ static bool trans_VTBL(DisasContext *s, arg_VTBL *a) return true; } - n = a->len + 1; - if ((a->vn + n) > 32) { + if ((a->vn + a->len + 1) > 32) { /* * This is UNPREDICTABLE; we choose to UNDEF to avoid the * helper function running off the end of the register file. */ return false; } - n <<= 3; - tmp = tcg_temp_new_i32(); - if (a->op) { - read_neon_element32(tmp, a->vd, 0, MO_32); - } else { - tcg_gen_movi_i32(tmp, 0); - } - tmp2 = tcg_temp_new_i32(); - read_neon_element32(tmp2, a->vm, 0, MO_32); - ptr1 = vfp_reg_ptr(true, a->vn); - tmp4 = tcg_const_i32(n); - gen_helper_neon_tbl(tmp2, tmp2, tmp, ptr1, tmp4); + desc = tcg_const_i32((a->vn << 2) | a->len); + def = tcg_temp_new_i64(); if (a->op) { - read_neon_element32(tmp, a->vd, 1, MO_32); + read_neon_element64(def, a->vd, 0, MO_64); } else { - tcg_gen_movi_i32(tmp, 0); + tcg_gen_movi_i64(def, 0); } - tmp3 = tcg_temp_new_i32(); - read_neon_element32(tmp3, a->vm, 1, MO_32); - gen_helper_neon_tbl(tmp3, tmp3, tmp, ptr1, tmp4); - tcg_temp_free_i32(tmp); - tcg_temp_free_i32(tmp4); - tcg_temp_free_ptr(ptr1); + val = tcg_temp_new_i64(); + read_neon_element64(val, a->vm, 0, MO_64); - write_neon_element32(tmp2, a->vd, 0, MO_32); - write_neon_element32(tmp3, a->vd, 1, MO_32); - tcg_temp_free_i32(tmp2); - tcg_temp_free_i32(tmp3); + gen_helper_neon_tbl(val, cpu_env, desc, val, def); + write_neon_element64(val, a->vd, 0, MO_64); + + tcg_temp_free_i64(def); + tcg_temp_free_i64(val); + tcg_temp_free_i32(desc); return true; } -- 2.20.1 From: Philippe Mathieu-Daudé <f4bug@amsat.org> We can use one MPC per SRAM bank, but we currently only wire the IRQ from the first expansion MPC to the IRQ splitter. Fix that. Fixes: bb75e16d5e6 ("hw/arm/iotkit: Wire up MPC interrupt lines") Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20201107193403.436146-2-f4bug@amsat.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/armsse.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hw/arm/armsse.c b/hw/arm/armsse.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/armsse.c +++ b/hw/arm/armsse.c @@ -XXX,XX +XXX,XX @@ static void armsse_realize(DeviceState *dev, Error **errp) qdev_get_gpio_in(dev_splitter, 0)); qdev_connect_gpio_out(dev_splitter, 0, qdev_get_gpio_in_named(dev_secctl, - "mpc_status", 0)); + "mpc_status", + i - IOTS_NUM_EXP_MPC)); } qdev_connect_gpio_out(dev_splitter, 1, -- 2.20.1 From: Philippe Mathieu-Daudé <f4bug@amsat.org> The system configuration controller (SYSCFG) doesn't have any output IRQ (and the INTC input #71 belongs to the UART6). Remove the invalid code. Fixes: db635521a02 ("stm32f205: Add the stm32f205 SoC") Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20201107193403.436146-3-f4bug@amsat.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/hw/misc/stm32f2xx_syscfg.h | 2 -- hw/arm/stm32f205_soc.c | 1 - hw/misc/stm32f2xx_syscfg.c | 2 -- 3 files changed, 5 deletions(-) diff --git a/include/hw/misc/stm32f2xx_syscfg.h b/include/hw/misc/stm32f2xx_syscfg.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/misc/stm32f2xx_syscfg.h +++ b/include/hw/misc/stm32f2xx_syscfg.h @@ -XXX,XX +XXX,XX @@ struct STM32F2XXSyscfgState { uint32_t syscfg_exticr3; uint32_t syscfg_exticr4; uint32_t syscfg_cmpcr; - - qemu_irq irq; }; #endif /* HW_STM32F2XX_SYSCFG_H */ diff --git a/hw/arm/stm32f205_soc.c b/hw/arm/stm32f205_soc.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/stm32f205_soc.c +++ b/hw/arm/stm32f205_soc.c @@ -XXX,XX +XXX,XX @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp) } busdev = SYS_BUS_DEVICE(dev); sysbus_mmio_map(busdev, 0, 0x40013800); - sysbus_connect_irq(busdev, 0, qdev_get_gpio_in(armv7m, 71)); /* Attach UART (uses USART registers) and USART controllers */ for (i = 0; i < STM_NUM_USARTS; i++) { diff --git a/hw/misc/stm32f2xx_syscfg.c b/hw/misc/stm32f2xx_syscfg.c index XXXXXXX..XXXXXXX 100644 --- a/hw/misc/stm32f2xx_syscfg.c +++ b/hw/misc/stm32f2xx_syscfg.c @@ -XXX,XX +XXX,XX @@ static void stm32f2xx_syscfg_init(Object *obj) { STM32F2XXSyscfgState *s = STM32F2XX_SYSCFG(obj); - sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->irq); - memory_region_init_io(&s->mmio, obj, &stm32f2xx_syscfg_ops, s, TYPE_STM32F2XX_SYSCFG, 0x400); sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->mmio); -- 2.20.1 From: Philippe Mathieu-Daudé <f4bug@amsat.org> omap2420_mpu_init() introduced in commit 827df9f3c5f ("Add basic OMAP2 chip support") takes care of creating the 3 UARTs. Then commit 58a26b477e9 ("Emulate a serial bluetooth HCI with H4+ extensions and attach to n8x0's UART") added n8x0_uart_setup() which create the UART and connects it to an IRQ output, overwritting the existing peripheral and its IRQ connection. This is incorrect. Fortunately we don't need to fix this, because commit 6da68df7f9b ("hw/arm/nseries: Replace the bluetooth chardev with a "null" chardev") removed the use of this peripheral. We can simply remove the code. Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20201107193403.436146-4-f4bug@amsat.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/nseries.c | 11 ----------- 1 file changed, 11 deletions(-) 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 void n8x0_cbus_setup(struct n800_s *s) cbus_attach(cbus, s->tahvo = tahvo_init(tahvo_irq, 1)); } -static void n8x0_uart_setup(struct n800_s *s) -{ - Chardev *radio = qemu_chr_new("bt-dummy-uart", "null", NULL); - /* - * Note: We used to connect N8X0_BT_RESET_GPIO and N8X0_BT_WKUP_GPIO - * here, but this code has been removed with the bluetooth backend. - */ - omap_uart_attach(s->mpu->uart[BT_UART], radio); -} - static void n8x0_usb_setup(struct n800_s *s) { SysBusDevice *dev; @@ -XXX,XX +XXX,XX @@ static void n8x0_init(MachineState *machine, n8x0_spi_setup(s); n8x0_dss_setup(s); n8x0_cbus_setup(s); - n8x0_uart_setup(s); if (machine_usb(machine)) { n8x0_usb_setup(s); } -- 2.20.1 From: Philippe Mathieu-Daudé <f4bug@amsat.org> The MusicPal board code connects both of the IRQ outputs of the UART to the same INTC qemu_irq. Connecting two qemu_irqs outputs directly to the same input is not valid as it produces subtly wrong behaviour (for instance if both the IRQ lines are high, and then one goes low, the INTC input will see this as a high-to-low transition even though the second IRQ line should still be holding it high). This kind of wiring needs an explicitly created OR gate; add one. Inspired-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20201107193403.436146-5-f4bug@amsat.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/musicpal.c | 17 +++++++++++++---- hw/arm/Kconfig | 1 + 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/hw/arm/musicpal.c b/hw/arm/musicpal.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/musicpal.c +++ b/hw/arm/musicpal.c @@ -XXX,XX +XXX,XX @@ #include "ui/console.h" #include "hw/i2c/i2c.h" #include "hw/irq.h" +#include "hw/or-irq.h" #include "hw/audio/wm8750.h" #include "sysemu/block-backend.h" #include "sysemu/runstate.h" @@ -XXX,XX +XXX,XX @@ #define MP_TIMER4_IRQ 7 #define MP_EHCI_IRQ 8 #define MP_ETH_IRQ 9 -#define MP_UART1_IRQ 11 -#define MP_UART2_IRQ 11 +#define MP_UART_SHARED_IRQ 11 #define MP_GPIO_IRQ 12 #define MP_RTC_IRQ 28 #define MP_AUDIO_IRQ 30 @@ -XXX,XX +XXX,XX @@ static void musicpal_init(MachineState *machine) ARMCPU *cpu; qemu_irq pic[32]; DeviceState *dev; + DeviceState *uart_orgate; DeviceState *i2c_dev; DeviceState *lcd_dev; DeviceState *key_dev; @@ -XXX,XX +XXX,XX @@ static void musicpal_init(MachineState *machine) pic[MP_TIMER2_IRQ], pic[MP_TIMER3_IRQ], pic[MP_TIMER4_IRQ], NULL); - serial_mm_init(address_space_mem, MP_UART1_BASE, 2, pic[MP_UART1_IRQ], + /* Logically OR both UART IRQs together */ + uart_orgate = DEVICE(object_new(TYPE_OR_IRQ)); + object_property_set_int(OBJECT(uart_orgate), "num-lines", 2, &error_fatal); + qdev_realize_and_unref(uart_orgate, NULL, &error_fatal); + qdev_connect_gpio_out(DEVICE(uart_orgate), 0, pic[MP_UART_SHARED_IRQ]); + + serial_mm_init(address_space_mem, MP_UART1_BASE, 2, + qdev_get_gpio_in(uart_orgate, 0), 1825000, serial_hd(0), DEVICE_NATIVE_ENDIAN); - serial_mm_init(address_space_mem, MP_UART2_BASE, 2, pic[MP_UART2_IRQ], + serial_mm_init(address_space_mem, MP_UART2_BASE, 2, + qdev_get_gpio_in(uart_orgate, 1), 1825000, serial_hd(1), DEVICE_NATIVE_ENDIAN); /* Register flash */ diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/Kconfig +++ b/hw/arm/Kconfig @@ -XXX,XX +XXX,XX @@ config MUSCA config MUSICPAL bool + select OR_IRQ select BITBANG_I2C select MARVELL_88W8618 select PTIMER -- 2.20.1 From: Philippe Mathieu-Daudé <f4bug@amsat.org> We don't need to fill the full pic[] array if we only use few of the interrupt lines. Directly call qdev_get_gpio_in() when necessary. Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20201107193403.436146-6-f4bug@amsat.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/musicpal.c | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/hw/arm/musicpal.c b/hw/arm/musicpal.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/musicpal.c +++ b/hw/arm/musicpal.c @@ -XXX,XX +XXX,XX @@ static struct arm_boot_info musicpal_binfo = { static void musicpal_init(MachineState *machine) { ARMCPU *cpu; - qemu_irq pic[32]; DeviceState *dev; + DeviceState *pic; DeviceState *uart_orgate; DeviceState *i2c_dev; DeviceState *lcd_dev; @@ -XXX,XX +XXX,XX @@ static void musicpal_init(MachineState *machine) &error_fatal); memory_region_add_subregion(address_space_mem, MP_SRAM_BASE, sram); - dev = sysbus_create_simple(TYPE_MV88W8618_PIC, MP_PIC_BASE, + pic = sysbus_create_simple(TYPE_MV88W8618_PIC, MP_PIC_BASE, qdev_get_gpio_in(DEVICE(cpu), ARM_CPU_IRQ)); - for (i = 0; i < 32; i++) { - pic[i] = qdev_get_gpio_in(dev, i); - } - sysbus_create_varargs(TYPE_MV88W8618_PIT, MP_PIT_BASE, pic[MP_TIMER1_IRQ], - pic[MP_TIMER2_IRQ], pic[MP_TIMER3_IRQ], - pic[MP_TIMER4_IRQ], NULL); + sysbus_create_varargs(TYPE_MV88W8618_PIT, MP_PIT_BASE, + qdev_get_gpio_in(pic, MP_TIMER1_IRQ), + qdev_get_gpio_in(pic, MP_TIMER2_IRQ), + qdev_get_gpio_in(pic, MP_TIMER3_IRQ), + qdev_get_gpio_in(pic, MP_TIMER4_IRQ), NULL); /* Logically OR both UART IRQs together */ uart_orgate = DEVICE(object_new(TYPE_OR_IRQ)); object_property_set_int(OBJECT(uart_orgate), "num-lines", 2, &error_fatal); qdev_realize_and_unref(uart_orgate, NULL, &error_fatal); - qdev_connect_gpio_out(DEVICE(uart_orgate), 0, pic[MP_UART_SHARED_IRQ]); + qdev_connect_gpio_out(DEVICE(uart_orgate), 0, + qdev_get_gpio_in(pic, MP_UART_SHARED_IRQ)); serial_mm_init(address_space_mem, MP_UART1_BASE, 2, qdev_get_gpio_in(uart_orgate, 0), @@ -XXX,XX +XXX,XX @@ static void musicpal_init(MachineState *machine) OBJECT(get_system_memory()), &error_fatal); sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal); sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, MP_ETH_BASE); - sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[MP_ETH_IRQ]); + sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, + qdev_get_gpio_in(pic, MP_ETH_IRQ)); sysbus_create_simple("mv88w8618_wlan", MP_WLAN_BASE, NULL); sysbus_create_simple(TYPE_MUSICPAL_MISC, MP_MISC_BASE, NULL); dev = sysbus_create_simple(TYPE_MUSICPAL_GPIO, MP_GPIO_BASE, - pic[MP_GPIO_IRQ]); + qdev_get_gpio_in(pic, MP_GPIO_IRQ)); i2c_dev = sysbus_create_simple("gpio_i2c", -1, NULL); i2c = (I2CBus *)qdev_get_child_bus(i2c_dev, "i2c"); @@ -XXX,XX +XXX,XX @@ static void musicpal_init(MachineState *machine) NULL); sysbus_realize_and_unref(s, &error_fatal); sysbus_mmio_map(s, 0, MP_AUDIO_BASE); - sysbus_connect_irq(s, 0, pic[MP_AUDIO_IRQ]); + sysbus_connect_irq(s, 0, qdev_get_gpio_in(pic, MP_AUDIO_IRQ)); musicpal_binfo.ram_size = MP_RAM_DEFAULT_SIZE; arm_load_kernel(cpu, machine, &musicpal_binfo); -- 2.20.1 The nseries machines have a codepath that allows them to load a secondary bootloader. This code wasn't checking that the load_image_targphys() succeeded. Check the return value and report the error to the user. While we're in the vicinity, fix the comment style of the comment documenting what this image load is doing. Fixes: Coverity CID 1192904 Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20201103114918.11807-1-peter.maydell@linaro.org --- hw/arm/nseries.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) 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 void n8x0_init(MachineState *machine, /* No, wait, better start at the ROM. */ s->mpu->cpu->env.regs[15] = OMAP2_Q2_BASE + 0x400000; - /* This is intended for loading the `secondary.bin' program from + /* + * This is intended for loading the `secondary.bin' program from * Nokia images (the NOLO bootloader). The entry point seems * to be at OMAP2_Q2_BASE + 0x400000. * @@ -XXX,XX +XXX,XX @@ static void n8x0_init(MachineState *machine, * for them the entry point needs to be set to OMAP2_SRAM_BASE. * * The code above is for loading the `zImage' file from Nokia - * images. */ - load_image_targphys(option_rom[0].name, OMAP2_Q2_BASE + 0x400000, - machine->ram_size - 0x400000); + * images. + */ + if (load_image_targphys(option_rom[0].name, + OMAP2_Q2_BASE + 0x400000, + machine->ram_size - 0x400000) < 0) { + error_report("Failed to load secondary bootloader %s", + option_rom[0].name); + exit(EXIT_FAILURE); + } n800_setup_nolo_tags(nolo_tags); cpu_physical_memory_write(OMAP2_SRAM_BASE, nolo_tags, 0x10000); -- 2.20.1 From: Havard Skinnemoen <hskinnemoen@google.com> The number of runs is equal to the number of 0-1 and 1-0 transitions, plus one. Currently, it's counting the number of times these transitions do _not_ happen, plus one. Source: https://nvlpubs.nist.gov/nistpubs/Legacy/SP/nistspecialpublication800-22r1a.pdf section 2.3.4 point (3). Signed-off-by: Havard Skinnemoen <hskinnemoen@google.com> Message-id: 20201103011457.2959989-2-hskinnemoen@google.com Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- tests/qtest/npcm7xx_rng-test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/qtest/npcm7xx_rng-test.c b/tests/qtest/npcm7xx_rng-test.c index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/npcm7xx_rng-test.c +++ b/tests/qtest/npcm7xx_rng-test.c @@ -XXX,XX +XXX,XX @@ static double calc_runs_p(const unsigned long *buf, unsigned int nr_bits) pi = (double)nr_ones / nr_bits; for (k = 0; k < nr_bits - 1; k++) { - vn_obs += !(test_bit(k, buf) ^ test_bit(k + 1, buf)); + vn_obs += (test_bit(k, buf) ^ test_bit(k + 1, buf)); } vn_obs += 1; -- 2.20.1 Checks for UNDEF cases should go before the "is VFP enabled?" access check, except in special cases. Move a stray UNDEF check in the VTBL trans function up above the access check. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20201109145324.2859-1-peter.maydell@linaro.org --- target/arm/translate-neon.c.inc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/target/arm/translate-neon.c.inc b/target/arm/translate-neon.c.inc index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-neon.c.inc +++ b/target/arm/translate-neon.c.inc @@ -XXX,XX +XXX,XX @@ static bool trans_VTBL(DisasContext *s, arg_VTBL *a) return false; } - if (!vfp_access_check(s)) { - return true; - } - if ((a->vn + a->len + 1) > 32) { /* * This is UNPREDICTABLE; we choose to UNDEF to avoid the @@ -XXX,XX +XXX,XX @@ static bool trans_VTBL(DisasContext *s, arg_VTBL *a) return false; } + if (!vfp_access_check(s)) { + return true; + } + desc = tcg_const_i32((a->vn << 2) | a->len); def = tcg_temp_new_i64(); if (a->op) { -- 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