Mailing List Archive: [PATCH v4 12/18] x86/mem_sharing: Enable mem

[PATCH v4 12/18] x86/mem_sharing: Enable mem_sharing on first memop

Jan 8, 2020, 9:14 AM

Post #1 of 3 (116 views)

It is wasteful to require separate hypercalls to enable sharing on both the
parent and the client domain during VM forking. To speed things up we enable
sharing on the first memop in case it wasn't already enabled.

Signed-off-by: Tamas K Lengyel <tamas.lengyel@intel.com>
---
xen/arch/x86/mm/mem_sharing.c | 36 +++++++++++++++++++++--------------
1 file changed, 22 insertions(+), 14 deletions(-)

diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c
index 3f36cd6bbc..b8a9228ecf 100644
--- a/xen/arch/x86/mm/mem_sharing.c
+++ b/xen/arch/x86/mm/mem_sharing.c
@@ -1412,6 +1412,24 @@ static int range_share(struct domain *d, struct domain *cd,
return rc;
}

+static inline int mem_sharing_control(struct domain *d, bool enable)
+{
+ if ( enable )
+ {
+ if ( unlikely(!is_hvm_domain(d)) )
+ return -ENOSYS;
+
+ if ( unlikely(!hap_enabled(d)) )
+ return -ENODEV;
+
+ if ( unlikely(is_iommu_enabled(d)) )
+ return -EXDEV;
+ }
+
+ d->arch.hvm.mem_sharing.enabled = enable;
+ return 0;
+}
+
int mem_sharing_memop(XEN_GUEST_HANDLE_PARAM(xen_mem_sharing_op_t) arg)
{
int rc;
@@ -1433,10 +1451,8 @@ int mem_sharing_memop(XEN_GUEST_HANDLE_PARAM(xen_mem_sharing_op_t) arg)
if ( rc )
goto out;

- /* Only HAP is supported */
- rc = -ENODEV;
- if ( !mem_sharing_enabled(d) )
- goto out;
+ if ( !mem_sharing_enabled(d) && (rc = mem_sharing_control(d, true)) )
+ return rc;

switch ( mso.op )
{
@@ -1703,18 +1719,10 @@ int mem_sharing_domctl(struct domain *d, struct xen_domctl_mem_sharing_op *mec)
{
int rc;

- /* Only HAP is supported */
- if ( !hap_enabled(d) )
- return -ENODEV;
-
- switch ( mec->op )
+ switch( mec->op )
{
case XEN_DOMCTL_MEM_SHARING_CONTROL:
- rc = 0;
- if ( unlikely(is_iommu_enabled(d) && mec->u.enable) )
- rc = -EXDEV;
- else
- d->arch.hvm.mem_sharing_enabled = mec->u.enable;
+ rc = mem_sharing_control(d, mec->u.enable);
break;

default:
--
2.20.1

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

Re: [PATCH v4 12/18] x86/mem_sharing: Enable mem_sharing on first memop [ In reply to ]

jbeulich at suse

Jan 16, 2020, 8:18 AM

Post #2 of 3 (113 views)

Permalink

On 08.01.2020 18:14, Tamas K Lengyel wrote:
> It is wasteful to require separate hypercalls to enable sharing on both the
> parent and the client domain during VM forking. To speed things up we enable
> sharing on the first memop in case it wasn't already enabled.
>
> Signed-off-by: Tamas K Lengyel <tamas.lengyel@intel.com>
> ---
> xen/arch/x86/mm/mem_sharing.c | 36 +++++++++++++++++++++--------------
> 1 file changed, 22 insertions(+), 14 deletions(-)
>
> diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c
> index 3f36cd6bbc..b8a9228ecf 100644
> --- a/xen/arch/x86/mm/mem_sharing.c
> +++ b/xen/arch/x86/mm/mem_sharing.c
> @@ -1412,6 +1412,24 @@ static int range_share(struct domain *d, struct domain *cd,
> return rc;
> }
>
> +static inline int mem_sharing_control(struct domain *d, bool enable)
> +{
> + if ( enable )
> + {
> + if ( unlikely(!is_hvm_domain(d)) )
> + return -ENOSYS;

-EOPNOTSUPP or some such please. ENOSYS has a very specific meaning,
which (according to my understanding) doesn't apply here.

> + if ( unlikely(!hap_enabled(d)) )
> + return -ENODEV;

Doesn't this allow dropping the HAP check from
mem_sharing_enabled(d)?

> + if ( unlikely(is_iommu_enabled(d)) )
> + return -EXDEV;
> + }
> +
> + d->arch.hvm.mem_sharing.enabled = enable;
> + return 0;
> +}
> +
> int mem_sharing_memop(XEN_GUEST_HANDLE_PARAM(xen_mem_sharing_op_t) arg)
> {
> int rc;
> @@ -1433,10 +1451,8 @@ int mem_sharing_memop(XEN_GUEST_HANDLE_PARAM(xen_mem_sharing_op_t) arg)
> if ( rc )
> goto out;
>
> - /* Only HAP is supported */
> - rc = -ENODEV;
> - if ( !mem_sharing_enabled(d) )
> - goto out;
> + if ( !mem_sharing_enabled(d) && (rc = mem_sharing_control(d, true)) )
> + return rc;

Perhaps already in patch 6, doesn't this eliminate the need for the
individual mem_sharing_enabled() checks in the case blocks?

> @@ -1703,18 +1719,10 @@ int mem_sharing_domctl(struct domain *d, struct xen_domctl_mem_sharing_op *mec)
> {
> int rc;
>
> - /* Only HAP is supported */
> - if ( !hap_enabled(d) )
> - return -ENODEV;
> -
> - switch ( mec->op )
> + switch( mec->op )

Please don't corrupt proper Xen style.

Jan

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

Re: [PATCH v4 12/18] x86/mem_sharing: Enable mem_sharing on first memop [ In reply to ]

tamas at tklengyel

Jan 16, 2020, 8:34 AM

Post #3 of 3 (113 views)

Permalink

On Thu, Jan 16, 2020 at 9:18 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 08.01.2020 18:14, Tamas K Lengyel wrote:
> > It is wasteful to require separate hypercalls to enable sharing on both the
> > parent and the client domain during VM forking. To speed things up we enable
> > sharing on the first memop in case it wasn't already enabled.
> >
> > Signed-off-by: Tamas K Lengyel <tamas.lengyel@intel.com>
> > ---
> > xen/arch/x86/mm/mem_sharing.c | 36 +++++++++++++++++++++--------------
> > 1 file changed, 22 insertions(+), 14 deletions(-)
> >
> > diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c
> > index 3f36cd6bbc..b8a9228ecf 100644
> > --- a/xen/arch/x86/mm/mem_sharing.c
> > +++ b/xen/arch/x86/mm/mem_sharing.c
> > @@ -1412,6 +1412,24 @@ static int range_share(struct domain *d, struct domain *cd,
> > return rc;
> > }
> >
> > +static inline int mem_sharing_control(struct domain *d, bool enable)
> > +{
> > + if ( enable )
> > + {
> > + if ( unlikely(!is_hvm_domain(d)) )
> > + return -ENOSYS;
>
> -EOPNOTSUPP or some such please. ENOSYS has a very specific meaning,
> which (according to my understanding) doesn't apply here.
>
> > + if ( unlikely(!hap_enabled(d)) )
> > + return -ENODEV;
>
> Doesn't this allow dropping the HAP check from
> mem_sharing_enabled(d)?

Yes, looks like it could be dropped from there.

>
> > + if ( unlikely(is_iommu_enabled(d)) )
> > + return -EXDEV;
> > + }
> > +
> > + d->arch.hvm.mem_sharing.enabled = enable;
> > + return 0;
> > +}
> > +
> > int mem_sharing_memop(XEN_GUEST_HANDLE_PARAM(xen_mem_sharing_op_t) arg)
> > {
> > int rc;
> > @@ -1433,10 +1451,8 @@ int mem_sharing_memop(XEN_GUEST_HANDLE_PARAM(xen_mem_sharing_op_t) arg)
> > if ( rc )
> > goto out;
> >
> > - /* Only HAP is supported */
> > - rc = -ENODEV;
> > - if ( !mem_sharing_enabled(d) )
> > - goto out;
> > + if ( !mem_sharing_enabled(d) && (rc = mem_sharing_control(d, true)) )
> > + return rc;
>
> Perhaps already in patch 6, doesn't this eliminate the need for the
> individual mem_sharing_enabled() checks in the case blocks?

Yes it does. I think I was planning on removing those checks but it
slipped my mind.

>
> > @@ -1703,18 +1719,10 @@ int mem_sharing_domctl(struct domain *d, struct xen_domctl_mem_sharing_op *mec)
> > {
> > int rc;
> >
> > - /* Only HAP is supported */
> > - if ( !hap_enabled(d) )
> > - return -ENODEV;
> > -
> > - switch ( mec->op )
> > + switch( mec->op )
>
> Please don't corrupt proper Xen style.

Ack.

Thanks!
Tamas

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