While "type" can include PGT_pae_xen_l2, "x" can't, as the bit is
cleared upon de-validation (see also the respective assertion earlier in
the function).

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

--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -3020,7 +3020,7 @@ static int _get_page_type(struct page_in
         if ( d && shadow_mode_enabled(d) )
             shadow_prepare_page_type_change(d, page);
 
-        if ( (x & PGT_type_mask) != type )
+        if ( (x ^ type) & PGT_type_mask )
         {
             /*
              * On type change we check to flush stale TLB entries. It is

On 26/07/2022 17:06, Jan Beulich wrote:
> While "type" can include PGT_pae_xen_l2, "x" can't, as the bit is
> cleared upon de-validation (see also the respective assertion earlier in
> the function).

While this statement is true, it doesn't really explain why this is
relevant (or not) to TLB flushing.

As far as the change goes, it's safe on 64bit builds of Xen (I think),
but would not be on 32bit builds when this logic was first written.

~Andrew

On 27.07.2022 20:25, Andrew Cooper wrote:
> On 26/07/2022 17:06, Jan Beulich wrote:
>> While "type" can include PGT_pae_xen_l2, "x" can't, as the bit is
>> cleared upon de-validation (see also the respective assertion earlier in
>> the function).
> 
> While this statement is true, it doesn't really explain why this is
> relevant (or not) to TLB flushing.
> 
> As far as the change goes, it's safe on 64bit builds of Xen (I think),
> but would not be on 32bit builds when this logic was first written.

Actually no, I don't think it's safe, and I therefore withdraw the
patch. (I'll re-base the subsequent one accordingly, which you did
give R-b for already.) Whatever content may have been in an L2 table
which is to become a PGT_pae_xen_l2 one may still be in TLBs / paging
structure caches, yet we'll replace those entries without further
flushing.

The opposite direction (previously PGT_pae_xen_l2 trying to become an
ordinary L2) is "fine": Due to the Xen entries still there, its
validation will simply fail.

Jan

On 27.07.2022 20:25, Andrew Cooper wrote:
> On 26/07/2022 17:06, Jan Beulich wrote:
>> While "type" can include PGT_pae_xen_l2, "x" can't, as the bit is
>> cleared upon de-validation (see also the respective assertion earlier in
>> the function).
> 
> While this statement is true, it doesn't really explain why this is
> relevant (or not) to TLB flushing.
> 
> As far as the change goes, it's safe on 64bit builds of Xen (I think),
> but would not be on 32bit builds when this logic was first written.

How that? The flush is needed if the type changes. If (for the purposes
here) PGT_pae_xen_l2 was considered part of the type, then the bit being
lost upon de-validation would mean we're flushing too little.

Jan