diff mbox series

[v3,2/8] x86/svm: silently drop writes to SYSCFG and related MSRs

Message ID 20200901105445.22277-3-roger.pau@citrix.com
State Superseded
Headers show
Series x86: switch default MSR behavior | expand

Commit Message

Roger Pau Monne Sept. 1, 2020, 10:54 a.m. UTC
The SYSCFG, TOP_MEM1 and TOP_MEM2 MSRs are currently exposed to guests
and writes are silently discarded. Make this explicit in the SVM code
now, and just return default constant values when attempting to read
any of the MSRs, while continuing to silently drop writes.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Changes since v2:
 - Return 0 from SYSCFG.
 - Merge switch cases.

Changes sincxe v1:
 - Return MtrrFixDramEn in MSR_K8_SYSCFG.
---
 xen/arch/x86/hvm/svm/svm.c | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Jan Beulich Sept. 4, 2020, 8:36 a.m. UTC | #1
On 01.09.2020 12:54, Roger Pau Monne wrote:
> The SYSCFG, TOP_MEM1 and TOP_MEM2 MSRs are currently exposed to guests
> and writes are silently discarded. Make this explicit in the SVM code
> now, and just return default constant values when attempting to read
> any of the MSRs, while continuing to silently drop writes.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

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

> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -1917,6 +1917,9 @@ static int svm_msr_read_intercept(unsigned int msr, uint64_t *msr_content)
>              goto gpf;
>          break;
>  
> +    case MSR_K8_SYSCFG:
> +    case MSR_K8_TOP_MEM1:
> +    case MSR_K8_TOP_MEM2:
>      case MSR_K8_VM_CR:
>          *msr_content = 0;
>          break;

Andrew, since you did suggest otherwise before, may I ask for an
explicit statement of yours here, be it in ack/nak form or something
less formal?

Jan
Andrew Cooper Sept. 4, 2020, 9:47 a.m. UTC | #2
On 04/09/2020 09:36, Jan Beulich wrote:
> On 01.09.2020 12:54, Roger Pau Monne wrote:
>> The SYSCFG, TOP_MEM1 and TOP_MEM2 MSRs are currently exposed to guests
>> and writes are silently discarded. Make this explicit in the SVM code
>> now, and just return default constant values when attempting to read
>> any of the MSRs, while continuing to silently drop writes.
>>
>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
>
>> --- a/xen/arch/x86/hvm/svm/svm.c
>> +++ b/xen/arch/x86/hvm/svm/svm.c
>> @@ -1917,6 +1917,9 @@ static int svm_msr_read_intercept(unsigned int msr, uint64_t *msr_content)
>>              goto gpf;
>>          break;
>>  
>> +    case MSR_K8_SYSCFG:
>> +    case MSR_K8_TOP_MEM1:
>> +    case MSR_K8_TOP_MEM2:
>>      case MSR_K8_VM_CR:
>>          *msr_content = 0;
>>          break;
> Andrew, since you did suggest otherwise before, may I ask for an
> explicit statement of yours here, be it in ack/nak form or something
> less formal?

I'm not entirely convinced it is a safe thing to do, but lets see what
happens.

There is likely to be a bug tail from the flipping the default behaviour
at the end of the series, and this is certainly a simpler set of logic
than the alternative.

~Andrew
diff mbox series

Patch

diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index ca3bbfcbb3..af584ff5d1 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -1917,6 +1917,9 @@  static int svm_msr_read_intercept(unsigned int msr, uint64_t *msr_content)
             goto gpf;
         break;
 
+    case MSR_K8_SYSCFG:
+    case MSR_K8_TOP_MEM1:
+    case MSR_K8_TOP_MEM2:
     case MSR_K8_VM_CR:
         *msr_content = 0;
         break;
@@ -2094,6 +2097,9 @@  static int svm_msr_write_intercept(unsigned int msr, uint64_t msr_content)
             goto gpf;
         break;
 
+    case MSR_K8_TOP_MEM1:
+    case MSR_K8_TOP_MEM2:
+    case MSR_K8_SYSCFG:
     case MSR_K8_VM_CR:
         /* ignore write. handle all bits as read-only. */
         break;