Re: [XEN v5 01/10] xen/arm: domain_build: Track unallocated pages using the frame number

[Julien Grall <julien@xxxxxxx>]

Hi,

On 19/04/2023 13:05, Michal Orzel wrote:
> On 13/04/2023 19:37, Ayan Kumar Halder wrote:
> > rangeset_{xxx}_range() functions are invoked with 'start' and 'size' as
> > arguments which are either 'uint64_t' or 'paddr_t'. However, the function
> > accepts 'unsigned long' for 'start' and 'size'. 'unsigned long' is 32 bits for
> > Arm32. Thus, there is an implicit downcasting from 'uint64_t'/'paddr_t' to
> > 'unsigned long' when invoking rangeset_{xxx}_range().
> >
> > So, it may seem there is a possibility of lose of data due to truncation.
> >
> > In reality, 'start' and 'size' are always page aligned. And Arm32 currently
> > supports 40 bits as the width of physical address.
> > So if the addresses are page aligned, the last 12 bits contain zeroes.
> > Thus, we could instead pass page frame number which will contain 28 bits (40-12
> > on Arm32) and this can be represented using 'unsigned long'.
> >
> > On Arm64, this change will not induce any adverse side effect as the width of
> > physical address is 48 bits.
> NIT: This reads as const, so it would be better to write: "as the max supported 
> width of ..."
>
> Thus, the width of 'gfn' (ie 48 - 12 = 36) can be
> > represented using 'unsigned long' (which is 64 bits wide).
> >
> > Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@xxxxxxx>
> With or without (after all this is just a commit msg):
It is always good to have accurate commit message for the future reader.

> Reviewed-by: Michal Orzel <michal.orzel@xxxxxxx>
Acked-by: Julien Grall <jgrall@xxxxxxxxxx>

--
Julien Grall