diff mbox series

[22/62] target/arm: Add secure parameter to get_phys_addr_pmsav8

Message ID 20220703082419.770989-23-richard.henderson@linaro.org
State Superseded
Headers show
Series target/arm: Implement FEAT_HAFDBS | expand

Commit Message

Richard Henderson July 3, 2022, 8:23 a.m. UTC
Remove the use of regime_is_secure from get_phys_addr_pmsav8.
Since we already had a local variable named secure, use that.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/ptw.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

Comments

Alex Bennée Aug. 10, 2022, 1:16 p.m. UTC | #1
Richard Henderson <richard.henderson@linaro.org> writes:

> Remove the use of regime_is_secure from get_phys_addr_pmsav8.
> Since we already had a local variable named secure, use that.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/arm/ptw.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/target/arm/ptw.c b/target/arm/ptw.c
> index d424dec729..f7892a0c48 100644
> --- a/target/arm/ptw.c
> +++ b/target/arm/ptw.c
> @@ -1960,10 +1960,9 @@ void v8m_security_lookup(CPUARMState *env, uint32_t address,
>  
>  static bool get_phys_addr_pmsav8(CPUARMState *env, uint32_t address,
>                                   MMUAccessType access_type, ARMMMUIdx mmu_idx,
> -                                 GetPhysAddrResult *result,
> +                                 bool secure, GetPhysAddrResult
>  *result,

NIT: why not use is_secure like all the other functions (and reformat
the commit subject to match too).

Otherwise:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Richard Henderson Aug. 10, 2022, 3:33 p.m. UTC | #2
On 8/10/22 06:16, Alex Bennée wrote:
> 
> Richard Henderson <richard.henderson@linaro.org> writes:
> 
>> Remove the use of regime_is_secure from get_phys_addr_pmsav8.
>> Since we already had a local variable named secure, use that.
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>   target/arm/ptw.c | 5 ++---
>>   1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/target/arm/ptw.c b/target/arm/ptw.c
>> index d424dec729..f7892a0c48 100644
>> --- a/target/arm/ptw.c
>> +++ b/target/arm/ptw.c
>> @@ -1960,10 +1960,9 @@ void v8m_security_lookup(CPUARMState *env, uint32_t address,
>>   
>>   static bool get_phys_addr_pmsav8(CPUARMState *env, uint32_t address,
>>                                    MMUAccessType access_type, ARMMMUIdx mmu_idx,
>> -                                 GetPhysAddrResult *result,
>> +                                 bool secure, GetPhysAddrResult
>>   *result,
> 
> NIT: why not use is_secure like all the other functions (and reformat
> the commit subject to match too).

It's right there in the commit message -- there was an existing local variable.


r~

> 
> Otherwise:
> 
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
>
Alex Bennée Aug. 10, 2022, 6:46 p.m. UTC | #3
Richard Henderson <richard.henderson@linaro.org> writes:

> On 8/10/22 06:16, Alex Bennée wrote:
>> Richard Henderson <richard.henderson@linaro.org> writes:
>> 
>>> Remove the use of regime_is_secure from get_phys_addr_pmsav8.
>>> Since we already had a local variable named secure, use that.
>>>
>>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>>> ---
>>>   target/arm/ptw.c | 5 ++---
>>>   1 file changed, 2 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/target/arm/ptw.c b/target/arm/ptw.c
>>> index d424dec729..f7892a0c48 100644
>>> --- a/target/arm/ptw.c
>>> +++ b/target/arm/ptw.c
>>> @@ -1960,10 +1960,9 @@ void v8m_security_lookup(CPUARMState *env, uint32_t address,
>>>     static bool get_phys_addr_pmsav8(CPUARMState *env, uint32_t
>>> address,
>>>                                    MMUAccessType access_type, ARMMMUIdx mmu_idx,
>>> -                                 GetPhysAddrResult *result,
>>> +                                 bool secure, GetPhysAddrResult
>>>   *result,
>> NIT: why not use is_secure like all the other functions (and
>> reformat
>> the commit subject to match too).
>
> It's right there in the commit message -- there was an existing local
> variable.

doh - sorry so focused on the mechanics I missed the explanation!
>
>
> r~
>
>> Otherwise:
>> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
>>
diff mbox series

Patch

diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index d424dec729..f7892a0c48 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -1960,10 +1960,9 @@  void v8m_security_lookup(CPUARMState *env, uint32_t address,
 
 static bool get_phys_addr_pmsav8(CPUARMState *env, uint32_t address,
                                  MMUAccessType access_type, ARMMMUIdx mmu_idx,
-                                 GetPhysAddrResult *result,
+                                 bool secure, GetPhysAddrResult *result,
                                  ARMMMUFaultInfo *fi)
 {
-    uint32_t secure = regime_is_secure(env, mmu_idx);
     V8M_SAttributes sattrs = {};
     bool ret;
 
@@ -2408,7 +2407,7 @@  bool get_phys_addr(CPUARMState *env, target_ulong address,
         if (arm_feature(env, ARM_FEATURE_V8)) {
             /* PMSAv8 */
             ret = get_phys_addr_pmsav8(env, address, access_type, mmu_idx,
-                                       result, fi);
+                                       is_secure, result, fi);
         } else if (arm_feature(env, ARM_FEATURE_V7)) {
             /* PMSAv7 */
             ret = get_phys_addr_pmsav7(env, address, access_type, mmu_idx,