From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH v6] x86/vlapic: don't silently accept bad vectors Date: Mon, 6 Oct 2014 16:16:24 +0100 Message-ID: <5432B248.6010601@citrix.com> References: <5432CD4B020000780003C882@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============4028408714307959018==" Return-path: Received: from mail6.bemta4.messagelabs.com ([85.158.143.247]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1XbA1Z-0006GB-G0 for xen-devel@lists.xenproject.org; Mon, 06 Oct 2014 15:16:33 +0000 In-Reply-To: <5432CD4B020000780003C882@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 --===============4028408714307959018== Content-Type: multipart/alternative; boundary="------------070907020205070200020204" --------------070907020205070200020204 Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit On 06/10/14 16:11, Jan Beulich wrote: > Vectors 0-15 are reserved, and a physical LAPIC - upon sending or > receiving one - would generate an APIC error instead of doing the > requested action. Make our emulation behave similarly. > > Signed-off-by: Jan Beulich What was windows actually tripping up on? ~Andrew > --- > v6: Only check "Lowest Priority" and "Fixed" delivery mode vectors in > vlapic_ipi(). Check the former regardless of whether a target to > send to was found. > > --- a/xen/arch/x86/hvm/vlapic.c > +++ b/xen/arch/x86/hvm/vlapic.c > @@ -123,10 +123,34 @@ static int vlapic_find_highest_irr(struc > return vlapic_find_highest_vector(&vlapic->regs->data[APIC_IRR]); > } > > +static void vlapic_error(struct vlapic *vlapic, unsigned int errmask) > +{ > + unsigned long flags; > + uint32_t esr; > + > + spin_lock_irqsave(&vlapic->esr_lock, flags); > + esr = vlapic_get_reg(vlapic, APIC_ESR); > + if ( (esr & errmask) != errmask ) > + { > + uint32_t lvterr = vlapic_get_reg(vlapic, APIC_LVTERR); > + > + vlapic_set_reg(vlapic, APIC_ESR, esr | errmask); > + if ( !(lvterr & APIC_LVT_MASKED) ) > + vlapic_set_irq(vlapic, lvterr & APIC_VECTOR_MASK, 0); > + } > + spin_unlock_irqrestore(&vlapic->esr_lock, flags); > +} > + > void vlapic_set_irq(struct vlapic *vlapic, uint8_t vec, uint8_t trig) > { > struct vcpu *target = vlapic_vcpu(vlapic); > > + if ( unlikely(vec < 16) ) > + { > + vlapic_error(vlapic, APIC_ESR_RECVILL); > + return; > + } > + > if ( trig ) > vlapic_set_vector(vec, &vlapic->regs->data[APIC_TMR]); > > @@ -459,11 +483,21 @@ void vlapic_ipi( > case APIC_DM_LOWEST: { > struct vlapic *target = vlapic_lowest_prio( > vlapic_domain(vlapic), vlapic, short_hand, dest, dest_mode); > - if ( target != NULL ) > + > + if ( unlikely((icr_low & APIC_VECTOR_MASK) < 16) ) > + vlapic_error(vlapic, APIC_ESR_SENDILL); > + else if ( target ) > vlapic_accept_irq(vlapic_vcpu(target), icr_low); > break; > } > > + case APIC_DM_FIXED: > + if ( unlikely((icr_low & APIC_VECTOR_MASK) < 16) ) > + { > + vlapic_error(vlapic, APIC_ESR_SENDILL); > + break; > + } > + /* fall through */ > default: { > struct vcpu *v; > bool_t batch = is_multicast_dest(vlapic, short_hand, dest, dest_mode); > @@ -1404,6 +1438,8 @@ int vlapic_init(struct vcpu *v) > if ( v->vcpu_id == 0 ) > vlapic->hw.apic_base_msr |= MSR_IA32_APICBASE_BSP; > > + spin_lock_init(&vlapic->esr_lock); > + > tasklet_init(&vlapic->init_sipi.tasklet, > vlapic_init_sipi_action, > (unsigned long)v); > --- a/xen/include/asm-x86/hvm/vlapic.h > +++ b/xen/include/asm-x86/hvm/vlapic.h > @@ -77,6 +77,7 @@ struct vlapic { > bool_t hw, regs; > uint32_t id, ldr; > } loaded; > + spinlock_t esr_lock; > struct periodic_time pt; > s_time_t timer_last_update; > struct page_info *regs_page; > > > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel --------------070907020205070200020204 Content-Type: text/html; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit
On 06/10/14 16:11, Jan Beulich wrote:
Vectors 0-15 are reserved, and a physical LAPIC - upon sending or
receiving one - would generate an APIC error instead of doing the
requested action. Make our emulation behave similarly.

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

What was windows actually tripping up on?

~Andrew

---
v6: Only check "Lowest Priority" and "Fixed" delivery mode vectors in
    vlapic_ipi(). Check the former regardless of whether a target to
    send to was found.

--- a/xen/arch/x86/hvm/vlapic.c
+++ b/xen/arch/x86/hvm/vlapic.c
@@ -123,10 +123,34 @@ static int vlapic_find_highest_irr(struc
     return vlapic_find_highest_vector(&vlapic->regs->data[APIC_IRR]);
 }
 
+static void vlapic_error(struct vlapic *vlapic, unsigned int errmask)
+{
+    unsigned long flags;
+    uint32_t esr;
+
+    spin_lock_irqsave(&vlapic->esr_lock, flags);
+    esr = vlapic_get_reg(vlapic, APIC_ESR);
+    if ( (esr & errmask) != errmask )
+    {
+        uint32_t lvterr = vlapic_get_reg(vlapic, APIC_LVTERR);
+
+        vlapic_set_reg(vlapic, APIC_ESR, esr | errmask);
+        if ( !(lvterr & APIC_LVT_MASKED) )
+            vlapic_set_irq(vlapic, lvterr & APIC_VECTOR_MASK, 0);
+    }
+    spin_unlock_irqrestore(&vlapic->esr_lock, flags);
+}
+
 void vlapic_set_irq(struct vlapic *vlapic, uint8_t vec, uint8_t trig)
 {
     struct vcpu *target = vlapic_vcpu(vlapic);
 
+    if ( unlikely(vec < 16) )
+    {
+        vlapic_error(vlapic, APIC_ESR_RECVILL);
+        return;
+    }
+
     if ( trig )
         vlapic_set_vector(vec, &vlapic->regs->data[APIC_TMR]);
 
@@ -459,11 +483,21 @@ void vlapic_ipi(
     case APIC_DM_LOWEST: {
         struct vlapic *target = vlapic_lowest_prio(
             vlapic_domain(vlapic), vlapic, short_hand, dest, dest_mode);
-        if ( target != NULL )
+
+        if ( unlikely((icr_low & APIC_VECTOR_MASK) < 16) )
+            vlapic_error(vlapic, APIC_ESR_SENDILL);
+        else if ( target )
             vlapic_accept_irq(vlapic_vcpu(target), icr_low);
         break;
     }
 
+    case APIC_DM_FIXED:
+        if ( unlikely((icr_low & APIC_VECTOR_MASK) < 16) )
+        {
+            vlapic_error(vlapic, APIC_ESR_SENDILL);
+            break;
+        }
+        /* fall through */
     default: {
         struct vcpu *v;
         bool_t batch = is_multicast_dest(vlapic, short_hand, dest, dest_mode);
@@ -1404,6 +1438,8 @@ int vlapic_init(struct vcpu *v)
     if ( v->vcpu_id == 0 )
         vlapic->hw.apic_base_msr |= MSR_IA32_APICBASE_BSP;
 
+    spin_lock_init(&vlapic->esr_lock);
+
     tasklet_init(&vlapic->init_sipi.tasklet,
                  vlapic_init_sipi_action,
                  (unsigned long)v);
--- a/xen/include/asm-x86/hvm/vlapic.h
+++ b/xen/include/asm-x86/hvm/vlapic.h
@@ -77,6 +77,7 @@ struct vlapic {
         bool_t               hw, regs;
         uint32_t             id, ldr;
     }                        loaded;
+    spinlock_t               esr_lock;
     struct periodic_time     pt;
     s_time_t                 timer_last_update;
     struct page_info         *regs_page;






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

--------------070907020205070200020204-- --===============4028408714307959018== 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 --===============4028408714307959018==--