Re: [Xen-devel] [PATCH v4 12/16] xen/mm: Switch common/memory.c to use typesafe MFN

[Julien Grall <julien.grall@xxxxxxx>]

Hi,

On 03/09/2018 05:33 PM, Wei Liu wrote:
> On Mon, Mar 05, 2018 at 07:41:54AM -0700, Jan Beulich wrote:
> > > > > On 05.03.18 at 15:18, <julien.grall@xxxxxxx> wrote:
> > > On 02/03/18 15:34, Jan Beulich wrote:
> > > > > > > On 21.02.18 at 15:02, <julien.grall@xxxxxxx> wrote:
> > > > > @@ -95,11 +101,18 @@ static unsigned int max_order(const struct domain *d)
> > > > >        return min(order, MAX_ORDER + 0U);
> > > > >    }
> > > > >    
> > > > > +/* Helper to copy a typesafe MFN to guest */
> > > > > +#define copy_mfn_to_guest(hnd, off, mfn)            \
> > > > > +    ({                                              \
> > > > > +        xen_pfn_t mfn_ = mfn_x(mfn);                \
> > > > > +        __copy_to_guest_offset(hnd, off, &mfn_, 1); \
> > > > > +    })
> > > > Hmm, not really nice, but what do you do.
> > > I am open to better suggestion. I wanted to avoid the conversion all
> > > over the code.
> > I have no better suggestion, I'm sorry, hence the "but what do
> > you do."
> >
> > > Also, do you have an opinion on Wei's suggestion:
> > >
> > > "What I meant was to make copy_{to,from}_guest* type-safe. I just feel it
> > > a bit strange you only created a wrapper for this file. I wonder why.
> > >
> > > Note I'm just asking question. That's not necessarily a good idea to
> > > turn them all in the end."
> > Well, I didn't really understand what he's after (in the context of
> > this series) - copy_{to,from}_guest() don't take or return MFNs or
> > GFNs.
> Fundamentally Julien's patch is to wrap around an existing API for this
> one file only. Why is this file special? Why not just make that class of
> APIs do what he wants?
>
> But that is going to be intrusive and a bit counter-intuitive.
I have quickly looked at it. The major problem I can see is it is not 
possible to generically define for any typesafe. Indeed, TYPE_SAFE(...) 
cannot define new macro and, AFAICT, it is not feasible to define static 
inline for copy_* helpers.
So we would need to introduce macros for each typesafe by hand. I can 
move copy_mfn_to_guest in xen/mm.h if people think it could be useful.
Cheers,

--
Julien Grall

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