From ec2c933701a438af478b03b904a2dbc9f1836941 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Sat, 5 Oct 2024 07:02:45 -0700 Subject: target/arm: Pass MemOp to get_phys_addr MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Zero is the safe do-nothing value for callers to use. Reviewed-by: Helge Deller Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Richard Henderson --- target/arm/ptw.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'target/arm/ptw.c') diff --git a/target/arm/ptw.c b/target/arm/ptw.c index 659855133c..373095a339 100644 --- a/target/arm/ptw.c +++ b/target/arm/ptw.c @@ -3572,7 +3572,7 @@ bool get_phys_addr_with_space_nogpc(CPUARMState *env, vaddr address, } bool get_phys_addr(CPUARMState *env, vaddr address, - MMUAccessType access_type, ARMMMUIdx mmu_idx, + MMUAccessType access_type, MemOp memop, ARMMMUIdx mmu_idx, GetPhysAddrResult *result, ARMMMUFaultInfo *fi) { S1Translate ptw = { -- cgit 1.4.1 From 29b4d7dbd21731fb974363f3d6b34d50f08fc24f Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Sat, 5 Oct 2024 07:05:58 -0700 Subject: target/arm: Pass MemOp to get_phys_addr_with_space_nogpc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Zero is the safe do-nothing value for callers to use. Reviewed-by: Helge Deller Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Peter Maydell Signed-off-by: Richard Henderson --- target/arm/helper.c | 9 +++++---- target/arm/internals.h | 3 ++- target/arm/ptw.c | 2 +- 3 files changed, 8 insertions(+), 6 deletions(-) (limited to 'target/arm/ptw.c') diff --git a/target/arm/helper.c b/target/arm/helper.c index 3f77b40734..0a731a38e8 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -3599,11 +3599,12 @@ static uint64_t do_ats_write(CPUARMState *env, uint64_t value, GetPhysAddrResult res = {}; /* - * I_MXTJT: Granule protection checks are not performed on the final address - * of a successful translation. + * I_MXTJT: Granule protection checks are not performed on the final + * address of a successful translation. This is a translation not a + * memory reference, so "memop = none = 0". */ - ret = get_phys_addr_with_space_nogpc(env, value, access_type, mmu_idx, ss, - &res, &fi); + ret = get_phys_addr_with_space_nogpc(env, value, access_type, 0, + mmu_idx, ss, &res, &fi); /* * ATS operations only do S1 or S1+S2 translations, so we never diff --git a/target/arm/internals.h b/target/arm/internals.h index 2b16579fa5..a6088d551c 100644 --- a/target/arm/internals.h +++ b/target/arm/internals.h @@ -1461,6 +1461,7 @@ bool get_phys_addr(CPUARMState *env, vaddr address, * @env: CPUARMState * @address: virtual address to get physical address for * @access_type: 0 for read, 1 for write, 2 for execute + * @memop: memory operation feeding this access, or 0 for none * @mmu_idx: MMU index indicating required translation regime * @space: security space for the access * @result: set on translation success. @@ -1470,7 +1471,7 @@ bool get_phys_addr(CPUARMState *env, vaddr address, * a Granule Protection Check on the resulting address. */ bool get_phys_addr_with_space_nogpc(CPUARMState *env, vaddr address, - MMUAccessType access_type, + MMUAccessType access_type, MemOp memop, ARMMMUIdx mmu_idx, ARMSecuritySpace space, GetPhysAddrResult *result, ARMMMUFaultInfo *fi) diff --git a/target/arm/ptw.c b/target/arm/ptw.c index 373095a339..9af86da597 100644 --- a/target/arm/ptw.c +++ b/target/arm/ptw.c @@ -3559,7 +3559,7 @@ static bool get_phys_addr_gpc(CPUARMState *env, S1Translate *ptw, } bool get_phys_addr_with_space_nogpc(CPUARMState *env, vaddr address, - MMUAccessType access_type, + MMUAccessType access_type, MemOp memop, ARMMMUIdx mmu_idx, ARMSecuritySpace space, GetPhysAddrResult *result, ARMMMUFaultInfo *fi) -- cgit 1.4.1 From 5458670b15b9176dd75beb9516427ffb88e00a17 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Sat, 5 Oct 2024 07:09:01 -0700 Subject: target/arm: Pass MemOp to get_phys_addr_gpc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Zero is the safe do-nothing value for callers to use. Pass the value through from get_phys_addr. Reviewed-by: Helge Deller Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Peter Maydell Signed-off-by: Richard Henderson --- target/arm/ptw.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) (limited to 'target/arm/ptw.c') diff --git a/target/arm/ptw.c b/target/arm/ptw.c index 9af86da597..e92537d8f2 100644 --- a/target/arm/ptw.c +++ b/target/arm/ptw.c @@ -81,7 +81,7 @@ static bool get_phys_addr_nogpc(CPUARMState *env, S1Translate *ptw, static bool get_phys_addr_gpc(CPUARMState *env, S1Translate *ptw, vaddr address, - MMUAccessType access_type, + MMUAccessType access_type, MemOp memop, GetPhysAddrResult *result, ARMMMUFaultInfo *fi); @@ -579,7 +579,7 @@ static bool S1_ptw_translate(CPUARMState *env, S1Translate *ptw, }; GetPhysAddrResult s2 = { }; - if (get_phys_addr_gpc(env, &s2ptw, addr, MMU_DATA_LOAD, &s2, fi)) { + if (get_phys_addr_gpc(env, &s2ptw, addr, MMU_DATA_LOAD, 0, &s2, fi)) { goto fail; } @@ -3543,7 +3543,7 @@ static bool get_phys_addr_nogpc(CPUARMState *env, S1Translate *ptw, static bool get_phys_addr_gpc(CPUARMState *env, S1Translate *ptw, vaddr address, - MMUAccessType access_type, + MMUAccessType access_type, MemOp memop, GetPhysAddrResult *result, ARMMMUFaultInfo *fi) { @@ -3641,7 +3641,8 @@ bool get_phys_addr(CPUARMState *env, vaddr address, } ptw.in_space = ss; - return get_phys_addr_gpc(env, &ptw, address, access_type, result, fi); + return get_phys_addr_gpc(env, &ptw, address, access_type, + memop, result, fi); } hwaddr arm_cpu_get_phys_page_attrs_debug(CPUState *cs, vaddr addr, @@ -3660,7 +3661,7 @@ hwaddr arm_cpu_get_phys_page_attrs_debug(CPUState *cs, vaddr addr, ARMMMUFaultInfo fi = {}; bool ret; - ret = get_phys_addr_gpc(env, &ptw, addr, MMU_DATA_LOAD, &res, &fi); + ret = get_phys_addr_gpc(env, &ptw, addr, MMU_DATA_LOAD, 0, &res, &fi); *attrs = res.f.attrs; if (ret) { -- cgit 1.4.1 From c6cd9f9fa9fc62531ad0836401dac3feb4e73b30 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Sat, 5 Oct 2024 07:36:10 -0700 Subject: target/arm: Pass MemOp to get_phys_addr_nogpc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Zero is the safe do-nothing value for callers to use. Pass the value through from get_phys_addr_gpc and get_phys_addr_with_space_nogpc. Reviewed-by: Helge Deller Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Peter Maydell Signed-off-by: Richard Henderson --- target/arm/ptw.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) (limited to 'target/arm/ptw.c') diff --git a/target/arm/ptw.c b/target/arm/ptw.c index e92537d8f2..0445c3ccf3 100644 --- a/target/arm/ptw.c +++ b/target/arm/ptw.c @@ -75,7 +75,7 @@ typedef struct S1Translate { static bool get_phys_addr_nogpc(CPUARMState *env, S1Translate *ptw, vaddr address, - MMUAccessType access_type, + MMUAccessType access_type, MemOp memop, GetPhysAddrResult *result, ARMMMUFaultInfo *fi); @@ -3313,7 +3313,7 @@ static bool get_phys_addr_twostage(CPUARMState *env, S1Translate *ptw, ARMSecuritySpace ipa_space; uint64_t hcr; - ret = get_phys_addr_nogpc(env, ptw, address, access_type, result, fi); + ret = get_phys_addr_nogpc(env, ptw, address, access_type, 0, result, fi); /* If S1 fails, return early. */ if (ret) { @@ -3339,7 +3339,7 @@ static bool get_phys_addr_twostage(CPUARMState *env, S1Translate *ptw, cacheattrs1 = result->cacheattrs; memset(result, 0, sizeof(*result)); - ret = get_phys_addr_nogpc(env, ptw, ipa, access_type, result, fi); + ret = get_phys_addr_nogpc(env, ptw, ipa, access_type, 0, result, fi); fi->s2addr = ipa; /* Combine the S1 and S2 perms. */ @@ -3406,7 +3406,7 @@ static bool get_phys_addr_twostage(CPUARMState *env, S1Translate *ptw, static bool get_phys_addr_nogpc(CPUARMState *env, S1Translate *ptw, vaddr address, - MMUAccessType access_type, + MMUAccessType access_type, MemOp memop, GetPhysAddrResult *result, ARMMMUFaultInfo *fi) { @@ -3547,7 +3547,8 @@ static bool get_phys_addr_gpc(CPUARMState *env, S1Translate *ptw, GetPhysAddrResult *result, ARMMMUFaultInfo *fi) { - if (get_phys_addr_nogpc(env, ptw, address, access_type, result, fi)) { + if (get_phys_addr_nogpc(env, ptw, address, access_type, + memop, result, fi)) { return true; } if (!granule_protection_check(env, result->f.phys_addr, @@ -3568,7 +3569,8 @@ bool get_phys_addr_with_space_nogpc(CPUARMState *env, vaddr address, .in_mmu_idx = mmu_idx, .in_space = space, }; - return get_phys_addr_nogpc(env, &ptw, address, access_type, result, fi); + return get_phys_addr_nogpc(env, &ptw, address, access_type, + memop, result, fi); } bool get_phys_addr(CPUARMState *env, vaddr address, -- cgit 1.4.1 From 21e5a2870e3c67ac903cde52c1c587a63d520d96 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Sat, 5 Oct 2024 07:39:14 -0700 Subject: target/arm: Pass MemOp through get_phys_addr_twostage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pass memop through get_phys_addr_twostage with its recursion with get_phys_addr_nogpc. Reviewed-by: Helge Deller Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Peter Maydell Signed-off-by: Richard Henderson --- target/arm/ptw.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) (limited to 'target/arm/ptw.c') diff --git a/target/arm/ptw.c b/target/arm/ptw.c index 0445c3ccf3..f1fca086a4 100644 --- a/target/arm/ptw.c +++ b/target/arm/ptw.c @@ -3301,7 +3301,7 @@ static bool get_phys_addr_disabled(CPUARMState *env, static bool get_phys_addr_twostage(CPUARMState *env, S1Translate *ptw, vaddr address, - MMUAccessType access_type, + MMUAccessType access_type, MemOp memop, GetPhysAddrResult *result, ARMMMUFaultInfo *fi) { @@ -3313,7 +3313,8 @@ static bool get_phys_addr_twostage(CPUARMState *env, S1Translate *ptw, ARMSecuritySpace ipa_space; uint64_t hcr; - ret = get_phys_addr_nogpc(env, ptw, address, access_type, 0, result, fi); + ret = get_phys_addr_nogpc(env, ptw, address, access_type, + memop, result, fi); /* If S1 fails, return early. */ if (ret) { @@ -3339,7 +3340,8 @@ static bool get_phys_addr_twostage(CPUARMState *env, S1Translate *ptw, cacheattrs1 = result->cacheattrs; memset(result, 0, sizeof(*result)); - ret = get_phys_addr_nogpc(env, ptw, ipa, access_type, 0, result, fi); + ret = get_phys_addr_nogpc(env, ptw, ipa, access_type, + memop, result, fi); fi->s2addr = ipa; /* Combine the S1 and S2 perms. */ @@ -3469,7 +3471,7 @@ static bool get_phys_addr_nogpc(CPUARMState *env, S1Translate *ptw, if (arm_feature(env, ARM_FEATURE_EL2) && !regime_translation_disabled(env, ARMMMUIdx_Stage2, ptw->in_space)) { return get_phys_addr_twostage(env, ptw, address, access_type, - result, fi); + memop, result, fi); } /* fall through */ -- cgit 1.4.1 From c053f40b598e62e7c52665bb280d745d87954385 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Sat, 5 Oct 2024 07:42:19 -0700 Subject: target/arm: Pass MemOp to get_phys_addr_lpae MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pass the value through from get_phys_addr_nogpc. Reviewed-by: Helge Deller Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Peter Maydell Signed-off-by: Richard Henderson --- target/arm/ptw.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'target/arm/ptw.c') diff --git a/target/arm/ptw.c b/target/arm/ptw.c index f1fca086a4..238b2c92a9 100644 --- a/target/arm/ptw.c +++ b/target/arm/ptw.c @@ -1684,12 +1684,13 @@ static bool nv_nv1_enabled(CPUARMState *env, S1Translate *ptw) * @ptw: Current and next stage parameters for the walk. * @address: virtual address to get physical address for * @access_type: MMU_DATA_LOAD, MMU_DATA_STORE or MMU_INST_FETCH + * @memop: memory operation feeding this access, or 0 for none * @result: set on translation success, * @fi: set to fault info if the translation fails */ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw, uint64_t address, - MMUAccessType access_type, + MMUAccessType access_type, MemOp memop, GetPhysAddrResult *result, ARMMMUFaultInfo *fi) { ARMCPU *cpu = env_archcpu(env); @@ -3534,7 +3535,8 @@ static bool get_phys_addr_nogpc(CPUARMState *env, S1Translate *ptw, } if (regime_using_lpae_format(env, mmu_idx)) { - return get_phys_addr_lpae(env, ptw, address, access_type, result, fi); + return get_phys_addr_lpae(env, ptw, address, access_type, + memop, result, fi); } else if (arm_feature(env, ARM_FEATURE_V7) || regime_sctlr(env, mmu_idx) & SCTLR_XP) { return get_phys_addr_v6(env, ptw, address, access_type, result, fi); -- cgit 1.4.1 From 64bda5106c56faa19a31e091aef3a08249cc4ad0 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Sat, 5 Oct 2024 07:52:10 -0700 Subject: target/arm: Move device detection earlier in get_phys_addr_lpae Determine cache attributes, and thence Device vs Normal memory, earlier in the function. We have an existing regime_is_stage2 if block into which this can be slotted. Reviewed-by: Peter Maydell Signed-off-by: Richard Henderson --- target/arm/ptw.c | 49 +++++++++++++++++++++++++------------------------ 1 file changed, 25 insertions(+), 24 deletions(-) (limited to 'target/arm/ptw.c') diff --git a/target/arm/ptw.c b/target/arm/ptw.c index 238b2c92a9..0a1a820362 100644 --- a/target/arm/ptw.c +++ b/target/arm/ptw.c @@ -2029,8 +2029,20 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw, xn = extract64(attrs, 53, 2); result->f.prot = get_S2prot(env, ap, xn, ptw->in_s1_is_el0); } + + result->cacheattrs.is_s2_format = true; + result->cacheattrs.attrs = extract32(attrs, 2, 4); + /* + * Security state does not really affect HCR_EL2.FWB; + * we only need to filter FWB for aa32 or other FEAT. + */ + device = S2_attrs_are_device(arm_hcr_el2_eff(env), + result->cacheattrs.attrs); } else { int nse, ns = extract32(attrs, 5, 1); + uint8_t attrindx; + uint64_t mair; + switch (out_space) { case ARMSS_Root: /* @@ -2102,6 +2114,19 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw, */ result->f.prot = get_S1prot(env, mmu_idx, aarch64, ap, xn, pxn, result->f.attrs.space, out_space); + + /* Index into MAIR registers for cache attributes */ + attrindx = extract32(attrs, 2, 3); + mair = env->cp15.mair_el[regime_el(env, mmu_idx)]; + assert(attrindx <= 7); + result->cacheattrs.is_s2_format = false; + result->cacheattrs.attrs = extract64(mair, attrindx * 8, 8); + + /* When in aarch64 mode, and BTI is enabled, remember GP in the TLB. */ + if (aarch64 && cpu_isar_feature(aa64_bti, cpu)) { + result->f.extra.arm.guarded = extract64(attrs, 50, 1); /* GP */ + } + device = S1_attrs_are_device(result->cacheattrs.attrs); } if (!(result->f.prot & (1 << access_type))) { @@ -2131,30 +2156,6 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw, result->f.attrs.space = out_space; result->f.attrs.secure = arm_space_is_secure(out_space); - if (regime_is_stage2(mmu_idx)) { - result->cacheattrs.is_s2_format = true; - result->cacheattrs.attrs = extract32(attrs, 2, 4); - /* - * Security state does not really affect HCR_EL2.FWB; - * we only need to filter FWB for aa32 or other FEAT. - */ - device = S2_attrs_are_device(arm_hcr_el2_eff(env), - result->cacheattrs.attrs); - } else { - /* Index into MAIR registers for cache attributes */ - uint8_t attrindx = extract32(attrs, 2, 3); - uint64_t mair = env->cp15.mair_el[regime_el(env, mmu_idx)]; - assert(attrindx <= 7); - result->cacheattrs.is_s2_format = false; - result->cacheattrs.attrs = extract64(mair, attrindx * 8, 8); - - /* When in aarch64 mode, and BTI is enabled, remember GP in the TLB. */ - if (aarch64 && cpu_isar_feature(aa64_bti, cpu)) { - result->f.extra.arm.guarded = extract64(attrs, 50, 1); /* GP */ - } - device = S1_attrs_are_device(result->cacheattrs.attrs); - } - /* * Enable alignment checks on Device memory. * -- cgit 1.4.1 From e530581ee06573fcf48c7f7a6c3f8ec6e5809243 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Sat, 5 Oct 2024 08:10:03 -0700 Subject: target/arm: Fix alignment fault priority in get_phys_addr_lpae Now that we have the MemOp for the access, we can order the alignment fault caused by memory type before the permission fault for the page. For subsequent page hits, permission and stage 2 checks are known to pass, and so the TLB_CHECK_ALIGNED fault raised in generic code is not mis-ordered. Reviewed-by: Peter Maydell Signed-off-by: Richard Henderson --- target/arm/ptw.c | 51 ++++++++++++++++++++++++++++++--------------------- 1 file changed, 30 insertions(+), 21 deletions(-) (limited to 'target/arm/ptw.c') diff --git a/target/arm/ptw.c b/target/arm/ptw.c index 0a1a820362..dd40268397 100644 --- a/target/arm/ptw.c +++ b/target/arm/ptw.c @@ -2129,6 +2129,36 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw, device = S1_attrs_are_device(result->cacheattrs.attrs); } + /* + * Enable alignment checks on Device memory. + * + * Per R_XCHFJ, the correct ordering for alignment, permission, + * and stage 2 faults is: + * - Alignment fault caused by the memory type + * - Permission fault + * - A stage 2 fault on the memory access + * Perform the alignment check now, so that we recognize it in + * the correct order. Set TLB_CHECK_ALIGNED so that any subsequent + * softmmu tlb hit will also check the alignment; clear along the + * non-device path so that tlb_fill_flags is consistent in the + * event of restart_atomic_update. + * + * In v7, for a CPU without the Virtualization Extensions this + * access is UNPREDICTABLE; we choose to make it take the alignment + * fault as is required for a v7VE CPU. (QEMU doesn't emulate any + * CPUs with ARM_FEATURE_LPAE but not ARM_FEATURE_V7VE anyway.) + */ + if (device) { + unsigned a_bits = memop_atomicity_bits(memop); + if (address & ((1 << a_bits) - 1)) { + fi->type = ARMFault_Alignment; + goto do_fault; + } + result->f.tlb_fill_flags = TLB_CHECK_ALIGNED; + } else { + result->f.tlb_fill_flags = 0; + } + if (!(result->f.prot & (1 << access_type))) { fi->type = ARMFault_Permission; goto do_fault; @@ -2156,27 +2186,6 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw, result->f.attrs.space = out_space; result->f.attrs.secure = arm_space_is_secure(out_space); - /* - * Enable alignment checks on Device memory. - * - * Per R_XCHFJ, this check is mis-ordered. The correct ordering - * for alignment, permission, and stage 2 faults should be: - * - Alignment fault caused by the memory type - * - Permission fault - * - A stage 2 fault on the memory access - * but due to the way the TCG softmmu TLB operates, we will have - * implicitly done the permission check and the stage2 lookup in - * finding the TLB entry, so the alignment check cannot be done sooner. - * - * In v7, for a CPU without the Virtualization Extensions this - * access is UNPREDICTABLE; we choose to make it take the alignment - * fault as is required for a v7VE CPU. (QEMU doesn't emulate any - * CPUs with ARM_FEATURE_LPAE but not ARM_FEATURE_V7VE anyway.) - */ - if (device) { - result->f.tlb_fill_flags |= TLB_CHECK_ALIGNED; - } - /* * For FEAT_LPA2 and effective DS, the SH field in the attributes * was re-purposed for output address bits. The SH attribute in -- cgit 1.4.1