| Message ID | 3f8ab6b4cf686e814d91961b564fede6d0c64030.1578503483.git.tamas.lengyel@intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | VM forking | expand |
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
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
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
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
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(-)