diff mbox series

[v2,2/5] x86/paging: drop set-allocation from final-teardown

Message ID 99270ce9-39d8-1f3e-f922-afc2c0289205@suse.com (mailing list archive)
State New, archived
Headers show
Series x86/mm: address aspects noticed during XSA-410 work | expand

Commit Message

Jan Beulich Jan. 9, 2023, 1:39 p.m. UTC
The fixes for XSA-410 have arranged for P2M pages being freed by P2M
code to be properly freed directly, rather than being put back on the
paging pool list. Therefore whatever p2m_teardown() may return will no
longer need taking care of here. Drop the code, leaving the assertions
in place and adding "total" back to the PAGING_PRINTK() message.

With merely the (optional) log message and the assertions left, there's
really no point anymore to hold the paging lock there, so drop that too.

Requested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
The remaining parts of hap_final_teardown() could be moved as well, at
the price of a CONFIG_HVM conditional. I wasn't sure whether that was
deemed reasonable.
---
v2: New.

Comments

Roger Pau Monne March 16, 2023, 12:34 p.m. UTC | #1
On Mon, Jan 09, 2023 at 02:39:52PM +0100, Jan Beulich wrote:
> The fixes for XSA-410 have arranged for P2M pages being freed by P2M
> code to be properly freed directly, rather than being put back on the
> paging pool list. Therefore whatever p2m_teardown() may return will no
> longer need taking care of here. Drop the code, leaving the assertions
> in place and adding "total" back to the PAGING_PRINTK() message.
> 
> With merely the (optional) log message and the assertions left, there's
> really no point anymore to hold the paging lock there, so drop that too.
> 
> Requested-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

> ---
> The remaining parts of hap_final_teardown() could be moved as well, at
> the price of a CONFIG_HVM conditional. I wasn't sure whether that was
> deemed reasonable.

I think it's cleaner to leave them as-is.

Thanks, Roger.
diff mbox series

Patch

--- a/xen/arch/x86/mm/paging.c
+++ b/xen/arch/x86/mm/paging.c
@@ -866,22 +866,13 @@  void paging_final_teardown(struct domain
     /* It is now safe to pull down the p2m map. */
     p2m_teardown(p2m_get_hostp2m(d), true, NULL);
 
-    /* Free any paging memory that the p2m teardown released. */
-    paging_lock(d);
-
-    if ( hap )
-        hap_set_allocation(d, 0, NULL);
-    else
-        shadow_set_allocation(d, 0, NULL);
-
-    PAGING_PRINTK("%pd done: free = %u, p2m = %u\n",
-                  d, d->arch.paging.free_pages, d->arch.paging.p2m_pages);
+    PAGING_PRINTK("%pd done: total = %u, free = %u, p2m = %u\n",
+                  d, d->arch.paging.total_pages,
+                  d->arch.paging.free_pages, d->arch.paging.p2m_pages);
     ASSERT(!d->arch.paging.p2m_pages);
     ASSERT(!d->arch.paging.free_pages);
     ASSERT(!d->arch.paging.total_pages);
 
-    paging_unlock(d);
-
     p2m_final_teardown(d);
 }