Re: [Xen-devel] [PATCH v4 1/6] xen: improve changes to xen_add_to_physmap

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2012-08-22 at 12:08 +0100, Stefano Stabellini wrote:
> This is an incremental patch on top of
> c0bc926083b5987a3e9944eec2c12ad0580100e2: in order to retain binary
> compatibility, it is better to introduce foreign_domid as part of a
> union containing both size and foreign_domid.
[...]
> diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h
> index b2adfbe..7d4ee26 100644
> --- a/xen/include/public/memory.h
> +++ b/xen/include/public/memory.h
> @@ -208,8 +208,12 @@ struct xen_add_to_physmap {
>      /* Which domain to change the mapping for. */
>      domid_t domid;
>  
> -    /* Number of pages to go through for gmfn_range */
> -    uint16_t    size;
> +    union {
> +        /* Number of pages to go through for gmfn_range */
> +        uint16_t    size;
> +        /* IFF gmfn_foreign */
> +        domid_t foreign_domid;
> +    } u;
>  
>      /* Source mapping space. */
>  #define XENMAPSPACE_shared_info  0 /* shared info page */
> @@ -217,8 +221,7 @@ struct xen_add_to_physmap {
>  #define XENMAPSPACE_gmfn         2 /* GMFN */
>  #define XENMAPSPACE_gmfn_range   3 /* GMFN range */
>  #define XENMAPSPACE_gmfn_foreign 4 /* GMFN from another guest */
> -    uint16_t space;
> -    domid_t foreign_domid; /* IFF gmfn_foreign */
> +    unsigned int space;
>  
>  #define XENMAPIDX_grant_table_status 0x80000000

Was this the final consensus on what this interface ought to look like?

Does it work for PVH too (Mukesh CCd)?

Might we prefer to have a batched version of this call? I don't think we
can shoehorn the necessary fields into xen_add_to_physmap_t though.

Do we think libxc will ever want to call a batch version of
XENMAPSPACE_gmfn_foreign ? If not then we can probably get away using
multicall batching. If yes then perhaps not -- libxc doesn'tdo
multicalls.

XENMAPSPACE_gmfn_range was added to avoid an issue with tonnes of
(iommu?) flushes when remapping large numbers of pages with
XENMAPSPACE_gmfn. It seems like XENMAPSPACE_gmfn_foreign  could suffer
the same issue, in which case multicalls won't cut it.

I guess XENMAPSPACE_gmfn_range works only on contiguous ranges in both P
and M space? That style probably doesn't work for foreign anyway (where
at least the M isn't going to be contiguous)

Sorry for not thinking about this until after all the faff with unions
was done...

Ian.





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