[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH v3 02/18] x86/mm: purge unneeded destroy_perdomain_mapping()



From: Roger Pau Monné <roger.pau@xxxxxxxxxx>

We want to change per-domain mappings to be per-vCPU mappings.  In
preparation for that, we want to arrange that
destroy_perdomain_mapping() work either with a single perdomain area,
or with a per-vCPU perdomain area.

There are two calls made from domain-scoped contexts; both calls turn
out to be unnecessary:

- destroy_perdomain_mapping() is not logically the undo of
  create_perdomain_mapping(), as the name and its use in
  hvm_domain_initialise() suggest.  create_ allocates a per-domain L3,
  but destroy_ tears down mappings without freeing it; and since the
  call here passes nr == 0, it tears down nothing at all.  The
  per-domain L3 page is actually freed by free_perdomain_mappings(),
  which hvm_domain_initialise()'s caller, arch_domain_create(),
  already invokes on its failure path.

- The call in pv_domain_destroy() is redundant at both of its call
  sites.  arch_domain_destroy() unconditionally calls
  free_perdomain_mappings(), which tears down the same entries and
  additionally frees the page-table structures; and the other caller,
  the error path of pv_domain_initialise(), returns into
  arch_domain_create()'s failure path, which invokes
  free_perdomain_mappings() as well.

One vCPU-scoped caller, pv_destroy_gdt_ldt_l1tab(), is covered the
same way: a vCPU is only ever destroyed together with its domain, or
on the failure path of creating it, and both routes end in
free_perdomain_mappings(); the GDT/LDT slots hold no area-owned pages,
so there is nothing else for the call to do.  While we're getting rid
of unneccessary callers, drop this one too.

Note that free_compat_arg_xlat() keeps its explicit teardown: in
addition to vCPU teardown, it serves switch_compat()'s undo path,
where the domain lives on as a 64-bit one and the argument-translation
pages have to go.  Add a comment to make sure it isn't missed.

Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
Assisted-by: Claude Code:claude-fable-5
Signed-off-by: George Dunlap <gwd@xxxxxxxxxxxxxx>
---
Changes in v3:
- Note that pv_domain_destroy()'s other call site, the error path of
  pv_domain_initialise(), is covered the same way (Jan).
- Also drop pv_destroy_gdt_ldt_l1tab(), the per-vCPU teardown call
  (Jan), and comment on why free_compat_arg_xlat() keeps its own.  The
  Reviewed-by tags given for the previous form are dropped.

Changes in v2:
- Added to the series

Changes since the previously posted version:
- Reworked the commit message to make it more clear how it fits in
  with the larger series.  No functional change.
---
 xen/arch/x86/hvm/hvm.c   |  1 -
 xen/arch/x86/pv/domain.c | 10 ----------
 xen/arch/x86/x86_64/mm.c |  5 +++++
 3 files changed, 5 insertions(+), 11 deletions(-)

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 192309c2fc..3924c1b204 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -730,7 +730,6 @@ int hvm_domain_initialise(struct domain *d,
     XFREE(d->arch.hvm.irq);
  fail0:
     hvm_destroy_cacheattr_region_list(d);
-    destroy_perdomain_mapping(d, PERDOMAIN_VIRT_START, 0);
  fail:
     hvm_domain_relinquish_resources(d);
     XFREE(d->arch.hvm.io_handler);
diff --git a/xen/arch/x86/pv/domain.c b/xen/arch/x86/pv/domain.c
index 03262a4952..07ea4bb4d2 100644
--- a/xen/arch/x86/pv/domain.c
+++ b/xen/arch/x86/pv/domain.c
@@ -332,12 +332,6 @@ static int pv_create_gdt_ldt_l1tab(struct vcpu *v)
                                     NULL);
 }
 
-static void pv_destroy_gdt_ldt_l1tab(struct vcpu *v)
-{
-    destroy_perdomain_mapping(v->domain, GDT_VIRT_START(v),
-                              1U << GDT_LDT_VCPU_SHIFT);
-}
-
 void pv_vcpu_destroy(struct vcpu *v)
 {
     if ( is_pv_32bit_vcpu(v) )
@@ -346,7 +340,6 @@ void pv_vcpu_destroy(struct vcpu *v)
         release_compat_l4(v);
     }
 
-    pv_destroy_gdt_ldt_l1tab(v);
     XFREE(v->arch.pv.trap_ctxt);
 }
 
@@ -394,9 +387,6 @@ void pv_domain_destroy(struct domain *d)
 {
     pv_l1tf_domain_destroy(d);
 
-    destroy_perdomain_mapping(d, GDT_LDT_VIRT_START,
-                              GDT_LDT_MBYTES << (20 - PAGE_SHIFT));
-
     XFREE(d->arch.pv.cpuidmasks);
 
     FREE_XENHEAP_PAGE(d->arch.pv.gdt_ldt_l1tab);
diff --git a/xen/arch/x86/x86_64/mm.c b/xen/arch/x86/x86_64/mm.c
index 8eadab7933..2a1805964d 100644
--- a/xen/arch/x86/x86_64/mm.c
+++ b/xen/arch/x86/x86_64/mm.c
@@ -737,6 +737,11 @@ int setup_compat_arg_xlat(struct vcpu *v)
                                     NULL, NIL(struct page_info *));
 }
 
+/*
+ * Besides vCPU teardown, which free_perdomain_mappings() would cover, this
+ * serves switch_compat()'s undo path: the domain lives on as a 64-bit one
+ * there, so the translation area has to go right away.
+ */
 void free_compat_arg_xlat(struct vcpu *v)
 {
     destroy_perdomain_mapping(v->domain, ARG_XLAT_START(v),
-- 
2.55.0




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.