Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1240978 > unrolled thread

Re: [PATCH 2/2] KVM: x86: fix edge EOI and IOAPIC reconfig race

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 2/2] KVM: x86: fix edge EOI and IOAPIC reconfig race Radim Krčmář <rkrcmar@redhat.com> - 2015-10-06 22:40 +0200
    Re: [PATCH 2/2] KVM: x86: fix edge EOI and IOAPIC reconfig race Paolo Bonzini <pbonzini@redhat.com> - 2015-10-07 11:40 +0200
      Re: [PATCH 2/2] KVM: x86: fix edge EOI and IOAPIC reconfig race Radim Krčmář <rkrcmar@redhat.com> - 2015-10-07 17:10 +0200

#1240978 — Re: [PATCH 2/2] KVM: x86: fix edge EOI and IOAPIC reconfig race

2015-08-15 02:00+0200, Paolo Bonzini:
>On 14/08/2015 10:38, Radim Krčmář wrote:
>>> How do you reproduce the bug?
>> I run rhel4 (2.6.9) kernel on 2 VCPUs and frequently alternate
>> smp_affinity of "timer".  The bug is hit within seconds.
> 
> Nice, I'll try to make a unit test for it on the plane. :)

(Time on planes is best spent by sleeping ;])

What do you think about the series?

I made a prototype kvm-unit-test ...
(Reproduces with APICv or split irqchip builds.)
---8<---
x86: test IO-APIC dest_id modification before

Regression test for kvm commit $TODO.
Run with '-smp 2'.

I had problems with handle_irq so this version uses asm workaround;
will fix that in final version.  Initialization also presumes that
everything will work as it did on my machines :)
---
 x86/ioapic.c | 47 +++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 45 insertions(+), 2 deletions(-)

diff --git a/x86/ioapic.c b/x86/ioapic.c
index 1fe1ccc9eadd..55f8eea03496 100644
--- a/x86/ioapic.c
+++ b/x86/ioapic.c
@@ -4,21 +4,27 @@
 #include "smp.h"
 #include "desc.h"
 #include "isr.h"
+#include "io.h"
 
 #define EDGE_TRIGGERED 0
 #define LEVEL_TRIGGERED 1
 
-static void set_ioapic_redir(unsigned line, unsigned vec, unsigned trig_mode)
+static void __set_ioapic_redir(unsigned line, u8 vec, bool trig_mode, u8 dest_id)
 {
 	ioapic_redir_entry_t e = {
 		.vector = vec,
-		.delivery_mode = 0,
 		.trig_mode = trig_mode,
+		.dest_id = dest_id,
 	};
 
 	ioapic_write_redir(line, e);
 }
 
+static void set_ioapic_redir(unsigned line, unsigned vec, unsigned trig_mode)
+{
+	__set_ioapic_redir(line, vec, trig_mode, 0);
+}
+
 static void set_irq_line(unsigned line, int val)
 {
 	asm volatile("out %0, %1" : : "a"((u8)val), "d"((u16)(0x2000 + line)));
@@ -298,6 +304,41 @@ static void test_ioapic_level_retrigger_mask(void)
 	set_mask(0x0e, false);
 }
 
+static volatile int pit_working = -1;
+
+static void __attribute__((used)) pit_isr_fn(void)
+{
+	if (!pit_working++);
+		__set_ioapic_redir(0x2, 0x84, EDGE_TRIGGERED, 1);
+	eoi();
+}
+
+void pit_isr(void);
+asm (
+     "pit_isr: \n"
+     "   call pit_isr_fn \n"
+#ifndef __x86_64__
+     "   iret"
+#else
+     "   iretq"
+#endif
+     );
+
+static void test_ioapic_eoi_bug(void)
+{
+	if (cpu_count() < 2)
+		return;
+
+	set_idt_entry(0x84, pit_isr, 0);
+	__set_ioapic_redir(0x2, 0x84, EDGE_TRIGGERED, 0);
+
+	outb(0x34, 0x43);
+
+	for (unsigned loops = 100000000; loops && pit_working < 1; loops--)
+		asm volatile ("nop");
+
+	report("dest_id reconfiguration before EOI", pit_working >= 1);
+}
 
 int main(void)
 {
@@ -325,5 +366,7 @@ int main(void)
 	test_ioapic_level_mask();
 	test_ioapic_level_retrigger_mask();
 
+	test_ioapic_eoi_bug();
+
 	return report_summary();
 }
-- 
2.5.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1241293

On 06/10/2015 22:33, Radim Krčmář wrote:
> 2015-08-15 02:00+0200, Paolo Bonzini:
>> On 14/08/2015 10:38, Radim Krčmář wrote:
>>>> How do you reproduce the bug?
>>> I run rhel4 (2.6.9) kernel on 2 VCPUs and frequently alternate
>>> smp_affinity of "timer".  The bug is hit within seconds.
>>
>> Nice, I'll try to make a unit test for it on the plane. :)
> 
> (Time on planes is best spent by sleeping ;])

That's what I ended up doing. :)

> What do you think about the series?

I had just a few small comments.  I had all but forgotten it, sorry.
The unit test makes the bug clear, thanks.  What's the issue with
handle_irq?

Paolo

> I made a prototype kvm-unit-test ...
> (Reproduces with APICv or split irqchip builds.)
> ---8<---
> x86: test IO-APIC dest_id modification before
> 
> Regression test for kvm commit $TODO.
> Run with '-smp 2'.
> 
> I had problems with handle_irq so this version uses asm workaround;
> will fix that in final version.  Initialization also presumes that
> everything will work as it did on my machines :)
> ---
>  x86/ioapic.c | 47 +++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 45 insertions(+), 2 deletions(-)
> 
> diff --git a/x86/ioapic.c b/x86/ioapic.c
> index 1fe1ccc9eadd..55f8eea03496 100644
> --- a/x86/ioapic.c
> +++ b/x86/ioapic.c
> @@ -4,21 +4,27 @@
>  #include "smp.h"
>  #include "desc.h"
>  #include "isr.h"
> +#include "io.h"
>  
>  #define EDGE_TRIGGERED 0
>  #define LEVEL_TRIGGERED 1
>  
> -static void set_ioapic_redir(unsigned line, unsigned vec, unsigned trig_mode)
> +static void __set_ioapic_redir(unsigned line, u8 vec, bool trig_mode, u8 dest_id)
>  {
>  	ioapic_redir_entry_t e = {
>  		.vector = vec,
> -		.delivery_mode = 0,
>  		.trig_mode = trig_mode,
> +		.dest_id = dest_id,
>  	};
>  
>  	ioapic_write_redir(line, e);
>  }
>  
> +static void set_ioapic_redir(unsigned line, unsigned vec, unsigned trig_mode)
> +{
> +	__set_ioapic_redir(line, vec, trig_mode, 0);
> +}
> +
>  static void set_irq_line(unsigned line, int val)
>  {
>  	asm volatile("out %0, %1" : : "a"((u8)val), "d"((u16)(0x2000 + line)));
> @@ -298,6 +304,41 @@ static void test_ioapic_level_retrigger_mask(void)
>  	set_mask(0x0e, false);
>  }
>  
> +static volatile int pit_working = -1;
> +
> +static void __attribute__((used)) pit_isr_fn(void)
> +{
> +	if (!pit_working++);
> +		__set_ioapic_redir(0x2, 0x84, EDGE_TRIGGERED, 1);
> +	eoi();
> +}
> +
> +void pit_isr(void);
> +asm (
> +     "pit_isr: \n"
> +     "   call pit_isr_fn \n"
> +#ifndef __x86_64__
> +     "   iret"
> +#else
> +     "   iretq"
> +#endif
> +     );
> +
> +static void test_ioapic_eoi_bug(void)
> +{
> +	if (cpu_count() < 2)
> +		return;
> +
> +	set_idt_entry(0x84, pit_isr, 0);
> +	__set_ioapic_redir(0x2, 0x84, EDGE_TRIGGERED, 0);
> +
> +	outb(0x34, 0x43);
> +
> +	for (unsigned loops = 100000000; loops && pit_working < 1; loops--)
> +		asm volatile ("nop");
> +
> +	report("dest_id reconfiguration before EOI", pit_working >= 1);
> +}
>  
>  int main(void)
>  {
> @@ -325,5 +366,7 @@ int main(void)
>  	test_ioapic_level_mask();
>  	test_ioapic_level_retrigger_mask();
>  
> +	test_ioapic_eoi_bug();
> +
>  	return report_summary();
>  }
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1241580

2015-10-07 11:31+0200, Paolo Bonzini:
>                                             What's the issue with
> handle_irq?

I get #PF instead of callback after redirecting to VCPU 1.
No idea what causes it, yet -- seeing handle_irq's iplementation made me
postpone debugging :)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web