On Fri, Feb 05, 2021 at 11:32:07AM +0000, Andrew Cooper wrote: > On 05/02/2021 10:56, Jürgen Groß wrote: > > On 05.02.21 11:14, Jan Beulich wrote: > >> (simply re-sending what was sent over 2 months ago) > >> > >> On 04.11.2020 11:50, Jan Beulich wrote: > >>> On 03.11.2020 18:31, Andrew Cooper wrote: > >>>> On 03/11/2020 17:06, Jan Beulich wrote: > >>>>> Prior to 4.15 Linux, when running in PV mode, did not install a #GP > >>>>> handler early enough to cover for example the rdmsrl_safe() of > >>>>> MSR_K8_TSEG_ADDR in bsp_init_amd() (not to speak of the unguarded > >>>>> read > >>>>> of MSR_K7_HWCR later in the same function). The respective change > >>>>> (42b3a4cb5609 "x86/xen: Support early interrupts in xen pv > >>>>> guests") was > >>>>> backported to 4.14, but no further - presumably since it wasn't > >>>>> really > >>>>> easy because of other dependencies. > >>>>> > >>>>> Therefore, to prevent our change in the handling of guest MSR > >>>>> accesses > >>>>> to render PV Linux 4.13 and older unusable on at least AMD > >>>>> systems, make > >>>>> the raising of #GP on these paths conditional upon the guest having > >>>>> installed a handler. Producing zero for reads and discarding writes > >>>>> isn't necessarily correct and may trip code trying to detect > >>>>> presence of > >>>>> MSRs early, but since such detection logic won't work without a #GP > >>>>> handler anyway, this ought to be a fair workaround. > >>>>> > >>>>> Signed-off-by: Jan Beulich <jbeul...@suse.com> > >>>> > >>>> I appreciate that we probably have to do something, but I don't think > >>>> this is a wise move. > >>> > >>> I wouldn't call it wise either, but I'm afraid something along > >>> these lines is necessary. > >>> > >>>> Linux is fundamentally buggy. It is deliberately looking for a > >>>> potential #GP fault given its use of rdmsrl_safe(). The reason > >>>> this bug > >>>> stayed hidden for so long was as a consequence of Xen's inappropriate > >>>> MSR handling for guests, and the reasons for changing Xen's behaviour > >>>> still stand. > >>> > >>> I agree. > >>> > >>>> This change, in particular, does not apply to any explicitly handled > >>>> MSRs, and therefore is not a comprehensive fix. > >>> > >>> But it's intentional that this deals with the situation in a > >>> generic way, not on a per-MSR basis. If we did as you suggest > >>> further down, we'd have to audit all Linux versions up to 4.14 > >>> for similar issues with other MSRs. I don't think this would > >>> be a practical thing to do, and I also don't think that leaving > >>> things as they are until we have concrete reports of problems > >>> is a viable option either. > >>> > >>> Adding explicit handling for the two offending MSRs (and any > >>> possible further ones we discover) would imo only be to avoid > >>> issuing the respective log messages. > >>> > >>>> Nor is it robust to > >>>> someone adding code to explicitly handling the impacted MSRs at a > >>>> later > >>>> date (which are are likely to need to do for HWCR), and which would > >>>> reintroduce this failure to boot. > >>> > >>> I'm afraid I don't understand. Looking at the two functions > >>> the patch alters, only X86EMUL_OKAY is used in return statements > >>> other than the final one. If this model is to be followed by > >>> future additions (which I think it ought to be; perhaps we > >>> should add comments to this effect), the code introduced here > >>> will take care of the situation nevertheless. > >>> > >>>> We should have the impacted MSRs handled explicitly, with a note > >>>> stating > >>>> this was a bug in Linux 4.14 and older. We already have workaround > >>>> for > >>>> similar bugs in Windows, and it also gives us a timeline to eventually > >>>> removing support for obsolete workarounds, rather than having a > >>>> "now and > >>>> in the future, we'll explicitly tolerate broken PV behaviour for > >>>> one bug > >>>> back in ancient linux". > >>> > >>> Comparing with Windows isn't very helpful; the patch here is > >>> specifically about PV, and would help other OSes as well in > >>> case they would have missed setting up exceptions early in > >>> just the PV-on-Xen case. For the HVM case I'd indeed rather > >>> see us go the route we've gone for Windows, if need be. > >> > >> As can be seen from this reply, we're not in agreement what to > >> do here. But we need to do something. I'm not sure how to get > >> unstuck discussions like this one ... > >> > >> Besides this suggestion of yours I also continue to have > >> trouble seeing what good it will do to record an exception to > >> inject into a guest when we know it didn't install a handler > >> yet. > > > > As we need to consider backports of processor bug mitigations > > in old guests, too, I think we need to have a "catch-all" > > fallback. > > > > Not being able to run an old updated guest until we add handling > > for a new MSR isn't a viable option IMO. > > You're trading off against issuing XSAs for all the unknown quantities > of sensitive in MSRs on all past and future platforms. This has > unbounded scope. > > Xen's previous behaviour was astoundingly stupid, and yes - we're > playing more than a decades worth of catchup in one release cycle. > > I'll absolutely take "care/tweaks need to happen crossing the Xen > 4.14=>4.15 boundary" over whack-a-mole for MSRs in the form of security > advisories.
I think I'm likely missing part of the point here - Jan's patch would just return 0 for reads, so there's no leak of unhandled MSRs? Hence I'm not seeing the XSA aspect of this. Roger.
