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

[PATCH v2 2/4] x86/shadow: slightly consolidate sh_unshadow_for_p2m_change() (part II)


  • To: "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Tue, 19 Jul 2022 08:51:45 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; 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=n/85LX8vVEsmVTn+P11FrOvSAKeuh/W8CeK3ApKBU1M=; b=fPr19Iy+ho4ZIo93SFtOEwThfKn/m15xv3LYegp1QIw0myQk/r6UYMAh2Azaq34gmgXnWOjRFLc7V9OBp+twtCp+SQPWYuIP0CBJqr6UgOSJBk7NjU8uZCd7fS86P8u/UGAwSixcnyBwElHfqr8pi4n6lWdNcYv4Dw6oSjHOVsFxXIZHntKQT28YVx1FXyNAIlf4XhqRudzQlPbup3kCywzqBqgB5T+CAW9MCWhh/nWD41q0SDNVEDu0nD33apVl7Aql9/DaRXLndnzdqDaq4+ZdA8RIiUklz6+es/y5tPJjf3FNb7KdnQYiHGLuTub+sp/TrMLzKTd37JXtd32AuA==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=AKXvUG0OV/UPnNhsehUSZ+3S89HUpoWJSBHAe+DpWBSk7ddS/VemjKezEgIqkf1p4E2RRt8pKXCtMtY3+zlNmBNHcSyoL9Sghz2Sr1u9Ce1UExpuwPX8WDf13DyJMtdj/DOEDewcAAoyJrsQMXSWR9W5bth4aNhy67xjt/WeH9yVXmEhD6+vkqXoiHjxOtYREAOk4iWvj4B8IKdmqvRFbH4bkTQ4w0vaqdsgDiHsFbgAUeoNOW/vmpJ4Rmd5lGj9zYE4iTV9se+j6gxIcTI7HPEYiLYuAE6vl6GwKbXkUt6dCXwZNOmxJEmnJ6g9wzlI0Ht31Ojkk1kOdVx0jzK7Vw==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
  • Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>, Tim Deegan <tim@xxxxxxx>
  • Delivery-date: Tue, 19 Jul 2022 06:51:52 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

Pull common checks out of the switch(). This includes extending a
_PAGE_PRESENT check to L1 as well, which presumably was deemed redundant
with p2m_is_valid() || p2m_is_grant(), but I think we are better off
being explicit in all cases. Note that for L2 (or higher) the grant
check isn't strictly necessary, as grants are only ever single pages.
Leave a respective assertion.

For L1 replace the moved out condition with an MFN comparison: There's
no need for any update or flushing when the two match.

Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
---
v2: Split from previous bigger patch. Add grant related assertion for
    L2.

--- a/xen/arch/x86/mm/shadow/hvm.c
+++ b/xen/arch/x86/mm/shadow/hvm.c
@@ -798,7 +798,7 @@ static void cf_check sh_unshadow_for_p2m
     struct domain *d, unsigned long gfn, l1_pgentry_t old, l1_pgentry_t new,
     unsigned int level)
 {
-    mfn_t omfn = l1e_get_mfn(old);
+    mfn_t omfn = l1e_get_mfn(old), nmfn;
     unsigned int oflags = l1e_get_flags(old);
     p2m_type_t p2mt = p2m_flags_to_type(oflags);
     bool flush = false;
@@ -810,19 +810,30 @@ static void cf_check sh_unshadow_for_p2m
     if ( unlikely(!d->arch.paging.shadow.total_pages) )
         return;
 
+    /* Only previously present / valid entries need processing. */
+    if ( !(oflags & _PAGE_PRESENT) ||
+         (!p2m_is_valid(p2mt) && !p2m_is_grant(p2mt)) ||
+         !mfn_valid(omfn) )
+        return;
+
+    nmfn = l1e_get_flags(new) & _PAGE_PRESENT ? l1e_get_mfn(new) : INVALID_MFN;
+
     switch ( level )
     {
     default:
         /*
          * The following assertion is to make sure we don't step on 1GB host
-         * page support of HVM guest.
+         * page support of HVM guest. Plus we rely on ->set_entry() to never
+         * be called with orders above PAGE_ORDER_2M, not even to install
+         * non-present entries (which in principle ought to be fine even
+         * without respective large page support).
          */
-        ASSERT(!((oflags & _PAGE_PRESENT) && (oflags & _PAGE_PSE)));
+        ASSERT(!(oflags & _PAGE_PSE));
         break;
 
     /* If we're removing an MFN from the p2m, remove it from the shadows too */
     case 1:
-        if ( (p2m_is_valid(p2mt) || p2m_is_grant(p2mt)) && mfn_valid(omfn) )
+        if ( !mfn_eq(nmfn, omfn) )
         {
             sh_remove_all_shadows_and_parents(d, omfn);
             if ( sh_remove_all_mappings(d, omfn, _gfn(gfn)) )
@@ -836,13 +847,13 @@ static void cf_check sh_unshadow_for_p2m
      * scheme, that's OK, but otherwise they must be unshadowed.
      */
     case 2:
-        if ( !(oflags & _PAGE_PRESENT) || !(oflags & _PAGE_PSE) )
+        if ( !(oflags & _PAGE_PSE) )
             break;
 
-        if ( p2m_is_valid(p2mt) && mfn_valid(omfn) )
+        ASSERT(!p2m_is_grant(p2mt));
+
         {
             unsigned int i;
-            mfn_t nmfn = l1e_get_mfn(new);
             l1_pgentry_t *npte = NULL;
 
             /* If we're replacing a superpage with a normal L1 page, map it */




 


Rackspace

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