[PATCH v2 for-4.14] x86/msr: Disallow access to Processor Trace MSRs

Andrew Cooper posted 1 patch 3 years, 10 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/xen tags/patchew/20200626112937.919-1-andrew.cooper3@citrix.com
xen/arch/x86/msr.c              | 2 ++
xen/include/asm-x86/msr-index.h | 8 ++++++++
2 files changed, 10 insertions(+)
[PATCH v2 for-4.14] x86/msr: Disallow access to Processor Trace MSRs
Posted by Andrew Cooper 3 years, 10 months ago
We do not expose the feature to guests, so should disallow access to the
respective MSRs.  For simplicity, drop the entire block of MSRs, not just the
subset which have been specified thus far.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Wei Liu <wl@xen.org>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Paul Durrant <paul@xen.org>
CC: Michał Leszczyński <michal.leszczynski@cert.pl>

Paul: For 4.14.  This needs backporting to older trees as well.

v2:
 * Drop the whole 0x560 => 0x58f block.
---
 xen/arch/x86/msr.c              | 2 ++
 xen/include/asm-x86/msr-index.h | 8 ++++++++
 2 files changed, 10 insertions(+)

diff --git a/xen/arch/x86/msr.c b/xen/arch/x86/msr.c
index 0bfb5839b2..22f921cc71 100644
--- a/xen/arch/x86/msr.c
+++ b/xen/arch/x86/msr.c
@@ -168,6 +168,7 @@ int guest_rdmsr(struct vcpu *v, uint32_t msr, uint64_t *val)
     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:
@@ -329,6 +330,7 @@ int guest_wrmsr(struct vcpu *v, uint32_t msr, uint64_t val)
     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:
diff --git a/xen/include/asm-x86/msr-index.h b/xen/include/asm-x86/msr-index.h
index b328a47ed8..0fe98af923 100644
--- a/xen/include/asm-x86/msr-index.h
+++ b/xen/include/asm-x86/msr-index.h
@@ -69,6 +69,14 @@
 #define MSR_MCU_OPT_CTRL                    0x00000123
 #define  MCU_OPT_CTRL_RNGDS_MITG_DIS        (_AC(1, ULL) <<  0)
 
+#define MSR_RTIT_OUTPUT_BASE                0x00000560
+#define MSR_RTIT_OUTPUT_MASK                0x00000561
+#define MSR_RTIT_CTL                        0x00000570
+#define MSR_RTIT_STATUS                     0x00000571
+#define MSR_RTIT_CR3_MATCH                  0x00000572
+#define MSR_RTIT_ADDR_A(n)                 (0x00000580 + (n) * 2)
+#define MSR_RTIT_ADDR_B(n)                 (0x00000581 + (n) * 2)
+
 #define MSR_U_CET                           0x000006a0
 #define MSR_S_CET                           0x000006a2
 #define  CET_SHSTK_EN                       (_AC(1, ULL) <<  0)
-- 
2.11.0


Re: [PATCH v2 for-4.14] x86/msr: Disallow access to Processor Trace MSRs
Posted by Wei Liu 3 years, 10 months ago
On Fri, Jun 26, 2020 at 12:29:37PM +0100, Andrew Cooper wrote:
> We do not expose the feature to guests, so should disallow access to the
> respective MSRs.  For simplicity, drop the entire block of MSRs, not just the
> subset which have been specified thus far.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Wei Liu <wl@xen.org>
> CC: Roger Pau Monné <roger.pau@citrix.com>
> CC: Paul Durrant <paul@xen.org>
> CC: Michał Leszczyński <michal.leszczynski@cert.pl>
> 
> Paul: For 4.14.  This needs backporting to older trees as well.
> 
> v2:
>  * Drop the whole 0x560 => 0x58f block.

Reviewed-by: Wei Liu <wl@xen.org>

I have not checked the MSR values against the manual, but the
modifications to guest_{rd,wr}msr look correct to me.

Wei.

> ---
>  xen/arch/x86/msr.c              | 2 ++
>  xen/include/asm-x86/msr-index.h | 8 ++++++++
>  2 files changed, 10 insertions(+)
> 
> diff --git a/xen/arch/x86/msr.c b/xen/arch/x86/msr.c
> index 0bfb5839b2..22f921cc71 100644
> --- a/xen/arch/x86/msr.c
> +++ b/xen/arch/x86/msr.c
> @@ -168,6 +168,7 @@ int guest_rdmsr(struct vcpu *v, uint32_t msr, uint64_t *val)
>      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:
> @@ -329,6 +330,7 @@ int guest_wrmsr(struct vcpu *v, uint32_t msr, uint64_t val)
>      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:
> diff --git a/xen/include/asm-x86/msr-index.h b/xen/include/asm-x86/msr-index.h
> index b328a47ed8..0fe98af923 100644
> --- a/xen/include/asm-x86/msr-index.h
> +++ b/xen/include/asm-x86/msr-index.h
> @@ -69,6 +69,14 @@
>  #define MSR_MCU_OPT_CTRL                    0x00000123
>  #define  MCU_OPT_CTRL_RNGDS_MITG_DIS        (_AC(1, ULL) <<  0)
>  
> +#define MSR_RTIT_OUTPUT_BASE                0x00000560
> +#define MSR_RTIT_OUTPUT_MASK                0x00000561
> +#define MSR_RTIT_CTL                        0x00000570
> +#define MSR_RTIT_STATUS                     0x00000571
> +#define MSR_RTIT_CR3_MATCH                  0x00000572
> +#define MSR_RTIT_ADDR_A(n)                 (0x00000580 + (n) * 2)
> +#define MSR_RTIT_ADDR_B(n)                 (0x00000581 + (n) * 2)
> +
>  #define MSR_U_CET                           0x000006a0
>  #define MSR_S_CET                           0x000006a2
>  #define  CET_SHSTK_EN                       (_AC(1, ULL) <<  0)
> -- 
> 2.11.0
> 

Re: [PATCH v2 for-4.14] x86/msr: Disallow access to Processor Trace MSRs
Posted by Jan Beulich 3 years, 10 months ago
On 26.06.2020 13:29, Andrew Cooper wrote:
> We do not expose the feature to guests, so should disallow access to the
> respective MSRs.  For simplicity, drop the entire block of MSRs, not just the
> subset which have been specified thus far.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

RE: [PATCH v2 for-4.14] x86/msr: Disallow access to Processor Trace MSRs
Posted by Paul Durrant 3 years, 10 months ago
> -----Original Message-----
> From: Andrew Cooper <andrew.cooper3@citrix.com>
> Sent: 26 June 2020 12:30
> To: Xen-devel <xen-devel@lists.xenproject.org>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>; Jan Beulich <JBeulich@suse.com>; Wei Liu <wl@xen.org>;
> Roger Pau Monné <roger.pau@citrix.com>; Paul Durrant <paul@xen.org>; Michał Leszczyński
> <michal.leszczynski@cert.pl>
> Subject: [PATCH v2 for-4.14] x86/msr: Disallow access to Processor Trace MSRs
> 
> We do not expose the feature to guests, so should disallow access to the
> respective MSRs.  For simplicity, drop the entire block of MSRs, not just the
> subset which have been specified thus far.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Wei Liu <wl@xen.org>
> CC: Roger Pau Monné <roger.pau@citrix.com>
> CC: Paul Durrant <paul@xen.org>
> CC: Michał Leszczyński <michal.leszczynski@cert.pl>
> 
> Paul: For 4.14.  This needs backporting to older trees as well.
> 
> v2:
>  * Drop the whole 0x560 => 0x58f block.

Looks ok to me.

Release-acked-by: Paul Durrant <paul@xen.org>

> ---
>  xen/arch/x86/msr.c              | 2 ++
>  xen/include/asm-x86/msr-index.h | 8 ++++++++
>  2 files changed, 10 insertions(+)
> 
> diff --git a/xen/arch/x86/msr.c b/xen/arch/x86/msr.c
> index 0bfb5839b2..22f921cc71 100644
> --- a/xen/arch/x86/msr.c
> +++ b/xen/arch/x86/msr.c
> @@ -168,6 +168,7 @@ int guest_rdmsr(struct vcpu *v, uint32_t msr, uint64_t *val)
>      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:
> @@ -329,6 +330,7 @@ int guest_wrmsr(struct vcpu *v, uint32_t msr, uint64_t val)
>      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:
> diff --git a/xen/include/asm-x86/msr-index.h b/xen/include/asm-x86/msr-index.h
> index b328a47ed8..0fe98af923 100644
> --- a/xen/include/asm-x86/msr-index.h
> +++ b/xen/include/asm-x86/msr-index.h
> @@ -69,6 +69,14 @@
>  #define MSR_MCU_OPT_CTRL                    0x00000123
>  #define  MCU_OPT_CTRL_RNGDS_MITG_DIS        (_AC(1, ULL) <<  0)
> 
> +#define MSR_RTIT_OUTPUT_BASE                0x00000560
> +#define MSR_RTIT_OUTPUT_MASK                0x00000561
> +#define MSR_RTIT_CTL                        0x00000570
> +#define MSR_RTIT_STATUS                     0x00000571
> +#define MSR_RTIT_CR3_MATCH                  0x00000572
> +#define MSR_RTIT_ADDR_A(n)                 (0x00000580 + (n) * 2)
> +#define MSR_RTIT_ADDR_B(n)                 (0x00000581 + (n) * 2)
> +
>  #define MSR_U_CET                           0x000006a0
>  #define MSR_S_CET                           0x000006a2
>  #define  CET_SHSTK_EN                       (_AC(1, ULL) <<  0)
> --
> 2.11.0