Re: [PATCH v3 1/2][4.15] x86/PV: conditionally avoid raising #GP for early guest MSR reads

[Jan Beulich <jbeulich@xxxxxxxx>]

On 12.03.2021 12:19, Andrew Cooper wrote:
> On 12/03/2021 07:54, 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 this paths conditional upon the guest having
>> installed a handler, provided of course the MSR can be read in the first
>> place (we would have raised #GP in that case even before). Producing
>> zero for reads 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 <jbeulich@xxxxxxxx>
>> Release-Acked-by: Ian Jackson <iwj@xxxxxxxxxxxxxx>
> 
> I am still of the firm belief that this is the wrong course of action.
> 
> It is deliberately papering over error handling bugs which, in the
> NetBSD case, literally created memory corruption scenarios.  (Yes - that
> was a WRMSR not RDMSR, but the general point still stands, particularly
> in light of your expectation to do the same to the WRMSR).
> 
> It is one thing to not realise that we have a trainwreck here.  Its
> totally another to take wilful action to keep current and all future
> guests broken in the same way.
> 
> The *only* case where it is acceptable to skip error handling is if the
> VM admin has specifically signed their life away to say that they'll
> accept the, now discovered, potential-memory-corrupion consequences.
> 
> Rogers patch already does this.

With _much_ bigger impact - it requires changing the behavior for the
entire lifetime of the domain, rather than just very early boot. And
as you may have seen, despite my fear that it may not be enough, Roger
and I have agreed to leave the WRMSR path alone.

Jan