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; this is the latest target-arm queue; most of this is a refactoring patchset from RTH for the arm page-table-walk emulation. thanks -- PMM The following changes since commit f1d33f55c47dfdaf8daacd618588ad3ae4c452d1: Merge tag 'pull-testing-gdbstub-plugins-gitdm-061022-3' of https://github.com/stsquad/qemu into staging (2022-10-06 07:11:56 -0400) are available in the Git repository at: https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20221010 for you to fetch changes up to 915f62844cf62e428c7c178149b5ff1cbe129b07: docs/system/arm/emulation.rst: Report FEAT_GTG support (2022-10-10 14:52:25 +0100) ---------------------------------------------------------------- target-arm queue: * Retry KVM_CREATE_VM call if it fails EINTR * allow setting SCR_EL3.EnTP2 when FEAT_SME is implemented * docs/nuvoton: Update URL for images * refactoring of page table walk code * hw/arm/boot: set CPTR_EL3.ESM and SCR_EL3.EnTP2 when booting Linux with EL3 * Don't allow guest to use unimplemented granule sizes * Report FEAT_GTG support ---------------------------------------------------------------- Jerome Forissier (2): target/arm: allow setting SCR_EL3.EnTP2 when FEAT_SME is implemented hw/arm/boot: set CPTR_EL3.ESM and SCR_EL3.EnTP2 when booting Linux with EL3 Joel Stanley (1): docs/nuvoton: Update URL for images Peter Maydell (4): target/arm/kvm: Retry KVM_CREATE_VM call if it fails EINTR target/arm: Don't allow guest to use unimplemented granule sizes target/arm: Use ARMGranuleSize in ARMVAParameters docs/system/arm/emulation.rst: Report FEAT_GTG support Richard Henderson (21): target/arm: Split s2walk_secure from ipa_secure in get_phys_addr target/arm: Make the final stage1+2 write to secure be unconditional target/arm: Add is_secure parameter to get_phys_addr_lpae target/arm: Fix S2 disabled check in S1_ptw_translate target/arm: Add is_secure parameter to regime_translation_disabled target/arm: Split out get_phys_addr_with_secure target/arm: Add is_secure parameter to v7m_read_half_insn target/arm: Add TBFLAG_M32.SECURE target/arm: Merge regime_is_secure into get_phys_addr target/arm: Add is_secure parameter to do_ats_write target/arm: Fold secure and non-secure a-profile mmu indexes target/arm: Reorg regime_translation_disabled target/arm: Drop secure check for HCR.TGE vs SCTLR_EL1.M target/arm: Introduce arm_hcr_el2_eff_secstate target/arm: Hoist read of *is_secure in S1_ptw_translate target/arm: Remove env argument from combined_attrs_fwb target/arm: Pass HCR to attribute subroutines. target/arm: Fix ATS12NSO* from S PL1 target/arm: Split out get_phys_addr_disabled target/arm: Fix cacheattr in get_phys_addr_disabled target/arm: Use tlb_set_page_full docs/system/arm/emulation.rst | 1 + docs/system/arm/nuvoton.rst | 4 +- target/arm/cpu-param.h | 2 +- target/arm/cpu.h | 181 ++++++++------ target/arm/internals.h | 150 ++++++----- hw/arm/boot.c | 4 + target/arm/helper.c | 332 ++++++++++++++---------- target/arm/kvm.c | 4 +- target/arm/m_helper.c | 29 ++- target/arm/ptw.c | 570 ++++++++++++++++++++++-------------------- target/arm/tlb_helper.c | 9 +- target/arm/translate-a64.c | 8 - target/arm/translate.c | 9 +- 13 files changed, 717 insertions(+), 586 deletions(-) Occasionally the KVM_CREATE_VM ioctl can return EINTR, even though there is no pending signal to be taken. In commit 94ccff13382055 we added a retry-on-EINTR loop to the KVM_CREATE_VM call in the generic KVM code. Adopt the same approach for the use of the ioctl in the Arm-specific KVM code (where we use it to create a scratch VM for probing for various things). For more information, see the mailing list thread: https://lore.kernel.org/qemu-devel/8735e0s1zw.wl-maz@kernel.org/ Reported-by: Vitaly Chikunov <vt@altlinux.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Vitaly Chikunov <vt@altlinux.org> Reviewed-by: Eric Auger <eric.auger@redhat.com> Acked-by: Marc Zyngier <maz@kernel.org> Message-id: 20220930113824.1933293-1-peter.maydell@linaro.org --- target/arm/kvm.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/target/arm/kvm.c b/target/arm/kvm.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/kvm.c +++ b/target/arm/kvm.c @@ -XXX,XX +XXX,XX @@ bool kvm_arm_create_scratch_host_vcpu(const uint32_t *cpus_to_try, if (max_vm_pa_size < 0) { max_vm_pa_size = 0; } - vmfd = ioctl(kvmfd, KVM_CREATE_VM, max_vm_pa_size); + do { + vmfd = ioctl(kvmfd, KVM_CREATE_VM, max_vm_pa_size); + } while (vmfd == -1 && errno == EINTR); if (vmfd < 0) { goto err; } -- 2.25.1 From: Jerome Forissier <jerome.forissier@linaro.org> Updates write_scr() to allow setting SCR_EL3.EnTP2 when FEAT_SME is implemented. SCR_EL3 being a 64-bit register, valid_mask is changed to uint64_t and the SCR_* constants in target/arm/cpu.h are extended to 64-bit so that masking and bitwise not (~) behave as expected. This enables booting Linux with Trusted Firmware-A at EL3 with "-M virt,secure=on -cpu max". Cc: qemu-stable@nongnu.org Fixes: 78cb9776662a ("target/arm: Enable SME for -cpu max") Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org> Reviewed-by: Andre Przywara <andre.przywara@arm.com> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20221004072354.27037-1-jerome.forissier@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/cpu.h | 54 ++++++++++++++++++++++----------------------- target/arm/helper.c | 5 ++++- 2 files changed, 31 insertions(+), 28 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 void xpsr_write(CPUARMState *env, uint32_t val, uint32_t mask) #define HPFAR_NS (1ULL << 63) -#define SCR_NS (1U << 0) -#define SCR_IRQ (1U << 1) -#define SCR_FIQ (1U << 2) -#define SCR_EA (1U << 3) -#define SCR_FW (1U << 4) -#define SCR_AW (1U << 5) -#define SCR_NET (1U << 6) -#define SCR_SMD (1U << 7) -#define SCR_HCE (1U << 8) -#define SCR_SIF (1U << 9) -#define SCR_RW (1U << 10) -#define SCR_ST (1U << 11) -#define SCR_TWI (1U << 12) -#define SCR_TWE (1U << 13) -#define SCR_TLOR (1U << 14) -#define SCR_TERR (1U << 15) -#define SCR_APK (1U << 16) -#define SCR_API (1U << 17) -#define SCR_EEL2 (1U << 18) -#define SCR_EASE (1U << 19) -#define SCR_NMEA (1U << 20) -#define SCR_FIEN (1U << 21) -#define SCR_ENSCXT (1U << 25) -#define SCR_ATA (1U << 26) -#define SCR_FGTEN (1U << 27) -#define SCR_ECVEN (1U << 28) -#define SCR_TWEDEN (1U << 29) +#define SCR_NS (1ULL << 0) +#define SCR_IRQ (1ULL << 1) +#define SCR_FIQ (1ULL << 2) +#define SCR_EA (1ULL << 3) +#define SCR_FW (1ULL << 4) +#define SCR_AW (1ULL << 5) +#define SCR_NET (1ULL << 6) +#define SCR_SMD (1ULL << 7) +#define SCR_HCE (1ULL << 8) +#define SCR_SIF (1ULL << 9) +#define SCR_RW (1ULL << 10) +#define SCR_ST (1ULL << 11) +#define SCR_TWI (1ULL << 12) +#define SCR_TWE (1ULL << 13) +#define SCR_TLOR (1ULL << 14) +#define SCR_TERR (1ULL << 15) +#define SCR_APK (1ULL << 16) +#define SCR_API (1ULL << 17) +#define SCR_EEL2 (1ULL << 18) +#define SCR_EASE (1ULL << 19) +#define SCR_NMEA (1ULL << 20) +#define SCR_FIEN (1ULL << 21) +#define SCR_ENSCXT (1ULL << 25) +#define SCR_ATA (1ULL << 26) +#define SCR_FGTEN (1ULL << 27) +#define SCR_ECVEN (1ULL << 28) +#define SCR_TWEDEN (1ULL << 29) #define SCR_TWEDEL MAKE_64BIT_MASK(30, 4) #define SCR_TME (1ULL << 34) #define SCR_AMVOFFEN (1ULL << 35) 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 vbar_write(CPUARMState *env, const ARMCPRegInfo *ri, static void scr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value) { /* Begin with base v8.0 state. */ - uint32_t valid_mask = 0x3fff; + uint64_t valid_mask = 0x3fff; ARMCPU *cpu = env_archcpu(env); /* @@ -XXX,XX +XXX,XX @@ static void scr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value) if (cpu_isar_feature(aa64_doublefault, cpu)) { valid_mask |= SCR_EASE | SCR_NMEA; } + if (cpu_isar_feature(aa64_sme, cpu)) { + valid_mask |= SCR_ENTP2; + } } else { valid_mask &= ~(SCR_RW | SCR_ST); if (cpu_isar_feature(aa32_ras, cpu)) { -- 2.25.1 From: Joel Stanley <joel@jms.id.au> openpower.xyz was retired some time ago. The OpenBMC Jenkins is where images can be found these days. Signed-off-by: Joel Stanley <joel@jms.id.au> Reviewed-by: Hao Wu <wuhaotsh@google.com> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20221004050042.22681-1-joel@jms.id.au Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- docs/system/arm/nuvoton.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/system/arm/nuvoton.rst b/docs/system/arm/nuvoton.rst index XXXXXXX..XXXXXXX 100644 --- a/docs/system/arm/nuvoton.rst +++ b/docs/system/arm/nuvoton.rst @@ -XXX,XX +XXX,XX @@ Boot options The Nuvoton machines can boot from an OpenBMC firmware image, or directly into a kernel using the ``-kernel`` option. OpenBMC images for ``quanta-gsj`` and -possibly others can be downloaded from the OpenPOWER jenkins : +possibly others can be downloaded from the OpenBMC jenkins : - https://openpower.xyz/ + https://jenkins.openbmc.org/ The firmware image should be attached as an MTD drive. Example : -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> The starting security state comes with the translation regime, not the current state of arm_is_secure_below_el3(). Create a new local variable, s2walk_secure, which does not need to be written back to result->attrs.secure -- we compute that value later, after the S2 walk is complete. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 20221001162318.153420-2-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/ptw.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 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 @@ bool get_phys_addr(CPUARMState *env, target_ulong address, hwaddr ipa; int s1_prot; int ret; - bool ipa_secure; + bool ipa_secure, s2walk_secure; ARMCacheAttrs cacheattrs1; ARMMMUIdx s2_mmu_idx; bool is_el0; @@ -XXX,XX +XXX,XX @@ bool get_phys_addr(CPUARMState *env, target_ulong address, ipa = result->phys; ipa_secure = result->attrs.secure; - if (arm_is_secure_below_el3(env)) { - if (ipa_secure) { - result->attrs.secure = !(env->cp15.vstcr_el2 & VSTCR_SW); - } else { - result->attrs.secure = !(env->cp15.vtcr_el2 & VTCR_NSW); - } + 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; } - s2_mmu_idx = (result->attrs.secure + s2_mmu_idx = (s2walk_secure ? ARMMMUIdx_Stage2_S : ARMMMUIdx_Stage2); is_el0 = mmu_idx == ARMMMUIdx_E10_0 || mmu_idx == ARMMMUIdx_SE10_0; @@ -XXX,XX +XXX,XX @@ bool get_phys_addr(CPUARMState *env, target_ulong address, result->cacheattrs); /* Check if IPA translates to secure or non-secure PA space. */ - if (arm_is_secure_below_el3(env)) { + if (is_secure) { if (ipa_secure) { result->attrs.secure = !(env->cp15.vstcr_el2 & (VSTCR_SA | VSTCR_SW)); -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> While the stage2 call to get_phys_addr_lpae should never set attrs.secure when given a non-secure input, it's just as easy to make the final update to attrs.secure be unconditional and false in the case of non-secure input. Suggested-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20221007152159.1414065-1-richard.henderson@linaro.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/ptw.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 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 @@ bool get_phys_addr(CPUARMState *env, target_ulong address, result->cacheattrs = combine_cacheattrs(env, cacheattrs1, result->cacheattrs); - /* Check if IPA translates to secure or non-secure PA space. */ - if (is_secure) { - if (ipa_secure) { - result->attrs.secure = - !(env->cp15.vstcr_el2 & (VSTCR_SA | VSTCR_SW)); - } else { - result->attrs.secure = - !((env->cp15.vtcr_el2 & (VTCR_NSA | VTCR_NSW)) - || (env->cp15.vstcr_el2 & (VSTCR_SA | VSTCR_SW))); - } - } + /* + * Check if IPA translates to secure or non-secure PA space. + * Note that VSTCR overrides VTCR and {N}SW overrides {N}SA. + */ + result->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 { /* -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Remove the use of regime_is_secure from get_phys_addr_lpae, using the new parameter instead. Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20221001162318.153420-3-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/ptw.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/target/arm/ptw.c b/target/arm/ptw.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/ptw.c +++ b/target/arm/ptw.c @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_lpae(CPUARMState *env, uint64_t address, MMUAccessType access_type, ARMMMUIdx mmu_idx, - bool s1_is_el0, GetPhysAddrResult *result, - ARMMMUFaultInfo *fi) + bool is_secure, bool s1_is_el0, + GetPhysAddrResult *result, ARMMMUFaultInfo *fi) __attribute__((nonnull)); /* This mapping is common between ID_AA64MMFR0.PARANGE and TCR_ELx.{I}PS. */ @@ -XXX,XX +XXX,XX @@ static hwaddr S1_ptw_translate(CPUARMState *env, ARMMMUIdx mmu_idx, GetPhysAddrResult s2 = {}; int ret; - ret = get_phys_addr_lpae(env, addr, MMU_DATA_LOAD, s2_mmu_idx, false, - &s2, fi); + ret = get_phys_addr_lpae(env, addr, MMU_DATA_LOAD, s2_mmu_idx, + *is_secure, false, &s2, fi); if (ret) { assert(fi->type != ARMFault_None); fi->s2addr = addr; @@ -XXX,XX +XXX,XX @@ static bool check_s2_mmu_setup(ARMCPU *cpu, bool is_aa64, int level, */ static bool get_phys_addr_lpae(CPUARMState *env, uint64_t address, MMUAccessType access_type, ARMMMUIdx mmu_idx, - bool s1_is_el0, GetPhysAddrResult *result, - ARMMMUFaultInfo *fi) + bool is_secure, bool s1_is_el0, + GetPhysAddrResult *result, ARMMMUFaultInfo *fi) { ARMCPU *cpu = env_archcpu(env); /* Read an LPAE long-descriptor translation table. */ @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_lpae(CPUARMState *env, uint64_t address, * remain non-secure. We implement this by just ORing in the NSTable/NS * bits at each step. */ - tableattrs = regime_is_secure(env, mmu_idx) ? 0 : (1 << 4); + tableattrs = is_secure ? 0 : (1 << 4); for (;;) { uint64_t descriptor; bool nstable; @@ -XXX,XX +XXX,XX @@ bool get_phys_addr(CPUARMState *env, target_ulong address, memset(result, 0, sizeof(*result)); ret = get_phys_addr_lpae(env, ipa, access_type, s2_mmu_idx, - is_el0, result, fi); + s2walk_secure, is_el0, result, fi); fi->s2addr = ipa; /* Combine the S1 and S2 perms. */ @@ -XXX,XX +XXX,XX @@ bool get_phys_addr(CPUARMState *env, target_ulong address, } if (regime_using_lpae_format(env, mmu_idx)) { - return get_phys_addr_lpae(env, address, access_type, mmu_idx, false, - result, fi); + return get_phys_addr_lpae(env, address, access_type, mmu_idx, + is_secure, 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); -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Pass the correct stage2 mmu_idx to regime_translation_disabled, which we computed afterward. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 20221001162318.153420-4-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/ptw.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/target/arm/ptw.c b/target/arm/ptw.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/ptw.c +++ b/target/arm/ptw.c @@ -XXX,XX +XXX,XX @@ static hwaddr S1_ptw_translate(CPUARMState *env, ARMMMUIdx mmu_idx, hwaddr addr, bool *is_secure, ARMMMUFaultInfo *fi) { + ARMMMUIdx s2_mmu_idx = *is_secure ? ARMMMUIdx_Stage2_S : ARMMMUIdx_Stage2; + if (arm_mmu_idx_is_stage1_of_2(mmu_idx) && - !regime_translation_disabled(env, ARMMMUIdx_Stage2)) { - ARMMMUIdx s2_mmu_idx = *is_secure ? ARMMMUIdx_Stage2_S - : ARMMMUIdx_Stage2; + !regime_translation_disabled(env, s2_mmu_idx)) { GetPhysAddrResult s2 = {}; int ret; -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Remove the use of regime_is_secure from regime_translation_disabled, using the new parameter instead. This fixes a bug in S1_ptw_translate and get_phys_addr where we had passed ARMMMUIdx_Stage2 and not ARMMMUIdx_Stage2_S to determine if Stage2 is disabled, affecting FEAT_SEL2. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20221001162318.153420-5-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/ptw.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 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 uint64_t regime_ttbr(CPUARMState *env, ARMMMUIdx mmu_idx, int ttbrn) } /* Return true if the specified stage of address translation is disabled */ -static bool regime_translation_disabled(CPUARMState *env, ARMMMUIdx mmu_idx) +static bool regime_translation_disabled(CPUARMState *env, ARMMMUIdx mmu_idx, + bool is_secure) { uint64_t hcr_el2; if (arm_feature(env, ARM_FEATURE_M)) { - switch (env->v7m.mpu_ctrl[regime_is_secure(env, mmu_idx)] & + switch (env->v7m.mpu_ctrl[is_secure] & (R_V7M_MPU_CTRL_ENABLE_MASK | R_V7M_MPU_CTRL_HFNMIENA_MASK)) { case R_V7M_MPU_CTRL_ENABLE_MASK: /* Enabled, but not for HardFault and NMI */ @@ -XXX,XX +XXX,XX @@ static bool regime_translation_disabled(CPUARMState *env, ARMMMUIdx mmu_idx) if (hcr_el2 & HCR_TGE) { /* TGE means that NS EL0/1 act as if SCTLR_EL1.M is zero */ - if (!regime_is_secure(env, mmu_idx) && regime_el(env, mmu_idx) == 1) { + if (!is_secure && regime_el(env, mmu_idx) == 1) { return true; } } @@ -XXX,XX +XXX,XX @@ static hwaddr S1_ptw_translate(CPUARMState *env, ARMMMUIdx mmu_idx, ARMMMUIdx s2_mmu_idx = *is_secure ? ARMMMUIdx_Stage2_S : ARMMMUIdx_Stage2; if (arm_mmu_idx_is_stage1_of_2(mmu_idx) && - !regime_translation_disabled(env, s2_mmu_idx)) { + !regime_translation_disabled(env, s2_mmu_idx, *is_secure)) { GetPhysAddrResult s2 = {}; int ret; @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_pmsav5(CPUARMState *env, uint32_t address, uint32_t base; bool is_user = regime_is_user(env, mmu_idx); - if (regime_translation_disabled(env, mmu_idx)) { + if (regime_translation_disabled(env, mmu_idx, is_secure)) { /* MPU disabled. */ result->phys = address; result->prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC; @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address, result->page_size = TARGET_PAGE_SIZE; result->prot = 0; - if (regime_translation_disabled(env, mmu_idx) || + if (regime_translation_disabled(env, mmu_idx, secure) || m_is_ppb_region(env, address)) { /* * MPU disabled or M profile PPB access: use default memory map. @@ -XXX,XX +XXX,XX @@ bool pmsav8_mpu_lookup(CPUARMState *env, uint32_t address, * are done in arm_v7m_load_vector(), which always does a direct * read using address_space_ldl(), rather than going via this function. */ - if (regime_translation_disabled(env, mmu_idx)) { /* MPU disabled */ + if (regime_translation_disabled(env, mmu_idx, secure)) { /* MPU disabled */ hit = true; } else if (m_is_ppb_region(env, address)) { hit = true; @@ -XXX,XX +XXX,XX @@ bool get_phys_addr(CPUARMState *env, target_ulong address, result, fi); /* If S1 fails or S2 is disabled, return early. */ - if (ret || regime_translation_disabled(env, ARMMMUIdx_Stage2)) { + if (ret || regime_translation_disabled(env, ARMMMUIdx_Stage2, + is_secure)) { return ret; } @@ -XXX,XX +XXX,XX @@ bool get_phys_addr(CPUARMState *env, target_ulong address, /* Definitely a real MMU, not an MPU */ - if (regime_translation_disabled(env, mmu_idx)) { + if (regime_translation_disabled(env, mmu_idx, is_secure)) { uint64_t hcr; uint8_t memattr; -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Retain the existing get_phys_addr interface using the security state derived from mmu_idx. Move the kerneldoc comments to the header file where they belong. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20221001162318.153420-6-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/internals.h | 40 ++++++++++++++++++++++++++++++++++++++ target/arm/ptw.c | 44 ++++++++++++++---------------------------- 2 files changed, 55 insertions(+), 29 deletions(-) diff --git a/target/arm/internals.h b/target/arm/internals.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/internals.h +++ b/target/arm/internals.h @@ -XXX,XX +XXX,XX @@ typedef struct GetPhysAddrResult { ARMCacheAttrs cacheattrs; } GetPhysAddrResult; +/** + * get_phys_addr_with_secure: get the physical address for a virtual address + * @env: CPUARMState + * @address: virtual address to get physical address for + * @access_type: 0 for read, 1 for write, 2 for execute + * @mmu_idx: MMU index indicating required translation regime + * @is_secure: security state for the access + * @result: set on translation success. + * @fi: set to fault info if the translation fails + * + * Find the physical address corresponding to the given virtual address, + * by doing a translation table walk on MMU based systems or using the + * MPU state on MPU based systems. + * + * Returns false if the translation was successful. Otherwise, phys_ptr, attrs, + * prot and page_size may not be filled in, and the populated fsr value provides + * information on why the translation aborted, in the format of a + * DFSR/IFSR fault register, with the following caveats: + * * we honour the short vs long DFSR format differences. + * * the WnR bit is never set (the caller must do this). + * * for PSMAv5 based systems we don't bother to return a full FSR format + * value. + */ +bool get_phys_addr_with_secure(CPUARMState *env, target_ulong address, + MMUAccessType access_type, + ARMMMUIdx mmu_idx, bool is_secure, + GetPhysAddrResult *result, ARMMMUFaultInfo *fi) + __attribute__((nonnull)); + +/** + * get_phys_addr: get the physical address for a virtual address + * @env: CPUARMState + * @address: virtual address to get physical address for + * @access_type: 0 for read, 1 for write, 2 for execute + * @mmu_idx: MMU index indicating required translation regime + * @result: set on translation success. + * @fi: set to fault info if the translation fails + * + * Similarly, but use the security regime of @mmu_idx. + */ bool get_phys_addr(CPUARMState *env, target_ulong address, MMUAccessType access_type, ARMMMUIdx mmu_idx, GetPhysAddrResult *result, ARMMMUFaultInfo *fi) diff --git a/target/arm/ptw.c b/target/arm/ptw.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/ptw.c +++ b/target/arm/ptw.c @@ -XXX,XX +XXX,XX @@ static ARMCacheAttrs combine_cacheattrs(CPUARMState *env, return ret; } -/** - * get_phys_addr - get the physical address for this virtual address - * - * Find the physical address corresponding to the given virtual address, - * by doing a translation table walk on MMU based systems or using the - * MPU state on MPU based systems. - * - * Returns false if the translation was successful. Otherwise, phys_ptr, attrs, - * prot and page_size may not be filled in, and the populated fsr value provides - * information on why the translation aborted, in the format of a - * DFSR/IFSR fault register, with the following caveats: - * * we honour the short vs long DFSR format differences. - * * the WnR bit is never set (the caller must do this). - * * for PSMAv5 based systems we don't bother to return a full FSR format - * value. - * - * @env: CPUARMState - * @address: virtual address to get physical address for - * @access_type: 0 for read, 1 for write, 2 for execute - * @mmu_idx: MMU index indicating required translation regime - * @result: set on translation success. - * @fi: set to fault info if the translation fails - */ -bool get_phys_addr(CPUARMState *env, target_ulong address, - MMUAccessType access_type, ARMMMUIdx mmu_idx, - GetPhysAddrResult *result, ARMMMUFaultInfo *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) { ARMMMUIdx s1_mmu_idx = stage_1_mmu_idx(mmu_idx); - bool is_secure = regime_is_secure(env, mmu_idx); if (mmu_idx != s1_mmu_idx) { /* @@ -XXX,XX +XXX,XX @@ bool get_phys_addr(CPUARMState *env, target_ulong address, ARMMMUIdx s2_mmu_idx; bool is_el0; - ret = get_phys_addr(env, address, access_type, s1_mmu_idx, - result, fi); + ret = get_phys_addr_with_secure(env, address, access_type, + s1_mmu_idx, is_secure, 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(CPUARMState *env, target_ulong address, } } +bool get_phys_addr(CPUARMState *env, target_ulong address, + MMUAccessType access_type, ARMMMUIdx mmu_idx, + GetPhysAddrResult *result, ARMMMUFaultInfo *fi) +{ + return get_phys_addr_with_secure(env, address, access_type, mmu_idx, + regime_is_secure(env, mmu_idx), + result, fi); +} + hwaddr arm_cpu_get_phys_page_attrs_debug(CPUState *cs, vaddr addr, MemTxAttrs *attrs) { -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Remove the use of regime_is_secure from v7m_read_half_insn, using the new parameter instead. As it happens, both callers pass true, propagated from the argument to arm_v7m_mmu_idx_for_secstate which created the mmu_idx argument, but that is a detail of v7m_handle_execute_nsc we need not expose to the callee. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20221001162318.153420-7-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/m_helper.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/target/arm/m_helper.c b/target/arm/m_helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/m_helper.c +++ b/target/arm/m_helper.c @@ -XXX,XX +XXX,XX @@ static bool do_v7m_function_return(ARMCPU *cpu) return true; } -static bool v7m_read_half_insn(ARMCPU *cpu, ARMMMUIdx mmu_idx, +static bool v7m_read_half_insn(ARMCPU *cpu, ARMMMUIdx mmu_idx, bool secure, uint32_t addr, uint16_t *insn) { /* @@ -XXX,XX +XXX,XX @@ static bool v7m_read_half_insn(ARMCPU *cpu, ARMMMUIdx mmu_idx, ARMMMUFaultInfo fi = {}; MemTxResult txres; - v8m_security_lookup(env, addr, MMU_INST_FETCH, mmu_idx, - regime_is_secure(env, mmu_idx), &sattrs); + v8m_security_lookup(env, addr, MMU_INST_FETCH, mmu_idx, secure, &sattrs); if (!sattrs.nsc || sattrs.ns) { /* * This must be the second half of the insn, and it straddles a @@ -XXX,XX +XXX,XX @@ static bool v7m_handle_execute_nsc(ARMCPU *cpu) /* We want to do the MPU lookup as secure; work out what mmu_idx that is */ mmu_idx = arm_v7m_mmu_idx_for_secstate(env, true); - if (!v7m_read_half_insn(cpu, mmu_idx, env->regs[15], &insn)) { + if (!v7m_read_half_insn(cpu, mmu_idx, true, env->regs[15], &insn)) { return false; } @@ -XXX,XX +XXX,XX @@ static bool v7m_handle_execute_nsc(ARMCPU *cpu) goto gen_invep; } - if (!v7m_read_half_insn(cpu, mmu_idx, env->regs[15] + 2, &insn)) { + if (!v7m_read_half_insn(cpu, mmu_idx, true, env->regs[15] + 2, &insn)) { return false; } -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Remove the use of regime_is_secure from arm_tr_init_disas_context. Instead, provide the value of v8m_secure directly from tb_flags. Rather than use regime_is_secure, use the env->v7m.secure directly, as per arm_mmu_idx_el. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20221001162318.153420-8-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/cpu.h | 2 ++ target/arm/helper.c | 4 ++++ target/arm/translate.c | 3 +-- 3 files changed, 7 insertions(+), 2 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 @@ FIELD(TBFLAG_M32, NEW_FP_CTXT_NEEDED, 3, 1) /* Not cached. */ FIELD(TBFLAG_M32, FPCCR_S_WRONG, 4, 1) /* Not cached. */ /* Set if MVE insns are definitely not predicated by VPR or LTPSIZE */ FIELD(TBFLAG_M32, MVE_NO_PRED, 5, 1) /* Not cached. */ +/* Set if in secure mode */ +FIELD(TBFLAG_M32, SECURE, 6, 1) /* * Bit usage when in AArch64 state 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 CPUARMTBFlags rebuild_hflags_m32(CPUARMState *env, int fp_el, DP_TBFLAG_M32(flags, STACKCHECK, 1); } + if (arm_feature(env, ARM_FEATURE_M_SECURITY) && env->v7m.secure) { + DP_TBFLAG_M32(flags, SECURE, 1); + } + return rebuild_hflags_common_32(env, fp_el, mmu_idx, flags); } diff --git a/target/arm/translate.c b/target/arm/translate.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -XXX,XX +XXX,XX @@ static void arm_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs) dc->vfp_enabled = 1; dc->be_data = MO_TE; dc->v7m_handler_mode = EX_TBFLAG_M32(tb_flags, HANDLER); - dc->v8m_secure = arm_feature(env, ARM_FEATURE_M_SECURITY) && - regime_is_secure(env, dc->mmu_idx); + dc->v8m_secure = EX_TBFLAG_M32(tb_flags, SECURE); dc->v8m_stackcheck = EX_TBFLAG_M32(tb_flags, STACKCHECK); dc->v8m_fpccr_s_wrong = EX_TBFLAG_M32(tb_flags, FPCCR_S_WRONG); dc->v7m_new_fp_ctxt_needed = -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> This is the last use of regime_is_secure; remove it entirely before changing the layout of ARMMMUIdx. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20221001162318.153420-9-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/internals.h | 42 ---------------------------------------- target/arm/ptw.c | 44 ++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 42 insertions(+), 44 deletions(-) diff --git a/target/arm/internals.h b/target/arm/internals.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/internals.h +++ b/target/arm/internals.h @@ -XXX,XX +XXX,XX @@ static inline bool regime_has_2_ranges(ARMMMUIdx mmu_idx) } } -/* Return true if this address translation regime is secure */ -static inline bool regime_is_secure(CPUARMState *env, ARMMMUIdx mmu_idx) -{ - switch (mmu_idx) { - case ARMMMUIdx_E10_0: - case ARMMMUIdx_E10_1: - case ARMMMUIdx_E10_1_PAN: - case ARMMMUIdx_E20_0: - case ARMMMUIdx_E20_2: - case ARMMMUIdx_E20_2_PAN: - case ARMMMUIdx_Stage1_E0: - case ARMMMUIdx_Stage1_E1: - case ARMMMUIdx_Stage1_E1_PAN: - case ARMMMUIdx_E2: - case ARMMMUIdx_Stage2: - case ARMMMUIdx_MPrivNegPri: - case ARMMMUIdx_MUserNegPri: - case ARMMMUIdx_MPriv: - case ARMMMUIdx_MUser: - return false; - case ARMMMUIdx_SE3: - case ARMMMUIdx_SE10_0: - case ARMMMUIdx_SE10_1: - case ARMMMUIdx_SE10_1_PAN: - case ARMMMUIdx_SE20_0: - case ARMMMUIdx_SE20_2: - case ARMMMUIdx_SE20_2_PAN: - case ARMMMUIdx_Stage1_SE0: - case ARMMMUIdx_Stage1_SE1: - case ARMMMUIdx_Stage1_SE1_PAN: - case ARMMMUIdx_SE2: - case ARMMMUIdx_Stage2_S: - case ARMMMUIdx_MSPrivNegPri: - case ARMMMUIdx_MSUserNegPri: - case ARMMMUIdx_MSPriv: - case ARMMMUIdx_MSUser: - return true; - default: - g_assert_not_reached(); - } -} - static inline bool regime_is_pan(CPUARMState *env, ARMMMUIdx mmu_idx) { switch (mmu_idx) { 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 @@ bool get_phys_addr(CPUARMState *env, target_ulong address, MMUAccessType access_type, ARMMMUIdx mmu_idx, GetPhysAddrResult *result, ARMMMUFaultInfo *fi) { + bool is_secure; + + switch (mmu_idx) { + case ARMMMUIdx_E10_0: + case ARMMMUIdx_E10_1: + case ARMMMUIdx_E10_1_PAN: + case ARMMMUIdx_E20_0: + case ARMMMUIdx_E20_2: + case ARMMMUIdx_E20_2_PAN: + case ARMMMUIdx_Stage1_E0: + case ARMMMUIdx_Stage1_E1: + case ARMMMUIdx_Stage1_E1_PAN: + case ARMMMUIdx_E2: + case ARMMMUIdx_Stage2: + case ARMMMUIdx_MPrivNegPri: + case ARMMMUIdx_MUserNegPri: + case ARMMMUIdx_MPriv: + case ARMMMUIdx_MUser: + is_secure = false; + break; + case ARMMMUIdx_SE3: + case ARMMMUIdx_SE10_0: + case ARMMMUIdx_SE10_1: + case ARMMMUIdx_SE10_1_PAN: + case ARMMMUIdx_SE20_0: + case ARMMMUIdx_SE20_2: + case ARMMMUIdx_SE20_2_PAN: + case ARMMMUIdx_Stage1_SE0: + case ARMMMUIdx_Stage1_SE1: + case ARMMMUIdx_Stage1_SE1_PAN: + case ARMMMUIdx_SE2: + case ARMMMUIdx_Stage2_S: + case ARMMMUIdx_MSPrivNegPri: + case ARMMMUIdx_MSUserNegPri: + case ARMMMUIdx_MSPriv: + case ARMMMUIdx_MSUser: + is_secure = true; + break; + default: + g_assert_not_reached(); + } return get_phys_addr_with_secure(env, address, access_type, mmu_idx, - regime_is_secure(env, mmu_idx), - result, fi); + is_secure, result, fi); } hwaddr arm_cpu_get_phys_page_attrs_debug(CPUState *cs, vaddr addr, -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Use get_phys_addr_with_secure directly. For a-profile, this is the one place where the value of is_secure may not equal arm_is_secure(env). Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20221001162318.153420-10-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/helper.c | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 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 CPAccessResult ats_access(CPUARMState *env, const ARMCPRegInfo *ri, #ifdef CONFIG_TCG static uint64_t do_ats_write(CPUARMState *env, uint64_t value, - MMUAccessType access_type, ARMMMUIdx mmu_idx) + MMUAccessType access_type, ARMMMUIdx mmu_idx, + bool is_secure) { bool ret; uint64_t par64; @@ -XXX,XX +XXX,XX @@ static uint64_t do_ats_write(CPUARMState *env, uint64_t value, ARMMMUFaultInfo fi = {}; GetPhysAddrResult res = {}; - ret = get_phys_addr(env, value, access_type, mmu_idx, &res, &fi); + ret = get_phys_addr_with_secure(env, value, access_type, mmu_idx, + is_secure, &res, &fi); /* * ATS operations only do S1 or S1+S2 translations, so we never @@ -XXX,XX +XXX,XX @@ static void ats_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value) switch (el) { case 3: mmu_idx = ARMMMUIdx_SE3; + secure = true; break; case 2: g_assert(!secure); /* ARMv8.4-SecEL2 is 64-bit only */ @@ -XXX,XX +XXX,XX @@ static void ats_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value) switch (el) { case 3: mmu_idx = ARMMMUIdx_SE10_0; + secure = true; break; case 2: g_assert(!secure); /* ARMv8.4-SecEL2 is 64-bit only */ @@ -XXX,XX +XXX,XX @@ static void ats_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value) case 4: /* stage 1+2 NonSecure PL1: ATS12NSOPR, ATS12NSOPW */ mmu_idx = ARMMMUIdx_E10_1; + secure = false; break; case 6: /* stage 1+2 NonSecure PL0: ATS12NSOUR, ATS12NSOUW */ mmu_idx = ARMMMUIdx_E10_0; + secure = false; break; default: g_assert_not_reached(); } - par64 = do_ats_write(env, value, access_type, mmu_idx); + par64 = do_ats_write(env, value, access_type, mmu_idx, secure); A32_BANKED_CURRENT_REG_SET(env, par, par64); #else @@ -XXX,XX +XXX,XX @@ static void ats1h_write(CPUARMState *env, const ARMCPRegInfo *ri, MMUAccessType access_type = ri->opc2 & 1 ? MMU_DATA_STORE : MMU_DATA_LOAD; uint64_t par64; - par64 = do_ats_write(env, value, access_type, ARMMMUIdx_E2); + /* There is no SecureEL2 for AArch32. */ + par64 = do_ats_write(env, value, access_type, ARMMMUIdx_E2, false); A32_BANKED_CURRENT_REG_SET(env, par, par64); #else @@ -XXX,XX +XXX,XX @@ static void ats_write64(CPUARMState *env, const ARMCPRegInfo *ri, break; case 6: /* AT S1E3R, AT S1E3W */ mmu_idx = ARMMMUIdx_SE3; + secure = true; break; default: g_assert_not_reached(); @@ -XXX,XX +XXX,XX @@ static void ats_write64(CPUARMState *env, const ARMCPRegInfo *ri, g_assert_not_reached(); } - env->cp15.par_el[1] = do_ats_write(env, value, access_type, mmu_idx); + env->cp15.par_el[1] = do_ats_write(env, value, access_type, + mmu_idx, secure); #else /* Handled by hardware accelerator. */ g_assert_not_reached(); -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> For a-profile aarch64, which does not bank system registers, it takes quite a lot of code to switch between security states. In the process, registers such as TCR_EL{1,2} must be swapped, which in itself requires the flushing of softmmu tlbs. Therefore it doesn't buy us anything to separate tlbs by security state. Retain the distinction between Stage2 and Stage2_S. This will be important as we implement FEAT_RME, and do not wish to add a third set of mmu indexes for Realm state. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20221001162318.153420-11-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/cpu-param.h | 2 +- target/arm/cpu.h | 72 +++++++------------ target/arm/internals.h | 31 +------- target/arm/helper.c | 144 +++++++++++++------------------------ target/arm/ptw.c | 25 ++----- target/arm/translate-a64.c | 8 --- target/arm/translate.c | 6 +- 7 files changed, 85 insertions(+), 203 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_MIN 10 #endif -#define NB_MMU_MODES 15 +#define NB_MMU_MODES 8 #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); * table over and over. * 6. we need separate EL1/EL2 mmu_idx for handling the Privileged Access * Never (PAN) bit within PSTATE. + * 7. we fold together the secure and non-secure regimes for A-profile, + * because there are no banked system registers for aarch64, so the + * process of switching between secure and non-secure is + * already heavyweight. * * This gives us the following list of cases: * - * NS EL0 EL1&0 stage 1+2 (aka NS PL0) - * NS EL1 EL1&0 stage 1+2 (aka NS PL1) - * NS EL1 EL1&0 stage 1+2 +PAN - * NS EL0 EL2&0 - * NS EL2 EL2&0 - * NS EL2 EL2&0 +PAN - * NS EL2 (aka NS PL2) - * S EL0 EL1&0 (aka S PL0) - * S EL1 EL1&0 (not used if EL3 is 32 bit) - * S EL1 EL1&0 +PAN - * S EL3 (aka S PL1) + * EL0 EL1&0 stage 1+2 (aka NS PL0) + * EL1 EL1&0 stage 1+2 (aka NS PL1) + * EL1 EL1&0 stage 1+2 +PAN + * EL0 EL2&0 + * EL2 EL2&0 + * EL2 EL2&0 +PAN + * EL2 (aka NS PL2) + * EL3 (aka S PL1) * - * for a total of 11 different mmu_idx. + * for a total of 8 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 NS EL0 and NS EL1 (and - * NS EL2 if we ever model a Cortex-R52). + * as A profile. They only need to distinguish EL0 and EL1 (and + * EL2 if we ever model a Cortex-R52). * * M profile CPUs are rather different as they do not have a true MMU. * They have the following different MMU indexes: @@ -XXX,XX +XXX,XX @@ bool write_cpustate_to_list(ARMCPU *cpu, bool kvm_sync); #define ARM_MMU_IDX_NOTLB 0x20 /* does not have a TLB */ #define ARM_MMU_IDX_M 0x40 /* M profile */ -/* Meanings of the bits for A profile mmu idx values */ -#define ARM_MMU_IDX_A_NS 0x8 - /* Meanings of the bits for M profile mmu idx values */ #define ARM_MMU_IDX_M_PRIV 0x1 #define ARM_MMU_IDX_M_NEGPRI 0x2 @@ -XXX,XX +XXX,XX @@ typedef enum ARMMMUIdx { /* * A-profile. */ - ARMMMUIdx_SE10_0 = 0 | ARM_MMU_IDX_A, - ARMMMUIdx_SE20_0 = 1 | ARM_MMU_IDX_A, - ARMMMUIdx_SE10_1 = 2 | ARM_MMU_IDX_A, - ARMMMUIdx_SE20_2 = 3 | ARM_MMU_IDX_A, - ARMMMUIdx_SE10_1_PAN = 4 | ARM_MMU_IDX_A, - ARMMMUIdx_SE20_2_PAN = 5 | ARM_MMU_IDX_A, - ARMMMUIdx_SE2 = 6 | ARM_MMU_IDX_A, - ARMMMUIdx_SE3 = 7 | ARM_MMU_IDX_A, - - ARMMMUIdx_E10_0 = ARMMMUIdx_SE10_0 | ARM_MMU_IDX_A_NS, - ARMMMUIdx_E20_0 = ARMMMUIdx_SE20_0 | ARM_MMU_IDX_A_NS, - ARMMMUIdx_E10_1 = ARMMMUIdx_SE10_1 | ARM_MMU_IDX_A_NS, - ARMMMUIdx_E20_2 = ARMMMUIdx_SE20_2 | ARM_MMU_IDX_A_NS, - ARMMMUIdx_E10_1_PAN = ARMMMUIdx_SE10_1_PAN | ARM_MMU_IDX_A_NS, - ARMMMUIdx_E20_2_PAN = ARMMMUIdx_SE20_2_PAN | ARM_MMU_IDX_A_NS, - ARMMMUIdx_E2 = ARMMMUIdx_SE2 | ARM_MMU_IDX_A_NS, + ARMMMUIdx_E10_0 = 0 | ARM_MMU_IDX_A, + ARMMMUIdx_E20_0 = 1 | ARM_MMU_IDX_A, + ARMMMUIdx_E10_1 = 2 | ARM_MMU_IDX_A, + ARMMMUIdx_E20_2 = 3 | ARM_MMU_IDX_A, + ARMMMUIdx_E10_1_PAN = 4 | ARM_MMU_IDX_A, + ARMMMUIdx_E20_2_PAN = 5 | ARM_MMU_IDX_A, + ARMMMUIdx_E2 = 6 | ARM_MMU_IDX_A, + ARMMMUIdx_E3 = 7 | ARM_MMU_IDX_A, /* * These are not allocated TLBs and are used only for AT system @@ -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, - ARMMMUIdx_Stage1_SE0 = 3 | ARM_MMU_IDX_NOTLB, - ARMMMUIdx_Stage1_SE1 = 4 | ARM_MMU_IDX_NOTLB, - ARMMMUIdx_Stage1_SE1_PAN = 5 | 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 @@ -XXX,XX +XXX,XX @@ typedef enum ARMMMUIdx { * 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 = 6 | ARM_MMU_IDX_NOTLB, - ARMMMUIdx_Stage2_S = 7 | ARM_MMU_IDX_NOTLB, + 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(E2), TO_CORE_BIT(E20_2), TO_CORE_BIT(E20_2_PAN), - TO_CORE_BIT(SE10_0), - TO_CORE_BIT(SE20_0), - TO_CORE_BIT(SE10_1), - TO_CORE_BIT(SE20_2), - TO_CORE_BIT(SE10_1_PAN), - TO_CORE_BIT(SE20_2_PAN), - TO_CORE_BIT(SE2), - TO_CORE_BIT(SE3), + TO_CORE_BIT(E3), TO_CORE_BIT(MUser), TO_CORE_BIT(MPriv), 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 @@ static inline bool regime_has_2_ranges(ARMMMUIdx mmu_idx) case ARMMMUIdx_Stage1_E0: case ARMMMUIdx_Stage1_E1: case ARMMMUIdx_Stage1_E1_PAN: - case ARMMMUIdx_Stage1_SE0: - case ARMMMUIdx_Stage1_SE1: - case ARMMMUIdx_Stage1_SE1_PAN: case ARMMMUIdx_E10_0: case ARMMMUIdx_E10_1: case ARMMMUIdx_E10_1_PAN: case ARMMMUIdx_E20_0: case ARMMMUIdx_E20_2: case ARMMMUIdx_E20_2_PAN: - case ARMMMUIdx_SE10_0: - case ARMMMUIdx_SE10_1: - case ARMMMUIdx_SE10_1_PAN: - case ARMMMUIdx_SE20_0: - case ARMMMUIdx_SE20_2: - case ARMMMUIdx_SE20_2_PAN: return true; default: return false; @@ -XXX,XX +XXX,XX @@ static inline bool regime_is_pan(CPUARMState *env, ARMMMUIdx mmu_idx) { switch (mmu_idx) { case ARMMMUIdx_Stage1_E1_PAN: - case ARMMMUIdx_Stage1_SE1_PAN: case ARMMMUIdx_E10_1_PAN: case ARMMMUIdx_E20_2_PAN: - case ARMMMUIdx_SE10_1_PAN: - case ARMMMUIdx_SE20_2_PAN: return true; default: return false; @@ -XXX,XX +XXX,XX @@ static inline bool regime_is_pan(CPUARMState *env, ARMMMUIdx mmu_idx) static inline uint32_t regime_el(CPUARMState *env, ARMMMUIdx mmu_idx) { switch (mmu_idx) { - case ARMMMUIdx_SE20_0: - case ARMMMUIdx_SE20_2: - case ARMMMUIdx_SE20_2_PAN: case ARMMMUIdx_E20_0: case ARMMMUIdx_E20_2: case ARMMMUIdx_E20_2_PAN: case ARMMMUIdx_Stage2: case ARMMMUIdx_Stage2_S: - case ARMMMUIdx_SE2: case ARMMMUIdx_E2: return 2; - case ARMMMUIdx_SE3: + case ARMMMUIdx_E3: return 3; - case ARMMMUIdx_SE10_0: - case ARMMMUIdx_Stage1_SE0: - return arm_el_is_aa64(env, 3) ? 1 : 3; - case ARMMMUIdx_SE10_1: - case ARMMMUIdx_SE10_1_PAN: + case ARMMMUIdx_E10_0: case ARMMMUIdx_Stage1_E0: + return arm_el_is_aa64(env, 3) || !arm_is_secure_below_el3(env) ? 1 : 3; case ARMMMUIdx_Stage1_E1: case ARMMMUIdx_Stage1_E1_PAN: - case ARMMMUIdx_Stage1_SE1: - case ARMMMUIdx_Stage1_SE1_PAN: - case ARMMMUIdx_E10_0: case ARMMMUIdx_E10_1: case ARMMMUIdx_E10_1_PAN: case ARMMMUIdx_MPrivNegPri: @@ -XXX,XX +XXX,XX @@ static inline bool arm_mmu_idx_is_stage1_of_2(ARMMMUIdx mmu_idx) case ARMMMUIdx_Stage1_E0: case ARMMMUIdx_Stage1_E1: case ARMMMUIdx_Stage1_E1_PAN: - case ARMMMUIdx_Stage1_SE0: - case ARMMMUIdx_Stage1_SE1: - case ARMMMUIdx_Stage1_SE1_PAN: return true; default: return false; 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 scr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value) /* Begin with base v8.0 state. */ uint64_t valid_mask = 0x3fff; ARMCPU *cpu = env_archcpu(env); + uint64_t changed; /* * Because SCR_EL3 is the "real" cpreg and SCR is the alias, reset always @@ -XXX,XX +XXX,XX @@ static void scr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value) /* Clear all-context RES0 bits. */ value &= valid_mask; - raw_write(env, ri, value); + changed = env->cp15.scr_el3 ^ value; + env->cp15.scr_el3 = value; + + /* + * If SCR_EL3.NS changes, i.e. arm_is_secure_below_el3, then + * we must invalidate all TLBs below EL3. + */ + if (changed & SCR_NS) { + tlb_flush_by_mmuidx(env_cpu(env), (ARMMMUIdxBit_E10_0 | + ARMMMUIdxBit_E20_0 | + ARMMMUIdxBit_E10_1 | + ARMMMUIdxBit_E20_2 | + ARMMMUIdxBit_E10_1_PAN | + ARMMMUIdxBit_E20_2_PAN | + ARMMMUIdxBit_E2)); + } } static void scr_reset(CPUARMState *env, const ARMCPRegInfo *ri) @@ -XXX,XX +XXX,XX @@ static int gt_phys_redir_timeridx(CPUARMState *env) case ARMMMUIdx_E20_0: case ARMMMUIdx_E20_2: case ARMMMUIdx_E20_2_PAN: - case ARMMMUIdx_SE20_0: - case ARMMMUIdx_SE20_2: - case ARMMMUIdx_SE20_2_PAN: return GTIMER_HYP; default: return GTIMER_PHYS; @@ -XXX,XX +XXX,XX @@ static int gt_virt_redir_timeridx(CPUARMState *env) case ARMMMUIdx_E20_0: case ARMMMUIdx_E20_2: case ARMMMUIdx_E20_2_PAN: - case ARMMMUIdx_SE20_0: - case ARMMMUIdx_SE20_2: - case ARMMMUIdx_SE20_2_PAN: return GTIMER_HYPVIRT; default: return GTIMER_VIRT; @@ -XXX,XX +XXX,XX @@ static void ats_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value) /* stage 1 current state PL1: ATS1CPR, ATS1CPW, ATS1CPRP, ATS1CPWP */ switch (el) { case 3: - mmu_idx = ARMMMUIdx_SE3; + mmu_idx = ARMMMUIdx_E3; secure = true; break; case 2: @@ -XXX,XX +XXX,XX @@ static void ats_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value) /* fall through */ case 1: if (ri->crm == 9 && (env->uncached_cpsr & CPSR_PAN)) { - mmu_idx = (secure ? ARMMMUIdx_Stage1_SE1_PAN - : ARMMMUIdx_Stage1_E1_PAN); + mmu_idx = ARMMMUIdx_Stage1_E1_PAN; } else { - mmu_idx = secure ? ARMMMUIdx_Stage1_SE1 : ARMMMUIdx_Stage1_E1; + mmu_idx = ARMMMUIdx_Stage1_E1; } break; default: @@ -XXX,XX +XXX,XX @@ static void ats_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value) /* stage 1 current state PL0: ATS1CUR, ATS1CUW */ switch (el) { case 3: - mmu_idx = ARMMMUIdx_SE10_0; + mmu_idx = ARMMMUIdx_E10_0; secure = true; break; case 2: @@ -XXX,XX +XXX,XX @@ static void ats_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value) mmu_idx = ARMMMUIdx_Stage1_E0; break; case 1: - mmu_idx = secure ? ARMMMUIdx_Stage1_SE0 : ARMMMUIdx_Stage1_E0; + mmu_idx = ARMMMUIdx_Stage1_E0; break; default: g_assert_not_reached(); @@ -XXX,XX +XXX,XX @@ static void ats_write64(CPUARMState *env, const ARMCPRegInfo *ri, switch (ri->opc1) { case 0: /* AT S1E1R, AT S1E1W, AT S1E1RP, AT S1E1WP */ if (ri->crm == 9 && (env->pstate & PSTATE_PAN)) { - mmu_idx = (secure ? ARMMMUIdx_Stage1_SE1_PAN - : ARMMMUIdx_Stage1_E1_PAN); + mmu_idx = ARMMMUIdx_Stage1_E1_PAN; } else { - mmu_idx = secure ? ARMMMUIdx_Stage1_SE1 : ARMMMUIdx_Stage1_E1; + mmu_idx = ARMMMUIdx_Stage1_E1; } break; case 4: /* AT S1E2R, AT S1E2W */ - mmu_idx = secure ? ARMMMUIdx_SE2 : ARMMMUIdx_E2; + mmu_idx = ARMMMUIdx_E2; break; case 6: /* AT S1E3R, AT S1E3W */ - mmu_idx = ARMMMUIdx_SE3; + mmu_idx = ARMMMUIdx_E3; secure = true; break; default: @@ -XXX,XX +XXX,XX @@ static void ats_write64(CPUARMState *env, const ARMCPRegInfo *ri, } break; case 2: /* AT S1E0R, AT S1E0W */ - mmu_idx = secure ? ARMMMUIdx_Stage1_SE0 : ARMMMUIdx_Stage1_E0; + mmu_idx = ARMMMUIdx_Stage1_E0; break; case 4: /* AT S12E1R, AT S12E1W */ - mmu_idx = secure ? ARMMMUIdx_SE10_1 : ARMMMUIdx_E10_1; + mmu_idx = ARMMMUIdx_E10_1; break; case 6: /* AT S12E0R, AT S12E0W */ - mmu_idx = secure ? ARMMMUIdx_SE10_0 : ARMMMUIdx_E10_0; + mmu_idx = ARMMMUIdx_E10_0; break; default: g_assert_not_reached(); @@ -XXX,XX +XXX,XX @@ static void vmsa_tcr_ttbr_el2_write(CPUARMState *env, const ARMCPRegInfo *ri, uint16_t mask = ARMMMUIdxBit_E20_2 | ARMMMUIdxBit_E20_2_PAN | ARMMMUIdxBit_E20_0; - - if (arm_is_secure_below_el3(env)) { - mask >>= ARM_MMU_IDX_A_NS; - } - tlb_flush_by_mmuidx(env_cpu(env), mask); } raw_write(env, ri, value); @@ -XXX,XX +XXX,XX @@ static void vttbr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint16_t mask = ARMMMUIdxBit_E10_1 | ARMMMUIdxBit_E10_1_PAN | ARMMMUIdxBit_E10_0; - - if (arm_is_secure_below_el3(env)) { - mask >>= ARM_MMU_IDX_A_NS; - } - tlb_flush_by_mmuidx(cs, mask); raw_write(env, ri, value); } @@ -XXX,XX +XXX,XX @@ static int vae1_tlbmask(CPUARMState *env) ARMMMUIdxBit_E10_1_PAN | ARMMMUIdxBit_E10_0; } - - if (arm_is_secure_below_el3(env)) { - mask >>= ARM_MMU_IDX_A_NS; - } - return mask; } @@ -XXX,XX +XXX,XX @@ static int vae1_tlbbits(CPUARMState *env, uint64_t addr) mmu_idx = ARMMMUIdx_E10_0; } - if (arm_is_secure_below_el3(env)) { - mmu_idx &= ~ARM_MMU_IDX_A_NS; - } - return tlbbits_for_regime(env, mmu_idx, addr); } @@ -XXX,XX +XXX,XX @@ static int alle1_tlbmask(CPUARMState *env) * stage 2 translations, whereas most other scopes only invalidate * stage 1 translations. */ - if (arm_is_secure_below_el3(env)) { - return ARMMMUIdxBit_SE10_1 | - ARMMMUIdxBit_SE10_1_PAN | - ARMMMUIdxBit_SE10_0; - } else { - return ARMMMUIdxBit_E10_1 | - ARMMMUIdxBit_E10_1_PAN | - ARMMMUIdxBit_E10_0; - } + return (ARMMMUIdxBit_E10_1 | + ARMMMUIdxBit_E10_1_PAN | + ARMMMUIdxBit_E10_0); } static int e2_tlbmask(CPUARMState *env) { - if (arm_is_secure_below_el3(env)) { - return ARMMMUIdxBit_SE20_0 | - ARMMMUIdxBit_SE20_2 | - ARMMMUIdxBit_SE20_2_PAN | - ARMMMUIdxBit_SE2; - } else { - return ARMMMUIdxBit_E20_0 | - ARMMMUIdxBit_E20_2 | - ARMMMUIdxBit_E20_2_PAN | - ARMMMUIdxBit_E2; - } + return (ARMMMUIdxBit_E20_0 | + ARMMMUIdxBit_E20_2 | + ARMMMUIdxBit_E20_2_PAN | + ARMMMUIdxBit_E2); } static void tlbi_aa64_alle1_write(CPUARMState *env, const ARMCPRegInfo *ri, @@ -XXX,XX +XXX,XX @@ static void tlbi_aa64_alle3_write(CPUARMState *env, const ARMCPRegInfo *ri, ARMCPU *cpu = env_archcpu(env); CPUState *cs = CPU(cpu); - tlb_flush_by_mmuidx(cs, ARMMMUIdxBit_SE3); + tlb_flush_by_mmuidx(cs, ARMMMUIdxBit_E3); } static void tlbi_aa64_alle1is_write(CPUARMState *env, const ARMCPRegInfo *ri, @@ -XXX,XX +XXX,XX @@ static void tlbi_aa64_alle3is_write(CPUARMState *env, const ARMCPRegInfo *ri, { CPUState *cs = env_cpu(env); - tlb_flush_by_mmuidx_all_cpus_synced(cs, ARMMMUIdxBit_SE3); + tlb_flush_by_mmuidx_all_cpus_synced(cs, ARMMMUIdxBit_E3); } static void tlbi_aa64_vae2_write(CPUARMState *env, const ARMCPRegInfo *ri, @@ -XXX,XX +XXX,XX @@ static void tlbi_aa64_vae3_write(CPUARMState *env, const ARMCPRegInfo *ri, CPUState *cs = CPU(cpu); uint64_t pageaddr = sextract64(value << 12, 0, 56); - tlb_flush_page_by_mmuidx(cs, pageaddr, ARMMMUIdxBit_SE3); + tlb_flush_page_by_mmuidx(cs, pageaddr, ARMMMUIdxBit_E3); } static void tlbi_aa64_vae1is_write(CPUARMState *env, const ARMCPRegInfo *ri, @@ -XXX,XX +XXX,XX @@ static void tlbi_aa64_vae2is_write(CPUARMState *env, const ARMCPRegInfo *ri, { CPUState *cs = env_cpu(env); uint64_t pageaddr = sextract64(value << 12, 0, 56); - bool secure = arm_is_secure_below_el3(env); - int mask = secure ? ARMMMUIdxBit_SE2 : ARMMMUIdxBit_E2; - int bits = tlbbits_for_regime(env, secure ? ARMMMUIdx_SE2 : ARMMMUIdx_E2, - pageaddr); + int bits = tlbbits_for_regime(env, ARMMMUIdx_E2, pageaddr); - tlb_flush_page_bits_by_mmuidx_all_cpus_synced(cs, pageaddr, mask, bits); + tlb_flush_page_bits_by_mmuidx_all_cpus_synced(cs, pageaddr, + ARMMMUIdxBit_E2, bits); } static void tlbi_aa64_vae3is_write(CPUARMState *env, const ARMCPRegInfo *ri, @@ -XXX,XX +XXX,XX @@ static void tlbi_aa64_vae3is_write(CPUARMState *env, const ARMCPRegInfo *ri, { CPUState *cs = env_cpu(env); uint64_t pageaddr = sextract64(value << 12, 0, 56); - int bits = tlbbits_for_regime(env, ARMMMUIdx_SE3, pageaddr); + int bits = tlbbits_for_regime(env, ARMMMUIdx_E3, pageaddr); tlb_flush_page_bits_by_mmuidx_all_cpus_synced(cs, pageaddr, - ARMMMUIdxBit_SE3, bits); + ARMMMUIdxBit_E3, bits); } #ifdef TARGET_AARCH64 @@ -XXX,XX +XXX,XX @@ static void tlbi_aa64_rvae1is_write(CPUARMState *env, static int vae2_tlbmask(CPUARMState *env) { - return (arm_is_secure_below_el3(env) - ? ARMMMUIdxBit_SE2 : ARMMMUIdxBit_E2); + return ARMMMUIdxBit_E2; } static void tlbi_aa64_rvae2_write(CPUARMState *env, @@ -XXX,XX +XXX,XX @@ static void tlbi_aa64_rvae3_write(CPUARMState *env, * flush-last-level-only. */ - do_rvae_write(env, value, ARMMMUIdxBit_SE3, - tlb_force_broadcast(env)); + do_rvae_write(env, value, ARMMMUIdxBit_E3, tlb_force_broadcast(env)); } static void tlbi_aa64_rvae3is_write(CPUARMState *env, @@ -XXX,XX +XXX,XX @@ static void tlbi_aa64_rvae3is_write(CPUARMState *env, * flush-last-level-only or inner/outer specific flushes. */ - do_rvae_write(env, value, ARMMMUIdxBit_SE3, true); + do_rvae_write(env, value, ARMMMUIdxBit_E3, true); } #endif @@ -XXX,XX +XXX,XX @@ uint64_t arm_sctlr(CPUARMState *env, int el) /* Only EL0 needs to be adjusted for EL1&0 or EL2&0. */ if (el == 0) { ARMMMUIdx mmu_idx = arm_mmu_idx_el(env, 0); - el = (mmu_idx == ARMMMUIdx_E20_0 || mmu_idx == ARMMMUIdx_SE20_0) - ? 2 : 1; + el = mmu_idx == ARMMMUIdx_E20_0 ? 2 : 1; } return env->cp15.sctlr_el[el]; } @@ -XXX,XX +XXX,XX @@ int arm_mmu_idx_to_el(ARMMMUIdx mmu_idx) switch (mmu_idx) { case ARMMMUIdx_E10_0: case ARMMMUIdx_E20_0: - case ARMMMUIdx_SE10_0: - case ARMMMUIdx_SE20_0: return 0; case ARMMMUIdx_E10_1: case ARMMMUIdx_E10_1_PAN: - case ARMMMUIdx_SE10_1: - case ARMMMUIdx_SE10_1_PAN: return 1; case ARMMMUIdx_E2: case ARMMMUIdx_E20_2: case ARMMMUIdx_E20_2_PAN: - case ARMMMUIdx_SE2: - case ARMMMUIdx_SE20_2: - case ARMMMUIdx_SE20_2_PAN: return 2; - case ARMMMUIdx_SE3: + case ARMMMUIdx_E3: return 3; default: g_assert_not_reached(); @@ -XXX,XX +XXX,XX @@ ARMMMUIdx arm_mmu_idx_el(CPUARMState *env, int el) } break; case 3: - return ARMMMUIdx_SE3; + return ARMMMUIdx_E3; default: g_assert_not_reached(); } - if (arm_is_secure_below_el3(env)) { - idx &= ~ARM_MMU_IDX_A_NS; - } - return idx; } @@ -XXX,XX +XXX,XX @@ static CPUARMTBFlags rebuild_hflags_a64(CPUARMState *env, int el, int fp_el, switch (mmu_idx) { case ARMMMUIdx_E10_1: case ARMMMUIdx_E10_1_PAN: - case ARMMMUIdx_SE10_1: - case ARMMMUIdx_SE10_1_PAN: /* TODO: ARMv8.3-NV */ DP_TBFLAG_A64(flags, UNPRIV, 1); break; case ARMMMUIdx_E20_2: case ARMMMUIdx_E20_2_PAN: - case ARMMMUIdx_SE20_2: - case ARMMMUIdx_SE20_2_PAN: /* * Note that EL20_2 is gated by HCR_EL2.E2H == 1, but EL20_0 is * gated by HCR_EL2.<E2H,TGE> == '11', and so is LDTR. 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 @@ unsigned int arm_pamax(ARMCPU *cpu) ARMMMUIdx stage_1_mmu_idx(ARMMMUIdx mmu_idx) { switch (mmu_idx) { - case ARMMMUIdx_SE10_0: - return ARMMMUIdx_Stage1_SE0; - case ARMMMUIdx_SE10_1: - return ARMMMUIdx_Stage1_SE1; - case ARMMMUIdx_SE10_1_PAN: - return ARMMMUIdx_Stage1_SE1_PAN; case ARMMMUIdx_E10_0: return ARMMMUIdx_Stage1_E0; case ARMMMUIdx_E10_1: @@ -XXX,XX +XXX,XX @@ static bool regime_translation_big_endian(CPUARMState *env, ARMMMUIdx mmu_idx) static bool regime_is_user(CPUARMState *env, ARMMMUIdx mmu_idx) { switch (mmu_idx) { - case ARMMMUIdx_SE10_0: case ARMMMUIdx_E20_0: - case ARMMMUIdx_SE20_0: case ARMMMUIdx_Stage1_E0: - case ARMMMUIdx_Stage1_SE0: case ARMMMUIdx_MUser: case ARMMMUIdx_MSUser: case ARMMMUIdx_MUserNegPri: @@ -XXX,XX +XXX,XX @@ bool get_phys_addr_with_secure(CPUARMState *env, target_ulong address, s2_mmu_idx = (s2walk_secure ? ARMMMUIdx_Stage2_S : ARMMMUIdx_Stage2); - is_el0 = mmu_idx == ARMMMUIdx_E10_0 || mmu_idx == ARMMMUIdx_SE10_0; + is_el0 = mmu_idx == ARMMMUIdx_E10_0; /* * S1 is done, now do S2 translation. @@ -XXX,XX +XXX,XX @@ bool get_phys_addr(CPUARMState *env, target_ulong address, case ARMMMUIdx_Stage1_E1: case ARMMMUIdx_Stage1_E1_PAN: case ARMMMUIdx_E2: + is_secure = arm_is_secure_below_el3(env); + break; case ARMMMUIdx_Stage2: case ARMMMUIdx_MPrivNegPri: case ARMMMUIdx_MUserNegPri: @@ -XXX,XX +XXX,XX @@ bool get_phys_addr(CPUARMState *env, target_ulong address, case ARMMMUIdx_MUser: is_secure = false; break; - case ARMMMUIdx_SE3: - case ARMMMUIdx_SE10_0: - case ARMMMUIdx_SE10_1: - case ARMMMUIdx_SE10_1_PAN: - case ARMMMUIdx_SE20_0: - case ARMMMUIdx_SE20_2: - case ARMMMUIdx_SE20_2_PAN: - case ARMMMUIdx_Stage1_SE0: - case ARMMMUIdx_Stage1_SE1: - case ARMMMUIdx_Stage1_SE1_PAN: - case ARMMMUIdx_SE2: + case ARMMMUIdx_E3: case ARMMMUIdx_Stage2_S: case ARMMMUIdx_MSPrivNegPri: case ARMMMUIdx_MSUserNegPri: 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 int get_a64_user_mem_index(DisasContext *s) case ARMMMUIdx_E20_2_PAN: useridx = ARMMMUIdx_E20_0; break; - case ARMMMUIdx_SE10_1: - case ARMMMUIdx_SE10_1_PAN: - useridx = ARMMMUIdx_SE10_0; - break; - case ARMMMUIdx_SE20_2: - case ARMMMUIdx_SE20_2_PAN: - useridx = ARMMMUIdx_SE20_0; - break; default: g_assert_not_reached(); } 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) * otherwise, access as if at PL0. */ switch (s->mmu_idx) { + case ARMMMUIdx_E3: case ARMMMUIdx_E2: /* this one is UNPREDICTABLE */ case ARMMMUIdx_E10_0: case ARMMMUIdx_E10_1: case ARMMMUIdx_E10_1_PAN: return arm_to_core_mmu_idx(ARMMMUIdx_E10_0); - case ARMMMUIdx_SE3: - case ARMMMUIdx_SE10_0: - case ARMMMUIdx_SE10_1: - case ARMMMUIdx_SE10_1_PAN: - return arm_to_core_mmu_idx(ARMMMUIdx_SE10_0); case ARMMMUIdx_MUser: case ARMMMUIdx_MPriv: return arm_to_core_mmu_idx(ARMMMUIdx_MUser); -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Use a switch on mmu_idx for the a-profile indexes, instead of three different if's vs regime_el and arm_mmu_idx_is_stage1_of_2. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20221001162318.153420-12-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/ptw.c | 32 +++++++++++++++++++++++++------- 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/target/arm/ptw.c b/target/arm/ptw.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/ptw.c +++ b/target/arm/ptw.c @@ -XXX,XX +XXX,XX @@ static bool regime_translation_disabled(CPUARMState *env, ARMMMUIdx mmu_idx, hcr_el2 = arm_hcr_el2_eff(env); - if (mmu_idx == ARMMMUIdx_Stage2 || mmu_idx == ARMMMUIdx_Stage2_S) { + switch (mmu_idx) { + case ARMMMUIdx_Stage2: + case ARMMMUIdx_Stage2_S: /* HCR.DC means HCR.VM behaves as 1 */ return (hcr_el2 & (HCR_DC | HCR_VM)) == 0; - } - if (hcr_el2 & HCR_TGE) { + case ARMMMUIdx_E10_0: + case ARMMMUIdx_E10_1: + case ARMMMUIdx_E10_1_PAN: /* TGE means that NS EL0/1 act as if SCTLR_EL1.M is zero */ - if (!is_secure && regime_el(env, mmu_idx) == 1) { + if (!is_secure && (hcr_el2 & HCR_TGE)) { return true; } - } + break; - if ((hcr_el2 & HCR_DC) && arm_mmu_idx_is_stage1_of_2(mmu_idx)) { + case ARMMMUIdx_Stage1_E0: + case ARMMMUIdx_Stage1_E1: + case ARMMMUIdx_Stage1_E1_PAN: /* HCR.DC means SCTLR_EL1.M behaves as 0 */ - return true; + if (hcr_el2 & HCR_DC) { + return true; + } + break; + + case ARMMMUIdx_E20_0: + case ARMMMUIdx_E20_2: + case ARMMMUIdx_E20_2_PAN: + case ARMMMUIdx_E2: + case ARMMMUIdx_E3: + break; + + default: + g_assert_not_reached(); } return (regime_sctlr(env, mmu_idx) & SCTLR_M) == 0; -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> The effect of TGE does not only apply to non-secure state, now that Secure EL2 exists. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20221001162318.153420-13-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/ptw.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/target/arm/ptw.c b/target/arm/ptw.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/ptw.c +++ b/target/arm/ptw.c @@ -XXX,XX +XXX,XX @@ static bool regime_translation_disabled(CPUARMState *env, ARMMMUIdx mmu_idx, case ARMMMUIdx_E10_0: case ARMMMUIdx_E10_1: case ARMMMUIdx_E10_1_PAN: - /* TGE means that NS EL0/1 act as if SCTLR_EL1.M is zero */ - if (!is_secure && (hcr_el2 & HCR_TGE)) { + /* TGE means that EL0/1 act as if SCTLR_EL1.M is zero */ + if (hcr_el2 & HCR_TGE) { return true; } break; -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> For page walking, we may require HCR for a security state that is not "current". Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20221001162318.153420-14-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/cpu.h | 20 +++++++++++++------- target/arm/helper.c | 11 ++++++++--- 2 files changed, 21 insertions(+), 10 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 bool arm_is_secure(CPUARMState *env) * Return true if the current security state has AArch64 EL2 or AArch32 Hyp. * This corresponds to the pseudocode EL2Enabled() */ +static inline bool arm_is_el2_enabled_secstate(CPUARMState *env, bool secure) +{ + return arm_feature(env, ARM_FEATURE_EL2) + && (!secure || (env->cp15.scr_el3 & SCR_EEL2)); +} + static inline bool arm_is_el2_enabled(CPUARMState *env) { - if (arm_feature(env, ARM_FEATURE_EL2)) { - if (arm_is_secure_below_el3(env)) { - return (env->cp15.scr_el3 & SCR_EEL2) != 0; - } - return true; - } - return false; + return arm_is_el2_enabled_secstate(env, arm_is_secure_below_el3(env)); } #else @@ -XXX,XX +XXX,XX @@ static inline bool arm_is_secure(CPUARMState *env) return false; } +static inline bool arm_is_el2_enabled_secstate(CPUARMState *env, bool secure) +{ + return false; +} + static inline bool arm_is_el2_enabled(CPUARMState *env) { return false; @@ -XXX,XX +XXX,XX @@ static inline bool arm_is_el2_enabled(CPUARMState *env) * "for all purposes other than a direct read or write access of HCR_EL2." * Not included here is HCR_RW. */ +uint64_t arm_hcr_el2_eff_secstate(CPUARMState *env, bool secure); uint64_t arm_hcr_el2_eff(CPUARMState *env); uint64_t arm_hcrx_el2_eff(CPUARMState *env); 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 hcr_writelow(CPUARMState *env, const ARMCPRegInfo *ri, } /* - * Return the effective value of HCR_EL2. + * Return the effective value of HCR_EL2, at the given security state. * Bits that are not included here: * RW (read from SCR_EL3.RW as needed) */ -uint64_t arm_hcr_el2_eff(CPUARMState *env) +uint64_t arm_hcr_el2_eff_secstate(CPUARMState *env, bool secure) { uint64_t ret = env->cp15.hcr_el2; - if (!arm_is_el2_enabled(env)) { + if (!arm_is_el2_enabled_secstate(env, secure)) { /* * "This register has no effect if EL2 is not enabled in the * current Security state". This is ARMv8.4-SecEL2 speak for @@ -XXX,XX +XXX,XX @@ uint64_t arm_hcr_el2_eff(CPUARMState *env) return ret; } +uint64_t arm_hcr_el2_eff(CPUARMState *env) +{ + return arm_hcr_el2_eff_secstate(env, arm_is_secure_below_el3(env)); +} + /* * Corresponds to ARM pseudocode function ELIsInHost(). */ -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Rename the argument to is_secure_ptr, and introduce a local variable is_secure with the value. We only write back to the pointer toward the end of the function. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20221001162318.153420-15-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/ptw.c | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 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 ptw_attrs_are_device(CPUARMState *env, 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, + hwaddr addr, bool *is_secure_ptr, ARMMMUFaultInfo *fi) { - ARMMMUIdx s2_mmu_idx = *is_secure ? ARMMMUIdx_Stage2_S : ARMMMUIdx_Stage2; + bool is_secure = *is_secure_ptr; + ARMMMUIdx s2_mmu_idx = is_secure ? ARMMMUIdx_Stage2_S : ARMMMUIdx_Stage2; if (arm_mmu_idx_is_stage1_of_2(mmu_idx) && - !regime_translation_disabled(env, s2_mmu_idx, *is_secure)) { + !regime_translation_disabled(env, s2_mmu_idx, is_secure)) { GetPhysAddrResult s2 = {}; int ret; ret = get_phys_addr_lpae(env, addr, MMU_DATA_LOAD, s2_mmu_idx, - *is_secure, false, &s2, fi); + is_secure, false, &s2, fi); if (ret) { assert(fi->type != ARMFault_None); fi->s2addr = addr; fi->stage2 = true; fi->s1ptw = true; - fi->s1ns = !*is_secure; + fi->s1ns = !is_secure; return ~0; } if ((arm_hcr_el2_eff(env) & HCR_PTW) && @@ -XXX,XX +XXX,XX @@ static hwaddr S1_ptw_translate(CPUARMState *env, ARMMMUIdx mmu_idx, fi->s2addr = addr; fi->stage2 = true; fi->s1ptw = true; - fi->s1ns = !*is_secure; + fi->s1ns = !is_secure; return ~0; } 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); + if (is_secure) { + is_secure = !(env->cp15.vstcr_el2 & VSTCR_SW); } else { - *is_secure = !(env->cp15.vtcr_el2 & VTCR_NSW); + is_secure = !(env->cp15.vtcr_el2 & VTCR_NSW); } + *is_secure_ptr = is_secure; } else { - assert(!*is_secure); + assert(!is_secure); } addr = s2.phys; -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> This value is unused. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 20221001162318.153420-16-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/ptw.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/target/arm/ptw.c b/target/arm/ptw.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/ptw.c +++ b/target/arm/ptw.c @@ -XXX,XX +XXX,XX @@ static uint8_t force_cacheattr_nibble_wb(uint8_t attr) * s1 and s2 for the HCR_EL2.FWB == 1 case, returning the * combined attributes in MAIR_EL1 format. */ -static uint8_t combined_attrs_fwb(CPUARMState *env, - ARMCacheAttrs s1, ARMCacheAttrs s2) +static uint8_t combined_attrs_fwb(ARMCacheAttrs s1, ARMCacheAttrs s2) { switch (s2.attrs) { case 7: @@ -XXX,XX +XXX,XX @@ static ARMCacheAttrs combine_cacheattrs(CPUARMState *env, /* Combine memory type and cacheability attributes */ if (arm_hcr_el2_eff(env) & HCR_FWB) { - ret.attrs = combined_attrs_fwb(env, s1, s2); + ret.attrs = combined_attrs_fwb(s1, s2); } else { ret.attrs = combined_attrs_nofwb(env, s1, s2); } -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> These subroutines did not need ENV for anything except retrieving the effective value of HCR anyway. We have computed the effective value of HCR in the callers, and this will be especially important for interpreting HCR in a non-current security state. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20221001162318.153420-17-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/ptw.c | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 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 regime_translation_disabled(CPUARMState *env, ARMMMUIdx mmu_idx, return (regime_sctlr(env, mmu_idx) & SCTLR_M) == 0; } -static bool ptw_attrs_are_device(CPUARMState *env, ARMCacheAttrs cacheattrs) +static bool ptw_attrs_are_device(uint64_t hcr, ARMCacheAttrs cacheattrs) { /* * For an S1 page table walk, the stage 1 attributes are always @@ -XXX,XX +XXX,XX @@ static bool ptw_attrs_are_device(CPUARMState *env, ARMCacheAttrs cacheattrs) * when cacheattrs.attrs bit [2] is 0. */ assert(cacheattrs.is_s2_format); - if (arm_hcr_el2_eff(env) & HCR_FWB) { + if (hcr & HCR_FWB) { return (cacheattrs.attrs & 0x4) == 0; } else { return (cacheattrs.attrs & 0xc) == 0; @@ -XXX,XX +XXX,XX @@ static hwaddr S1_ptw_translate(CPUARMState *env, ARMMMUIdx mmu_idx, if (arm_mmu_idx_is_stage1_of_2(mmu_idx) && !regime_translation_disabled(env, s2_mmu_idx, is_secure)) { GetPhysAddrResult s2 = {}; + uint64_t hcr; int ret; ret = get_phys_addr_lpae(env, addr, MMU_DATA_LOAD, s2_mmu_idx, @@ -XXX,XX +XXX,XX @@ static hwaddr S1_ptw_translate(CPUARMState *env, ARMMMUIdx mmu_idx, fi->s1ns = !is_secure; return ~0; } - if ((arm_hcr_el2_eff(env) & HCR_PTW) && - ptw_attrs_are_device(env, s2.cacheattrs)) { + + hcr = arm_hcr_el2_eff(env); + if ((hcr & HCR_PTW) && ptw_attrs_are_device(hcr, s2.cacheattrs)) { /* * PTW set and S1 walk touched S2 Device memory: * generate Permission fault. @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_pmsav8(CPUARMState *env, uint32_t address, * ref: shared/translation/attrs/S2AttrDecode() * .../S2ConvertAttrsHints() */ -static uint8_t convert_stage2_attrs(CPUARMState *env, uint8_t s2attrs) +static uint8_t convert_stage2_attrs(uint64_t hcr, uint8_t s2attrs) { uint8_t hiattr = extract32(s2attrs, 2, 2); uint8_t loattr = extract32(s2attrs, 0, 2); uint8_t hihint = 0, lohint = 0; if (hiattr != 0) { /* normal memory */ - if (arm_hcr_el2_eff(env) & HCR_CD) { /* cache disabled */ + if (hcr & HCR_CD) { /* cache disabled */ hiattr = loattr = 1; /* non-cacheable */ } else { if (hiattr != 1) { /* Write-through or write-back */ @@ -XXX,XX +XXX,XX @@ static uint8_t combine_cacheattr_nibble(uint8_t s1, uint8_t s2) * s1 and s2 for the HCR_EL2.FWB == 0 case, returning the * combined attributes in MAIR_EL1 format. */ -static uint8_t combined_attrs_nofwb(CPUARMState *env, +static uint8_t combined_attrs_nofwb(uint64_t hcr, ARMCacheAttrs s1, ARMCacheAttrs s2) { uint8_t s1lo, s2lo, s1hi, s2hi, s2_mair_attrs, ret_attrs; - s2_mair_attrs = convert_stage2_attrs(env, s2.attrs); + s2_mair_attrs = convert_stage2_attrs(hcr, s2.attrs); s1lo = extract32(s1.attrs, 0, 4); s2lo = extract32(s2_mair_attrs, 0, 4); @@ -XXX,XX +XXX,XX @@ static uint8_t combined_attrs_fwb(ARMCacheAttrs s1, ARMCacheAttrs s2) * @s1: Attributes from stage 1 walk * @s2: Attributes from stage 2 walk */ -static ARMCacheAttrs combine_cacheattrs(CPUARMState *env, +static ARMCacheAttrs combine_cacheattrs(uint64_t hcr, ARMCacheAttrs s1, ARMCacheAttrs s2) { ARMCacheAttrs ret; @@ -XXX,XX +XXX,XX @@ static ARMCacheAttrs combine_cacheattrs(CPUARMState *env, } /* Combine memory type and cacheability attributes */ - if (arm_hcr_el2_eff(env) & HCR_FWB) { + if (hcr & HCR_FWB) { ret.attrs = combined_attrs_fwb(s1, s2); } else { - ret.attrs = combined_attrs_nofwb(env, s1, s2); + ret.attrs = combined_attrs_nofwb(hcr, s1, s2); } /* @@ -XXX,XX +XXX,XX @@ bool get_phys_addr_with_secure(CPUARMState *env, target_ulong address, ARMCacheAttrs cacheattrs1; ARMMMUIdx s2_mmu_idx; bool is_el0; + uint64_t hcr; ret = get_phys_addr_with_secure(env, address, access_type, s1_mmu_idx, is_secure, result, fi); @@ -XXX,XX +XXX,XX @@ bool get_phys_addr_with_secure(CPUARMState *env, target_ulong address, } /* Combine the S1 and S2 cache attributes. */ - if (arm_hcr_el2_eff(env) & HCR_DC) { + hcr = arm_hcr_el2_eff(env); + if (hcr & HCR_DC) { /* * HCR.DC forces the first stage attributes to * Normal Non-Shareable, @@ -XXX,XX +XXX,XX @@ bool get_phys_addr_with_secure(CPUARMState *env, target_ulong address, } cacheattrs1.shareability = 0; } - result->cacheattrs = combine_cacheattrs(env, cacheattrs1, + result->cacheattrs = combine_cacheattrs(hcr, cacheattrs1, result->cacheattrs); /* -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Use arm_hcr_el2_eff_secstate instead of arm_hcr_el2_eff, so that we use is_secure instead of the current security state. These AT* operations have been broken since arm_hcr_el2_eff gained a check for "el2 enabled" for Secure EL2. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20221001162318.153420-18-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/ptw.c | 8 ++++---- 1 file changed, 4 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 regime_translation_disabled(CPUARMState *env, ARMMMUIdx mmu_idx, } } - hcr_el2 = arm_hcr_el2_eff(env); + hcr_el2 = arm_hcr_el2_eff_secstate(env, is_secure); switch (mmu_idx) { case ARMMMUIdx_Stage2: @@ -XXX,XX +XXX,XX @@ static hwaddr S1_ptw_translate(CPUARMState *env, ARMMMUIdx mmu_idx, return ~0; } - hcr = arm_hcr_el2_eff(env); + hcr = arm_hcr_el2_eff_secstate(env, is_secure); if ((hcr & HCR_PTW) && ptw_attrs_are_device(hcr, s2.cacheattrs)) { /* * PTW set and S1 walk touched S2 Device memory: @@ -XXX,XX +XXX,XX @@ bool get_phys_addr_with_secure(CPUARMState *env, target_ulong address, } /* Combine the S1 and S2 cache attributes. */ - hcr = arm_hcr_el2_eff(env); + hcr = arm_hcr_el2_eff_secstate(env, is_secure); if (hcr & HCR_DC) { /* * HCR.DC forces the first stage attributes to @@ -XXX,XX +XXX,XX @@ bool get_phys_addr_with_secure(CPUARMState *env, target_ulong address, result->page_size = TARGET_PAGE_SIZE; /* Fill in cacheattr a-la AArch64.TranslateAddressS1Off. */ - hcr = arm_hcr_el2_eff(env); + hcr = arm_hcr_el2_eff_secstate(env, is_secure); result->cacheattrs.shareability = 0; result->cacheattrs.is_s2_format = false; if (hcr & HCR_DC) { -- 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: 20221001162318.153420-19-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/ptw.c | 138 +++++++++++++++++++++++++---------------------- 1 file changed, 74 insertions(+), 64 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 ARMCacheAttrs combine_cacheattrs(uint64_t hcr, return ret; } +/* + * MMU disabled. S1 addresses within aa64 translation regimes are + * still checked for bounds -- see AArch64.S1DisabledOutput(). + */ +static bool get_phys_addr_disabled(CPUARMState *env, target_ulong address, + MMUAccessType access_type, + ARMMMUIdx mmu_idx, bool is_secure, + GetPhysAddrResult *result, + ARMMMUFaultInfo *fi) +{ + uint64_t hcr; + uint8_t memattr; + + if (mmu_idx != ARMMMUIdx_Stage2 && mmu_idx != ARMMMUIdx_Stage2_S) { + int 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]; + int addrtop, tbi; + + tbi = aa64_va_parameter_tbi(tcr, mmu_idx); + if (access_type == MMU_INST_FETCH) { + tbi &= ~aa64_va_parameter_tbid(tcr, mmu_idx); + } + tbi = (tbi >> extract64(address, 55, 1)) & 1; + addrtop = (tbi ? 55 : 63); + + if (extract64(address, pamax, addrtop - pamax + 1) != 0) { + fi->type = ARMFault_AddressSize; + fi->level = 0; + fi->stage2 = false; + return 1; + } + + /* + * When TBI is disabled, we've just validated that all of the + * bits above PAMax are zero, so logically we only need to + * clear the top byte for TBI. But it's clearer to follow + * the pseudocode set of addrdesc.paddress. + */ + address = extract64(address, 0, 52); + } + } + + result->phys = address; + result->prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC; + result->page_size = TARGET_PAGE_SIZE; + + /* Fill in cacheattr a-la AArch64.TranslateAddressS1Off. */ + hcr = arm_hcr_el2_eff_secstate(env, is_secure); + result->cacheattrs.shareability = 0; + result->cacheattrs.is_s2_format = false; + if (hcr & HCR_DC) { + if (hcr & HCR_DCT) { + memattr = 0xf0; /* Tagged, Normal, WB, RWA */ + } else { + memattr = 0xff; /* Normal, WB, RWA */ + } + } else if (access_type == MMU_INST_FETCH) { + if (regime_sctlr(env, mmu_idx) & SCTLR_I) { + memattr = 0xee; /* Normal, WT, RA, NT */ + } else { + memattr = 0x44; /* Normal, NC, No */ + } + result->cacheattrs.shareability = 2; /* outer sharable */ + } else { + memattr = 0x00; /* Device, nGnRnE */ + } + result->cacheattrs.attrs = memattr; + return 0; +} + bool get_phys_addr_with_secure(CPUARMState *env, target_ulong address, MMUAccessType access_type, ARMMMUIdx mmu_idx, bool is_secure, GetPhysAddrResult *result, @@ -XXX,XX +XXX,XX @@ bool get_phys_addr_with_secure(CPUARMState *env, target_ulong address, /* Definitely a real MMU, not an MPU */ if (regime_translation_disabled(env, mmu_idx, is_secure)) { - uint64_t hcr; - uint8_t memattr; - - /* - * MMU disabled. S1 addresses within aa64 translation regimes are - * still checked for bounds -- see AArch64.TranslateAddressS1Off. - */ - if (mmu_idx != ARMMMUIdx_Stage2 && mmu_idx != ARMMMUIdx_Stage2_S) { - int 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]; - int addrtop, tbi; - - tbi = aa64_va_parameter_tbi(tcr, mmu_idx); - if (access_type == MMU_INST_FETCH) { - tbi &= ~aa64_va_parameter_tbid(tcr, mmu_idx); - } - tbi = (tbi >> extract64(address, 55, 1)) & 1; - addrtop = (tbi ? 55 : 63); - - if (extract64(address, pamax, addrtop - pamax + 1) != 0) { - fi->type = ARMFault_AddressSize; - fi->level = 0; - fi->stage2 = false; - return 1; - } - - /* - * When TBI is disabled, we've just validated that all of the - * bits above PAMax are zero, so logically we only need to - * clear the top byte for TBI. But it's clearer to follow - * the pseudocode set of addrdesc.paddress. - */ - address = extract64(address, 0, 52); - } - } - result->phys = address; - result->prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC; - result->page_size = TARGET_PAGE_SIZE; - - /* Fill in cacheattr a-la AArch64.TranslateAddressS1Off. */ - hcr = arm_hcr_el2_eff_secstate(env, is_secure); - result->cacheattrs.shareability = 0; - result->cacheattrs.is_s2_format = false; - if (hcr & HCR_DC) { - if (hcr & HCR_DCT) { - memattr = 0xf0; /* Tagged, Normal, WB, RWA */ - } else { - memattr = 0xff; /* Normal, WB, RWA */ - } - } else if (access_type == MMU_INST_FETCH) { - if (regime_sctlr(env, mmu_idx) & SCTLR_I) { - memattr = 0xee; /* Normal, WT, RA, NT */ - } else { - memattr = 0x44; /* Normal, NC, No */ - } - result->cacheattrs.shareability = 2; /* outer sharable */ - } else { - memattr = 0x00; /* Device, nGnRnE */ - } - result->cacheattrs.attrs = memattr; - return 0; + return get_phys_addr_disabled(env, address, access_type, mmu_idx, + is_secure, result, fi); } - if (regime_using_lpae_format(env, mmu_idx)) { return get_phys_addr_lpae(env, address, access_type, mmu_idx, is_secure, false, result, fi); -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Do not apply memattr or shareability for Stage2 translations. Make sure to apply HCR_{DC,DCT} only to Regime_EL10, per the pseudocode in AArch64.S1DisabledOutput. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 20221001162318.153420-20-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/ptw.c | 48 +++++++++++++++++++++++++----------------------- 1 file changed, 25 insertions(+), 23 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, GetPhysAddrResult *result, ARMMMUFaultInfo *fi) { - uint64_t hcr; - uint8_t memattr; + uint8_t memattr = 0x00; /* Device nGnRnE */ + uint8_t shareability = 0; /* non-sharable */ if (mmu_idx != ARMMMUIdx_Stage2 && mmu_idx != ARMMMUIdx_Stage2_S) { int 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, */ address = extract64(address, 0, 52); } + + /* Fill in cacheattr a-la AArch64.TranslateAddressS1Off. */ + if (r_el == 1) { + uint64_t hcr = arm_hcr_el2_eff_secstate(env, is_secure); + if (hcr & HCR_DC) { + if (hcr & HCR_DCT) { + memattr = 0xf0; /* Tagged, Normal, WB, RWA */ + } else { + memattr = 0xff; /* Normal, WB, RWA */ + } + } + } + if (memattr == 0 && access_type == MMU_INST_FETCH) { + if (regime_sctlr(env, mmu_idx) & SCTLR_I) { + memattr = 0xee; /* Normal, WT, RA, NT */ + } else { + memattr = 0x44; /* Normal, NC, No */ + } + shareability = 2; /* outer sharable */ + } + result->cacheattrs.is_s2_format = false; } result->phys = address; result->prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC; result->page_size = TARGET_PAGE_SIZE; - - /* Fill in cacheattr a-la AArch64.TranslateAddressS1Off. */ - hcr = arm_hcr_el2_eff_secstate(env, is_secure); - result->cacheattrs.shareability = 0; - result->cacheattrs.is_s2_format = false; - if (hcr & HCR_DC) { - if (hcr & HCR_DCT) { - memattr = 0xf0; /* Tagged, Normal, WB, RWA */ - } else { - memattr = 0xff; /* Normal, WB, RWA */ - } - } else if (access_type == MMU_INST_FETCH) { - if (regime_sctlr(env, mmu_idx) & SCTLR_I) { - memattr = 0xee; /* Normal, WT, RA, NT */ - } else { - memattr = 0x44; /* Normal, NC, No */ - } - result->cacheattrs.shareability = 2; /* outer sharable */ - } else { - memattr = 0x00; /* Device, nGnRnE */ - } + result->cacheattrs.shareability = shareability; result->cacheattrs.attrs = memattr; return 0; } -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Adjust GetPhysAddrResult to fill in CPUTLBEntryFull, so that it may be passed directly to tlb_set_page_full. The change is large, but mostly mechanical. The major non-mechanical change is page_size -> lg_page_size. Most of the time this is obvious, and is related to TARGET_PAGE_BITS. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 20221001162318.153420-21-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/internals.h | 5 +- target/arm/helper.c | 12 +-- target/arm/m_helper.c | 20 ++--- target/arm/ptw.c | 179 ++++++++++++++++++++-------------------- target/arm/tlb_helper.c | 9 +- 5 files changed, 111 insertions(+), 114 deletions(-) diff --git a/target/arm/internals.h b/target/arm/internals.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/internals.h +++ b/target/arm/internals.h @@ -XXX,XX +XXX,XX @@ typedef struct ARMCacheAttrs { /* Fields that are valid upon success. */ typedef struct GetPhysAddrResult { - hwaddr phys; - target_ulong page_size; - int prot; - MemTxAttrs attrs; + CPUTLBEntryFull f; ARMCacheAttrs cacheattrs; } GetPhysAddrResult; 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 uint64_t do_ats_write(CPUARMState *env, uint64_t value, /* Create a 64-bit PAR */ par64 = (1 << 11); /* LPAE bit always set */ if (!ret) { - par64 |= res.phys & ~0xfffULL; - if (!res.attrs.secure) { + par64 |= res.f.phys_addr & ~0xfffULL; + if (!res.f.attrs.secure) { par64 |= (1 << 9); /* NS */ } par64 |= (uint64_t)res.cacheattrs.attrs << 56; /* ATTR */ @@ -XXX,XX +XXX,XX @@ static uint64_t do_ats_write(CPUARMState *env, uint64_t value, */ if (!ret) { /* We do not set any attribute bits in the PAR */ - if (res.page_size == (1 << 24) + if (res.f.lg_page_size == 24 && arm_feature(env, ARM_FEATURE_V7)) { - par64 = (res.phys & 0xff000000) | (1 << 1); + par64 = (res.f.phys_addr & 0xff000000) | (1 << 1); } else { - par64 = res.phys & 0xfffff000; + par64 = res.f.phys_addr & 0xfffff000; } - if (!res.attrs.secure) { + if (!res.f.attrs.secure) { par64 |= (1 << 9); /* NS */ } } else { diff --git a/target/arm/m_helper.c b/target/arm/m_helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/m_helper.c +++ b/target/arm/m_helper.c @@ -XXX,XX +XXX,XX @@ static bool v7m_stack_write(ARMCPU *cpu, uint32_t addr, uint32_t value, } goto pend_fault; } - address_space_stl_le(arm_addressspace(cs, res.attrs), res.phys, value, - res.attrs, &txres); + address_space_stl_le(arm_addressspace(cs, res.f.attrs), res.f.phys_addr, + value, res.f.attrs, &txres); if (txres != MEMTX_OK) { /* BusFault trying to write the data */ if (mode == STACK_LAZYFP) { @@ -XXX,XX +XXX,XX @@ static bool v7m_stack_read(ARMCPU *cpu, uint32_t *dest, uint32_t addr, goto pend_fault; } - value = address_space_ldl(arm_addressspace(cs, res.attrs), res.phys, - res.attrs, &txres); + value = address_space_ldl(arm_addressspace(cs, res.f.attrs), + res.f.phys_addr, res.f.attrs, &txres); if (txres != MEMTX_OK) { /* BusFault trying to read the data */ qemu_log_mask(CPU_LOG_INT, "...BusFault with BFSR.UNSTKERR\n"); @@ -XXX,XX +XXX,XX @@ static bool v7m_read_half_insn(ARMCPU *cpu, ARMMMUIdx mmu_idx, bool secure, qemu_log_mask(CPU_LOG_INT, "...really MemManage with CFSR.IACCVIOL\n"); return false; } - *insn = address_space_lduw_le(arm_addressspace(cs, res.attrs), res.phys, - res.attrs, &txres); + *insn = address_space_lduw_le(arm_addressspace(cs, res.f.attrs), + res.f.phys_addr, res.f.attrs, &txres); if (txres != MEMTX_OK) { env->v7m.cfsr[M_REG_NS] |= R_V7M_CFSR_IBUSERR_MASK; armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_BUS, false); @@ -XXX,XX +XXX,XX @@ static bool v7m_read_sg_stack_word(ARMCPU *cpu, ARMMMUIdx mmu_idx, } return false; } - value = address_space_ldl(arm_addressspace(cs, res.attrs), res.phys, - res.attrs, &txres); + value = address_space_ldl(arm_addressspace(cs, res.f.attrs), + res.f.phys_addr, res.f.attrs, &txres); if (txres != MEMTX_OK) { /* BusFault trying to read the data */ qemu_log_mask(CPU_LOG_INT, @@ -XXX,XX +XXX,XX @@ uint32_t HELPER(v7m_tt)(CPUARMState *env, uint32_t addr, uint32_t op) } else { mrvalid = true; } - r = res.prot & PAGE_READ; - rw = res.prot & PAGE_WRITE; + r = res.f.prot & PAGE_READ; + rw = res.f.prot & PAGE_WRITE; } else { r = false; rw = false; diff --git a/target/arm/ptw.c b/target/arm/ptw.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/ptw.c +++ b/target/arm/ptw.c @@ -XXX,XX +XXX,XX @@ static hwaddr S1_ptw_translate(CPUARMState *env, ARMMMUIdx mmu_idx, assert(!is_secure); } - addr = s2.phys; + addr = s2.f.phys_addr; } return addr; } @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_v5(CPUARMState *env, uint32_t address, /* 1Mb section. */ phys_addr = (desc & 0xfff00000) | (address & 0x000fffff); ap = (desc >> 10) & 3; - result->page_size = 1024 * 1024; + result->f.lg_page_size = 20; /* 1MB */ } else { /* Lookup l2 entry. */ if (type == 1) { @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_v5(CPUARMState *env, uint32_t address, case 1: /* 64k page. */ phys_addr = (desc & 0xffff0000) | (address & 0xffff); ap = (desc >> (4 + ((address >> 13) & 6))) & 3; - result->page_size = 0x10000; + result->f.lg_page_size = 16; break; case 2: /* 4k page. */ phys_addr = (desc & 0xfffff000) | (address & 0xfff); ap = (desc >> (4 + ((address >> 9) & 6))) & 3; - result->page_size = 0x1000; + result->f.lg_page_size = 12; break; case 3: /* 1k page, or ARMv6/XScale "extended small (4k) page" */ if (type == 1) { @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_v5(CPUARMState *env, uint32_t address, if (arm_feature(env, ARM_FEATURE_XSCALE) || arm_feature(env, ARM_FEATURE_V6)) { phys_addr = (desc & 0xfffff000) | (address & 0xfff); - result->page_size = 0x1000; + result->f.lg_page_size = 12; } else { /* * UNPREDICTABLE in ARMv5; we choose to take a @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_v5(CPUARMState *env, uint32_t address, } } else { phys_addr = (desc & 0xfffffc00) | (address & 0x3ff); - result->page_size = 0x400; + result->f.lg_page_size = 10; } ap = (desc >> 4) & 3; break; @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_v5(CPUARMState *env, uint32_t address, g_assert_not_reached(); } } - result->prot = ap_to_rw_prot(env, mmu_idx, ap, domain_prot); - result->prot |= result->prot ? PAGE_EXEC : 0; - if (!(result->prot & (1 << access_type))) { + result->f.prot = ap_to_rw_prot(env, mmu_idx, ap, domain_prot); + result->f.prot |= result->f.prot ? PAGE_EXEC : 0; + if (!(result->f.prot & (1 << access_type))) { /* Access permission fault. */ fi->type = ARMFault_Permission; goto do_fault; } - result->phys = phys_addr; + result->f.phys_addr = phys_addr; return false; do_fault: fi->domain = domain; @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_v6(CPUARMState *env, uint32_t address, phys_addr = (desc & 0xff000000) | (address & 0x00ffffff); phys_addr |= (uint64_t)extract32(desc, 20, 4) << 32; phys_addr |= (uint64_t)extract32(desc, 5, 4) << 36; - result->page_size = 0x1000000; + result->f.lg_page_size = 24; /* 16MB */ } else { /* Section. */ phys_addr = (desc & 0xfff00000) | (address & 0x000fffff); - result->page_size = 0x100000; + result->f.lg_page_size = 20; /* 1MB */ } ap = ((desc >> 10) & 3) | ((desc >> 13) & 4); xn = desc & (1 << 4); @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_v6(CPUARMState *env, uint32_t address, case 1: /* 64k page. */ phys_addr = (desc & 0xffff0000) | (address & 0xffff); xn = desc & (1 << 15); - result->page_size = 0x10000; + result->f.lg_page_size = 16; break; case 2: case 3: /* 4k page. */ phys_addr = (desc & 0xfffff000) | (address & 0xfff); xn = desc & 1; - result->page_size = 0x1000; + result->f.lg_page_size = 12; break; default: /* Never happens, but compiler isn't smart enough to tell. */ @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_v6(CPUARMState *env, uint32_t address, } } if (domain_prot == 3) { - result->prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC; + result->f.prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC; } else { if (pxn && !regime_is_user(env, mmu_idx)) { xn = 1; @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_v6(CPUARMState *env, uint32_t address, fi->type = ARMFault_AccessFlag; goto do_fault; } - result->prot = simple_ap_to_rw_prot(env, mmu_idx, ap >> 1); + result->f.prot = simple_ap_to_rw_prot(env, mmu_idx, ap >> 1); } else { - result->prot = ap_to_rw_prot(env, mmu_idx, ap, domain_prot); + result->f.prot = ap_to_rw_prot(env, mmu_idx, ap, domain_prot); } - if (result->prot && !xn) { - result->prot |= PAGE_EXEC; + if (result->f.prot && !xn) { + result->f.prot |= PAGE_EXEC; } - if (!(result->prot & (1 << access_type))) { + if (!(result->f.prot & (1 << access_type))) { /* Access permission fault. */ fi->type = ARMFault_Permission; goto do_fault; @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_v6(CPUARMState *env, uint32_t address, * the CPU doesn't support TZ or this is a non-secure translation * regime, because the attribute will already be non-secure. */ - result->attrs.secure = false; + result->f.attrs.secure = false; } - result->phys = phys_addr; + result->f.phys_addr = phys_addr; return false; do_fault: fi->domain = domain; @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_lpae(CPUARMState *env, uint64_t address, if (mmu_idx == ARMMMUIdx_Stage2 || mmu_idx == ARMMMUIdx_Stage2_S) { ns = mmu_idx == ARMMMUIdx_Stage2; xn = extract32(attrs, 11, 2); - result->prot = get_S2prot(env, ap, xn, s1_is_el0); + result->f.prot = get_S2prot(env, ap, xn, s1_is_el0); } else { ns = extract32(attrs, 3, 1); xn = extract32(attrs, 12, 1); pxn = extract32(attrs, 11, 1); - result->prot = get_S1prot(env, mmu_idx, aarch64, ap, ns, xn, pxn); + result->f.prot = get_S1prot(env, mmu_idx, aarch64, ap, ns, xn, pxn); } fault_type = ARMFault_Permission; - if (!(result->prot & (1 << access_type))) { + if (!(result->f.prot & (1 << access_type))) { goto do_fault; } @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_lpae(CPUARMState *env, uint64_t address, * the CPU doesn't support TZ or this is a non-secure translation * regime, because the attribute will already be non-secure. */ - result->attrs.secure = false; + 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->attrs) = true; + arm_tlb_bti_gp(&result->f.attrs) = true; } if (mmu_idx == ARMMMUIdx_Stage2 || mmu_idx == ARMMMUIdx_Stage2_S) { @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_lpae(CPUARMState *env, uint64_t address, result->cacheattrs.shareability = extract32(attrs, 6, 2); } - result->phys = descaddr; - result->page_size = page_size; + result->f.phys_addr = descaddr; + result->f.lg_page_size = ctz64(page_size); return false; do_fault: @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_pmsav5(CPUARMState *env, uint32_t address, if (regime_translation_disabled(env, mmu_idx, is_secure)) { /* MPU disabled. */ - result->phys = address; - result->prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC; + result->f.phys_addr = address; + result->f.prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC; return false; } - result->phys = address; + result->f.phys_addr = address; for (n = 7; n >= 0; n--) { base = env->cp15.c6_region[n]; if ((base & 1) == 0) { @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_pmsav5(CPUARMState *env, uint32_t address, fi->level = 1; return true; } - result->prot = PAGE_READ | PAGE_WRITE; + result->f.prot = PAGE_READ | PAGE_WRITE; break; case 2: - result->prot = PAGE_READ; + result->f.prot = PAGE_READ; if (!is_user) { - result->prot |= PAGE_WRITE; + result->f.prot |= PAGE_WRITE; } break; case 3: - result->prot = PAGE_READ | PAGE_WRITE; + result->f.prot = PAGE_READ | PAGE_WRITE; break; case 5: if (is_user) { @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_pmsav5(CPUARMState *env, uint32_t address, fi->level = 1; return true; } - result->prot = PAGE_READ; + result->f.prot = PAGE_READ; break; case 6: - result->prot = PAGE_READ; + result->f.prot = PAGE_READ; break; default: /* Bad permission. */ @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_pmsav5(CPUARMState *env, uint32_t address, fi->level = 1; return true; } - result->prot |= PAGE_EXEC; + result->f.prot |= PAGE_EXEC; return false; } static void get_phys_addr_pmsav7_default(CPUARMState *env, ARMMMUIdx mmu_idx, - int32_t address, int *prot) + int32_t address, uint8_t *prot) { if (!arm_feature(env, ARM_FEATURE_M)) { *prot = PAGE_READ | PAGE_WRITE; @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address, int n; bool is_user = regime_is_user(env, mmu_idx); - result->phys = address; - result->page_size = TARGET_PAGE_SIZE; - result->prot = 0; + result->f.phys_addr = address; + result->f.lg_page_size = TARGET_PAGE_BITS; + result->f.prot = 0; if (regime_translation_disabled(env, mmu_idx, secure) || m_is_ppb_region(env, address)) { @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address, * which always does a direct read using address_space_ldl(), rather * than going via this function, so we don't need to check that here. */ - get_phys_addr_pmsav7_default(env, mmu_idx, address, &result->prot); + get_phys_addr_pmsav7_default(env, mmu_idx, address, &result->f.prot); } else { /* MPU enabled */ for (n = (int)cpu->pmsav7_dregion - 1; n >= 0; n--) { /* region search */ @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address, if (ranges_overlap(base, rmask, address & TARGET_PAGE_MASK, TARGET_PAGE_SIZE)) { - result->page_size = 1; + result->f.lg_page_size = 0; } continue; } @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address, continue; } if (rsize < TARGET_PAGE_BITS) { - result->page_size = 1 << rsize; + result->f.lg_page_size = rsize; } break; } @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address, fi->type = ARMFault_Background; return true; } - get_phys_addr_pmsav7_default(env, mmu_idx, address, &result->prot); + get_phys_addr_pmsav7_default(env, mmu_idx, address, + &result->f.prot); } else { /* a MPU hit! */ uint32_t ap = extract32(env->pmsav7.dracr[n], 8, 3); uint32_t xn = extract32(env->pmsav7.dracr[n], 12, 1); @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address, case 5: break; /* no access */ case 3: - result->prot |= PAGE_WRITE; + result->f.prot |= PAGE_WRITE; /* fall through */ case 2: case 6: - result->prot |= PAGE_READ | PAGE_EXEC; + result->f.prot |= PAGE_READ | PAGE_EXEC; break; case 7: /* for v7M, same as 6; for R profile a reserved value */ if (arm_feature(env, ARM_FEATURE_M)) { - result->prot |= PAGE_READ | PAGE_EXEC; + result->f.prot |= PAGE_READ | PAGE_EXEC; break; } /* fall through */ @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address, case 1: case 2: case 3: - result->prot |= PAGE_WRITE; + result->f.prot |= PAGE_WRITE; /* fall through */ case 5: case 6: - result->prot |= PAGE_READ | PAGE_EXEC; + result->f.prot |= PAGE_READ | PAGE_EXEC; break; case 7: /* for v7M, same as 6; for R profile a reserved value */ if (arm_feature(env, ARM_FEATURE_M)) { - result->prot |= PAGE_READ | PAGE_EXEC; + result->f.prot |= PAGE_READ | PAGE_EXEC; break; } /* fall through */ @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address, /* execute never */ if (xn) { - result->prot &= ~PAGE_EXEC; + result->f.prot &= ~PAGE_EXEC; } } } fi->type = ARMFault_Permission; fi->level = 1; - return !(result->prot & (1 << access_type)); + return !(result->f.prot & (1 << access_type)); } bool pmsav8_mpu_lookup(CPUARMState *env, uint32_t address, @@ -XXX,XX +XXX,XX @@ bool pmsav8_mpu_lookup(CPUARMState *env, uint32_t address, uint32_t addr_page_base = address & TARGET_PAGE_MASK; uint32_t addr_page_limit = addr_page_base + (TARGET_PAGE_SIZE - 1); - result->page_size = TARGET_PAGE_SIZE; - result->phys = address; - result->prot = 0; + result->f.lg_page_size = TARGET_PAGE_BITS; + result->f.phys_addr = address; + result->f.prot = 0; if (mregion) { *mregion = -1; } @@ -XXX,XX +XXX,XX @@ bool pmsav8_mpu_lookup(CPUARMState *env, uint32_t address, ranges_overlap(base, limit - base + 1, addr_page_base, TARGET_PAGE_SIZE)) { - result->page_size = 1; + result->f.lg_page_size = 0; } continue; } if (base > addr_page_base || limit < addr_page_limit) { - result->page_size = 1; + result->f.lg_page_size = 0; } if (matchregion != -1) { @@ -XXX,XX +XXX,XX @@ bool pmsav8_mpu_lookup(CPUARMState *env, uint32_t address, if (matchregion == -1) { /* hit using the background region */ - get_phys_addr_pmsav7_default(env, mmu_idx, address, &result->prot); + get_phys_addr_pmsav7_default(env, mmu_idx, address, &result->f.prot); } else { uint32_t ap = extract32(env->pmsav8.rbar[secure][matchregion], 1, 2); uint32_t xn = extract32(env->pmsav8.rbar[secure][matchregion], 0, 1); @@ -XXX,XX +XXX,XX @@ bool pmsav8_mpu_lookup(CPUARMState *env, uint32_t address, xn = 1; } - result->prot = simple_ap_to_rw_prot(env, mmu_idx, ap); - if (result->prot && !xn && !(pxn && !is_user)) { - result->prot |= PAGE_EXEC; + result->f.prot = simple_ap_to_rw_prot(env, mmu_idx, ap); + if (result->f.prot && !xn && !(pxn && !is_user)) { + result->f.prot |= PAGE_EXEC; } /* * We don't need to look the attribute up in the MAIR0/MAIR1 @@ -XXX,XX +XXX,XX @@ bool pmsav8_mpu_lookup(CPUARMState *env, uint32_t address, fi->type = ARMFault_Permission; fi->level = 1; - return !(result->prot & (1 << access_type)); + return !(result->f.prot & (1 << access_type)); } static bool v8m_is_sau_exempt(CPUARMState *env, @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_pmsav8(CPUARMState *env, uint32_t address, } else { fi->type = ARMFault_QEMU_SFault; } - result->page_size = sattrs.subpage ? 1 : TARGET_PAGE_SIZE; - result->phys = address; - result->prot = 0; + result->f.lg_page_size = sattrs.subpage ? 0 : TARGET_PAGE_BITS; + result->f.phys_addr = address; + result->f.prot = 0; return true; } } else { @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_pmsav8(CPUARMState *env, uint32_t address, * might downgrade a secure access to nonsecure. */ if (sattrs.ns) { - result->attrs.secure = false; + result->f.attrs.secure = false; } else if (!secure) { /* * NS access to S memory must fault. @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_pmsav8(CPUARMState *env, uint32_t address, * for M_FAKE_FSR_SFAULT in arm_v7m_cpu_do_interrupt(). */ fi->type = ARMFault_QEMU_SFault; - result->page_size = sattrs.subpage ? 1 : TARGET_PAGE_SIZE; - result->phys = address; - result->prot = 0; + result->f.lg_page_size = sattrs.subpage ? 0 : TARGET_PAGE_BITS; + result->f.phys_addr = address; + result->f.prot = 0; return true; } } @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_pmsav8(CPUARMState *env, uint32_t address, ret = pmsav8_mpu_lookup(env, address, access_type, mmu_idx, secure, result, fi, NULL); if (sattrs.subpage) { - result->page_size = 1; + result->f.lg_page_size = 0; } return ret; } @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_disabled(CPUARMState *env, target_ulong address, result->cacheattrs.is_s2_format = false; } - result->phys = address; - result->prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC; - result->page_size = TARGET_PAGE_SIZE; + result->f.phys_addr = address; + result->f.prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC; + result->f.lg_page_size = TARGET_PAGE_BITS; result->cacheattrs.shareability = shareability; result->cacheattrs.attrs = memattr; return 0; @@ -XXX,XX +XXX,XX @@ bool get_phys_addr_with_secure(CPUARMState *env, target_ulong address, return ret; } - ipa = result->phys; - ipa_secure = result->attrs.secure; + 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 @@ -XXX,XX +XXX,XX @@ bool get_phys_addr_with_secure(CPUARMState *env, target_ulong address, * Save the stage1 results so that we may merge * prot and cacheattrs later. */ - s1_prot = result->prot; + s1_prot = result->f.prot; cacheattrs1 = result->cacheattrs; memset(result, 0, sizeof(*result)); @@ -XXX,XX +XXX,XX @@ bool get_phys_addr_with_secure(CPUARMState *env, target_ulong address, fi->s2addr = ipa; /* Combine the S1 and S2 perms. */ - result->prot &= s1_prot; + result->f.prot &= s1_prot; /* If S2 fails, return early. */ if (ret) { @@ -XXX,XX +XXX,XX @@ bool get_phys_addr_with_secure(CPUARMState *env, target_ulong address, * Check if IPA translates to secure or non-secure PA space. * Note that VSTCR overrides VTCR and {N}SW overrides {N}SA. */ - result->attrs.secure = + result->f.attrs.secure = (is_secure && !(env->cp15.vstcr_el2 & (VSTCR_SA | VSTCR_SW)) && (ipa_secure @@ -XXX,XX +XXX,XX @@ bool get_phys_addr_with_secure(CPUARMState *env, target_ulong address, * cannot upgrade an non-secure translation regime's attributes * to secure. */ - result->attrs.secure = is_secure; - result->attrs.user = regime_is_user(env, mmu_idx); + result->f.attrs.secure = is_secure; + result->f.attrs.user = regime_is_user(env, mmu_idx); /* * Fast Context Switch Extension. This doesn't exist at all in v8. @@ -XXX,XX +XXX,XX @@ bool get_phys_addr_with_secure(CPUARMState *env, target_ulong address, if (arm_feature(env, ARM_FEATURE_PMSA)) { bool ret; - result->page_size = TARGET_PAGE_SIZE; + result->f.lg_page_size = TARGET_PAGE_BITS; if (arm_feature(env, ARM_FEATURE_V8)) { /* PMSAv8 */ @@ -XXX,XX +XXX,XX @@ bool get_phys_addr_with_secure(CPUARMState *env, target_ulong address, (access_type == MMU_DATA_STORE ? "writing" : "execute"), (uint32_t)address, mmu_idx, ret ? "Miss" : "Hit", - result->prot & PAGE_READ ? 'r' : '-', - result->prot & PAGE_WRITE ? 'w' : '-', - result->prot & PAGE_EXEC ? 'x' : '-'); + result->f.prot & PAGE_READ ? 'r' : '-', + result->f.prot & PAGE_WRITE ? 'w' : '-', + result->f.prot & PAGE_EXEC ? 'x' : '-'); return ret; } @@ -XXX,XX +XXX,XX @@ hwaddr arm_cpu_get_phys_page_attrs_debug(CPUState *cs, vaddr addr, bool ret; ret = get_phys_addr(env, addr, MMU_DATA_LOAD, mmu_idx, &res, &fi); - *attrs = res.attrs; + *attrs = res.f.attrs; if (ret) { return -1; } - return res.phys; + return res.f.phys_addr; } 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, * target page size are handled specially, so for those we * pass in the exact addresses. */ - if (res.page_size >= TARGET_PAGE_SIZE) { - res.phys &= TARGET_PAGE_MASK; + if (res.f.lg_page_size >= TARGET_PAGE_BITS) { + 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.attrs) = true; + arm_tlb_mte_tagged(&res.f.attrs) = true; } - tlb_set_page_with_attrs(cs, address, res.phys, res.attrs, - res.prot, mmu_idx, res.page_size); + tlb_set_page_full(cs, mmu_idx, address, &res.f); return true; } else if (probe) { return false; -- 2.25.1 From: Jerome Forissier <jerome.forissier@linaro.org> According to the Linux kernel booting.rst [1], CPTR_EL3.ESM and SCR_EL3.EnTP2 must be initialized to 1 when EL3 is present and FEAT_SME is advertised. This has to be taken care of when QEMU boots directly into the kernel (i.e., "-M virt,secure=on -cpu max -kernel Image"). Cc: qemu-stable@nongnu.org Fixes: 78cb9776662a ("target/arm: Enable SME for -cpu max") Link: [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/arm64/booting.rst?h=v6.0#n321 Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org> Message-id: 20221003145641.1921467-1-jerome.forissier@linaro.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/boot.c | 4 ++++ 1 file changed, 4 insertions(+) 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 @@ static void do_cpu_reset(void *opaque) if (cpu_isar_feature(aa64_sve, cpu)) { env->cp15.cptr_el[3] |= R_CPTR_EL3_EZ_MASK; } + if (cpu_isar_feature(aa64_sme, cpu)) { + env->cp15.cptr_el[3] |= R_CPTR_EL3_ESM_MASK; + env->cp15.scr_el3 |= SCR_ENTP2; + } /* AArch64 kernels never boot in secure mode */ assert(!info->secure_boot); /* This hook is only supported for AArch32 currently: -- 2.25.1 Arm CPUs support some subset of the granule (page) sizes 4K, 16K and 64K. The guest selects the one it wants using bits in the TCR_ELx registers. If it tries to program these registers with a value that is either reserved or which requests a size that the CPU does not implement, the architecture requires that the CPU behaves as if the field was programmed to some size that has been implemented. Currently we don't implement this, and instead let the guest use any granule size, even if the CPU ID register fields say it isn't present. Make aa64_va_parameters() check against the supported granule size and force use of a different one if it is not implemented. (A subsequent commit will make ARMVAParameters use the new enum rather than the current pair of using16k/using64k bools.) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 20221003162315.2833797-2-peter.maydell@linaro.org --- target/arm/cpu.h | 33 +++++++++++++ target/arm/internals.h | 9 ++++ target/arm/helper.c | 102 +++++++++++++++++++++++++++++++++++++---- 3 files changed, 136 insertions(+), 8 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 bool isar_feature_aa64_tgran16_2_lpa2(const ARMISARegisters *id) return t >= 3 || (t == 0 && isar_feature_aa64_tgran16_lpa2(id)); } +static inline bool isar_feature_aa64_tgran4(const ARMISARegisters *id) +{ + return FIELD_SEX64(id->id_aa64mmfr0, ID_AA64MMFR0, TGRAN4) >= 0; +} + +static inline bool isar_feature_aa64_tgran16(const ARMISARegisters *id) +{ + return FIELD_EX64(id->id_aa64mmfr0, ID_AA64MMFR0, TGRAN16) >= 1; +} + +static inline bool isar_feature_aa64_tgran64(const ARMISARegisters *id) +{ + return FIELD_SEX64(id->id_aa64mmfr0, ID_AA64MMFR0, TGRAN64) >= 0; +} + +static inline bool isar_feature_aa64_tgran4_2(const ARMISARegisters *id) +{ + unsigned t = FIELD_EX64(id->id_aa64mmfr0, ID_AA64MMFR0, TGRAN4_2); + return t >= 2 || (t == 0 && isar_feature_aa64_tgran4(id)); +} + +static inline bool isar_feature_aa64_tgran16_2(const ARMISARegisters *id) +{ + unsigned t = FIELD_EX64(id->id_aa64mmfr0, ID_AA64MMFR0, TGRAN16_2); + return t >= 2 || (t == 0 && isar_feature_aa64_tgran16(id)); +} + +static inline bool isar_feature_aa64_tgran64_2(const ARMISARegisters *id) +{ + unsigned t = FIELD_EX64(id->id_aa64mmfr0, ID_AA64MMFR0, TGRAN64_2); + return t >= 2 || (t == 0 && isar_feature_aa64_tgran64(id)); +} + static inline bool isar_feature_aa64_ccidx(const ARMISARegisters *id) { return FIELD_EX64(id->id_aa64mmfr2, ID_AA64MMFR2, CCIDX) != 0; 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 @@ static inline uint32_t aarch64_pstate_valid_mask(const ARMISARegisters *id) return valid; } +/* Granule size (i.e. page size) */ +typedef enum ARMGranuleSize { + /* Same order as TG0 encoding */ + Gran4K, + Gran64K, + Gran16K, + GranInvalid, +} ARMGranuleSize; + /* * Parameters of a given virtual address, as extracted from the * translation control register (TCR) for a given regime. 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 int aa64_va_parameter_tcma(uint64_t tcr, ARMMMUIdx mmu_idx) } } +static ARMGranuleSize tg0_to_gran_size(int tg) +{ + switch (tg) { + case 0: + return Gran4K; + case 1: + return Gran64K; + case 2: + return Gran16K; + default: + return GranInvalid; + } +} + +static ARMGranuleSize tg1_to_gran_size(int tg) +{ + switch (tg) { + case 1: + return Gran16K; + case 2: + return Gran4K; + case 3: + return Gran64K; + default: + return GranInvalid; + } +} + +static inline bool have4k(ARMCPU *cpu, bool stage2) +{ + return stage2 ? cpu_isar_feature(aa64_tgran4_2, cpu) + : cpu_isar_feature(aa64_tgran4, cpu); +} + +static inline bool have16k(ARMCPU *cpu, bool stage2) +{ + return stage2 ? cpu_isar_feature(aa64_tgran16_2, cpu) + : cpu_isar_feature(aa64_tgran16, cpu); +} + +static inline bool have64k(ARMCPU *cpu, bool stage2) +{ + return stage2 ? cpu_isar_feature(aa64_tgran64_2, cpu) + : cpu_isar_feature(aa64_tgran64, cpu); +} + +static ARMGranuleSize sanitize_gran_size(ARMCPU *cpu, ARMGranuleSize gran, + bool stage2) +{ + switch (gran) { + case Gran4K: + if (have4k(cpu, stage2)) { + return gran; + } + break; + case Gran16K: + if (have16k(cpu, stage2)) { + return gran; + } + break; + case Gran64K: + if (have64k(cpu, stage2)) { + return gran; + } + break; + case GranInvalid: + break; + } + /* + * If the guest selects a granule size that isn't implemented, + * the architecture requires that we behave as if it selected one + * that is (with an IMPDEF choice of which one to pick). We choose + * to implement the smallest supported granule size. + */ + if (have4k(cpu, stage2)) { + return Gran4K; + } + if (have16k(cpu, stage2)) { + return Gran16K; + } + assert(have64k(cpu, stage2)); + return Gran64K; +} + ARMVAParameters aa64_va_parameters(CPUARMState *env, uint64_t va, ARMMMUIdx mmu_idx, bool data) { uint64_t tcr = regime_tcr(env, mmu_idx); bool epd, hpd, using16k, using64k, tsz_oob, ds; int select, tsz, tbi, max_tsz, min_tsz, ps, sh; + ARMGranuleSize gran; ARMCPU *cpu = env_archcpu(env); + bool stage2 = mmu_idx == ARMMMUIdx_Stage2 || mmu_idx == ARMMMUIdx_Stage2_S; if (!regime_has_2_ranges(mmu_idx)) { select = 0; tsz = extract32(tcr, 0, 6); - using64k = extract32(tcr, 14, 1); - using16k = extract32(tcr, 15, 1); - if (mmu_idx == ARMMMUIdx_Stage2 || mmu_idx == ARMMMUIdx_Stage2_S) { + gran = tg0_to_gran_size(extract32(tcr, 14, 2)); + if (stage2) { /* VTCR_EL2 */ hpd = false; } else { @@ -XXX,XX +XXX,XX @@ ARMVAParameters aa64_va_parameters(CPUARMState *env, uint64_t va, select = extract64(va, 55, 1); if (!select) { tsz = extract32(tcr, 0, 6); + gran = tg0_to_gran_size(extract32(tcr, 14, 2)); epd = extract32(tcr, 7, 1); sh = extract32(tcr, 12, 2); - using64k = extract32(tcr, 14, 1); - using16k = extract32(tcr, 15, 1); hpd = extract64(tcr, 41, 1); } else { - int tg = extract32(tcr, 30, 2); - using16k = tg == 1; - using64k = tg == 3; tsz = extract32(tcr, 16, 6); + gran = tg1_to_gran_size(extract32(tcr, 30, 2)); epd = extract32(tcr, 23, 1); sh = extract32(tcr, 28, 2); hpd = extract64(tcr, 42, 1); @@ -XXX,XX +XXX,XX @@ ARMVAParameters aa64_va_parameters(CPUARMState *env, uint64_t va, ds = extract64(tcr, 59, 1); } + gran = sanitize_gran_size(cpu, gran, stage2); + using64k = gran == Gran64K; + using16k = gran == Gran16K; + if (cpu_isar_feature(aa64_st, cpu)) { max_tsz = 48 - using64k; } else { -- 2.25.1 Now we have an enum for the granule size, use it in the ARMVAParameters struct instead of the using16k/using64k bools. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20221003162315.2833797-3-peter.maydell@linaro.org --- target/arm/internals.h | 23 +++++++++++++++++++++-- target/arm/helper.c | 39 ++++++++++++++++++++++++++++----------- target/arm/ptw.c | 8 +------- 3 files changed, 50 insertions(+), 20 deletions(-) diff --git a/target/arm/internals.h b/target/arm/internals.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/internals.h +++ b/target/arm/internals.h @@ -XXX,XX +XXX,XX @@ typedef enum ARMGranuleSize { GranInvalid, } ARMGranuleSize; +/** + * arm_granule_bits: Return address size of the granule in bits + * + * Return the address size of the granule in bits. This corresponds + * to the pseudocode TGxGranuleBits(). + */ +static inline int arm_granule_bits(ARMGranuleSize gran) +{ + switch (gran) { + case Gran64K: + return 16; + case Gran16K: + return 14; + case Gran4K: + return 12; + default: + g_assert_not_reached(); + } +} + /* * Parameters of a given virtual address, as extracted from the * translation control register (TCR) for a given regime. @@ -XXX,XX +XXX,XX @@ typedef struct ARMVAParameters { bool tbi : 1; bool epd : 1; bool hpd : 1; - bool using16k : 1; - bool using64k : 1; bool tsz_oob : 1; /* tsz has been clamped to legal range */ bool ds : 1; + ARMGranuleSize gran : 2; } ARMVAParameters; ARMVAParameters aa64_va_parameters(CPUARMState *env, uint64_t va, 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 @@ typedef struct { uint64_t length; } TLBIRange; +static ARMGranuleSize tlbi_range_tg_to_gran_size(int tg) +{ + /* + * Note that the TLBI range TG field encoding differs from both + * TG0 and TG1 encodings. + */ + switch (tg) { + case 1: + return Gran4K; + case 2: + return Gran16K; + case 3: + return Gran64K; + default: + return GranInvalid; + } +} + static TLBIRange tlbi_aa64_get_range(CPUARMState *env, ARMMMUIdx mmuidx, uint64_t value) { @@ -XXX,XX +XXX,XX @@ static TLBIRange tlbi_aa64_get_range(CPUARMState *env, ARMMMUIdx mmuidx, uint64_t select = sextract64(value, 36, 1); ARMVAParameters param = aa64_va_parameters(env, select, mmuidx, true); TLBIRange ret = { }; + ARMGranuleSize gran; page_size_granule = extract64(value, 46, 2); + gran = tlbi_range_tg_to_gran_size(page_size_granule); /* The granule encoded in value must match the granule in use. */ - if (page_size_granule != (param.using64k ? 3 : param.using16k ? 2 : 1)) { + if (gran != param.gran) { qemu_log_mask(LOG_GUEST_ERROR, "Invalid tlbi page size granule %d\n", page_size_granule); return ret; } - page_shift = (page_size_granule - 1) * 2 + 12; + page_shift = arm_granule_bits(gran); num = extract64(value, 39, 5); scale = extract64(value, 44, 2); exponent = (5 * scale) + 1; @@ -XXX,XX +XXX,XX @@ ARMVAParameters aa64_va_parameters(CPUARMState *env, uint64_t va, ARMMMUIdx mmu_idx, bool data) { uint64_t tcr = regime_tcr(env, mmu_idx); - bool epd, hpd, using16k, using64k, tsz_oob, ds; + bool epd, hpd, tsz_oob, ds; int select, tsz, tbi, max_tsz, min_tsz, ps, sh; ARMGranuleSize gran; ARMCPU *cpu = env_archcpu(env); @@ -XXX,XX +XXX,XX @@ ARMVAParameters aa64_va_parameters(CPUARMState *env, uint64_t va, } gran = sanitize_gran_size(cpu, gran, stage2); - using64k = gran == Gran64K; - using16k = gran == Gran16K; if (cpu_isar_feature(aa64_st, cpu)) { - max_tsz = 48 - using64k; + max_tsz = 48 - (gran == Gran64K); } else { max_tsz = 39; } @@ -XXX,XX +XXX,XX @@ ARMVAParameters aa64_va_parameters(CPUARMState *env, uint64_t va, * adjust the effective value of DS, as documented. */ min_tsz = 16; - if (using64k) { + if (gran == Gran64K) { if (cpu_isar_feature(aa64_lva, cpu)) { min_tsz = 12; } @@ -XXX,XX +XXX,XX @@ ARMVAParameters aa64_va_parameters(CPUARMState *env, uint64_t va, switch (mmu_idx) { case ARMMMUIdx_Stage2: case ARMMMUIdx_Stage2_S: - if (using16k) { + if (gran == Gran16K) { ds = cpu_isar_feature(aa64_tgran16_2_lpa2, cpu); } else { ds = cpu_isar_feature(aa64_tgran4_2_lpa2, cpu); } break; default: - if (using16k) { + if (gran == Gran16K) { ds = cpu_isar_feature(aa64_tgran16_lpa2, cpu); } else { ds = cpu_isar_feature(aa64_tgran4_lpa2, cpu); @@ -XXX,XX +XXX,XX @@ ARMVAParameters aa64_va_parameters(CPUARMState *env, uint64_t va, .tbi = tbi, .epd = epd, .hpd = hpd, - .using16k = using16k, - .using64k = using64k, .tsz_oob = tsz_oob, .ds = ds, + .gran = gran, }; } 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, } } - if (param.using64k) { - stride = 13; - } else if (param.using16k) { - stride = 11; - } else { - stride = 9; - } + stride = arm_granule_bits(param.gran) - 3; /* * Note that QEMU ignores shareability and cacheability attributes, -- 2.25.1 FEAT_GTG is a change tho the ID register ID_AA64MMFR0_EL1 so that it can report a different set of supported granule (page) sizes for stage 1 and stage 2 translation tables. As of commit c20281b2a5048 we already report the granule sizes that way for '-cpu max', and now we also correctly make attempts to use unimplemented granule sizes fail, so we can report the support of the feature in the documentation. Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 20221003162315.2833797-4-peter.maydell@linaro.org --- docs/system/arm/emulation.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/system/arm/emulation.rst b/docs/system/arm/emulation.rst index XXXXXXX..XXXXXXX 100644 --- a/docs/system/arm/emulation.rst +++ b/docs/system/arm/emulation.rst @@ -XXX,XX +XXX,XX @@ the following architecture extensions: - FEAT_FRINTTS (Floating-point to integer instructions) - FEAT_FlagM (Flag manipulation instructions v2) - FEAT_FlagM2 (Enhancements to flag manipulation instructions) +- FEAT_GTG (Guest translation granule size) - FEAT_HCX (Support for the HCRX_EL2 register) - FEAT_HPDS (Hierarchical permission disables) - FEAT_I8MM (AArch64 Int8 matrix multiplication instructions) -- 2.25.1