Re: [Xen-devel] [PATCH v5 03/12] arm/mem_access: Add defines supporting PTs with varying page sizes

[Sergej Proskurin <proskurin@xxxxxxxxxxxxx>]

Hi Julien,

On 07/04/2017 06:15 PM, Julien Grall wrote:
> Hi Sergej,
> 

[...]

>> +
>> +#define GUEST_TABLE_OFFSET(offs, gran)          ((paddr_t)(offs) &
>> lpae_entry_mask(gran))
>> +#define
>> GUEST_TABLE_OFFSET_HELPERS(gran)                                             
>>   
>> \
>> +static inline vaddr_t third_guest_table_offset_##gran##K(vaddr_t
>> gva)                   \
> 
> Sorry I haven't spot it before. This is not going to work properly on
> 32-bit if you use vaddr_t. Indeed, input for stage-2 page-table (i.e
> IPA) will be 40-bit. But vaddr_t is 32-bit. So you to use paddr_t here
> and in all the helpers below.
> 

I agree that IPAs won't work properly on AArch32. However, we don't walk
the second stage translation tables with the introduced code (yet?). In
fact, second stage translation walks in software are not supported at
the moment. I understand why you would think in this direction, with
ARM's nested virtualization support coming up, where we might need to
walk the second stage translation tables in sw. Yet, with the current
implementation, we work on on GVAs (not IPAs) and hence the vaddr_t
should not present an issue (except that the now missing CONFIG_ARM_64
#ifdef's in the long-descriptor translation table walk create compile
issues as we need to support both different page granularities and
zeroeth-level offsets which work on gva's > 32bit on AArch64).

If you wish to see the implementation extended in the future to support
walking the 2nd stage address translation, then I will gladly change
vaddr_t to paddr_t.

Cheers,
~Sergej

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