| Message ID | 20250108142659.99490-2-roger.pau@citrix.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | x86: adventures in Address Space Isolation | expand |
Hi, I noticed the same duplication while moving mapcache initialization code, but didn't want to touch it while doing that. Good to see these two lines gone. On Wed Jan 8, 2025 at 2:26 PM GMT, Roger Pau Monne wrote: > The destroy_perdomain_mapping() call in the hvm_domain_initialise() fail path > is useless. destroy_perdomain_mapping() called with nr == 0 is effectively a > no op, as there are not entries torn down. Remove the call, as > arch_domain_create() already calls free_perdomain_mappings() on failure. > > There's also a call to destroy_perdomain_mapping() in pv_domain_destroy() which > is also not needed. arch_domain_destroy() will already unconditionally call > free_perdomain_mappings(), which does the same as destroy_perdomain_mapping(), > plus additionally frees the page table structures. > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > --- > xen/arch/x86/hvm/hvm.c | 1 - > xen/arch/x86/pv/domain.c | 3 --- > 2 files changed, 4 deletions(-) > > diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c > index 922c9b3af64d..70fdddae583d 100644 > --- a/xen/arch/x86/hvm/hvm.c > +++ b/xen/arch/x86/hvm/hvm.c > @@ -708,7 +708,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 7aef628f55be..bc7cd0c62f0e 100644 > --- a/xen/arch/x86/pv/domain.c > +++ b/xen/arch/x86/pv/domain.c > @@ -345,9 +345,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); Reviewed-by: Alejandro Vallejo <alejandro.vallejo@cloud.com> Cheers, Alejandro
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c index 922c9b3af64d..70fdddae583d 100644 --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -708,7 +708,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 7aef628f55be..bc7cd0c62f0e 100644 --- a/xen/arch/x86/pv/domain.c +++ b/xen/arch/x86/pv/domain.c @@ -345,9 +345,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);
The destroy_perdomain_mapping() call in the hvm_domain_initialise() fail path is useless. destroy_perdomain_mapping() called with nr == 0 is effectively a no op, as there are not entries torn down. Remove the call, as arch_domain_create() already calls free_perdomain_mappings() on failure. There's also a call to destroy_perdomain_mapping() in pv_domain_destroy() which is also not needed. arch_domain_destroy() will already unconditionally call free_perdomain_mappings(), which does the same as destroy_perdomain_mapping(), plus additionally frees the page table structures. Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- xen/arch/x86/hvm/hvm.c | 1 - xen/arch/x86/pv/domain.c | 3 --- 2 files changed, 4 deletions(-)