On 29.04.2022 10:00, Roger Pau Monné wrote: > On Thu, Apr 28, 2022 at 05:53:17PM +0200, Jan Beulich wrote: >> Hello, >> >> in the course of analyzing the i915 driver causing boot to fail in >> Linux 5.18 I found that Linux, for all the years, has been running >> in PV mode as if PAT was (mostly) disabled. This is a result of >> them tying PAT initialization to MTRR initialization, while we >> offer PAT but not MTRR in CPUID output. This was different before >> our moving to CPU featuresets, and as such one could view this >> behavior as a regression from that change. >> >> The first question here is whether not exposing MTRR as a feature >> was really intended, in particular also for PV Dom0. The XenoLinux >> kernel and its forward ports did make use of XENPF_*_memtype to >> deal with MTRRs. That's functionality which (maybe for a good >> reason) never made it into the pvops kernel. Note that PVH Dom0 >> does have access to the original settings, as the host values are >> used as initial state there. >> >> The next question would be how we could go about improving the >> situation. For the particular issue in 5.18 I've found a relatively >> simple solution [1] (which also looks to help graphics performance >> on other systems, according to my initial observations with using >> the change), albeit its simplicity likely means it either is wrong >> in some way, or might not be liked for looking hacky and/or abusive. > > I wonder whether the patch needs to be limited to the CPUID Hypervisor > bit being present. If the PAT MSR is readable and returns a value != > 0 then PAT should be available?
I simply didn't want to be too "aggressive". There may be reasons why they want things to be the way they are on native. All I really care about is that things are broken on PV Xen; as such I wouldn't even mind tightening the check to xen_pv_domain(). But first I need feedback from the maintainers there anyway. > I guess we should instead check that the current PAT value matches > what Linux expects, before declaring PAT enabled? I don't think such a check is needed, the code ... > Note there's already a comment in init_cache_modes that refers to Xen > in the check for PAT CPUID bit. ... in __init_cache_modes() already does it the other way around: Adapt behavior to what is found in PAT. > We might want to expand that comment > (or add one to the new check you are adding). I don't see what further information you would want to put there. Jan
