[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v5 02/10] x86/mm: Avoid hard-coding PAT in get_page_from_l1e()
On Thu, Dec 22, 2022 at 10:51:08AM +0100, Jan Beulich wrote: > On 20.12.2022 09:19, Jan Beulich wrote: > > On 20.12.2022 02:07, Demi Marie Obenour wrote: > >> get_page_from_l1e() relied on Xen's choice of PAT, which is brittle in > >> the face of future PAT changes. Instead, compute the actual cacheability > >> used by the CPU and switch on that, as this will work no matter what PAT > >> Xen uses. > >> > >> No functional change intended. This code is itself questionable and may > >> be removed in the future, but removing it would be an observable > >> behavior change and so is out of scope for this patch series. > >> > >> Signed-off-by: Demi Marie Obenour <demi@xxxxxxxxxxxxxxxxxxxxxx> > >> --- > >> Changes since v4: > >> - Do not add new pte_flags_to_cacheability() helper, as this code may be > >> removed in the near future and so adding a new helper for it is a bad > >> idea. > >> - Do not BUG() in the event of an unexpected cacheability. This cannot > >> happen, but it is simpler to force such types to UC than to prove that > >> the BUG() is not reachable. > >> > >> Changes since v3: > >> - Compute and use the actual cacheability as seen by the processor. > >> > >> Changes since v2: > >> - Improve commit message. > >> --- > >> xen/arch/x86/mm.c | 14 ++++++++------ > >> 1 file changed, 8 insertions(+), 6 deletions(-) > >> > >> diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c > >> index > >> 78b1972e4170cacccc9c37c6e64e76e66a7da87f..dba6c77ef2f7ed7fcb7f7e526583ccadd35e62cc > >> 100644 > >> --- a/xen/arch/x86/mm.c > >> +++ b/xen/arch/x86/mm.c > >> @@ -959,14 +959,16 @@ get_page_from_l1e( > >> flip = _PAGE_RW; > >> } > >> > >> - switch ( l1f & PAGE_CACHE_ATTRS ) > >> + switch ( 0xFF & (XEN_MSR_PAT >> (8 * > >> pte_flags_to_cacheattr(l1f))) ) > >> { > >> - case 0: /* WB */ > >> - flip |= _PAGE_PWT | _PAGE_PCD; > >> + case X86_MT_UC: > >> + case X86_MT_UCM: > >> + case X86_MT_WC: > >> + /* not cacheable */ > >> break; > >> - case _PAGE_PWT: /* WT */ > >> - case _PAGE_PWT | _PAGE_PAT: /* WP */ > >> - flip |= _PAGE_PCD | (l1f & _PAGE_PAT); > >> + default: > >> + /* cacheable */ > >> + flip |= ((l1f & PAGE_CACHE_ATTRS) ^ _PAGE_UC); > >> break; > > > > In v4 the comment here was "cacheable, force to UC". The latter aspect is > > quite relevant (and iirc also what Andrew had asked for to have as a > > comment). But with this now being the default case, the comment (in either > > this or the earlier form) would become stale. A forward compatible way of > > wording this would e.g. be "force any other type to UC". With an adjustment > > along these lines (which I think could be done while committing) > > Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx> > > If you had replied signaling your consent (and perhaps the preferred by you > wording), I would have committed this. Now it's going to be v6 afaic ... > > Jan Sorry about that. "potentially cacheable, force to UC" is the wording I have planned for v6, along with "not cacheable, allow" in the other case. Feel free to go ahead and commit if you want. -- Sincerely, Demi Marie Obenour (she/her/hers) Invisible Things Lab Attachment:
signature.asc
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |