diff mbox series

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

Message ID 20200721163528.80080-1-richard.henderson@linaro.org
State Superseded
Headers show
Series [for-5.1] target/arm: Always pass cacheattr in S1_ptw_translate | expand

Commit Message

Richard Henderson July 21, 2020, 4:35 p.m. UTC
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(-)

-- 
2.25.1

Comments

Jan Kiszka July 21, 2020, 4:48 p.m. UTC | #1
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
Peter Maydell July 24, 2020, 1:22 p.m. UTC | #2
On Tue, 21 Jul 2020 at 17:35, Richard Henderson
<richard.henderson@linaro.org> 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(-)




Applied to target-arm.next, thanks.

-- PMM
diff mbox series

Patch

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;