| Message ID | 40042c40-a2ba-e491-d16a-4bacbfc6154e@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86: XSA-40{1,2,8} follow-up | expand |
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. 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
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
--- 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
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>