Re: [PATCH 3/3] x86/msr: Fix Solaris and turbostat following XSA-351

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 16/03/2021 16:56, Roger Pau Monné wrote:
> On Tue, Mar 16, 2021 at 04:18:44PM +0000, Andrew Cooper wrote:
>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Thanks!
>
>> ---
>> CC: Jan Beulich <JBeulich@xxxxxxxx>
>> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
>> CC: Wei Liu <wl@xxxxxxx>
>> CC: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
>> CC: Ian Jackson <iwj@xxxxxxxxxxxxxx>
>>
>> For 4.15 This wants backporting to all security trees, as it is a fix to a
>> regression introduced in XSA-351.
>>
>> Also it means that users don't need msr_relaxed=1 to unbreak Solaris guests,
>> which is a strict useability improvement.
>> ---
>>  xen/arch/x86/msr.c | 13 ++++++++++++-
>>  1 file changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/xen/arch/x86/msr.c b/xen/arch/x86/msr.c
>> index 5927b6811b..a83a1d7fba 100644
>> --- a/xen/arch/x86/msr.c
>> +++ b/xen/arch/x86/msr.c
>> @@ -188,7 +188,6 @@ int guest_rdmsr(struct vcpu *v, uint32_t msr, uint64_t 
>> *val)
>>      case MSR_TSX_CTRL:
>>      case MSR_MCU_OPT_CTRL:
>>      case MSR_RTIT_OUTPUT_BASE ... MSR_RTIT_ADDR_B(7):
>> -    case MSR_RAPL_POWER_UNIT:
>>      case MSR_PKG_POWER_LIMIT  ... MSR_PKG_POWER_INFO:
>>      case MSR_DRAM_POWER_LIMIT ... MSR_DRAM_POWER_INFO:
>>      case MSR_PP0_POWER_LIMIT  ... MSR_PP0_POLICY:
>> @@ -284,6 +283,18 @@ int guest_rdmsr(struct vcpu *v, uint32_t msr, uint64_t 
>> *val)
>>              goto gp_fault;
>>          break;
>>  
>> +    case MSR_RAPL_POWER_UNIT:
>> +        /*
>> +         * This MSR is non-architectural.  However, some versions of Solaris
>> +         * blindly reads it without a #GP guard, and some versions of
>> +         * turbostat crash after expecting a read of /proc/cpu/0/msr not to
>> +         * fail.  Read as zero on Intel hardware.
>> +         */
>> +        if ( !(cp->x86_vendor & X86_VENDOR_INTEL) )
>> +            goto gp_fault;
> AFAICT from Linux usage this is Intel specific (not present in any of
> the clones).

Indeed.

>
>> +        *val = 0;
>> +        break;
> Do we also need to care about MSR_AMD_RAPL_POWER_UNIT (0xc0010299) for
> Solaris?

AMD has a CPUID bit for this interface, 0x80000007.EDX[14].

~Andrew