Hi Jan, On 25/05/16 14:12, Jan Beulich wrote:
On 25.05.16 at 13:41, <julien.gr...@arm.com> wrote:--- a/xen/arch/arm/mm.c +++ b/xen/arch/arm/mm.c @@ -1143,6 +1143,10 @@ int xenmem_add_to_physmap_one( break; } case XENMAPSPACE_dev_mmio: + /* The field 'foreign_domid' is reserved for future use */ + if ( foreign_domid ) + return -ENOSYS;This should return -EINVAL or maybe -EOPNOTSUPP, but definitely not -ENOSYS.
I will use -EOPNOTSUPP.
--- a/xen/common/memory.c +++ b/xen/common/memory.c @@ -639,9 +639,11 @@ static int xenmem_add_to_physmap(struct domain *d, { unsigned int done = 0; long rc = 0; + /* The field 'foreign_id' should be 0 when mapping MMIO. */ + domid_t inv = (xatp->space != XENMAPSPACE_dev_mmio) ? DOMID_INVALID : 0;This is a bad type for something that now isn't a domain ID anymore. Please use u16 or even better unsigned int. Eventually we should fix xenmem_add_to_physmap_one()'s respective parameter type accordingly. Also I think the condition would better be space == gmfn_foreign.
Ok.
@@ -658,7 +660,7 @@ static int xenmem_add_to_physmap(struct domain *d, while ( xatp->size > done ) { - rc = xenmem_add_to_physmap_one(d, xatp->space, DOMID_INVALID, + rc = xenmem_add_to_physmap_one(d, xatp->space, inv, xatp->idx, xatp->gpfn);This instance you could actually leave alone (as it's dealing with XENMAPSPACE_gmfn_range only).--- a/xen/include/public/memory.h +++ b/xen/include/public/memory.h @@ -259,7 +259,7 @@ struct xen_add_to_physmap_batch { /* Number of pages to go through */ uint16_t size; - domid_t foreign_domid; /* IFF gmfn_foreign */ + domid_t foreign_domid; /* IFF gmfn_foreign. Should be 0 for other spaces. */I wonder whether we shouldn't fix up the structure here right away, instead of deferring that to after 4.7. After all, as above, we don't really want a domain ID here generally anymore, so this should either become "u16 aux" (or some such) or a union (all of course only for new enough __XEN_INTERFACE_VERSION__). Plus I think we will want this to be IN/OUT, such that if the implementation, rather than failing, uses a replacement attribute, that could be communicated back. Of course that would matter only if we don't go the union route mentioned above.
I will give a look to use an union. Regards, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
