[PATCH 05/11] x86/shadow: move bogus HVM checks in sh_pagetable_dying()

Jan Beulich posted 11 patches 3 years, 1 month ago
There is a newer version of this series
[PATCH 05/11] x86/shadow: move bogus HVM checks in sh_pagetable_dying()
Posted by Jan Beulich 3 years, 1 month ago
Perhaps these should have been dropped right in 2fb2dee1ac62 ("x86/mm:
pagetable_dying() is HVM-only"). Convert both to assertions, noting that
in particular the one in the 3-level variant of the function comes too
late anyway - first thing there we access the HVM part of a union.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/mm/shadow/multi.c
+++ b/xen/arch/x86/mm/shadow/multi.c
@@ -3780,6 +3780,8 @@ static void cf_check sh_pagetable_dying(
     unsigned long l3gfn;
     mfn_t l3mfn;
 
+    ASSERT(is_hvm_domain(d));
+
     gcr3 = v->arch.hvm.guest_cr[3];
     /* fast path: the pagetable belongs to the current context */
     if ( gcr3 == gpa )
@@ -3822,7 +3824,7 @@ static void cf_check sh_pagetable_dying(
                    : shadow_hash_lookup(d, mfn_x(gmfn), SH_type_l2_pae_shadow);
         }
 
-        if ( mfn_valid(smfn) && is_hvm_domain(d) )
+        if ( mfn_valid(smfn) )
         {
             gmfn = _mfn(mfn_to_page(smfn)->v.sh.back);
             mfn_to_page(gmfn)->pagetable_dying = true;
@@ -3854,6 +3856,8 @@ static void cf_check sh_pagetable_dying(
     mfn_t smfn, gmfn;
     p2m_type_t p2mt;
 
+    ASSERT(is_hvm_domain(d));
+
     gmfn = get_gfn_query(d, _gfn(gpa >> PAGE_SHIFT), &p2mt);
     paging_lock(d);
 
@@ -3863,7 +3867,7 @@ static void cf_check sh_pagetable_dying(
     smfn = shadow_hash_lookup(d, mfn_x(gmfn), SH_type_l4_64_shadow);
 #endif
 
-    if ( mfn_valid(smfn) && is_hvm_domain(d) )
+    if ( mfn_valid(smfn) )
     {
         mfn_to_page(gmfn)->pagetable_dying = true;
         shadow_unhook_mappings(d, smfn, 1/* user pages only */);
Re: [PATCH 05/11] x86/shadow: move bogus HVM checks in sh_pagetable_dying()
Posted by Andrew Cooper 3 years, 1 month ago
On 05/01/2023 4:04 pm, Jan Beulich wrote:
> Perhaps these should have been dropped right in 2fb2dee1ac62 ("x86/mm:
> pagetable_dying() is HVM-only"). Convert both to assertions, noting that
> in particular the one in the 3-level variant of the function comes too

"came too late"?

It doesn't any more with this change in place.

> late anyway - first thing there we access the HVM part of a union.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Re: [PATCH 05/11] x86/shadow: move bogus HVM checks in sh_pagetable_dying()
Posted by Jan Beulich 3 years ago
On 06.01.2023 02:00, Andrew Cooper wrote:
> On 05/01/2023 4:04 pm, Jan Beulich wrote:
>> Perhaps these should have been dropped right in 2fb2dee1ac62 ("x86/mm:
>> pagetable_dying() is HVM-only"). Convert both to assertions, noting that
>> in particular the one in the 3-level variant of the function comes too
> 
> "came too late"?
> 
> It doesn't any more with this change in place.

Fine with me either way, so I've changed it. Iirc in particular George has
been advocating for writing descriptions in present tense, even if in the
course of a change the stated fact changes as well.

>> late anyway - first thing there we access the HVM part of a union.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

Thanks.

Jan