Re: [PATCH 3/4] x86: Allow non-faulting accesses to non-emulated MSRs if policy permits this

[Jan Beulich <jbeulich@xxxxxxxx>]

On 08.01.2021 21:39, boris.ostrovsky@xxxxxxxxxx wrote:
> On 1/8/21 10:18 AM, Jan Beulich wrote:
>> On 07.01.2021 21:34, Boris Ostrovsky wrote:
>>> Starting with commit 84e848fd7a16 ("x86/hvm: disallow access to unknown 
>>> MSRs")
>>> accesses to unhandled MSRs result in #GP sent to the guest. This caused a
>>> regression for Solaris who tries to acccess MSR_RAPL_POWER_UNIT and (unlike,
>> Nit: One c too many.
>>
>>> for example, Linux) does not catch exceptions when accessing MSRs that
>>> potentially may not be present.
>> Just to re-raise the question raised by Andrew already earlier
>> on: Has Solaris been fixed in the meantime, or is this at least
>> firmly planned to happen?
> 
> I was told they will open a bug.

"Will", not "did"?

>> @@ -3319,10 +3319,8 @@ static int vmx_msr_write_intercept(unsigned int msr, 
>> uint64_t msr_content)
>>               is_last_branch_msr(msr) )
>>              break;
>>  
>> -        gdprintk(XENLOG_WARNING,
>> -                 "WRMSR 0x%08x val 0x%016"PRIx64" unimplemented\n",
>> -                 msr, msr_content);
>> -        goto gp_fault;
>> +        if ( guest_unhandled_msr(v, msr, &msr_content, true) )
>> +            goto gp_fault;
>>      }
>>  
>>      return X86EMUL_OKAY;
>> These functions also get used from the insn emulator, when it
>> needs to fetch an MSR value (not necessarily in the context of
>> emulating RDMSR or WRMSR). I wonder whether applying this
>> behavior in that case is actually correct. It would only be if
>> we would settle on it being a requirement that any such MSRs
>> have to have emulation present in one of the handlers.
> 
> 
> Hmm.. Yes, I did not consider this. I am not convinced this will
> always result in correct behavior for the emulator so I will
> need to pass down a parameter. Unless there is a way to figure
> out whether we are running in the emulator (which I don't
> immediately see)

Passing a parameter for this is sort of ugly, but I presume
unavoidable. The more that what you need to know is not "running
in emulator", but "guest RDMSR/WRMSR" - this can also happen
through the emulator.

Jan