Re: [PATCH 2/3] x86/msr: Forward port XSA-351 changes from 4.14

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Tue, Mar 16, 2021 at 04:18:43PM +0000, Andrew Cooper wrote:
> staging was not impacted by XSA-351 at the time of release, due to c/s
> 322ec7c89f and 84e848fd7a which disallows read access by default.
> 
> Forward port the XSA-351 changes to make the code structure consistent between
> 4.14 and 4.15.
> 
> This removes logspew for guests probing for the RAPL interface.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> ---
> 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>
> 
> Technically this breaks Solaris/turbostat insofar as you can no longer use
> msr_relaxed to "fix" the guest.  The subsequent patch will unbreak it
> differently.
> 
> For 4.15.  Restoring behaviour closer to 4.14, and prereq for a bugfix needing
> backporting.
> ---
>  xen/arch/x86/msr.c              | 19 +++++++++++++++++++
>  xen/include/asm-x86/msr-index.h | 39 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 58 insertions(+)
> 
> diff --git a/xen/arch/x86/msr.c b/xen/arch/x86/msr.c
> index c3a988bd11..5927b6811b 100644
> --- a/xen/arch/x86/msr.c
> +++ b/xen/arch/x86/msr.c
> @@ -188,6 +188,13 @@ 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:
> +    case MSR_PP1_POWER_LIMIT  ... MSR_PP1_POLICY:
> +    case MSR_PLATFORM_ENERGY_COUNTER:
> +    case MSR_PLATFORM_POWER_LIMIT:
>      case MSR_U_CET:
>      case MSR_S_CET:
>      case MSR_PL0_SSP ... MSR_INTERRUPT_SSP_TABLE:
> @@ -195,6 +202,8 @@ int guest_rdmsr(struct vcpu *v, uint32_t msr, uint64_t 
> *val)
>      case MSR_AMD64_LWP_CBADDR:
>      case MSR_PPIN_CTL:
>      case MSR_PPIN:
> +    case MSR_F15H_CU_POWER ... MSR_F15H_CU_MAX_POWER:
> +    case MSR_AMD_RAPL_POWER_UNIT ... MSR_AMD_PKG_ENERGY_STATUS:
>      case MSR_AMD_PPIN_CTL:
>      case MSR_AMD_PPIN:
>          goto gp_fault;
> @@ -412,6 +421,7 @@ int guest_wrmsr(struct vcpu *v, uint32_t msr, uint64_t 
> val)
>      case MSR_INTEL_CORE_THREAD_COUNT:
>      case MSR_INTEL_PLATFORM_INFO:
>      case MSR_ARCH_CAPABILITIES:
> +    case MSR_IA32_PERF_STATUS:

Should the MSR_IA32_PERF_STATUS addition maybe be part of the previous
commit, as it's not related to the XSA-351 content?

The rest LGTM:

Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

I wonder whether we could squash this with 3/3 for staging commit, and
then only backport 3/3 for older branches. But it's likely too much
work just to prevent breaking msr_relaxed for Solaris for a single
commit time span.

Thanks, Roger.