Re: [Xen-devel] [PATCH for-next 4/9] xen/grant-table: Make arch specific macros typesafe

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 18.02.19 at 12:35, <julien.grall@xxxxxxx> wrote:
> --- a/xen/include/asm-arm/grant_table.h
> +++ b/xen/include/asm-arm/grant_table.h
> @@ -67,15 +67,15 @@ void gnttab_mark_dirty(struct domain *d, mfn_t mfn);
>      } while ( 0 )
>  
>  #define gnttab_get_frame_gfn(gt, st, idx) ({                             \
> -   _gfn((st) ? gnttab_status_gmfn(NULL, gt, idx)                         \
> -             : gnttab_shared_gmfn(NULL, gt, idx));                       \
> +   (st) ? gnttab_status_gfn(NULL, gt, idx)                               \
> +             : gnttab_shared_gfn(NULL, gt, idx);                         \

Please also adjust indentation of the latter line.

> --- a/xen/include/asm-x86/grant_table.h
> +++ b/xen/include/asm-x86/grant_table.h
> @@ -43,24 +43,20 @@ static inline int replace_grant_host_mapping(uint64_t 
> addr, mfn_t frame,
>  #define gnttab_destroy_arch(gt) do {} while ( 0 )
>  #define gnttab_set_frame_gfn(gt, st, idx, gfn) do {} while ( 0 )
>  #define gnttab_get_frame_gfn(gt, st, idx) ({                             \
> -    unsigned long mfn_ = (st) ? gnttab_status_mfn(gt, idx)               \
> -                              : gnttab_shared_mfn(gt, idx);              \
> -    unsigned long gpfn_ = get_gpfn_from_mfn(mfn_);                       \
> +    mfn_t mfn_ = (st) ? gnttab_status_mfn(gt, idx)                       \
> +                      : gnttab_shared_mfn(gt, idx);                      \
> +    unsigned long gpfn_ = get_gpfn_from_mfn(mfn_x(mfn_));                \
>      VALID_M2P(gpfn_) ? _gfn(gpfn_) : INVALID_GFN;                        \
>  })
>  
> -#define gnttab_shared_mfn(t, i)                         \
> -    ((virt_to_maddr((t)->shared_raw[i]) >> PAGE_SHIFT))
> +#define gnttab_shared_mfn(t, i) _mfn(__virt_to_mfn((t)->shared_raw[i]))
>  
> -#define gnttab_shared_gmfn(d, t, i)                     \
> -    (mfn_to_gmfn(d, gnttab_shared_mfn(t, i)))
> +#define gnttab_shared_gfn(d, t, i) _gfn(mfn_to_gfn(d, gnttab_shared_mfn(t, 
> i)))

This and ...

> +#define gnttab_status_mfn(t, i) _mfn(__virt_to_mfn((t)->status[i]))
>  
> -#define gnttab_status_mfn(t, i)                         \
> -    ((virt_to_maddr((t)->status[i]) >> PAGE_SHIFT))
> -
> -#define gnttab_status_gmfn(d, t, i)                     \
> -    (mfn_to_gmfn(d, gnttab_status_mfn(t, i)))
> +#define gnttab_status_gfn(d, t, i) \
> +    _gfn(mfn_to_gfn(d, gnttab_status_mfn(t, i)))

... this would already benefit from mfn_to_gfn() returning gfn_t.
Either way non-Arm parts
Acked-by: Jan Beulich <jbeulich@xxxxxxxx>

Jan



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