* [PATCH v6] x86/vlapic: don't silently accept bad vectors
@ 2014-10-06 15:11 Jan Beulich
2014-10-06 15:16 ` Andrew Cooper
2014-10-06 15:44 ` Andrew Cooper
0 siblings, 2 replies; 5+ messages in thread
From: Jan Beulich @ 2014-10-06 15:11 UTC (permalink / raw)
To: xen-devel; +Cc: Keir Fraser
[-- Attachment #1: Type: text/plain, Size: 3074 bytes --]
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>
---
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;
[-- Attachment #2: x86-HVM-LAPIC-bad-vector.patch --]
[-- Type: text/plain, Size: 3117 bytes --]
x86/vlapic: don't silently accept bad vectors
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>
---
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;
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v6] x86/vlapic: don't silently accept bad vectors
2014-10-06 15:11 [PATCH v6] x86/vlapic: don't silently accept bad vectors Jan Beulich
@ 2014-10-06 15:16 ` Andrew Cooper
2014-10-06 15:29 ` Jan Beulich
2014-10-06 15:44 ` Andrew Cooper
1 sibling, 1 reply; 5+ messages in thread
From: Andrew Cooper @ 2014-10-06 15:16 UTC (permalink / raw)
To: Jan Beulich, xen-devel; +Cc: Keir Fraser
[-- Attachment #1.1: Type: text/plain, Size: 3391 bytes --]
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
[-- Attachment #1.2: Type: text/html, Size: 4153 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v6] x86/vlapic: don't silently accept bad vectors
2014-10-06 15:16 ` Andrew Cooper
@ 2014-10-06 15:29 ` Jan Beulich
2014-10-06 15:33 ` Andrew Cooper
0 siblings, 1 reply; 5+ messages in thread
From: Jan Beulich @ 2014-10-06 15:29 UTC (permalink / raw)
To: Andrew Cooper; +Cc: xen-devel, Keir Fraser
>>> On 06.10.14 at 17:16, <andrew.cooper3@citrix.com> wrote:
> 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?
It sends one or more NMI IPIs when about to reboot (with, as
mandated, the vector field being zero).
Jan
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v6] x86/vlapic: don't silently accept bad vectors
2014-10-06 15:29 ` Jan Beulich
@ 2014-10-06 15:33 ` Andrew Cooper
0 siblings, 0 replies; 5+ messages in thread
From: Andrew Cooper @ 2014-10-06 15:33 UTC (permalink / raw)
To: Jan Beulich; +Cc: xen-devel, Keir Fraser
On 06/10/14 16:29, Jan Beulich wrote:
>>>> On 06.10.14 at 17:16, <andrew.cooper3@citrix.com> wrote:
>> 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?
> It sends one or more NMI IPIs when about to reboot (with, as
> mandated, the vector field being zero).
>
> Jan
Ah yes - that would do it.
~Andrew
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v6] x86/vlapic: don't silently accept bad vectors
2014-10-06 15:11 [PATCH v6] x86/vlapic: don't silently accept bad vectors Jan Beulich
2014-10-06 15:16 ` Andrew Cooper
@ 2014-10-06 15:44 ` Andrew Cooper
1 sibling, 0 replies; 5+ messages in thread
From: Andrew Cooper @ 2014-10-06 15:44 UTC (permalink / raw)
To: Jan Beulich, xen-devel; +Cc: Keir Fraser
[-- Attachment #1.1: Type: text/plain, Size: 3498 bytes --]
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>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> 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
[-- Attachment #1.2: Type: text/html, Size: 4224 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-10-06 15:44 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-10-06 15:11 [PATCH v6] x86/vlapic: don't silently accept bad vectors Jan Beulich
2014-10-06 15:16 ` Andrew Cooper
2014-10-06 15:29 ` Jan Beulich
2014-10-06 15:33 ` Andrew Cooper
2014-10-06 15:44 ` Andrew Cooper
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.