Re: [Xen-devel] [PATCH v2 1/3] xen/arm: fix nr_pdxs calculation

[Julien Grall <julien.grall@xxxxxxx>]

Hi Stefano,

On 5/8/19 11:47 PM, Stefano Stabellini wrote:
> pfn_to_pdx expects an address, not a size, as a parameter. It expects
> the end address, and the masks calculations compensate for any holes
> between start and end. Pass the end address to pfn_to_pdx. 
The wording is a bit difficult to read. Don't you miss a "So" or 
"Therefore" somewhere?

> Also remove
s/remove/substract/?

> from the result pfn_to_pdx(start_address) because we know that we
> don't need to cover any memory in the range 0-start in the frametable.
The only reason we can substract pfn_to_pdx(start_address) here is 
because we store the initial PDX in frametable_base_pdx. Without that, 
the frametable would need to cover the range 0 - start.
The code looks good to me.

> Remove the variable `nr_pages' because it is unused.
>
> Signed-off-by: Stefano Stabellini <stefanos@xxxxxxxxxx>
> CC: JBeulich@xxxxxxxx
> ---
> Changes in v2:
> - use mfn_to_pdx and maddr_to_mfn along with mfn_add()
> ---
>   xen/arch/arm/mm.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index 01ae2cccc0..58d71d3c23 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -874,8 +874,8 @@ void __init setup_xenheap_mappings(unsigned long base_mfn,
>   /* Map a frame table to cover physical addresses ps through pe */
>   void __init setup_frametable_mappings(paddr_t ps, paddr_t pe)
>   {
> -    unsigned long nr_pages = (pe - ps) >> PAGE_SHIFT;
> -    unsigned long nr_pdxs = pfn_to_pdx(nr_pages);
> +    unsigned long nr_pdxs = mfn_to_pdx(mfn_add(maddr_to_mfn(pe), -1)) -
> +                            mfn_to_pdx(maddr_to_mfn(ps)) + 1;
>       unsigned long frametable_size = nr_pdxs * sizeof(struct page_info);
>       mfn_t base_mfn;
>       const unsigned long mapping_size = frametable_size < MB(32) ? MB(2) : 
> MB(32);
Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel