[v1] x86/msr: don't inject #GP when trying to read FEATURE_CONTROL

[PATCH] x86/msr: don't inject #GP when trying to read FEATURE_CONTROL

Posted by Roger Pau Monne 3 years, 4 months ago

Windows 10 will triple fault if #GP is injected when attempting to
read the FEATURE_CONTROL MSR on Intel or compatible hardware. Fix this
by injecting a #GP only when the vendor doesn't support the MSR, even
if there are no features to expose.

Fixes: 39ab598c50a2 ('x86/pv: allow reading FEATURE_CONTROL MSR')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
 xen/arch/x86/msr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/x86/msr.c b/xen/arch/x86/msr.c
index be8e363862..38b0a046e1 100644
--- a/xen/arch/x86/msr.c
+++ b/xen/arch/x86/msr.c
@@ -176,7 +176,7 @@ int guest_rdmsr(struct vcpu *v, uint32_t msr, uint64_t *val)
     switch ( msr )
     {
     case MSR_IA32_FEATURE_CONTROL:
-        if ( !cp->basic.vmx && !vmce_has_lmce(v) )
+        if ( !(cp->x86_vendor & (X86_VENDOR_INTEL | X86_VENDOR_CENTAUR)) )
             goto gp_fault;
 
         *val = IA32_FEATURE_CONTROL_LOCK;
-- 
2.29.2

Re: [PATCH] x86/msr: don't inject #GP when trying to read FEATURE_CONTROL

Posted by Jan Beulich 3 years, 4 months ago

On 27.11.2020 11:46, Roger Pau Monne wrote:
> Windows 10 will triple fault if #GP is injected when attempting to
> read the FEATURE_CONTROL MSR on Intel or compatible hardware. Fix this
> by injecting a #GP only when the vendor doesn't support the MSR, even
> if there are no features to expose.
> 
> Fixes: 39ab598c50a2 ('x86/pv: allow reading FEATURE_CONTROL MSR')
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

In principle
Acked-by: Jan Beulich <jbeulich@suse.com>

However, iirc it was Andrew who had suggested the conditional you
now replace, so I'd like to wait for him to voice a view.

> --- a/xen/arch/x86/msr.c
> +++ b/xen/arch/x86/msr.c
> @@ -176,7 +176,7 @@ int guest_rdmsr(struct vcpu *v, uint32_t msr, uint64_t *val)
>      switch ( msr )
>      {
>      case MSR_IA32_FEATURE_CONTROL:
> -        if ( !cp->basic.vmx && !vmce_has_lmce(v) )
> +        if ( !(cp->x86_vendor & (X86_VENDOR_INTEL | X86_VENDOR_CENTAUR)) )

What about Shanghai? init_shanghai() calling init_intel_cacheinfo()
suggests to me it's at least as Intel-like as Centaur/VIA.

Jan

Re: [PATCH] x86/msr: don't inject #GP when trying to read FEATURE_CONTROL

Posted by Roger Pau Monné 3 years, 3 months ago

On Fri, Nov 27, 2020 at 11:56:25AM +0100, Jan Beulich wrote:
> On 27.11.2020 11:46, Roger Pau Monne wrote:
> > Windows 10 will triple fault if #GP is injected when attempting to
> > read the FEATURE_CONTROL MSR on Intel or compatible hardware. Fix this
> > by injecting a #GP only when the vendor doesn't support the MSR, even
> > if there are no features to expose.
> > 
> > Fixes: 39ab598c50a2 ('x86/pv: allow reading FEATURE_CONTROL MSR')
> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> 
> In principle
> Acked-by: Jan Beulich <jbeulich@suse.com>
> 
> However, iirc it was Andrew who had suggested the conditional you
> now replace, so I'd like to wait for him to voice a view.
> 
> > --- a/xen/arch/x86/msr.c
> > +++ b/xen/arch/x86/msr.c
> > @@ -176,7 +176,7 @@ int guest_rdmsr(struct vcpu *v, uint32_t msr, uint64_t *val)
> >      switch ( msr )
> >      {
> >      case MSR_IA32_FEATURE_CONTROL:
> > -        if ( !cp->basic.vmx && !vmce_has_lmce(v) )
> > +        if ( !(cp->x86_vendor & (X86_VENDOR_INTEL | X86_VENDOR_CENTAUR)) )
> 
> What about Shanghai? init_shanghai() calling init_intel_cacheinfo()
> suggests to me it's at least as Intel-like as Centaur/VIA.

Right, and it also has VMX AFAICT. I'm not sure whether we could also
gate on the presence of VMX and LMCE on the physical CPU. I will send
and updated version with Shanghai added and will keep your Ack.

Thanks, Roger.