|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v2 04/12] x86/altp2m: basic data structures and support routines.
On 22/06/15 19:56, Ed White wrote:
> Add the basic data structures needed to support alternate p2m's and
> the functions to initialise them and tear them down.
>
> Although Intel hardware can handle 512 EPTP's per hardware thread
> concurrently, only 10 per domain are supported in this patch for
> performance reasons.
>
> The iterator in hap_enable() does need to handle 512, so that is now
> uint16_t.
>
> This change also splits the p2m lock into one lock type for altp2m's
> and another type for all other p2m's. The purpose of this is to place
> the altp2m list lock between the types, so the list lock can be
> acquired whilst holding the host p2m lock.
>
> Signed-off-by: Ed White <edmund.h.white@xxxxxxxxx>
> ---
> xen/arch/x86/hvm/Makefile | 2 +
> xen/arch/x86/hvm/altp2mhvm.c | 82 ++++++++++++++++++++++++++++
> xen/arch/x86/hvm/hvm.c | 21 ++++++++
> xen/arch/x86/mm/hap/hap.c | 31 ++++++++++-
> xen/arch/x86/mm/mm-locks.h | 33 +++++++++++-
> xen/arch/x86/mm/p2m.c | 103
> ++++++++++++++++++++++++++++++++++++
> xen/include/asm-x86/domain.h | 10 ++++
> xen/include/asm-x86/hvm/altp2mhvm.h | 38 +++++++++++++
> xen/include/asm-x86/hvm/hvm.h | 17 ++++++
> xen/include/asm-x86/hvm/vcpu.h | 9 ++++
> xen/include/asm-x86/p2m.h | 30 ++++++++++-
> 11 files changed, 372 insertions(+), 4 deletions(-)
> create mode 100644 xen/arch/x86/hvm/altp2mhvm.c
> create mode 100644 xen/include/asm-x86/hvm/altp2mhvm.h
>
> diff --git a/xen/arch/x86/hvm/Makefile b/xen/arch/x86/hvm/Makefile
> index 69af47f..da4475d 100644
> --- a/xen/arch/x86/hvm/Makefile
> +++ b/xen/arch/x86/hvm/Makefile
> @@ -1,6 +1,7 @@
> subdir-y += svm
> subdir-y += vmx
>
> +obj-y += altp2mhvm.o
This file is already in a directory named hvm. I would name it simply
altp2m.c
Similarly, altp2mhvm in function names seems redundant, where altp2m
would suffice. We will never be in a position of offering altp2m to PV
domains.
> obj-y += asid.o
> obj-y += emulate.o
> obj-y += event.o
> @@ -24,3 +25,4 @@ obj-y += vmsi.o
> obj-y += vpic.o
> obj-y += vpt.o
> obj-y += vpmu.o
> +
Spurious whitespace change.
> diff --git a/xen/arch/x86/hvm/altp2mhvm.c b/xen/arch/x86/hvm/altp2mhvm.c
> new file mode 100644
> index 0000000..802fe5b
> --- /dev/null
> +++ b/xen/arch/x86/hvm/altp2mhvm.c
> @@ -0,0 +1,82 @@
> +/*
> + * Alternate p2m HVM
> + * Copyright (c) 2014, Intel Corporation.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> with
> + * this program; if not, write to the Free Software Foundation, Inc., 59
> Temple
> + * Place - Suite 330, Boston, MA 02111-1307 USA.
> + */
> +
> +#include <asm/hvm/support.h>
> +#include <asm/hvm/hvm.h>
> +#include <asm/p2m.h>
> +#include <asm/hvm/altp2mhvm.h>
> +
> +void
> +altp2mhvm_vcpu_reset(struct vcpu *v)
> +{
> + struct altp2mvcpu *av = &vcpu_altp2mhvm(v);
> +
> + av->p2midx = INVALID_ALTP2M;
> + av->veinfo_gfn = 0;
> +
> + if ( hvm_funcs.ahvm_vcpu_reset )
> + hvm_funcs.ahvm_vcpu_reset(v);
> +}
> +
> +int
> +altp2mhvm_vcpu_initialise(struct vcpu *v)
> +{
> + int rc = -EOPNOTSUPP;
> +
> + if ( v != current )
> + vcpu_pause(v);
> +
> + if ( !hvm_funcs.ahvm_vcpu_initialise ||
> + (hvm_funcs.ahvm_vcpu_initialise(v) == 0) )
> + {
> + rc = 0;
> + altp2mhvm_vcpu_reset(v);
> + vcpu_altp2mhvm(v).p2midx = 0;
> + atomic_inc(&p2m_get_altp2m(v)->active_vcpus);
> +
> + ahvm_vcpu_update_eptp(v);
> + }
> +
> + if ( v != current )
> + vcpu_unpause(v);
> +
> + return rc;
> +}
> +
> +void
> +altp2mhvm_vcpu_destroy(struct vcpu *v)
> +{
> + struct p2m_domain *p2m;
> +
> + if ( v != current )
> + vcpu_pause(v);
> +
> + if ( hvm_funcs.ahvm_vcpu_destroy )
> + hvm_funcs.ahvm_vcpu_destroy(v);
> +
> + if ( (p2m = p2m_get_altp2m(v)) )
> + atomic_dec(&p2m->active_vcpus);
> +
> + altp2mhvm_vcpu_reset(v);
> +
> + ahvm_vcpu_update_eptp(v);
> + ahvm_vcpu_update_vmfunc_ve(v);
> +
> + if ( v != current )
> + vcpu_unpause(v);
> +}
Please put an variable block at the bottom of the file.
/*
* Local variables:
* mode: C
* c-file-style: "BSD"
* c-basic-offset: 4
* tab-width: 4
* indent-tabs-mode: nil
* End:
*/
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index af68d44..d75c12d 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -58,6 +58,7 @@
> #include <asm/hvm/cacheattr.h>
> #include <asm/hvm/trace.h>
> #include <asm/hvm/nestedhvm.h>
> +#include <asm/hvm/altp2mhvm.h>
> #include <asm/hvm/event.h>
> #include <asm/mtrr.h>
> #include <asm/apic.h>
> @@ -2373,6 +2374,7 @@ void hvm_vcpu_destroy(struct vcpu *v)
> {
> hvm_all_ioreq_servers_remove_vcpu(v->domain, v);
>
> + altp2mhvm_vcpu_destroy(v);
> nestedhvm_vcpu_destroy(v);
>
> free_compat_arg_xlat(v);
> @@ -6486,6 +6488,25 @@ bool_t hvm_altp2m_supported()
> return hvm_funcs.altp2m_supported;
> }
>
> +void ahvm_vcpu_update_eptp(struct vcpu *v)
> +{
> + if (hvm_funcs.ahvm_vcpu_update_eptp)
> + hvm_funcs.ahvm_vcpu_update_eptp(v);
> +}
> +
> +void ahvm_vcpu_update_vmfunc_ve(struct vcpu *v)
> +{
> + if (hvm_funcs.ahvm_vcpu_update_vmfunc_ve)
> + hvm_funcs.ahvm_vcpu_update_vmfunc_ve(v);
> +}
> +
> +bool_t ahvm_vcpu_emulate_ve(struct vcpu *v)
> +{
> + if (hvm_funcs.ahvm_vcpu_emulate_ve)
> + return hvm_funcs.ahvm_vcpu_emulate_ve(v);
> + return 0;
> +}
> +
> /*
> * Local variables:
> * mode: C
> diff --git a/xen/arch/x86/mm/hap/hap.c b/xen/arch/x86/mm/hap/hap.c
> index d0d3f1e..202aa42 100644
> --- a/xen/arch/x86/mm/hap/hap.c
> +++ b/xen/arch/x86/mm/hap/hap.c
> @@ -459,7 +459,7 @@ void hap_domain_init(struct domain *d)
> int hap_enable(struct domain *d, u32 mode)
> {
> unsigned int old_pages;
> - uint8_t i;
> + uint16_t i;
> int rv = 0;
>
> domain_pause(d);
> @@ -498,6 +498,24 @@ int hap_enable(struct domain *d, u32 mode)
> goto out;
> }
>
> + /* Init alternate p2m data */
> + if ( (d->arch.altp2m_eptp = alloc_xenheap_page()) == NULL )
Please use alloc_domheap_page() and map_domain_page_global() so the
allocation is accounted against the domain.
> + {
> + rv = -ENOMEM;
> + goto out;
> + }
> +
> + for (i = 0; i < MAX_EPTP; i++)
> + d->arch.altp2m_eptp[i] = ~0ul;
> +
> + for (i = 0; i < MAX_ALTP2M; i++) {
> + rv = p2m_alloc_table(d->arch.altp2m_p2m[i]);
> + if ( rv != 0 )
> + goto out;
> + }
> +
> + d->arch.altp2m_active = 0;
> +
> /* Now let other users see the new mode */
> d->arch.paging.mode = mode | PG_HAP_enable;
>
> @@ -510,6 +528,17 @@ void hap_final_teardown(struct domain *d)
> {
> uint8_t i;
>
> + d->arch.altp2m_active = 0;
> +
> + if ( d->arch.altp2m_eptp ) {
> + free_xenheap_page(d->arch.altp2m_eptp);
> + d->arch.altp2m_eptp = NULL;
> + }
> +
> + for (i = 0; i < MAX_ALTP2M; i++) {
> + p2m_teardown(d->arch.altp2m_p2m[i]);
> + }
> +
> /* Destroy nestedp2m's first */
> for (i = 0; i < MAX_NESTEDP2M; i++) {
> p2m_teardown(d->arch.nested_p2m[i]);
> diff --git a/xen/arch/x86/mm/mm-locks.h b/xen/arch/x86/mm/mm-locks.h
> index b4f035e..954b345 100644
> --- a/xen/arch/x86/mm/mm-locks.h
> +++ b/xen/arch/x86/mm/mm-locks.h
> @@ -217,7 +217,7 @@ declare_mm_lock(nestedp2m)
> #define nestedp2m_lock(d) mm_lock(nestedp2m, &(d)->arch.nested_p2m_lock)
> #define nestedp2m_unlock(d) mm_unlock(&(d)->arch.nested_p2m_lock)
>
> -/* P2M lock (per-p2m-table)
> +/* P2M lock (per-non-alt-p2m-table)
> *
> * This protects all queries and updates to the p2m table.
> * Queries may be made under the read lock but all modifications
> @@ -228,7 +228,36 @@ declare_mm_lock(nestedp2m)
> */
>
> declare_mm_rwlock(p2m);
Normally, we expect the declarations to be in the nesting order, and
sofar, this is the case. Please leave a comment here explaining why the
p2m lock declaration is now removed from the rest of its implementation.
> -#define p2m_lock(p) mm_write_lock(p2m, &(p)->lock);
> +
> +/* Alternate P2M list lock (per-domain)
> + *
> + * A per-domain lock that protects the list of alternate p2m's.
> + * Any operation that walks the list needs to acquire this lock.
> + * Additionally, before destroying an alternate p2m all VCPU's
> + * in the target domain must be paused. */
> +
> +declare_mm_lock(altp2mlist)
> +#define altp2m_lock(d) mm_lock(altp2mlist, &(d)->arch.altp2m_lock)
> +#define altp2m_unlock(d) mm_unlock(&(d)->arch.altp2m_lock)
> +
> +/* P2M lock (per-altp2m-table)
> + *
> + * This protects all queries and updates to the p2m table.
> + * Queries may be made under the read lock but all modifications
> + * need the main (write) lock.
> + *
> + * The write lock is recursive as it is common for a code path to look
> + * up a gfn and later mutate it.
> + */
> +
> +declare_mm_rwlock(altp2m);
> +#define p2m_lock(p) \
> +{ \
> + if ( p2m_is_altp2m(p) ) \
> + mm_write_lock(altp2m, &(p)->lock); \
> + else \
> + mm_write_lock(p2m, &(p)->lock); \
> +}
> #define p2m_unlock(p) mm_write_unlock(&(p)->lock);
> #define gfn_lock(p,g,o) p2m_lock(p)
> #define gfn_unlock(p,g,o) p2m_unlock(p)
> diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
> index 1fd1194..87b4b75 100644
> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -35,6 +35,7 @@
> #include <asm/hvm/vmx/vmx.h> /* ept_p2m_init() */
> #include <asm/mem_sharing.h>
> #include <asm/hvm/nestedhvm.h>
> +#include <asm/hvm/altp2mhvm.h>
> #include <asm/hvm/svm/amd-iommu-proto.h>
> #include <xsm/xsm.h>
>
> @@ -183,6 +184,45 @@ static void p2m_teardown_nestedp2m(struct domain *d)
> }
> }
>
> +static void p2m_teardown_altp2m(struct domain *d);
You can avoid this forward declaration by moving the implementation of
p2m_teardown_altp2m() up here.
> +
> +static int p2m_init_altp2m(struct domain *d)
> +{
> + uint8_t i;
> + struct p2m_domain *p2m;
> +
> + mm_lock_init(&d->arch.altp2m_lock);
> + for (i = 0; i < MAX_ALTP2M; i++)
> + {
> + d->arch.altp2m_p2m[i] = p2m = p2m_init_one(d);
> + if ( p2m == NULL )
> + {
> + p2m_teardown_altp2m(d);
> + return -ENOMEM;
> + }
> + p2m->p2m_class = p2m_alternate;
> + p2m->access_required = 1;
> + _atomic_set(&p2m->active_vcpus, 0);
> + }
> +
> + return 0;
> +}
> +
> +static void p2m_teardown_altp2m(struct domain *d)
> +{
> + uint8_t i;
> + struct p2m_domain *p2m;
> +
> + for (i = 0; i < MAX_ALTP2M; i++)
> + {
> + if ( !d->arch.altp2m_p2m[i] )
> + continue;
> + p2m = d->arch.altp2m_p2m[i];
> + p2m_free_one(p2m);
> + d->arch.altp2m_p2m[i] = NULL;
> + }
> +}
> +
> int p2m_init(struct domain *d)
> {
> int rc;
> @@ -196,7 +236,14 @@ int p2m_init(struct domain *d)
> * (p2m_init runs too early for HVM_PARAM_* options) */
> rc = p2m_init_nestedp2m(d);
> if ( rc )
> + {
> p2m_teardown_hostp2m(d);
> + return rc;
> + }
> +
> + rc = p2m_init_altp2m(d);
> + if ( rc )
> + p2m_teardown_altp2m(d);
>
> return rc;
> }
> @@ -1920,6 +1967,62 @@ int unmap_mmio_regions(struct domain *d,
> return err;
> }
>
> +bool_t p2m_find_altp2m_by_eptp(struct domain *d, uint64_t eptp, unsigned
> long *idx)
Wouldn't it be better to return index directly, using INVALID_ALTP2M as
a sentinel, rather than returning idx via pointer?
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |