Re: [Xen-devel] [PATCH v4 16/16] xen: Convert page_to_mfn and mfn_to_page to use typesafe MFN

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 21.02.18 at 15:02, <julien.grall@xxxxxxx> 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?

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

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

> @@ -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 <jbeulich@xxxxxxxx>

Jan


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