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

[Sergej Proskurin <proskurin@xxxxxxxxxxxxx>]

Hi Julien,

[...]
 
>>>> +
>>>> +    /*
>>>> +     * As we have considered up to 2 MSBs of the GVA for mapping the
>>>> first
>>>> +     * level translation table, we need to make sure that we limit
>>>> the table
>>>> +     * offset that is is indexed by GVA<31-n:20> to max 10 bits to
>>>> avoid
>>>> +     * exceeding the page size limit.
>>>> +     */
>>>> +    mask = ((1ULL << 10) - 1);
>>>> +    pte = table[offsets[level] & mask];
>>>
>>> This looks quite complex for just reading a pte. I think it would be
>>> worth to leverage the vgic_guest_access_memory for that (same in LPAE).
>>> It would also add safety if the offsets the table is mistakenly
>>> computed
>>> (from the current code, I can't convince myself the offset will always
>>> be right).
>>
>> As far as I understand, we would still need to use the same offsets even
>> if we used vgic_access_guest_memory. Also, the only significant
>> difference between my implementation and vgic_access_guest_memory is
>> that gic_access_guest_memory checks whether we write over page
>> boundaries, which is quite hard to achieve as the offsets are limited in
>> number so that they don't cross page boundaries.
>
> +        /* Make sure that we consider the bits ttbr<12:14-n> if n >
> 2. */
> +        mask = ((1ULL << 12) - 1) & ~((1ULL << (14 - n)) - 1);
> +        table = (short_desc_t *)((unsigned long)table | (unsigned
> long)(ttbr & mask));
>
> [...]
>
> +    mask = ((1ULL << 10) - 1);
> +    pte = table[offsets[level] & mask];
>
> Are you able to prove me this will never cross a page boundary?
> Personally, when I read that I cannot convince myself that this will
> never cross happen. This is guest memory mapped, so if there is any
> coding error, you may access unmapped memory and then DOS Xen. Not
> very nice :).
>
> We have a function that read/write into guest memory with all safety
> check associated to it. We should take advantage of any helpers that
> will mitigate any potential miscalculations as you would just the
> wrong IPA. It is better than a DOS and also avoid open-coding so it is
> much easier to update any change in the way we access the guest memory.

Fair enough. I agree, this would indeed make things easier as we would
need to incorporate potential changes to guest memory access logic only
in one place. I will incorporate your suggestions into my code before my
next patch submission. Thank you.

Cheers,
~Sergej


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