HVMOP_altp2m_set_domain_state does not domain_pause(), presumably
on purpose (as it was originally supposed to cater to a in-guest
agent, and a domain pausing itself is not a good idea).
This can lead to domain crashes in the vmx_vmexit_handler() code
that checks if the guest has the ability to switch EPTP without an
exit. That code can __vmread() the host p2m's EPT_POINTER
(before HVMOP_altp2m_set_domain_state "for_each_vcpu()" has a
chance to run altp2m_vcpu_initialise(), but after
d->arch.altp2m_active is set).
This patch reorganizes the code so that d->arch.altp2m_active
is set to true only after all the init work has been done, and
to false before the uninit work begins. This required adding
a new bool parameter altp2m_vcpu_update_p2m(), which relied
on d->arch.altp2m_active being set before it's called.
While at it, I've changed a couple of bool_t's to bool's.
Signed-off-by: Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>
---
xen/arch/x86/hvm/hvm.c | 17 ++++++++++++++---
xen/arch/x86/hvm/vmx/vmx.c | 4 ++--
xen/arch/x86/mm/altp2m.c | 4 ++--
xen/arch/x86/mm/p2m.c | 4 ++--
xen/include/asm-x86/altp2m.h | 2 +-
xen/include/asm-x86/domain.h | 2 +-
xen/include/asm-x86/hvm/hvm.h | 6 +++---
7 files changed, 25 insertions(+), 14 deletions(-)
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 21944e9..4d1d13d 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -4525,7 +4525,7 @@ static int do_altp2m_op(
case HVMOP_altp2m_set_domain_state:
{
struct vcpu *v;
- bool_t ostate;
+ bool ostate, nstate;
if ( nestedhvm_enabled(d) )
{
@@ -4534,12 +4534,16 @@ static int do_altp2m_op(
}
ostate = d->arch.altp2m_active;
- d->arch.altp2m_active = !!a.u.domain_state.state;
+ nstate = !!a.u.domain_state.state;
/* If the alternate p2m state has changed, handle appropriately */
- if ( d->arch.altp2m_active != ostate &&
+ if ( nstate != ostate &&
(ostate || !(rc = p2m_init_altp2m_by_id(d, 0))) )
{
+ /* First mark altp2m as disabled, then altp2m_vcpu_destroy(). */
+ if ( ostate )
+ d->arch.altp2m_active = false;
+
for_each_vcpu( d, v )
{
if ( !ostate )
@@ -4550,7 +4554,14 @@ static int do_altp2m_op(
if ( ostate )
p2m_flush_altp2m(d);
+ else
+ /*
+ * Wait until altp2m_vcpu_initialise() is done before marking
+ * altp2m as being enabled for the domain.
+ */
+ d->arch.altp2m_active = true;
}
+
break;
}
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 64af8bf..e542568 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -2150,13 +2150,13 @@ static bool_t vmx_is_singlestep_supported(void)
return !!cpu_has_monitor_trap_flag;
}
-static void vmx_vcpu_update_eptp(struct vcpu *v)
+static void vmx_vcpu_update_eptp(struct vcpu *v, bool altp2m_enabled)
{
struct domain *d = v->domain;
struct p2m_domain *p2m = NULL;
struct ept_data *ept;
- if ( altp2m_active(d) )
+ if ( altp2m_enabled )
p2m = p2m_get_altp2m(v);
if ( !p2m )
p2m = p2m_get_hostp2m(d);
diff --git a/xen/arch/x86/mm/altp2m.c b/xen/arch/x86/mm/altp2m.c
index 930bdc2..c51303b 100644
--- a/xen/arch/x86/mm/altp2m.c
+++ b/xen/arch/x86/mm/altp2m.c
@@ -39,7 +39,7 @@ altp2m_vcpu_initialise(struct vcpu *v)
vcpu_altp2m(v).p2midx = 0;
atomic_inc(&p2m_get_altp2m(v)->active_vcpus);
- altp2m_vcpu_update_p2m(v);
+ altp2m_vcpu_update_p2m(v, true);
if ( v != current )
vcpu_unpause(v);
@@ -58,7 +58,7 @@ altp2m_vcpu_destroy(struct vcpu *v)
altp2m_vcpu_reset(v);
- altp2m_vcpu_update_p2m(v);
+ altp2m_vcpu_update_p2m(v, false);
altp2m_vcpu_update_vmfunc_ve(v);
if ( v != current )
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index d14ce57..f9e96fc 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -2332,7 +2332,7 @@ bool_t p2m_switch_vcpu_altp2m_by_id(struct vcpu *v,
unsigned int idx)
atomic_dec(&p2m_get_altp2m(v)->active_vcpus);
vcpu_altp2m(v).p2midx = idx;
atomic_inc(&p2m_get_altp2m(v)->active_vcpus);
- altp2m_vcpu_update_p2m(v);
+ altp2m_vcpu_update_p2m(v, altp2m_active(d));
}
rc = 1;
}
@@ -2573,7 +2573,7 @@ int p2m_switch_domain_altp2m_by_id(struct domain *d,
unsigned int idx)
atomic_dec(&p2m_get_altp2m(v)->active_vcpus);
vcpu_altp2m(v).p2midx = idx;
atomic_inc(&p2m_get_altp2m(v)->active_vcpus);
- altp2m_vcpu_update_p2m(v);
+ altp2m_vcpu_update_p2m(v, altp2m_active(d));
}
rc = 0;
diff --git a/xen/include/asm-x86/altp2m.h b/xen/include/asm-x86/altp2m.h
index 3befcf6..e3df27f 100644
--- a/xen/include/asm-x86/altp2m.h
+++ b/xen/include/asm-x86/altp2m.h
@@ -33,7 +33,7 @@ static inline bool altp2m_active(const struct domain *d)
/* Alternate p2m VCPU */
void altp2m_vcpu_initialise(struct vcpu *v);
void altp2m_vcpu_destroy(struct vcpu *v);
-void altp2m_vcpu_reset(struct vcpu *v);
+void altp2m_vcpu_disable_ve(struct vcpu *v);