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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Sat, 16 Sep 2017, Julien Grall wrote:
> 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]).
 
LOL
This is a good sign: it means I am getting more familiar with the
mfn_x/_mfn syntax :-D


> I really don't mind which one to use. But we should stay consistent with the
> macros to use for non-typesafe version.


Of course. Let's keep the patch as is.

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