| Message ID | 20200901105445.22277-2-roger.pau@citrix.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | x86: switch default MSR behavior | expand |
On 01.09.2020 12:54, Roger Pau Monne wrote: > Such handling consist in checking that no bits have been changed from > the read value, if that's the case silently drop the write, otherwise > inject a fault. > > At least Windows guests will expect to write to the MISC_ENABLE MSR > with the same value that's been read from it. > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> > --- > xen/arch/x86/hvm/vmx/vmx.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) Btw - early next week I think I'll time out waiting for a VMX side ack for this change. Jan
> From: Roger Pau Monne <roger.pau@citrix.com> > Sent: Tuesday, September 1, 2020 6:55 PM > > Such handling consist in checking that no bits have been changed from > the read value, if that's the case silently drop the write, otherwise > inject a fault. > > At least Windows guests will expect to write to the MISC_ENABLE MSR > with the same value that's been read from it. for better readability could you also add this line to the code comment below? with that: Reviewed-by: Kevin Tian <kevin.tian@intel.com> > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> > --- > xen/arch/x86/hvm/vmx/vmx.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c > index a0d58ffbe2..4717e50d4a 100644 > --- a/xen/arch/x86/hvm/vmx/vmx.c > +++ b/xen/arch/x86/hvm/vmx/vmx.c > @@ -3163,7 +3163,7 @@ static int vmx_msr_write_intercept(unsigned int > msr, uint64_t msr_content) > > switch ( msr ) > { > - uint64_t rsvd; > + uint64_t rsvd, tmp; > > case MSR_IA32_SYSENTER_CS: > __vmwrite(GUEST_SYSENTER_CS, msr_content); > @@ -3301,6 +3301,13 @@ static int vmx_msr_write_intercept(unsigned int > msr, uint64_t msr_content) > /* None of these MSRs are writeable. */ > goto gp_fault; > > + case MSR_IA32_MISC_ENABLE: > + /* Silently drop writes that don't change the reported value. */ > + if ( vmx_msr_read_intercept(msr, &tmp) != X86EMUL_OKAY || > + tmp != msr_content ) > + goto gp_fault; > + break; > + > case MSR_P6_PERFCTR(0)...MSR_P6_PERFCTR(7): > case MSR_P6_EVNTSEL(0)...MSR_P6_EVNTSEL(7): > case MSR_CORE_PERF_FIXED_CTR0...MSR_CORE_PERF_FIXED_CTR2: > -- > 2.28.0
On 07.09.2020 05:25, Tian, Kevin wrote: >> From: Roger Pau Monne <roger.pau@citrix.com> >> Sent: Tuesday, September 1, 2020 6:55 PM >> >> Such handling consist in checking that no bits have been changed from >> the read value, if that's the case silently drop the write, otherwise >> inject a fault. >> >> At least Windows guests will expect to write to the MISC_ENABLE MSR >> with the same value that's been read from it. > > for better readability could you also add this line to the code comment > below? > > with that: > > Reviewed-by: Kevin Tian <kevin.tian@intel.com> I'll fold this in while committing. Jan
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c index a0d58ffbe2..4717e50d4a 100644 --- a/xen/arch/x86/hvm/vmx/vmx.c +++ b/xen/arch/x86/hvm/vmx/vmx.c @@ -3163,7 +3163,7 @@ static int vmx_msr_write_intercept(unsigned int msr, uint64_t msr_content) switch ( msr ) { - uint64_t rsvd; + uint64_t rsvd, tmp; case MSR_IA32_SYSENTER_CS: __vmwrite(GUEST_SYSENTER_CS, msr_content); @@ -3301,6 +3301,13 @@ static int vmx_msr_write_intercept(unsigned int msr, uint64_t msr_content) /* None of these MSRs are writeable. */ goto gp_fault; + case MSR_IA32_MISC_ENABLE: + /* Silently drop writes that don't change the reported value. */ + if ( vmx_msr_read_intercept(msr, &tmp) != X86EMUL_OKAY || + tmp != msr_content ) + goto gp_fault; + break; + case MSR_P6_PERFCTR(0)...MSR_P6_PERFCTR(7): case MSR_P6_EVNTSEL(0)...MSR_P6_EVNTSEL(7): case MSR_CORE_PERF_FIXED_CTR0...MSR_CORE_PERF_FIXED_CTR2: