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

[PATCH v3] x86/altp2m: p2m_altp2m_propagate_change() should honor present page order


  • To: "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Thu, 27 Jan 2022 16:06:53 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=none; dmarc=none; dkim=none; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=3ERwCWiy09e/X7t2Ph27wCKPGARBVC8AE1wNG+vQlrM=; b=LxQJ0U/6jAa8WyW3kY55t3o5yob5H809/wuCFvJ1zRxM6xo18LXTtqBmNGQG7wwIkGz+D8papCXLrDavPxUifoLOOS2/6ntWPZyBte4ZDrBmPJokXyaOOq+AuCkQi0+BTdznvAznlbSwnjbM5Elc6nfzF5WACtDG1fl2jGtihtEeLSmD+nrxsxz4HrbBBJXnN4jV9Tg6rGoXGDcTXcfGbW38s60+9N8+8imBslMA9631LslCg/MVs+wzvjOn58vI2e7mhabHtlpU41sNf+BEQzU9Uw4TyfdoDehlBT1bLa3lMKKidZ7evYK/U6f/6a3uIYU8UzV1rRpICo79FLwoSg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=LYvvsp2As2D9tJ0nTTiS4u02ubzE1o/NpgxvXUmGuYn5Up6+/LCDsp6mEJd5yfAFEyCdESjgUOY2JAqWl4V0dCtVGj65KinkrYmsMLcPekSSUHbfds+/cDKpSqsRFifPtZKJiJYF9ctjcZdCyOl8+tzUh1kgLfjXyjye1FmtgwcAY8vnAJQ6DAcHP7QXXMZXC4c3VxIMymSRAwSSU+oJJi9XihV2zKH3bRxTwrn276EWH0s+Q5G7xIjnpUkJwPT3zAU9+tyiY/rg107nSlsXRhQ56W9n5XcBlYff8QpuB5QFxV3ILnOTVC+dXLqapFXPeih7k81HwZKb1q39Pg/otQ==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
  • Cc: Tamas K Lengyel <tamas@xxxxxxxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Petre Pircalabu <ppircalabu@xxxxxxxxxxxxxxx>, Alexandru Isaila <aisaila@xxxxxxxxxxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>
  • Delivery-date: Thu, 27 Jan 2022 15:07:02 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

For higher order mappings the comparison against p2m->min_remapped_gfn
needs to take the upper bound of the covered GFN range into account, not
just the base GFN. Otherwise, i.e. when dropping a mapping overlapping
the remapped range but the base GFN outside of that range, an altp2m may
wrongly not get reset.

Note that there's no need to call get_gfn_type_access() ahead of the
check against the remapped range boundaries: None of its outputs are
needed earlier, and p2m_reset_altp2m() doesn't require the lock to be
held. In fact this avoids a latent lock order violation: With per-GFN
locking p2m_reset_altp2m() not only doesn't require the GFN lock to be
held, but holding such a lock would actually not be allowed, as the
function acquires a P2M lock.

Local variables are moved into the more narrow scope (one is deleted
altogether) to help see their actual life ranges.

Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
---
Note that this addresses only half of the problem: get_gfn_type_access()
would also need invoking for all of the involved GFNs, not just the 1st
one.
---
v3: Don't pass the minimum of both orders to p2m_set_entry() (as was the
    case in the original code). Restore get_gfn_type_access() return
    value checking.

--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -2534,9 +2534,6 @@ int p2m_altp2m_propagate_change(struct d
                                 p2m_type_t p2mt, p2m_access_t p2ma)
 {
     struct p2m_domain *p2m;
-    p2m_access_t a;
-    p2m_type_t t;
-    mfn_t m;
     unsigned int i;
     unsigned int reset_count = 0;
     unsigned int last_reset_idx = ~0;
@@ -2549,15 +2546,16 @@ int p2m_altp2m_propagate_change(struct d
 
     for ( i = 0; i < MAX_ALTP2M; i++ )
     {
+        p2m_type_t t;
+        p2m_access_t a;
+
         if ( d->arch.altp2m_eptp[i] == mfn_x(INVALID_MFN) )
             continue;
 
         p2m = d->arch.altp2m_p2m[i];
-        m = get_gfn_type_access(p2m, gfn_x(gfn), &t, &a, 0, NULL);
 
         /* Check for a dropped page that may impact this altp2m */
-        if ( mfn_eq(mfn, INVALID_MFN) &&
-             gfn_x(gfn) >= p2m->min_remapped_gfn &&
+        if ( gfn_x(gfn) + (1UL << page_order) > p2m->min_remapped_gfn &&
              gfn_x(gfn) <= p2m->max_remapped_gfn )
         {
             if ( !reset_count++ )
@@ -2568,8 +2566,6 @@ int p2m_altp2m_propagate_change(struct d
             else
             {
                 /* At least 2 altp2m's impacted, so reset everything */
-                __put_gfn(p2m, gfn_x(gfn));
-
                 for ( i = 0; i < MAX_ALTP2M; i++ )
                 {
                     if ( i == last_reset_idx ||
@@ -2583,16 +2579,19 @@ int p2m_altp2m_propagate_change(struct d
                 break;
             }
         }
-        else if ( !mfn_eq(m, INVALID_MFN) )
+        else if ( !mfn_eq(get_gfn_type_access(p2m, gfn_x(gfn), &t, &a, 0,
+                                              NULL), INVALID_MFN) )
         {
             int rc = p2m_set_entry(p2m, gfn, mfn, page_order, p2mt, p2ma);
 
             /* Best effort: Don't bail on error. */
             if ( !ret )
                 ret = rc;
-        }
 
-        __put_gfn(p2m, gfn_x(gfn));
+            __put_gfn(p2m, gfn_x(gfn));
+        }
+        else
+            __put_gfn(p2m, gfn_x(gfn));
     }
 
     altp2m_list_unlock(d);




 


Rackspace

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