[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v4 12/16] xen/mm: Switch common/memory.c to use typesafe MFN
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
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |