Re: [Xen-devel] [PATCH v4 2/7] xen: xsm: flask: introduce XENMAPSPACE_gmfn_share for memory sharing

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 30.01.18 at 18:50, <blackskygg@xxxxxxxxx> wrote:
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -4126,6 +4126,10 @@ int xenmem_add_to_physmap_one(
>          }
>          case XENMAPSPACE_gmfn_foreign:
>              return p2m_add_foreign(d, idx, gfn_x(gpfn), extra.foreign_domid);
> +        case XENMAPSPACE_gmfn_share:
> +            gdprintk(XENLOG_WARNING,
> +                     "XENMAPSPACE_gmfn_share is currently not supported on 
> x86\n");
> +            break;

Please don't - a hypervisor log message isn't really useful here. It
should be the tool stack to disallow respective options on x86
until that's implemented.

> --- a/xen/include/public/memory.h
> +++ b/xen/include/public/memory.h
> @@ -227,6 +227,10 @@ DEFINE_XEN_GUEST_HANDLE(xen_machphys_mapping_t);
>                                        Stage-2 using the Normal Memory
>                                        Inner/Outer Write-Back Cacheable
>                                        memory attribute. */
> +#define XENMAPSPACE_gmfn_share   6 /* Same as *_gmfn_foreign, but this is
> +                                      for a privileged dom to share pages
> +                                      between two doms. */
> +

The comment doesn't make clear why XENMAPSPACE_gmfn_foreign
then can't be used. In particular it is left open how _both_ domains
would be specified.

Also XENMAPSPACE_gmfn_foreign is restricted to
XENMEM_add_to_physmap_batch (a comment says so) - how about
this new one? According to the actual code changes you do, there's
no meaningful difference, in which case the restriction should be
named here as well.

> --- a/xen/include/xsm/dummy.h
> +++ b/xen/include/xsm/dummy.h
> @@ -521,6 +521,12 @@ static XSM_INLINE int 
> xsm_map_gmfn_foreign(XSM_DEFAULT_ARG struct domain *d, str
>      return xsm_default_action(action, d, t);
>  }
>  
> +static XSM_INLINE int xsm_map_gmfn_share(XSM_DEFAULT_ARG struct domain *d, 
> struct domain *t)

Line length.

> +{
> +    XSM_ASSERT_ACTION(XSM_TARGET);
> +    return xsm_default_action(action, current->domain, t);

How does this represent a proper default equivalent of ...

--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -1196,6 +1196,12 @@ static int flask_map_gmfn_foreign(struct domain *d, 
struct domain *t)
     return domain_has_perm(d, t, SECCLASS_MMU, MMU__MAP_READ | MMU__MAP_WRITE);
 }
 
+static int flask_map_gmfn_share(struct domain *d, struct domain *t)
+{
+    return current_has_perm(t, SECCLASS_MMU, MMU__MAP_READ | MMU__MAP_WRITE) ?:
+        domain_has_perm(d, t, SECCLASS_MMU, MMU__SHARE_MEM);

... this?

Jan


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