target-arm queue. This has the "plumb txattrs through various bits of exec.c" patches, and a collection of bug fixes from various people. thanks -- PMM The following changes since commit a3ac12fba028df90f7b3dbec924995c126c41022: Merge remote-tracking branch 'remotes/ehabkost/tags/numa-next-pull-request' into staging (2018-05-31 11:12:36 +0100) are available in the Git repository at: git://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20180531 for you to fetch changes up to 49d1dca0520ea71bc21867fab6647f474fcf857b: KVM: GIC: Fix memory leak due to calling kvm_init_irq_routing twice (2018-05-31 14:52:53 +0100) ---------------------------------------------------------------- target-arm queue: * target/arm: Honour FPCR.FZ in FRECPX * MAINTAINERS: Add entries for newer MPS2 boards and devices * hw/intc/arm_gicv3: Fix APxR<n> register dispatching * arm_gicv3_kvm: fix bug in writing zero bits back to the in-kernel GIC state * tcg: Fix helper function vs host abi for float16 * arm: fix qemu crash on startup with -bios option * arm: fix malloc type mismatch * xlnx-zdma: Correct mem leaks and memset to zero on desc unaligned errors * Correct CPACR reset value for v7 cores * memory.h: Improve IOMMU related documentation * exec: Plumb transaction attributes through various functions in preparation for allowing IOMMUs to see them * vmstate.h: Provide VMSTATE_BOOL_SUB_ARRAY * ARM: ACPI: Fix use-after-free due to memory realloc * KVM: GIC: Fix memory leak due to calling kvm_init_irq_routing twice ---------------------------------------------------------------- Francisco Iglesias (1): xlnx-zdma: Correct mem leaks and memset to zero on desc unaligned errors Igor Mammedov (1): arm: fix qemu crash on startup with -bios option Jan Kiszka (1): hw/intc/arm_gicv3: Fix APxR<n> register dispatching Paolo Bonzini (1): arm: fix malloc type mismatch Peter Maydell (17): target/arm: Honour FPCR.FZ in FRECPX MAINTAINERS: Add entries for newer MPS2 boards and devices Correct CPACR reset value for v7 cores memory.h: Improve IOMMU related documentation Make tb_invalidate_phys_addr() take a MemTxAttrs argument Make address_space_translate{, _cached}() take a MemTxAttrs argument Make address_space_map() take a MemTxAttrs argument Make address_space_access_valid() take a MemTxAttrs argument Make flatview_extend_translation() take a MemTxAttrs argument Make memory_region_access_valid() take a MemTxAttrs argument Make MemoryRegion valid.accepts callback take a MemTxAttrs argument Make flatview_access_valid() take a MemTxAttrs argument Make flatview_translate() take a MemTxAttrs argument Make address_space_get_iotlb_entry() take a MemTxAttrs argument Make flatview_do_translate() take a MemTxAttrs argument Make address_space_translate_iommu take a MemTxAttrs argument vmstate.h: Provide VMSTATE_BOOL_SUB_ARRAY Richard Henderson (1): tcg: Fix helper function vs host abi for float16 Shannon Zhao (3): arm_gicv3_kvm: increase clroffset accordingly ARM: ACPI: Fix use-after-free due to memory realloc KVM: GIC: Fix memory leak due to calling kvm_init_irq_routing twice include/exec/exec-all.h | 5 +- include/exec/helper-head.h | 2 +- include/exec/memory-internal.h | 3 +- include/exec/memory.h | 128 +++++++++++++++++++++++++++++++++++------ include/migration/vmstate.h | 3 + include/sysemu/dma.h | 6 +- accel/tcg/translate-all.c | 4 +- exec.c | 95 ++++++++++++++++++------------ hw/arm/boot.c | 18 +++--- hw/arm/virt-acpi-build.c | 20 +++++-- hw/dma/xlnx-zdma.c | 10 +++- hw/hppa/dino.c | 3 +- hw/intc/arm_gic_kvm.c | 1 - hw/intc/arm_gicv3_cpuif.c | 12 ++-- hw/intc/arm_gicv3_kvm.c | 2 +- hw/nvram/fw_cfg.c | 12 ++-- hw/s390x/s390-pci-inst.c | 3 +- hw/scsi/esp.c | 3 +- hw/vfio/common.c | 3 +- hw/virtio/vhost.c | 3 +- hw/xen/xen_pt_msi.c | 3 +- memory.c | 12 ++-- memory_ldst.inc.c | 18 +++--- target/arm/gdbstub.c | 3 +- target/arm/helper-a64.c | 41 +++++++------ target/arm/helper.c | 90 ++++++++++++++++------------- target/ppc/mmu-hash64.c | 3 +- target/riscv/helper.c | 2 +- target/s390x/diag.c | 6 +- target/s390x/excp_helper.c | 3 +- target/s390x/mmu_helper.c | 3 +- target/s390x/sigp.c | 3 +- target/xtensa/op_helper.c | 3 +- MAINTAINERS | 9 ++- 34 files changed, 353 insertions(+), 182 deletions(-) The FRECPX instructions should (like most other floating point operations) honour the FPCR.FZ bit which specifies whether input denormals should be flushed to zero (or FZ16 for the half-precision version). We forgot to implement this, which doesn't affect the results (since the calculation doesn't actually care about the mantissa bits) but did mean we were failing to set the FPSR.IDC bit. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20180521172712.19930-1-peter.maydell@linaro.org --- target/arm/helper-a64.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/target/arm/helper-a64.c b/target/arm/helper-a64.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper-a64.c +++ b/target/arm/helper-a64.c @@ -XXX,XX +XXX,XX @@ float16 HELPER(frecpx_f16)(float16 a, void *fpstp) return nan; } + a = float16_squash_input_denormal(a, fpst); + val16 = float16_val(a); sbit = 0x8000 & val16; exp = extract32(val16, 10, 5); @@ -XXX,XX +XXX,XX @@ float32 HELPER(frecpx_f32)(float32 a, void *fpstp) return nan; } + a = float32_squash_input_denormal(a, fpst); + val32 = float32_val(a); sbit = 0x80000000ULL & val32; exp = extract32(val32, 23, 8); @@ -XXX,XX +XXX,XX @@ float64 HELPER(frecpx_f64)(float64 a, void *fpstp) return nan; } + a = float64_squash_input_denormal(a, fpst); + val64 = float64_val(a); sbit = 0x8000000000000000ULL & val64; exp = extract64(float64_val(a), 52, 11); -- 2.17.1 Add entries to MAINTAINERS to cover the newer MPS2 boards and the new devices they use. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 20180518153157.14899-1-peter.maydell@linaro.org --- MAINTAINERS | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index XXXXXXX..XXXXXXX 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -XXX,XX +XXX,XX @@ F: hw/timer/cmsdk-apb-timer.c F: include/hw/timer/cmsdk-apb-timer.h F: hw/char/cmsdk-apb-uart.c F: include/hw/char/cmsdk-apb-uart.h +F: hw/misc/tz-ppc.c +F: include/hw/misc/tz-ppc.h ARM cores M: Peter Maydell <peter.maydell@linaro.org> @@ -XXX,XX +XXX,XX @@ M: Peter Maydell <peter.maydell@linaro.org> L: qemu-arm@nongnu.org S: Maintained F: hw/arm/mps2.c -F: hw/misc/mps2-scc.c -F: include/hw/misc/mps2-scc.h +F: hw/arm/mps2-tz.c +F: hw/misc/mps2-*.c +F: include/hw/misc/mps2-*.h +F: hw/arm/iotkit.c +F: include/hw/arm/iotkit.h Musicpal M: Jan Kiszka <jan.kiszka@web.de> -- 2.17.1 From: Jan Kiszka <jan.kiszka@siemens.com> There was a nasty flip in identifying which register group an access is targeting. The issue caused spuriously raised priorities of the guest when handing CPUs over in the Jailhouse hypervisor. Cc: qemu-stable@nongnu.org Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> Message-id: 28b927d3-da58-bce4-cc13-bfec7f9b1cb9@siemens.com Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/intc/arm_gicv3_cpuif.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c index XXXXXXX..XXXXXXX 100644 --- a/hw/intc/arm_gicv3_cpuif.c +++ b/hw/intc/arm_gicv3_cpuif.c @@ -XXX,XX +XXX,XX @@ static uint64_t icv_ap_read(CPUARMState *env, const ARMCPRegInfo *ri) { GICv3CPUState *cs = icc_cs_from_env(env); int regno = ri->opc2 & 3; - int grp = ri->crm & 1 ? GICV3_G0 : GICV3_G1NS; + int grp = (ri->crm & 1) ? GICV3_G1NS : GICV3_G0; uint64_t value = cs->ich_apr[grp][regno]; trace_gicv3_icv_ap_read(ri->crm & 1, regno, gicv3_redist_affid(cs), value); @@ -XXX,XX +XXX,XX @@ static void icv_ap_write(CPUARMState *env, const ARMCPRegInfo *ri, { GICv3CPUState *cs = icc_cs_from_env(env); int regno = ri->opc2 & 3; - int grp = ri->crm & 1 ? GICV3_G0 : GICV3_G1NS; + int grp = (ri->crm & 1) ? GICV3_G1NS : GICV3_G0; trace_gicv3_icv_ap_write(ri->crm & 1, regno, gicv3_redist_affid(cs), value); @@ -XXX,XX +XXX,XX @@ static uint64_t icc_ap_read(CPUARMState *env, const ARMCPRegInfo *ri) uint64_t value; int regno = ri->opc2 & 3; - int grp = ri->crm & 1 ? GICV3_G0 : GICV3_G1; + int grp = (ri->crm & 1) ? GICV3_G1 : GICV3_G0; if (icv_access(env, grp == GICV3_G0 ? HCR_FMO : HCR_IMO)) { return icv_ap_read(env, ri); @@ -XXX,XX +XXX,XX @@ static void icc_ap_write(CPUARMState *env, const ARMCPRegInfo *ri, GICv3CPUState *cs = icc_cs_from_env(env); int regno = ri->opc2 & 3; - int grp = ri->crm & 1 ? GICV3_G0 : GICV3_G1; + int grp = (ri->crm & 1) ? GICV3_G1 : GICV3_G0; if (icv_access(env, grp == GICV3_G0 ? HCR_FMO : HCR_IMO)) { icv_ap_write(env, ri, value); @@ -XXX,XX +XXX,XX @@ static uint64_t ich_ap_read(CPUARMState *env, const ARMCPRegInfo *ri) { GICv3CPUState *cs = icc_cs_from_env(env); int regno = ri->opc2 & 3; - int grp = ri->crm & 1 ? GICV3_G0 : GICV3_G1NS; + int grp = (ri->crm & 1) ? GICV3_G1NS : GICV3_G0; uint64_t value; value = cs->ich_apr[grp][regno]; @@ -XXX,XX +XXX,XX @@ static void ich_ap_write(CPUARMState *env, const ARMCPRegInfo *ri, { GICv3CPUState *cs = icc_cs_from_env(env); int regno = ri->opc2 & 3; - int grp = ri->crm & 1 ? GICV3_G0 : GICV3_G1NS; + int grp = (ri->crm & 1) ? GICV3_G1NS : GICV3_G0; trace_gicv3_ich_ap_write(ri->crm & 1, regno, gicv3_redist_affid(cs), value); -- 2.17.1 From: Shannon Zhao <zhaoshenglong@huawei.com> It forgot to increase clroffset during the loop. So it only clear the first 4 bytes. Fixes: 367b9f527becdd20ddf116e17a3c0c2bbc486920 Cc: qemu-stable@nongnu.org Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com> Reviewed-by: Eric Auger <eric.auger@redhat.com> Message-id: 1527047633-12368-1-git-send-email-zhaoshenglong@huawei.com Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/intc/arm_gicv3_kvm.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c index XXXXXXX..XXXXXXX 100644 --- a/hw/intc/arm_gicv3_kvm.c +++ b/hw/intc/arm_gicv3_kvm.c @@ -XXX,XX +XXX,XX @@ static void kvm_dist_putbmp(GICv3State *s, uint32_t offset, if (clroffset != 0) { reg = 0; kvm_gicd_access(s, clroffset, &reg, true); + clroffset += 4; } reg = *gic_bmp_ptr32(bmp, irq); kvm_gicd_access(s, offset, &reg, true); -- 2.17.1 From: Richard Henderson <richard.henderson@linaro.org> Depending on the host abi, float16, aka uint16_t, values are passed and returned either zero-extended in the host register or with garbage at the top of the host register. The tcg code generator has so far been assuming garbage, as that matches the x86 abi, but this is incorrect for other host abis. Further, target/arm has so far been assuming zero-extended results, so that it may store the 16-bit value into a 32-bit slot with the high 16-bits already clear. Rectify both problems by mapping "f16" in the helper definition to uint32_t instead of (a typedef for) uint16_t. This forces the host compiler to assume garbage in the upper 16 bits on input and to zero-extend the result on output. Cc: qemu-stable@nongnu.org Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Tested-by: Laurent Desnogues <laurent.desnogues@gmail.com> Message-id: 20180522175629.24932-1-richard.henderson@linaro.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/exec/helper-head.h | 2 +- target/arm/helper-a64.c | 35 +++++++++-------- target/arm/helper.c | 80 +++++++++++++++++++------------------- 3 files changed, 59 insertions(+), 58 deletions(-) diff --git a/include/exec/helper-head.h b/include/exec/helper-head.h index XXXXXXX..XXXXXXX 100644 --- a/include/exec/helper-head.h +++ b/include/exec/helper-head.h @@ -XXX,XX +XXX,XX @@ #define dh_ctype_int int #define dh_ctype_i64 uint64_t #define dh_ctype_s64 int64_t -#define dh_ctype_f16 float16 +#define dh_ctype_f16 uint32_t #define dh_ctype_f32 float32 #define dh_ctype_f64 float64 #define dh_ctype_ptr void * diff --git a/target/arm/helper-a64.c b/target/arm/helper-a64.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper-a64.c +++ b/target/arm/helper-a64.c @@ -XXX,XX +XXX,XX @@ static inline uint32_t float_rel_to_flags(int res) return flags; } -uint64_t HELPER(vfp_cmph_a64)(float16 x, float16 y, void *fp_status) +uint64_t HELPER(vfp_cmph_a64)(uint32_t x, uint32_t y, void *fp_status) { return float_rel_to_flags(float16_compare_quiet(x, y, fp_status)); } -uint64_t HELPER(vfp_cmpeh_a64)(float16 x, float16 y, void *fp_status) +uint64_t HELPER(vfp_cmpeh_a64)(uint32_t x, uint32_t y, void *fp_status) { return float_rel_to_flags(float16_compare(x, y, fp_status)); } @@ -XXX,XX +XXX,XX @@ uint64_t HELPER(neon_cgt_f64)(float64 a, float64 b, void *fpstp) #define float64_three make_float64(0x4008000000000000ULL) #define float64_one_point_five make_float64(0x3FF8000000000000ULL) -float16 HELPER(recpsf_f16)(float16 a, float16 b, void *fpstp) +uint32_t HELPER(recpsf_f16)(uint32_t a, uint32_t b, void *fpstp) { float_status *fpst = fpstp; @@ -XXX,XX +XXX,XX @@ float64 HELPER(recpsf_f64)(float64 a, float64 b, void *fpstp) return float64_muladd(a, b, float64_two, 0, fpst); } -float16 HELPER(rsqrtsf_f16)(float16 a, float16 b, void *fpstp) +uint32_t HELPER(rsqrtsf_f16)(uint32_t a, uint32_t b, void *fpstp) { float_status *fpst = fpstp; @@ -XXX,XX +XXX,XX @@ uint64_t HELPER(neon_addlp_u16)(uint64_t a) } /* Floating-point reciprocal exponent - see FPRecpX in ARM ARM */ -float16 HELPER(frecpx_f16)(float16 a, void *fpstp) +uint32_t HELPER(frecpx_f16)(uint32_t a, void *fpstp) { float_status *fpst = fpstp; uint16_t val16, sbit; @@ -XXX,XX +XXX,XX @@ void HELPER(casp_be_parallel)(CPUARMState *env, uint32_t rs, uint64_t addr, #define ADVSIMD_HELPER(name, suffix) HELPER(glue(glue(advsimd_, name), suffix)) #define ADVSIMD_HALFOP(name) \ -float16 ADVSIMD_HELPER(name, h)(float16 a, float16 b, void *fpstp) \ +uint32_t ADVSIMD_HELPER(name, h)(uint32_t a, uint32_t b, void *fpstp) \ { \ float_status *fpst = fpstp; \ return float16_ ## name(a, b, fpst); \ @@ -XXX,XX +XXX,XX @@ ADVSIMD_HALFOP(mulx) ADVSIMD_TWOHALFOP(mulx) /* fused multiply-accumulate */ -float16 HELPER(advsimd_muladdh)(float16 a, float16 b, float16 c, void *fpstp) +uint32_t HELPER(advsimd_muladdh)(uint32_t a, uint32_t b, uint32_t c, + void *fpstp) { float_status *fpst = fpstp; return float16_muladd(a, b, c, 0, fpst); @@ -XXX,XX +XXX,XX @@ uint32_t HELPER(advsimd_muladd2h)(uint32_t two_a, uint32_t two_b, #define ADVSIMD_CMPRES(test) (test) ? 0xffff : 0 -uint32_t HELPER(advsimd_ceq_f16)(float16 a, float16 b, void *fpstp) +uint32_t HELPER(advsimd_ceq_f16)(uint32_t a, uint32_t b, void *fpstp) { float_status *fpst = fpstp; int compare = float16_compare_quiet(a, b, fpst); return ADVSIMD_CMPRES(compare == float_relation_equal); } -uint32_t HELPER(advsimd_cge_f16)(float16 a, float16 b, void *fpstp) +uint32_t HELPER(advsimd_cge_f16)(uint32_t a, uint32_t b, void *fpstp) { float_status *fpst = fpstp; int compare = float16_compare(a, b, fpst); @@ -XXX,XX +XXX,XX @@ uint32_t HELPER(advsimd_cge_f16)(float16 a, float16 b, void *fpstp) compare == float_relation_equal); } -uint32_t HELPER(advsimd_cgt_f16)(float16 a, float16 b, void *fpstp) +uint32_t HELPER(advsimd_cgt_f16)(uint32_t a, uint32_t b, void *fpstp) { float_status *fpst = fpstp; int compare = float16_compare(a, b, fpst); return ADVSIMD_CMPRES(compare == float_relation_greater); } -uint32_t HELPER(advsimd_acge_f16)(float16 a, float16 b, void *fpstp) +uint32_t HELPER(advsimd_acge_f16)(uint32_t a, uint32_t b, void *fpstp) { float_status *fpst = fpstp; float16 f0 = float16_abs(a); @@ -XXX,XX +XXX,XX @@ uint32_t HELPER(advsimd_acge_f16)(float16 a, float16 b, void *fpstp) compare == float_relation_equal); } -uint32_t HELPER(advsimd_acgt_f16)(float16 a, float16 b, void *fpstp) +uint32_t HELPER(advsimd_acgt_f16)(uint32_t a, uint32_t b, void *fpstp) { float_status *fpst = fpstp; float16 f0 = float16_abs(a); @@ -XXX,XX +XXX,XX @@ uint32_t HELPER(advsimd_acgt_f16)(float16 a, float16 b, void *fpstp) } /* round to integral */ -float16 HELPER(advsimd_rinth_exact)(float16 x, void *fp_status) +uint32_t HELPER(advsimd_rinth_exact)(uint32_t x, void *fp_status) { return float16_round_to_int(x, fp_status); } -float16 HELPER(advsimd_rinth)(float16 x, void *fp_status) +uint32_t HELPER(advsimd_rinth)(uint32_t x, void *fp_status) { int old_flags = get_float_exception_flags(fp_status), new_flags; float16 ret; @@ -XXX,XX +XXX,XX @@ float16 HELPER(advsimd_rinth)(float16 x, void *fp_status) * setting the mode appropriately before calling the helper. */ -uint32_t HELPER(advsimd_f16tosinth)(float16 a, void *fpstp) +uint32_t HELPER(advsimd_f16tosinth)(uint32_t a, void *fpstp) { float_status *fpst = fpstp; @@ -XXX,XX +XXX,XX @@ uint32_t HELPER(advsimd_f16tosinth)(float16 a, void *fpstp) return float16_to_int16(a, fpst); } -uint32_t HELPER(advsimd_f16touinth)(float16 a, void *fpstp) +uint32_t HELPER(advsimd_f16touinth)(uint32_t a, void *fpstp) { float_status *fpst = fpstp; @@ -XXX,XX +XXX,XX @@ uint32_t HELPER(advsimd_f16touinth)(float16 a, void *fpstp) * Square Root and Reciprocal square root */ -float16 HELPER(sqrt_f16)(float16 a, void *fpstp) +uint32_t HELPER(sqrt_f16)(uint32_t a, void *fpstp) { float_status *s = fpstp; 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 @@ DO_VFP_cmp(d, float64) /* Integer to float and float to integer conversions */ -#define CONV_ITOF(name, fsz, sign) \ - float##fsz HELPER(name)(uint32_t x, void *fpstp) \ -{ \ - float_status *fpst = fpstp; \ - return sign##int32_to_##float##fsz((sign##int32_t)x, fpst); \ +#define CONV_ITOF(name, ftype, fsz, sign) \ +ftype HELPER(name)(uint32_t x, void *fpstp) \ +{ \ + float_status *fpst = fpstp; \ + return sign##int32_to_##float##fsz((sign##int32_t)x, fpst); \ } -#define CONV_FTOI(name, fsz, sign, round) \ -uint32_t HELPER(name)(float##fsz x, void *fpstp) \ -{ \ - float_status *fpst = fpstp; \ - if (float##fsz##_is_any_nan(x)) { \ - float_raise(float_flag_invalid, fpst); \ - return 0; \ - } \ - return float##fsz##_to_##sign##int32##round(x, fpst); \ +#define CONV_FTOI(name, ftype, fsz, sign, round) \ +uint32_t HELPER(name)(ftype x, void *fpstp) \ +{ \ + float_status *fpst = fpstp; \ + if (float##fsz##_is_any_nan(x)) { \ + float_raise(float_flag_invalid, fpst); \ + return 0; \ + } \ + return float##fsz##_to_##sign##int32##round(x, fpst); \ } -#define FLOAT_CONVS(name, p, fsz, sign) \ -CONV_ITOF(vfp_##name##to##p, fsz, sign) \ -CONV_FTOI(vfp_to##name##p, fsz, sign, ) \ -CONV_FTOI(vfp_to##name##z##p, fsz, sign, _round_to_zero) +#define FLOAT_CONVS(name, p, ftype, fsz, sign) \ + CONV_ITOF(vfp_##name##to##p, ftype, fsz, sign) \ + CONV_FTOI(vfp_to##name##p, ftype, fsz, sign, ) \ + CONV_FTOI(vfp_to##name##z##p, ftype, fsz, sign, _round_to_zero) -FLOAT_CONVS(si, h, 16, ) -FLOAT_CONVS(si, s, 32, ) -FLOAT_CONVS(si, d, 64, ) -FLOAT_CONVS(ui, h, 16, u) -FLOAT_CONVS(ui, s, 32, u) -FLOAT_CONVS(ui, d, 64, u) +FLOAT_CONVS(si, h, uint32_t, 16, ) +FLOAT_CONVS(si, s, float32, 32, ) +FLOAT_CONVS(si, d, float64, 64, ) +FLOAT_CONVS(ui, h, uint32_t, 16, u) +FLOAT_CONVS(ui, s, float32, 32, u) +FLOAT_CONVS(ui, d, float64, 64, u) #undef CONV_ITOF #undef CONV_FTOI @@ -XXX,XX +XXX,XX @@ static float16 do_postscale_fp16(float64 f, int shift, float_status *fpst) return float64_to_float16(float64_scalbn(f, -shift, fpst), true, fpst); } -float16 HELPER(vfp_sltoh)(uint32_t x, uint32_t shift, void *fpst) +uint32_t HELPER(vfp_sltoh)(uint32_t x, uint32_t shift, void *fpst) { return do_postscale_fp16(int32_to_float64(x, fpst), shift, fpst); } -float16 HELPER(vfp_ultoh)(uint32_t x, uint32_t shift, void *fpst) +uint32_t HELPER(vfp_ultoh)(uint32_t x, uint32_t shift, void *fpst) { return do_postscale_fp16(uint32_to_float64(x, fpst), shift, fpst); } -float16 HELPER(vfp_sqtoh)(uint64_t x, uint32_t shift, void *fpst) +uint32_t HELPER(vfp_sqtoh)(uint64_t x, uint32_t shift, void *fpst) { return do_postscale_fp16(int64_to_float64(x, fpst), shift, fpst); } -float16 HELPER(vfp_uqtoh)(uint64_t x, uint32_t shift, void *fpst) +uint32_t HELPER(vfp_uqtoh)(uint64_t x, uint32_t shift, void *fpst) { return do_postscale_fp16(uint64_to_float64(x, fpst), shift, fpst); } @@ -XXX,XX +XXX,XX @@ static float64 do_prescale_fp16(float16 f, int shift, float_status *fpst) } } -uint32_t HELPER(vfp_toshh)(float16 x, uint32_t shift, void *fpst) +uint32_t HELPER(vfp_toshh)(uint32_t x, uint32_t shift, void *fpst) { return float64_to_int16(do_prescale_fp16(x, shift, fpst), fpst); } -uint32_t HELPER(vfp_touhh)(float16 x, uint32_t shift, void *fpst) +uint32_t HELPER(vfp_touhh)(uint32_t x, uint32_t shift, void *fpst) { return float64_to_uint16(do_prescale_fp16(x, shift, fpst), fpst); } -uint32_t HELPER(vfp_toslh)(float16 x, uint32_t shift, void *fpst) +uint32_t HELPER(vfp_toslh)(uint32_t x, uint32_t shift, void *fpst) { return float64_to_int32(do_prescale_fp16(x, shift, fpst), fpst); } -uint32_t HELPER(vfp_toulh)(float16 x, uint32_t shift, void *fpst) +uint32_t HELPER(vfp_toulh)(uint32_t x, uint32_t shift, void *fpst) { return float64_to_uint32(do_prescale_fp16(x, shift, fpst), fpst); } -uint64_t HELPER(vfp_tosqh)(float16 x, uint32_t shift, void *fpst) +uint64_t HELPER(vfp_tosqh)(uint32_t x, uint32_t shift, void *fpst) { return float64_to_int64(do_prescale_fp16(x, shift, fpst), fpst); } -uint64_t HELPER(vfp_touqh)(float16 x, uint32_t shift, void *fpst) +uint64_t HELPER(vfp_touqh)(uint32_t x, uint32_t shift, void *fpst) { return float64_to_uint64(do_prescale_fp16(x, shift, fpst), fpst); } @@ -XXX,XX +XXX,XX @@ uint32_t HELPER(set_neon_rmode)(uint32_t rmode, CPUARMState *env) } /* Half precision conversions. */ -float32 HELPER(vfp_fcvt_f16_to_f32)(float16 a, void *fpstp, uint32_t ahp_mode) +float32 HELPER(vfp_fcvt_f16_to_f32)(uint32_t a, void *fpstp, uint32_t ahp_mode) { /* Squash FZ16 to 0 for the duration of conversion. In this case, * it would affect flushing input denormals. @@ -XXX,XX +XXX,XX @@ float32 HELPER(vfp_fcvt_f16_to_f32)(float16 a, void *fpstp, uint32_t ahp_mode) return r; } -float16 HELPER(vfp_fcvt_f32_to_f16)(float32 a, void *fpstp, uint32_t ahp_mode) +uint32_t HELPER(vfp_fcvt_f32_to_f16)(float32 a, void *fpstp, uint32_t ahp_mode) { /* Squash FZ16 to 0 for the duration of conversion. In this case, * it would affect flushing output denormals. @@ -XXX,XX +XXX,XX @@ float16 HELPER(vfp_fcvt_f32_to_f16)(float32 a, void *fpstp, uint32_t ahp_mode) return r; } -float64 HELPER(vfp_fcvt_f16_to_f64)(float16 a, void *fpstp, uint32_t ahp_mode) +float64 HELPER(vfp_fcvt_f16_to_f64)(uint32_t a, void *fpstp, uint32_t ahp_mode) { /* Squash FZ16 to 0 for the duration of conversion. In this case, * it would affect flushing input denormals. @@ -XXX,XX +XXX,XX @@ float64 HELPER(vfp_fcvt_f16_to_f64)(float16 a, void *fpstp, uint32_t ahp_mode) return r; } -float16 HELPER(vfp_fcvt_f64_to_f16)(float64 a, void *fpstp, uint32_t ahp_mode) +uint32_t HELPER(vfp_fcvt_f64_to_f16)(float64 a, void *fpstp, uint32_t ahp_mode) { /* Squash FZ16 to 0 for the duration of conversion. In this case, * it would affect flushing output denormals. @@ -XXX,XX +XXX,XX @@ static bool round_to_inf(float_status *fpst, bool sign_bit) g_assert_not_reached(); } -float16 HELPER(recpe_f16)(float16 input, void *fpstp) +uint32_t HELPER(recpe_f16)(uint32_t input, void *fpstp) { float_status *fpst = fpstp; float16 f16 = float16_squash_input_denormal(input, fpst); @@ -XXX,XX +XXX,XX @@ static uint64_t recip_sqrt_estimate(int *exp , int exp_off, uint64_t frac) return extract64(estimate, 0, 8) << 44; } -float16 HELPER(rsqrte_f16)(float16 input, void *fpstp) +uint32_t HELPER(rsqrte_f16)(uint32_t input, void *fpstp) { float_status *s = fpstp; float16 f16 = float16_squash_input_denormal(input, s); -- 2.17.1 From: Igor Mammedov <imammedo@redhat.com> When QEMU is started with following CLI -machine virt,gic-version=3,accel=kvm -cpu host -bios AAVMF_CODE.fd it crashes with abort at accel/kvm/kvm-all.c:2164: KVM_SET_DEVICE_ATTR failed: Group 6 attr 0x000000000000c665: Invalid argument Which is caused by implicit dependency of kvm_arm_gicv3_reset() on arm_gicv3_icc_reset() where the later is called by CPU reset reset callback. However commit: 3b77f6c arm/boot: split load_dtb() from arm_load_kernel() broke CPU reset callback registration in case arm_load_kernel() ... if (!info->kernel_filename || info->firmware_loaded) branch is taken, i.e. it's sufficient to provide a firmware or do not provide kernel on CLI to skip cpu reset callback registration, where before offending commit the callback has been registered unconditionally. Fix it by registering the callback right at the beginning of arm_load_kernel() unconditionally instead of doing it at the end. NOTE: we probably should eliminate that dependency anyways as well as separate arch CPU reset parts from arm_load_kernel() into CPU itself, but that refactoring that I probably would have to do anyways later for CPU hotplug to work. Reported-by: Auger Eric <eric.auger@redhat.com> Signed-off-by: Igor Mammedov <imammedo@redhat.com> Reviewed-by: Eric Auger <eric.auger@redhat.com> Tested-by: Eric Auger <eric.auger@redhat.com> Message-id: 1527070950-208350-1-git-send-email-imammedo@redhat.com Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/boot.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/hw/arm/boot.c b/hw/arm/boot.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/boot.c +++ b/hw/arm/boot.c @@ -XXX,XX +XXX,XX @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info) static const ARMInsnFixup *primary_loader; AddressSpace *as = arm_boot_address_space(cpu, info); + /* CPU objects (unlike devices) are not automatically reset on system + * reset, so we must always register a handler to do so. If we're + * actually loading a kernel, the handler is also responsible for + * arranging that we start it correctly. + */ + for (cs = first_cpu; cs; cs = CPU_NEXT(cs)) { + qemu_register_reset(do_cpu_reset, ARM_CPU(cs)); + } + /* The board code is not supposed to set secure_board_setup unless * running its code in secure mode is actually possible, and KVM * doesn't support secure. @@ -XXX,XX +XXX,XX @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info) ARM_CPU(cs)->env.boot_info = info; } - /* CPU objects (unlike devices) are not automatically reset on system - * reset, so we must always register a handler to do so. If we're - * actually loading a kernel, the handler is also responsible for - * arranging that we start it correctly. - */ - for (cs = first_cpu; cs; cs = CPU_NEXT(cs)) { - qemu_register_reset(do_cpu_reset, ARM_CPU(cs)); - } - if (!info->skip_dtb_autoload && have_dtb(info)) { if (arm_load_dtb(info->dtb_start, info, info->dtb_limit, as) < 0) { exit(1); -- 2.17.1 From: Paolo Bonzini <pbonzini@redhat.com> cpregs_keys is an uint32_t* so the allocation should use uint32_t. g_new is even better because it is type-safe. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/gdbstub.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/gdbstub.c +++ b/target/arm/gdbstub.c @@ -XXX,XX +XXX,XX @@ int arm_gen_dynamic_xml(CPUState *cs) RegisterSysregXmlParam param = {cs, s}; cpu->dyn_xml.num_cpregs = 0; - cpu->dyn_xml.cpregs_keys = g_malloc(sizeof(uint32_t *) * - g_hash_table_size(cpu->cp_regs)); + cpu->dyn_xml.cpregs_keys = g_new(uint32_t, g_hash_table_size(cpu->cp_regs)); g_string_printf(s, "<?xml version=\"1.0\"?>"); g_string_append_printf(s, "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">"); g_string_append_printf(s, "<feature name=\"org.qemu.gdb.arm.sys.regs\">"); -- 2.17.1 From: Francisco Iglesias <frasse.iglesias@gmail.com> Coverity found that the string return by 'object_get_canonical_path' was not being freed at two locations in the model (CID 1391294 and CID 1391293) and also that a memset was being called with a value greater than the max of a byte on the second argument (CID 1391286). This patch corrects this by adding the freeing of the strings and also changing to memset to zero instead on descriptor unaligned errors. Signed-off-by: Francisco Iglesias <frasse.iglesias@gmail.com> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20180528184859.3530-1-frasse.iglesias@gmail.com Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/dma/xlnx-zdma.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/hw/dma/xlnx-zdma.c b/hw/dma/xlnx-zdma.c index XXXXXXX..XXXXXXX 100644 --- a/hw/dma/xlnx-zdma.c +++ b/hw/dma/xlnx-zdma.c @@ -XXX,XX +XXX,XX @@ static bool zdma_load_descriptor(XlnxZDMA *s, uint64_t addr, void *buf) qemu_log_mask(LOG_GUEST_ERROR, "zdma: unaligned descriptor at %" PRIx64, addr); - memset(buf, 0xdeadbeef, sizeof(XlnxZDMADescr)); + memset(buf, 0x0, sizeof(XlnxZDMADescr)); s->error = true; return false; } @@ -XXX,XX +XXX,XX @@ static uint64_t zdma_read(void *opaque, hwaddr addr, unsigned size) RegisterInfo *r = &s->regs_info[addr / 4]; if (!r->data) { + gchar *path = object_get_canonical_path(OBJECT(s)); qemu_log("%s: Decode error: read from %" HWADDR_PRIx "\n", - object_get_canonical_path(OBJECT(s)), + path, addr); + g_free(path); ARRAY_FIELD_DP32(s->regs, ZDMA_CH_ISR, INV_APB, true); zdma_ch_imr_update_irq(s); return 0; @@ -XXX,XX +XXX,XX @@ static void zdma_write(void *opaque, hwaddr addr, uint64_t value, RegisterInfo *r = &s->regs_info[addr / 4]; if (!r->data) { + gchar *path = object_get_canonical_path(OBJECT(s)); qemu_log("%s: Decode error: write to %" HWADDR_PRIx "=%" PRIx64 "\n", - object_get_canonical_path(OBJECT(s)), + path, addr, value); + g_free(path); ARRAY_FIELD_DP32(s->regs, ZDMA_CH_ISR, INV_APB, true); zdma_ch_imr_update_irq(s); return; -- 2.17.1 In commit f0aff255700 we made cpacr_write() enforce that some CPACR bits are RAZ/WI and some are RAO/WI for ARMv7 cores. Unfortunately we forgot to also update the register's reset value. The effect was that (a) a guest that read CPACR on reset would not see ones in the RAO bits, and (b) if you did a migration before the guest did a write to the CPACR then the migration would fail because the destination would enforce the RAO bits and then complain that they didn't match the zero value from the source. Implement reset for the CPACR using a custom reset function that just calls cpacr_write(), to avoid having to duplicate the logic for which bits are RAO. This bug would affect migration for TCG CPUs which are ARMv7 with VFP but without one of Neon or VFPv3. Reported-by: Cédric Le Goater <clg@kaod.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Tested-by: Cédric Le Goater <clg@kaod.org> Message-id: 20180522173713.26282-1-peter.maydell@linaro.org --- target/arm/helper.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/target/arm/helper.c b/target/arm/helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -XXX,XX +XXX,XX @@ static void cpacr_write(CPUARMState *env, const ARMCPRegInfo *ri, env->cp15.cpacr_el1 = value; } +static void cpacr_reset(CPUARMState *env, const ARMCPRegInfo *ri) +{ + /* Call cpacr_write() so that we reset with the correct RAO bits set + * for our CPU features. + */ + cpacr_write(env, ri, 0); +} + static CPAccessResult cpacr_access(CPUARMState *env, const ARMCPRegInfo *ri, bool isread) { @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo v6_cp_reginfo[] = { { .name = "CPACR", .state = ARM_CP_STATE_BOTH, .opc0 = 3, .crn = 1, .crm = 0, .opc1 = 0, .opc2 = 2, .accessfn = cpacr_access, .access = PL1_RW, .fieldoffset = offsetof(CPUARMState, cp15.cpacr_el1), - .resetvalue = 0, .writefn = cpacr_write }, + .resetfn = cpacr_reset, .writefn = cpacr_write }, REGINFO_SENTINEL }; -- 2.17.1 Add more detail to the documentation for memory_region_init_iommu() and other IOMMU-related functions and data structures. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Eric Auger <eric.auger@redhat.com> Message-id: 20180521140402.23318-2-peter.maydell@linaro.org --- include/exec/memory.h | 105 ++++++++++++++++++++++++++++++++++++++---- 1 file changed, 95 insertions(+), 10 deletions(-) diff --git a/include/exec/memory.h b/include/exec/memory.h index XXXXXXX..XXXXXXX 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -XXX,XX +XXX,XX @@ enum IOMMUMemoryRegionAttr { IOMMU_ATTR_SPAPR_TCE_FD }; +/** + * IOMMUMemoryRegionClass: + * + * All IOMMU implementations need to subclass TYPE_IOMMU_MEMORY_REGION + * and provide an implementation of at least the @translate method here + * to handle requests to the memory region. Other methods are optional. + * + * The IOMMU implementation must use the IOMMU notifier infrastructure + * to report whenever mappings are changed, by calling + * memory_region_notify_iommu() (or, if necessary, by calling + * memory_region_notify_one() for each registered notifier). + */ typedef struct IOMMUMemoryRegionClass { /* private */ struct DeviceClass parent_class; /* - * Return a TLB entry that contains a given address. Flag should - * be the access permission of this translation operation. We can - * set flag to IOMMU_NONE to mean that we don't need any - * read/write permission checks, like, when for region replay. + * Return a TLB entry that contains a given address. + * + * The IOMMUAccessFlags indicated via @flag are optional and may + * be specified as IOMMU_NONE to indicate that the caller needs + * the full translation information for both reads and writes. If + * the access flags are specified then the IOMMU implementation + * may use this as an optimization, to stop doing a page table + * walk as soon as it knows that the requested permissions are not + * allowed. If IOMMU_NONE is passed then the IOMMU must do the + * full page table walk and report the permissions in the returned + * IOMMUTLBEntry. (Note that this implies that an IOMMU may not + * return different mappings for reads and writes.) + * + * The returned information remains valid while the caller is + * holding the big QEMU lock or is inside an RCU critical section; + * if the caller wishes to cache the mapping beyond that it must + * register an IOMMU notifier so it can invalidate its cached + * information when the IOMMU mapping changes. + * + * @iommu: the IOMMUMemoryRegion + * @hwaddr: address to be translated within the memory region + * @flag: requested access permissions */ IOMMUTLBEntry (*translate)(IOMMUMemoryRegion *iommu, hwaddr addr, IOMMUAccessFlags flag); - /* Returns minimum supported page size */ + /* Returns minimum supported page size in bytes. + * If this method is not provided then the minimum is assumed to + * be TARGET_PAGE_SIZE. + * + * @iommu: the IOMMUMemoryRegion + */ uint64_t (*get_min_page_size)(IOMMUMemoryRegion *iommu); - /* Called when IOMMU Notifier flag changed */ + /* Called when IOMMU Notifier flag changes (ie when the set of + * events which IOMMU users are requesting notification for changes). + * Optional method -- need not be provided if the IOMMU does not + * need to know exactly which events must be notified. + * + * @iommu: the IOMMUMemoryRegion + * @old_flags: events which previously needed to be notified + * @new_flags: events which now need to be notified + */ void (*notify_flag_changed)(IOMMUMemoryRegion *iommu, IOMMUNotifierFlag old_flags, IOMMUNotifierFlag new_flags); - /* Set this up to provide customized IOMMU replay function */ + /* Called to handle memory_region_iommu_replay(). + * + * The default implementation of memory_region_iommu_replay() is to + * call the IOMMU translate method for every page in the address space + * with flag == IOMMU_NONE and then call the notifier if translate + * returns a valid mapping. If this method is implemented then it + * overrides the default behaviour, and must provide the full semantics + * of memory_region_iommu_replay(), by calling @notifier for every + * translation present in the IOMMU. + * + * Optional method -- an IOMMU only needs to provide this method + * if the default is inefficient or produces undesirable side effects. + * + * Note: this is not related to record-and-replay functionality. + */ void (*replay)(IOMMUMemoryRegion *iommu, IOMMUNotifier *notifier); - /* Get IOMMU misc attributes */ - int (*get_attr)(IOMMUMemoryRegion *iommu, enum IOMMUMemoryRegionAttr, + /* Get IOMMU misc attributes. This is an optional method that + * can be used to allow users of the IOMMU to get implementation-specific + * information. The IOMMU implements this method to handle calls + * by IOMMU users to memory_region_iommu_get_attr() by filling in + * the arbitrary data pointer for any IOMMUMemoryRegionAttr values that + * the IOMMU supports. If the method is unimplemented then + * memory_region_iommu_get_attr() will always return -EINVAL. + * + * @iommu: the IOMMUMemoryRegion + * @attr: attribute being queried + * @data: memory to fill in with the attribute data + * + * Returns 0 on success, or a negative errno; in particular + * returns -EINVAL for unrecognized or unimplemented attribute types. + */ + int (*get_attr)(IOMMUMemoryRegion *iommu, enum IOMMUMemoryRegionAttr attr, void *data); } IOMMUMemoryRegionClass; @@ -XXX,XX +XXX,XX @@ static inline void memory_region_init_reservation(MemoryRegion *mr, * An IOMMU region translates addresses and forwards accesses to a target * memory region. * + * The IOMMU implementation must define a subclass of TYPE_IOMMU_MEMORY_REGION. + * @_iommu_mr should be a pointer to enough memory for an instance of + * that subclass, @instance_size is the size of that subclass, and + * @mrtypename is its name. This function will initialize @_iommu_mr as an + * instance of the subclass, and its methods will then be called to handle + * accesses to the memory region. See the documentation of + * #IOMMUMemoryRegionClass for further details. + * * @_iommu_mr: the #IOMMUMemoryRegion to be initialized * @instance_size: the IOMMUMemoryRegion subclass instance size * @mrtypename: the type name of the #IOMMUMemoryRegion @@ -XXX,XX +XXX,XX @@ void memory_region_register_iommu_notifier(MemoryRegion *mr, * a notifier with the minimum page granularity returned by * mr->iommu_ops->get_page_size(). * + * Note: this is not related to record-and-replay functionality. + * * @iommu_mr: the memory region to observe * @n: the notifier to which to replay iommu mappings */ @@ -XXX,XX +XXX,XX @@ void memory_region_iommu_replay(IOMMUMemoryRegion *iommu_mr, IOMMUNotifier *n); * memory_region_iommu_replay_all: replay existing IOMMU translations * to all the notifiers registered. * + * Note: this is not related to record-and-replay functionality. + * * @iommu_mr: the memory region to observe */ void memory_region_iommu_replay_all(IOMMUMemoryRegion *iommu_mr); @@ -XXX,XX +XXX,XX @@ void memory_region_unregister_iommu_notifier(MemoryRegion *mr, * memory_region_iommu_get_attr: return an IOMMU attr if get_attr() is * defined on the IOMMU. * - * Returns 0 if succeded, error code otherwise. + * Returns 0 on success, or a negative errno otherwise. In particular, + * -EINVAL indicates that the IOMMU does not support the requested + * attribute. * * @iommu_mr: the memory region * @attr: the requested attribute -- 2.17.1 As part of plumbing MemTxAttrs down to the IOMMU translate method, add MemTxAttrs as an argument to tb_invalidate_phys_addr(). Its callers either have an attrs value to hand, or don't care and can use MEMTXATTRS_UNSPECIFIED. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Message-id: 20180521140402.23318-3-peter.maydell@linaro.org --- include/exec/exec-all.h | 5 +++-- accel/tcg/translate-all.c | 2 +- exec.c | 2 +- target/xtensa/op_helper.c | 3 ++- 4 files changed, 7 insertions(+), 5 deletions(-) diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h index XXXXXXX..XXXXXXX 100644 --- a/include/exec/exec-all.h +++ b/include/exec/exec-all.h @@ -XXX,XX +XXX,XX @@ void tlb_set_page_with_attrs(CPUState *cpu, target_ulong vaddr, void tlb_set_page(CPUState *cpu, target_ulong vaddr, hwaddr paddr, int prot, int mmu_idx, target_ulong size); -void tb_invalidate_phys_addr(AddressSpace *as, hwaddr addr); +void tb_invalidate_phys_addr(AddressSpace *as, hwaddr addr, MemTxAttrs attrs); void probe_write(CPUArchState *env, target_ulong addr, int size, int mmu_idx, uintptr_t retaddr); #else @@ -XXX,XX +XXX,XX @@ static inline void tlb_flush_by_mmuidx_all_cpus_synced(CPUState *cpu, uint16_t idxmap) { } -static inline void tb_invalidate_phys_addr(AddressSpace *as, hwaddr addr) +static inline void tb_invalidate_phys_addr(AddressSpace *as, hwaddr addr, + MemTxAttrs attrs) { } #endif diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c index XXXXXXX..XXXXXXX 100644 --- a/accel/tcg/translate-all.c +++ b/accel/tcg/translate-all.c @@ -XXX,XX +XXX,XX @@ static TranslationBlock *tb_find_pc(uintptr_t tc_ptr) } #if !defined(CONFIG_USER_ONLY) -void tb_invalidate_phys_addr(AddressSpace *as, hwaddr addr) +void tb_invalidate_phys_addr(AddressSpace *as, hwaddr addr, MemTxAttrs attrs) { ram_addr_t ram_addr; MemoryRegion *mr; diff --git a/exec.c b/exec.c index XXXXXXX..XXXXXXX 100644 --- a/exec.c +++ b/exec.c @@ -XXX,XX +XXX,XX @@ static void breakpoint_invalidate(CPUState *cpu, target_ulong pc) if (phys != -1) { /* Locks grabbed by tb_invalidate_phys_addr */ tb_invalidate_phys_addr(cpu->cpu_ases[asidx].as, - phys | (pc & ~TARGET_PAGE_MASK)); + phys | (pc & ~TARGET_PAGE_MASK), attrs); } } #endif diff --git a/target/xtensa/op_helper.c b/target/xtensa/op_helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/xtensa/op_helper.c +++ b/target/xtensa/op_helper.c @@ -XXX,XX +XXX,XX @@ static void tb_invalidate_virtual_addr(CPUXtensaState *env, uint32_t vaddr) int ret = xtensa_get_physical_addr(env, false, vaddr, 2, 0, &paddr, &page_size, &access); if (ret == 0) { - tb_invalidate_phys_addr(&address_space_memory, paddr); + tb_invalidate_phys_addr(&address_space_memory, paddr, + MEMTXATTRS_UNSPECIFIED); } } -- 2.17.1 As part of plumbing MemTxAttrs down to the IOMMU translate method, add MemTxAttrs as an argument to address_space_translate() and address_space_translate_cached(). Callers either have an attrs value to hand, or don't care and can use MEMTXATTRS_UNSPECIFIED. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20180521140402.23318-4-peter.maydell@linaro.org --- include/exec/memory.h | 4 +++- accel/tcg/translate-all.c | 2 +- exec.c | 14 +++++++++----- hw/vfio/common.c | 3 ++- memory_ldst.inc.c | 18 +++++++++--------- target/riscv/helper.c | 2 +- 6 files changed, 25 insertions(+), 18 deletions(-) diff --git a/include/exec/memory.h b/include/exec/memory.h index XXXXXXX..XXXXXXX 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -XXX,XX +XXX,XX @@ IOMMUTLBEntry address_space_get_iotlb_entry(AddressSpace *as, hwaddr addr, * #MemoryRegion. * @len: pointer to length * @is_write: indicates the transfer direction + * @attrs: memory attributes */ MemoryRegion *flatview_translate(FlatView *fv, hwaddr addr, hwaddr *xlat, @@ -XXX,XX +XXX,XX @@ MemoryRegion *flatview_translate(FlatView *fv, static inline MemoryRegion *address_space_translate(AddressSpace *as, hwaddr addr, hwaddr *xlat, - hwaddr *len, bool is_write) + hwaddr *len, bool is_write, + MemTxAttrs attrs) { return flatview_translate(address_space_to_flatview(as), addr, xlat, len, is_write); diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c index XXXXXXX..XXXXXXX 100644 --- a/accel/tcg/translate-all.c +++ b/accel/tcg/translate-all.c @@ -XXX,XX +XXX,XX @@ void tb_invalidate_phys_addr(AddressSpace *as, hwaddr addr, MemTxAttrs attrs) hwaddr l = 1; rcu_read_lock(); - mr = address_space_translate(as, addr, &addr, &l, false); + mr = address_space_translate(as, addr, &addr, &l, false, attrs); if (!(memory_region_is_ram(mr) || memory_region_is_romd(mr))) { rcu_read_unlock(); diff --git a/exec.c b/exec.c index XXXXXXX..XXXXXXX 100644 --- a/exec.c +++ b/exec.c @@ -XXX,XX +XXX,XX @@ static inline void cpu_physical_memory_write_rom_internal(AddressSpace *as, rcu_read_lock(); while (len > 0) { l = len; - mr = address_space_translate(as, addr, &addr1, &l, true); + mr = address_space_translate(as, addr, &addr1, &l, true, + MEMTXATTRS_UNSPECIFIED); if (!(memory_region_is_ram(mr) || memory_region_is_romd(mr))) { @@ -XXX,XX +XXX,XX @@ void address_space_cache_destroy(MemoryRegionCache *cache) */ static inline MemoryRegion *address_space_translate_cached( MemoryRegionCache *cache, hwaddr addr, hwaddr *xlat, - hwaddr *plen, bool is_write) + hwaddr *plen, bool is_write, MemTxAttrs attrs) { MemoryRegionSection section; MemoryRegion *mr; @@ -XXX,XX +XXX,XX @@ address_space_read_cached_slow(MemoryRegionCache *cache, hwaddr addr, MemoryRegion *mr; l = len; - mr = address_space_translate_cached(cache, addr, &addr1, &l, false); + mr = address_space_translate_cached(cache, addr, &addr1, &l, false, + MEMTXATTRS_UNSPECIFIED); flatview_read_continue(cache->fv, addr, MEMTXATTRS_UNSPECIFIED, buf, len, addr1, l, mr); @@ -XXX,XX +XXX,XX @@ address_space_write_cached_slow(MemoryRegionCache *cache, hwaddr addr, MemoryRegion *mr; l = len; - mr = address_space_translate_cached(cache, addr, &addr1, &l, true); + mr = address_space_translate_cached(cache, addr, &addr1, &l, true, + MEMTXATTRS_UNSPECIFIED); flatview_write_continue(cache->fv, addr, MEMTXATTRS_UNSPECIFIED, buf, len, addr1, l, mr); @@ -XXX,XX +XXX,XX @@ bool cpu_physical_memory_is_io(hwaddr phys_addr) rcu_read_lock(); mr = address_space_translate(&address_space_memory, - phys_addr, &phys_addr, &l, false); + phys_addr, &phys_addr, &l, false, + MEMTXATTRS_UNSPECIFIED); res = !(memory_region_is_ram(mr) || memory_region_is_romd(mr)); rcu_read_unlock(); diff --git a/hw/vfio/common.c b/hw/vfio/common.c index XXXXXXX..XXXXXXX 100644 --- a/hw/vfio/common.c +++ b/hw/vfio/common.c @@ -XXX,XX +XXX,XX @@ static bool vfio_get_vaddr(IOMMUTLBEntry *iotlb, void **vaddr, */ mr = address_space_translate(&address_space_memory, iotlb->translated_addr, - &xlat, &len, writable); + &xlat, &len, writable, + MEMTXATTRS_UNSPECIFIED); if (!memory_region_is_ram(mr)) { error_report("iommu map to non memory area %"HWADDR_PRIx"", xlat); diff --git a/memory_ldst.inc.c b/memory_ldst.inc.c index XXXXXXX..XXXXXXX 100644 --- a/memory_ldst.inc.c +++ b/memory_ldst.inc.c @@ -XXX,XX +XXX,XX @@ static inline uint32_t glue(address_space_ldl_internal, SUFFIX)(ARG1_DECL, bool release_lock = false; RCU_READ_LOCK(); - mr = TRANSLATE(addr, &addr1, &l, false); + mr = TRANSLATE(addr, &addr1, &l, false, attrs); if (l < 4 || !IS_DIRECT(mr, false)) { release_lock |= prepare_mmio_access(mr); @@ -XXX,XX +XXX,XX @@ static inline uint64_t glue(address_space_ldq_internal, SUFFIX)(ARG1_DECL, bool release_lock = false; RCU_READ_LOCK(); - mr = TRANSLATE(addr, &addr1, &l, false); + mr = TRANSLATE(addr, &addr1, &l, false, attrs); if (l < 8 || !IS_DIRECT(mr, false)) { release_lock |= prepare_mmio_access(mr); @@ -XXX,XX +XXX,XX @@ uint32_t glue(address_space_ldub, SUFFIX)(ARG1_DECL, bool release_lock = false; RCU_READ_LOCK(); - mr = TRANSLATE(addr, &addr1, &l, false); + mr = TRANSLATE(addr, &addr1, &l, false, attrs); if (!IS_DIRECT(mr, false)) { release_lock |= prepare_mmio_access(mr); @@ -XXX,XX +XXX,XX @@ static inline uint32_t glue(address_space_lduw_internal, SUFFIX)(ARG1_DECL, bool release_lock = false; RCU_READ_LOCK(); - mr = TRANSLATE(addr, &addr1, &l, false); + mr = TRANSLATE(addr, &addr1, &l, false, attrs); if (l < 2 || !IS_DIRECT(mr, false)) { release_lock |= prepare_mmio_access(mr); @@ -XXX,XX +XXX,XX @@ void glue(address_space_stl_notdirty, SUFFIX)(ARG1_DECL, bool release_lock = false; RCU_READ_LOCK(); - mr = TRANSLATE(addr, &addr1, &l, true); + mr = TRANSLATE(addr, &addr1, &l, true, attrs); if (l < 4 || !IS_DIRECT(mr, true)) { release_lock |= prepare_mmio_access(mr); @@ -XXX,XX +XXX,XX @@ static inline void glue(address_space_stl_internal, SUFFIX)(ARG1_DECL, bool release_lock = false; RCU_READ_LOCK(); - mr = TRANSLATE(addr, &addr1, &l, true); + mr = TRANSLATE(addr, &addr1, &l, true, attrs); if (l < 4 || !IS_DIRECT(mr, true)) { release_lock |= prepare_mmio_access(mr); @@ -XXX,XX +XXX,XX @@ void glue(address_space_stb, SUFFIX)(ARG1_DECL, bool release_lock = false; RCU_READ_LOCK(); - mr = TRANSLATE(addr, &addr1, &l, true); + mr = TRANSLATE(addr, &addr1, &l, true, attrs); if (!IS_DIRECT(mr, true)) { release_lock |= prepare_mmio_access(mr); r = memory_region_dispatch_write(mr, addr1, val, 1, attrs); @@ -XXX,XX +XXX,XX @@ static inline void glue(address_space_stw_internal, SUFFIX)(ARG1_DECL, bool release_lock = false; RCU_READ_LOCK(); - mr = TRANSLATE(addr, &addr1, &l, true); + mr = TRANSLATE(addr, &addr1, &l, true, attrs); if (l < 2 || !IS_DIRECT(mr, true)) { release_lock |= prepare_mmio_access(mr); @@ -XXX,XX +XXX,XX @@ static void glue(address_space_stq_internal, SUFFIX)(ARG1_DECL, bool release_lock = false; RCU_READ_LOCK(); - mr = TRANSLATE(addr, &addr1, &l, true); + mr = TRANSLATE(addr, &addr1, &l, true, attrs); if (l < 8 || !IS_DIRECT(mr, true)) { release_lock |= prepare_mmio_access(mr); diff --git a/target/riscv/helper.c b/target/riscv/helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/riscv/helper.c +++ b/target/riscv/helper.c @@ -XXX,XX +XXX,XX @@ restart: MemoryRegion *mr; hwaddr l = sizeof(target_ulong), addr1; mr = address_space_translate(cs->as, pte_addr, - &addr1, &l, false); + &addr1, &l, false, MEMTXATTRS_UNSPECIFIED); if (memory_access_is_direct(mr, true)) { target_ulong *pte_pa = qemu_map_ram_ptr(mr->ram_block, addr1); -- 2.17.1 As part of plumbing MemTxAttrs down to the IOMMU translate method, add MemTxAttrs as an argument to address_space_map(). Its callers either have an attrs value to hand, or don't care and can use MEMTXATTRS_UNSPECIFIED. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20180521140402.23318-5-peter.maydell@linaro.org --- include/exec/memory.h | 3 ++- include/sysemu/dma.h | 3 ++- exec.c | 6 ++++-- target/ppc/mmu-hash64.c | 3 ++- 4 files changed, 10 insertions(+), 5 deletions(-) diff --git a/include/exec/memory.h b/include/exec/memory.h index XXXXXXX..XXXXXXX 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -XXX,XX +XXX,XX @@ bool address_space_access_valid(AddressSpace *as, hwaddr addr, int len, bool is_ * @addr: address within that address space * @plen: pointer to length of buffer; updated on return * @is_write: indicates the transfer direction + * @attrs: memory attributes */ void *address_space_map(AddressSpace *as, hwaddr addr, - hwaddr *plen, bool is_write); + hwaddr *plen, bool is_write, MemTxAttrs attrs); /* address_space_unmap: Unmaps a memory region previously mapped by address_space_map() * diff --git a/include/sysemu/dma.h b/include/sysemu/dma.h index XXXXXXX..XXXXXXX 100644 --- a/include/sysemu/dma.h +++ b/include/sysemu/dma.h @@ -XXX,XX +XXX,XX @@ static inline void *dma_memory_map(AddressSpace *as, hwaddr xlen = *len; void *p; - p = address_space_map(as, addr, &xlen, dir == DMA_DIRECTION_FROM_DEVICE); + p = address_space_map(as, addr, &xlen, dir == DMA_DIRECTION_FROM_DEVICE, + MEMTXATTRS_UNSPECIFIED); *len = xlen; return p; } diff --git a/exec.c b/exec.c index XXXXXXX..XXXXXXX 100644 --- a/exec.c +++ b/exec.c @@ -XXX,XX +XXX,XX @@ flatview_extend_translation(FlatView *fv, hwaddr addr, void *address_space_map(AddressSpace *as, hwaddr addr, hwaddr *plen, - bool is_write) + bool is_write, + MemTxAttrs attrs) { hwaddr len = *plen; hwaddr l, xlat; @@ -XXX,XX +XXX,XX @@ void *cpu_physical_memory_map(hwaddr addr, hwaddr *plen, int is_write) { - return address_space_map(&address_space_memory, addr, plen, is_write); + return address_space_map(&address_space_memory, addr, plen, is_write, + MEMTXATTRS_UNSPECIFIED); } void cpu_physical_memory_unmap(void *buffer, hwaddr len, diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c index XXXXXXX..XXXXXXX 100644 --- a/target/ppc/mmu-hash64.c +++ b/target/ppc/mmu-hash64.c @@ -XXX,XX +XXX,XX @@ const ppc_hash_pte64_t *ppc_hash64_map_hptes(PowerPCCPU *cpu, return NULL; } - hptes = address_space_map(CPU(cpu)->as, base + pte_offset, &plen, false); + hptes = address_space_map(CPU(cpu)->as, base + pte_offset, &plen, false, + MEMTXATTRS_UNSPECIFIED); if (plen < (n * HASH_PTE_SIZE_64)) { hw_error("%s: Unable to map all requested HPTEs\n", __func__); } -- 2.17.1 As part of plumbing MemTxAttrs down to the IOMMU translate method, add MemTxAttrs as an argument to address_space_access_valid(). Its callers either have an attrs value to hand, or don't care and can use MEMTXATTRS_UNSPECIFIED. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20180521140402.23318-6-peter.maydell@linaro.org --- include/exec/memory.h | 4 +++- include/sysemu/dma.h | 3 ++- exec.c | 3 ++- target/s390x/diag.c | 6 ++++-- target/s390x/excp_helper.c | 3 ++- target/s390x/mmu_helper.c | 3 ++- target/s390x/sigp.c | 3 ++- 7 files changed, 17 insertions(+), 8 deletions(-) diff --git a/include/exec/memory.h b/include/exec/memory.h index XXXXXXX..XXXXXXX 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -XXX,XX +XXX,XX @@ static inline MemoryRegion *address_space_translate(AddressSpace *as, * @addr: address within that address space * @len: length of the area to be checked * @is_write: indicates the transfer direction + * @attrs: memory attributes */ -bool address_space_access_valid(AddressSpace *as, hwaddr addr, int len, bool is_write); +bool address_space_access_valid(AddressSpace *as, hwaddr addr, int len, + bool is_write, MemTxAttrs attrs); /* address_space_map: map a physical memory region into a host virtual address * diff --git a/include/sysemu/dma.h b/include/sysemu/dma.h index XXXXXXX..XXXXXXX 100644 --- a/include/sysemu/dma.h +++ b/include/sysemu/dma.h @@ -XXX,XX +XXX,XX @@ static inline bool dma_memory_valid(AddressSpace *as, DMADirection dir) { return address_space_access_valid(as, addr, len, - dir == DMA_DIRECTION_FROM_DEVICE); + dir == DMA_DIRECTION_FROM_DEVICE, + MEMTXATTRS_UNSPECIFIED); } static inline int dma_memory_rw_relaxed(AddressSpace *as, dma_addr_t addr, diff --git a/exec.c b/exec.c index XXXXXXX..XXXXXXX 100644 --- a/exec.c +++ b/exec.c @@ -XXX,XX +XXX,XX @@ static bool flatview_access_valid(FlatView *fv, hwaddr addr, int len, } bool address_space_access_valid(AddressSpace *as, hwaddr addr, - int len, bool is_write) + int len, bool is_write, + MemTxAttrs attrs) { FlatView *fv; bool result; diff --git a/target/s390x/diag.c b/target/s390x/diag.c index XXXXXXX..XXXXXXX 100644 --- a/target/s390x/diag.c +++ b/target/s390x/diag.c @@ -XXX,XX +XXX,XX @@ void handle_diag_308(CPUS390XState *env, uint64_t r1, uint64_t r3, uintptr_t ra) return; } if (!address_space_access_valid(&address_space_memory, addr, - sizeof(IplParameterBlock), false)) { + sizeof(IplParameterBlock), false, + MEMTXATTRS_UNSPECIFIED)) { s390_program_interrupt(env, PGM_ADDRESSING, ILEN_AUTO, ra); return; } @@ -XXX,XX +XXX,XX @@ out: return; } if (!address_space_access_valid(&address_space_memory, addr, - sizeof(IplParameterBlock), true)) { + sizeof(IplParameterBlock), true, + MEMTXATTRS_UNSPECIFIED)) { s390_program_interrupt(env, PGM_ADDRESSING, ILEN_AUTO, ra); return; } diff --git a/target/s390x/excp_helper.c b/target/s390x/excp_helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/s390x/excp_helper.c +++ b/target/s390x/excp_helper.c @@ -XXX,XX +XXX,XX @@ int s390_cpu_handle_mmu_fault(CPUState *cs, vaddr orig_vaddr, int size, /* check out of RAM access */ if (!address_space_access_valid(&address_space_memory, raddr, - TARGET_PAGE_SIZE, rw)) { + TARGET_PAGE_SIZE, rw, + MEMTXATTRS_UNSPECIFIED)) { DPRINTF("%s: raddr %" PRIx64 " > ram_size %" PRIx64 "\n", __func__, (uint64_t)raddr, (uint64_t)ram_size); trigger_pgm_exception(env, PGM_ADDRESSING, ILEN_AUTO); diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/s390x/mmu_helper.c +++ b/target/s390x/mmu_helper.c @@ -XXX,XX +XXX,XX @@ static int translate_pages(S390CPU *cpu, vaddr addr, int nr_pages, return ret; } if (!address_space_access_valid(&address_space_memory, pages[i], - TARGET_PAGE_SIZE, is_write)) { + TARGET_PAGE_SIZE, is_write, + MEMTXATTRS_UNSPECIFIED)) { trigger_access_exception(env, PGM_ADDRESSING, ILEN_AUTO, 0); return -EFAULT; } diff --git a/target/s390x/sigp.c b/target/s390x/sigp.c index XXXXXXX..XXXXXXX 100644 --- a/target/s390x/sigp.c +++ b/target/s390x/sigp.c @@ -XXX,XX +XXX,XX @@ static void sigp_set_prefix(CPUState *cs, run_on_cpu_data arg) cpu_synchronize_state(cs); if (!address_space_access_valid(&address_space_memory, addr, - sizeof(struct LowCore), false)) { + sizeof(struct LowCore), false, + MEMTXATTRS_UNSPECIFIED)) { set_sigp_status(si, SIGP_STAT_INVALID_PARAMETER); return; } -- 2.17.1 As part of plumbing MemTxAttrs down to the IOMMU translate method, add MemTxAttrs as an argument to flatview_extend_translation(). Its callers either have an attrs value to hand, or don't care and can use MEMTXATTRS_UNSPECIFIED. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20180521140402.23318-7-peter.maydell@linaro.org --- exec.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/exec.c b/exec.c index XXXXXXX..XXXXXXX 100644 --- a/exec.c +++ b/exec.c @@ -XXX,XX +XXX,XX @@ bool address_space_access_valid(AddressSpace *as, hwaddr addr, static hwaddr flatview_extend_translation(FlatView *fv, hwaddr addr, - hwaddr target_len, - MemoryRegion *mr, hwaddr base, hwaddr len, - bool is_write) + hwaddr target_len, + MemoryRegion *mr, hwaddr base, hwaddr len, + bool is_write, MemTxAttrs attrs) { hwaddr done = 0; hwaddr xlat; @@ -XXX,XX +XXX,XX @@ void *address_space_map(AddressSpace *as, memory_region_ref(mr); *plen = flatview_extend_translation(fv, addr, len, mr, xlat, - l, is_write); + l, is_write, attrs); ptr = qemu_ram_ptr_length(mr->ram_block, xlat, plen, true); rcu_read_unlock(); @@ -XXX,XX +XXX,XX @@ int64_t address_space_cache_init(MemoryRegionCache *cache, mr = cache->mrs.mr; memory_region_ref(mr); if (memory_access_is_direct(mr, is_write)) { + /* We don't care about the memory attributes here as we're only + * doing this if we found actual RAM, which behaves the same + * regardless of attributes; so UNSPECIFIED is fine. + */ l = flatview_extend_translation(cache->fv, addr, len, mr, - cache->xlat, l, is_write); + cache->xlat, l, is_write, + MEMTXATTRS_UNSPECIFIED); cache->ptr = qemu_ram_ptr_length(mr->ram_block, cache->xlat, &l, true); } else { cache->ptr = NULL; -- 2.17.1 As part of plumbing MemTxAttrs down to the IOMMU translate method, add MemTxAttrs as an argument to memory_region_access_valid(). Its callers either have an attrs value to hand, or don't care and can use MEMTXATTRS_UNSPECIFIED. The callsite in flatview_access_valid() is part of a recursive loop flatview_access_valid() -> memory_region_access_valid() -> subpage_accepts() -> flatview_access_valid(); we make it pass MEMTXATTRS_UNSPECIFIED for now, until the next several commits have plumbed an attrs parameter through the rest of the loop and we can add an attrs parameter to flatview_access_valid(). Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20180521140402.23318-8-peter.maydell@linaro.org --- include/exec/memory-internal.h | 3 ++- exec.c | 4 +++- hw/s390x/s390-pci-inst.c | 3 ++- memory.c | 7 ++++--- 4 files changed, 11 insertions(+), 6 deletions(-) diff --git a/include/exec/memory-internal.h b/include/exec/memory-internal.h index XXXXXXX..XXXXXXX 100644 --- a/include/exec/memory-internal.h +++ b/include/exec/memory-internal.h @@ -XXX,XX +XXX,XX @@ void flatview_unref(FlatView *view); extern const MemoryRegionOps unassigned_mem_ops; bool memory_region_access_valid(MemoryRegion *mr, hwaddr addr, - unsigned size, bool is_write); + unsigned size, bool is_write, + MemTxAttrs attrs); void flatview_add_to_dispatch(FlatView *fv, MemoryRegionSection *section); AddressSpaceDispatch *address_space_dispatch_new(FlatView *fv); diff --git a/exec.c b/exec.c index XXXXXXX..XXXXXXX 100644 --- a/exec.c +++ b/exec.c @@ -XXX,XX +XXX,XX @@ static bool flatview_access_valid(FlatView *fv, hwaddr addr, int len, mr = flatview_translate(fv, addr, &xlat, &l, is_write); if (!memory_access_is_direct(mr, is_write)) { l = memory_access_size(mr, l, addr); - if (!memory_region_access_valid(mr, xlat, l, is_write)) { + /* When our callers all have attrs we'll pass them through here */ + if (!memory_region_access_valid(mr, xlat, l, is_write, + MEMTXATTRS_UNSPECIFIED)) { return false; } } diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c index XXXXXXX..XXXXXXX 100644 --- a/hw/s390x/s390-pci-inst.c +++ b/hw/s390x/s390-pci-inst.c @@ -XXX,XX +XXX,XX @@ int pcistb_service_call(S390CPU *cpu, uint8_t r1, uint8_t r3, uint64_t gaddr, mr = s390_get_subregion(mr, offset, len); offset -= mr->addr; - if (!memory_region_access_valid(mr, offset, len, true)) { + if (!memory_region_access_valid(mr, offset, len, true, + MEMTXATTRS_UNSPECIFIED)) { s390_program_interrupt(env, PGM_OPERAND, 6, ra); return 0; } diff --git a/memory.c b/memory.c index XXXXXXX..XXXXXXX 100644 --- a/memory.c +++ b/memory.c @@ -XXX,XX +XXX,XX @@ static const MemoryRegionOps ram_device_mem_ops = { bool memory_region_access_valid(MemoryRegion *mr, hwaddr addr, unsigned size, - bool is_write) + bool is_write, + MemTxAttrs attrs) { int access_size_min, access_size_max; int access_size, i; @@ -XXX,XX +XXX,XX @@ MemTxResult memory_region_dispatch_read(MemoryRegion *mr, { MemTxResult r; - if (!memory_region_access_valid(mr, addr, size, false)) { + if (!memory_region_access_valid(mr, addr, size, false, attrs)) { *pval = unassigned_mem_read(mr, addr, size); return MEMTX_DECODE_ERROR; } @@ -XXX,XX +XXX,XX @@ MemTxResult memory_region_dispatch_write(MemoryRegion *mr, unsigned size, MemTxAttrs attrs) { - if (!memory_region_access_valid(mr, addr, size, true)) { + if (!memory_region_access_valid(mr, addr, size, true, attrs)) { unassigned_mem_write(mr, addr, data, size); return MEMTX_DECODE_ERROR; } -- 2.17.1 As part of plumbing MemTxAttrs down to the IOMMU translate method, add MemTxAttrs as an argument to the MemoryRegion valid.accepts callback. We'll need this for subpage_accepts(). We could take the approach we used with the read and write callbacks and add new a new _with_attrs version, but since there are so few implementations of the accepts hook we just change them all. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20180521140402.23318-9-peter.maydell@linaro.org --- include/exec/memory.h | 3 ++- exec.c | 9 ++++++--- hw/hppa/dino.c | 3 ++- hw/nvram/fw_cfg.c | 12 ++++++++---- hw/scsi/esp.c | 3 ++- hw/xen/xen_pt_msi.c | 3 ++- memory.c | 5 +++-- 7 files changed, 25 insertions(+), 13 deletions(-) diff --git a/include/exec/memory.h b/include/exec/memory.h index XXXXXXX..XXXXXXX 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -XXX,XX +XXX,XX @@ struct MemoryRegionOps { * as a machine check exception). */ bool (*accepts)(void *opaque, hwaddr addr, - unsigned size, bool is_write); + unsigned size, bool is_write, + MemTxAttrs attrs); } valid; /* Internal implementation constraints: */ struct { diff --git a/exec.c b/exec.c index XXXXXXX..XXXXXXX 100644 --- a/exec.c +++ b/exec.c @@ -XXX,XX +XXX,XX @@ static void notdirty_mem_write(void *opaque, hwaddr ram_addr, } static bool notdirty_mem_accepts(void *opaque, hwaddr addr, - unsigned size, bool is_write) + unsigned size, bool is_write, + MemTxAttrs attrs) { return is_write; } @@ -XXX,XX +XXX,XX @@ static MemTxResult subpage_write(void *opaque, hwaddr addr, } static bool subpage_accepts(void *opaque, hwaddr addr, - unsigned len, bool is_write) + unsigned len, bool is_write, + MemTxAttrs attrs) { subpage_t *subpage = opaque; #if defined(DEBUG_SUBPAGE) @@ -XXX,XX +XXX,XX @@ static void readonly_mem_write(void *opaque, hwaddr addr, } static bool readonly_mem_accepts(void *opaque, hwaddr addr, - unsigned size, bool is_write) + unsigned size, bool is_write, + MemTxAttrs attrs) { return is_write; } diff --git a/hw/hppa/dino.c b/hw/hppa/dino.c index XXXXXXX..XXXXXXX 100644 --- a/hw/hppa/dino.c +++ b/hw/hppa/dino.c @@ -XXX,XX +XXX,XX @@ static void gsc_to_pci_forwarding(DinoState *s) } static bool dino_chip_mem_valid(void *opaque, hwaddr addr, - unsigned size, bool is_write) + unsigned size, bool is_write, + MemTxAttrs attrs) { switch (addr) { case DINO_IAR0: diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c index XXXXXXX..XXXXXXX 100644 --- a/hw/nvram/fw_cfg.c +++ b/hw/nvram/fw_cfg.c @@ -XXX,XX +XXX,XX @@ static void fw_cfg_dma_mem_write(void *opaque, hwaddr addr, } static bool fw_cfg_dma_mem_valid(void *opaque, hwaddr addr, - unsigned size, bool is_write) + unsigned size, bool is_write, + MemTxAttrs attrs) { return !is_write || ((size == 4 && (addr == 0 || addr == 4)) || (size == 8 && addr == 0)); } static bool fw_cfg_data_mem_valid(void *opaque, hwaddr addr, - unsigned size, bool is_write) + unsigned size, bool is_write, + MemTxAttrs attrs) { return addr == 0; } @@ -XXX,XX +XXX,XX @@ static void fw_cfg_ctl_mem_write(void *opaque, hwaddr addr, } static bool fw_cfg_ctl_mem_valid(void *opaque, hwaddr addr, - unsigned size, bool is_write) + unsigned size, bool is_write, + MemTxAttrs attrs) { return is_write && size == 2; } @@ -XXX,XX +XXX,XX @@ static void fw_cfg_comb_write(void *opaque, hwaddr addr, } static bool fw_cfg_comb_valid(void *opaque, hwaddr addr, - unsigned size, bool is_write) + unsigned size, bool is_write, + MemTxAttrs attrs) { return (size == 1) || (is_write && size == 2); } diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c index XXXXXXX..XXXXXXX 100644 --- a/hw/scsi/esp.c +++ b/hw/scsi/esp.c @@ -XXX,XX +XXX,XX @@ void esp_reg_write(ESPState *s, uint32_t saddr, uint64_t val) } static bool esp_mem_accepts(void *opaque, hwaddr addr, - unsigned size, bool is_write) + unsigned size, bool is_write, + MemTxAttrs attrs) { return (size == 1) || (is_write && size == 4); } diff --git a/hw/xen/xen_pt_msi.c b/hw/xen/xen_pt_msi.c index XXXXXXX..XXXXXXX 100644 --- a/hw/xen/xen_pt_msi.c +++ b/hw/xen/xen_pt_msi.c @@ -XXX,XX +XXX,XX @@ static uint64_t pci_msix_read(void *opaque, hwaddr addr, } static bool pci_msix_accepts(void *opaque, hwaddr addr, - unsigned size, bool is_write) + unsigned size, bool is_write, + MemTxAttrs attrs) { return !(addr & (size - 1)); } diff --git a/memory.c b/memory.c index XXXXXXX..XXXXXXX 100644 --- a/memory.c +++ b/memory.c @@ -XXX,XX +XXX,XX @@ static void unassigned_mem_write(void *opaque, hwaddr addr, } static bool unassigned_mem_accepts(void *opaque, hwaddr addr, - unsigned size, bool is_write) + unsigned size, bool is_write, + MemTxAttrs attrs) { return false; } @@ -XXX,XX +XXX,XX @@ bool memory_region_access_valid(MemoryRegion *mr, access_size = MAX(MIN(size, access_size_max), access_size_min); for (i = 0; i < size; i += access_size) { if (!mr->ops->valid.accepts(mr->opaque, addr + i, access_size, - is_write)) { + is_write, attrs)) { return false; } } -- 2.17.1 As part of plumbing MemTxAttrs down to the IOMMU translate method, add MemTxAttrs as an argument to flatview_access_valid(). Its callers now all have an attrs value to hand, so we can correct our earlier temporary use of MEMTXATTRS_UNSPECIFIED. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20180521140402.23318-10-peter.maydell@linaro.org --- exec.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/exec.c b/exec.c index XXXXXXX..XXXXXXX 100644 --- a/exec.c +++ b/exec.c @@ -XXX,XX +XXX,XX @@ static MemTxResult flatview_read(FlatView *fv, hwaddr addr, static MemTxResult flatview_write(FlatView *fv, hwaddr addr, MemTxAttrs attrs, const uint8_t *buf, int len); static bool flatview_access_valid(FlatView *fv, hwaddr addr, int len, - bool is_write); + bool is_write, MemTxAttrs attrs); static MemTxResult subpage_read(void *opaque, hwaddr addr, uint64_t *data, unsigned len, MemTxAttrs attrs) @@ -XXX,XX +XXX,XX @@ static bool subpage_accepts(void *opaque, hwaddr addr, #endif return flatview_access_valid(subpage->fv, addr + subpage->base, - len, is_write); + len, is_write, attrs); } static const MemoryRegionOps subpage_ops = { @@ -XXX,XX +XXX,XX @@ static void cpu_notify_map_clients(void) } static bool flatview_access_valid(FlatView *fv, hwaddr addr, int len, - bool is_write) + bool is_write, MemTxAttrs attrs) { MemoryRegion *mr; hwaddr l, xlat; @@ -XXX,XX +XXX,XX @@ static bool flatview_access_valid(FlatView *fv, hwaddr addr, int len, mr = flatview_translate(fv, addr, &xlat, &l, is_write); if (!memory_access_is_direct(mr, is_write)) { l = memory_access_size(mr, l, addr); - /* When our callers all have attrs we'll pass them through here */ - if (!memory_region_access_valid(mr, xlat, l, is_write, - MEMTXATTRS_UNSPECIFIED)) { + if (!memory_region_access_valid(mr, xlat, l, is_write, attrs)) { return false; } } @@ -XXX,XX +XXX,XX @@ bool address_space_access_valid(AddressSpace *as, hwaddr addr, rcu_read_lock(); fv = address_space_to_flatview(as); - result = flatview_access_valid(fv, addr, len, is_write); + result = flatview_access_valid(fv, addr, len, is_write, attrs); rcu_read_unlock(); return result; } -- 2.17.1 As part of plumbing MemTxAttrs down to the IOMMU translate method, add MemTxAttrs as an argument to flatview_translate(); all its callers now have attrs available. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20180521140402.23318-11-peter.maydell@linaro.org --- include/exec/memory.h | 7 ++++--- exec.c | 17 +++++++++-------- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/include/exec/memory.h b/include/exec/memory.h index XXXXXXX..XXXXXXX 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -XXX,XX +XXX,XX @@ IOMMUTLBEntry address_space_get_iotlb_entry(AddressSpace *as, hwaddr addr, */ MemoryRegion *flatview_translate(FlatView *fv, hwaddr addr, hwaddr *xlat, - hwaddr *len, bool is_write); + hwaddr *len, bool is_write, + MemTxAttrs attrs); static inline MemoryRegion *address_space_translate(AddressSpace *as, hwaddr addr, hwaddr *xlat, @@ -XXX,XX +XXX,XX @@ static inline MemoryRegion *address_space_translate(AddressSpace *as, MemTxAttrs attrs) { return flatview_translate(address_space_to_flatview(as), - addr, xlat, len, is_write); + addr, xlat, len, is_write, attrs); } /* address_space_access_valid: check for validity of accessing an address @@ -XXX,XX +XXX,XX @@ MemTxResult address_space_read(AddressSpace *as, hwaddr addr, rcu_read_lock(); fv = address_space_to_flatview(as); l = len; - mr = flatview_translate(fv, addr, &addr1, &l, false); + mr = flatview_translate(fv, addr, &addr1, &l, false, attrs); if (len == l && memory_access_is_direct(mr, false)) { ptr = qemu_map_ram_ptr(mr->ram_block, addr1); memcpy(buf, ptr, len); diff --git a/exec.c b/exec.c index XXXXXXX..XXXXXXX 100644 --- a/exec.c +++ b/exec.c @@ -XXX,XX +XXX,XX @@ iotlb_fail: /* Called from RCU critical section */ MemoryRegion *flatview_translate(FlatView *fv, hwaddr addr, hwaddr *xlat, - hwaddr *plen, bool is_write) + hwaddr *plen, bool is_write, + MemTxAttrs attrs) { MemoryRegion *mr; MemoryRegionSection section; @@ -XXX,XX +XXX,XX @@ static MemTxResult flatview_write_continue(FlatView *fv, hwaddr addr, } l = len; - mr = flatview_translate(fv, addr, &addr1, &l, true); + mr = flatview_translate(fv, addr, &addr1, &l, true, attrs); } return result; @@ -XXX,XX +XXX,XX @@ static MemTxResult flatview_write(FlatView *fv, hwaddr addr, MemTxAttrs attrs, MemTxResult result = MEMTX_OK; l = len; - mr = flatview_translate(fv, addr, &addr1, &l, true); + mr = flatview_translate(fv, addr, &addr1, &l, true, attrs); result = flatview_write_continue(fv, addr, attrs, buf, len, addr1, l, mr); @@ -XXX,XX +XXX,XX @@ MemTxResult flatview_read_continue(FlatView *fv, hwaddr addr, } l = len; - mr = flatview_translate(fv, addr, &addr1, &l, false); + mr = flatview_translate(fv, addr, &addr1, &l, false, attrs); } return result; @@ -XXX,XX +XXX,XX @@ static MemTxResult flatview_read(FlatView *fv, hwaddr addr, MemoryRegion *mr; l = len; - mr = flatview_translate(fv, addr, &addr1, &l, false); + mr = flatview_translate(fv, addr, &addr1, &l, false, attrs); return flatview_read_continue(fv, addr, attrs, buf, len, addr1, l, mr); } @@ -XXX,XX +XXX,XX @@ static bool flatview_access_valid(FlatView *fv, hwaddr addr, int len, while (len > 0) { l = len; - mr = flatview_translate(fv, addr, &xlat, &l, is_write); + mr = flatview_translate(fv, addr, &xlat, &l, is_write, attrs); if (!memory_access_is_direct(mr, is_write)) { l = memory_access_size(mr, l, addr); if (!memory_region_access_valid(mr, xlat, l, is_write, attrs)) { @@ -XXX,XX +XXX,XX @@ flatview_extend_translation(FlatView *fv, hwaddr addr, len = target_len; this_mr = flatview_translate(fv, addr, &xlat, - &len, is_write); + &len, is_write, attrs); if (this_mr != mr || xlat != base + done) { return done; } @@ -XXX,XX +XXX,XX @@ void *address_space_map(AddressSpace *as, l = len; rcu_read_lock(); fv = address_space_to_flatview(as); - mr = flatview_translate(fv, addr, &xlat, &l, is_write); + mr = flatview_translate(fv, addr, &xlat, &l, is_write, attrs); if (!memory_access_is_direct(mr, is_write)) { if (atomic_xchg(&bounce.in_use, true)) { -- 2.17.1 As part of plumbing MemTxAttrs down to the IOMMU translate method, add MemTxAttrs as an argument to address_space_get_iotlb_entry(). Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20180521140402.23318-12-peter.maydell@linaro.org --- include/exec/memory.h | 2 +- exec.c | 2 +- hw/virtio/vhost.c | 3 ++- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/include/exec/memory.h b/include/exec/memory.h index XXXXXXX..XXXXXXX 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -XXX,XX +XXX,XX @@ void address_space_cache_destroy(MemoryRegionCache *cache); * entry. Should be called from an RCU critical section. */ IOMMUTLBEntry address_space_get_iotlb_entry(AddressSpace *as, hwaddr addr, - bool is_write); + bool is_write, MemTxAttrs attrs); /* address_space_translate: translate an address range into an address space * into a MemoryRegion and an address range into that section. Should be diff --git a/exec.c b/exec.c index XXXXXXX..XXXXXXX 100644 --- a/exec.c +++ b/exec.c @@ -XXX,XX +XXX,XX @@ static MemoryRegionSection flatview_do_translate(FlatView *fv, /* Called from RCU critical section */ IOMMUTLBEntry address_space_get_iotlb_entry(AddressSpace *as, hwaddr addr, - bool is_write) + bool is_write, MemTxAttrs attrs) { MemoryRegionSection section; hwaddr xlat, page_mask; diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c index XXXXXXX..XXXXXXX 100644 --- a/hw/virtio/vhost.c +++ b/hw/virtio/vhost.c @@ -XXX,XX +XXX,XX @@ int vhost_device_iotlb_miss(struct vhost_dev *dev, uint64_t iova, int write) trace_vhost_iotlb_miss(dev, 1); iotlb = address_space_get_iotlb_entry(dev->vdev->dma_as, - iova, write); + iova, write, + MEMTXATTRS_UNSPECIFIED); if (iotlb.target_as != NULL) { ret = vhost_memory_region_lookup(dev, iotlb.translated_addr, &uaddr, &len); -- 2.17.1 As part of plumbing MemTxAttrs down to the IOMMU translate method, add MemTxAttrs as an argument to flatview_do_translate(). Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20180521140402.23318-13-peter.maydell@linaro.org --- exec.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/exec.c b/exec.c index XXXXXXX..XXXXXXX 100644 --- a/exec.c +++ b/exec.c @@ -XXX,XX +XXX,XX @@ unassigned: * @is_write: whether the translation operation is for write * @is_mmio: whether this can be MMIO, set true if it can * @target_as: the address space targeted by the IOMMU + * @attrs: memory transaction attributes * * This function is called from RCU critical section */ @@ -XXX,XX +XXX,XX @@ static MemoryRegionSection flatview_do_translate(FlatView *fv, hwaddr *page_mask_out, bool is_write, bool is_mmio, - AddressSpace **target_as) + AddressSpace **target_as, + MemTxAttrs attrs) { MemoryRegionSection *section; IOMMUMemoryRegion *iommu_mr; @@ -XXX,XX +XXX,XX @@ IOMMUTLBEntry address_space_get_iotlb_entry(AddressSpace *as, hwaddr addr, * but page mask. */ section = flatview_do_translate(address_space_to_flatview(as), addr, &xlat, - NULL, &page_mask, is_write, false, &as); + NULL, &page_mask, is_write, false, &as, + attrs); /* Illegal translation */ if (section.mr == &io_mem_unassigned) { @@ -XXX,XX +XXX,XX @@ MemoryRegion *flatview_translate(FlatView *fv, hwaddr addr, hwaddr *xlat, /* This can be MMIO, so setup MMIO bit. */ section = flatview_do_translate(fv, addr, xlat, plen, NULL, - is_write, true, &as); + is_write, true, &as, attrs); mr = section.mr; if (xen_enabled() && memory_access_is_direct(mr, is_write)) { -- 2.17.1 As part of plumbing MemTxAttrs down to the IOMMU translate method, add MemTxAttrs as an argument to address_space_translate_iommu(). Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20180521140402.23318-14-peter.maydell@linaro.org --- exec.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/exec.c b/exec.c index XXXXXXX..XXXXXXX 100644 --- a/exec.c +++ b/exec.c @@ -XXX,XX +XXX,XX @@ address_space_translate_internal(AddressSpaceDispatch *d, hwaddr addr, hwaddr *x * @is_write: whether the translation operation is for write * @is_mmio: whether this can be MMIO, set true if it can * @target_as: the address space targeted by the IOMMU + * @attrs: transaction attributes * * This function is called from RCU critical section. It is the common * part of flatview_do_translate and address_space_translate_cached. @@ -XXX,XX +XXX,XX @@ static MemoryRegionSection address_space_translate_iommu(IOMMUMemoryRegion *iomm hwaddr *page_mask_out, bool is_write, bool is_mmio, - AddressSpace **target_as) + AddressSpace **target_as, + MemTxAttrs attrs) { MemoryRegionSection *section; hwaddr page_mask = (hwaddr)-1; @@ -XXX,XX +XXX,XX @@ static MemoryRegionSection flatview_do_translate(FlatView *fv, return address_space_translate_iommu(iommu_mr, xlat, plen_out, page_mask_out, is_write, is_mmio, - target_as); + target_as, attrs); } if (page_mask_out) { /* Not behind an IOMMU, use default page size. */ @@ -XXX,XX +XXX,XX @@ static inline MemoryRegion *address_space_translate_cached( section = address_space_translate_iommu(iommu_mr, xlat, plen, NULL, is_write, true, - &target_as); + &target_as, attrs); return section.mr; } -- 2.17.1 Provide a VMSTATE_BOOL_SUB_ARRAY to go with VMSTATE_UINT8_SUB_ARRAY and friends. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Message-id: 20180521140402.23318-23-peter.maydell@linaro.org --- include/migration/vmstate.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h index XXXXXXX..XXXXXXX 100644 --- a/include/migration/vmstate.h +++ b/include/migration/vmstate.h @@ -XXX,XX +XXX,XX @@ extern const VMStateInfo vmstate_info_qtailq; #define VMSTATE_BOOL_ARRAY(_f, _s, _n) \ VMSTATE_BOOL_ARRAY_V(_f, _s, _n, 0) +#define VMSTATE_BOOL_SUB_ARRAY(_f, _s, _start, _num) \ + VMSTATE_SUB_ARRAY(_f, _s, _start, _num, 0, vmstate_info_bool, bool) + #define VMSTATE_UINT16_ARRAY_V(_f, _s, _n, _v) \ VMSTATE_ARRAY(_f, _s, _n, _v, vmstate_info_uint16, uint16_t) -- 2.17.1 From: Shannon Zhao <zhaoshenglong@huawei.com> acpi_data_push uses g_array_set_size to resize the memory size. If there is no enough contiguous memory, the address will be changed. So previous pointer could not be used any more. It must update the pointer and use the new one. Also, previous codes wrongly use le32 conversion of iort->node_offset for subsequent computations that will result incorrect value if host is not litlle endian. So use the non-converted one instead. Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com> Reviewed-by: Eric Auger <eric.auger@redhat.com> Message-id: 1527663951-14552-1-git-send-email-zhaoshenglong@huawei.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/virt-acpi-build.c | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -XXX,XX +XXX,XX @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) AcpiIortItsGroup *its; AcpiIortTable *iort; AcpiIortSmmu3 *smmu; - size_t node_size, iort_length, smmu_offset = 0; + size_t node_size, iort_node_offset, iort_length, smmu_offset = 0; AcpiIortRC *rc; iort = acpi_data_push(table_data, sizeof(*iort)); @@ -XXX,XX +XXX,XX @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) iort_length = sizeof(*iort); iort->node_count = cpu_to_le32(nb_nodes); - iort->node_offset = cpu_to_le32(sizeof(*iort)); + /* + * Use a copy in case table_data->data moves during acpi_data_push + * operations. + */ + iort_node_offset = sizeof(*iort); + iort->node_offset = cpu_to_le32(iort_node_offset); /* ITS group node */ node_size = sizeof(*its) + sizeof(uint32_t); @@ -XXX,XX +XXX,XX @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) int irq = vms->irqmap[VIRT_SMMU]; /* SMMUv3 node */ - smmu_offset = iort->node_offset + node_size; + smmu_offset = iort_node_offset + node_size; node_size = sizeof(*smmu) + sizeof(*idmap); iort_length += node_size; smmu = acpi_data_push(table_data, node_size); @@ -XXX,XX +XXX,XX @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) idmap->id_count = cpu_to_le32(0xFFFF); idmap->output_base = 0; /* output IORT node is the ITS group node (the first node) */ - idmap->output_reference = cpu_to_le32(iort->node_offset); + idmap->output_reference = cpu_to_le32(iort_node_offset); } /* Root Complex Node */ @@ -XXX,XX +XXX,XX @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) idmap->output_reference = cpu_to_le32(smmu_offset); } else { /* output IORT node is the ITS group node (the first node) */ - idmap->output_reference = cpu_to_le32(iort->node_offset); + idmap->output_reference = cpu_to_le32(iort_node_offset); } + /* + * Update the pointer address in case table_data->data moves during above + * acpi_data_push operations. + */ + iort = (AcpiIortTable *)(table_data->data + iort_start); iort->length = cpu_to_le32(iort_length); build_header(linker, table_data, (void *)(table_data->data + iort_start), -- 2.17.1 From: Shannon Zhao <zhaoshenglong@huawei.com> kvm_irqchip_create called by kvm_init will call kvm_init_irq_routing to initialize global capability variables. If we call kvm_init_irq_routing in GIC realize function, previous allocated memory will leak. Fix this by deleting the unnecessary call. Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com> Reviewed-by: Eric Auger <eric.auger@redhat.com> Message-id: 1527750994-14360-1-git-send-email-zhaoshenglong@huawei.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/intc/arm_gic_kvm.c | 1 - hw/intc/arm_gicv3_kvm.c | 1 - 2 files changed, 2 deletions(-) diff --git a/hw/intc/arm_gic_kvm.c b/hw/intc/arm_gic_kvm.c index XXXXXXX..XXXXXXX 100644 --- a/hw/intc/arm_gic_kvm.c +++ b/hw/intc/arm_gic_kvm.c @@ -XXX,XX +XXX,XX @@ static void kvm_arm_gic_realize(DeviceState *dev, Error **errp) if (kvm_has_gsi_routing()) { /* set up irq routing */ - kvm_init_irq_routing(kvm_state); for (i = 0; i < s->num_irq - GIC_INTERNAL; ++i) { kvm_irqchip_add_irq_route(kvm_state, i, 0, i); } diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c index XXXXXXX..XXXXXXX 100644 --- a/hw/intc/arm_gicv3_kvm.c +++ b/hw/intc/arm_gicv3_kvm.c @@ -XXX,XX +XXX,XX @@ static void kvm_arm_gicv3_realize(DeviceState *dev, Error **errp) if (kvm_has_gsi_routing()) { /* set up irq routing */ - kvm_init_irq_routing(kvm_state); for (i = 0; i < s->num_irq - GIC_INTERNAL; ++i) { kvm_irqchip_add_irq_route(kvm_state, i, 0, i); } -- 2.17.1 Hi; here's the latest arm pullreq. This is mostly patches from RTH, plus a couple of other more minor things. Switching to PCREL is the big one, hopefully should improve performance. thanks -- PMM The following changes since commit 214a8da23651f2472b296b3293e619fd58d9e212: Merge tag 'for-upstream' of https://gitlab.com/bonzini/qemu into staging (2022-10-18 11:14:31 -0400) are available in the Git repository at: https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20221020 for you to fetch changes up to 5db899303799e49209016a93289b8694afa1449e: hw/ide/microdrive: Use device_cold_reset() for self-resets (2022-10-20 12:11:53 +0100) ---------------------------------------------------------------- target-arm queue: * Switch to TARGET_TB_PCREL * More pagetable-walk refactoring preparatory to HAFDBS * update the cortex-a15 MIDR to latest rev * hw/char/pl011: fix baud rate calculation * hw/ide/microdrive: Use device_cold_reset() for self-resets ---------------------------------------------------------------- Alex Bennée (1): target/arm: update the cortex-a15 MIDR to latest rev Baruch Siach (1): hw/char/pl011: fix baud rate calculation Peter Maydell (1): hw/ide/microdrive: Use device_cold_reset() for self-resets Richard Henderson (21): target/arm: Enable TARGET_PAGE_ENTRY_EXTRA target/arm: Use probe_access_full for MTE target/arm: Use probe_access_full for BTI target/arm: Add ARMMMUIdx_Phys_{S,NS} target/arm: Move ARMMMUIdx_Stage2 to a real tlb mmu_idx target/arm: Restrict tlb flush from vttbr_write to vmid change target/arm: Split out S1Translate type target/arm: Plumb debug into S1Translate target/arm: Move be test for regime into S1TranslateResult target/arm: Use softmmu tlbs for page table walking target/arm: Split out get_phys_addr_twostage target/arm: Use bool consistently for get_phys_addr subroutines target/arm: Introduce curr_insn_len target/arm: Change gen_goto_tb to work on displacements target/arm: Change gen_*set_pc_im to gen_*update_pc target/arm: Change gen_exception_insn* to work on displacements target/arm: Remove gen_exception_internal_insn pc argument target/arm: Change gen_jmp* to work on displacements target/arm: Introduce gen_pc_plus_diff for aarch64 target/arm: Introduce gen_pc_plus_diff for aarch32 target/arm: Enable TARGET_TB_PCREL target/arm/cpu-param.h | 17 +- target/arm/cpu.h | 47 ++-- target/arm/internals.h | 1 + target/arm/sve_ldst_internal.h | 1 + target/arm/translate-a32.h | 2 +- target/arm/translate.h | 66 ++++- hw/char/pl011.c | 2 +- hw/ide/microdrive.c | 8 +- target/arm/cpu.c | 23 +- target/arm/cpu_tcg.c | 4 +- target/arm/helper.c | 155 +++++++++--- target/arm/mte_helper.c | 62 ++--- target/arm/ptw.c | 535 +++++++++++++++++++++++++---------------- target/arm/sve_helper.c | 54 ++--- target/arm/tlb_helper.c | 24 +- target/arm/translate-a64.c | 220 ++++++++++------- target/arm/translate-m-nocp.c | 8 +- target/arm/translate-mve.c | 2 +- target/arm/translate-vfp.c | 10 +- target/arm/translate.c | 284 +++++++++++++--------- 20 files changed, 918 insertions(+), 607 deletions(-) From: Baruch Siach <baruch@tkos.co.il> The PL011 TRM says that "UARTIBRD = 0 is invalid and UARTFBRD is ignored when this is the case". But the code looks at FBRD for the invalid case. Fix this. Signed-off-by: Baruch Siach <baruch@tkos.co.il> Message-id: 1408f62a2e45665816527d4845ffde650957d5ab.1665051588.git.baruchs-c@neureality.ai Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/char/pl011.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/char/pl011.c b/hw/char/pl011.c index XXXXXXX..XXXXXXX 100644 --- a/hw/char/pl011.c +++ b/hw/char/pl011.c @@ -XXX,XX +XXX,XX @@ static unsigned int pl011_get_baudrate(const PL011State *s) { uint64_t clk; - if (s->fbrd == 0) { + if (s->ibrd == 0) { return 0; } -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> The CPUTLBEntryFull structure now stores the original pte attributes, as well as the physical address. Therefore, we no longer need a separate bit in MemTxAttrs, nor do we need to walk the tree of memory regions. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20221011031911.2408754-3-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/cpu.h | 1 - target/arm/sve_ldst_internal.h | 1 + target/arm/mte_helper.c | 62 ++++++++++------------------------ target/arm/sve_helper.c | 54 ++++++++++------------------- target/arm/tlb_helper.c | 4 --- 5 files changed, 36 insertions(+), 86 deletions(-) diff --git a/target/arm/cpu.h b/target/arm/cpu.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -XXX,XX +XXX,XX @@ static inline MemTxAttrs *typecheck_memtxattrs(MemTxAttrs *x) * generic target bits directly. */ #define arm_tlb_bti_gp(x) (typecheck_memtxattrs(x)->target_tlb_bit0) -#define arm_tlb_mte_tagged(x) (typecheck_memtxattrs(x)->target_tlb_bit1) /* * AArch64 usage of the PAGE_TARGET_* bits for linux-user. diff --git a/target/arm/sve_ldst_internal.h b/target/arm/sve_ldst_internal.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/sve_ldst_internal.h +++ b/target/arm/sve_ldst_internal.h @@ -XXX,XX +XXX,XX @@ typedef struct { void *host; int flags; MemTxAttrs attrs; + bool tagged; } SVEHostPage; bool sve_probe_page(SVEHostPage *info, bool nofault, CPUARMState *env, diff --git a/target/arm/mte_helper.c b/target/arm/mte_helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/mte_helper.c +++ b/target/arm/mte_helper.c @@ -XXX,XX +XXX,XX @@ static uint8_t *allocation_tag_mem(CPUARMState *env, int ptr_mmu_idx, TARGET_PAGE_BITS - LOG2_TAG_GRANULE - 1); return tags + index; #else - uintptr_t index; CPUTLBEntryFull *full; + MemTxAttrs attrs; int in_page, flags; - ram_addr_t ptr_ra; hwaddr ptr_paddr, tag_paddr, xlat; MemoryRegion *mr; ARMASIdx tag_asi; @@ -XXX,XX +XXX,XX @@ static uint8_t *allocation_tag_mem(CPUARMState *env, int ptr_mmu_idx, * valid. Indicate to probe_access_flags no-fault, then assert that * we received a valid page. */ - flags = probe_access_flags(env, ptr, ptr_access, ptr_mmu_idx, - ra == 0, &host, ra); + flags = probe_access_full(env, ptr, ptr_access, ptr_mmu_idx, + ra == 0, &host, &full, ra); assert(!(flags & TLB_INVALID_MASK)); - /* - * Find the CPUTLBEntryFull for ptr. This *must* be present in the TLB - * because we just found the mapping. - * TODO: Perhaps there should be a cputlb helper that returns a - * matching tlb entry + iotlb entry. - */ - index = tlb_index(env, ptr_mmu_idx, ptr); -# ifdef CONFIG_DEBUG_TCG - { - CPUTLBEntry *entry = tlb_entry(env, ptr_mmu_idx, ptr); - target_ulong comparator = (ptr_access == MMU_DATA_LOAD - ? entry->addr_read - : tlb_addr_write(entry)); - g_assert(tlb_hit(comparator, ptr)); - } -# endif - full = &env_tlb(env)->d[ptr_mmu_idx].fulltlb[index]; - /* If the virtual page MemAttr != Tagged, access unchecked. */ - if (!arm_tlb_mte_tagged(&full->attrs)) { + if (full->pte_attrs != 0xf0) { return NULL; } @@ -XXX,XX +XXX,XX @@ static uint8_t *allocation_tag_mem(CPUARMState *env, int ptr_mmu_idx, return NULL; } + /* + * Remember these values across the second lookup below, + * which may invalidate this pointer via tlb resize. + */ + ptr_paddr = full->phys_addr; + attrs = full->attrs; + full = NULL; + /* * The Normal memory access can extend to the next page. E.g. a single * 8-byte access to the last byte of a page will check only the last @@ -XXX,XX +XXX,XX @@ static uint8_t *allocation_tag_mem(CPUARMState *env, int ptr_mmu_idx, */ in_page = -(ptr | TARGET_PAGE_MASK); if (unlikely(ptr_size > in_page)) { - void *ignore; - flags |= probe_access_flags(env, ptr + in_page, ptr_access, - ptr_mmu_idx, ra == 0, &ignore, ra); + flags |= probe_access_full(env, ptr + in_page, ptr_access, + ptr_mmu_idx, ra == 0, &host, &full, ra); assert(!(flags & TLB_INVALID_MASK)); } @@ -XXX,XX +XXX,XX @@ static uint8_t *allocation_tag_mem(CPUARMState *env, int ptr_mmu_idx, if (unlikely(flags & TLB_WATCHPOINT)) { int wp = ptr_access == MMU_DATA_LOAD ? BP_MEM_READ : BP_MEM_WRITE; assert(ra != 0); - cpu_check_watchpoint(env_cpu(env), ptr, ptr_size, - full->attrs, wp, ra); + cpu_check_watchpoint(env_cpu(env), ptr, ptr_size, attrs, wp, ra); } - /* - * Find the physical address within the normal mem space. - * The memory region lookup must succeed because TLB_MMIO was - * not set in the cputlb lookup above. - */ - mr = memory_region_from_host(host, &ptr_ra); - tcg_debug_assert(mr != NULL); - tcg_debug_assert(memory_region_is_ram(mr)); - ptr_paddr = ptr_ra; - do { - ptr_paddr += mr->addr; - mr = mr->container; - } while (mr); - /* Convert to the physical address in tag space. */ tag_paddr = ptr_paddr >> (LOG2_TAG_GRANULE + 1); /* Look up the address in tag space. */ - tag_asi = full->attrs.secure ? ARMASIdx_TagS : ARMASIdx_TagNS; + tag_asi = attrs.secure ? ARMASIdx_TagS : ARMASIdx_TagNS; tag_as = cpu_get_address_space(env_cpu(env), tag_asi); mr = address_space_translate(tag_as, tag_paddr, &xlat, NULL, - tag_access == MMU_DATA_STORE, - full->attrs); + tag_access == MMU_DATA_STORE, attrs); /* * Note that @mr will never be NULL. If there is nothing in the address diff --git a/target/arm/sve_helper.c b/target/arm/sve_helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/sve_helper.c +++ b/target/arm/sve_helper.c @@ -XXX,XX +XXX,XX @@ bool sve_probe_page(SVEHostPage *info, bool nofault, CPUARMState *env, */ addr = useronly_clean_ptr(addr); +#ifdef CONFIG_USER_ONLY flags = probe_access_flags(env, addr, access_type, mmu_idx, nofault, &info->host, retaddr); + memset(&info->attrs, 0, sizeof(info->attrs)); + /* Require both ANON and MTE; see allocation_tag_mem(). */ + info->tagged = (flags & PAGE_ANON) && (flags & PAGE_MTE); +#else + CPUTLBEntryFull *full; + flags = probe_access_full(env, addr, access_type, mmu_idx, nofault, + &info->host, &full, retaddr); + info->attrs = full->attrs; + info->tagged = full->pte_attrs == 0xf0; +#endif info->flags = flags; if (flags & TLB_INVALID_MASK) { @@ -XXX,XX +XXX,XX @@ bool sve_probe_page(SVEHostPage *info, bool nofault, CPUARMState *env, /* Ensure that info->host[] is relative to addr, not addr + mem_off. */ info->host -= mem_off; - -#ifdef CONFIG_USER_ONLY - memset(&info->attrs, 0, sizeof(info->attrs)); - /* Require both MAP_ANON and PROT_MTE -- see allocation_tag_mem. */ - arm_tlb_mte_tagged(&info->attrs) = - (flags & PAGE_ANON) && (flags & PAGE_MTE); -#else - /* - * Find the iotlbentry for addr and return the transaction attributes. - * This *must* be present in the TLB because we just found the mapping. - */ - { - uintptr_t index = tlb_index(env, mmu_idx, addr); - -# ifdef CONFIG_DEBUG_TCG - CPUTLBEntry *entry = tlb_entry(env, mmu_idx, addr); - target_ulong comparator = (access_type == MMU_DATA_LOAD - ? entry->addr_read - : tlb_addr_write(entry)); - g_assert(tlb_hit(comparator, addr)); -# endif - - CPUTLBEntryFull *full = &env_tlb(env)->d[mmu_idx].fulltlb[index]; - info->attrs = full->attrs; - } -#endif - return true; } @@ -XXX,XX +XXX,XX @@ void sve_cont_ldst_mte_check(SVEContLdSt *info, CPUARMState *env, intptr_t mem_off, reg_off, reg_last; /* Process the page only if MemAttr == Tagged. */ - if (arm_tlb_mte_tagged(&info->page[0].attrs)) { + if (info->page[0].tagged) { mem_off = info->mem_off_first[0]; reg_off = info->reg_off_first[0]; reg_last = info->reg_off_split; @@ -XXX,XX +XXX,XX @@ void sve_cont_ldst_mte_check(SVEContLdSt *info, CPUARMState *env, } mem_off = info->mem_off_first[1]; - if (mem_off >= 0 && arm_tlb_mte_tagged(&info->page[1].attrs)) { + if (mem_off >= 0 && info->page[1].tagged) { reg_off = info->reg_off_first[1]; reg_last = info->reg_off_last[1]; @@ -XXX,XX +XXX,XX @@ void sve_ldnfff1_r(CPUARMState *env, void *vg, const target_ulong addr, * Disable MTE checking if the Tagged bit is not set. Since TBI must * be set within MTEDESC for MTE, !mtedesc => !mte_active. */ - if (!arm_tlb_mte_tagged(&info.page[0].attrs)) { + if (!info.page[0].tagged) { mtedesc = 0; } @@ -XXX,XX +XXX,XX @@ void sve_ld1_z(CPUARMState *env, void *vd, uint64_t *vg, void *vm, cpu_check_watchpoint(env_cpu(env), addr, msize, info.attrs, BP_MEM_READ, retaddr); } - if (mtedesc && arm_tlb_mte_tagged(&info.attrs)) { + if (mtedesc && info.tagged) { mte_check(env, mtedesc, addr, retaddr); } if (unlikely(info.flags & TLB_MMIO)) { @@ -XXX,XX +XXX,XX @@ void sve_ld1_z(CPUARMState *env, void *vd, uint64_t *vg, void *vm, msize, info.attrs, BP_MEM_READ, retaddr); } - if (mtedesc && arm_tlb_mte_tagged(&info.attrs)) { + if (mtedesc && info.tagged) { mte_check(env, mtedesc, addr, retaddr); } tlb_fn(env, &scratch, reg_off, addr, retaddr); @@ -XXX,XX +XXX,XX @@ void sve_ldff1_z(CPUARMState *env, void *vd, uint64_t *vg, void *vm, (env_cpu(env), addr, msize) & BP_MEM_READ)) { goto fault; } - if (mtedesc && - arm_tlb_mte_tagged(&info.attrs) && - !mte_probe(env, mtedesc, addr)) { + if (mtedesc && info.tagged && !mte_probe(env, mtedesc, addr)) { goto fault; } @@ -XXX,XX +XXX,XX @@ void sve_st1_z(CPUARMState *env, void *vd, uint64_t *vg, void *vm, info.attrs, BP_MEM_WRITE, retaddr); } - if (mtedesc && arm_tlb_mte_tagged(&info.attrs)) { + if (mtedesc && info.tagged) { mte_check(env, mtedesc, addr, retaddr); } } 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, res.f.phys_addr &= TARGET_PAGE_MASK; address &= TARGET_PAGE_MASK; } - /* Notice and record tagged memory. */ - if (cpu_isar_feature(aa64_mte, cpu) && res.cacheattrs.attrs == 0xf0) { - arm_tlb_mte_tagged(&res.f.attrs) = true; - } res.f.pte_attrs = res.cacheattrs.attrs; res.f.shareability = res.cacheattrs.shareability; -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Add a field to TARGET_PAGE_ENTRY_EXTRA to hold the guarded bit. In is_guarded_page, use probe_access_full instead of just guessing that the tlb entry is still present. Also handles the FIXME about executing from device memory. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20221011031911.2408754-4-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/cpu-param.h | 9 +++++---- target/arm/cpu.h | 13 ------------- target/arm/internals.h | 1 + target/arm/ptw.c | 7 ++++--- target/arm/translate-a64.c | 21 ++++++++++----------- 5 files changed, 20 insertions(+), 31 deletions(-) diff --git a/target/arm/cpu-param.h b/target/arm/cpu-param.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu-param.h +++ b/target/arm/cpu-param.h @@ -XXX,XX +XXX,XX @@ * * For ARMMMUIdx_Stage2*, pte_attrs is the S2 descriptor bits [5:2]. * Otherwise, pte_attrs is the same as the MAIR_EL1 8-bit format. - * For shareability, as in the SH field of the VMSAv8-64 PTEs. + * For shareability and guarded, as in the SH and GP fields respectively + * of the VMSAv8-64 PTEs. */ # define TARGET_PAGE_ENTRY_EXTRA \ - uint8_t pte_attrs; \ - uint8_t shareability; - + uint8_t pte_attrs; \ + uint8_t shareability; \ + bool guarded; #endif #define NB_MMU_MODES 8 diff --git a/target/arm/cpu.h b/target/arm/cpu.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -XXX,XX +XXX,XX @@ static inline uint64_t *aa64_vfp_qreg(CPUARMState *env, unsigned regno) /* Shared between translate-sve.c and sve_helper.c. */ extern const uint64_t pred_esz_masks[5]; -/* Helper for the macros below, validating the argument type. */ -static inline MemTxAttrs *typecheck_memtxattrs(MemTxAttrs *x) -{ - return x; -} - -/* - * Lvalue macros for ARM TLB bits that we must cache in the TCG TLB. - * Using these should be a bit more self-documenting than using the - * generic target bits directly. - */ -#define arm_tlb_bti_gp(x) (typecheck_memtxattrs(x)->target_tlb_bit0) - /* * AArch64 usage of the PAGE_TARGET_* bits for linux-user. * Note that with the Linux kernel, PROT_MTE may not be cleared by mprotect 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 { unsigned int attrs:8; unsigned int shareability:2; /* as in the SH field of the VMSAv8-64 PTEs */ bool is_s2_format:1; + bool guarded:1; /* guarded bit of the v8-64 PTE */ } ARMCacheAttrs; /* Fields that are valid upon success. */ 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, */ result->f.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(&result->f.attrs) = true; + + /* When in aarch64 mode, and BTI is enabled, remember GP in the TLB. */ + if (aarch64 && cpu_isar_feature(aa64_bti, cpu)) { + result->f.guarded = guarded; } if (mmu_idx == ARMMMUIdx_Stage2 || mmu_idx == ARMMMUIdx_Stage2_S) { 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 bool is_guarded_page(CPUARMState *env, DisasContext *s) #ifdef CONFIG_USER_ONLY return page_get_flags(addr) & PAGE_BTI; #else + CPUTLBEntryFull *full; + void *host; int mmu_idx = arm_to_core_mmu_idx(s->mmu_idx); - unsigned int index = tlb_index(env, mmu_idx, addr); - CPUTLBEntry *entry = tlb_entry(env, mmu_idx, addr); + int flags; /* * We test this immediately after reading an insn, which means - * that any normal page must be in the TLB. The only exception - * would be for executing from flash or device memory, which - * does not retain the TLB entry. - * - * FIXME: Assume false for those, for now. We could use - * arm_cpu_get_phys_page_attrs_debug to re-read the page - * table entry even for that case. + * that the TLB entry must be present and valid, and thus this + * access will never raise an exception. */ - return (tlb_hit(entry->addr_code, addr) && - arm_tlb_bti_gp(&env_tlb(env)->d[mmu_idx].fulltlb[index].attrs)); + flags = probe_access_full(env, addr, MMU_INST_FETCH, mmu_idx, + false, &host, &full, 0); + assert(!(flags & TLB_INVALID_MASK)); + + return full->guarded; #endif } -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Not yet used, but add mmu indexes for 1-1 mapping to physical addresses. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20221011031911.2408754-5-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/cpu-param.h | 2 +- target/arm/cpu.h | 7 ++++++- target/arm/ptw.c | 19 +++++++++++++++++-- 3 files changed, 24 insertions(+), 4 deletions(-) diff --git a/target/arm/cpu-param.h b/target/arm/cpu-param.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu-param.h +++ b/target/arm/cpu-param.h @@ -XXX,XX +XXX,XX @@ bool guarded; #endif -#define NB_MMU_MODES 8 +#define NB_MMU_MODES 10 #endif diff --git a/target/arm/cpu.h b/target/arm/cpu.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -XXX,XX +XXX,XX @@ bool write_cpustate_to_list(ARMCPU *cpu, bool kvm_sync); * EL2 EL2&0 +PAN * EL2 (aka NS PL2) * EL3 (aka S PL1) + * Physical (NS & S) * - * for a total of 8 different mmu_idx. + * for a total of 10 different mmu_idx. * * R profile CPUs have an MPU, but can use the same set of MMU indexes * as A profile. They only need to distinguish EL0 and EL1 (and @@ -XXX,XX +XXX,XX @@ typedef enum ARMMMUIdx { ARMMMUIdx_E2 = 6 | ARM_MMU_IDX_A, ARMMMUIdx_E3 = 7 | ARM_MMU_IDX_A, + /* TLBs with 1-1 mapping to the physical address spaces. */ + ARMMMUIdx_Phys_NS = 8 | ARM_MMU_IDX_A, + ARMMMUIdx_Phys_S = 9 | ARM_MMU_IDX_A, + /* * These are not allocated TLBs and are used only for AT system * instructions or for the first stage of an S12 page table walk. 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 regime_translation_disabled(CPUARMState *env, ARMMMUIdx mmu_idx, case ARMMMUIdx_E3: break; + case ARMMMUIdx_Phys_NS: + case ARMMMUIdx_Phys_S: + /* No translation for physical address spaces. */ + return true; + default: g_assert_not_reached(); } @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_disabled(CPUARMState *env, target_ulong address, { uint8_t memattr = 0x00; /* Device nGnRnE */ uint8_t shareability = 0; /* non-sharable */ + int r_el; - if (mmu_idx != ARMMMUIdx_Stage2 && mmu_idx != ARMMMUIdx_Stage2_S) { - int r_el = regime_el(env, mmu_idx); + switch (mmu_idx) { + case ARMMMUIdx_Stage2: + case ARMMMUIdx_Stage2_S: + case ARMMMUIdx_Phys_NS: + case ARMMMUIdx_Phys_S: + break; + default: + r_el = regime_el(env, mmu_idx); if (arm_el_is_aa64(env, r_el)) { int pamax = arm_pamax(env_archcpu(env)); uint64_t tcr = env->cp15.tcr_el[r_el]; @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_disabled(CPUARMState *env, target_ulong address, shareability = 2; /* outer sharable */ } result->cacheattrs.is_s2_format = false; + break; } result->f.phys_addr = address; @@ -XXX,XX +XXX,XX @@ bool get_phys_addr(CPUARMState *env, target_ulong address, is_secure = arm_is_secure_below_el3(env); break; case ARMMMUIdx_Stage2: + case ARMMMUIdx_Phys_NS: case ARMMMUIdx_MPrivNegPri: case ARMMMUIdx_MUserNegPri: case ARMMMUIdx_MPriv: @@ -XXX,XX +XXX,XX @@ bool get_phys_addr(CPUARMState *env, target_ulong address, break; case ARMMMUIdx_E3: case ARMMMUIdx_Stage2_S: + case ARMMMUIdx_Phys_S: case ARMMMUIdx_MSPrivNegPri: case ARMMMUIdx_MSUserNegPri: case ARMMMUIdx_MSPriv: -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> We had been marking this ARM_MMU_IDX_NOTLB, move it to a real tlb. Flush the tlb when invalidating stage 1+2 translations. Re-use alle1_tlbmask() for other instances of EL1&0 + Stage2. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 20221011031911.2408754-6-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/cpu-param.h | 2 +- target/arm/cpu.h | 23 ++++--- target/arm/helper.c | 151 ++++++++++++++++++++++++++++++----------- 3 files changed, 127 insertions(+), 49 deletions(-) diff --git a/target/arm/cpu-param.h b/target/arm/cpu-param.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu-param.h +++ b/target/arm/cpu-param.h @@ -XXX,XX +XXX,XX @@ bool guarded; #endif -#define NB_MMU_MODES 10 +#define NB_MMU_MODES 12 #endif diff --git a/target/arm/cpu.h b/target/arm/cpu.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -XXX,XX +XXX,XX @@ bool write_cpustate_to_list(ARMCPU *cpu, bool kvm_sync); * EL2 (aka NS PL2) * EL3 (aka S PL1) * Physical (NS & S) + * Stage2 (NS & S) * - * for a total of 10 different mmu_idx. + * for a total of 12 different mmu_idx. * * R profile CPUs have an MPU, but can use the same set of MMU indexes * as A profile. They only need to distinguish EL0 and EL1 (and @@ -XXX,XX +XXX,XX @@ typedef enum ARMMMUIdx { ARMMMUIdx_Phys_NS = 8 | ARM_MMU_IDX_A, ARMMMUIdx_Phys_S = 9 | ARM_MMU_IDX_A, + /* + * Used for second stage of an S12 page table walk, or for descriptor + * loads during first stage of an S1 page table walk. Note that both + * are in use simultaneously for SecureEL2: the security state for + * the S2 ptw is selected by the NS bit from the S1 ptw. + */ + ARMMMUIdx_Stage2 = 10 | ARM_MMU_IDX_A, + ARMMMUIdx_Stage2_S = 11 | ARM_MMU_IDX_A, + /* * These are not allocated TLBs and are used only for AT system * instructions or for the first stage of an S12 page table walk. @@ -XXX,XX +XXX,XX @@ typedef enum ARMMMUIdx { ARMMMUIdx_Stage1_E0 = 0 | ARM_MMU_IDX_NOTLB, ARMMMUIdx_Stage1_E1 = 1 | ARM_MMU_IDX_NOTLB, ARMMMUIdx_Stage1_E1_PAN = 2 | ARM_MMU_IDX_NOTLB, - /* - * Not allocated a TLB: used only for second stage of an S12 page - * table walk, or for descriptor loads during first stage of an S1 - * page table walk. Note that if we ever want to have a TLB for this - * then various TLB flush insns which currently are no-ops or flush - * only stage 1 MMU indexes will need to change to flush stage 2. - */ - ARMMMUIdx_Stage2 = 3 | ARM_MMU_IDX_NOTLB, - ARMMMUIdx_Stage2_S = 4 | ARM_MMU_IDX_NOTLB, /* * M-profile. @@ -XXX,XX +XXX,XX @@ typedef enum ARMMMUIdxBit { TO_CORE_BIT(E20_2), TO_CORE_BIT(E20_2_PAN), TO_CORE_BIT(E3), + TO_CORE_BIT(Stage2), + TO_CORE_BIT(Stage2_S), TO_CORE_BIT(MUser), TO_CORE_BIT(MPriv), diff --git a/target/arm/helper.c b/target/arm/helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -XXX,XX +XXX,XX @@ static void contextidr_write(CPUARMState *env, const ARMCPRegInfo *ri, raw_write(env, ri, value); } +static int alle1_tlbmask(CPUARMState *env) +{ + /* + * Note that the 'ALL' scope must invalidate both stage 1 and + * stage 2 translations, whereas most other scopes only invalidate + * stage 1 translations. + */ + return (ARMMMUIdxBit_E10_1 | + ARMMMUIdxBit_E10_1_PAN | + ARMMMUIdxBit_E10_0 | + ARMMMUIdxBit_Stage2 | + ARMMMUIdxBit_Stage2_S); +} + + /* IS variants of TLB operations must affect all cores */ static void tlbiall_is_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value) @@ -XXX,XX +XXX,XX @@ static void tlbiall_nsnh_write(CPUARMState *env, const ARMCPRegInfo *ri, { CPUState *cs = env_cpu(env); - tlb_flush_by_mmuidx(cs, - ARMMMUIdxBit_E10_1 | - ARMMMUIdxBit_E10_1_PAN | - ARMMMUIdxBit_E10_0); + tlb_flush_by_mmuidx(cs, alle1_tlbmask(env)); } static void tlbiall_nsnh_is_write(CPUARMState *env, const ARMCPRegInfo *ri, @@ -XXX,XX +XXX,XX @@ static void tlbiall_nsnh_is_write(CPUARMState *env, const ARMCPRegInfo *ri, { CPUState *cs = env_cpu(env); - tlb_flush_by_mmuidx_all_cpus_synced(cs, - ARMMMUIdxBit_E10_1 | - ARMMMUIdxBit_E10_1_PAN | - ARMMMUIdxBit_E10_0); + tlb_flush_by_mmuidx_all_cpus_synced(cs, alle1_tlbmask(env)); } @@ -XXX,XX +XXX,XX @@ static void tlbimva_hyp_is_write(CPUARMState *env, const ARMCPRegInfo *ri, ARMMMUIdxBit_E2); } +static void tlbiipas2_hyp_write(CPUARMState *env, const ARMCPRegInfo *ri, + uint64_t value) +{ + CPUState *cs = env_cpu(env); + uint64_t pageaddr = (value & MAKE_64BIT_MASK(0, 28)) << 12; + + tlb_flush_page_by_mmuidx(cs, pageaddr, ARMMMUIdxBit_Stage2); +} + +static void tlbiipas2is_hyp_write(CPUARMState *env, const ARMCPRegInfo *ri, + uint64_t value) +{ + CPUState *cs = env_cpu(env); + uint64_t pageaddr = (value & MAKE_64BIT_MASK(0, 28)) << 12; + + tlb_flush_page_by_mmuidx_all_cpus_synced(cs, pageaddr, ARMMMUIdxBit_Stage2); +} + static const ARMCPRegInfo cp_reginfo[] = { /* Define the secure and non-secure FCSE identifier CP registers * separately because there is no secure bank in V8 (no _EL3). This allows @@ -XXX,XX +XXX,XX @@ static void vttbr_write(CPUARMState *env, const ARMCPRegInfo *ri, /* * A change in VMID to the stage2 page table (Stage2) invalidates - * the combined stage 1&2 tlbs (EL10_1 and EL10_0). + * the stage2 and combined stage 1&2 tlbs (EL10_1 and EL10_0). */ if (raw_read(env, ri) != value) { - uint16_t mask = ARMMMUIdxBit_E10_1 | - ARMMMUIdxBit_E10_1_PAN | - ARMMMUIdxBit_E10_0; - tlb_flush_by_mmuidx(cs, mask); + tlb_flush_by_mmuidx(cs, alle1_tlbmask(env)); raw_write(env, ri, value); } } @@ -XXX,XX +XXX,XX @@ static void tlbi_aa64_vmalle1_write(CPUARMState *env, const ARMCPRegInfo *ri, } } -static int alle1_tlbmask(CPUARMState *env) -{ - /* - * Note that the 'ALL' scope must invalidate both stage 1 and - * stage 2 translations, whereas most other scopes only invalidate - * stage 1 translations. - */ - return (ARMMMUIdxBit_E10_1 | - ARMMMUIdxBit_E10_1_PAN | - ARMMMUIdxBit_E10_0); -} - static int e2_tlbmask(CPUARMState *env) { return (ARMMMUIdxBit_E20_0 | @@ -XXX,XX +XXX,XX @@ static void tlbi_aa64_vae3is_write(CPUARMState *env, const ARMCPRegInfo *ri, ARMMMUIdxBit_E3, bits); } +static int ipas2e1_tlbmask(CPUARMState *env, int64_t value) +{ + /* + * The MSB of value is the NS field, which only applies if SEL2 + * is implemented and SCR_EL3.NS is not set (i.e. in secure mode). + */ + return (value >= 0 + && cpu_isar_feature(aa64_sel2, env_archcpu(env)) + && arm_is_secure_below_el3(env) + ? ARMMMUIdxBit_Stage2_S + : ARMMMUIdxBit_Stage2); +} + +static void tlbi_aa64_ipas2e1_write(CPUARMState *env, const ARMCPRegInfo *ri, + uint64_t value) +{ + CPUState *cs = env_cpu(env); + int mask = ipas2e1_tlbmask(env, value); + uint64_t pageaddr = sextract64(value << 12, 0, 56); + + if (tlb_force_broadcast(env)) { + tlb_flush_page_by_mmuidx_all_cpus_synced(cs, pageaddr, mask); + } else { + tlb_flush_page_by_mmuidx(cs, pageaddr, mask); + } +} + +static void tlbi_aa64_ipas2e1is_write(CPUARMState *env, const ARMCPRegInfo *ri, + uint64_t value) +{ + CPUState *cs = env_cpu(env); + int mask = ipas2e1_tlbmask(env, value); + uint64_t pageaddr = sextract64(value << 12, 0, 56); + + tlb_flush_page_by_mmuidx_all_cpus_synced(cs, pageaddr, mask); +} + #ifdef TARGET_AARCH64 typedef struct { uint64_t base; @@ -XXX,XX +XXX,XX @@ static void tlbi_aa64_rvae3is_write(CPUARMState *env, do_rvae_write(env, value, ARMMMUIdxBit_E3, true); } + +static void tlbi_aa64_ripas2e1_write(CPUARMState *env, const ARMCPRegInfo *ri, + uint64_t value) +{ + do_rvae_write(env, value, ipas2e1_tlbmask(env, value), + tlb_force_broadcast(env)); +} + +static void tlbi_aa64_ripas2e1is_write(CPUARMState *env, + const ARMCPRegInfo *ri, + uint64_t value) +{ + do_rvae_write(env, value, ipas2e1_tlbmask(env, value), true); +} #endif static CPAccessResult aa64_zva_access(CPUARMState *env, const ARMCPRegInfo *ri, @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo v8_cp_reginfo[] = { .writefn = tlbi_aa64_vae1_write }, { .name = "TLBI_IPAS2E1IS", .state = ARM_CP_STATE_AA64, .opc0 = 1, .opc1 = 4, .crn = 8, .crm = 0, .opc2 = 1, - .access = PL2_W, .type = ARM_CP_NOP }, + .access = PL2_W, .type = ARM_CP_NO_RAW, + .writefn = tlbi_aa64_ipas2e1is_write }, { .name = "TLBI_IPAS2LE1IS", .state = ARM_CP_STATE_AA64, .opc0 = 1, .opc1 = 4, .crn = 8, .crm = 0, .opc2 = 5, - .access = PL2_W, .type = ARM_CP_NOP }, + .access = PL2_W, .type = ARM_CP_NO_RAW, + .writefn = tlbi_aa64_ipas2e1is_write }, { .name = "TLBI_ALLE1IS", .state = ARM_CP_STATE_AA64, .opc0 = 1, .opc1 = 4, .crn = 8, .crm = 3, .opc2 = 4, .access = PL2_W, .type = ARM_CP_NO_RAW, @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo v8_cp_reginfo[] = { .writefn = tlbi_aa64_alle1is_write }, { .name = "TLBI_IPAS2E1", .state = ARM_CP_STATE_AA64, .opc0 = 1, .opc1 = 4, .crn = 8, .crm = 4, .opc2 = 1, - .access = PL2_W, .type = ARM_CP_NOP }, + .access = PL2_W, .type = ARM_CP_NO_RAW, + .writefn = tlbi_aa64_ipas2e1_write }, { .name = "TLBI_IPAS2LE1", .state = ARM_CP_STATE_AA64, .opc0 = 1, .opc1 = 4, .crn = 8, .crm = 4, .opc2 = 5, - .access = PL2_W, .type = ARM_CP_NOP }, + .access = PL2_W, .type = ARM_CP_NO_RAW, + .writefn = tlbi_aa64_ipas2e1_write }, { .name = "TLBI_ALLE1", .state = ARM_CP_STATE_AA64, .opc0 = 1, .opc1 = 4, .crn = 8, .crm = 7, .opc2 = 4, .access = PL2_W, .type = ARM_CP_NO_RAW, @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo v8_cp_reginfo[] = { .writefn = tlbimva_hyp_is_write }, { .name = "TLBIIPAS2", .cp = 15, .opc1 = 4, .crn = 8, .crm = 4, .opc2 = 1, - .type = ARM_CP_NOP, .access = PL2_W }, + .type = ARM_CP_NO_RAW, .access = PL2_W, + .writefn = tlbiipas2_hyp_write }, { .name = "TLBIIPAS2IS", .cp = 15, .opc1 = 4, .crn = 8, .crm = 0, .opc2 = 1, - .type = ARM_CP_NOP, .access = PL2_W }, + .type = ARM_CP_NO_RAW, .access = PL2_W, + .writefn = tlbiipas2is_hyp_write }, { .name = "TLBIIPAS2L", .cp = 15, .opc1 = 4, .crn = 8, .crm = 4, .opc2 = 5, - .type = ARM_CP_NOP, .access = PL2_W }, + .type = ARM_CP_NO_RAW, .access = PL2_W, + .writefn = tlbiipas2_hyp_write }, { .name = "TLBIIPAS2LIS", .cp = 15, .opc1 = 4, .crn = 8, .crm = 0, .opc2 = 5, - .type = ARM_CP_NOP, .access = PL2_W }, + .type = ARM_CP_NO_RAW, .access = PL2_W, + .writefn = tlbiipas2is_hyp_write }, /* 32 bit cache operations */ { .name = "ICIALLUIS", .cp = 15, .opc1 = 0, .crn = 7, .crm = 1, .opc2 = 0, .type = ARM_CP_NOP, .access = PL1_W, .accessfn = aa64_cacheop_pou_access }, @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo tlbirange_reginfo[] = { .writefn = tlbi_aa64_rvae1_write }, { .name = "TLBI_RIPAS2E1IS", .state = ARM_CP_STATE_AA64, .opc0 = 1, .opc1 = 4, .crn = 8, .crm = 0, .opc2 = 2, - .access = PL2_W, .type = ARM_CP_NOP }, + .access = PL2_W, .type = ARM_CP_NO_RAW, + .writefn = tlbi_aa64_ripas2e1is_write }, { .name = "TLBI_RIPAS2LE1IS", .state = ARM_CP_STATE_AA64, .opc0 = 1, .opc1 = 4, .crn = 8, .crm = 0, .opc2 = 6, - .access = PL2_W, .type = ARM_CP_NOP }, + .access = PL2_W, .type = ARM_CP_NO_RAW, + .writefn = tlbi_aa64_ripas2e1is_write }, { .name = "TLBI_RVAE2IS", .state = ARM_CP_STATE_AA64, .opc0 = 1, .opc1 = 4, .crn = 8, .crm = 2, .opc2 = 1, .access = PL2_W, .type = ARM_CP_NO_RAW | ARM_CP_EL3_NO_EL2_UNDEF, @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo tlbirange_reginfo[] = { .writefn = tlbi_aa64_rvae2is_write }, { .name = "TLBI_RIPAS2E1", .state = ARM_CP_STATE_AA64, .opc0 = 1, .opc1 = 4, .crn = 8, .crm = 4, .opc2 = 2, - .access = PL2_W, .type = ARM_CP_NOP }, - { .name = "TLBI_RIPAS2LE1", .state = ARM_CP_STATE_AA64, + .access = PL2_W, .type = ARM_CP_NO_RAW, + .writefn = tlbi_aa64_ripas2e1_write }, + { .name = "TLBI_RIPAS2LE1", .state = ARM_CP_STATE_AA64, .opc0 = 1, .opc1 = 4, .crn = 8, .crm = 4, .opc2 = 6, - .access = PL2_W, .type = ARM_CP_NOP }, + .access = PL2_W, .type = ARM_CP_NO_RAW, + .writefn = tlbi_aa64_ripas2e1_write }, { .name = "TLBI_RVAE2OS", .state = ARM_CP_STATE_AA64, .opc0 = 1, .opc1 = 4, .crn = 8, .crm = 5, .opc2 = 1, .access = PL2_W, .type = ARM_CP_NO_RAW | ARM_CP_EL3_NO_EL2_UNDEF, -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Compare only the VMID field when considering whether we need to flush. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 20221011031911.2408754-7-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/helper.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/target/arm/helper.c b/target/arm/helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -XXX,XX +XXX,XX @@ static void vttbr_write(CPUARMState *env, const ARMCPRegInfo *ri, * A change in VMID to the stage2 page table (Stage2) invalidates * the stage2 and combined stage 1&2 tlbs (EL10_1 and EL10_0). */ - if (raw_read(env, ri) != value) { + if (extract64(raw_read(env, ri) ^ value, 48, 16) != 0) { tlb_flush_by_mmuidx(cs, alle1_tlbmask(env)); - raw_write(env, ri, value); } + raw_write(env, ri, value); } static const ARMCPRegInfo vmsa_pmsa_cp_reginfo[] = { -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Consolidate most of the inputs and outputs of S1_ptw_translate into a single structure. Plumb this through arm_ld*_ptw from the controlling get_phys_addr_* routine. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 20221011031911.2408754-8-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/ptw.c | 140 ++++++++++++++++++++++++++--------------------- 1 file changed, 79 insertions(+), 61 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 @@ #include "idau.h" -static bool get_phys_addr_lpae(CPUARMState *env, uint64_t address, - MMUAccessType access_type, ARMMMUIdx mmu_idx, - bool is_secure, bool s1_is_el0, +typedef struct S1Translate { + ARMMMUIdx in_mmu_idx; + bool in_secure; + bool out_secure; + hwaddr out_phys; +} S1Translate; + +static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw, + uint64_t address, + MMUAccessType access_type, bool s1_is_el0, GetPhysAddrResult *result, ARMMMUFaultInfo *fi) __attribute__((nonnull)); @@ -XXX,XX +XXX,XX @@ static bool ptw_attrs_are_device(uint64_t hcr, ARMCacheAttrs cacheattrs) } /* Translate a S1 pagetable walk through S2 if needed. */ -static hwaddr S1_ptw_translate(CPUARMState *env, ARMMMUIdx mmu_idx, - hwaddr addr, bool *is_secure_ptr, - ARMMMUFaultInfo *fi) +static bool S1_ptw_translate(CPUARMState *env, S1Translate *ptw, + hwaddr addr, ARMMMUFaultInfo *fi) { - bool is_secure = *is_secure_ptr; + bool is_secure = ptw->in_secure; ARMMMUIdx s2_mmu_idx = is_secure ? ARMMMUIdx_Stage2_S : ARMMMUIdx_Stage2; - if (arm_mmu_idx_is_stage1_of_2(mmu_idx) && + if (arm_mmu_idx_is_stage1_of_2(ptw->in_mmu_idx) && !regime_translation_disabled(env, s2_mmu_idx, is_secure)) { GetPhysAddrResult s2 = {}; + S1Translate s2ptw = { + .in_mmu_idx = s2_mmu_idx, + .in_secure = is_secure, + }; uint64_t hcr; int ret; - ret = get_phys_addr_lpae(env, addr, MMU_DATA_LOAD, s2_mmu_idx, - is_secure, false, &s2, fi); + ret = get_phys_addr_lpae(env, &s2ptw, addr, MMU_DATA_LOAD, + false, &s2, fi); if (ret) { assert(fi->type != ARMFault_None); fi->s2addr = addr; fi->stage2 = true; fi->s1ptw = true; fi->s1ns = !is_secure; - return ~0; + return false; } hcr = arm_hcr_el2_eff_secstate(env, is_secure); @@ -XXX,XX +XXX,XX @@ static hwaddr S1_ptw_translate(CPUARMState *env, ARMMMUIdx mmu_idx, fi->stage2 = true; fi->s1ptw = true; fi->s1ns = !is_secure; - return ~0; + return false; } if (arm_is_secure_below_el3(env)) { @@ -XXX,XX +XXX,XX @@ static hwaddr S1_ptw_translate(CPUARMState *env, ARMMMUIdx mmu_idx, } else { is_secure = !(env->cp15.vtcr_el2 & VTCR_NSW); } - *is_secure_ptr = is_secure; } else { assert(!is_secure); } addr = s2.f.phys_addr; } - return addr; + + ptw->out_secure = is_secure; + ptw->out_phys = addr; + return true; } /* All loads done in the course of a page table walk go through here. */ -static uint32_t arm_ldl_ptw(CPUARMState *env, hwaddr addr, bool is_secure, - ARMMMUIdx mmu_idx, ARMMMUFaultInfo *fi) +static uint32_t arm_ldl_ptw(CPUARMState *env, S1Translate *ptw, hwaddr addr, + ARMMMUFaultInfo *fi) { CPUState *cs = env_cpu(env); MemTxAttrs attrs = {}; @@ -XXX,XX +XXX,XX @@ static uint32_t arm_ldl_ptw(CPUARMState *env, hwaddr addr, bool is_secure, AddressSpace *as; uint32_t data; - addr = S1_ptw_translate(env, mmu_idx, addr, &is_secure, fi); - attrs.secure = is_secure; - as = arm_addressspace(cs, attrs); - if (fi->s1ptw) { + if (!S1_ptw_translate(env, ptw, addr, fi)) { return 0; } - if (regime_translation_big_endian(env, mmu_idx)) { + addr = ptw->out_phys; + attrs.secure = ptw->out_secure; + as = arm_addressspace(cs, attrs); + if (regime_translation_big_endian(env, ptw->in_mmu_idx)) { data = address_space_ldl_be(as, addr, attrs, &result); } else { data = address_space_ldl_le(as, addr, attrs, &result); @@ -XXX,XX +XXX,XX @@ static uint32_t arm_ldl_ptw(CPUARMState *env, hwaddr addr, bool is_secure, return 0; } -static uint64_t arm_ldq_ptw(CPUARMState *env, hwaddr addr, bool is_secure, - ARMMMUIdx mmu_idx, ARMMMUFaultInfo *fi) +static uint64_t arm_ldq_ptw(CPUARMState *env, S1Translate *ptw, hwaddr addr, + ARMMMUFaultInfo *fi) { CPUState *cs = env_cpu(env); MemTxAttrs attrs = {}; @@ -XXX,XX +XXX,XX @@ static uint64_t arm_ldq_ptw(CPUARMState *env, hwaddr addr, bool is_secure, AddressSpace *as; uint64_t data; - addr = S1_ptw_translate(env, mmu_idx, addr, &is_secure, fi); - attrs.secure = is_secure; - as = arm_addressspace(cs, attrs); - if (fi->s1ptw) { + if (!S1_ptw_translate(env, ptw, addr, fi)) { return 0; } - if (regime_translation_big_endian(env, mmu_idx)) { + addr = ptw->out_phys; + attrs.secure = ptw->out_secure; + as = arm_addressspace(cs, attrs); + if (regime_translation_big_endian(env, ptw->in_mmu_idx)) { data = address_space_ldq_be(as, addr, attrs, &result); } else { data = address_space_ldq_le(as, addr, attrs, &result); @@ -XXX,XX +XXX,XX @@ static int simple_ap_to_rw_prot(CPUARMState *env, ARMMMUIdx mmu_idx, int ap) return simple_ap_to_rw_prot_is_user(ap, regime_is_user(env, mmu_idx)); } -static bool get_phys_addr_v5(CPUARMState *env, uint32_t address, - MMUAccessType access_type, ARMMMUIdx mmu_idx, - bool is_secure, GetPhysAddrResult *result, - ARMMMUFaultInfo *fi) +static bool get_phys_addr_v5(CPUARMState *env, S1Translate *ptw, + uint32_t address, MMUAccessType access_type, + 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, /* Pagetable walk. */ /* Lookup l1 descriptor. */ - if (!get_level1_table_address(env, mmu_idx, &table, address)) { + if (!get_level1_table_address(env, ptw->in_mmu_idx, &table, address)) { /* Section translation fault if page walk is disabled by PD0 or PD1 */ fi->type = ARMFault_Translation; goto do_fault; } - desc = arm_ldl_ptw(env, table, is_secure, mmu_idx, fi); + desc = arm_ldl_ptw(env, ptw, table, fi); if (fi->type != ARMFault_None) { goto do_fault; } type = (desc & 3); domain = (desc >> 5) & 0x0f; - if (regime_el(env, mmu_idx) == 1) { + if (regime_el(env, ptw->in_mmu_idx) == 1) { dacr = env->cp15.dacr_ns; } else { dacr = env->cp15.dacr_s; @@ -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, is_secure, mmu_idx, fi); + desc = arm_ldl_ptw(env, ptw, table, fi); if (fi->type != ARMFault_None) { goto do_fault; } @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_v5(CPUARMState *env, uint32_t address, g_assert_not_reached(); } } - result->f.prot = ap_to_rw_prot(env, mmu_idx, ap, domain_prot); + result->f.prot = ap_to_rw_prot(env, ptw->in_mmu_idx, ap, domain_prot); result->f.prot |= result->f.prot ? PAGE_EXEC : 0; if (!(result->f.prot & (1 << access_type))) { /* Access permission fault. */ @@ -XXX,XX +XXX,XX @@ do_fault: return true; } -static bool get_phys_addr_v6(CPUARMState *env, uint32_t address, - MMUAccessType access_type, ARMMMUIdx mmu_idx, - bool is_secure, GetPhysAddrResult *result, - ARMMMUFaultInfo *fi) +static bool get_phys_addr_v6(CPUARMState *env, S1Translate *ptw, + uint32_t address, MMUAccessType access_type, + GetPhysAddrResult *result, ARMMMUFaultInfo *fi) { ARMCPU *cpu = env_archcpu(env); + ARMMMUIdx mmu_idx = ptw->in_mmu_idx; int level = 1; uint32_t table; uint32_t desc; @@ -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, is_secure, mmu_idx, fi); + desc = arm_ldl_ptw(env, ptw, table, 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, is_secure, mmu_idx, fi); + desc = arm_ldl_ptw(env, ptw, table, fi); if (fi->type != ARMFault_None) { goto do_fault; } @@ -XXX,XX +XXX,XX @@ static bool check_s2_mmu_setup(ARMCPU *cpu, bool is_aa64, int level, * the WnR bit is never set (the caller must do this). * * @env: CPUARMState + * @ptw: Current and next stage parameters for the walk. * @address: virtual address to get physical address for * @access_type: MMU_DATA_LOAD, MMU_DATA_STORE or MMU_INST_FETCH - * @mmu_idx: MMU index indicating required translation regime - * @s1_is_el0: if @mmu_idx is ARMMMUIdx_Stage2 (so this is a stage 2 page - * table walk), must be true if this is stage 2 of a stage 1+2 + * @s1_is_el0: if @ptw->in_mmu_idx is ARMMMUIdx_Stage2 + * (so this is a stage 2 page 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. * @result: set on translation success, * @fi: set to fault info if the translation fails */ -static bool get_phys_addr_lpae(CPUARMState *env, uint64_t address, - MMUAccessType access_type, ARMMMUIdx mmu_idx, - bool is_secure, bool s1_is_el0, +static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw, + uint64_t address, + MMUAccessType access_type, bool s1_is_el0, GetPhysAddrResult *result, ARMMMUFaultInfo *fi) { ARMCPU *cpu = env_archcpu(env); + ARMMMUIdx mmu_idx = ptw->in_mmu_idx; + bool is_secure = ptw->in_secure; /* Read an LPAE long-descriptor translation table. */ ARMFaultType fault_type = ARMFault_Translation; uint32_t level; @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_lpae(CPUARMState *env, uint64_t address, descaddr |= (address >> (stride * (4 - level))) & indexmask; descaddr &= ~7ULL; nstable = extract32(tableattrs, 4, 1); - descriptor = arm_ldq_ptw(env, descaddr, !nstable, mmu_idx, fi); + ptw->in_secure = !nstable; + descriptor = arm_ldq_ptw(env, ptw, descaddr, fi); if (fi->type != ARMFault_None) { goto do_fault; } @@ -XXX,XX +XXX,XX @@ bool get_phys_addr_with_secure(CPUARMState *env, target_ulong address, ARMMMUFaultInfo *fi) { ARMMMUIdx s1_mmu_idx = stage_1_mmu_idx(mmu_idx); + S1Translate ptw; if (mmu_idx != s1_mmu_idx) { /* @@ -XXX,XX +XXX,XX @@ bool get_phys_addr_with_secure(CPUARMState *env, target_ulong address, int ret; bool ipa_secure, s2walk_secure; ARMCacheAttrs cacheattrs1; - ARMMMUIdx s2_mmu_idx; bool is_el0; uint64_t hcr; @@ -XXX,XX +XXX,XX @@ bool get_phys_addr_with_secure(CPUARMState *env, target_ulong address, s2walk_secure = false; } - s2_mmu_idx = (s2walk_secure - ? ARMMMUIdx_Stage2_S : ARMMMUIdx_Stage2); + ptw.in_mmu_idx = + s2walk_secure ? ARMMMUIdx_Stage2_S : ARMMMUIdx_Stage2; + ptw.in_secure = s2walk_secure; is_el0 = mmu_idx == ARMMMUIdx_E10_0; /* @@ -XXX,XX +XXX,XX @@ bool get_phys_addr_with_secure(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, - s2walk_secure, is_el0, result, fi); + ret = get_phys_addr_lpae(env, &ptw, ipa, access_type, + is_el0, result, fi); fi->s2addr = ipa; /* Combine the S1 and S2 perms. */ @@ -XXX,XX +XXX,XX @@ bool get_phys_addr_with_secure(CPUARMState *env, target_ulong address, return get_phys_addr_disabled(env, address, access_type, mmu_idx, is_secure, result, fi); } + + ptw.in_mmu_idx = mmu_idx; + ptw.in_secure = is_secure; + if (regime_using_lpae_format(env, mmu_idx)) { - return get_phys_addr_lpae(env, address, access_type, mmu_idx, - is_secure, false, result, fi); + return get_phys_addr_lpae(env, &ptw, address, access_type, false, + result, fi); } else if (regime_sctlr(env, mmu_idx) & SCTLR_XP) { - return get_phys_addr_v6(env, address, access_type, mmu_idx, - is_secure, result, fi); + return get_phys_addr_v6(env, &ptw, address, access_type, result, fi); } else { - return get_phys_addr_v5(env, address, access_type, mmu_idx, - is_secure, result, fi); + return get_phys_addr_v5(env, &ptw, address, access_type, result, fi); } } -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Before using softmmu page tables for the ptw, plumb down a debug parameter so that we can query page table entries from gdbstub without modifying cpu state. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20221011031911.2408754-9-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/ptw.c | 55 ++++++++++++++++++++++++++++++++---------------- 1 file changed, 37 insertions(+), 18 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 @@ typedef struct S1Translate { ARMMMUIdx in_mmu_idx; bool in_secure; + bool in_debug; bool out_secure; hwaddr out_phys; } S1Translate; @@ -XXX,XX +XXX,XX @@ static bool S1_ptw_translate(CPUARMState *env, S1Translate *ptw, S1Translate s2ptw = { .in_mmu_idx = s2_mmu_idx, .in_secure = is_secure, + .in_debug = ptw->in_debug, }; uint64_t hcr; int ret; @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_disabled(CPUARMState *env, target_ulong address, return 0; } -bool get_phys_addr_with_secure(CPUARMState *env, target_ulong address, - MMUAccessType access_type, ARMMMUIdx mmu_idx, - bool is_secure, GetPhysAddrResult *result, - ARMMMUFaultInfo *fi) +static bool get_phys_addr_with_struct(CPUARMState *env, S1Translate *ptw, + target_ulong address, + MMUAccessType access_type, + GetPhysAddrResult *result, + ARMMMUFaultInfo *fi) { + ARMMMUIdx mmu_idx = ptw->in_mmu_idx; ARMMMUIdx s1_mmu_idx = stage_1_mmu_idx(mmu_idx); - S1Translate ptw; + bool is_secure = ptw->in_secure; if (mmu_idx != s1_mmu_idx) { /* @@ -XXX,XX +XXX,XX @@ bool get_phys_addr_with_secure(CPUARMState *env, target_ulong address, bool is_el0; uint64_t hcr; - ret = get_phys_addr_with_secure(env, address, access_type, - s1_mmu_idx, is_secure, result, fi); + ptw->in_mmu_idx = s1_mmu_idx; + ret = get_phys_addr_with_struct(env, ptw, address, access_type, + result, fi); /* If S1 fails or S2 is disabled, return early. */ if (ret || regime_translation_disabled(env, ARMMMUIdx_Stage2, @@ -XXX,XX +XXX,XX @@ bool get_phys_addr_with_secure(CPUARMState *env, target_ulong address, s2walk_secure = false; } - ptw.in_mmu_idx = + ptw->in_mmu_idx = s2walk_secure ? ARMMMUIdx_Stage2_S : ARMMMUIdx_Stage2; - ptw.in_secure = s2walk_secure; + ptw->in_secure = s2walk_secure; is_el0 = mmu_idx == ARMMMUIdx_E10_0; /* @@ -XXX,XX +XXX,XX @@ bool get_phys_addr_with_secure(CPUARMState *env, target_ulong address, cacheattrs1 = result->cacheattrs; memset(result, 0, sizeof(*result)); - ret = get_phys_addr_lpae(env, &ptw, ipa, access_type, + ret = get_phys_addr_lpae(env, ptw, ipa, access_type, is_el0, result, fi); fi->s2addr = ipa; @@ -XXX,XX +XXX,XX @@ bool get_phys_addr_with_secure(CPUARMState *env, target_ulong address, is_secure, result, fi); } - ptw.in_mmu_idx = mmu_idx; - ptw.in_secure = is_secure; - if (regime_using_lpae_format(env, mmu_idx)) { - return get_phys_addr_lpae(env, &ptw, address, access_type, false, + return get_phys_addr_lpae(env, ptw, address, access_type, false, result, fi); } else if (regime_sctlr(env, mmu_idx) & SCTLR_XP) { - return get_phys_addr_v6(env, &ptw, address, access_type, result, fi); + return get_phys_addr_v6(env, ptw, address, access_type, result, fi); } else { - return get_phys_addr_v5(env, &ptw, address, access_type, result, fi); + return get_phys_addr_v5(env, ptw, address, access_type, result, fi); } } +bool get_phys_addr_with_secure(CPUARMState *env, target_ulong address, + MMUAccessType access_type, ARMMMUIdx mmu_idx, + bool is_secure, GetPhysAddrResult *result, + ARMMMUFaultInfo *fi) +{ + S1Translate ptw = { + .in_mmu_idx = mmu_idx, + .in_secure = is_secure, + }; + return get_phys_addr_with_struct(env, &ptw, address, access_type, + result, fi); +} + bool get_phys_addr(CPUARMState *env, target_ulong address, MMUAccessType access_type, ARMMMUIdx mmu_idx, GetPhysAddrResult *result, ARMMMUFaultInfo *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; + S1Translate ptw = { + .in_mmu_idx = arm_mmu_idx(env), + .in_secure = arm_is_secure(env), + .in_debug = true, + }; GetPhysAddrResult res = {}; ARMMMUFaultInfo fi = {}; - ARMMMUIdx mmu_idx = arm_mmu_idx(env); bool ret; - ret = get_phys_addr(env, addr, MMU_DATA_LOAD, mmu_idx, &res, &fi); + ret = get_phys_addr_with_struct(env, &ptw, addr, MMU_DATA_LOAD, &res, &fi); *attrs = res.f.attrs; if (ret) { -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Hoist this test out of arm_ld[lq]_ptw into S1_ptw_translate. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20221011031911.2408754-10-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/ptw.c | 6 ++++-- 1 file changed, 4 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 @@ typedef struct S1Translate { bool in_secure; bool in_debug; bool out_secure; + bool out_be; hwaddr out_phys; } S1Translate; @@ -XXX,XX +XXX,XX @@ static bool S1_ptw_translate(CPUARMState *env, S1Translate *ptw, ptw->out_secure = is_secure; ptw->out_phys = addr; + ptw->out_be = regime_translation_big_endian(env, ptw->in_mmu_idx); return true; } @@ -XXX,XX +XXX,XX @@ static uint32_t arm_ldl_ptw(CPUARMState *env, S1Translate *ptw, hwaddr addr, addr = ptw->out_phys; attrs.secure = ptw->out_secure; as = arm_addressspace(cs, attrs); - if (regime_translation_big_endian(env, ptw->in_mmu_idx)) { + if (ptw->out_be) { data = address_space_ldl_be(as, addr, attrs, &result); } else { data = address_space_ldl_le(as, addr, attrs, &result); @@ -XXX,XX +XXX,XX @@ static uint64_t arm_ldq_ptw(CPUARMState *env, S1Translate *ptw, hwaddr addr, addr = ptw->out_phys; attrs.secure = ptw->out_secure; as = arm_addressspace(cs, attrs); - if (regime_translation_big_endian(env, ptw->in_mmu_idx)) { + if (ptw->out_be) { data = address_space_ldq_be(as, addr, attrs, &result); } else { data = address_space_ldq_le(as, addr, attrs, &result); -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> So far, limit the change to S1_ptw_translate, arm_ldl_ptw, and arm_ldq_ptw. Use probe_access_full to find the host address, and if so use a host load. If the probe fails, we've got our fault info already. On the off chance that page tables are not in RAM, continue to use the address_space_ld* functions. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20221011031911.2408754-11-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/cpu.h | 5 + target/arm/ptw.c | 196 +++++++++++++++++++++++++--------------- target/arm/tlb_helper.c | 17 +++- 3 files changed, 144 insertions(+), 74 deletions(-) diff --git a/target/arm/cpu.h b/target/arm/cpu.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -XXX,XX +XXX,XX @@ typedef struct CPUARMTBFlags { target_ulong flags2; } CPUARMTBFlags; +typedef struct ARMMMUFaultInfo ARMMMUFaultInfo; + typedef struct CPUArchState { /* Regs for current mode. */ uint32_t regs[16]; @@ -XXX,XX +XXX,XX @@ typedef struct CPUArchState { struct CPUBreakpoint *cpu_breakpoint[16]; struct CPUWatchpoint *cpu_watchpoint[16]; + /* Optional fault info across tlb lookup. */ + ARMMMUFaultInfo *tlb_fi; + /* Fields up to this point are cleared by a CPU reset */ struct {} end_reset_fields; 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 @@ #include "qemu/osdep.h" #include "qemu/log.h" #include "qemu/range.h" +#include "exec/exec-all.h" #include "cpu.h" #include "internals.h" #include "idau.h" @@ -XXX,XX +XXX,XX @@ typedef struct S1Translate { bool out_secure; bool out_be; hwaddr out_phys; + void *out_host; } S1Translate; static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw, @@ -XXX,XX +XXX,XX @@ static bool regime_translation_disabled(CPUARMState *env, ARMMMUIdx mmu_idx, return (regime_sctlr(env, mmu_idx) & SCTLR_M) == 0; } -static bool ptw_attrs_are_device(uint64_t hcr, ARMCacheAttrs cacheattrs) +static bool S2_attrs_are_device(uint64_t hcr, uint8_t attrs) { /* * For an S1 page table walk, the stage 1 attributes are always @@ -XXX,XX +XXX,XX @@ static bool ptw_attrs_are_device(uint64_t hcr, ARMCacheAttrs cacheattrs) * With HCR_EL2.FWB == 1 this is when descriptor bit [4] is 0, ie * when cacheattrs.attrs bit [2] is 0. */ - assert(cacheattrs.is_s2_format); if (hcr & HCR_FWB) { - return (cacheattrs.attrs & 0x4) == 0; + return (attrs & 0x4) == 0; } else { - return (cacheattrs.attrs & 0xc) == 0; + return (attrs & 0xc) == 0; } } @@ -XXX,XX +XXX,XX @@ static bool S1_ptw_translate(CPUARMState *env, S1Translate *ptw, hwaddr addr, ARMMMUFaultInfo *fi) { bool is_secure = ptw->in_secure; + ARMMMUIdx mmu_idx = ptw->in_mmu_idx; ARMMMUIdx s2_mmu_idx = is_secure ? ARMMMUIdx_Stage2_S : ARMMMUIdx_Stage2; + bool s2_phys = false; + uint8_t pte_attrs; + bool pte_secure; - if (arm_mmu_idx_is_stage1_of_2(ptw->in_mmu_idx) && - !regime_translation_disabled(env, s2_mmu_idx, is_secure)) { - GetPhysAddrResult s2 = {}; - S1Translate s2ptw = { - .in_mmu_idx = s2_mmu_idx, - .in_secure = is_secure, - .in_debug = ptw->in_debug, - }; - uint64_t hcr; - int ret; + if (!arm_mmu_idx_is_stage1_of_2(mmu_idx) + || regime_translation_disabled(env, s2_mmu_idx, is_secure)) { + s2_mmu_idx = is_secure ? ARMMMUIdx_Phys_S : ARMMMUIdx_Phys_NS; + s2_phys = true; + } - ret = get_phys_addr_lpae(env, &s2ptw, addr, MMU_DATA_LOAD, - false, &s2, fi); - if (ret) { - assert(fi->type != ARMFault_None); - fi->s2addr = addr; - fi->stage2 = true; - fi->s1ptw = true; - fi->s1ns = !is_secure; - return false; + if (unlikely(ptw->in_debug)) { + /* + * From gdbstub, do not use softmmu so that we don't modify the + * state of the cpu at all, including softmmu tlb contents. + */ + if (s2_phys) { + ptw->out_phys = addr; + pte_attrs = 0; + pte_secure = is_secure; + } else { + S1Translate s2ptw = { + .in_mmu_idx = s2_mmu_idx, + .in_secure = is_secure, + .in_debug = true, + }; + GetPhysAddrResult s2 = { }; + if (!get_phys_addr_lpae(env, &s2ptw, addr, MMU_DATA_LOAD, + false, &s2, fi)) { + goto fail; + } + ptw->out_phys = s2.f.phys_addr; + pte_attrs = s2.cacheattrs.attrs; + pte_secure = s2.f.attrs.secure; } + ptw->out_host = NULL; + } else { + CPUTLBEntryFull *full; + int flags; - hcr = arm_hcr_el2_eff_secstate(env, is_secure); - if ((hcr & HCR_PTW) && ptw_attrs_are_device(hcr, s2.cacheattrs)) { + env->tlb_fi = fi; + flags = probe_access_full(env, addr, MMU_DATA_LOAD, + arm_to_core_mmu_idx(s2_mmu_idx), + true, &ptw->out_host, &full, 0); + env->tlb_fi = NULL; + + if (unlikely(flags & TLB_INVALID_MASK)) { + goto fail; + } + ptw->out_phys = full->phys_addr; + pte_attrs = full->pte_attrs; + pte_secure = full->attrs.secure; + } + + if (!s2_phys) { + uint64_t hcr = arm_hcr_el2_eff_secstate(env, is_secure); + + if ((hcr & HCR_PTW) && S2_attrs_are_device(hcr, pte_attrs)) { /* * PTW set and S1 walk touched S2 Device memory: * generate Permission fault. @@ -XXX,XX +XXX,XX @@ static bool S1_ptw_translate(CPUARMState *env, S1Translate *ptw, fi->s1ns = !is_secure; return false; } - - if (arm_is_secure_below_el3(env)) { - /* Check if page table walk is to secure or non-secure PA space. */ - if (is_secure) { - is_secure = !(env->cp15.vstcr_el2 & VSTCR_SW); - } else { - is_secure = !(env->cp15.vtcr_el2 & VTCR_NSW); - } - } else { - assert(!is_secure); - } - - addr = s2.f.phys_addr; } - ptw->out_secure = is_secure; - ptw->out_phys = addr; - ptw->out_be = regime_translation_big_endian(env, ptw->in_mmu_idx); + /* Check if page table walk is to secure or non-secure PA space. */ + ptw->out_secure = (is_secure + && !(pte_secure + ? env->cp15.vstcr_el2 & VSTCR_SW + : env->cp15.vtcr_el2 & VTCR_NSW)); + ptw->out_be = regime_translation_big_endian(env, mmu_idx); return true; + + fail: + assert(fi->type != ARMFault_None); + fi->s2addr = addr; + fi->stage2 = true; + fi->s1ptw = true; + fi->s1ns = !is_secure; + return false; } /* All loads done in the course of a page table walk go through here. */ @@ -XXX,XX +XXX,XX @@ static uint32_t arm_ldl_ptw(CPUARMState *env, S1Translate *ptw, hwaddr addr, ARMMMUFaultInfo *fi) { CPUState *cs = env_cpu(env); - MemTxAttrs attrs = {}; - MemTxResult result = MEMTX_OK; - AddressSpace *as; uint32_t data; if (!S1_ptw_translate(env, ptw, addr, fi)) { + /* Failure. */ + assert(fi->s1ptw); return 0; } - addr = ptw->out_phys; - attrs.secure = ptw->out_secure; - as = arm_addressspace(cs, attrs); - if (ptw->out_be) { - data = address_space_ldl_be(as, addr, attrs, &result); + + if (likely(ptw->out_host)) { + /* Page tables are in RAM, and we have the host address. */ + if (ptw->out_be) { + data = ldl_be_p(ptw->out_host); + } else { + data = ldl_le_p(ptw->out_host); + } } else { - data = address_space_ldl_le(as, addr, attrs, &result); + /* Page tables are in MMIO. */ + MemTxAttrs attrs = { .secure = ptw->out_secure }; + AddressSpace *as = arm_addressspace(cs, attrs); + MemTxResult result = MEMTX_OK; + + if (ptw->out_be) { + data = address_space_ldl_be(as, ptw->out_phys, attrs, &result); + } else { + data = address_space_ldl_le(as, ptw->out_phys, attrs, &result); + } + if (unlikely(result != MEMTX_OK)) { + fi->type = ARMFault_SyncExternalOnWalk; + fi->ea = arm_extabort_type(result); + return 0; + } } - if (result == MEMTX_OK) { - return data; - } - fi->type = ARMFault_SyncExternalOnWalk; - fi->ea = arm_extabort_type(result); - return 0; + return data; } static uint64_t arm_ldq_ptw(CPUARMState *env, S1Translate *ptw, hwaddr addr, ARMMMUFaultInfo *fi) { CPUState *cs = env_cpu(env); - MemTxAttrs attrs = {}; - MemTxResult result = MEMTX_OK; - AddressSpace *as; uint64_t data; if (!S1_ptw_translate(env, ptw, addr, fi)) { + /* Failure. */ + assert(fi->s1ptw); return 0; } - addr = ptw->out_phys; - attrs.secure = ptw->out_secure; - as = arm_addressspace(cs, attrs); - if (ptw->out_be) { - data = address_space_ldq_be(as, addr, attrs, &result); + + if (likely(ptw->out_host)) { + /* Page tables are in RAM, and we have the host address. */ + if (ptw->out_be) { + data = ldq_be_p(ptw->out_host); + } else { + data = ldq_le_p(ptw->out_host); + } } else { - data = address_space_ldq_le(as, addr, attrs, &result); + /* Page tables are in MMIO. */ + MemTxAttrs attrs = { .secure = ptw->out_secure }; + AddressSpace *as = arm_addressspace(cs, attrs); + MemTxResult result = MEMTX_OK; + + if (ptw->out_be) { + data = address_space_ldq_be(as, ptw->out_phys, attrs, &result); + } else { + data = address_space_ldq_le(as, ptw->out_phys, attrs, &result); + } + if (unlikely(result != MEMTX_OK)) { + fi->type = ARMFault_SyncExternalOnWalk; + fi->ea = arm_extabort_type(result); + return 0; + } } - if (result == MEMTX_OK) { - return data; - } - fi->type = ARMFault_SyncExternalOnWalk; - fi->ea = arm_extabort_type(result); - return 0; + return data; } static bool get_level1_table_address(CPUARMState *env, ARMMMUIdx mmu_idx, 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, bool probe, uintptr_t retaddr) { ARMCPU *cpu = ARM_CPU(cs); - ARMMMUFaultInfo fi = {}; GetPhysAddrResult res = {}; + ARMMMUFaultInfo local_fi, *fi; int ret; + /* + * Allow S1_ptw_translate to see any fault generated here. + * Since this may recurse, read and clear. + */ + fi = cpu->env.tlb_fi; + if (fi) { + cpu->env.tlb_fi = NULL; + } else { + fi = memset(&local_fi, 0, sizeof(local_fi)); + } + /* * Walk the page table and (if the mapping exists) add the page * to the TLB. On success, return true. Otherwise, if probing, @@ -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), - &res, &fi); + &res, fi); if (likely(!ret)) { /* * Map a single [sub]page. Regions smaller than our declared @@ -XXX,XX +XXX,XX @@ bool arm_cpu_tlb_fill(CPUState *cs, vaddr address, int size, } else { /* now we have a real cpu fault */ cpu_restore_state(cs, retaddr, true); - arm_deliver_fault(cpu, address, access_type, mmu_idx, &fi); + arm_deliver_fault(cpu, address, access_type, mmu_idx, fi); } } #else -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20221011031911.2408754-12-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/ptw.c | 191 +++++++++++++++++++++++++---------------------- 1 file changed, 100 insertions(+), 91 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, S1Translate *ptw, GetPhysAddrResult *result, ARMMMUFaultInfo *fi) __attribute__((nonnull)); +static bool get_phys_addr_with_struct(CPUARMState *env, S1Translate *ptw, + target_ulong address, + MMUAccessType access_type, + GetPhysAddrResult *result, + ARMMMUFaultInfo *fi) + __attribute__((nonnull)); + /* This mapping is common between ID_AA64MMFR0.PARANGE and TCR_ELx.{I}PS. */ static const uint8_t pamax_map[] = { [0] = 32, @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_disabled(CPUARMState *env, target_ulong address, return 0; } +static bool get_phys_addr_twostage(CPUARMState *env, S1Translate *ptw, + target_ulong address, + MMUAccessType access_type, + GetPhysAddrResult *result, + ARMMMUFaultInfo *fi) +{ + hwaddr ipa; + int s1_prot; + int ret; + bool is_secure = ptw->in_secure; + bool ipa_secure, s2walk_secure; + ARMCacheAttrs cacheattrs1; + bool is_el0; + uint64_t hcr; + + ret = get_phys_addr_with_struct(env, ptw, address, access_type, result, fi); + + /* If S1 fails or S2 is disabled, return early. */ + if (ret || regime_translation_disabled(env, ARMMMUIdx_Stage2, is_secure)) { + return ret; + } + + ipa = result->f.phys_addr; + ipa_secure = result->f.attrs.secure; + if (is_secure) { + /* Select TCR based on the NS bit from the S1 walk. */ + s2walk_secure = !(ipa_secure + ? env->cp15.vstcr_el2 & VSTCR_SW + : env->cp15.vtcr_el2 & VTCR_NSW); + } else { + assert(!ipa_secure); + s2walk_secure = false; + } + + is_el0 = ptw->in_mmu_idx == ARMMMUIdx_Stage1_E0; + ptw->in_mmu_idx = s2walk_secure ? ARMMMUIdx_Stage2_S : ARMMMUIdx_Stage2; + ptw->in_secure = s2walk_secure; + + /* + * S1 is done, now do S2 translation. + * Save the stage1 results so that we may merge prot and cacheattrs later. + */ + s1_prot = result->f.prot; + cacheattrs1 = result->cacheattrs; + memset(result, 0, sizeof(*result)); + + ret = get_phys_addr_lpae(env, ptw, ipa, access_type, is_el0, result, fi); + fi->s2addr = ipa; + + /* Combine the S1 and S2 perms. */ + result->f.prot &= s1_prot; + + /* If S2 fails, return early. */ + if (ret) { + return ret; + } + + /* Combine the S1 and S2 cache attributes. */ + hcr = arm_hcr_el2_eff_secstate(env, is_secure); + if (hcr & HCR_DC) { + /* + * HCR.DC forces the first stage attributes to + * Normal Non-Shareable, + * Inner Write-Back Read-Allocate Write-Allocate, + * Outer Write-Back Read-Allocate Write-Allocate. + * Do not overwrite Tagged within attrs. + */ + if (cacheattrs1.attrs != 0xf0) { + cacheattrs1.attrs = 0xff; + } + cacheattrs1.shareability = 0; + } + result->cacheattrs = combine_cacheattrs(hcr, cacheattrs1, + result->cacheattrs); + + /* + * Check if IPA translates to secure or non-secure PA space. + * Note that VSTCR overrides VTCR and {N}SW overrides {N}SA. + */ + result->f.attrs.secure = + (is_secure + && !(env->cp15.vstcr_el2 & (VSTCR_SA | VSTCR_SW)) + && (ipa_secure + || !(env->cp15.vtcr_el2 & (VTCR_NSA | VTCR_NSW)))); + + return 0; +} + static bool get_phys_addr_with_struct(CPUARMState *env, S1Translate *ptw, target_ulong address, MMUAccessType access_type, @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_with_struct(CPUARMState *env, S1Translate *ptw, if (mmu_idx != s1_mmu_idx) { /* * Call ourselves recursively to do the stage 1 and then stage 2 - * translations if mmu_idx is a two-stage regime. + * translations if mmu_idx is a two-stage regime, and EL2 present. + * Otherwise, a stage1+stage2 translation is just stage 1. */ + ptw->in_mmu_idx = mmu_idx = s1_mmu_idx; if (arm_feature(env, ARM_FEATURE_EL2)) { - hwaddr ipa; - int s1_prot; - int ret; - bool ipa_secure, s2walk_secure; - ARMCacheAttrs cacheattrs1; - bool is_el0; - uint64_t hcr; - - ptw->in_mmu_idx = s1_mmu_idx; - ret = get_phys_addr_with_struct(env, ptw, address, access_type, - result, fi); - - /* If S1 fails or S2 is disabled, return early. */ - if (ret || regime_translation_disabled(env, ARMMMUIdx_Stage2, - is_secure)) { - return ret; - } - - ipa = result->f.phys_addr; - ipa_secure = result->f.attrs.secure; - if (is_secure) { - /* Select TCR based on the NS bit from the S1 walk. */ - s2walk_secure = !(ipa_secure - ? env->cp15.vstcr_el2 & VSTCR_SW - : env->cp15.vtcr_el2 & VTCR_NSW); - } else { - assert(!ipa_secure); - s2walk_secure = false; - } - - ptw->in_mmu_idx = - s2walk_secure ? ARMMMUIdx_Stage2_S : ARMMMUIdx_Stage2; - ptw->in_secure = s2walk_secure; - is_el0 = mmu_idx == ARMMMUIdx_E10_0; - - /* - * S1 is done, now do S2 translation. - * Save the stage1 results so that we may merge - * prot and cacheattrs later. - */ - s1_prot = result->f.prot; - cacheattrs1 = result->cacheattrs; - memset(result, 0, sizeof(*result)); - - ret = get_phys_addr_lpae(env, ptw, ipa, access_type, - is_el0, result, fi); - fi->s2addr = ipa; - - /* Combine the S1 and S2 perms. */ - result->f.prot &= s1_prot; - - /* If S2 fails, return early. */ - if (ret) { - return ret; - } - - /* Combine the S1 and S2 cache attributes. */ - hcr = arm_hcr_el2_eff_secstate(env, is_secure); - if (hcr & HCR_DC) { - /* - * HCR.DC forces the first stage attributes to - * Normal Non-Shareable, - * Inner Write-Back Read-Allocate Write-Allocate, - * Outer Write-Back Read-Allocate Write-Allocate. - * Do not overwrite Tagged within attrs. - */ - if (cacheattrs1.attrs != 0xf0) { - cacheattrs1.attrs = 0xff; - } - cacheattrs1.shareability = 0; - } - result->cacheattrs = combine_cacheattrs(hcr, cacheattrs1, - result->cacheattrs); - - /* - * Check if IPA translates to secure or non-secure PA space. - * Note that VSTCR overrides VTCR and {N}SW overrides {N}SA. - */ - result->f.attrs.secure = - (is_secure - && !(env->cp15.vstcr_el2 & (VSTCR_SA | VSTCR_SW)) - && (ipa_secure - || !(env->cp15.vtcr_el2 & (VTCR_NSA | VTCR_NSW)))); - - return 0; - } else { - /* - * For non-EL2 CPUs a stage1+stage2 translation is just stage 1. - */ - mmu_idx = stage_1_mmu_idx(mmu_idx); + return get_phys_addr_twostage(env, ptw, address, access_type, + result, fi); } } -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> The return type of the functions is already bool, but in a few instances we used an integer type with the return statement. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20221011031911.2408754-13-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/ptw.c | 7 +++---- 1 file changed, 3 insertions(+), 4 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_disabled(CPUARMState *env, target_ulong address, result->f.lg_page_size = TARGET_PAGE_BITS; result->cacheattrs.shareability = shareability; result->cacheattrs.attrs = memattr; - return 0; + return false; } static bool get_phys_addr_twostage(CPUARMState *env, S1Translate *ptw, @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_twostage(CPUARMState *env, S1Translate *ptw, { hwaddr ipa; int s1_prot; - int ret; bool is_secure = ptw->in_secure; - bool ipa_secure, s2walk_secure; + bool ret, ipa_secure, s2walk_secure; ARMCacheAttrs cacheattrs1; bool is_el0; uint64_t hcr; @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_twostage(CPUARMState *env, S1Translate *ptw, && (ipa_secure || !(env->cp15.vtcr_el2 & (VTCR_NSA | VTCR_NSW)))); - return 0; + return false; } static bool get_phys_addr_with_struct(CPUARMState *env, S1Translate *ptw, -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> A simple helper to retrieve the length of the current insn. Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20221020030641.2066807-2-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/translate.h | 5 +++++ target/arm/translate-vfp.c | 2 +- target/arm/translate.c | 5 ++--- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/target/arm/translate.h b/target/arm/translate.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate.h +++ b/target/arm/translate.h @@ -XXX,XX +XXX,XX @@ static inline void disas_set_insn_syndrome(DisasContext *s, uint32_t syn) s->insn_start = NULL; } +static inline int curr_insn_len(DisasContext *s) +{ + return s->base.pc_next - s->pc_curr; +} + /* is_jmp field values */ #define DISAS_JUMP DISAS_TARGET_0 /* only pc was modified dynamically */ /* CPU state was modified dynamically; exit to main loop for interrupts. */ diff --git a/target/arm/translate-vfp.c b/target/arm/translate-vfp.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-vfp.c +++ b/target/arm/translate-vfp.c @@ -XXX,XX +XXX,XX @@ static bool vfp_access_check_a(DisasContext *s, bool ignore_vfp_enabled) if (s->sme_trap_nonstreaming) { gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_smetrap(SME_ET_Streaming, - s->base.pc_next - s->pc_curr == 2)); + curr_insn_len(s) == 2)); return false; } 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 ISSInfo make_issinfo(DisasContext *s, int rd, bool p, bool w) /* ISS not valid if writeback */ if (p && !w) { ret = rd; - if (s->base.pc_next - s->pc_curr == 2) { + if (curr_insn_len(s) == 2) { ret |= ISSIs16Bit; } } else { @@ -XXX,XX +XXX,XX @@ static void arm_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu) /* nothing more to generate */ break; case DISAS_WFI: - gen_helper_wfi(cpu_env, - tcg_constant_i32(dc->base.pc_next - dc->pc_curr)); + gen_helper_wfi(cpu_env, tcg_constant_i32(curr_insn_len(dc))); /* * The helper doesn't necessarily throw an exception, but we * must go back to the main loop to check for interrupts anyway. -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> In preparation for TARGET_TB_PCREL, reduce reliance on absolute values. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20221020030641.2066807-3-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/translate-a64.c | 40 ++++++++++++++++++++------------------ target/arm/translate.c | 10 ++++++---- 2 files changed, 27 insertions(+), 23 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 inline bool use_goto_tb(DisasContext *s, uint64_t dest) return translator_use_goto_tb(&s->base, dest); } -static inline void gen_goto_tb(DisasContext *s, int n, uint64_t dest) +static void gen_goto_tb(DisasContext *s, int n, int64_t diff) { + uint64_t dest = s->pc_curr + diff; + if (use_goto_tb(s, dest)) { tcg_gen_goto_tb(n); gen_a64_set_pc_im(dest); @@ -XXX,XX +XXX,XX @@ static inline AArch64DecodeFn *lookup_disas_fn(const AArch64DecodeTable *table, */ static void disas_uncond_b_imm(DisasContext *s, uint32_t insn) { - uint64_t addr = s->pc_curr + sextract32(insn, 0, 26) * 4; + int64_t diff = sextract32(insn, 0, 26) * 4; if (insn & (1U << 31)) { /* BL Branch with link */ @@ -XXX,XX +XXX,XX @@ static void disas_uncond_b_imm(DisasContext *s, uint32_t insn) /* B Branch / BL Branch with link */ reset_btype(s); - gen_goto_tb(s, 0, addr); + gen_goto_tb(s, 0, diff); } /* Compare and branch (immediate) @@ -XXX,XX +XXX,XX @@ static void disas_uncond_b_imm(DisasContext *s, uint32_t insn) static void disas_comp_b_imm(DisasContext *s, uint32_t insn) { unsigned int sf, op, rt; - uint64_t addr; + int64_t diff; TCGLabel *label_match; TCGv_i64 tcg_cmp; sf = extract32(insn, 31, 1); op = extract32(insn, 24, 1); /* 0: CBZ; 1: CBNZ */ rt = extract32(insn, 0, 5); - addr = s->pc_curr + sextract32(insn, 5, 19) * 4; + diff = sextract32(insn, 5, 19) * 4; tcg_cmp = read_cpu_reg(s, rt, sf); label_match = gen_new_label(); @@ -XXX,XX +XXX,XX @@ static void disas_comp_b_imm(DisasContext *s, uint32_t insn) tcg_gen_brcondi_i64(op ? TCG_COND_NE : TCG_COND_EQ, tcg_cmp, 0, label_match); - gen_goto_tb(s, 0, s->base.pc_next); + gen_goto_tb(s, 0, 4); gen_set_label(label_match); - gen_goto_tb(s, 1, addr); + gen_goto_tb(s, 1, diff); } /* Test and branch (immediate) @@ -XXX,XX +XXX,XX @@ static void disas_comp_b_imm(DisasContext *s, uint32_t insn) static void disas_test_b_imm(DisasContext *s, uint32_t insn) { unsigned int bit_pos, op, rt; - uint64_t addr; + int64_t diff; TCGLabel *label_match; TCGv_i64 tcg_cmp; bit_pos = (extract32(insn, 31, 1) << 5) | extract32(insn, 19, 5); op = extract32(insn, 24, 1); /* 0: TBZ; 1: TBNZ */ - addr = s->pc_curr + sextract32(insn, 5, 14) * 4; + diff = sextract32(insn, 5, 14) * 4; rt = extract32(insn, 0, 5); tcg_cmp = tcg_temp_new_i64(); @@ -XXX,XX +XXX,XX @@ static void disas_test_b_imm(DisasContext *s, uint32_t insn) tcg_gen_brcondi_i64(op ? TCG_COND_NE : TCG_COND_EQ, tcg_cmp, 0, label_match); tcg_temp_free_i64(tcg_cmp); - gen_goto_tb(s, 0, s->base.pc_next); + gen_goto_tb(s, 0, 4); gen_set_label(label_match); - gen_goto_tb(s, 1, addr); + gen_goto_tb(s, 1, diff); } /* Conditional branch (immediate) @@ -XXX,XX +XXX,XX @@ static void disas_test_b_imm(DisasContext *s, uint32_t insn) static void disas_cond_b_imm(DisasContext *s, uint32_t insn) { unsigned int cond; - uint64_t addr; + int64_t diff; if ((insn & (1 << 4)) || (insn & (1 << 24))) { unallocated_encoding(s); return; } - addr = s->pc_curr + sextract32(insn, 5, 19) * 4; + diff = sextract32(insn, 5, 19) * 4; cond = extract32(insn, 0, 4); reset_btype(s); @@ -XXX,XX +XXX,XX @@ static void disas_cond_b_imm(DisasContext *s, uint32_t insn) /* genuinely conditional branches */ TCGLabel *label_match = gen_new_label(); arm_gen_test_cc(cond, label_match); - gen_goto_tb(s, 0, s->base.pc_next); + gen_goto_tb(s, 0, 4); gen_set_label(label_match); - gen_goto_tb(s, 1, addr); + gen_goto_tb(s, 1, diff); } else { /* 0xe and 0xf are both "always" conditions */ - gen_goto_tb(s, 0, addr); + gen_goto_tb(s, 0, diff); } } @@ -XXX,XX +XXX,XX @@ static void handle_sync(DisasContext *s, uint32_t insn, * any pending interrupts immediately. */ reset_btype(s); - gen_goto_tb(s, 0, s->base.pc_next); + gen_goto_tb(s, 0, 4); return; case 7: /* SB */ @@ -XXX,XX +XXX,XX @@ static void handle_sync(DisasContext *s, uint32_t insn, * MB and end the TB instead. */ tcg_gen_mb(TCG_MO_ALL | TCG_BAR_SC); - gen_goto_tb(s, 0, s->base.pc_next); + gen_goto_tb(s, 0, 4); return; default: @@ -XXX,XX +XXX,XX @@ static void aarch64_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu) switch (dc->base.is_jmp) { case DISAS_NEXT: case DISAS_TOO_MANY: - gen_goto_tb(dc, 1, dc->base.pc_next); + gen_goto_tb(dc, 1, 4); break; default: case DISAS_UPDATE_EXIT: diff --git a/target/arm/translate.c b/target/arm/translate.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -XXX,XX +XXX,XX @@ static void gen_goto_ptr(void) * cpu_loop_exec. Any live exit_requests will be processed as we * enter the next TB. */ -static void gen_goto_tb(DisasContext *s, int n, target_ulong dest) +static void gen_goto_tb(DisasContext *s, int n, int diff) { + target_ulong dest = s->pc_curr + diff; + if (translator_use_goto_tb(&s->base, dest)) { tcg_gen_goto_tb(n); gen_set_pc_im(s, dest); @@ -XXX,XX +XXX,XX @@ static inline void gen_jmp_tb(DisasContext *s, uint32_t dest, int tbno) * gen_jmp(); * on the second call to gen_jmp(). */ - gen_goto_tb(s, tbno, dest); + gen_goto_tb(s, tbno, dest - s->pc_curr); break; case DISAS_UPDATE_NOCHAIN: case DISAS_UPDATE_EXIT: @@ -XXX,XX +XXX,XX @@ static void arm_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu) switch (dc->base.is_jmp) { case DISAS_NEXT: case DISAS_TOO_MANY: - gen_goto_tb(dc, 1, dc->base.pc_next); + gen_goto_tb(dc, 1, curr_insn_len(dc)); break; case DISAS_UPDATE_NOCHAIN: gen_set_pc_im(dc, dc->base.pc_next); @@ -XXX,XX +XXX,XX @@ static void arm_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu) gen_set_pc_im(dc, dc->base.pc_next); gen_singlestep_exception(dc); } else { - gen_goto_tb(dc, 1, dc->base.pc_next); + gen_goto_tb(dc, 1, curr_insn_len(dc)); } } } -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> In preparation for TARGET_TB_PCREL, reduce reliance on absolute values by passing in pc difference. Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20221020030641.2066807-4-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/translate-a32.h | 2 +- target/arm/translate.h | 6 ++-- target/arm/translate-a64.c | 32 +++++++++--------- target/arm/translate-vfp.c | 2 +- target/arm/translate.c | 68 ++++++++++++++++++++------------------ 5 files changed, 56 insertions(+), 54 deletions(-) diff --git a/target/arm/translate-a32.h b/target/arm/translate-a32.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-a32.h +++ b/target/arm/translate-a32.h @@ -XXX,XX +XXX,XX @@ void write_neon_element64(TCGv_i64 src, int reg, int ele, MemOp memop); TCGv_i32 add_reg_for_lit(DisasContext *s, int reg, int ofs); void gen_set_cpsr(TCGv_i32 var, uint32_t mask); void gen_set_condexec(DisasContext *s); -void gen_set_pc_im(DisasContext *s, target_ulong val); +void gen_update_pc(DisasContext *s, target_long diff); void gen_lookup_tb(DisasContext *s); long vfp_reg_offset(bool dp, unsigned reg); long neon_full_reg_offset(unsigned reg); diff --git a/target/arm/translate.h b/target/arm/translate.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate.h +++ b/target/arm/translate.h @@ -XXX,XX +XXX,XX @@ static inline int curr_insn_len(DisasContext *s) * For instructions which want an immediate exit to the main loop, as opposed * to attempting to use lookup_and_goto_ptr. Unlike DISAS_UPDATE_EXIT, this * doesn't write the PC on exiting the translation loop so you need to ensure - * something (gen_a64_set_pc_im or runtime helper) has done so before we reach + * something (gen_a64_update_pc or runtime helper) has done so before we reach * return from cpu_tb_exec. */ #define DISAS_EXIT DISAS_TARGET_9 @@ -XXX,XX +XXX,XX @@ static inline int curr_insn_len(DisasContext *s) #ifdef TARGET_AARCH64 void a64_translate_init(void); -void gen_a64_set_pc_im(uint64_t val); +void gen_a64_update_pc(DisasContext *s, target_long diff); extern const TranslatorOps aarch64_translator_ops; #else static inline void a64_translate_init(void) { } -static inline void gen_a64_set_pc_im(uint64_t val) +static inline void gen_a64_update_pc(DisasContext *s, target_long diff) { } #endif 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 reset_btype(DisasContext *s) } } -void gen_a64_set_pc_im(uint64_t val) +void gen_a64_update_pc(DisasContext *s, target_long diff) { - tcg_gen_movi_i64(cpu_pc, val); + tcg_gen_movi_i64(cpu_pc, s->pc_curr + diff); } /* @@ -XXX,XX +XXX,XX @@ static void gen_exception_internal(int excp) static void gen_exception_internal_insn(DisasContext *s, uint64_t pc, int excp) { - gen_a64_set_pc_im(pc); + gen_a64_update_pc(s, pc - s->pc_curr); gen_exception_internal(excp); s->base.is_jmp = DISAS_NORETURN; } static void gen_exception_bkpt_insn(DisasContext *s, uint32_t syndrome) { - gen_a64_set_pc_im(s->pc_curr); + gen_a64_update_pc(s, 0); gen_helper_exception_bkpt_insn(cpu_env, tcg_constant_i32(syndrome)); s->base.is_jmp = DISAS_NORETURN; } @@ -XXX,XX +XXX,XX @@ static void gen_goto_tb(DisasContext *s, int n, int64_t diff) if (use_goto_tb(s, dest)) { tcg_gen_goto_tb(n); - gen_a64_set_pc_im(dest); + gen_a64_update_pc(s, diff); tcg_gen_exit_tb(s->base.tb, n); s->base.is_jmp = DISAS_NORETURN; } else { - gen_a64_set_pc_im(dest); + gen_a64_update_pc(s, diff); if (s->ss_active) { gen_step_complete_exception(s); } else { @@ -XXX,XX +XXX,XX @@ static void handle_sys(DisasContext *s, uint32_t insn, bool isread, uint32_t syndrome; syndrome = syn_aa64_sysregtrap(op0, op1, op2, crn, crm, rt, isread); - gen_a64_set_pc_im(s->pc_curr); + gen_a64_update_pc(s, 0); gen_helper_access_check_cp_reg(cpu_env, tcg_constant_ptr(ri), tcg_constant_i32(syndrome), @@ -XXX,XX +XXX,XX @@ static void handle_sys(DisasContext *s, uint32_t insn, bool isread, * The readfn or writefn might raise an exception; * synchronize the CPU state in case it does. */ - gen_a64_set_pc_im(s->pc_curr); + gen_a64_update_pc(s, 0); } /* Handle special cases first */ @@ -XXX,XX +XXX,XX @@ static void disas_exc(DisasContext *s, uint32_t insn) /* The pre HVC helper handles cases when HVC gets trapped * as an undefined insn by runtime configuration. */ - gen_a64_set_pc_im(s->pc_curr); + gen_a64_update_pc(s, 0); gen_helper_pre_hvc(cpu_env); gen_ss_advance(s); gen_exception_insn_el(s, s->base.pc_next, EXCP_HVC, @@ -XXX,XX +XXX,XX @@ static void disas_exc(DisasContext *s, uint32_t insn) unallocated_encoding(s); break; } - gen_a64_set_pc_im(s->pc_curr); + gen_a64_update_pc(s, 0); gen_helper_pre_smc(cpu_env, tcg_constant_i32(syn_aa64_smc(imm16))); gen_ss_advance(s); gen_exception_insn_el(s, s->base.pc_next, EXCP_SMC, @@ -XXX,XX +XXX,XX @@ static void aarch64_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu) */ switch (dc->base.is_jmp) { default: - gen_a64_set_pc_im(dc->base.pc_next); + gen_a64_update_pc(dc, 4); /* fall through */ case DISAS_EXIT: case DISAS_JUMP: @@ -XXX,XX +XXX,XX @@ static void aarch64_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu) break; default: case DISAS_UPDATE_EXIT: - gen_a64_set_pc_im(dc->base.pc_next); + gen_a64_update_pc(dc, 4); /* fall through */ case DISAS_EXIT: tcg_gen_exit_tb(NULL, 0); break; case DISAS_UPDATE_NOCHAIN: - gen_a64_set_pc_im(dc->base.pc_next); + gen_a64_update_pc(dc, 4); /* fall through */ case DISAS_JUMP: tcg_gen_lookup_and_goto_ptr(); @@ -XXX,XX +XXX,XX @@ static void aarch64_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu) case DISAS_SWI: break; case DISAS_WFE: - gen_a64_set_pc_im(dc->base.pc_next); + gen_a64_update_pc(dc, 4); gen_helper_wfe(cpu_env); break; case DISAS_YIELD: - gen_a64_set_pc_im(dc->base.pc_next); + gen_a64_update_pc(dc, 4); gen_helper_yield(cpu_env); break; case DISAS_WFI: @@ -XXX,XX +XXX,XX @@ static void aarch64_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu) * This is a special case because we don't want to just halt * the CPU if trying to debug across a WFI. */ - gen_a64_set_pc_im(dc->base.pc_next); + gen_a64_update_pc(dc, 4); gen_helper_wfi(cpu_env, tcg_constant_i32(4)); /* * The helper doesn't necessarily throw an exception, but we diff --git a/target/arm/translate-vfp.c b/target/arm/translate-vfp.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-vfp.c +++ b/target/arm/translate-vfp.c @@ -XXX,XX +XXX,XX @@ static bool trans_VMSR_VMRS(DisasContext *s, arg_VMSR_VMRS *a) case ARM_VFP_FPSID: if (s->current_el == 1) { gen_set_condexec(s); - gen_set_pc_im(s, s->pc_curr); + gen_update_pc(s, 0); gen_helper_check_hcr_el2_trap(cpu_env, tcg_constant_i32(a->rt), tcg_constant_i32(a->reg)); diff --git a/target/arm/translate.c b/target/arm/translate.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -XXX,XX +XXX,XX @@ void gen_set_condexec(DisasContext *s) } } -void gen_set_pc_im(DisasContext *s, target_ulong val) +void gen_update_pc(DisasContext *s, target_long diff) { - tcg_gen_movi_i32(cpu_R[15], val); + tcg_gen_movi_i32(cpu_R[15], s->pc_curr + diff); } /* Set PC and Thumb state from var. var is marked as dead. */ @@ -XXX,XX +XXX,XX @@ static inline void gen_bxns(DisasContext *s, int rm) /* The bxns helper may raise an EXCEPTION_EXIT exception, so in theory * we need to sync state before calling it, but: - * - we don't need to do gen_set_pc_im() because the bxns helper will + * - we don't need to do gen_update_pc() because the bxns helper will * always set the PC itself * - we don't need to do gen_set_condexec() because BXNS is UNPREDICTABLE * unless it's outside an IT block or the last insn in an IT block, @@ -XXX,XX +XXX,XX @@ static inline void gen_blxns(DisasContext *s, int rm) * We do however need to set the PC, because the blxns helper reads it. * The blxns helper may throw an exception. */ - gen_set_pc_im(s, s->base.pc_next); + gen_update_pc(s, curr_insn_len(s)); gen_helper_v7m_blxns(cpu_env, var); tcg_temp_free_i32(var); s->base.is_jmp = DISAS_EXIT; @@ -XXX,XX +XXX,XX @@ static inline void gen_hvc(DisasContext *s, int imm16) * as an undefined insn by runtime configuration (ie before * the insn really executes). */ - gen_set_pc_im(s, s->pc_curr); + gen_update_pc(s, 0); gen_helper_pre_hvc(cpu_env); /* Otherwise we will treat this as a real exception which * happens after execution of the insn. (The distinction matters @@ -XXX,XX +XXX,XX @@ static inline void gen_hvc(DisasContext *s, int imm16) * for single stepping.) */ s->svc_imm = imm16; - gen_set_pc_im(s, s->base.pc_next); + gen_update_pc(s, curr_insn_len(s)); s->base.is_jmp = DISAS_HVC; } @@ -XXX,XX +XXX,XX @@ static inline void gen_smc(DisasContext *s) /* As with HVC, we may take an exception either before or after * the insn executes. */ - gen_set_pc_im(s, s->pc_curr); + gen_update_pc(s, 0); gen_helper_pre_smc(cpu_env, tcg_constant_i32(syn_aa32_smc())); - gen_set_pc_im(s, s->base.pc_next); + gen_update_pc(s, curr_insn_len(s)); s->base.is_jmp = DISAS_SMC; } static void gen_exception_internal_insn(DisasContext *s, uint32_t pc, int excp) { gen_set_condexec(s); - gen_set_pc_im(s, pc); + gen_update_pc(s, pc - s->pc_curr); gen_exception_internal(excp); s->base.is_jmp = DISAS_NORETURN; } @@ -XXX,XX +XXX,XX @@ static void gen_exception_insn_el_v(DisasContext *s, uint64_t pc, int excp, uint32_t syn, TCGv_i32 tcg_el) { if (s->aarch64) { - gen_a64_set_pc_im(pc); + gen_a64_update_pc(s, pc - s->pc_curr); } else { gen_set_condexec(s); - gen_set_pc_im(s, pc); + gen_update_pc(s, pc - s->pc_curr); } gen_exception_el_v(excp, syn, tcg_el); s->base.is_jmp = DISAS_NORETURN; @@ -XXX,XX +XXX,XX @@ void gen_exception_insn_el(DisasContext *s, uint64_t pc, int excp, void gen_exception_insn(DisasContext *s, uint64_t pc, int excp, uint32_t syn) { if (s->aarch64) { - gen_a64_set_pc_im(pc); + gen_a64_update_pc(s, pc - s->pc_curr); } else { gen_set_condexec(s); - gen_set_pc_im(s, pc); + gen_update_pc(s, pc - s->pc_curr); } gen_exception(excp, syn); s->base.is_jmp = DISAS_NORETURN; @@ -XXX,XX +XXX,XX @@ void gen_exception_insn(DisasContext *s, uint64_t pc, int excp, uint32_t syn) static void gen_exception_bkpt_insn(DisasContext *s, uint32_t syn) { gen_set_condexec(s); - gen_set_pc_im(s, s->pc_curr); + gen_update_pc(s, 0); gen_helper_exception_bkpt_insn(cpu_env, tcg_constant_i32(syn)); s->base.is_jmp = DISAS_NORETURN; } @@ -XXX,XX +XXX,XX @@ static void gen_goto_tb(DisasContext *s, int n, int diff) if (translator_use_goto_tb(&s->base, dest)) { tcg_gen_goto_tb(n); - gen_set_pc_im(s, dest); + gen_update_pc(s, diff); tcg_gen_exit_tb(s->base.tb, n); } else { - gen_set_pc_im(s, dest); + gen_update_pc(s, diff); gen_goto_ptr(); } s->base.is_jmp = DISAS_NORETURN; @@ -XXX,XX +XXX,XX @@ static void gen_goto_tb(DisasContext *s, int n, int diff) /* Jump, specifying which TB number to use if we gen_goto_tb() */ static inline void gen_jmp_tb(DisasContext *s, uint32_t dest, int tbno) { + int diff = dest - s->pc_curr; + if (unlikely(s->ss_active)) { /* An indirect jump so that we still trigger the debug exception. */ - gen_set_pc_im(s, dest); + gen_update_pc(s, diff); s->base.is_jmp = DISAS_JUMP; return; } @@ -XXX,XX +XXX,XX @@ static inline void gen_jmp_tb(DisasContext *s, uint32_t dest, int tbno) * gen_jmp(); * on the second call to gen_jmp(). */ - gen_goto_tb(s, tbno, dest - s->pc_curr); + gen_goto_tb(s, tbno, diff); break; case DISAS_UPDATE_NOCHAIN: case DISAS_UPDATE_EXIT: @@ -XXX,XX +XXX,XX @@ static inline void gen_jmp_tb(DisasContext *s, uint32_t dest, int tbno) * Avoid using goto_tb so we really do exit back to the main loop * and don't chain to another TB. */ - gen_set_pc_im(s, dest); + gen_update_pc(s, diff); gen_goto_ptr(); s->base.is_jmp = DISAS_NORETURN; break; @@ -XXX,XX +XXX,XX @@ static void gen_msr_banked(DisasContext *s, int r, int sysm, int rn) /* Sync state because msr_banked() can raise exceptions */ gen_set_condexec(s); - gen_set_pc_im(s, s->pc_curr); + gen_update_pc(s, 0); tcg_reg = load_reg(s, rn); gen_helper_msr_banked(cpu_env, tcg_reg, tcg_constant_i32(tgtmode), @@ -XXX,XX +XXX,XX @@ static void gen_mrs_banked(DisasContext *s, int r, int sysm, int rn) /* Sync state because mrs_banked() can raise exceptions */ gen_set_condexec(s); - gen_set_pc_im(s, s->pc_curr); + gen_update_pc(s, 0); tcg_reg = tcg_temp_new_i32(); gen_helper_mrs_banked(tcg_reg, cpu_env, tcg_constant_i32(tgtmode), @@ -XXX,XX +XXX,XX @@ static void do_coproc_insn(DisasContext *s, int cpnum, int is64, } gen_set_condexec(s); - gen_set_pc_im(s, s->pc_curr); + gen_update_pc(s, 0); gen_helper_access_check_cp_reg(cpu_env, tcg_constant_ptr(ri), tcg_constant_i32(syndrome), @@ -XXX,XX +XXX,XX @@ static void do_coproc_insn(DisasContext *s, int cpnum, int is64, * synchronize the CPU state in case it does. */ gen_set_condexec(s); - gen_set_pc_im(s, s->pc_curr); + gen_update_pc(s, 0); } /* Handle special cases first */ @@ -XXX,XX +XXX,XX @@ static void do_coproc_insn(DisasContext *s, int cpnum, int is64, unallocated_encoding(s); return; } - gen_set_pc_im(s, s->base.pc_next); + gen_update_pc(s, curr_insn_len(s)); s->base.is_jmp = DISAS_WFI; return; default: @@ -XXX,XX +XXX,XX @@ static void gen_srs(DisasContext *s, addr = tcg_temp_new_i32(); /* get_r13_banked() will raise an exception if called from System mode */ gen_set_condexec(s); - gen_set_pc_im(s, s->pc_curr); + gen_update_pc(s, 0); gen_helper_get_r13_banked(addr, cpu_env, tcg_constant_i32(mode)); switch (amode) { case 0: /* DA */ @@ -XXX,XX +XXX,XX @@ static bool trans_YIELD(DisasContext *s, arg_YIELD *a) * scheduling of other vCPUs. */ if (!(tb_cflags(s->base.tb) & CF_PARALLEL)) { - gen_set_pc_im(s, s->base.pc_next); + gen_update_pc(s, curr_insn_len(s)); s->base.is_jmp = DISAS_YIELD; } return true; @@ -XXX,XX +XXX,XX @@ static bool trans_WFE(DisasContext *s, arg_WFE *a) * implemented so we can't sleep like WFI does. */ if (!(tb_cflags(s->base.tb) & CF_PARALLEL)) { - gen_set_pc_im(s, s->base.pc_next); + gen_update_pc(s, curr_insn_len(s)); s->base.is_jmp = DISAS_WFE; } return true; @@ -XXX,XX +XXX,XX @@ static bool trans_WFE(DisasContext *s, arg_WFE *a) static bool trans_WFI(DisasContext *s, arg_WFI *a) { /* For WFI, halt the vCPU until an IRQ. */ - gen_set_pc_im(s, s->base.pc_next); + gen_update_pc(s, curr_insn_len(s)); s->base.is_jmp = DISAS_WFI; return true; } @@ -XXX,XX +XXX,XX @@ static bool trans_SVC(DisasContext *s, arg_SVC *a) (a->imm == semihost_imm)) { gen_exception_internal_insn(s, s->pc_curr, EXCP_SEMIHOST); } else { - gen_set_pc_im(s, s->base.pc_next); + gen_update_pc(s, curr_insn_len(s)); s->svc_imm = a->imm; s->base.is_jmp = DISAS_SWI; } @@ -XXX,XX +XXX,XX @@ static void arm_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu) case DISAS_TOO_MANY: case DISAS_UPDATE_EXIT: case DISAS_UPDATE_NOCHAIN: - gen_set_pc_im(dc, dc->base.pc_next); + gen_update_pc(dc, curr_insn_len(dc)); /* fall through */ default: /* FIXME: Single stepping a WFI insn will not halt the CPU. */ @@ -XXX,XX +XXX,XX @@ static void arm_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu) gen_goto_tb(dc, 1, curr_insn_len(dc)); break; case DISAS_UPDATE_NOCHAIN: - gen_set_pc_im(dc, dc->base.pc_next); + gen_update_pc(dc, curr_insn_len(dc)); /* fall through */ case DISAS_JUMP: gen_goto_ptr(); break; case DISAS_UPDATE_EXIT: - gen_set_pc_im(dc, dc->base.pc_next); + gen_update_pc(dc, curr_insn_len(dc)); /* fall through */ default: /* indicate that the hash table must be used to find the next TB */ @@ -XXX,XX +XXX,XX @@ static void arm_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu) gen_set_label(dc->condlabel); gen_set_condexec(dc); if (unlikely(dc->ss_active)) { - gen_set_pc_im(dc, dc->base.pc_next); + gen_update_pc(dc, curr_insn_len(dc)); gen_singlestep_exception(dc); } else { gen_goto_tb(dc, 1, curr_insn_len(dc)); -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> In preparation for TARGET_TB_PCREL, reduce reliance on absolute values. Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20221020030641.2066807-5-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/translate.h | 5 +++-- target/arm/translate-a64.c | 28 ++++++++++------------- target/arm/translate-m-nocp.c | 6 ++--- target/arm/translate-mve.c | 2 +- target/arm/translate-vfp.c | 6 ++--- target/arm/translate.c | 42 +++++++++++++++++------------------ 6 files changed, 43 insertions(+), 46 deletions(-) diff --git a/target/arm/translate.h b/target/arm/translate.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate.h +++ b/target/arm/translate.h @@ -XXX,XX +XXX,XX @@ void arm_jump_cc(DisasCompare *cmp, TCGLabel *label); void arm_gen_test_cc(int cc, TCGLabel *label); MemOp pow2_align(unsigned i); void unallocated_encoding(DisasContext *s); -void gen_exception_insn_el(DisasContext *s, uint64_t pc, int excp, +void gen_exception_insn_el(DisasContext *s, target_long pc_diff, int excp, uint32_t syn, uint32_t target_el); -void gen_exception_insn(DisasContext *s, uint64_t pc, int excp, uint32_t syn); +void gen_exception_insn(DisasContext *s, target_long pc_diff, + int excp, uint32_t syn); /* Return state of Alternate Half-precision flag, caller frees result */ static inline TCGv_i32 get_ahp_flag(void) 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 bool fp_access_check_only(DisasContext *s) assert(!s->fp_access_checked); s->fp_access_checked = true; - gen_exception_insn_el(s, s->pc_curr, EXCP_UDEF, + gen_exception_insn_el(s, 0, EXCP_UDEF, syn_fp_access_trap(1, 0xe, false, 0), s->fp_excp_el); return false; @@ -XXX,XX +XXX,XX @@ static bool fp_access_check(DisasContext *s) return false; } if (s->sme_trap_nonstreaming && s->is_nonstreaming) { - gen_exception_insn(s, s->pc_curr, EXCP_UDEF, + gen_exception_insn(s, 0, EXCP_UDEF, syn_smetrap(SME_ET_Streaming, false)); return false; } @@ -XXX,XX +XXX,XX @@ bool sve_access_check(DisasContext *s) goto fail_exit; } } else if (s->sve_excp_el) { - gen_exception_insn_el(s, s->pc_curr, EXCP_UDEF, + gen_exception_insn_el(s, 0, EXCP_UDEF, syn_sve_access_trap(), s->sve_excp_el); goto fail_exit; } @@ -XXX,XX +XXX,XX @@ bool sve_access_check(DisasContext *s) static bool sme_access_check(DisasContext *s) { if (s->sme_excp_el) { - gen_exception_insn_el(s, s->pc_curr, EXCP_UDEF, + gen_exception_insn_el(s, 0, EXCP_UDEF, syn_smetrap(SME_ET_AccessTrap, false), s->sme_excp_el); return false; @@ -XXX,XX +XXX,XX @@ bool sme_enabled_check_with_svcr(DisasContext *s, unsigned req) return false; } if (FIELD_EX64(req, SVCR, SM) && !s->pstate_sm) { - gen_exception_insn(s, s->pc_curr, EXCP_UDEF, + gen_exception_insn(s, 0, EXCP_UDEF, syn_smetrap(SME_ET_NotStreaming, false)); return false; } if (FIELD_EX64(req, SVCR, ZA) && !s->pstate_za) { - gen_exception_insn(s, s->pc_curr, EXCP_UDEF, + gen_exception_insn(s, 0, EXCP_UDEF, syn_smetrap(SME_ET_InactiveZA, false)); return false; } @@ -XXX,XX +XXX,XX @@ static void gen_sysreg_undef(DisasContext *s, bool isread, } else { syndrome = syn_uncategorized(); } - gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syndrome); + gen_exception_insn(s, 0, EXCP_UDEF, syndrome); } /* MRS - move from system register @@ -XXX,XX +XXX,XX @@ static void disas_exc(DisasContext *s, uint32_t insn) switch (op2_ll) { case 1: /* SVC */ gen_ss_advance(s); - gen_exception_insn(s, s->base.pc_next, EXCP_SWI, - syn_aa64_svc(imm16)); + gen_exception_insn(s, 4, EXCP_SWI, syn_aa64_svc(imm16)); break; case 2: /* HVC */ if (s->current_el == 0) { @@ -XXX,XX +XXX,XX @@ static void disas_exc(DisasContext *s, uint32_t insn) gen_a64_update_pc(s, 0); gen_helper_pre_hvc(cpu_env); gen_ss_advance(s); - gen_exception_insn_el(s, s->base.pc_next, EXCP_HVC, - syn_aa64_hvc(imm16), 2); + gen_exception_insn_el(s, 4, EXCP_HVC, syn_aa64_hvc(imm16), 2); break; case 3: /* SMC */ if (s->current_el == 0) { @@ -XXX,XX +XXX,XX @@ static void disas_exc(DisasContext *s, uint32_t insn) gen_a64_update_pc(s, 0); gen_helper_pre_smc(cpu_env, tcg_constant_i32(syn_aa64_smc(imm16))); gen_ss_advance(s); - gen_exception_insn_el(s, s->base.pc_next, EXCP_SMC, - syn_aa64_smc(imm16), 3); + gen_exception_insn_el(s, 4, EXCP_SMC, syn_aa64_smc(imm16), 3); break; default: unallocated_encoding(s); @@ -XXX,XX +XXX,XX @@ static void aarch64_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) * Illegal execution state. This has priority over BTI * exceptions, but comes after instruction abort exceptions. */ - gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_illegalstate()); + gen_exception_insn(s, 0, EXCP_UDEF, syn_illegalstate()); return; } @@ -XXX,XX +XXX,XX @@ static void aarch64_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) if (s->btype != 0 && s->guarded_page && !btype_destination_ok(insn, s->bt, s->btype)) { - gen_exception_insn(s, s->pc_curr, EXCP_UDEF, - syn_btitrap(s->btype)); + gen_exception_insn(s, 0, EXCP_UDEF, syn_btitrap(s->btype)); return; } } else { diff --git a/target/arm/translate-m-nocp.c b/target/arm/translate-m-nocp.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-m-nocp.c +++ b/target/arm/translate-m-nocp.c @@ -XXX,XX +XXX,XX @@ static bool trans_VSCCLRM(DisasContext *s, arg_VSCCLRM *a) tcg_gen_brcondi_i32(TCG_COND_EQ, sfpa, 0, s->condlabel); if (s->fp_excp_el != 0) { - gen_exception_insn_el(s, s->pc_curr, EXCP_NOCP, + gen_exception_insn_el(s, 0, EXCP_NOCP, syn_uncategorized(), s->fp_excp_el); return true; } @@ -XXX,XX +XXX,XX @@ static bool trans_NOCP(DisasContext *s, arg_nocp *a) } if (a->cp != 10) { - gen_exception_insn(s, s->pc_curr, EXCP_NOCP, syn_uncategorized()); + gen_exception_insn(s, 0, EXCP_NOCP, syn_uncategorized()); return true; } if (s->fp_excp_el != 0) { - gen_exception_insn_el(s, s->pc_curr, EXCP_NOCP, + gen_exception_insn_el(s, 0, EXCP_NOCP, syn_uncategorized(), s->fp_excp_el); return true; } diff --git a/target/arm/translate-mve.c b/target/arm/translate-mve.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-mve.c +++ b/target/arm/translate-mve.c @@ -XXX,XX +XXX,XX @@ bool mve_eci_check(DisasContext *s) return true; default: /* Reserved value: INVSTATE UsageFault */ - gen_exception_insn(s, s->pc_curr, EXCP_INVSTATE, syn_uncategorized()); + gen_exception_insn(s, 0, EXCP_INVSTATE, syn_uncategorized()); return false; } } diff --git a/target/arm/translate-vfp.c b/target/arm/translate-vfp.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-vfp.c +++ b/target/arm/translate-vfp.c @@ -XXX,XX +XXX,XX @@ static bool vfp_access_check_a(DisasContext *s, bool ignore_vfp_enabled) int coproc = arm_dc_feature(s, ARM_FEATURE_V8) ? 0 : 0xa; uint32_t syn = syn_fp_access_trap(1, 0xe, false, coproc); - gen_exception_insn_el(s, s->pc_curr, EXCP_UDEF, syn, s->fp_excp_el); + gen_exception_insn_el(s, 0, EXCP_UDEF, syn, s->fp_excp_el); return false; } @@ -XXX,XX +XXX,XX @@ static bool vfp_access_check_a(DisasContext *s, bool ignore_vfp_enabled) * appear to be any insns which touch VFP which are allowed. */ if (s->sme_trap_nonstreaming) { - gen_exception_insn(s, s->pc_curr, EXCP_UDEF, + gen_exception_insn(s, 0, EXCP_UDEF, syn_smetrap(SME_ET_Streaming, curr_insn_len(s) == 2)); return false; @@ -XXX,XX +XXX,XX @@ bool vfp_access_check_m(DisasContext *s, bool skip_context_update) * the encoding space handled by the patterns in m-nocp.decode, * and for them we may need to raise NOCP here. */ - gen_exception_insn_el(s, s->pc_curr, EXCP_NOCP, + gen_exception_insn_el(s, 0, EXCP_NOCP, syn_uncategorized(), s->fp_excp_el); return false; } diff --git a/target/arm/translate.c b/target/arm/translate.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -XXX,XX +XXX,XX @@ static void gen_exception(int excp, uint32_t syndrome) tcg_constant_i32(syndrome)); } -static void gen_exception_insn_el_v(DisasContext *s, uint64_t pc, int excp, - uint32_t syn, TCGv_i32 tcg_el) +static void gen_exception_insn_el_v(DisasContext *s, target_long pc_diff, + int excp, uint32_t syn, TCGv_i32 tcg_el) { if (s->aarch64) { - gen_a64_update_pc(s, pc - s->pc_curr); + gen_a64_update_pc(s, pc_diff); } else { gen_set_condexec(s); - gen_update_pc(s, pc - s->pc_curr); + gen_update_pc(s, pc_diff); } gen_exception_el_v(excp, syn, tcg_el); s->base.is_jmp = DISAS_NORETURN; } -void gen_exception_insn_el(DisasContext *s, uint64_t pc, int excp, +void gen_exception_insn_el(DisasContext *s, target_long pc_diff, int excp, uint32_t syn, uint32_t target_el) { - gen_exception_insn_el_v(s, pc, excp, syn, tcg_constant_i32(target_el)); + gen_exception_insn_el_v(s, pc_diff, excp, syn, + tcg_constant_i32(target_el)); } -void gen_exception_insn(DisasContext *s, uint64_t pc, int excp, uint32_t syn) +void gen_exception_insn(DisasContext *s, target_long pc_diff, + int excp, uint32_t syn) { if (s->aarch64) { - gen_a64_update_pc(s, pc - s->pc_curr); + gen_a64_update_pc(s, pc_diff); } else { gen_set_condexec(s); - gen_update_pc(s, pc - s->pc_curr); + gen_update_pc(s, pc_diff); } gen_exception(excp, syn); s->base.is_jmp = DISAS_NORETURN; @@ -XXX,XX +XXX,XX @@ static void gen_exception_bkpt_insn(DisasContext *s, uint32_t syn) void unallocated_encoding(DisasContext *s) { /* Unallocated and reserved encodings are uncategorized */ - gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized()); + gen_exception_insn(s, 0, EXCP_UDEF, syn_uncategorized()); } /* Force a TB lookup after an instruction that changes the CPU state. */ @@ -XXX,XX +XXX,XX @@ static bool msr_banked_access_decode(DisasContext *s, int r, int sysm, int rn, tcg_el = tcg_constant_i32(3); } - gen_exception_insn_el_v(s, s->pc_curr, EXCP_UDEF, + gen_exception_insn_el_v(s, 0, EXCP_UDEF, syn_uncategorized(), tcg_el); tcg_temp_free_i32(tcg_el); return false; @@ -XXX,XX +XXX,XX @@ static bool msr_banked_access_decode(DisasContext *s, int r, int sysm, int rn, undef: /* If we get here then some access check did not pass */ - gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized()); + gen_exception_insn(s, 0, EXCP_UDEF, syn_uncategorized()); return false; } @@ -XXX,XX +XXX,XX @@ static void gen_srs(DisasContext *s, * For the UNPREDICTABLE cases we choose to UNDEF. */ if (s->current_el == 1 && !s->ns && mode == ARM_CPU_MODE_MON) { - gen_exception_insn_el(s, s->pc_curr, EXCP_UDEF, - syn_uncategorized(), 3); + gen_exception_insn_el(s, 0, EXCP_UDEF, syn_uncategorized(), 3); return; } @@ -XXX,XX +XXX,XX @@ static bool trans_WLS(DisasContext *s, arg_WLS *a) * Do the check-and-raise-exception by hand. */ if (s->fp_excp_el) { - gen_exception_insn_el(s, s->pc_curr, EXCP_NOCP, + gen_exception_insn_el(s, 0, EXCP_NOCP, syn_uncategorized(), s->fp_excp_el); return true; } @@ -XXX,XX +XXX,XX @@ static bool trans_LE(DisasContext *s, arg_LE *a) tmp = load_cpu_field(v7m.ltpsize); tcg_gen_brcondi_i32(TCG_COND_EQ, tmp, 4, skipexc); tcg_temp_free_i32(tmp); - gen_exception_insn(s, s->pc_curr, EXCP_INVSTATE, syn_uncategorized()); + gen_exception_insn(s, 0, EXCP_INVSTATE, syn_uncategorized()); gen_set_label(skipexc); } @@ -XXX,XX +XXX,XX @@ static void disas_arm_insn(DisasContext *s, unsigned int insn) * UsageFault exception. */ if (arm_dc_feature(s, ARM_FEATURE_M)) { - gen_exception_insn(s, s->pc_curr, EXCP_INVSTATE, syn_uncategorized()); + gen_exception_insn(s, 0, EXCP_INVSTATE, syn_uncategorized()); return; } @@ -XXX,XX +XXX,XX @@ static void disas_arm_insn(DisasContext *s, unsigned int insn) * Illegal execution state. This has priority over BTI * exceptions, but comes after instruction abort exceptions. */ - gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_illegalstate()); + gen_exception_insn(s, 0, EXCP_UDEF, syn_illegalstate()); return; } @@ -XXX,XX +XXX,XX @@ static void thumb_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) * Illegal execution state. This has priority over BTI * exceptions, but comes after instruction abort exceptions. */ - gen_exception_insn(dc, dc->pc_curr, EXCP_UDEF, syn_illegalstate()); + gen_exception_insn(dc, 0, EXCP_UDEF, syn_illegalstate()); return; } @@ -XXX,XX +XXX,XX @@ static void thumb_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) */ tcg_remove_ops_after(dc->insn_eci_rewind); dc->condjmp = 0; - gen_exception_insn(dc, dc->pc_curr, EXCP_INVSTATE, - syn_uncategorized()); + gen_exception_insn(dc, 0, EXCP_INVSTATE, syn_uncategorized()); } arm_post_translate_insn(dc); -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> In preparation for TARGET_TB_PCREL, reduce reliance on absolute values. Since we always pass dc->pc_curr, fold the arithmetic to zero displacement. Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20221020030641.2066807-6-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/translate-a64.c | 6 +++--- target/arm/translate.c | 10 +++++----- 2 files changed, 8 insertions(+), 8 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 gen_exception_internal(int excp) gen_helper_exception_internal(cpu_env, tcg_constant_i32(excp)); } -static void gen_exception_internal_insn(DisasContext *s, uint64_t pc, int excp) +static void gen_exception_internal_insn(DisasContext *s, int excp) { - gen_a64_update_pc(s, pc - s->pc_curr); + gen_a64_update_pc(s, 0); gen_exception_internal(excp); s->base.is_jmp = DISAS_NORETURN; } @@ -XXX,XX +XXX,XX @@ static void disas_exc(DisasContext *s, uint32_t insn) * Secondly, "HLT 0xf000" is the A64 semihosting syscall instruction. */ if (semihosting_enabled(s->current_el == 0) && imm16 == 0xf000) { - gen_exception_internal_insn(s, s->pc_curr, EXCP_SEMIHOST); + gen_exception_internal_insn(s, EXCP_SEMIHOST); } else { unallocated_encoding(s); } 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 inline void gen_smc(DisasContext *s) s->base.is_jmp = DISAS_SMC; } -static void gen_exception_internal_insn(DisasContext *s, uint32_t pc, int excp) +static void gen_exception_internal_insn(DisasContext *s, int excp) { gen_set_condexec(s); - gen_update_pc(s, pc - s->pc_curr); + gen_update_pc(s, 0); gen_exception_internal(excp); s->base.is_jmp = DISAS_NORETURN; } @@ -XXX,XX +XXX,XX @@ static inline void gen_hlt(DisasContext *s, int imm) */ if (semihosting_enabled(s->current_el != 0) && (imm == (s->thumb ? 0x3c : 0xf000))) { - gen_exception_internal_insn(s, s->pc_curr, EXCP_SEMIHOST); + gen_exception_internal_insn(s, EXCP_SEMIHOST); return; } @@ -XXX,XX +XXX,XX @@ static bool trans_BKPT(DisasContext *s, arg_BKPT *a) if (arm_dc_feature(s, ARM_FEATURE_M) && semihosting_enabled(s->current_el == 0) && (a->imm == 0xab)) { - gen_exception_internal_insn(s, s->pc_curr, EXCP_SEMIHOST); + gen_exception_internal_insn(s, EXCP_SEMIHOST); } else { gen_exception_bkpt_insn(s, syn_aa32_bkpt(a->imm, false)); } @@ -XXX,XX +XXX,XX @@ static bool trans_SVC(DisasContext *s, arg_SVC *a) if (!arm_dc_feature(s, ARM_FEATURE_M) && semihosting_enabled(s->current_el == 0) && (a->imm == semihost_imm)) { - gen_exception_internal_insn(s, s->pc_curr, EXCP_SEMIHOST); + gen_exception_internal_insn(s, EXCP_SEMIHOST); } else { gen_update_pc(s, curr_insn_len(s)); s->svc_imm = a->imm; -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> In preparation for TARGET_TB_PCREL, reduce reliance on absolute values. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20221020030641.2066807-7-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/translate.c | 37 +++++++++++++++++++++---------------- 1 file changed, 21 insertions(+), 16 deletions(-) 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 uint32_t read_pc(DisasContext *s) return s->pc_curr + (s->thumb ? 4 : 8); } +/* The pc_curr difference for an architectural jump. */ +static target_long jmp_diff(DisasContext *s, target_long diff) +{ + return diff + (s->thumb ? 4 : 8); +} + /* Set a variable to the value of a CPU register. */ void load_reg_var(DisasContext *s, TCGv_i32 var, int reg) { @@ -XXX,XX +XXX,XX @@ static void gen_goto_ptr(void) * cpu_loop_exec. Any live exit_requests will be processed as we * enter the next TB. */ -static void gen_goto_tb(DisasContext *s, int n, int diff) +static void gen_goto_tb(DisasContext *s, int n, target_long diff) { target_ulong dest = s->pc_curr + diff; @@ -XXX,XX +XXX,XX @@ static void gen_goto_tb(DisasContext *s, int n, int diff) } /* Jump, specifying which TB number to use if we gen_goto_tb() */ -static inline void gen_jmp_tb(DisasContext *s, uint32_t dest, int tbno) +static void gen_jmp_tb(DisasContext *s, target_long diff, int tbno) { - int diff = dest - s->pc_curr; - if (unlikely(s->ss_active)) { /* An indirect jump so that we still trigger the debug exception. */ gen_update_pc(s, diff); @@ -XXX,XX +XXX,XX @@ static inline void gen_jmp_tb(DisasContext *s, uint32_t dest, int tbno) } } -static inline void gen_jmp(DisasContext *s, uint32_t dest) +static inline void gen_jmp(DisasContext *s, target_long diff) { - gen_jmp_tb(s, dest, 0); + gen_jmp_tb(s, diff, 0); } static inline void gen_mulxy(TCGv_i32 t0, TCGv_i32 t1, int x, int y) @@ -XXX,XX +XXX,XX @@ static bool trans_CLRM(DisasContext *s, arg_CLRM *a) static bool trans_B(DisasContext *s, arg_i *a) { - gen_jmp(s, read_pc(s) + a->imm); + gen_jmp(s, jmp_diff(s, a->imm)); return true; } @@ -XXX,XX +XXX,XX @@ static bool trans_B_cond_thumb(DisasContext *s, arg_ci *a) return true; } arm_skip_unless(s, a->cond); - gen_jmp(s, read_pc(s) + a->imm); + gen_jmp(s, jmp_diff(s, a->imm)); return true; } static bool trans_BL(DisasContext *s, arg_i *a) { tcg_gen_movi_i32(cpu_R[14], s->base.pc_next | s->thumb); - gen_jmp(s, read_pc(s) + a->imm); + gen_jmp(s, jmp_diff(s, a->imm)); return true; } @@ -XXX,XX +XXX,XX @@ static bool trans_BLX_i(DisasContext *s, arg_BLX_i *a) } tcg_gen_movi_i32(cpu_R[14], s->base.pc_next | s->thumb); store_cpu_field_constant(!s->thumb, thumb); - gen_jmp(s, (read_pc(s) & ~3) + a->imm); + /* This jump is computed from an aligned PC: subtract off the low bits. */ + gen_jmp(s, jmp_diff(s, a->imm - (s->pc_curr & 3))); return true; } @@ -XXX,XX +XXX,XX @@ static bool trans_WLS(DisasContext *s, arg_WLS *a) * when we take this upcoming exit from this TB, so gen_jmp_tb() is OK. */ } - gen_jmp_tb(s, s->base.pc_next, 1); + gen_jmp_tb(s, curr_insn_len(s), 1); gen_set_label(nextlabel); - gen_jmp(s, read_pc(s) + a->imm); + gen_jmp(s, jmp_diff(s, a->imm)); return true; } @@ -XXX,XX +XXX,XX @@ static bool trans_LE(DisasContext *s, arg_LE *a) if (a->f) { /* Loop-forever: just jump back to the loop start */ - gen_jmp(s, read_pc(s) - a->imm); + gen_jmp(s, jmp_diff(s, -a->imm)); return true; } @@ -XXX,XX +XXX,XX @@ static bool trans_LE(DisasContext *s, arg_LE *a) tcg_temp_free_i32(decr); } /* Jump back to the loop start */ - gen_jmp(s, read_pc(s) - a->imm); + gen_jmp(s, jmp_diff(s, -a->imm)); gen_set_label(loopend); if (a->tp) { @@ -XXX,XX +XXX,XX @@ static bool trans_LE(DisasContext *s, arg_LE *a) store_cpu_field(tcg_constant_i32(4), v7m.ltpsize); } /* End TB, continuing to following insn */ - gen_jmp_tb(s, s->base.pc_next, 1); + gen_jmp_tb(s, curr_insn_len(s), 1); return true; } @@ -XXX,XX +XXX,XX @@ static bool trans_CBZ(DisasContext *s, arg_CBZ *a) tcg_gen_brcondi_i32(a->nz ? TCG_COND_EQ : TCG_COND_NE, tmp, 0, s->condlabel); tcg_temp_free_i32(tmp); - gen_jmp(s, read_pc(s) + a->imm); + gen_jmp(s, jmp_diff(s, a->imm)); return true; } -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> In preparation for TARGET_TB_PCREL, reduce reliance on absolute values. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20221020030641.2066807-8-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/translate-a64.c | 41 +++++++++++++++++++++++++++----------- 1 file changed, 29 insertions(+), 12 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 reset_btype(DisasContext *s) } } +static void gen_pc_plus_diff(DisasContext *s, TCGv_i64 dest, target_long diff) +{ + tcg_gen_movi_i64(dest, s->pc_curr + diff); +} + void gen_a64_update_pc(DisasContext *s, target_long diff) { - tcg_gen_movi_i64(cpu_pc, s->pc_curr + diff); + gen_pc_plus_diff(s, cpu_pc, diff); } /* @@ -XXX,XX +XXX,XX @@ static void disas_uncond_b_imm(DisasContext *s, uint32_t insn) if (insn & (1U << 31)) { /* BL Branch with link */ - tcg_gen_movi_i64(cpu_reg(s, 30), s->base.pc_next); + gen_pc_plus_diff(s, cpu_reg(s, 30), curr_insn_len(s)); } /* B Branch / BL Branch with link */ @@ -XXX,XX +XXX,XX @@ static void disas_uncond_b_reg(DisasContext *s, uint32_t insn) default: goto do_unallocated; } - gen_a64_set_pc(s, dst); /* BLR also needs to load return address */ if (opc == 1) { - tcg_gen_movi_i64(cpu_reg(s, 30), s->base.pc_next); + TCGv_i64 lr = cpu_reg(s, 30); + if (dst == lr) { + TCGv_i64 tmp = new_tmp_a64(s); + tcg_gen_mov_i64(tmp, dst); + dst = tmp; + } + gen_pc_plus_diff(s, lr, curr_insn_len(s)); } + gen_a64_set_pc(s, dst); break; case 8: /* BRAA */ @@ -XXX,XX +XXX,XX @@ static void disas_uncond_b_reg(DisasContext *s, uint32_t insn) } else { dst = cpu_reg(s, rn); } - gen_a64_set_pc(s, dst); /* BLRAA also needs to load return address */ if (opc == 9) { - tcg_gen_movi_i64(cpu_reg(s, 30), s->base.pc_next); + TCGv_i64 lr = cpu_reg(s, 30); + if (dst == lr) { + TCGv_i64 tmp = new_tmp_a64(s); + tcg_gen_mov_i64(tmp, dst); + dst = tmp; + } + gen_pc_plus_diff(s, lr, curr_insn_len(s)); } + gen_a64_set_pc(s, dst); break; case 4: /* ERET */ @@ -XXX,XX +XXX,XX @@ static void disas_ld_lit(DisasContext *s, uint32_t insn) tcg_rt = cpu_reg(s, rt); - clean_addr = tcg_constant_i64(s->pc_curr + imm); + clean_addr = new_tmp_a64(s); + gen_pc_plus_diff(s, clean_addr, imm); if (is_vector) { do_fp_ld(s, rt, clean_addr, size); } else { @@ -XXX,XX +XXX,XX @@ static void disas_ldst(DisasContext *s, uint32_t insn) static void disas_pc_rel_adr(DisasContext *s, uint32_t insn) { unsigned int page, rd; - uint64_t base; - uint64_t offset; + int64_t offset; page = extract32(insn, 31, 1); /* SignExtend(immhi:immlo) -> offset */ offset = sextract64(insn, 5, 19); offset = offset << 2 | extract32(insn, 29, 2); rd = extract32(insn, 0, 5); - base = s->pc_curr; if (page) { /* ADRP (page based) */ - base &= ~0xfff; offset <<= 12; + /* The page offset is ok for TARGET_TB_PCREL. */ + offset -= s->pc_curr & 0xfff; } - tcg_gen_movi_i64(cpu_reg(s, rd), base + offset); + gen_pc_plus_diff(s, cpu_reg(s, rd), offset); } /* -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> In preparation for TARGET_TB_PCREL, reduce reliance on absolute values. Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20221020030641.2066807-9-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/translate.c | 38 +++++++++++++++++++++----------------- 1 file changed, 21 insertions(+), 17 deletions(-) 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 inline int get_a32_user_mem_index(DisasContext *s) } } -/* The architectural value of PC. */ -static uint32_t read_pc(DisasContext *s) -{ - return s->pc_curr + (s->thumb ? 4 : 8); -} - /* The pc_curr difference for an architectural jump. */ static target_long jmp_diff(DisasContext *s, target_long diff) { return diff + (s->thumb ? 4 : 8); } +static void gen_pc_plus_diff(DisasContext *s, TCGv_i32 var, target_long diff) +{ + tcg_gen_movi_i32(var, s->pc_curr + diff); +} + /* Set a variable to the value of a CPU register. */ void load_reg_var(DisasContext *s, TCGv_i32 var, int reg) { if (reg == 15) { - tcg_gen_movi_i32(var, read_pc(s)); + gen_pc_plus_diff(s, var, jmp_diff(s, 0)); } else { tcg_gen_mov_i32(var, cpu_R[reg]); } @@ -XXX,XX +XXX,XX @@ TCGv_i32 add_reg_for_lit(DisasContext *s, int reg, int ofs) TCGv_i32 tmp = tcg_temp_new_i32(); if (reg == 15) { - tcg_gen_movi_i32(tmp, (read_pc(s) & ~3) + ofs); + /* + * This address is computed from an aligned PC: + * subtract off the low bits. + */ + gen_pc_plus_diff(s, tmp, jmp_diff(s, ofs - (s->pc_curr & 3))); } else { tcg_gen_addi_i32(tmp, cpu_R[reg], ofs); } @@ -XXX,XX +XXX,XX @@ void unallocated_encoding(DisasContext *s) /* Force a TB lookup after an instruction that changes the CPU state. */ void gen_lookup_tb(DisasContext *s) { - tcg_gen_movi_i32(cpu_R[15], s->base.pc_next); + gen_pc_plus_diff(s, cpu_R[15], curr_insn_len(s)); s->base.is_jmp = DISAS_EXIT; } @@ -XXX,XX +XXX,XX @@ static bool trans_BLX_r(DisasContext *s, arg_BLX_r *a) return false; } tmp = load_reg(s, a->rm); - tcg_gen_movi_i32(cpu_R[14], s->base.pc_next | s->thumb); + gen_pc_plus_diff(s, cpu_R[14], curr_insn_len(s) | s->thumb); gen_bx(s, tmp); return true; } @@ -XXX,XX +XXX,XX @@ static bool trans_B_cond_thumb(DisasContext *s, arg_ci *a) static bool trans_BL(DisasContext *s, arg_i *a) { - tcg_gen_movi_i32(cpu_R[14], s->base.pc_next | s->thumb); + gen_pc_plus_diff(s, cpu_R[14], curr_insn_len(s) | s->thumb); gen_jmp(s, jmp_diff(s, a->imm)); return true; } @@ -XXX,XX +XXX,XX @@ static bool trans_BLX_i(DisasContext *s, arg_BLX_i *a) if (s->thumb && (a->imm & 2)) { return false; } - tcg_gen_movi_i32(cpu_R[14], s->base.pc_next | s->thumb); + gen_pc_plus_diff(s, cpu_R[14], curr_insn_len(s) | s->thumb); store_cpu_field_constant(!s->thumb, thumb); /* This jump is computed from an aligned PC: subtract off the low bits. */ gen_jmp(s, jmp_diff(s, a->imm - (s->pc_curr & 3))); @@ -XXX,XX +XXX,XX @@ static bool trans_BLX_i(DisasContext *s, arg_BLX_i *a) static bool trans_BL_BLX_prefix(DisasContext *s, arg_BL_BLX_prefix *a) { assert(!arm_dc_feature(s, ARM_FEATURE_THUMB2)); - tcg_gen_movi_i32(cpu_R[14], read_pc(s) + (a->imm << 12)); + gen_pc_plus_diff(s, cpu_R[14], jmp_diff(s, a->imm << 12)); return true; } @@ -XXX,XX +XXX,XX @@ static bool trans_BL_suffix(DisasContext *s, arg_BL_suffix *a) assert(!arm_dc_feature(s, ARM_FEATURE_THUMB2)); tcg_gen_addi_i32(tmp, cpu_R[14], (a->imm << 1) | 1); - tcg_gen_movi_i32(cpu_R[14], s->base.pc_next | 1); + gen_pc_plus_diff(s, cpu_R[14], curr_insn_len(s) | 1); gen_bx(s, tmp); return true; } @@ -XXX,XX +XXX,XX @@ static bool trans_BLX_suffix(DisasContext *s, arg_BLX_suffix *a) tmp = tcg_temp_new_i32(); tcg_gen_addi_i32(tmp, cpu_R[14], a->imm << 1); tcg_gen_andi_i32(tmp, tmp, 0xfffffffc); - tcg_gen_movi_i32(cpu_R[14], s->base.pc_next | 1); + gen_pc_plus_diff(s, cpu_R[14], curr_insn_len(s) | 1); gen_bx(s, tmp); return true; } @@ -XXX,XX +XXX,XX @@ static bool op_tbranch(DisasContext *s, arg_tbranch *a, bool half) tcg_gen_add_i32(addr, addr, tmp); gen_aa32_ld_i32(s, tmp, addr, get_mem_index(s), half ? MO_UW : MO_UB); - tcg_temp_free_i32(addr); tcg_gen_add_i32(tmp, tmp, tmp); - tcg_gen_addi_i32(tmp, tmp, read_pc(s)); + gen_pc_plus_diff(s, addr, jmp_diff(s, 0)); + tcg_gen_add_i32(tmp, tmp, addr); + tcg_temp_free_i32(addr); store_reg(s, 15, tmp); return true; } -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20221020030641.2066807-10-richard.henderson@linaro.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/cpu-param.h | 2 + target/arm/translate.h | 50 +++++++++++++++- target/arm/cpu.c | 23 ++++---- target/arm/translate-a64.c | 64 +++++++++++++------- target/arm/translate-m-nocp.c | 2 +- target/arm/translate.c | 108 +++++++++++++++++++++++----------- 6 files changed, 178 insertions(+), 71 deletions(-) diff --git a/target/arm/cpu-param.h b/target/arm/cpu-param.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu-param.h +++ b/target/arm/cpu-param.h @@ -XXX,XX +XXX,XX @@ # define TARGET_PAGE_BITS_VARY # define TARGET_PAGE_BITS_MIN 10 +# define TARGET_TB_PCREL 1 + /* * Cache the attrs and shareability fields from the page table entry. * diff --git a/target/arm/translate.h b/target/arm/translate.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate.h +++ b/target/arm/translate.h @@ -XXX,XX +XXX,XX @@ /* internal defines */ + +/* + * Save pc_save across a branch, so that we may restore the value from + * before the branch at the point the label is emitted. + */ +typedef struct DisasLabel { + TCGLabel *label; + target_ulong pc_save; +} DisasLabel; + typedef struct DisasContext { DisasContextBase base; const ARMISARegisters *isar; /* The address of the current instruction being translated. */ target_ulong pc_curr; + /* + * For TARGET_TB_PCREL, the full value of cpu_pc is not known + * (although the page offset is known). For convenience, the + * translation loop uses the full virtual address that triggered + * the translation, from base.pc_start through pc_curr. + * For efficiency, we do not update cpu_pc for every instruction. + * Instead, pc_save has the value of pc_curr at the time of the + * last update to cpu_pc, which allows us to compute the addend + * needed to bring cpu_pc current: pc_curr - pc_save. + * If cpu_pc now contains the destination of an indirect branch, + * pc_save contains -1 to indicate that relative updates are no + * longer possible. + */ + target_ulong pc_save; target_ulong page_start; uint32_t insn; /* Nonzero if this instruction has been conditionally skipped. */ int condjmp; /* The label that will be jumped to when the instruction is skipped. */ - TCGLabel *condlabel; + DisasLabel condlabel; /* Thumb-2 conditional execution bits. */ int condexec_mask; int condexec_cond; @@ -XXX,XX +XXX,XX @@ typedef struct DisasContext { * after decode (ie after any UNDEF checks) */ bool eci_handled; - /* TCG op to rewind to if this turns out to be an invalid ECI state */ - TCGOp *insn_eci_rewind; int sctlr_b; MemOp be_data; #if !defined(CONFIG_USER_ONLY) @@ -XXX,XX +XXX,XX @@ static inline MemOp finalize_memop(DisasContext *s, MemOp opc) */ uint64_t asimd_imm_const(uint32_t imm, int cmode, int op); +/* + * gen_disas_label: + * Create a label and cache a copy of pc_save. + */ +static inline DisasLabel gen_disas_label(DisasContext *s) +{ + return (DisasLabel){ + .label = gen_new_label(), + .pc_save = s->pc_save, + }; +} + +/* + * set_disas_label: + * Emit a label and restore the cached copy of pc_save. + */ +static inline void set_disas_label(DisasContext *s, DisasLabel l) +{ + gen_set_label(l.label); + s->pc_save = l.pc_save; +} + /* * Helpers for implementing sets of trans_* functions. * Defer the implementation of NAME to FUNC, with optional extra arguments. diff --git a/target/arm/cpu.c b/target/arm/cpu.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu.c +++ b/target/arm/cpu.c @@ -XXX,XX +XXX,XX @@ static vaddr arm_cpu_get_pc(CPUState *cs) void arm_cpu_synchronize_from_tb(CPUState *cs, const TranslationBlock *tb) { - ARMCPU *cpu = ARM_CPU(cs); - CPUARMState *env = &cpu->env; - - /* - * It's OK to look at env for the current mode here, because it's - * never possible for an AArch64 TB to chain to an AArch32 TB. - */ - if (is_a64(env)) { - env->pc = tb_pc(tb); - } else { - env->regs[15] = tb_pc(tb); + /* The program counter is always up to date with TARGET_TB_PCREL. */ + if (!TARGET_TB_PCREL) { + CPUARMState *env = cs->env_ptr; + /* + * It's OK to look at env for the current mode here, because it's + * never possible for an AArch64 TB to chain to an AArch32 TB. + */ + if (is_a64(env)) { + env->pc = tb_pc(tb); + } else { + env->regs[15] = tb_pc(tb); + } } } #endif /* CONFIG_TCG */ 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 reset_btype(DisasContext *s) static void gen_pc_plus_diff(DisasContext *s, TCGv_i64 dest, target_long diff) { - tcg_gen_movi_i64(dest, s->pc_curr + diff); + assert(s->pc_save != -1); + if (TARGET_TB_PCREL) { + tcg_gen_addi_i64(dest, cpu_pc, (s->pc_curr - s->pc_save) + diff); + } else { + tcg_gen_movi_i64(dest, s->pc_curr + diff); + } } void gen_a64_update_pc(DisasContext *s, target_long diff) { gen_pc_plus_diff(s, cpu_pc, diff); + s->pc_save = s->pc_curr + diff; } /* @@ -XXX,XX +XXX,XX @@ static void gen_a64_set_pc(DisasContext *s, TCGv_i64 src) * then loading an address into the PC will clear out any tag. */ gen_top_byte_ignore(s, cpu_pc, src, s->tbii); + s->pc_save = -1; } /* @@ -XXX,XX +XXX,XX @@ static inline bool use_goto_tb(DisasContext *s, uint64_t dest) static void gen_goto_tb(DisasContext *s, int n, int64_t diff) { - uint64_t dest = s->pc_curr + diff; - - if (use_goto_tb(s, dest)) { - tcg_gen_goto_tb(n); - gen_a64_update_pc(s, diff); + if (use_goto_tb(s, s->pc_curr + diff)) { + /* + * For pcrel, the pc must always be up-to-date on entry to + * the linked TB, so that it can use simple additions for all + * further adjustments. For !pcrel, the linked TB is compiled + * to know its full virtual address, so we can delay the + * update to pc to the unlinked path. A long chain of links + * can thus avoid many updates to the PC. + */ + if (TARGET_TB_PCREL) { + gen_a64_update_pc(s, diff); + tcg_gen_goto_tb(n); + } else { + tcg_gen_goto_tb(n); + gen_a64_update_pc(s, diff); + } tcg_gen_exit_tb(s->base.tb, n); s->base.is_jmp = DISAS_NORETURN; } else { @@ -XXX,XX +XXX,XX @@ static void disas_comp_b_imm(DisasContext *s, uint32_t insn) { unsigned int sf, op, rt; int64_t diff; - TCGLabel *label_match; + DisasLabel match; TCGv_i64 tcg_cmp; sf = extract32(insn, 31, 1); @@ -XXX,XX +XXX,XX @@ static void disas_comp_b_imm(DisasContext *s, uint32_t insn) diff = sextract32(insn, 5, 19) * 4; tcg_cmp = read_cpu_reg(s, rt, sf); - label_match = gen_new_label(); - reset_btype(s); - tcg_gen_brcondi_i64(op ? TCG_COND_NE : TCG_COND_EQ, - tcg_cmp, 0, label_match); + match = gen_disas_label(s); + tcg_gen_brcondi_i64(op ? TCG_COND_NE : TCG_COND_EQ, + tcg_cmp, 0, match.label); gen_goto_tb(s, 0, 4); - gen_set_label(label_match); + set_disas_label(s, match); gen_goto_tb(s, 1, diff); } @@ -XXX,XX +XXX,XX @@ static void disas_test_b_imm(DisasContext *s, uint32_t insn) { unsigned int bit_pos, op, rt; int64_t diff; - TCGLabel *label_match; + DisasLabel match; TCGv_i64 tcg_cmp; bit_pos = (extract32(insn, 31, 1) << 5) | extract32(insn, 19, 5); @@ -XXX,XX +XXX,XX @@ static void disas_test_b_imm(DisasContext *s, uint32_t insn) tcg_cmp = tcg_temp_new_i64(); tcg_gen_andi_i64(tcg_cmp, cpu_reg(s, rt), (1ULL << bit_pos)); - label_match = gen_new_label(); reset_btype(s); + + match = gen_disas_label(s); tcg_gen_brcondi_i64(op ? TCG_COND_NE : TCG_COND_EQ, - tcg_cmp, 0, label_match); + tcg_cmp, 0, match.label); tcg_temp_free_i64(tcg_cmp); gen_goto_tb(s, 0, 4); - gen_set_label(label_match); + set_disas_label(s, match); gen_goto_tb(s, 1, diff); } @@ -XXX,XX +XXX,XX @@ static void disas_cond_b_imm(DisasContext *s, uint32_t insn) reset_btype(s); if (cond < 0x0e) { /* genuinely conditional branches */ - TCGLabel *label_match = gen_new_label(); - arm_gen_test_cc(cond, label_match); + DisasLabel match = gen_disas_label(s); + arm_gen_test_cc(cond, match.label); gen_goto_tb(s, 0, 4); - gen_set_label(label_match); + set_disas_label(s, match); gen_goto_tb(s, 1, diff); } else { /* 0xe and 0xf are both "always" conditions */ @@ -XXX,XX +XXX,XX @@ static void aarch64_tr_init_disas_context(DisasContextBase *dcbase, dc->isar = &arm_cpu->isar; dc->condjmp = 0; - + dc->pc_save = dc->base.pc_first; dc->aarch64 = true; dc->thumb = false; dc->sctlr_b = 0; @@ -XXX,XX +XXX,XX @@ static void aarch64_tr_tb_start(DisasContextBase *db, CPUState *cpu) static void aarch64_tr_insn_start(DisasContextBase *dcbase, CPUState *cpu) { DisasContext *dc = container_of(dcbase, DisasContext, base); + target_ulong pc_arg = dc->base.pc_next; - tcg_gen_insn_start(dc->base.pc_next, 0, 0); + if (TARGET_TB_PCREL) { + pc_arg &= ~TARGET_PAGE_MASK; + } + tcg_gen_insn_start(pc_arg, 0, 0); dc->insn_start = tcg_last_op(); } diff --git a/target/arm/translate-m-nocp.c b/target/arm/translate-m-nocp.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-m-nocp.c +++ b/target/arm/translate-m-nocp.c @@ -XXX,XX +XXX,XX @@ static bool trans_VSCCLRM(DisasContext *s, arg_VSCCLRM *a) tcg_gen_andi_i32(sfpa, sfpa, R_V7M_CONTROL_SFPA_MASK); tcg_gen_or_i32(sfpa, sfpa, aspen); arm_gen_condlabel(s); - tcg_gen_brcondi_i32(TCG_COND_EQ, sfpa, 0, s->condlabel); + tcg_gen_brcondi_i32(TCG_COND_EQ, sfpa, 0, s->condlabel.label); if (s->fp_excp_el != 0) { gen_exception_insn_el(s, 0, EXCP_NOCP, 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 @@ uint64_t asimd_imm_const(uint32_t imm, int cmode, int op) void arm_gen_condlabel(DisasContext *s) { if (!s->condjmp) { - s->condlabel = gen_new_label(); + s->condlabel = gen_disas_label(s); s->condjmp = 1; } } @@ -XXX,XX +XXX,XX @@ static target_long jmp_diff(DisasContext *s, target_long diff) static void gen_pc_plus_diff(DisasContext *s, TCGv_i32 var, target_long diff) { - tcg_gen_movi_i32(var, s->pc_curr + diff); + assert(s->pc_save != -1); + if (TARGET_TB_PCREL) { + tcg_gen_addi_i32(var, cpu_R[15], (s->pc_curr - s->pc_save) + diff); + } else { + tcg_gen_movi_i32(var, s->pc_curr + diff); + } } /* Set a variable to the value of a CPU register. */ @@ -XXX,XX +XXX,XX @@ void store_reg(DisasContext *s, int reg, TCGv_i32 var) */ tcg_gen_andi_i32(var, var, s->thumb ? ~1 : ~3); s->base.is_jmp = DISAS_JUMP; + s->pc_save = -1; } else if (reg == 13 && arm_dc_feature(s, ARM_FEATURE_M)) { /* For M-profile SP bits [1:0] are always zero */ tcg_gen_andi_i32(var, var, ~3); @@ -XXX,XX +XXX,XX @@ void gen_set_condexec(DisasContext *s) void gen_update_pc(DisasContext *s, target_long diff) { - tcg_gen_movi_i32(cpu_R[15], s->pc_curr + diff); + gen_pc_plus_diff(s, cpu_R[15], diff); + s->pc_save = s->pc_curr + diff; } /* Set PC and Thumb state from var. var is marked as dead. */ @@ -XXX,XX +XXX,XX @@ static inline void gen_bx(DisasContext *s, TCGv_i32 var) tcg_gen_andi_i32(cpu_R[15], var, ~1); tcg_gen_andi_i32(var, var, 1); store_cpu_field(var, thumb); + s->pc_save = -1; } /* @@ -XXX,XX +XXX,XX @@ static inline void gen_bx_excret(DisasContext *s, TCGv_i32 var) static inline void gen_bx_excret_final_code(DisasContext *s) { /* Generate the code to finish possible exception return and end the TB */ - TCGLabel *excret_label = gen_new_label(); + DisasLabel excret_label = gen_disas_label(s); uint32_t min_magic; if (arm_dc_feature(s, ARM_FEATURE_M_SECURITY)) { @@ -XXX,XX +XXX,XX @@ static inline void gen_bx_excret_final_code(DisasContext *s) } /* Is the new PC value in the magic range indicating exception return? */ - tcg_gen_brcondi_i32(TCG_COND_GEU, cpu_R[15], min_magic, excret_label); + tcg_gen_brcondi_i32(TCG_COND_GEU, cpu_R[15], min_magic, excret_label.label); /* No: end the TB as we would for a DISAS_JMP */ if (s->ss_active) { gen_singlestep_exception(s); } else { tcg_gen_exit_tb(NULL, 0); } - gen_set_label(excret_label); + set_disas_label(s, excret_label); /* Yes: this is an exception return. * At this point in runtime env->regs[15] and env->thumb will hold * the exception-return magic number, which do_v7m_exception_exit() @@ -XXX,XX +XXX,XX @@ static void gen_goto_ptr(void) */ static void gen_goto_tb(DisasContext *s, int n, target_long diff) { - target_ulong dest = s->pc_curr + diff; - - if (translator_use_goto_tb(&s->base, dest)) { - tcg_gen_goto_tb(n); - gen_update_pc(s, diff); + if (translator_use_goto_tb(&s->base, s->pc_curr + diff)) { + /* + * For pcrel, the pc must always be up-to-date on entry to + * the linked TB, so that it can use simple additions for all + * further adjustments. For !pcrel, the linked TB is compiled + * to know its full virtual address, so we can delay the + * update to pc to the unlinked path. A long chain of links + * can thus avoid many updates to the PC. + */ + if (TARGET_TB_PCREL) { + gen_update_pc(s, diff); + tcg_gen_goto_tb(n); + } else { + tcg_gen_goto_tb(n); + gen_update_pc(s, diff); + } tcg_gen_exit_tb(s->base.tb, n); } else { gen_update_pc(s, diff); @@ -XXX,XX +XXX,XX @@ static void gen_srs(DisasContext *s, static void arm_skip_unless(DisasContext *s, uint32_t cond) { arm_gen_condlabel(s); - arm_gen_test_cc(cond ^ 1, s->condlabel); + arm_gen_test_cc(cond ^ 1, s->condlabel.label); } @@ -XXX,XX +XXX,XX @@ static bool trans_WLS(DisasContext *s, arg_WLS *a) { /* M-profile low-overhead while-loop start */ TCGv_i32 tmp; - TCGLabel *nextlabel; + DisasLabel nextlabel; if (!dc_isar_feature(aa32_lob, s)) { return false; @@ -XXX,XX +XXX,XX @@ static bool trans_WLS(DisasContext *s, arg_WLS *a) } } - nextlabel = gen_new_label(); - tcg_gen_brcondi_i32(TCG_COND_EQ, cpu_R[a->rn], 0, nextlabel); + nextlabel = gen_disas_label(s); + tcg_gen_brcondi_i32(TCG_COND_EQ, cpu_R[a->rn], 0, nextlabel.label); tmp = load_reg(s, a->rn); store_reg(s, 14, tmp); if (a->size != 4) { @@ -XXX,XX +XXX,XX @@ static bool trans_WLS(DisasContext *s, arg_WLS *a) } gen_jmp_tb(s, curr_insn_len(s), 1); - gen_set_label(nextlabel); + set_disas_label(s, nextlabel); gen_jmp(s, jmp_diff(s, a->imm)); return true; } @@ -XXX,XX +XXX,XX @@ static bool trans_LE(DisasContext *s, arg_LE *a) * any faster. */ TCGv_i32 tmp; - TCGLabel *loopend; + DisasLabel loopend; bool fpu_active; if (!dc_isar_feature(aa32_lob, s)) { @@ -XXX,XX +XXX,XX @@ static bool trans_LE(DisasContext *s, arg_LE *a) if (!a->tp && dc_isar_feature(aa32_mve, s) && fpu_active) { /* Need to do a runtime check for LTPSIZE != 4 */ - TCGLabel *skipexc = gen_new_label(); + DisasLabel skipexc = gen_disas_label(s); tmp = load_cpu_field(v7m.ltpsize); - tcg_gen_brcondi_i32(TCG_COND_EQ, tmp, 4, skipexc); + tcg_gen_brcondi_i32(TCG_COND_EQ, tmp, 4, skipexc.label); tcg_temp_free_i32(tmp); gen_exception_insn(s, 0, EXCP_INVSTATE, syn_uncategorized()); - gen_set_label(skipexc); + set_disas_label(s, skipexc); } if (a->f) { @@ -XXX,XX +XXX,XX @@ static bool trans_LE(DisasContext *s, arg_LE *a) * loop decrement value is 1. For LETP we need to calculate the decrement * value from LTPSIZE. */ - loopend = gen_new_label(); + loopend = gen_disas_label(s); if (!a->tp) { - tcg_gen_brcondi_i32(TCG_COND_LEU, cpu_R[14], 1, loopend); + tcg_gen_brcondi_i32(TCG_COND_LEU, cpu_R[14], 1, loopend.label); tcg_gen_addi_i32(cpu_R[14], cpu_R[14], -1); } else { /* @@ -XXX,XX +XXX,XX @@ static bool trans_LE(DisasContext *s, arg_LE *a) tcg_gen_shl_i32(decr, tcg_constant_i32(1), decr); tcg_temp_free_i32(ltpsize); - tcg_gen_brcond_i32(TCG_COND_LEU, cpu_R[14], decr, loopend); + tcg_gen_brcond_i32(TCG_COND_LEU, cpu_R[14], decr, loopend.label); tcg_gen_sub_i32(cpu_R[14], cpu_R[14], decr); tcg_temp_free_i32(decr); @@ -XXX,XX +XXX,XX @@ static bool trans_LE(DisasContext *s, arg_LE *a) /* Jump back to the loop start */ gen_jmp(s, jmp_diff(s, -a->imm)); - gen_set_label(loopend); + set_disas_label(s, loopend); if (a->tp) { /* Exits from tail-pred loops must reset LTPSIZE to 4 */ store_cpu_field(tcg_constant_i32(4), v7m.ltpsize); @@ -XXX,XX +XXX,XX @@ static bool trans_CBZ(DisasContext *s, arg_CBZ *a) arm_gen_condlabel(s); tcg_gen_brcondi_i32(a->nz ? TCG_COND_EQ : TCG_COND_NE, - tmp, 0, s->condlabel); + tmp, 0, s->condlabel.label); tcg_temp_free_i32(tmp); gen_jmp(s, jmp_diff(s, a->imm)); return true; @@ -XXX,XX +XXX,XX @@ static void arm_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs) dc->isar = &cpu->isar; dc->condjmp = 0; - + dc->pc_save = dc->base.pc_first; dc->aarch64 = false; dc->thumb = EX_TBFLAG_AM32(tb_flags, THUMB); dc->be_data = EX_TBFLAG_ANY(tb_flags, BE_DATA) ? MO_BE : MO_LE; @@ -XXX,XX +XXX,XX @@ static void arm_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs) */ dc->eci = dc->condexec_mask = dc->condexec_cond = 0; dc->eci_handled = false; - dc->insn_eci_rewind = NULL; if (condexec & 0xf) { dc->condexec_mask = (condexec & 0xf) << 1; dc->condexec_cond = condexec >> 4; @@ -XXX,XX +XXX,XX @@ static void arm_tr_insn_start(DisasContextBase *dcbase, CPUState *cpu) * fields here. */ uint32_t condexec_bits; + target_ulong pc_arg = dc->base.pc_next; + if (TARGET_TB_PCREL) { + pc_arg &= ~TARGET_PAGE_MASK; + } if (dc->eci) { condexec_bits = dc->eci << 4; } else { condexec_bits = (dc->condexec_cond << 4) | (dc->condexec_mask >> 1); } - tcg_gen_insn_start(dc->base.pc_next, condexec_bits, 0); + tcg_gen_insn_start(pc_arg, condexec_bits, 0); dc->insn_start = tcg_last_op(); } @@ -XXX,XX +XXX,XX @@ static bool arm_check_ss_active(DisasContext *dc) static void arm_post_translate_insn(DisasContext *dc) { - if (dc->condjmp && !dc->base.is_jmp) { - gen_set_label(dc->condlabel); + if (dc->condjmp && dc->base.is_jmp == DISAS_NEXT) { + if (dc->pc_save != dc->condlabel.pc_save) { + gen_update_pc(dc, dc->condlabel.pc_save - dc->pc_save); + } + gen_set_label(dc->condlabel.label); dc->condjmp = 0; } translator_loop_temp_check(&dc->base); @@ -XXX,XX +XXX,XX @@ static void thumb_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) uint32_t pc = dc->base.pc_next; uint32_t insn; bool is_16bit; + /* TCG op to rewind to if this turns out to be an invalid ECI state */ + TCGOp *insn_eci_rewind = NULL; + target_ulong insn_eci_pc_save = -1; /* Misaligned thumb PC is architecturally impossible. */ assert((dc->base.pc_next & 1) == 0); @@ -XXX,XX +XXX,XX @@ static void thumb_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) * insn" case. We will rewind to the marker (ie throwing away * all the generated code) and instead emit "take exception". */ - dc->insn_eci_rewind = tcg_last_op(); + insn_eci_rewind = tcg_last_op(); + insn_eci_pc_save = dc->pc_save; } if (dc->condexec_mask && !thumb_insn_is_unconditional(dc, insn)) { @@ -XXX,XX +XXX,XX @@ static void thumb_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) * Insn wasn't valid for ECI/ICI at all: undo what we * just generated and instead emit an exception */ - tcg_remove_ops_after(dc->insn_eci_rewind); + tcg_remove_ops_after(insn_eci_rewind); + dc->pc_save = insn_eci_pc_save; dc->condjmp = 0; gen_exception_insn(dc, 0, EXCP_INVSTATE, syn_uncategorized()); } @@ -XXX,XX +XXX,XX @@ static void arm_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu) if (dc->condjmp) { /* "Condition failed" instruction codepath for the branch/trap insn */ - gen_set_label(dc->condlabel); + set_disas_label(dc, dc->condlabel); gen_set_condexec(dc); if (unlikely(dc->ss_active)) { gen_update_pc(dc, curr_insn_len(dc)); @@ -XXX,XX +XXX,XX @@ void restore_state_to_opc(CPUARMState *env, TranslationBlock *tb, target_ulong *data) { if (is_a64(env)) { - env->pc = data[0]; + if (TARGET_TB_PCREL) { + env->pc = (env->pc & TARGET_PAGE_MASK) | data[0]; + } else { + env->pc = data[0]; + } env->condexec_bits = 0; env->exception.syndrome = data[2] << ARM_INSN_START_WORD2_SHIFT; } else { - env->regs[15] = data[0]; + if (TARGET_TB_PCREL) { + env->regs[15] = (env->regs[15] & TARGET_PAGE_MASK) | data[0]; + } else { + env->regs[15] = data[0]; + } env->condexec_bits = data[1]; env->exception.syndrome = data[2] << ARM_INSN_START_WORD2_SHIFT; } -- 2.25.1 Currently the microdrive code uses device_legacy_reset() to reset itself, and has its reset method call reset on the IDE bus as the last thing it does. Switch to using device_cold_reset(). The only concrete microdrive device is the TYPE_DSCM1XXXX; it is not command-line pluggable, so it is used only by the old pxa2xx Arm boards 'akita', 'borzoi', 'spitz', 'terrier' and 'tosa'. You might think that this would result in the IDE bus being reset automatically, but it does not, because the IDEBus type does not set the BusClass::reset method. Instead the controller must explicitly call ide_bus_reset(). We therefore leave that call in md_reset(). Note also that because the PCMCIA card device is a direct subclass of TYPE_DEVICE and we don't model the PCMCIA controller-to-card interface as a qbus, PCMCIA cards are not on any qbus and so they don't get reset when the system is reset. The reset only happens via the dscm1xxxx_attach() and dscm1xxxx_detach() functions during machine creation. Because our aim here is merely to try to get rid of calls to the device_legacy_reset() function, we leave these other dubious reset-related issues alone. (They all stem from this code being absolutely ancient.) Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Message-id: 20221013174042.1602926-1-peter.maydell@linaro.org --- hw/ide/microdrive.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/hw/ide/microdrive.c b/hw/ide/microdrive.c index XXXXXXX..XXXXXXX 100644 --- a/hw/ide/microdrive.c +++ b/hw/ide/microdrive.c @@ -XXX,XX +XXX,XX @@ static void md_attr_write(PCMCIACardState *card, uint32_t at, uint8_t value) case 0x00: /* Configuration Option Register */ s->opt = value & 0xcf; if (value & OPT_SRESET) { - device_legacy_reset(DEVICE(s)); + device_cold_reset(DEVICE(s)); } md_interrupt_update(s); break; @@ -XXX,XX +XXX,XX @@ static void md_common_write(PCMCIACardState *card, uint32_t at, uint16_t value) case 0xe: /* Device Control */ s->ctrl = value; if (value & CTRL_SRST) { - device_legacy_reset(DEVICE(s)); + device_cold_reset(DEVICE(s)); } md_interrupt_update(s); break; @@ -XXX,XX +XXX,XX @@ static int dscm1xxxx_attach(PCMCIACardState *card) md->attr_base = pcc->cis[0x74] | (pcc->cis[0x76] << 8); md->io_base = 0x0; - device_legacy_reset(DEVICE(md)); + device_cold_reset(DEVICE(md)); md_interrupt_update(md); return 0; @@ -XXX,XX +XXX,XX @@ static int dscm1xxxx_detach(PCMCIACardState *card) { MicroDriveState *md = MICRODRIVE(card); - device_legacy_reset(DEVICE(md)); + device_cold_reset(DEVICE(md)); return 0; } -- 2.25.1