From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH v2 1/2] x86/HVM: fix miscellaneous aspects of x2APIC emulation Date: Thu, 11 Sep 2014 16:39:13 +0100 Message-ID: <5411C221.6060109@citrix.com> References: <541070390200007800033519@mail.emea.novell.com> <541071AA0200007800033530@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============7497729734945942434==" Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1XS6Sw-0002Vr-P4 for xen-devel@lists.xenproject.org; Thu, 11 Sep 2014 15:39:22 +0000 In-Reply-To: <541071AA0200007800033530@mail.emea.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich , xen-devel Cc: Keir Fraser List-Id: xen-devel@lists.xenproject.org --===============7497729734945942434== Content-Type: multipart/alternative; boundary="------------090009090208090004080704" --------------090009090208090004080704 Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: quoted-printable On 10/09/14 14:43, Jan Beulich wrote: > - generate #GP on invalid APIC base MSR transitions > - fail reads from the self-IPI register (which is write-only) > - handle self-IPI writes and the ICR2 half of ICR writes largely in > hvm_x2apic_msr_write() > > Signed-off-by: Jan Beulich > --- > v2: Split from main patch. > > --- a/xen/arch/x86/hvm/hvm.c > +++ b/xen/arch/x86/hvm/hvm.c > @@ -4499,7 +4499,8 @@ int hvm_msr_write_intercept(unsigned int > break; > =20 > case MSR_IA32_APICBASE: > - vlapic_msr_set(vcpu_vlapic(v), msr_content); > + if ( !vlapic_msr_set(vcpu_vlapic(v), msr_content) ) > + goto gp_fault; > break; > =20 > case MSR_IA32_TSC_DEADLINE: > --- a/xen/arch/x86/hvm/vlapic.c > +++ b/xen/arch/x86/hvm/vlapic.c > @@ -602,6 +602,7 @@ int hvm_x2apic_msr_read(struct vcpu *v,=20 > break; > =20 > case APIC_ICR2: > + case APIC_SELF_IPI: APIC_EOI is also write-only, generates #GP(0) on on rdmsr, and isn't caught by vlapic_read_aligned(). Otherwise, Reviewed-by: Andrew Cooper > return 1; > } > =20 > @@ -692,9 +693,7 @@ static int vlapic_reg_write(struct vcpu=20 > break; > =20 > case APIC_SELF_IPI: > - rc =3D vlapic_x2apic_mode(vlapic) > - ? vlapic_reg_write(v, APIC_ICR, 0x40000 | (val & 0xff)) > - : X86EMUL_UNHANDLEABLE; > + rc =3D X86EMUL_UNHANDLEABLE; > break; > =20 > case APIC_ICR: > @@ -704,9 +703,7 @@ static int vlapic_reg_write(struct vcpu=20 > break; > =20 > case APIC_ICR2: > - if ( !vlapic_x2apic_mode(vlapic) ) > - val &=3D 0xff000000; > - vlapic_set_reg(vlapic, APIC_ICR2, val); > + vlapic_set_reg(vlapic, APIC_ICR2, val & 0xff000000); > break; > =20 > case APIC_LVTT: /* LVT Timer Reg */ > @@ -865,16 +862,17 @@ int hvm_x2apic_msr_write(struct vcpu *v, > =20 > switch ( offset ) > { > - int rc; > - > case APIC_ICR: > - rc =3D vlapic_reg_write(v, APIC_ICR2, (uint32_t)(msr_content >= > 32)); > - if ( rc ) > - return rc; > + vlapic_set_reg(vlapic, APIC_ICR2, msr_content >> 32); > break; > =20 > case APIC_ICR2: > return X86EMUL_UNHANDLEABLE; > + > + case APIC_SELF_IPI: > + offset =3D APIC_ICR; > + msr_content =3D APIC_DEST_SELF | (uint8_t)msr_content; > + break; > } > =20 > return vlapic_reg_write(v, offset, (uint32_t)msr_content); > @@ -893,10 +891,12 @@ const struct hvm_mmio_handler vlapic_mmi > .write_handler =3D vlapic_write > }; > =20 > -void vlapic_msr_set(struct vlapic *vlapic, uint64_t value) > +bool_t vlapic_msr_set(struct vlapic *vlapic, uint64_t value) > { > if ( (vlapic->hw.apic_base_msr ^ value) & MSR_IA32_APICBASE_ENABLE= ) > { > + if ( unlikely(value & MSR_IA32_APICBASE_EXTD) ) > + return 0; > if ( value & MSR_IA32_APICBASE_ENABLE ) > { > vlapic_reset(vlapic); > @@ -905,10 +905,15 @@ void vlapic_msr_set(struct vlapic *vlapi > } > else > { > + if ( unlikely(vlapic->hw.apic_base_msr & MSR_IA32_APICBASE= _EXTD) ) > + return 0; > vlapic->hw.disabled |=3D VLAPIC_HW_DISABLED; > pt_may_unmask_irq(vlapic_domain(vlapic), NULL); > } > } > + else if ( !(value & MSR_IA32_APICBASE_ENABLE) && > + unlikely(value & MSR_IA32_APICBASE_EXTD) ) > + return 0; > =20 > vlapic->hw.apic_base_msr =3D value; > =20 > @@ -923,6 +928,8 @@ void vlapic_msr_set(struct vlapic *vlapi > =20 > HVM_DBG_LOG(DBG_LEVEL_VLAPIC, > "apic base msr is 0x%016"PRIx64, vlapic->hw.apic_base_= msr); > + > + return 1; > } > =20 > uint64_t vlapic_tdt_msr_get(struct vlapic *vlapic) > @@ -1206,6 +1213,10 @@ static int lapic_load_hidden(struct doma > if ( hvm_load_entry_zeroextend(LAPIC, h, &s->hw) !=3D 0 )=20 > return -EINVAL; > =20 > + if ( !(s->hw.apic_base_msr & MSR_IA32_APICBASE_ENABLE) && > + unlikely(s->hw.apic_base_msr & MSR_IA32_APICBASE_EXTD) ) > + return -EINVAL; > + > vmx_vlapic_msr_changed(v); > =20 > return 0; > --- a/xen/include/asm-x86/hvm/vlapic.h > +++ b/xen/include/asm-x86/hvm/vlapic.h > @@ -106,7 +106,7 @@ void vlapic_destroy(struct vcpu *v); > =20 > void vlapic_reset(struct vlapic *vlapic); > =20 > -void vlapic_msr_set(struct vlapic *vlapic, uint64_t value); > +bool_t vlapic_msr_set(struct vlapic *vlapic, uint64_t value); > void vlapic_tdt_msr_set(struct vlapic *vlapic, uint64_t value); > uint64_t vlapic_tdt_msr_get(struct vlapic *vlapic); > =20 > > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel --------------090009090208090004080704 Content-Type: text/html; charset="ISO-8859-1" Content-Transfer-Encoding: 8bit
On 10/09/14 14:43, Jan Beulich wrote:
- generate #GP on invalid APIC base MSR transitions
- fail reads from the self-IPI register (which is write-only)
- handle self-IPI writes and the ICR2 half of ICR writes largely in
  hvm_x2apic_msr_write()

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v2: Split from main patch.

--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -4499,7 +4499,8 @@ int hvm_msr_write_intercept(unsigned int
         break;
 
     case MSR_IA32_APICBASE:
-        vlapic_msr_set(vcpu_vlapic(v), msr_content);
+        if ( !vlapic_msr_set(vcpu_vlapic(v), msr_content) )
+            goto gp_fault;
         break;
 
     case MSR_IA32_TSC_DEADLINE:
--- a/xen/arch/x86/hvm/vlapic.c
+++ b/xen/arch/x86/hvm/vlapic.c
@@ -602,6 +602,7 @@ int hvm_x2apic_msr_read(struct vcpu *v, 
         break;
 
     case APIC_ICR2:
+    case APIC_SELF_IPI:

APIC_EOI is also write-only, generates #GP(0) on on rdmsr, and isn't caught by vlapic_read_aligned().

Otherwise, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

         return 1;
     }
 
@@ -692,9 +693,7 @@ static int vlapic_reg_write(struct vcpu 
         break;
 
     case APIC_SELF_IPI:
-        rc = vlapic_x2apic_mode(vlapic)
-            ? vlapic_reg_write(v, APIC_ICR, 0x40000 | (val & 0xff))
-            : X86EMUL_UNHANDLEABLE;
+        rc = X86EMUL_UNHANDLEABLE;
         break;
 
     case APIC_ICR:
@@ -704,9 +703,7 @@ static int vlapic_reg_write(struct vcpu 
         break;
 
     case APIC_ICR2:
-        if ( !vlapic_x2apic_mode(vlapic) )
-            val &= 0xff000000;
-        vlapic_set_reg(vlapic, APIC_ICR2, val);
+        vlapic_set_reg(vlapic, APIC_ICR2, val & 0xff000000);
         break;
 
     case APIC_LVTT:         /* LVT Timer Reg */
@@ -865,16 +862,17 @@ int hvm_x2apic_msr_write(struct vcpu *v,
 
     switch ( offset )
     {
-        int rc;
-
     case APIC_ICR:
-        rc = vlapic_reg_write(v, APIC_ICR2, (uint32_t)(msr_content >> 32));
-        if ( rc )
-            return rc;
+        vlapic_set_reg(vlapic, APIC_ICR2, msr_content >> 32);
         break;
 
     case APIC_ICR2:
         return X86EMUL_UNHANDLEABLE;
+
+    case APIC_SELF_IPI:
+        offset = APIC_ICR;
+        msr_content = APIC_DEST_SELF | (uint8_t)msr_content;
+        break;
     }
 
     return vlapic_reg_write(v, offset, (uint32_t)msr_content);
@@ -893,10 +891,12 @@ const struct hvm_mmio_handler vlapic_mmi
     .write_handler = vlapic_write
 };
 
-void vlapic_msr_set(struct vlapic *vlapic, uint64_t value)
+bool_t vlapic_msr_set(struct vlapic *vlapic, uint64_t value)
 {
     if ( (vlapic->hw.apic_base_msr ^ value) & MSR_IA32_APICBASE_ENABLE )
     {
+        if ( unlikely(value & MSR_IA32_APICBASE_EXTD) )
+            return 0;
         if ( value & MSR_IA32_APICBASE_ENABLE )
         {
             vlapic_reset(vlapic);
@@ -905,10 +905,15 @@ void vlapic_msr_set(struct vlapic *vlapi
         }
         else
         {
+            if ( unlikely(vlapic->hw.apic_base_msr & MSR_IA32_APICBASE_EXTD) )
+                return 0;
             vlapic->hw.disabled |= VLAPIC_HW_DISABLED;
             pt_may_unmask_irq(vlapic_domain(vlapic), NULL);
         }
     }
+    else if ( !(value & MSR_IA32_APICBASE_ENABLE) &&
+              unlikely(value & MSR_IA32_APICBASE_EXTD) )
+        return 0;
 
     vlapic->hw.apic_base_msr = value;
 
@@ -923,6 +928,8 @@ void vlapic_msr_set(struct vlapic *vlapi
 
     HVM_DBG_LOG(DBG_LEVEL_VLAPIC,
                 "apic base msr is 0x%016"PRIx64, vlapic->hw.apic_base_msr);
+
+    return 1;
 }
 
 uint64_t  vlapic_tdt_msr_get(struct vlapic *vlapic)
@@ -1206,6 +1213,10 @@ static int lapic_load_hidden(struct doma
     if ( hvm_load_entry_zeroextend(LAPIC, h, &s->hw) != 0 ) 
         return -EINVAL;
 
+    if ( !(s->hw.apic_base_msr & MSR_IA32_APICBASE_ENABLE) &&
+         unlikely(s->hw.apic_base_msr & MSR_IA32_APICBASE_EXTD) )
+        return -EINVAL;
+
     vmx_vlapic_msr_changed(v);
 
     return 0;
--- a/xen/include/asm-x86/hvm/vlapic.h
+++ b/xen/include/asm-x86/hvm/vlapic.h
@@ -106,7 +106,7 @@ void vlapic_destroy(struct vcpu *v);
 
 void vlapic_reset(struct vlapic *vlapic);
 
-void vlapic_msr_set(struct vlapic *vlapic, uint64_t value);
+bool_t vlapic_msr_set(struct vlapic *vlapic, uint64_t value);
 void vlapic_tdt_msr_set(struct vlapic *vlapic, uint64_t value);
 uint64_t vlapic_tdt_msr_get(struct vlapic *vlapic);
 





_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

--------------090009090208090004080704-- --===============7497729734945942434== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --===============7497729734945942434==--