Re: [PATCH RFC for-4.15] x86/msr: introduce an option for legacy MSR behavior selection

[Jan Beulich <jbeulich@xxxxxxxx>]

On 04.03.2021 11:05, Roger Pau Monné wrote:
> On Thu, Mar 04, 2021 at 09:48:25AM +0100, Jan Beulich wrote:
>> On 03.03.2021 16:38, Roger Pau Monné wrote:
>>> It also raises the question whether we will allow any more exceptions
>>> to the MSR policy, like we did for Windows and OpenBSD in:
>>>
>>> http://xenbits.xen.org/gitweb/?p=xen.git;a=commit;h=ca88a43e660c75796656a544e54a648c60d26ef0
>>> http://xenbits.xen.org/gitweb/?p=xen.git;a=commit;h=4175fd3ccd17face664036fa98e9329aa317f7b6
>>>
>>> Or if we are just going to require those guests to enable the legacy
>>> MSR handling instead.
>>
>> It is my understanding that Andrew's view is that adding such special
>> cases is the only acceptable thing. As voiced before I don't share
>> this view, as it means we're going to be entirely reactive to people
>> reporting issues, when I think we should be pro-active as far as is
>> reasonable. Independent of any pro-active measures there'll still be
>> enough reasons for reactive changes - for example I assume Linux
>> would now issue the log message from
>>
>>      if (cpu_has(c, X86_FEATURE_CONSTANT_TSC)) {
>>
>>              if (c->x86 > 0x10 ||
>>                  (c->x86 == 0x10 && c->x86_model >= 0x2)) {
>>                      u64 val;
>>
>>                      rdmsrl(MSR_K7_HWCR, val);
>>                      if (!(val & BIT(24)))
>>                              pr_warn(FW_BUG "TSC doesn't count with P0 
>> frequency!\n");
>>              }
>>      }
>>
>> since we surface a zero value right now (but I didn't verify this in
>> practice yet).
> 
> I think we inject a #GP to the guest if it tries to access
> MSR_K7_HWCR? As I don't see this MSR handled explicitly in
> svm_msr_read_intercept. So Linux would complain from the unchecked MSR
> access and the MSR value not having the bit set.

Right - my description was of the behavior with my workaround
already in place.

> This one seems like a fine candidate to implement in
> svm_msr_read_intercept, because Xen needs to return a specific value
> for this MSR.
> 
> Regarding the global approach to fixing the fallout from the MSR
> policy change, I don't see why we couldn't do a mix between pro-active
> and reactive.
> 
> I think we must have an option to fallback to something similar to the
> old policy for HVM guests so that users have a way to get their guests
> running after an update without requiring a code change.
> 
> That doesn't mean we can't reactively add the missing MSRs as we go
> discovering them. I would even print a warning when using such
> fallback legacy MSR handling option that you need to report the issue
> to xen-devel because the option might be removed in future releases.
> 
> Does the above seem like a sensible plan?

I think so, yes. I wonder what Andrew thinks, though.

Jan