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

Re: [Xen-devel] [PATCH v2 04/24] xen/arm: mm: Redefine mfn_to_virt to use typesafe

Hi Stefano,

On 09/16/2017 12:56 AM, Stefano Stabellini wrote:
On Tue, 12 Sep 2017, Julien Grall wrote:
This add a bit more safety in the memory subsystem code.

Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
  xen/arch/arm/mm.c | 16 +++++++++-------
  1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 965d0573a4..5716ef1123 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -47,6 +47,8 @@ struct domain *dom_xen, *dom_io, *dom_cow;
  /* Override macros from asm/page.h to make them work with mfn_t */
  #undef virt_to_mfn
  #define virt_to_mfn(va) _mfn(__virt_to_mfn(va))
+#undef mfn_to_virt
+#define mfn_to_virt(mfn) __mfn_to_virt(mfn_x(mfn))
/* Static start-of-day pagetables that we use before the allocators
   * are up. These are used by all CPUs during bringup before switching
@@ -837,7 +839,7 @@ void __init setup_xenheap_mappings(unsigned long base_mfn,
       * Virtual address aligned to previous 1GB to match physical
       * address alignment done above.
-    vaddr = (vaddr_t)mfn_to_virt(base_mfn) & FIRST_MASK;
+    vaddr = (vaddr_t)__mfn_to_virt(base_mfn) & FIRST_MASK;

Don't you think it would be better to do mfn_to_virt(_mfn(base_mfn)) in
this patch? This is just bike-shedding, but I think it would be more
obviously consistent. Other than that, it looks good.

Well, last time I used mfn_x/_mfn in similar condition, you requested to use the __* version (see [1]).

I really don't mind which one to use. But we should stay consistent with the macros to use for non-typesafe version.


[1] http://patches.linaro.org/patch/105386/

Julien Grall

Xen-devel mailing list



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