Re: [Xen-devel] [PATCH for-next 1/9] xen/arm: Use mfn_to_pdx instead of pfn_to_pdx when possible

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 18 Feb 2019, Julien Grall wrote:
> mfn_to_pdx adds more safety than pfn_to_pdx. Replace all but on place in
> the Arm code to use the former.

This is good but maybe we can go even further.

You should also be able to replace one call site of pfn_to_pdx in
mfn_valid and the one in maddr_to_virt. Something like this:


diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h
index eafa26f..b3455ea 100644
--- a/xen/include/asm-arm/mm.h
+++ b/xen/include/asm-arm/mm.h
@@ -209,7 +209,7 @@ static inline void __iomem *ioremap_wc(paddr_t start, 
size_t len)
 /* XXX -- account for base */
 #define mfn_valid(mfn)        ({                                              \
     unsigned long __m_f_n = mfn_x(mfn);                                       \
-    likely(pfn_to_pdx(__m_f_n) >= frametable_base_pdx && 
__mfn_valid(__m_f_n)); \
+    likely(mfn_to_pdx(mfn) >= frametable_base_pdx && __mfn_valid(__m_f_n)); \
 })
 
 /* Convert between machine frame numbers and page-info structures. */
@@ -253,7 +253,7 @@ static inline void *maddr_to_virt(paddr_t ma)
 #else
 static inline void *maddr_to_virt(paddr_t ma)
 {
-    ASSERT(pfn_to_pdx(ma >> PAGE_SHIFT) < (DIRECTMAP_SIZE >> PAGE_SHIFT));
+    ASSERT(mfn_to_pdx(maddr_to_mfn(ma)) < (DIRECTMAP_SIZE >> PAGE_SHIFT));
     return (void *)(XENHEAP_VIRT_START -
                     mfn_to_maddr(xenheap_mfn_start) +
                     ((ma & ma_va_bottom_mask) |



> No functional changes.
> 
> Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
> ---
>  xen/arch/arm/mm.c        | 2 +-
>  xen/include/asm-arm/mm.h | 6 +++---
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index 01ae2cccc0..be5338bb4c 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -886,7 +886,7 @@ void __init setup_frametable_mappings(paddr_t ps, paddr_t 
> pe)
>      int i;
>  #endif
>  
> -    frametable_base_pdx = pfn_to_pdx(ps >> PAGE_SHIFT);
> +    frametable_base_pdx = mfn_to_pdx(maddr_to_mfn(ps));
>      /* Round up to 2M or 32M boundary, as appropriate. */
>      frametable_size = ROUNDUP(frametable_size, mapping_size);
>      base_mfn = alloc_boot_pages(frametable_size >> PAGE_SHIFT, 32<<(20-12));
> diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h
> index eafa26f56e..7b6aaf5e3f 100644
> --- a/xen/include/asm-arm/mm.h
> +++ b/xen/include/asm-arm/mm.h
> @@ -225,7 +225,7 @@ static inline void __iomem *ioremap_wc(paddr_t start, 
> size_t len)
>  /* Convert between frame number and address formats.  */
>  #define pfn_to_paddr(pfn) ((paddr_t)(pfn) << PAGE_SHIFT)
>  #define paddr_to_pfn(pa)  ((unsigned long)((pa) >> PAGE_SHIFT))
> -#define paddr_to_pdx(pa)    pfn_to_pdx(paddr_to_pfn(pa))
> +#define paddr_to_pdx(pa)    mfn_to_pdx(maddr_to_mfn(pa))
>  #define gfn_to_gaddr(gfn)   pfn_to_paddr(gfn_x(gfn))
>  #define gaddr_to_gfn(ga)    _gfn(paddr_to_pfn(ga))
>  #define mfn_to_maddr(mfn)   pfn_to_paddr(mfn_x(mfn))
> @@ -253,7 +253,7 @@ static inline void *maddr_to_virt(paddr_t ma)
>  #else
>  static inline void *maddr_to_virt(paddr_t ma)
>  {
> -    ASSERT(pfn_to_pdx(ma >> PAGE_SHIFT) < (DIRECTMAP_SIZE >> PAGE_SHIFT));
> +    ASSERT(mfn_to_pdx(maddr_to_mfn(ma)) < (DIRECTMAP_SIZE >> PAGE_SHIFT));
>      return (void *)(XENHEAP_VIRT_START -
>                      mfn_to_maddr(xenheap_mfn_start) +
>                      ((ma & ma_va_bottom_mask) |
> @@ -301,7 +301,7 @@ static inline struct page_info *virt_to_page(const void 
> *v)
>      ASSERT(va < xenheap_virt_end);
>  
>      pdx = (va - XENHEAP_VIRT_START) >> PAGE_SHIFT;
> -    pdx += pfn_to_pdx(mfn_x(xenheap_mfn_start));
> +    pdx += mfn_to_pdx(xenheap_mfn_start);
>      return frame_table + pdx - frametable_base_pdx;
>  }
>  
> -- 
> 2.11.0
> 

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