[v2,8/8] x86/msr: Drop compatibility #GP handling in guest_{rd, wr}msr()

Commit Message

Roger Pau Monne Aug. 20, 2020, 3:08 p.m. UTC

From: Andrew Cooper <[e-mail]>

Now that the main PV/HVM MSR handlers raise #GP for all unknown MSRs, there is
no need to special case these MSRs any more.

Signed-off-by: Andrew Cooper <[e-mail]>
Reviewed-by: Roger Pau Monné <[e-mail]>
---
Changes since v1:
 - New in this version.
---
 xen/arch/x86/msr.c | 46 ----------------------------------------------
 1 file changed, 46 deletions(-)

Comments

Jan Beulich Aug. 28, 2020, 8:55 a.m. UTC | #1

On 20.08.2020 17:08, Roger Pau Monne wrote:
> Now that the main PV/HVM MSR handlers raise #GP for all unknown MSRs, there is
> no need to special case these MSRs any more.

I agree, yet I'd like to point out (based on past backporting of
security fixes) that this change may end up being a good source
of backporting mistakes, as 4.14 may need chunks added here which
on master / staging have no place anymore.

Jan

Roger Pau Monne Aug. 31, 2020, 3:22 p.m. UTC | #2

On Fri, Aug 28, 2020 at 10:55:29AM +0200, Jan Beulich wrote:
> On 20.08.2020 17:08, Roger Pau Monne wrote:
> > Now that the main PV/HVM MSR handlers raise #GP for all unknown MSRs, there is
> > no need to special case these MSRs any more.
> 
> I agree, yet I'd like to point out (based on past backporting of
> security fixes) that this change may end up being a good source
> of backporting mistakes, as 4.14 may need chunks added here which
> on master / staging have no place anymore.

Even if we leave this chunk out, patches for > 4.14 won't need to add
anything to make a MSR return a #GP, so it's likely that the chunk
will have to be added for backports anyway.

I really have no idea of how to help with backporting with such change
in place.

Thanks, Roger.

Patch

diff --git a/xen/arch/x86/msr.c b/xen/arch/x86/msr.c
index bb0dd5ff0a..560719c2aa 100644
--- a/xen/arch/x86/msr.c
+++ b/xen/arch/x86/msr.c
@@ -159,29 +159,6 @@  int guest_rdmsr(struct vcpu *v, uint32_t msr, uint64_t *val)
 
     switch ( msr )
     {
-    case MSR_AMD_PATCHLOADER:
-    case MSR_IA32_UCODE_WRITE:
-    case MSR_PRED_CMD:
-    case MSR_FLUSH_CMD:
-        /* Write-only */
-    case MSR_TEST_CTRL:
-    case MSR_CORE_CAPABILITIES:
-    case MSR_TSX_FORCE_ABORT:
-    case MSR_TSX_CTRL:
-    case MSR_MCU_OPT_CTRL:
-    case MSR_RTIT_OUTPUT_BASE ... MSR_RTIT_ADDR_B(7):
-    case MSR_U_CET:
-    case MSR_S_CET:
-    case MSR_PL0_SSP ... MSR_INTERRUPT_SSP_TABLE:
-    case MSR_AMD64_LWP_CFG:
-    case MSR_AMD64_LWP_CBADDR:
-    case MSR_PPIN_CTL:
-    case MSR_PPIN:
-    case MSR_AMD_PPIN_CTL:
-    case MSR_AMD_PPIN:
-        /* Not offered to guests. */
-        goto gp_fault;
-
     case MSR_IA32_FEATURE_CONTROL:
         if ( !(cp->x86_vendor & X86_VENDOR_INTEL) )
             goto gp_fault;
@@ -349,29 +326,6 @@  int guest_wrmsr(struct vcpu *v, uint32_t msr, uint64_t val)
     {
         uint64_t rsvd;
 
-    case MSR_IA32_PLATFORM_ID:
-    case MSR_CORE_CAPABILITIES:
-    case MSR_INTEL_CORE_THREAD_COUNT:
-    case MSR_INTEL_PLATFORM_INFO:
-    case MSR_ARCH_CAPABILITIES:
-        /* Read-only */
-    case MSR_TEST_CTRL:
-    case MSR_TSX_FORCE_ABORT:
-    case MSR_TSX_CTRL:
-    case MSR_MCU_OPT_CTRL:
-    case MSR_RTIT_OUTPUT_BASE ... MSR_RTIT_ADDR_B(7):
-    case MSR_U_CET:
-    case MSR_S_CET:
-    case MSR_PL0_SSP ... MSR_INTERRUPT_SSP_TABLE:
-    case MSR_AMD64_LWP_CFG:
-    case MSR_AMD64_LWP_CBADDR:
-    case MSR_PPIN_CTL:
-    case MSR_PPIN:
-    case MSR_AMD_PPIN_CTL:
-    case MSR_AMD_PPIN:
-        /* Not offered to guests. */
-        goto gp_fault;
-
     case MSR_AMD_PATCHLEVEL:
         BUILD_BUG_ON(MSR_IA32_UCODE_REV != MSR_AMD_PATCHLEVEL);
         /*