Re: [Xen-devel] [PATCH v2 04/24] xen/arm: mm: Redefine mfn_to_virt to use typesafe

[Julien Grall <julien.grall@xxxxxxx>]

Hi Stefano,

On 09/16/2017 12:56 AM, Stefano Stabellini wrote:
> On Tue, 12 Sep 2017, Julien Grall wrote:
> > This add a bit more safety in the memory subsystem code.
> >
> > Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
> > ---
> >   xen/arch/arm/mm.c | 16 +++++++++-------
> >   1 file changed, 9 insertions(+), 7 deletions(-)
> >
> > diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> > index 965d0573a4..5716ef1123 100644
> > --- a/xen/arch/arm/mm.c
> > +++ b/xen/arch/arm/mm.c
> > @@ -47,6 +47,8 @@ struct domain *dom_xen, *dom_io, *dom_cow;
> >   /* 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 mfn_to_virt
> > +#define mfn_to_virt(mfn) __mfn_to_virt(mfn_x(mfn))
> >   
> >   /* Static start-of-day pagetables that we use before the allocators
> >    * are up. These are used by all CPUs during bringup before switching
> > @@ -837,7 +839,7 @@ void __init setup_xenheap_mappings(unsigned long base_mfn,
> >        * Virtual address aligned to previous 1GB to match physical
> >        * address alignment done above.
> >        */
> > -    vaddr = (vaddr_t)mfn_to_virt(base_mfn) & FIRST_MASK;
> > +    vaddr = (vaddr_t)__mfn_to_virt(base_mfn) & FIRST_MASK;
> Don't you think it would be better to do mfn_to_virt(_mfn(base_mfn)) in
> this patch? This is just bike-shedding, but I think it would be more
> obviously consistent. Other than that, it looks good.
Well, last time I used mfn_x/_mfn in similar condition, you requested to 
use the __* version (see [1]).
I really don't mind which one to use. But we should stay consistent with 
the macros to use for non-typesafe version.
Cheers,

[1] http://patches.linaro.org/patch/105386/

--
Julien Grall

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