Re: [PATCH for-5.1] target/arm: Always pass cacheattr in S1_ptw_translate

[Jan Kiszka]

On 21.07.20 18:35, Richard Henderson wrote:
> When we changed the interface of get_phys_addr_lpae to require
> the cacheattr parameter, this spot was missed.  The compiler is
> unable to detect the use of NULL vs the nonnull attribute here.
>
> Fixes: 7e98e21c098
> Reported-by: Jan Kiszka <jan.kiszka@siemens.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/arm/helper.c | 19 ++++++-------------
>   1 file changed, 6 insertions(+), 13 deletions(-)
>
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index c69a2baf1d..8ef0fb478f 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -10204,21 +10204,11 @@ static hwaddr S1_ptw_translate(CPUARMState *env, 
> ARMMMUIdx mmu_idx,
>           int s2prot;
>           int ret;
>           ARMCacheAttrs cacheattrs = {};
> -        ARMCacheAttrs *pcacheattrs = NULL;
> -
> -        if (env->cp15.hcr_el2 & HCR_PTW) {
> -            /*
> -             * PTW means we must fault if this S1 walk touches S2 Device
> -             * memory; otherwise we don't care about the attributes and can
> -             * save the S2 translation the effort of computing them.
> -             */
> -            pcacheattrs = &cacheattrs;
> -        }
>   
>           ret = get_phys_addr_lpae(env, addr, MMU_DATA_LOAD, ARMMMUIdx_Stage2,
>                                    false,
>                                    &s2pa, &txattrs, &s2prot, &s2size, fi,
> -                                 pcacheattrs);
> +                                 &cacheattrs);
>           if (ret) {
>               assert(fi->type != ARMFault_None);
>               fi->s2addr = addr;
> @@ -10226,8 +10216,11 @@ static hwaddr S1_ptw_translate(CPUARMState *env, 
> ARMMMUIdx mmu_idx,
>               fi->s1ptw = true;
>               return ~0;
>           }
> -        if (pcacheattrs && (pcacheattrs->attrs & 0xf0) == 0) {
> -            /* Access was to Device memory: generate Permission fault */
> +        if ((env->cp15.hcr_el2 & HCR_PTW) && (cacheattrs.attrs & 0xf0) == 0) {
> +            /*
> +             * PTW set and S1 walk touched S2 Device memory:
> +             * generate Permission fault.
> +             */
>               fi->type = ARMFault_Permission;
>               fi->s2addr = addr;
>               fi->stage2 = true;

Jup:

Tested-by: Jan Kiszka <jan.kiskza@siemens.com>

Thanks,
Jan

--
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux