Mailing List Archive

[PATCH v4 06/18] x86/mem_sharing: define mem_sharing_domain to hold some scattered variables
Create struct mem_sharing_domain under hvm_domain and move mem sharing
variables into it from p2m_domain and hvm_domain.

Expose the mem_sharing_enabled macro to be used consistently across Xen.

Remove some duplicate calls to mem_sharing_enabled in mem_sharing.c

Signed-off-by: Tamas K Lengyel <tamas.lengyel@intel.com>
---
xen/arch/x86/mm/mem_sharing.c | 10 ++++------
xen/drivers/passthrough/pci.c | 3 +--
xen/include/asm-x86/hvm/domain.h | 6 +++++-
xen/include/asm-x86/mem_sharing.h | 16 ++++++++++++++++
xen/include/asm-x86/p2m.h | 4 ----
5 files changed, 26 insertions(+), 13 deletions(-)

diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c
index f6187403a0..3aa61c30e6 100644
--- a/xen/arch/x86/mm/mem_sharing.c
+++ b/xen/arch/x86/mm/mem_sharing.c
@@ -197,9 +197,6 @@ static shr_handle_t get_next_handle(void)
return x + 1;
}

-#define mem_sharing_enabled(d) \
- (is_hvm_domain(d) && (d)->arch.hvm.mem_sharing_enabled)
-
static atomic_t nr_saved_mfns = ATOMIC_INIT(0);
static atomic_t nr_shared_mfns = ATOMIC_INIT(0);

@@ -1309,6 +1306,7 @@ int __mem_sharing_unshare_page(struct domain *d,
int relinquish_shared_pages(struct domain *d)
{
int rc = 0;
+ struct mem_sharing_domain *msd = &d->arch.hvm.mem_sharing;
struct p2m_domain *p2m = p2m_get_hostp2m(d);
unsigned long gfn, count = 0;

@@ -1316,7 +1314,7 @@ int relinquish_shared_pages(struct domain *d)
return 0;

p2m_lock(p2m);
- for ( gfn = p2m->next_shared_gfn_to_relinquish;
+ for ( gfn = msd->next_shared_gfn_to_relinquish;
gfn <= p2m->max_mapped_pfn; gfn++ )
{
p2m_access_t a;
@@ -1351,7 +1349,7 @@ int relinquish_shared_pages(struct domain *d)
{
if ( hypercall_preempt_check() )
{
- p2m->next_shared_gfn_to_relinquish = gfn + 1;
+ msd->next_shared_gfn_to_relinquish = gfn + 1;
rc = -ERESTART;
break;
}
@@ -1437,7 +1435,7 @@ int mem_sharing_memop(XEN_GUEST_HANDLE_PARAM(xen_mem_sharing_op_t) arg)

/* Only HAP is supported */
rc = -ENODEV;
- if ( !hap_enabled(d) || !d->arch.hvm.mem_sharing_enabled )
+ if ( !mem_sharing_enabled(d) )
goto out;

switch ( mso.op )
diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index c07a63981a..65d1d457ff 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -1498,8 +1498,7 @@ static int assign_device(struct domain *d, u16 seg, u8 bus, u8 devfn, u32 flag)
/* Prevent device assign if mem paging or mem sharing have been
* enabled for this domain */
if ( d != dom_io &&
- unlikely((is_hvm_domain(d) &&
- d->arch.hvm.mem_sharing_enabled) ||
+ unlikely(mem_sharing_enabled(d) ||
vm_event_check_ring(d->vm_event_paging) ||
p2m_get_hostp2m(d)->global_logdirty) )
return -EXDEV;
diff --git a/xen/include/asm-x86/hvm/domain.h b/xen/include/asm-x86/hvm/domain.h
index bcc5621797..8f70ba2b1a 100644
--- a/xen/include/asm-x86/hvm/domain.h
+++ b/xen/include/asm-x86/hvm/domain.h
@@ -29,6 +29,7 @@
#include <asm/hvm/viridian.h>
#include <asm/hvm/vmx/vmcs.h>
#include <asm/hvm/svm/vmcb.h>
+#include <asm/mem_sharing.h>
#include <public/grant_table.h>
#include <public/hvm/params.h>
#include <public/hvm/save.h>
@@ -156,7 +157,6 @@ struct hvm_domain {

struct viridian_domain *viridian;

- bool_t mem_sharing_enabled;
bool_t qemu_mapcache_invalidate;
bool_t is_s3_suspended;

@@ -192,6 +192,10 @@ struct hvm_domain {
struct vmx_domain vmx;
struct svm_domain svm;
};
+
+#ifdef CONFIG_MEM_SHARING
+ struct mem_sharing_domain mem_sharing;
+#endif
};

#endif /* __ASM_X86_HVM_DOMAIN_H__ */
diff --git a/xen/include/asm-x86/mem_sharing.h b/xen/include/asm-x86/mem_sharing.h
index cf7848709f..13114b6346 100644
--- a/xen/include/asm-x86/mem_sharing.h
+++ b/xen/include/asm-x86/mem_sharing.h
@@ -26,6 +26,20 @@

#ifdef CONFIG_MEM_SHARING

+struct mem_sharing_domain
+{
+ bool enabled;
+
+ /*
+ * When releasing shared gfn's in a preemptible manner, recall where
+ * to resume the search.
+ */
+ unsigned long next_shared_gfn_to_relinquish;
+};
+
+#define mem_sharing_enabled(d) \
+ (hap_enabled(d) && (d)->arch.hvm.mem_sharing.enabled)
+
/* Auditing of memory sharing code? */
#ifndef NDEBUG
#define MEM_SHARING_AUDIT 1
@@ -104,6 +118,8 @@ int relinquish_shared_pages(struct domain *d);

#else

+#define mem_sharing_enabled(d) false
+
static inline unsigned int mem_sharing_get_nr_saved_mfns(void)
{
return 0;
diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h
index 7399c4a897..8defa90306 100644
--- a/xen/include/asm-x86/p2m.h
+++ b/xen/include/asm-x86/p2m.h
@@ -305,10 +305,6 @@ struct p2m_domain {
unsigned long min_remapped_gfn;
unsigned long max_remapped_gfn;

- /* When releasing shared gfn's in a preemptible manner, recall where
- * to resume the search */
- unsigned long next_shared_gfn_to_relinquish;
-
#ifdef CONFIG_HVM
/* Populate-on-demand variables
* All variables are protected with the pod lock. We cannot rely on
--
2.20.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [PATCH v4 06/18] x86/mem_sharing: define mem_sharing_domain to hold some scattered variables [ In reply to ]
On 08.01.2020 18:14, Tamas K Lengyel wrote:
> Create struct mem_sharing_domain under hvm_domain and move mem sharing
> variables into it from p2m_domain and hvm_domain.
>
> Expose the mem_sharing_enabled macro to be used consistently across Xen.
>
> Remove some duplicate calls to mem_sharing_enabled in mem_sharing.c
>
> Signed-off-by: Tamas K Lengyel <tamas.lengyel@intel.com>

Acked-by: Jan Beulich <jbeulich@suse.com>
with one question:

> @@ -192,6 +192,10 @@ struct hvm_domain {
> struct vmx_domain vmx;
> struct svm_domain svm;
> };
> +
> +#ifdef CONFIG_MEM_SHARING
> + struct mem_sharing_domain mem_sharing;
> +#endif

Are you intending to add fields to this new struct? If so,
should the field here become a pointer, and the structure
allocated only when actually needed?

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [PATCH v4 06/18] x86/mem_sharing: define mem_sharing_domain to hold some scattered variables [ In reply to ]
On Thu, Jan 16, 2020 at 8:23 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 08.01.2020 18:14, Tamas K Lengyel wrote:
> > Create struct mem_sharing_domain under hvm_domain and move mem sharing
> > variables into it from p2m_domain and hvm_domain.
> >
> > Expose the mem_sharing_enabled macro to be used consistently across Xen.
> >
> > Remove some duplicate calls to mem_sharing_enabled in mem_sharing.c
> >
> > Signed-off-by: Tamas K Lengyel <tamas.lengyel@intel.com>
>
> Acked-by: Jan Beulich <jbeulich@suse.com>
> with one question:
>
> > @@ -192,6 +192,10 @@ struct hvm_domain {
> > struct vmx_domain vmx;
> > struct svm_domain svm;
> > };
> > +
> > +#ifdef CONFIG_MEM_SHARING
> > + struct mem_sharing_domain mem_sharing;
> > +#endif
>
> Are you intending to add fields to this new struct? If so,
> should the field here become a pointer, and the structure
> allocated only when actually needed?
>

At the moment there are no additional variables planned to be added.
If/when we do we can consider turning this into a pointer, at which
point we can also get rid of the "enabled" field and turn the
mem_sharing_enabled macro into a NULL-pointer check instead. For now I
wouldn't bother because its not like we save much by doing so.

Thanks,
Tamas

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [PATCH v4 06/18] x86/mem_sharing: define mem_sharing_domain to hold some scattered variables [ In reply to ]
On 16.01.2020 17:05, Tamas K Lengyel wrote:
> On Thu, Jan 16, 2020 at 8:23 AM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 08.01.2020 18:14, Tamas K Lengyel wrote:
>>> Create struct mem_sharing_domain under hvm_domain and move mem sharing
>>> variables into it from p2m_domain and hvm_domain.
>>>
>>> Expose the mem_sharing_enabled macro to be used consistently across Xen.
>>>
>>> Remove some duplicate calls to mem_sharing_enabled in mem_sharing.c
>>>
>>> Signed-off-by: Tamas K Lengyel <tamas.lengyel@intel.com>
>>
>> Acked-by: Jan Beulich <jbeulich@suse.com>
>> with one question:
>>
>>> @@ -192,6 +192,10 @@ struct hvm_domain {
>>> struct vmx_domain vmx;
>>> struct svm_domain svm;
>>> };
>>> +
>>> +#ifdef CONFIG_MEM_SHARING
>>> + struct mem_sharing_domain mem_sharing;
>>> +#endif
>>
>> Are you intending to add fields to this new struct? If so,
>> should the field here become a pointer, and the structure
>> allocated only when actually needed?
>>
>
> At the moment there are no additional variables planned to be added.
> If/when we do we can consider turning this into a pointer, at which
> point we can also get rid of the "enabled" field and turn the
> mem_sharing_enabled macro into a NULL-pointer check instead. For now I
> wouldn't bother because its not like we save much by doing so.

Thanks for clarifying.

Jan

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