Re: [Xen-devel] [PATCH v4 7/9] arm/mem_access: Add long-descriptor based gpt

[Sergej Proskurin <proskurin@xxxxxxxxxxxxx>]

Hi Julien,

[...]

>> +static bool get_ttbr_and_gran_64bit(uint64_t *ttbr, unsigned int *gran,
>> +                                    register_t tcr, enum active_ttbr
>> ttbrx)
>> +{
>> +    bool disabled;
>> +
>> +    if ( ttbrx == TTBR0_ACTIVE )
>> +    {
>> +        /* Normalize granule size. */
>> +        switch ( tcr & TCR_TG0_MASK )
>> +        {
>> +        case TCR_TG0_16K:
>> +            *gran = GRANULE_SIZE_INDEX_16K;
>> +            break;
>> +        case TCR_TG0_64K:
>> +            *gran = GRANULE_SIZE_INDEX_64K;
>> +            break;
>> +        default:
> 
> Please explain why you use 4K by default.
> 

Fair question. According to ARM DDI 0487B.a D7-2487, if the
TCR_EL1.{TG0|TG1} holds a reserved value, it is implementation defined
how the value is interpreted by the underlying hardware. My
implementation in v4 strongly followed the pseudo-code in ARM DDI
0487B.a, which suggested to use fall back to 4K by default.

However, agree with you at this point. My next patch series explicitly
checks if 4K has to be set or not and returns an error on reserved
values as we cannot be know how the hardware behaves on reserved values.

[...]

>> +
>> +    /*
>> +     * According to to ARM DDI 0487B.a J1-5927, we return an error if
>> the found
>> +     * PTE is invalid or holds a reserved entry (PTE<1:0> == x0)) or
>> if the PTE
>> +     * maps a memory block at level 3 (PTE<1:0> == 01).
>> +     */
>> +    if ( !lpae_valid(pte) || ((level == 3) && lpae_mapping(pte)) )
> 
> If you look at the comment on top of lpae_mapping, it should only be
> used for L0..L2 ptes. So why are you using it on L3 ptes?
> 

Yes, I saw the comment. Yet, I would like to check exactly for this
mapping. If the mapping would as in the check above, it would be an
error, which is treated accordingly. In v3, you have suggested to look
at the the lpae_is_superpage macro, which, in my opinion, is not the
right construct to use at this point. If you should not like this check,
I could fall back to my previous implementation:

if ( !p2m_valid(pte) || ((level == 3) && !p2m_table(pte)) )
    return -EFAULT;


Thank you,
~Sergej

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel