diff mbox series

[08/22] include/exec/memattrs: Add two bits of space to MemTxAttrs

Message ID 20230124000027.3565716-9-richard.henderson@linaro.org
State Superseded
Headers show
Series target/arm: Implement FEAT_RME | expand

Commit Message

Richard Henderson Jan. 24, 2023, midnight UTC
We will need 2 bits to represent ARMSecurityState.

Do not attempt to replace or widen secure, even though it
logically overlaps the new field -- there are uses within
e.g. hw/block/pflash_cfi01.c, which don't know anything
specific about ARM.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 include/exec/memattrs.h | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

Comments

Peter Maydell Feb. 7, 2023, 3:05 p.m. UTC | #1
On Tue, 24 Jan 2023 at 00:01, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> We will need 2 bits to represent ARMSecurityState.
>
> Do not attempt to replace or widen secure, even though it
> logically overlaps the new field -- there are uses within
> e.g. hw/block/pflash_cfi01.c, which don't know anything
> specific about ARM.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  include/exec/memattrs.h | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/include/exec/memattrs.h b/include/exec/memattrs.h
> index 9fb98bc1ef..d04170aa27 100644
> --- a/include/exec/memattrs.h
> +++ b/include/exec/memattrs.h
> @@ -29,10 +29,17 @@ typedef struct MemTxAttrs {
>       * "didn't specify" if necessary.
>       */
>      unsigned int unspecified:1;
> -    /* ARM/AMBA: TrustZone Secure access
> +    /*
> +     * ARM/AMBA: TrustZone Secure access
>       * x86: System Management Mode access
>       */
>      unsigned int secure:1;
> +    /*
> +     * ARM: ArmSecuritySpace.  This partially overlaps secure, but it is
> +     * easier to have both fields to assist code that does not understand
> +     * ARMv9 RME, or no specific knowledge of ARM at all (e.g. pflash).
> +     */
> +    unsigned int space:2;
>      /* Memory access is usermode (unprivileged) */
>      unsigned int user:1;

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

I guess we aren't so tight on bits in this struct as to
warrant keeping the extra RME info in a single bit (which
should in theory be possible).

thanks
-- PMM
Richard Henderson Feb. 8, 2023, 10:12 p.m. UTC | #2
On 2/7/23 05:05, Peter Maydell wrote:
> On Tue, 24 Jan 2023 at 00:01, Richard Henderson
> <richard.henderson@linaro.org> wrote:
>>
>> We will need 2 bits to represent ARMSecurityState.
>>
>> Do not attempt to replace or widen secure, even though it
>> logically overlaps the new field -- there are uses within
>> e.g. hw/block/pflash_cfi01.c, which don't know anything
>> specific about ARM.
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>   include/exec/memattrs.h | 9 ++++++++-
>>   1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/exec/memattrs.h b/include/exec/memattrs.h
>> index 9fb98bc1ef..d04170aa27 100644
>> --- a/include/exec/memattrs.h
>> +++ b/include/exec/memattrs.h
>> @@ -29,10 +29,17 @@ typedef struct MemTxAttrs {
>>        * "didn't specify" if necessary.
>>        */
>>       unsigned int unspecified:1;
>> -    /* ARM/AMBA: TrustZone Secure access
>> +    /*
>> +     * ARM/AMBA: TrustZone Secure access
>>        * x86: System Management Mode access
>>        */
>>       unsigned int secure:1;
>> +    /*
>> +     * ARM: ArmSecuritySpace.  This partially overlaps secure, but it is
>> +     * easier to have both fields to assist code that does not understand
>> +     * ARMv9 RME, or no specific knowledge of ARM at all (e.g. pflash).
>> +     */
>> +    unsigned int space:2;
>>       /* Memory access is usermode (unprivileged) */
>>       unsigned int user:1;
> 
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> 
> I guess we aren't so tight on bits in this struct as to
> warrant keeping the extra RME info in a single bit (which
> should in theory be possible).

Indeed not.  And the 3 target_* bits at the end are unused after recent changes.


r~
diff mbox series

Patch

diff --git a/include/exec/memattrs.h b/include/exec/memattrs.h
index 9fb98bc1ef..d04170aa27 100644
--- a/include/exec/memattrs.h
+++ b/include/exec/memattrs.h
@@ -29,10 +29,17 @@  typedef struct MemTxAttrs {
      * "didn't specify" if necessary.
      */
     unsigned int unspecified:1;
-    /* ARM/AMBA: TrustZone Secure access
+    /*
+     * ARM/AMBA: TrustZone Secure access
      * x86: System Management Mode access
      */
     unsigned int secure:1;
+    /*
+     * ARM: ArmSecuritySpace.  This partially overlaps secure, but it is
+     * easier to have both fields to assist code that does not understand
+     * ARMv9 RME, or no specific knowledge of ARM at all (e.g. pflash).
+     */
+    unsigned int space:2;
     /* Memory access is usermode (unprivileged) */
     unsigned int user:1;
     /*