| Message ID | 20201102105802.39332-10-remi.denis.courmont@huawei.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ARM Secure EL2 extension | expand |
On 11/2/20 2:57 AM, remi.denis.courmont@huawei.com wrote: > From: Rémi Denis-Courmont <remi.denis.courmont@huawei.com> > > In the secure stage 2 translation regime, the VSTCR.SW and VTCR.NSW > bits can invert the secure flag for pagetable walks. This patchset > allows S1_ptw_translate() to change the non-secure bit. > > Signed-off-by: Rémi Denis-Courmont <remi.denis.courmont@huawei.com> > --- > target/arm/helper.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/target/arm/helper.c b/target/arm/helper.c > index 4c86e4f57c..7c70460e65 100644 > --- a/target/arm/helper.c > +++ b/target/arm/helper.c > @@ -10403,7 +10403,7 @@ static bool get_level1_table_address(CPUARMState *env, ARMMMUIdx mmu_idx, > > /* Translate a S1 pagetable walk through S2 if needed. */ > static hwaddr S1_ptw_translate(CPUARMState *env, ARMMMUIdx mmu_idx, > - hwaddr addr, MemTxAttrs txattrs, > + hwaddr addr, bool *is_secure, > ARMMMUFaultInfo *fi) > { > ARMMMUIdx s2_mmu_idx; > @@ -10415,6 +10415,9 @@ static hwaddr S1_ptw_translate(CPUARMState *env, ARMMMUIdx mmu_idx, > int s2prot; > int ret; > ARMCacheAttrs cacheattrs = {}; > + MemTxAttrs txattrs = {}; > + > + assert(!*is_secure); /* TODO: S-EL2 */ Are you sure that you don't want to pass in txattrs via pointer instead? This change by itself looks questionable. I guess I'll have to look forward to the other patch... r~
Le tiistaina 3. marraskuuta 2020, 21.54.48 EET Richard Henderson a écrit : > On 11/2/20 2:57 AM, remi.denis.courmont@huawei.com wrote: > > From: Rémi Denis-Courmont <remi.denis.courmont@huawei.com> > > > > In the secure stage 2 translation regime, the VSTCR.SW and VTCR.NSW > > bits can invert the secure flag for pagetable walks. This patchset > > allows S1_ptw_translate() to change the non-secure bit. > > > > Signed-off-by: Rémi Denis-Courmont <remi.denis.courmont@huawei.com> > > --- > > > > target/arm/helper.c | 9 ++++++--- > > 1 file changed, 6 insertions(+), 3 deletions(-) > > > > diff --git a/target/arm/helper.c b/target/arm/helper.c > > index 4c86e4f57c..7c70460e65 100644 > > --- a/target/arm/helper.c > > +++ b/target/arm/helper.c > > @@ -10403,7 +10403,7 @@ static bool get_level1_table_address(CPUARMState > > *env, ARMMMUIdx mmu_idx,> > > /* Translate a S1 pagetable walk through S2 if needed. */ > > static hwaddr S1_ptw_translate(CPUARMState *env, ARMMMUIdx mmu_idx, > > > > - hwaddr addr, MemTxAttrs txattrs, > > + hwaddr addr, bool *is_secure, > > > > ARMMMUFaultInfo *fi) > > > > { > > > > ARMMMUIdx s2_mmu_idx; > > > > @@ -10415,6 +10415,9 @@ static hwaddr S1_ptw_translate(CPUARMState *env, > > ARMMMUIdx mmu_idx,> > > int s2prot; > > int ret; > > ARMCacheAttrs cacheattrs = {}; > > > > + MemTxAttrs txattrs = {}; > > + > > + assert(!*is_secure); /* TODO: S-EL2 */ > > Are you sure that you don't want to pass in txattrs via pointer instead? That's possible too, and more like the existing code. Though I thought it clearer to pass only a pointer to the secure bit in/out, seen as that's the only in/out parameter. > This change by itself looks questionable. I guess I'll have to look > forward to the other patch... -- Rémi Denis-Courmont
diff --git a/target/arm/helper.c b/target/arm/helper.c index 4c86e4f57c..7c70460e65 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -10403,7 +10403,7 @@ static bool get_level1_table_address(CPUARMState *env, ARMMMUIdx mmu_idx, /* Translate a S1 pagetable walk through S2 if needed. */ static hwaddr S1_ptw_translate(CPUARMState *env, ARMMMUIdx mmu_idx, - hwaddr addr, MemTxAttrs txattrs, + hwaddr addr, bool *is_secure, ARMMMUFaultInfo *fi) { ARMMMUIdx s2_mmu_idx; @@ -10415,6 +10415,9 @@ static hwaddr S1_ptw_translate(CPUARMState *env, ARMMMUIdx mmu_idx, int s2prot; int ret; ARMCacheAttrs cacheattrs = {}; + MemTxAttrs txattrs = {}; + + assert(!*is_secure); /* TODO: S-EL2 */ ret = get_phys_addr_lpae(env, addr, MMU_DATA_LOAD, s2_mmu_idx, false, &s2pa, &txattrs, &s2prot, &s2size, fi, @@ -10453,9 +10456,9 @@ static uint32_t arm_ldl_ptw(CPUState *cs, hwaddr addr, bool is_secure, AddressSpace *as; uint32_t data; + addr = S1_ptw_translate(env, mmu_idx, addr, &is_secure, fi); attrs.secure = is_secure; as = arm_addressspace(cs, attrs); - addr = S1_ptw_translate(env, mmu_idx, addr, attrs, fi); if (fi->s1ptw) { return 0; } @@ -10482,9 +10485,9 @@ static uint64_t arm_ldq_ptw(CPUState *cs, hwaddr addr, bool is_secure, AddressSpace *as; uint64_t data; + addr = S1_ptw_translate(env, mmu_idx, addr, &is_secure, fi); attrs.secure = is_secure; as = arm_addressspace(cs, attrs); - addr = S1_ptw_translate(env, mmu_idx, addr, attrs, fi); if (fi->s1ptw) { return 0; }