|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [PATCH v4 29/30] x86/svm: allocate VMCBs from the domain heap behind the vCPU state window
The VMCB holds the full guest register save area, but lives on the
xenheap, where it stays reachable from every context. Move it to the
domain heap and map it at a fixed slot in the owning vCPU's state
window, so that under Address Space Isolation it is only mapped while
that vCPU's page tables are loaded. Hardware is unaffected: VMRUN,
VMLOAD and VMSAVE address the VMCB physically, and vmcb_pa now derives
from the page rather than from virt_to_maddr() on a directmap VA.
A vCPU can have two VMCBs: the primary (nv_n1vmcx) and the nested L2
one (nv_n2vmcx). Both move, taking the first two slots in the window
stride. The nested code's swapping of svm->vmcb / svm->vmcb_pa
between the two works unchanged, now swapping between two window VAs.
free_vmcb() takes a VMCB out of the window with unmap_vcpu_state()
before freeing the page: without per-vCPU page tables the window is in
the domain-wide area, which outlives a vCPU whose creation fails.
The window VA is only valid in the owning vCPU's context.
svm_vmcb_map() and svm_vmcb_map_one() return it for v == current, and
a transient map_domain_page() mapping otherwise, which svm_vmcb_unmap()
drops with unmap_vcpu_state_page(), along with this CPU's translations
of it. svm_ctxt_switch_to() no longer touches the VMCB, as it runs
before write_ptbase(); svm_ctxt_switch_from() keeps dereferencing the
window VA, as the outgoing page tables are still loaded at that point.
Runtime-tested under QEMU's SVM emulation with a PVH dom0 on two vCPUs,
with and without per-vCPU page-tables: VMCB allocation, VMRUN from the
window and context switches. Nested SVM, save/restore, the 'v' debug
key and a guest using debug registers remain to be exercised.
Assisted-by: Claude Code:claude-fable-5, Claude Code:claude-opus-5-5
Signed-off-by: George Dunlap <gwd@xxxxxxxxxxxxxx>
---
Changes in v4:
- New in this version.
---
xen/arch/x86/hvm/svm/nestedsvm.c | 8 +++---
xen/arch/x86/hvm/svm/svm.c | 10 ++++++++
xen/arch/x86/hvm/svm/vmcb.c | 43 ++++++++++++++++++++++----------
xen/arch/x86/hvm/svm/vmcb.h | 15 ++++++++---
4 files changed, 56 insertions(+), 20 deletions(-)
diff --git a/xen/arch/x86/hvm/svm/nestedsvm.c b/xen/arch/x86/hvm/svm/nestedsvm.c
index dc853e5f1a..1599a9908d 100644
--- a/xen/arch/x86/hvm/svm/nestedsvm.c
+++ b/xen/arch/x86/hvm/svm/nestedsvm.c
@@ -71,10 +71,9 @@ int cf_check nsvm_vcpu_initialise(struct vcpu *v)
goto err;
memset(msrpm, 0x0, MSRPM_SIZE);
- nv->nv_n2vmcx = alloc_vmcb();
+ nv->nv_n2vmcx = alloc_vmcb(v, VCPU_STATE_IDX_N2VMCB, &nv->nv_n2vmcx_pa);
if ( nv->nv_n2vmcx == NULL )
goto err;
- nv->nv_n2vmcx_pa = virt_to_maddr(nv->nv_n2vmcx);
return 0;
@@ -95,7 +94,10 @@ void cf_check nsvm_vcpu_destroy(struct vcpu *v)
* of l1 vmcb page.
*/
if ( nv->nv_n1vmcx )
+ {
v->arch.hvm.svm.vmcb = nv->nv_n1vmcx;
+ v->arch.hvm.svm.vmcb_pa = nv->nv_n1vmcx_pa;
+ }
if ( svm->ns_cached_msrpm )
{
@@ -113,7 +115,7 @@ void cf_check nsvm_vcpu_destroy(struct vcpu *v)
if ( nv->nv_n2vmcx )
{
- free_vmcb(nv->nv_n2vmcx);
+ free_vmcb(v, VCPU_STATE_IDX_N2VMCB, nv->nv_n2vmcx_pa);
nv->nv_n2vmcx = NULL;
nv->nv_n2vmcx_pa = INVALID_PADDR;
}
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 3de8da7ea3..6cb3ff4258 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -265,6 +265,10 @@ static void cf_check svm_enable_msr_interception(struct
domain *d, uint32_t msr)
static void svm_save_dr(struct vcpu *v)
{
+ /*
+ * @v is the outgoing vCPU, whose page tables are still loaded: the
+ * VMCB's window VA serves, although current has moved on.
+ */
struct vmcb_struct *vmcb = v->arch.hvm.svm.vmcb;
unsigned int flag_dr_dirty = v->arch.hvm.flag_dr_dirty;
@@ -942,6 +946,12 @@ static void cf_check svm_ctxt_switch_to(struct vcpu *v)
{
int cpu = smp_processor_id();
+ /*
+ * NB we cannot access the VMCB here: this runs before
+ * write_ptbase(), so the incoming vCPU's state window is not
+ * reachable yet.
+ */
+
/*
* This is required, because VMRUN does consistency check and some of the
* DOM0 selectors are pointing to invalid GDT locations, and cause AMD
diff --git a/xen/arch/x86/hvm/svm/vmcb.c b/xen/arch/x86/hvm/svm/vmcb.c
index f5321b03fb..4b398742a9 100644
--- a/xen/arch/x86/hvm/svm/vmcb.c
+++ b/xen/arch/x86/hvm/svm/vmcb.c
@@ -6,6 +6,7 @@
*
*/
+#include <xen/domain_page.h>
#include <xen/init.h>
#include <xen/keyhandler.h>
#include <xen/lib.h>
@@ -23,30 +24,40 @@
#include "svm.h"
#include "vmcb.h"
-struct vmcb_struct *alloc_vmcb(void)
+struct vmcb_struct *alloc_vmcb(struct vcpu *v, unsigned int idx, paddr_t *pa)
{
- struct vmcb_struct *vmcb;
+ struct page_info *pg = alloc_domheap_page(v->domain, MEMF_no_owner);
+ mfn_t mfn;
+
+ ASSERT(idx < VCPU_STATE_NR_PAGES);
- vmcb = alloc_xenheap_page();
- if ( vmcb == NULL )
+ if ( pg == NULL )
{
printk(XENLOG_WARNING "Warning: failed to allocate vmcb.\n");
return NULL;
}
- clear_page(vmcb);
- return vmcb;
+ mfn = page_to_mfn(pg);
+ clear_domain_page(mfn);
+ populate_perdomain_mapping(v, VCPU_STATE_ADDR(v, idx), &mfn, 1,
+ __PAGE_HYPERVISOR_RW);
+ *pa = mfn_to_maddr(mfn);
+
+ return (struct vmcb_struct *)VCPU_STATE_ADDR(v, idx);
}
-void free_vmcb(struct vmcb_struct *vmcb)
+void free_vmcb(struct vcpu *v, unsigned int idx, paddr_t pa)
{
- free_xenheap_page(vmcb);
+ ASSERT(idx < VCPU_STATE_NR_PAGES);
+
+ unmap_vcpu_state(v, idx, 1);
+ free_domheap_page(maddr_to_page(pa));
}
struct vmcb_struct *svm_vmcb_map_one(const struct vcpu *v,
struct vmcb_struct *va, paddr_t pa)
{
- return va;
+ return v == current ? va : map_domain_page(maddr_to_mfn(pa));
}
struct vmcb_struct *svm_vmcb_map(const struct vcpu *v)
@@ -56,6 +67,8 @@ struct vmcb_struct *svm_vmcb_map(const struct vcpu *v)
void svm_vmcb_unmap(const struct vcpu *v, const struct vmcb_struct *vmcb)
{
+ if ( v != current )
+ unmap_vcpu_state_page(vmcb);
}
/* This function can directly access fields which are covered by clean bits. */
@@ -219,26 +232,29 @@ int svm_create_vmcb(struct vcpu *v)
int rc;
if ( (nv->nv_n1vmcx == NULL) &&
- (nv->nv_n1vmcx = alloc_vmcb()) == NULL )
+ (nv->nv_n1vmcx = alloc_vmcb(v, VCPU_STATE_IDX_VMCB,
+ &nv->nv_n1vmcx_pa)) == NULL )
{
printk("Failed to create a new VMCB\n");
return -ENOMEM;
}
svm->vmcb = nv->nv_n1vmcx;
+ svm->vmcb_pa = nv->nv_n1vmcx_pa;
vmcb = svm_vmcb_map(v);
rc = construct_vmcb(v, vmcb);
svm_vmcb_unmap(v, vmcb);
if ( rc != 0 )
{
- free_vmcb(nv->nv_n1vmcx);
+ free_vmcb(v, VCPU_STATE_IDX_VMCB, nv->nv_n1vmcx_pa);
nv->nv_n1vmcx = NULL;
+ nv->nv_n1vmcx_pa = INVALID_PADDR;
svm->vmcb = NULL;
+ svm->vmcb_pa = INVALID_PADDR;
return rc;
}
- svm->vmcb_pa = nv->nv_n1vmcx_pa = virt_to_maddr(svm->vmcb);
return 0;
}
@@ -248,7 +264,7 @@ void svm_destroy_vmcb(struct vcpu *v)
struct svm_vcpu *svm = &v->arch.hvm.svm;
if ( nv->nv_n1vmcx != NULL )
- free_vmcb(nv->nv_n1vmcx);
+ free_vmcb(v, VCPU_STATE_IDX_VMCB, nv->nv_n1vmcx_pa);
if ( svm->msrpm != NULL )
{
@@ -260,6 +276,7 @@ void svm_destroy_vmcb(struct vcpu *v)
nv->nv_n1vmcx = NULL;
nv->nv_n1vmcx_pa = INVALID_PADDR;
svm->vmcb = NULL;
+ svm->vmcb_pa = INVALID_PADDR;
}
static void svm_dump_sel(const char *name, const struct segment_register *s)
diff --git a/xen/arch/x86/hvm/svm/vmcb.h b/xen/arch/x86/hvm/svm/vmcb.h
index 9d61767ec7..d899beb46e 100644
--- a/xen/arch/x86/hvm/svm/vmcb.h
+++ b/xen/arch/x86/hvm/svm/vmcb.h
@@ -556,12 +556,19 @@ struct vmcb_struct {
uint64_t res18[291];
};
-struct vmcb_struct *alloc_vmcb(void);
-void free_vmcb(struct vmcb_struct *vmcb);
+/*
+ * VMCBs are domain-heap pages: hardware addresses them physically, and
+ * Xen maps each one at a fixed slot in the owning vCPU's state window,
+ * so the returned virtual address is valid only in that vCPU's context.
+ * @idx is the VCPU_STATE_IDX_* slot to place the mapping in.
+ */
+struct vmcb_struct *alloc_vmcb(struct vcpu *v, unsigned int idx, paddr_t *pa);
+void free_vmcb(struct vcpu *v, unsigned int idx, paddr_t pa);
/*
- * Access a VMCB of a vCPU that may not be current. svm_vmcb_map() takes
- * the active VMCB, svm_vmcb_map_one() a specific one.
+ * Access a VMCB of a vCPU that may not be current: the window VA for
+ * current, a transient mapping otherwise. svm_vmcb_map() takes the active
+ * VMCB, svm_vmcb_map_one() a specific one.
*/
struct vmcb_struct *svm_vmcb_map_one(const struct vcpu *v,
struct vmcb_struct *va, paddr_t pa);
--
2.55.0
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |