Hi Paul,
On 06/10/17 10:11, Paul Durrant wrote:
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 0410b1e86b..1e7a0c6c40 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -38,12 +38,6 @@ static unsigned int __read_mostly max_vmid =
MAX_VMID_8_BIT;
#define P2M_ROOT_PAGES (1<<P2M_ROOT_ORDER)
-/* Override macros from asm/mm.h to make them work with mfn_t */
-#undef mfn_to_page
-#define mfn_to_page(mfn) __mfn_to_page(mfn_x(mfn))
-#undef page_to_mfn
-#define page_to_mfn(pg) _mfn(__page_to_mfn(pg))
-
unsigned int __read_mostly p2m_ipa_bits;
/* Helpers to lookup the properties of each level */
@@ -98,7 +92,7 @@ void dump_p2m_lookup(struct domain *d, paddr_t
addr)
printk("dom%d IPA 0x%"PRIpaddr"\n", d->domain_id, addr);
printk("P2M @ %p mfn:0x%lx\n",
- p2m->root, __page_to_mfn(p2m->root));
+ p2m->root, mfn_x(page_to_mfn(p2m->root)));
The format specifier should really be using PRI_mfn now. Same goes for others
below.
Similarly we could do much more clean-up in each chunk. So where do I
stop? That's why I wrote down in this comment I will not handle all the
clean-up...
diff --git a/xen/arch/x86/pv/descriptor-tables.c
b/xen/arch/x86/pv/descriptor-tables.c
index 81973af124..f2b20f9910 100644
--- a/xen/arch/x86/pv/descriptor-tables.c
+++ b/xen/arch/x86/pv/descriptor-tables.c
@@ -25,16 +25,6 @@
#include <asm/p2m.h>
#include <asm/pv/mm.h>
-/* Override macros from asm/page.h to make them work with mfn_t */
-#undef mfn_to_page
-#define mfn_to_page(mfn) __mfn_to_page(mfn_x(mfn))
-#undef page_to_mfn
-#define page_to_mfn(pg) _mfn(__page_to_mfn(pg))
-
-/*******************
- * Descriptor Tables
- */
-
Is the comment wrong?
[...]
diff --git a/xen/arch/x86/pv/emul-priv-op.c b/xen/arch/x86/pv/emul-priv-
op.c
index dd90713acf..9ccbd021ef 100644
--- a/xen/arch/x86/pv/emul-priv-op.c
+++ b/xen/arch/x86/pv/emul-priv-op.c
@@ -43,16 +43,6 @@
#include "emulate.h"
#include "mm.h"
-/* Override macros from asm/page.h to make them work with mfn_t */
-#undef mfn_to_page
-#define mfn_to_page(mfn) __mfn_to_page(mfn_x(mfn))
-#undef page_to_mfn
-#define page_to_mfn(pg) _mfn(__page_to_mfn(pg))
-
-/***********************
- * I/O emulation support
- */
-
What's wrong with the comment?
The file is dedicated to I/O emulation support as said in the header and
the name. I can understand why it was there given there was macros
defined not related to I/O. Now they are dropped, why would you need a
comment to separate includes and the code?
[...]
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 86506f3747..b85394d1f9 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -811,7 +811,7 @@ int wrmsr_hypervisor_regs(uint32_t idx, uint64_t val)
gdprintk(XENLOG_WARNING,
"Bad GMFN %lx (MFN %lx) to MSR %08x\n",
- gmfn, page ? page_to_mfn(page) : -1UL, base);
+ gmfn, page ? mfn_x(page_to_mfn(page)) : -1UL, base);
Would this not be better as mfn_x(page ? page_to_mfn(page) : INVALID_MFN), as
you have done elsewhere?
See above.
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
