Hi Jan,
On 03/02/2018 04:08 PM, Jan Beulich wrote:
On 21.02.18 at 15:02, <julien.gr...@arm.com> wrote:
--- 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
- */
Why does this comment go away?
From an earlier review, Andrew said:
"If you're making this change, please take out the Descriptor Tables
comment like you do with I/O below, because the entire file is dedicated
to descriptor table support and it will save me one item on a cleanup
patch :)."
The descriptor one got remove by 634afe43ac "x86/pv: Rename
invalidate_shadow_ldt() to pv_destroy_ldt()". So it is not part of this
patch anymore.
@@ -478,10 +478,10 @@ extern paddr_t mem_hotplug;
#define SHARED_M2P(_e) ((_e) == SHARED_M2P_ENTRY)
#define compat_machine_to_phys_mapping ((unsigned int
*)RDWR_COMPAT_MPT_VIRT_START)
-#define _set_gpfn_from_mfn(mfn, pfn) ({ \
- struct domain *d = page_get_owner(__mfn_to_page(mfn)); \
- unsigned long entry = (d && (d == dom_cow)) ? \
- SHARED_M2P_ENTRY : (pfn); \
+#define _set_gpfn_from_mfn(mfn, pfn) ({ \
+ struct domain *d = page_get_owner(mfn_to_page(_mfn(mfn))); \
+ unsigned long entry = (d && (d == dom_cow)) ? \
+ SHARED_M2P_ENTRY : (pfn); \
Please don't break the alignment of the backslashes here. It also looks
like three of the four lines could be left alone altogether.
I am not sure why I modified the 3 other lines. I fixed it.
@@ -157,10 +157,10 @@ static inline l4_pgentry_t l4e_from_paddr(paddr_t pa,
unsigned int flags)
#define l4e_from_intpte(intpte) ((l4_pgentry_t) { (intpte_t)(intpte) })
/* Construct a pte from a page pointer and access flags. */
-#define l1e_from_page(page, flags) l1e_from_pfn(__page_to_mfn(page), (flags))
-#define l2e_from_page(page, flags) l2e_from_pfn(__page_to_mfn(page), (flags))
-#define l3e_from_page(page, flags) l3e_from_pfn(__page_to_mfn(page), (flags))
-#define l4e_from_page(page, flags) l4e_from_pfn(__page_to_mfn(page), (flags))
+#define l1e_from_page(page, flags) l1e_from_mfn(page_to_mfn(page), (flags))
+#define l2e_from_page(page, flags) l2e_from_mfn(page_to_mfn(page), (flags))
+#define l3e_from_page(page, flags) l3e_from_mfn(page_to_mfn(page), (flags))
+#define l4e_from_page(page, flags) l4e_from_mfn(page_to_mfn(page), (flags))
Would again have been nice if you got rid of the extra parentheses
here at the same time.
I admit, I don't spend my time trying to find the possible cleanup in
the x86 code. I just do mechanical change and when I get bored I do a
bit more.
@@ -240,12 +240,12 @@ void copy_page_sse2(void *, const void *);
#define __mfn_to_virt(mfn) (maddr_to_virt((paddr_t)(mfn) << PAGE_SHIFT))
/* Convert between machine frame numbers and page-info structures. */
-#define __mfn_to_page(mfn) (frame_table + pfn_to_pdx(mfn))
-#define __page_to_mfn(pg) pdx_to_pfn((unsigned long)((pg) - frame_table))
+#define mfn_to_page(mfn) (frame_table + mfn_to_pdx(mfn))
+#define page_to_mfn(pg) pdx_to_mfn((unsigned long)((pg) - frame_table))
/* Convert between machine addresses and page-info structures. */
-#define __maddr_to_page(ma) __mfn_to_page((ma) >> PAGE_SHIFT)
-#define __page_to_maddr(pg) ((paddr_t)__page_to_mfn(pg) << PAGE_SHIFT)
+#define __maddr_to_page(ma) mfn_to_page(maddr_to_mfn(ma))
+#define __page_to_maddr(pg) (mfn_to_maddr(page_to_mfn(pg)))
Same here.
With at least the first two items taken care of, relevant x86 pieces
Acked-by: Jan Beulich <jbeul...@suse.com>
I don't plan to address the first one as Andrew were happy with it.
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel