| Message ID | 20190508224727.11549-1-sstabellini@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2,1/3] xen/arm: fix nr_pdxs calculation | expand |
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@xilinx.com> > CC: JBeulich@suse.com > --- > 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,
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);
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. Also remove 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. Remove the variable `nr_pages' because it is unused. Signed-off-by: Stefano Stabellini <stefanos@xilinx.com> CC: JBeulich@suse.com --- 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(-)