[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



Hi,

On 4/15/19 10:55 PM, Stefano Stabellini wrote:
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)); \

This is quite undesirable, you will end up to evaluate mfn twice here.

The other solution is to turn _m_f_n to an mfn_t but then it does make much difference as you would need to use a mfn_x(mfn) in the code.

  })
/* 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) |


I fail to see what this chunk adds compare to the existing one...

@@ -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) |
... here.

Cheers,

--
Julien Grall

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.