| Message ID | 20221111182535.64844-3-alex.bennee@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | use MemTxAttrs to avoid current_cpu in hw/ | expand |
On 11/12/22 04:25, Alex Bennée wrote: > diff --git a/target/arm/ptw.c b/target/arm/ptw.c > index 3745ac9723..4b6683f90d 100644 > --- a/target/arm/ptw.c > +++ b/target/arm/ptw.c > @@ -2634,6 +2634,7 @@ static bool get_phys_addr_twostage(CPUARMState *env, S1Translate *ptw, > s1_lgpgsz = result->f.lg_page_size; > cacheattrs1 = result->cacheattrs; > memset(result, 0, sizeof(*result)); > + result->f.attrs = MEMTXATTRS_CPU(env_cpu(env)); Ouch. This means that f.secure has been reset too, which would break Secure EL1 running under Secure EL2. I'll prepare a fix for 7.2... Anyway, Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~ > > ret = get_phys_addr_lpae(env, ptw, ipa, access_type, is_el0, result, fi); > fi->s2addr = ipa; > @@ -2872,7 +2873,7 @@ hwaddr arm_cpu_get_phys_page_attrs_debug(CPUState *cs, vaddr addr, > .in_secure = arm_is_secure(env), > .in_debug = true, > }; > - GetPhysAddrResult res = {}; > + GetPhysAddrResult res = { .f.attrs = MEMTXATTRS_CPU(cs) }; > ARMMMUFaultInfo fi = {}; > bool ret; > > diff --git a/target/arm/tlb_helper.c b/target/arm/tlb_helper.c > index 0f4f4fc809..5960269421 100644 > --- a/target/arm/tlb_helper.c > +++ b/target/arm/tlb_helper.c > @@ -208,7 +208,7 @@ bool arm_cpu_tlb_fill(CPUState *cs, vaddr address, int size, > bool probe, uintptr_t retaddr) > { > ARMCPU *cpu = ARM_CPU(cs); > - GetPhysAddrResult res = {}; > + GetPhysAddrResult res = { .f.attrs = MEMTXATTRS_CPU(cs) }; > ARMMMUFaultInfo local_fi, *fi; > int ret; >
On 11/12/22 04:25, Alex Bennée wrote: > @@ -2872,7 +2873,7 @@ hwaddr arm_cpu_get_phys_page_attrs_debug(CPUState *cs, vaddr addr, > .in_secure = arm_is_secure(env), > .in_debug = true, > }; > - GetPhysAddrResult res = {}; > + GetPhysAddrResult res = { .f.attrs = MEMTXATTRS_CPU(cs) }; > ARMMMUFaultInfo fi = {}; > bool ret; > > diff --git a/target/arm/tlb_helper.c b/target/arm/tlb_helper.c > index 0f4f4fc809..5960269421 100644 > --- a/target/arm/tlb_helper.c > +++ b/target/arm/tlb_helper.c > @@ -208,7 +208,7 @@ bool arm_cpu_tlb_fill(CPUState *cs, vaddr address, int size, > bool probe, uintptr_t retaddr) > { > ARMCPU *cpu = ARM_CPU(cs); > - GetPhysAddrResult res = {}; > + GetPhysAddrResult res = { .f.attrs = MEMTXATTRS_CPU(cs) }; Not the right level for these. Should be set in get_phys_addr_with_struct, alongside .secure right at the top of the function. r~
diff --git a/target/arm/ptw.c b/target/arm/ptw.c index 3745ac9723..4b6683f90d 100644 --- a/target/arm/ptw.c +++ b/target/arm/ptw.c @@ -2634,6 +2634,7 @@ static bool get_phys_addr_twostage(CPUARMState *env, S1Translate *ptw, s1_lgpgsz = result->f.lg_page_size; cacheattrs1 = result->cacheattrs; memset(result, 0, sizeof(*result)); + result->f.attrs = MEMTXATTRS_CPU(env_cpu(env)); ret = get_phys_addr_lpae(env, ptw, ipa, access_type, is_el0, result, fi); fi->s2addr = ipa; @@ -2872,7 +2873,7 @@ hwaddr arm_cpu_get_phys_page_attrs_debug(CPUState *cs, vaddr addr, .in_secure = arm_is_secure(env), .in_debug = true, }; - GetPhysAddrResult res = {}; + GetPhysAddrResult res = { .f.attrs = MEMTXATTRS_CPU(cs) }; ARMMMUFaultInfo fi = {}; bool ret; diff --git a/target/arm/tlb_helper.c b/target/arm/tlb_helper.c index 0f4f4fc809..5960269421 100644 --- a/target/arm/tlb_helper.c +++ b/target/arm/tlb_helper.c @@ -208,7 +208,7 @@ bool arm_cpu_tlb_fill(CPUState *cs, vaddr address, int size, bool probe, uintptr_t retaddr) { ARMCPU *cpu = ARM_CPU(cs); - GetPhysAddrResult res = {}; + GetPhysAddrResult res = { .f.attrs = MEMTXATTRS_CPU(cs) }; ARMMMUFaultInfo local_fi, *fi; int ret;
Both arm_cpu_tlb_fill (for normal IO) and arm_cpu_get_phys_page_attrs_debug (for debug access) come through get_phys_addr which is setting the other memory attributes for the transaction. As these are all by definition CPU accesses we can also set the requested_type/index as appropriate. We also have to handle where the attributes are totally reset if we call into get_phys_addr_twostage. Signed-off-by: Alex Bennée <alex.bennee@linaro.org> --- v3 - reword commit summary v5 - fix for new *result ABI - use MEMTXATTRS_CPU to fill in the initial values - also reset attrs in get_phys_addr_twostage --- target/arm/ptw.c | 3 ++- target/arm/tlb_helper.c | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-)