(no subject)

[Lin Ming]

Hi all,

These 2 patches try to fix the "perf top" soft lockups under Xen
reported by Steven at: https://lkml.org/lkml/2012/2/9/506

I tested it with 3.4-rc2 and "perf top" works well now.

Steven,
Could you please help to test it too?

The soft lockup code path is:

__irq_work_queue
  arch_irq_work_raise
    apic->send_IPI_self(IRQ_WORK_VECTOR);
      apic_send_IPI_self
        __default_send_IPI_shortcut
          __xapic_wait_icr_idle

static inline void __xapic_wait_icr_idle(void)
{
        while (native_apic_mem_read(APIC_ICR) & APIC_ICR_BUSY)
                cpu_relax();
}

The lockup happens at above while looop.
                                                                
The cause is that Xen has not implemented the APIC IPI interface yet.
Xen has IPI interface: xen_send_IPI_one, but it's only used in
xen_smp_send_reschedule, xen_smp_send_call_function_ipi and
xen_smp_send_call_function_single_ipi, etc.

So we need to implement Xen's APIC IPI interface as Ben's patch does.
And implement Xen's IRQ_WORK_VECTOR handler.

Ben Guthro (1):
      xen: implement apic ipi interface

Lin Ming (1):
      xen: implement IRQ_WORK_VECTOR handler

 arch/x86/include/asm/xen/events.h |    1 +
 arch/x86/xen/enlighten.c          |    7 ++
 arch/x86/xen/smp.c                |  111 +++++++++++++++++++++++++++++++++++-
 arch/x86/xen/smp.h                |   12 ++++
 4 files changed, 127 insertions(+), 4 deletions(-)

Any comment is appreciated.
Lin Ming

[PATCH 1/2] xen: implement apic ipi interface

[Lin Ming]

From: Ben Guthro <ben@guthro.net>

Map native ipi vector to xen vector.
Implement apic ipi interface with xen_send_IPI_one.

Signed-off-by: Ben Guthro <ben@guthro.net>
Signed-off-by: Lin Ming <mlin@ss.pku.edu.cn>
---

Ben,

This patch was taken from part of your patch at:
https://lkml.org/lkml/2012/2/10/681

Is it OK that I added your SOB?

I did some change to map native ipi vector to xen vector.
Could you please review?

Thanks.

 arch/x86/xen/enlighten.c |    7 ++++
 arch/x86/xen/smp.c       |   81 +++++++++++++++++++++++++++++++++++++++++++--
 arch/x86/xen/smp.h       |   12 +++++++
 3 files changed, 96 insertions(+), 4 deletions(-)
 create mode 100644 arch/x86/xen/smp.h

diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index 4f51beb..be7dbc8 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -74,6 +74,7 @@
 
 #include "xen-ops.h"
 #include "mmu.h"
+#include "smp.h"
 #include "multicalls.h"
 
 EXPORT_SYMBOL_GPL(hypercall_page);
@@ -849,6 +850,12 @@ static void set_xen_basic_apic_ops(void)
 	apic->icr_write = xen_apic_icr_write;
 	apic->wait_icr_idle = xen_apic_wait_icr_idle;
 	apic->safe_wait_icr_idle = xen_safe_apic_wait_icr_idle;
+
+	apic->send_IPI_allbutself = xen_send_IPI_allbutself;
+	apic->send_IPI_mask_allbutself = xen_send_IPI_mask_allbutself;
+	apic->send_IPI_mask = xen_send_IPI_mask;
+	apic->send_IPI_all = xen_send_IPI_all;
+	apic->send_IPI_self = xen_send_IPI_self;
 }
 
 #endif
diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c
index 5fac691..2dc6628 100644
--- a/arch/x86/xen/smp.c
+++ b/arch/x86/xen/smp.c
@@ -465,8 +465,8 @@ static void xen_smp_send_reschedule(int cpu)
 	xen_send_IPI_one(cpu, XEN_RESCHEDULE_VECTOR);
 }
 
-static void xen_send_IPI_mask(const struct cpumask *mask,
-			      enum ipi_vector vector)
+static void __xen_send_IPI_mask(const struct cpumask *mask,
+			      int vector)
 {
 	unsigned cpu;
 
@@ -478,7 +478,7 @@ static void xen_smp_send_call_function_ipi(const struct cpumask *mask)
 {
 	int cpu;
 
-	xen_send_IPI_mask(mask, XEN_CALL_FUNCTION_VECTOR);
+	__xen_send_IPI_mask(mask, XEN_CALL_FUNCTION_VECTOR);
 
 	/* Make sure other vcpus get a chance to run if they need to. */
 	for_each_cpu(cpu, mask) {
@@ -491,10 +491,83 @@ static void xen_smp_send_call_function_ipi(const struct cpumask *mask)
 
 static void xen_smp_send_call_function_single_ipi(int cpu)
 {
-	xen_send_IPI_mask(cpumask_of(cpu),
+	__xen_send_IPI_mask(cpumask_of(cpu),
 			  XEN_CALL_FUNCTION_SINGLE_VECTOR);
 }
 
+static inline int xen_map_vector(int vector)
+{
+	int xen_vector;
+
+	switch (vector) {
+	case RESCHEDULE_VECTOR:
+		xen_vector = XEN_RESCHEDULE_VECTOR;
+		break;
+	case CALL_FUNCTION_VECTOR:
+		xen_vector = XEN_CALL_FUNCTION_VECTOR;
+		break;
+	case CALL_FUNCTION_SINGLE_VECTOR:
+		xen_vector = XEN_CALL_FUNCTION_SINGLE_VECTOR;
+		break;
+	default:
+		xen_vector = -1;
+		printk(KERN_ERR "xen: vector 0x%x is not implemented\n",
+			vector);
+	}
+
+	return xen_vector;
+}
+
+void xen_send_IPI_mask(const struct cpumask *mask,
+			      int vector)
+{
+	int xen_vector = xen_map_vector(vector);
+
+	if (xen_vector >= 0)
+		__xen_send_IPI_mask(mask, xen_vector);
+}
+
+void xen_send_IPI_all(int vector)
+{
+	int xen_vector = xen_map_vector(vector);
+
+	if (xen_vector >= 0)
+		__xen_send_IPI_mask(cpu_online_mask, xen_vector);
+}
+
+void xen_send_IPI_self(int vector)
+{
+	int xen_vector = xen_map_vector(vector);
+
+	if (xen_vector >= 0)
+		xen_send_IPI_one(smp_processor_id(), xen_vector);
+}
+
+void xen_send_IPI_mask_allbutself(const struct cpumask *mask,
+				int vector)
+{
+	unsigned cpu;
+	unsigned int this_cpu = smp_processor_id();
+
+	if (!(num_online_cpus() > 1))
+		return;
+
+	for_each_cpu_and(cpu, mask, cpu_online_mask) {
+		if (this_cpu == cpu)
+			continue;
+
+		xen_smp_send_call_function_single_ipi(cpu);
+	}
+}
+
+void xen_send_IPI_allbutself(int vector)
+{
+	int xen_vector = xen_map_vector(vector);
+
+	if (xen_vector >= 0)
+		xen_send_IPI_mask_allbutself(cpu_online_mask, xen_vector);
+}
+
 static irqreturn_t xen_call_function_interrupt(int irq, void *dev_id)
 {
 	irq_enter();
diff --git a/arch/x86/xen/smp.h b/arch/x86/xen/smp.h
new file mode 100644
index 0000000..8981a76
--- /dev/null
+++ b/arch/x86/xen/smp.h
@@ -0,0 +1,12 @@
+#ifndef _XEN_SMP_H
+
+extern void xen_send_IPI_mask(const struct cpumask *mask,
+			      int vector);
+extern void xen_send_IPI_mask_allbutself(const struct cpumask *mask,
+				int vector);
+extern void xen_send_IPI_allbutself(int vector);
+extern void physflat_send_IPI_allbutself(int vector);
+extern void xen_send_IPI_all(int vector);
+extern void xen_send_IPI_self(int vector);
+
+#endif
-- 
1.7.2.5

[PATCH 2/2] xen: implement IRQ_WORK_VECTOR handler

[Lin Ming]

Signed-off-by: Lin Ming <mlin@ss.pku.edu.cn>
---
 arch/x86/include/asm/xen/events.h |    1 +
 arch/x86/xen/smp.c                |   30 ++++++++++++++++++++++++++++++
 2 files changed, 31 insertions(+), 0 deletions(-)

diff --git a/arch/x86/include/asm/xen/events.h b/arch/x86/include/asm/xen/events.h
index 1df3541..cc146d5 100644
--- a/arch/x86/include/asm/xen/events.h
+++ b/arch/x86/include/asm/xen/events.h
@@ -6,6 +6,7 @@ enum ipi_vector {
 	XEN_CALL_FUNCTION_VECTOR,
 	XEN_CALL_FUNCTION_SINGLE_VECTOR,
 	XEN_SPIN_UNLOCK_VECTOR,
+	XEN_IRQ_WORK_VECTOR,
 
 	XEN_NR_IPIS,
 };
diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c
index 2dc6628..92ad12d 100644
--- a/arch/x86/xen/smp.c
+++ b/arch/x86/xen/smp.c
@@ -16,6 +16,7 @@
 #include <linux/err.h>
 #include <linux/slab.h>
 #include <linux/smp.h>
+#include <linux/irq_work.h>
 
 #include <asm/paravirt.h>
 #include <asm/desc.h>
@@ -41,10 +42,12 @@ cpumask_var_t xen_cpu_initialized_map;
 static DEFINE_PER_CPU(int, xen_resched_irq);
 static DEFINE_PER_CPU(int, xen_callfunc_irq);
 static DEFINE_PER_CPU(int, xen_callfuncsingle_irq);
+static DEFINE_PER_CPU(int, xen_irq_work);
 static DEFINE_PER_CPU(int, xen_debug_irq) = -1;
 
 static irqreturn_t xen_call_function_interrupt(int irq, void *dev_id);
 static irqreturn_t xen_call_function_single_interrupt(int irq, void *dev_id);
+static irqreturn_t xen_irq_work_interrupt(int irq, void *dev_id);
 
 /*
  * Reschedule call back.
@@ -143,6 +146,17 @@ static int xen_smp_intr_init(unsigned int cpu)
 		goto fail;
 	per_cpu(xen_callfuncsingle_irq, cpu) = rc;
 
+	callfunc_name = kasprintf(GFP_KERNEL, "irqwork%d", cpu);
+	rc = bind_ipi_to_irqhandler(XEN_IRQ_WORK_VECTOR,
+				    cpu,
+				    xen_irq_work_interrupt,
+				    IRQF_DISABLED|IRQF_PERCPU|IRQF_NOBALANCING,
+				    callfunc_name,
+				    NULL);
+	if (rc < 0)
+		goto fail;
+	per_cpu(xen_irq_work, cpu) = rc;
+
 	return 0;
 
  fail:
@@ -155,6 +169,8 @@ static int xen_smp_intr_init(unsigned int cpu)
 	if (per_cpu(xen_callfuncsingle_irq, cpu) >= 0)
 		unbind_from_irqhandler(per_cpu(xen_callfuncsingle_irq, cpu),
 				       NULL);
+	if (per_cpu(xen_irq_work, cpu) >= 0)
+		unbind_from_irqhandler(per_cpu(xen_irq_work, cpu), NULL);
 
 	return rc;
 }
@@ -509,6 +525,9 @@ static inline int xen_map_vector(int vector)
 	case CALL_FUNCTION_SINGLE_VECTOR:
 		xen_vector = XEN_CALL_FUNCTION_SINGLE_VECTOR;
 		break;
+	case IRQ_WORK_VECTOR:
+		xen_vector = XEN_IRQ_WORK_VECTOR;
+		break;
 	default:
 		xen_vector = -1;
 		printk(KERN_ERR "xen: vector 0x%x is not implemented\n",
@@ -588,6 +607,16 @@ static irqreturn_t xen_call_function_single_interrupt(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
+static irqreturn_t xen_irq_work_interrupt(int irq, void *dev_id)
+{
+	irq_enter();
+	inc_irq_stat(apic_irq_work_irqs);
+	irq_work_run();
+	irq_exit();
+
+	return IRQ_HANDLED;
+}
+
 static const struct smp_ops xen_smp_ops __initconst = {
 	.smp_prepare_boot_cpu = xen_smp_prepare_boot_cpu,
 	.smp_prepare_cpus = xen_smp_prepare_cpus,
@@ -634,6 +663,7 @@ static void xen_hvm_cpu_die(unsigned int cpu)
 	unbind_from_irqhandler(per_cpu(xen_callfunc_irq, cpu), NULL);
 	unbind_from_irqhandler(per_cpu(xen_debug_irq, cpu), NULL);
 	unbind_from_irqhandler(per_cpu(xen_callfuncsingle_irq, cpu), NULL);
+	unbind_from_irqhandler(per_cpu(xen_irq_work, cpu), NULL);
 	native_cpu_die(cpu);
 }
 
-- 
1.7.2.5

Re: [PATCH 2/2] xen: implement IRQ_WORK_VECTOR handler

[Konrad Rzeszutek Wilk]

On Sun, Apr 15, 2012 at 02:09:32PM +0800, Lin Ming wrote:
> Signed-off-by: Lin Ming <mlin@ss.pku.edu.cn>
> ---
>  arch/x86/include/asm/xen/events.h |    1 +
>  arch/x86/xen/smp.c                |   30 ++++++++++++++++++++++++++++++
>  2 files changed, 31 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/x86/include/asm/xen/events.h b/arch/x86/include/asm/xen/events.h
> index 1df3541..cc146d5 100644
> --- a/arch/x86/include/asm/xen/events.h
> +++ b/arch/x86/include/asm/xen/events.h
> @@ -6,6 +6,7 @@ enum ipi_vector {
>  	XEN_CALL_FUNCTION_VECTOR,
>  	XEN_CALL_FUNCTION_SINGLE_VECTOR,
>  	XEN_SPIN_UNLOCK_VECTOR,
> +	XEN_IRQ_WORK_VECTOR,
>  
>  	XEN_NR_IPIS,
>  };
> diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c
> index 2dc6628..92ad12d 100644
> --- a/arch/x86/xen/smp.c
> +++ b/arch/x86/xen/smp.c
> @@ -16,6 +16,7 @@
>  #include <linux/err.h>
>  #include <linux/slab.h>
>  #include <linux/smp.h>
> +#include <linux/irq_work.h>
>  
>  #include <asm/paravirt.h>
>  #include <asm/desc.h>
> @@ -41,10 +42,12 @@ cpumask_var_t xen_cpu_initialized_map;
>  static DEFINE_PER_CPU(int, xen_resched_irq);
>  static DEFINE_PER_CPU(int, xen_callfunc_irq);
>  static DEFINE_PER_CPU(int, xen_callfuncsingle_irq);
> +static DEFINE_PER_CPU(int, xen_irq_work);
>  static DEFINE_PER_CPU(int, xen_debug_irq) = -1;
>  
>  static irqreturn_t xen_call_function_interrupt(int irq, void *dev_id);
>  static irqreturn_t xen_call_function_single_interrupt(int irq, void *dev_id);
> +static irqreturn_t xen_irq_work_interrupt(int irq, void *dev_id);
>  
>  /*
>   * Reschedule call back.
> @@ -143,6 +146,17 @@ static int xen_smp_intr_init(unsigned int cpu)
>  		goto fail;
>  	per_cpu(xen_callfuncsingle_irq, cpu) = rc;
>  
> +	callfunc_name = kasprintf(GFP_KERNEL, "irqwork%d", cpu);
> +	rc = bind_ipi_to_irqhandler(XEN_IRQ_WORK_VECTOR,
> +				    cpu,
> +				    xen_irq_work_interrupt,
> +				    IRQF_DISABLED|IRQF_PERCPU|IRQF_NOBALANCING,
> +				    callfunc_name,
> +				    NULL);
> +	if (rc < 0)
> +		goto fail;
> +	per_cpu(xen_irq_work, cpu) = rc;
> +
>  	return 0;
>  
>   fail:
> @@ -155,6 +169,8 @@ static int xen_smp_intr_init(unsigned int cpu)
>  	if (per_cpu(xen_callfuncsingle_irq, cpu) >= 0)
>  		unbind_from_irqhandler(per_cpu(xen_callfuncsingle_irq, cpu),
>  				       NULL);
> +	if (per_cpu(xen_irq_work, cpu) >= 0)
> +		unbind_from_irqhandler(per_cpu(xen_irq_work, cpu), NULL);
>  
>  	return rc;
>  }
> @@ -509,6 +525,9 @@ static inline int xen_map_vector(int vector)
>  	case CALL_FUNCTION_SINGLE_VECTOR:
>  		xen_vector = XEN_CALL_FUNCTION_SINGLE_VECTOR;
>  		break;
> +	case IRQ_WORK_VECTOR:
> +		xen_vector = XEN_IRQ_WORK_VECTOR;
> +		break;
>  	default:
>  		xen_vector = -1;
>  		printk(KERN_ERR "xen: vector 0x%x is not implemented\n",
> @@ -588,6 +607,16 @@ static irqreturn_t xen_call_function_single_interrupt(int irq, void *dev_id)
>  	return IRQ_HANDLED;
>  }
>  
> +static irqreturn_t xen_irq_work_interrupt(int irq, void *dev_id)
> +{
> +	irq_enter();
> +	inc_irq_stat(apic_irq_work_irqs);
> +	irq_work_run();

I think this usually done the other way around:

irq_work_run()
inc_irq_stat(apic_irq_work_irqs)

Or is there an excellent reason for doing it this way?

> +	irq_exit();
> +
> +	return IRQ_HANDLED;
> +}
> +
>  static const struct smp_ops xen_smp_ops __initconst = {
>  	.smp_prepare_boot_cpu = xen_smp_prepare_boot_cpu,
>  	.smp_prepare_cpus = xen_smp_prepare_cpus,
> @@ -634,6 +663,7 @@ static void xen_hvm_cpu_die(unsigned int cpu)
>  	unbind_from_irqhandler(per_cpu(xen_callfunc_irq, cpu), NULL);
>  	unbind_from_irqhandler(per_cpu(xen_debug_irq, cpu), NULL);
>  	unbind_from_irqhandler(per_cpu(xen_callfuncsingle_irq, cpu), NULL);
> +	unbind_from_irqhandler(per_cpu(xen_irq_work, cpu), NULL);
>  	native_cpu_die(cpu);
>  }
>  
> -- 
> 1.7.2.5

Re: [PATCH 2/2] xen: implement IRQ_WORK_VECTOR handler

[Lin Ming]

On Tue, Apr 17, 2012 at 4:32 AM, Konrad Rzeszutek Wilk
<konrad.wilk@oracle.com> wrote:
> On Sun, Apr 15, 2012 at 02:09:32PM +0800, Lin Ming wrote:
>> Signed-off-by: Lin Ming <mlin@ss.pku.edu.cn>
>> ---
>>  arch/x86/include/asm/xen/events.h |    1 +
>>  arch/x86/xen/smp.c                |   30 ++++++++++++++++++++++++++++++
>>  2 files changed, 31 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/xen/events.h b/arch/x86/include/asm/xen/events.h
>> index 1df3541..cc146d5 100644
>> --- a/arch/x86/include/asm/xen/events.h
>> +++ b/arch/x86/include/asm/xen/events.h
>> @@ -6,6 +6,7 @@ enum ipi_vector {
>>       XEN_CALL_FUNCTION_VECTOR,
>>       XEN_CALL_FUNCTION_SINGLE_VECTOR,
>>       XEN_SPIN_UNLOCK_VECTOR,
>> +     XEN_IRQ_WORK_VECTOR,
>>
>>       XEN_NR_IPIS,
>>  };
>> diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c
>> index 2dc6628..92ad12d 100644
>> --- a/arch/x86/xen/smp.c
>> +++ b/arch/x86/xen/smp.c
>> @@ -16,6 +16,7 @@
>>  #include <linux/err.h>
>>  #include <linux/slab.h>
>>  #include <linux/smp.h>
>> +#include <linux/irq_work.h>
>>
>>  #include <asm/paravirt.h>
>>  #include <asm/desc.h>
>> @@ -41,10 +42,12 @@ cpumask_var_t xen_cpu_initialized_map;
>>  static DEFINE_PER_CPU(int, xen_resched_irq);
>>  static DEFINE_PER_CPU(int, xen_callfunc_irq);
>>  static DEFINE_PER_CPU(int, xen_callfuncsingle_irq);
>> +static DEFINE_PER_CPU(int, xen_irq_work);
>>  static DEFINE_PER_CPU(int, xen_debug_irq) = -1;
>>
>>  static irqreturn_t xen_call_function_interrupt(int irq, void *dev_id);
>>  static irqreturn_t xen_call_function_single_interrupt(int irq, void *dev_id);
>> +static irqreturn_t xen_irq_work_interrupt(int irq, void *dev_id);
>>
>>  /*
>>   * Reschedule call back.
>> @@ -143,6 +146,17 @@ static int xen_smp_intr_init(unsigned int cpu)
>>               goto fail;
>>       per_cpu(xen_callfuncsingle_irq, cpu) = rc;
>>
>> +     callfunc_name = kasprintf(GFP_KERNEL, "irqwork%d", cpu);
>> +     rc = bind_ipi_to_irqhandler(XEN_IRQ_WORK_VECTOR,
>> +                                 cpu,
>> +                                 xen_irq_work_interrupt,
>> +                                 IRQF_DISABLED|IRQF_PERCPU|IRQF_NOBALANCING,
>> +                                 callfunc_name,
>> +                                 NULL);
>> +     if (rc < 0)
>> +             goto fail;
>> +     per_cpu(xen_irq_work, cpu) = rc;
>> +
>>       return 0;
>>
>>   fail:
>> @@ -155,6 +169,8 @@ static int xen_smp_intr_init(unsigned int cpu)
>>       if (per_cpu(xen_callfuncsingle_irq, cpu) >= 0)
>>               unbind_from_irqhandler(per_cpu(xen_callfuncsingle_irq, cpu),
>>                                      NULL);
>> +     if (per_cpu(xen_irq_work, cpu) >= 0)
>> +             unbind_from_irqhandler(per_cpu(xen_irq_work, cpu), NULL);
>>
>>       return rc;
>>  }
>> @@ -509,6 +525,9 @@ static inline int xen_map_vector(int vector)
>>       case CALL_FUNCTION_SINGLE_VECTOR:
>>               xen_vector = XEN_CALL_FUNCTION_SINGLE_VECTOR;
>>               break;
>> +     case IRQ_WORK_VECTOR:
>> +             xen_vector = XEN_IRQ_WORK_VECTOR;
>> +             break;
>>       default:
>>               xen_vector = -1;
>>               printk(KERN_ERR "xen: vector 0x%x is not implemented\n",
>> @@ -588,6 +607,16 @@ static irqreturn_t xen_call_function_single_interrupt(int irq, void *dev_id)
>>       return IRQ_HANDLED;
>>  }
>>
>> +static irqreturn_t xen_irq_work_interrupt(int irq, void *dev_id)
>> +{
>> +     irq_enter();
>> +     inc_irq_stat(apic_irq_work_irqs);
>> +     irq_work_run();
>
> I think this usually done the other way around:
>
> irq_work_run()
> inc_irq_stat(apic_irq_work_irqs)
>
> Or is there an excellent reason for doing it this way?

Copy & paste from smp_irq_work_interrupt().
But I think there is no much difference.

Anyway, I can change it if needed.

Thanks,
Lin Ming

>
>> +     irq_exit();
>> +
>> +     return IRQ_HANDLED;
>> +}
>> +
>>  static const struct smp_ops xen_smp_ops __initconst = {
>>       .smp_prepare_boot_cpu = xen_smp_prepare_boot_cpu,
>>       .smp_prepare_cpus = xen_smp_prepare_cpus,
>> @@ -634,6 +663,7 @@ static void xen_hvm_cpu_die(unsigned int cpu)
>>       unbind_from_irqhandler(per_cpu(xen_callfunc_irq, cpu), NULL);
>>       unbind_from_irqhandler(per_cpu(xen_debug_irq, cpu), NULL);
>>       unbind_from_irqhandler(per_cpu(xen_callfuncsingle_irq, cpu), NULL);
>> +     unbind_from_irqhandler(per_cpu(xen_irq_work, cpu), NULL);
>>       native_cpu_die(cpu);
>>  }
>>
>> --
>> 1.7.2.5

Re: [PATCH 1/2] xen: implement apic ipi interface

[Konrad Rzeszutek Wilk]

On Sun, Apr 15, 2012 at 02:09:31PM +0800, Lin Ming wrote:
> From: Ben Guthro <ben@guthro.net>
> 
> Map native ipi vector to xen vector.
> Implement apic ipi interface with xen_send_IPI_one.

I keep on getting this (so PV guest without any SMP support,
and with just PV frontend drivers):


echo "CONFIG_PARAVIRT_GUEST=y" > linux.config
echo "CONFIG_XEN=y" >> linux.config
echo "CONFIG_XEN_XENBUS_FRONTEND=m" >> linux.config
cat xen.config | grep FRONTEND >> linux.config
echo "# CONFIG_XEN_PRIVILEGED_GUEST is not set" >> linux.config
echo "# CONFIG_XEN_DOM0 is not set" >> linux.config
echo "# CONFIG_ACPI is not set" >> linux.config
echo "# CONFIG_PCI is not set" >> linux.config
echo "# CONFIG_XENFS is not set" >> linux.config
echo "# CONFIG_SMP is not set" >> linux.config


make -j$((4 * 2)) -C /home/konrad/ssd/konrad/xtt-compile/linux O=/home/konrad/ssd/konrad/xtt-compile/linux-build- defconfig
make[2]: Entering directory `/home/konrad/ssd/konrad/linux'
  GEN     /home/konrad/ssd/konrad/xtt-compile/linux-build-/Makefile
*** Default configuration is based on 'x86_64_defconfig'
#
# configuration written to .config

cat linux.config >> linux-build-/.config

make -j4 ..

  LD      init/built-in.o
  LD      .tmp_vmlinux1
arch/x86/built-in.o: In function `xen_start_kernel':
(.init.text+0x5a8): undefined reference to `xen_send_IPI_allbutself'
arch/x86/built-in.o: In function `xen_start_kernel':
(.init.text+0x5b3): undefined reference to `xen_send_IPI_mask_allbutself'
arch/x86/built-in.o: In function `xen_start_kernel':
(.init.text+0x5be): undefined reference to `xen_send_IPI_mask'
arch/x86/built-in.o: In function `xen_start_kernel':
(.init.text+0x5c9): undefined reference to `xen_send_IPI_all'
arch/x86/built-in.o: In function `xen_start_kernel':
(.init.text+0x5d4): undefined reference to `xen_send_IPI_self'
make[2]: *** [.tmp_vmlinux1] Error 1
> 
> Signed-off-by: Ben Guthro <ben@guthro.net>
> Signed-off-by: Lin Ming <mlin@ss.pku.edu.cn>
> ---
> 
> Ben,
> 
> This patch was taken from part of your patch at:
> https://lkml.org/lkml/2012/2/10/681
> 
> Is it OK that I added your SOB?
> 
> I did some change to map native ipi vector to xen vector.
> Could you please review?
> 
> Thanks.
> 
>  arch/x86/xen/enlighten.c |    7 ++++
>  arch/x86/xen/smp.c       |   81 +++++++++++++++++++++++++++++++++++++++++++--
>  arch/x86/xen/smp.h       |   12 +++++++
>  3 files changed, 96 insertions(+), 4 deletions(-)
>  create mode 100644 arch/x86/xen/smp.h
> 
> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> index 4f51beb..be7dbc8 100644
> --- a/arch/x86/xen/enlighten.c
> +++ b/arch/x86/xen/enlighten.c
> @@ -74,6 +74,7 @@
>  
>  #include "xen-ops.h"
>  #include "mmu.h"
> +#include "smp.h"
>  #include "multicalls.h"
>  
>  EXPORT_SYMBOL_GPL(hypercall_page);
> @@ -849,6 +850,12 @@ static void set_xen_basic_apic_ops(void)
>  	apic->icr_write = xen_apic_icr_write;
>  	apic->wait_icr_idle = xen_apic_wait_icr_idle;
>  	apic->safe_wait_icr_idle = xen_safe_apic_wait_icr_idle;
> +
> +	apic->send_IPI_allbutself = xen_send_IPI_allbutself;
> +	apic->send_IPI_mask_allbutself = xen_send_IPI_mask_allbutself;
> +	apic->send_IPI_mask = xen_send_IPI_mask;
> +	apic->send_IPI_all = xen_send_IPI_all;
> +	apic->send_IPI_self = xen_send_IPI_self;
>  }
>  
>  #endif
> diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c
> index 5fac691..2dc6628 100644
> --- a/arch/x86/xen/smp.c
> +++ b/arch/x86/xen/smp.c
> @@ -465,8 +465,8 @@ static void xen_smp_send_reschedule(int cpu)
>  	xen_send_IPI_one(cpu, XEN_RESCHEDULE_VECTOR);
>  }
>  
> -static void xen_send_IPI_mask(const struct cpumask *mask,
> -			      enum ipi_vector vector)
> +static void __xen_send_IPI_mask(const struct cpumask *mask,
> +			      int vector)
>  {
>  	unsigned cpu;
>  
> @@ -478,7 +478,7 @@ static void xen_smp_send_call_function_ipi(const struct cpumask *mask)
>  {
>  	int cpu;
>  
> -	xen_send_IPI_mask(mask, XEN_CALL_FUNCTION_VECTOR);
> +	__xen_send_IPI_mask(mask, XEN_CALL_FUNCTION_VECTOR);
>  
>  	/* Make sure other vcpus get a chance to run if they need to. */
>  	for_each_cpu(cpu, mask) {
> @@ -491,10 +491,83 @@ static void xen_smp_send_call_function_ipi(const struct cpumask *mask)
>  
>  static void xen_smp_send_call_function_single_ipi(int cpu)
>  {
> -	xen_send_IPI_mask(cpumask_of(cpu),
> +	__xen_send_IPI_mask(cpumask_of(cpu),
>  			  XEN_CALL_FUNCTION_SINGLE_VECTOR);
>  }
>  
> +static inline int xen_map_vector(int vector)
> +{
> +	int xen_vector;
> +
> +	switch (vector) {
> +	case RESCHEDULE_VECTOR:
> +		xen_vector = XEN_RESCHEDULE_VECTOR;
> +		break;
> +	case CALL_FUNCTION_VECTOR:
> +		xen_vector = XEN_CALL_FUNCTION_VECTOR;
> +		break;
> +	case CALL_FUNCTION_SINGLE_VECTOR:
> +		xen_vector = XEN_CALL_FUNCTION_SINGLE_VECTOR;
> +		break;
> +	default:
> +		xen_vector = -1;
> +		printk(KERN_ERR "xen: vector 0x%x is not implemented\n",
> +			vector);
> +	}
> +
> +	return xen_vector;
> +}
> +
> +void xen_send_IPI_mask(const struct cpumask *mask,
> +			      int vector)
> +{
> +	int xen_vector = xen_map_vector(vector);
> +
> +	if (xen_vector >= 0)
> +		__xen_send_IPI_mask(mask, xen_vector);
> +}
> +
> +void xen_send_IPI_all(int vector)
> +{
> +	int xen_vector = xen_map_vector(vector);
> +
> +	if (xen_vector >= 0)
> +		__xen_send_IPI_mask(cpu_online_mask, xen_vector);
> +}
> +
> +void xen_send_IPI_self(int vector)
> +{
> +	int xen_vector = xen_map_vector(vector);
> +
> +	if (xen_vector >= 0)
> +		xen_send_IPI_one(smp_processor_id(), xen_vector);
> +}
> +
> +void xen_send_IPI_mask_allbutself(const struct cpumask *mask,
> +				int vector)
> +{
> +	unsigned cpu;
> +	unsigned int this_cpu = smp_processor_id();
> +
> +	if (!(num_online_cpus() > 1))
> +		return;
> +
> +	for_each_cpu_and(cpu, mask, cpu_online_mask) {
> +		if (this_cpu == cpu)
> +			continue;
> +
> +		xen_smp_send_call_function_single_ipi(cpu);
> +	}
> +}
> +
> +void xen_send_IPI_allbutself(int vector)
> +{
> +	int xen_vector = xen_map_vector(vector);
> +
> +	if (xen_vector >= 0)
> +		xen_send_IPI_mask_allbutself(cpu_online_mask, xen_vector);
> +}
> +
>  static irqreturn_t xen_call_function_interrupt(int irq, void *dev_id)
>  {
>  	irq_enter();
> diff --git a/arch/x86/xen/smp.h b/arch/x86/xen/smp.h
> new file mode 100644
> index 0000000..8981a76
> --- /dev/null
> +++ b/arch/x86/xen/smp.h
> @@ -0,0 +1,12 @@
> +#ifndef _XEN_SMP_H
> +
> +extern void xen_send_IPI_mask(const struct cpumask *mask,
> +			      int vector);
> +extern void xen_send_IPI_mask_allbutself(const struct cpumask *mask,
> +				int vector);
> +extern void xen_send_IPI_allbutself(int vector);
> +extern void physflat_send_IPI_allbutself(int vector);
> +extern void xen_send_IPI_all(int vector);
> +extern void xen_send_IPI_self(int vector);
> +
> +#endif
> -- 
> 1.7.2.5

Re: [PATCH 1/2] xen: implement apic ipi interface

[Konrad Rzeszutek Wilk]

On Thu, Apr 19, 2012 at 03:02:17PM -0400, Konrad Rzeszutek Wilk wrote:
> On Sun, Apr 15, 2012 at 02:09:31PM +0800, Lin Ming wrote:
> > From: Ben Guthro <ben@guthro.net>
> > 
> > Map native ipi vector to xen vector.
> > Implement apic ipi interface with xen_send_IPI_one.
> 
> I keep on getting this (so PV guest without any SMP support,
> and with just PV frontend drivers):
> 
> 
> echo "CONFIG_PARAVIRT_GUEST=y" > linux.config
> echo "CONFIG_XEN=y" >> linux.config
> echo "CONFIG_XEN_XENBUS_FRONTEND=m" >> linux.config
> cat xen.config | grep FRONTEND >> linux.config
> echo "# CONFIG_XEN_PRIVILEGED_GUEST is not set" >> linux.config
> echo "# CONFIG_XEN_DOM0 is not set" >> linux.config
> echo "# CONFIG_ACPI is not set" >> linux.config
> echo "# CONFIG_PCI is not set" >> linux.config
> echo "# CONFIG_XENFS is not set" >> linux.config
> echo "# CONFIG_SMP is not set" >> linux.config
> 
> 
> make -j$((4 * 2)) -C /home/konrad/ssd/konrad/xtt-compile/linux O=/home/konrad/ssd/konrad/xtt-compile/linux-build- defconfig
> make[2]: Entering directory `/home/konrad/ssd/konrad/linux'
>   GEN     /home/konrad/ssd/konrad/xtt-compile/linux-build-/Makefile
> *** Default configuration is based on 'x86_64_defconfig'
> #
> # configuration written to .config
> 
> cat linux.config >> linux-build-/.config
> 
> make -j4 ..
> 
>   LD      init/built-in.o
>   LD      .tmp_vmlinux1
> arch/x86/built-in.o: In function `xen_start_kernel':
> (.init.text+0x5a8): undefined reference to `xen_send_IPI_allbutself'
> arch/x86/built-in.o: In function `xen_start_kernel':
> (.init.text+0x5b3): undefined reference to `xen_send_IPI_mask_allbutself'
> arch/x86/built-in.o: In function `xen_start_kernel':
> (.init.text+0x5be): undefined reference to `xen_send_IPI_mask'
> arch/x86/built-in.o: In function `xen_start_kernel':
> (.init.text+0x5c9): undefined reference to `xen_send_IPI_all'
> arch/x86/built-in.o: In function `xen_start_kernel':
> (.init.text+0x5d4): undefined reference to `xen_send_IPI_self'
> make[2]: *** [.tmp_vmlinux1] Error 1

And this fixes it [feel free to squash it in the patch]


commit dc4e2feaef1d22f8c4a257755af52a0b24e55a54
Author: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Date:   Thu Apr 19 15:09:08 2012 -0400

    xen/smp: Fix compile issues if no CONFIG_SMP
    
    Fixes:
    arch/x86/built-in.o: In function `xen_start_kernel':
    (.init.text+0x5a8): undefined reference to `xen_send_IPI_allbutself'
    arch/x86/built-in.o: In function `xen_start_kernel':
    (.init.text+0x5b3): undefined reference to `xen_send_IPI_mask_allbutself'
    arch/x86/built-in.o: In function `xen_start_kernel':
    (.init.text+0x5be): undefined reference to `xen_send_IPI_mask'
    arch/x86/built-in.o: In function `xen_start_kernel':
    (.init.text+0x5c9): undefined reference to `xen_send_IPI_all'
    arch/x86/built-in.o: In function `xen_start_kernel':
    (.init.text+0x5d4): undefined reference to `xen_send_IPI_self'
    
    Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index 20b3b23..9cf8f35 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -868,12 +868,13 @@ static void set_xen_basic_apic_ops(void)
 	apic->icr_write = xen_apic_icr_write;
 	apic->wait_icr_idle = xen_apic_wait_icr_idle;
 	apic->safe_wait_icr_idle = xen_safe_apic_wait_icr_idle;
-
+#ifdef CONFIG_SMP
 	apic->send_IPI_allbutself = xen_send_IPI_allbutself;
 	apic->send_IPI_mask_allbutself = xen_send_IPI_mask_allbutself;
 	apic->send_IPI_mask = xen_send_IPI_mask;
 	apic->send_IPI_all = xen_send_IPI_all;
 	apic->send_IPI_self = xen_send_IPI_self;
+#endif
 }
 
 #endif

Re: [Xen-devel] [PATCH 2/2] xen: implement IRQ_WORK_VECTOR handler

[Konrad Rzeszutek Wilk]

> >> +static irqreturn_t xen_irq_work_interrupt(int irq, void *dev_id)
> >> +{
> >> +     irq_enter();
> >> +     inc_irq_stat(apic_irq_work_irqs);
> >> +     irq_work_run();
> >
> > I think this usually done the other way around:
> >
> > irq_work_run()
> > inc_irq_stat(apic_irq_work_irqs)
> >
> > Or is there an excellent reason for doing it this way?
> 
> Copy & paste from smp_irq_work_interrupt().
> But I think there is no much difference.
> 
> Anyway, I can change it if needed.

Please do. It looks at odds with the other usages in this file.