AP in armv7 short descriptor mode has 3 bits and also domain, which
makes it incompatible with other arm schemas.
To make it possible to share get_S1prot between armv8, armv7 long
format, armv7 short format and armv6 it's easier to make caller
decode AP.
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
---
target/arm/ptw.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index 9849949508..50eed0f811 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -1357,25 +1357,24 @@ static int get_S2prot(CPUARMState *env, int s2ap, int xn, bool s1_is_el0)
* @env: CPUARMState
* @mmu_idx: MMU index indicating required translation regime
* @is_aa64: TRUE if AArch64
- * @ap: The 2-bit simple AP (AP[2:1])
+ * @user_rw: Translated AP for user access
+ * @prot_rw: Translated AP for privileged access
* @xn: XN (execute-never) bit
* @pxn: PXN (privileged execute-never) bit
* @in_pa: The original input pa space
* @out_pa: The output pa space, modified by NSTable, NS, and NSE
*/
static int get_S1prot(CPUARMState *env, ARMMMUIdx mmu_idx, bool is_aa64,
- int ap, int xn, int pxn,
+ int user_rw, int prot_rw, int xn, int pxn,
ARMSecuritySpace in_pa, ARMSecuritySpace out_pa)
{
ARMCPU *cpu = env_archcpu(env);
bool is_user = regime_is_user(env, mmu_idx);
- int prot_rw, user_rw;
bool have_wxn;
int wxn = 0;
assert(!regime_is_stage2(mmu_idx));
- user_rw = simple_ap_to_rw_prot_is_user(ap, true);
if (is_user) {
prot_rw = user_rw;
} else {
@@ -1393,8 +1392,6 @@ static int get_S1prot(CPUARMState *env, ARMMMUIdx mmu_idx, bool is_aa64,
regime_is_pan(env, mmu_idx) &&
(regime_sctlr(env, mmu_idx) & SCTLR_EPAN) && !xn) {
prot_rw = 0;
- } else {
- prot_rw = simple_ap_to_rw_prot_is_user(ap, false);
}
}
@@ -2044,6 +2041,8 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
int nse, ns = extract32(attrs, 5, 1);
uint8_t attrindx;
uint64_t mair;
+ int ap_usr = simple_ap_to_rw_prot_is_user(ap, true);
+ int ap_priv = simple_ap_to_rw_prot_is_user(ap, false);
switch (out_space) {
case ARMSS_Root:
@@ -2114,7 +2113,7 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
* Note that we modified ptw->in_space earlier for NSTable, but
* result->f.attrs retains a copy of the original security space.
*/
- result->f.prot = get_S1prot(env, mmu_idx, aarch64, ap, xn, pxn,
+ result->f.prot = get_S1prot(env, mmu_idx, aarch64, ap_usr, ap_priv, xn, pxn,
result->f.attrs.space, out_space);
/* Index into MAIR registers for cache attributes */
--
2.46.0On Sun, 17 Nov 2024 at 13:50, Pavel Skripkin <paskripkin@gmail.com> wrote:
>
> AP in armv7 short descriptor mode has 3 bits and also domain, which
> makes it incompatible with other arm schemas.
>
> To make it possible to share get_S1prot between armv8, armv7 long
> format, armv7 short format and armv6 it's easier to make caller
> decode AP.
Yep, I agree that this is the best approach to sharing the
function between short and long desc.
> Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
> ---
> target/arm/ptw.c | 13 ++++++-------
> 1 file changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/target/arm/ptw.c b/target/arm/ptw.c
> index 9849949508..50eed0f811 100644
> --- a/target/arm/ptw.c
> +++ b/target/arm/ptw.c
> @@ -1357,25 +1357,24 @@ static int get_S2prot(CPUARMState *env, int s2ap, int xn, bool s1_is_el0)
> * @env: CPUARMState
> * @mmu_idx: MMU index indicating required translation regime
> * @is_aa64: TRUE if AArch64
> - * @ap: The 2-bit simple AP (AP[2:1])
> + * @user_rw: Translated AP for user access
> + * @prot_rw: Translated AP for privileged access
> * @xn: XN (execute-never) bit
> * @pxn: PXN (privileged execute-never) bit
> * @in_pa: The original input pa space
> * @out_pa: The output pa space, modified by NSTable, NS, and NSE
> */
> static int get_S1prot(CPUARMState *env, ARMMMUIdx mmu_idx, bool is_aa64,
> - int ap, int xn, int pxn,
> + int user_rw, int prot_rw, int xn, int pxn,
> ARMSecuritySpace in_pa, ARMSecuritySpace out_pa)
> {
> ARMCPU *cpu = env_archcpu(env);
> bool is_user = regime_is_user(env, mmu_idx);
> - int prot_rw, user_rw;
> bool have_wxn;
> int wxn = 0;
>
> assert(!regime_is_stage2(mmu_idx));
>
> - user_rw = simple_ap_to_rw_prot_is_user(ap, true);
> if (is_user) {
> prot_rw = user_rw;
> } else {
> @@ -1393,8 +1392,6 @@ static int get_S1prot(CPUARMState *env, ARMMMUIdx mmu_idx, bool is_aa64,
> regime_is_pan(env, mmu_idx) &&
> (regime_sctlr(env, mmu_idx) & SCTLR_EPAN) && !xn) {
> prot_rw = 0;
> - } else {
> - prot_rw = simple_ap_to_rw_prot_is_user(ap, false);
> }
> }
>
> @@ -2044,6 +2041,8 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
> int nse, ns = extract32(attrs, 5, 1);
> uint8_t attrindx;
> uint64_t mair;
> + int ap_usr = simple_ap_to_rw_prot_is_user(ap, true);
> + int ap_priv = simple_ap_to_rw_prot_is_user(ap, false);
Doing the calls here is too early, because after this point
there is a FEAT_NV case which may change the value of ap.
Also, could we give the variables the same name as the
get_S1prot() argument names, for consistency? (The return
values from simple_ap_to_rw_prot_is_user() are not ap bit
fields, they're rw indications, so the S1prot argument names
seem more appropriate here anyway.)
> switch (out_space) {
> case ARMSS_Root:
> @@ -2114,7 +2113,7 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
> * Note that we modified ptw->in_space earlier for NSTable, but
> * result->f.attrs retains a copy of the original security space.
> */
> - result->f.prot = get_S1prot(env, mmu_idx, aarch64, ap, xn, pxn,
> + result->f.prot = get_S1prot(env, mmu_idx, aarch64, ap_usr, ap_priv, xn, pxn,
> result->f.attrs.space, out_space);
>
> /* Index into MAIR registers for cache attributes */
thanks
-- PMM
© 2016 - 2026 Red Hat, Inc.