Re: [Xen-devel] [PATCH v2 9/9] xen: Convert __page_to_mfn and __mfn_to_page to use typesafe MFN

[Julien Grall <julien.grall@xxxxxxx>]

Hi Jan,

On 09/10/17 12:42, Jan Beulich wrote:
> > > > On 05.10.17 at 19:42, <julien.grall@xxxxxxxxxx> wrote:
> > --- a/xen/arch/arm/domain_build.c
> > +++ b/xen/arch/arm/domain_build.c
> > @@ -50,8 +50,6 @@ struct map_range_data
> >   /* 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 page_to_mfn
> > -#define page_to_mfn(pg) _mfn(__page_to_mfn(pg))
> With the patch dropping (I assume) all overrides of this kind, what
> is the difference between the double-underscore-prefixed versions
> of the two constructs you convert here and the plain ones? If
> there's none (which I think is what the result here is meant to be),
> then ideally the patch would drop the former altogether. In case
> this means touching a lot more code, then at least I'd expect you
> to convert all instances you touch anyway, and that you in
> particular don't introduce any new ones.
>
> But wait - the patch even introduces new overrides (doing the
> inverse). What's the deal here? If that's again to limit patch size,
> then I'd still prefer the global aliases to go away, and local (per
> file) aliases to be retained as needed.
It introduces new overrides because some of the code is not trivial to 
convert to use mfn_t. It needs more effort to see whether it is worth it 
to convert them and I think is out of scope of this series.
This series is meant to reduce the number of place we override 
page_to_mfn to an handful numbers whilst still enforcing the use of 
mfn_t by default.
But I am not entirely sure what you are suggesting here. Are you saying 
we could define page_to_mfn/mfn_to_page on every file?
Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel