Re: [PATCH 14/18] target/arm: secure stage 2 translation regime

2021-01-11 Thread Richard Henderson 
On 12/18/20 12:37 AM, remi.denis.courm...@huawei.com wrote:
> @@ -11286,8 +11299,10 @@ static bool get_phys_addr_lpae(CPUARMState *env, 
> uint64_t address,
>  
>  ap = extract32(attrs, 4, 2);
>  
> -if (mmu_idx == ARMMMUIdx_Stage2) {
> -ns = true;
> +if (mmu_idx == ARMMMUIdx_Stage2 || mmu_idx == ARMMMUIdx_Stage2_S) {
> +if (mmu_idx == ARMMMUIdx_Stage2) {
> +ns = true;
> +}
>  xn = extract32(attrs, 11, 2);

Does this want an unconditional

  ns = mmu_idx == ARMMMUIdx_Stage2;

When can ns be true and mmu_idx == ARMMMUIdx_Stage2_S?

Otherwise,

Reviewed-by: Richard Henderson 


r~

Re: [PATCH 14/18] target/arm: secure stage 2 translation regime

2021-01-11 Thread Richard Henderson 
On 12/18/20 12:37 AM, remi.denis.courm...@huawei.com wrote:
> @@ -3586,10 +3586,10 @@ 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_SE10_1_PAN
> +mmu_idx = (secure ? ARMMMUIdx_Stage1_SE1_PAN
> : ARMMMUIdx_Stage1_E1_PAN);
>  } else {
> -mmu_idx = secure ? ARMMMUIdx_SE10_1 : ARMMMUIdx_Stage1_E1;
> +mmu_idx = secure ? ARMMMUIdx_Stage1_SE1 : 
> ARMMMUIdx_Stage1_E1;
>  }
>  break;

Was this a bug that we weren't treating SE10 properly vs two-stage lookup?  If
so, it warrants mentioning in the patch description.


r~