Re: [Qemu-devel] [PATCH 04/11] target/arm: Record the GP bit for a page in MemTxAttrs

[Richard Henderson]

On 1/22/19 5:26 AM, Peter Maydell wrote:
> On Thu, 10 Jan 2019 at 12:17, Richard Henderson
> <address@hidden> wrote:
>>
>> This isn't really a transaction attribute, but that's the most
>> convenient place to hold a random bit of information within the
>> softmmu tlb.
>>
>> Signed-off-by: Richard Henderson <address@hidden>
>> ---
>>  include/exec/memattrs.h | 2 ++
>>  target/arm/helper.c     | 6 ++++++
>>  2 files changed, 8 insertions(+)
>>
>> diff --git a/include/exec/memattrs.h b/include/exec/memattrs.h
>> index d4a1642098..39d61188e1 100644
>> --- a/include/exec/memattrs.h
>> +++ b/include/exec/memattrs.h
>> @@ -35,6 +35,8 @@ typedef struct MemTxAttrs {
>>      unsigned int secure:1;
>>      /* Memory access is usermode (unprivileged) */
>>      unsigned int user:1;
>> +    /* Page is marked as "guarded" */
>> +    unsigned int guarded:1;
> 
> Given that this isn't a real transaction attribute in the traditional
> sense, and it's pretty Arm-specific, I think we could do with a
> more expansive comment than this...

I have split this out to a separate patch, rearranged this to
target_tlb_bit[0-2], with a large block comment.  We will need some more of
these bits for for system mode v8.5-MemTag anyway.


>> +        guarded |= extract64(descriptor, 50, 1);  /* GP */
> 
> Do we need to do the logical-OR here? Since this is a
> block/page entry bit with no similar bit in the table
> descriptors, there's no merging to be done (ie we
> only execute this code once and 'guarded' will always
> be 'false' before execution of the |=.)

The document that I have has exactly one sentence about this, and does not
specify whether the bit is akin to the page table attributes (which appear at
every table level) or not.

As written above, this will execute more than once.


r~