2012-05-07 13:19:50

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] vsmp: Ignore IOAPIC IRQ affinity if possible


* Ido Yariv <[email protected]> wrote:

> From: Ravikiran Thirumalai <[email protected]>
>
> vSMP can route interrupts more optimally based on internal knowledge the
> OS does not have. In order to support this optimization, all CPUs must
> be able to handle all possible IOAPIC interrupts.
>
> Fix this by setting the vector allocation domain for all CPUs and by
> enabling this feature in vSMP.
>
> Signed-off-by: Ravikiran Thirumalai <[email protected]>
> Signed-off-by: Shai Fultheim <[email protected]>
> [[email protected]: rebased, simplified, and reworded the commit message]
> Signed-off-by: Ido Yariv <[email protected]>
> ---
> Changes from v1:
> - Updated Kiran's email address
>
> arch/x86/kernel/apic/probe_64.c | 10 ++++++++++
> arch/x86/kernel/vsmp_64.c | 15 +++++++++++----
> 2 files changed, 21 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/kernel/apic/probe_64.c b/arch/x86/kernel/apic/probe_64.c
> index 3fe9866..0f96703 100644
> --- a/arch/x86/kernel/apic/probe_64.c
> +++ b/arch/x86/kernel/apic/probe_64.c
> @@ -29,6 +29,15 @@ static int apicid_phys_pkg_id(int initial_apic_id, int index_msb)
> }
>
> /*
> + * In vSMP, all cpus should be capable of handling interrupts, regardless of
> + * the apic used.
> + */
> +static void fill_vector_allocation_domain(int cpu, struct cpumask *retmask)
> +{
> + cpumask_setall(retmask);
> +}
> +
> +/*
> * Check the APIC IDs in bios_cpu_apicid and choose the APIC mode.
> */
> void __init default_setup_apic_routing(void)
> @@ -51,6 +60,7 @@ void __init default_setup_apic_routing(void)
> if (is_vsmp_box()) {
> /* need to update phys_pkg_id */
> apic->phys_pkg_id = apicid_phys_pkg_id;
> + apic->vector_allocation_domain = fill_vector_allocation_domain;
> }
> }

This is_vsmp_box() special case should really move into its own
apic handler.

Thanks,

Ingo


Subject: RE: [PATCH v2 2/2] vsmp: Ignore IOAPIC IRQ affinity if possible

Ingo Molnar <[email protected]> wrote:
> * Ido Yariv <[email protected]> wrote:

> > @@ -51,6 +60,7 @@ void __init default_setup_apic_routing(void)
> > if (is_vsmp_box()) {
> > /* need to update phys_pkg_id */
> > apic->phys_pkg_id = apicid_phys_pkg_id;
> > + apic->vector_allocation_domain =
> fill_vector_allocation_domain;
> > }
> > }
>
> This is_vsmp_box() special case should really move into its own
> apic handler.
>
> Thanks,

Ingo,

vSMP Foundation virtualize multiple systems as one. The Guest OS is using the underlying hardware APIC driver, and therefore we must allow the various APIC drivers to kick in. When using vSMP Foundation APIC driver is selected by the kernel based on the hardware that is being virtualized, and that can be APIC, X2APIC phys, X2APIC cluster, summit, etc.

If we would to implement an APIC handler, we would not be able to use the native hardware APIC driver (unless we do an ugly hack).

The above optimization applies to all APIC drivers. It allow vSMP Foundation to route interrupts to the "closest" processor. For this we need to override the vector_allocation_domain of any APIC that selected (as noted, selection depends on real HW).

I hope this will allow you to ack our approach.

Regards,
Shai.

2012-05-09 09:18:33

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] vsmp: Ignore IOAPIC IRQ affinity if possible


* Shai Fultheim ([email protected]) <[email protected]> wrote:

> Ingo Molnar <[email protected]> wrote:
> > * Ido Yariv <[email protected]> wrote:
>
> > > @@ -51,6 +60,7 @@ void __init default_setup_apic_routing(void)
> > > if (is_vsmp_box()) {
> > > /* need to update phys_pkg_id */
> > > apic->phys_pkg_id = apicid_phys_pkg_id;
> > > + apic->vector_allocation_domain =
> > fill_vector_allocation_domain;
> > > }
> > > }
> >
> > This is_vsmp_box() special case should really move into its own
> > apic handler.
> >
> > Thanks,
>
> Ingo,
>
> vSMP Foundation virtualize multiple systems as one. The Guest
> OS is using the underlying hardware APIC driver, and therefore
> we must allow the various APIC drivers to kick in. When using
> vSMP Foundation APIC driver is selected by the kernel based on
> the hardware that is being virtualized, and that can be APIC,
> X2APIC phys, X2APIC cluster, summit, etc.
>
> If we would to implement an APIC handler, we would not be able
> to use the native hardware APIC driver (unless we do an ugly
> hack).

If you need a callback on that level then there's a cleaner
mechanism than open-coding vsmp handlers into generic code:

- create a new platform callback in x86_platform_ops
- call it from apic_64.c if set
- fill in the vSMP specific handler from the vSMP init code

Thanks,

Ingo

2012-05-09 21:21:31

by Ido Yariv

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] vsmp: Ignore IOAPIC IRQ affinity if possible

Hi Ingo,

On Wed, May 09, 2012 at 11:18:27AM +0200, Ingo Molnar wrote:
>
> * Shai Fultheim ([email protected]) <[email protected]> wrote:
>
> > Ingo Molnar <[email protected]> wrote:
> > > * Ido Yariv <[email protected]> wrote:
> >
> > > > @@ -51,6 +60,7 @@ void __init default_setup_apic_routing(void)
> > > > if (is_vsmp_box()) {
> > > > /* need to update phys_pkg_id */
> > > > apic->phys_pkg_id = apicid_phys_pkg_id;
> > > > + apic->vector_allocation_domain =
> > > fill_vector_allocation_domain;
> > > > }
> > > > }
> > >
> > > This is_vsmp_box() special case should really move into its own
> > > apic handler.
> > >
> > > Thanks,
> >
> > Ingo,
> >
> > vSMP Foundation virtualize multiple systems as one. The Guest
> > OS is using the underlying hardware APIC driver, and therefore
> > we must allow the various APIC drivers to kick in. When using
> > vSMP Foundation APIC driver is selected by the kernel based on
> > the hardware that is being virtualized, and that can be APIC,
> > X2APIC phys, X2APIC cluster, summit, etc.
> >
> > If we would to implement an APIC handler, we would not be able
> > to use the native hardware APIC driver (unless we do an ugly
> > hack).
>
> If you need a callback on that level then there's a cleaner
> mechanism than open-coding vsmp handlers into generic code:
>
> - create a new platform callback in x86_platform_ops
> - call it from apic_64.c if set
> - fill in the vSMP specific handler from the vSMP init code

Sounds good.

We'll look into it and post an alternative patch to this one.

Thanks,
Ido.

2012-06-02 22:11:57

by Ido Yariv

[permalink] [raw]
Subject: [PATCH 1/2] x86: Introduce apic post-initialization callback

Some architectures (such as vSMP) need to slightly adjust the underlying
apic structure. Add an apic post-initialization to x86_platform_ops for
this purpose and use it for adjusting the apic structure on vSMP
systems.

Signed-off-by: Ido Yariv <[email protected]>
Acked-by: Shai Fultheim <[email protected]>
---
arch/x86/include/asm/x86_init.h | 2 ++
arch/x86/kernel/apic/probe_32.c | 3 +++
arch/x86/kernel/apic/probe_64.c | 11 ++---------
arch/x86/kernel/vsmp_64.c | 13 +++++++++++++
4 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/arch/x86/include/asm/x86_init.h b/arch/x86/include/asm/x86_init.h
index c090af1..c377d9c 100644
--- a/arch/x86/include/asm/x86_init.h
+++ b/arch/x86/include/asm/x86_init.h
@@ -164,6 +164,7 @@ struct x86_cpuinit_ops {
* @i8042_detect pre-detect if i8042 controller exists
* @save_sched_clock_state: save state for sched_clock() on suspend
* @restore_sched_clock_state: restore state for sched_clock() on resume
+ * @apic_post_init: adjust apic if neeeded
*/
struct x86_platform_ops {
unsigned long (*calibrate_tsc)(void);
@@ -177,6 +178,7 @@ struct x86_platform_ops {
int (*i8042_detect)(void);
void (*save_sched_clock_state)(void);
void (*restore_sched_clock_state)(void);
+ void (*apic_post_init)(void);
};

struct pci_dev;
diff --git a/arch/x86/kernel/apic/probe_32.c b/arch/x86/kernel/apic/probe_32.c
index 1b291da..8616d51 100644
--- a/arch/x86/kernel/apic/probe_32.c
+++ b/arch/x86/kernel/apic/probe_32.c
@@ -208,6 +208,9 @@ void __init default_setup_apic_routing(void)

if (apic->setup_apic_routing)
apic->setup_apic_routing();
+
+ if (x86_platform.apic_post_init)
+ x86_platform.apic_post_init();
}

void __init generic_apic_probe(void)
diff --git a/arch/x86/kernel/apic/probe_64.c b/arch/x86/kernel/apic/probe_64.c
index 3fe9866..1793dba 100644
--- a/arch/x86/kernel/apic/probe_64.c
+++ b/arch/x86/kernel/apic/probe_64.c
@@ -23,11 +23,6 @@
#include <asm/ipi.h>
#include <asm/setup.h>

-static int apicid_phys_pkg_id(int initial_apic_id, int index_msb)
-{
- return hard_smp_processor_id() >> index_msb;
-}
-
/*
* Check the APIC IDs in bios_cpu_apicid and choose the APIC mode.
*/
@@ -48,10 +43,8 @@ void __init default_setup_apic_routing(void)
}
}

- if (is_vsmp_box()) {
- /* need to update phys_pkg_id */
- apic->phys_pkg_id = apicid_phys_pkg_id;
- }
+ if (x86_platform.apic_post_init)
+ x86_platform.apic_post_init();
}

/* Same for both flat and physical. */
diff --git a/arch/x86/kernel/vsmp_64.c b/arch/x86/kernel/vsmp_64.c
index 8eeb55a..59eea85 100644
--- a/arch/x86/kernel/vsmp_64.c
+++ b/arch/x86/kernel/vsmp_64.c
@@ -187,12 +187,25 @@ static void __init vsmp_cap_cpus(void)
#endif
}

+static int apicid_phys_pkg_id(int initial_apic_id, int index_msb)
+{
+ return hard_smp_processor_id() >> index_msb;
+}
+
+static void vsmp_apic_post_init(void)
+{
+ /* need to update phys_pkg_id */
+ apic->phys_pkg_id = apicid_phys_pkg_id;
+}
+
void __init vsmp_init(void)
{
detect_vsmp_box();
if (!is_vsmp_box())
return;

+ x86_platform.apic_post_init = vsmp_apic_post_init;
+
vsmp_cap_cpus();

set_vsmp_pv_ops();
--
1.7.7.6

2012-06-02 22:12:23

by Ido Yariv

[permalink] [raw]
Subject: [PATCH v3 2/2] vsmp: Ignore IOAPIC IRQ affinity if possible

From: Ravikiran Thirumalai <[email protected]>

vSMP can route interrupts more optimally based on internal knowledge the
OS does not have. In order to support this optimization, all CPUs must
be able to handle all possible IOAPIC interrupts.

Fix this by setting the vector allocation domain for all CPUs and by
enabling this feature in vSMP.

Signed-off-by: Ravikiran Thirumalai <[email protected]>
Signed-off-by: Shai Fultheim <[email protected]>
[[email protected]: rebased, simplified, and reworded the commit message]
Signed-off-by: Ido Yariv <[email protected]>
---
Changes from v2:
- Set the vector allocation domain in the apic_post_init callback

arch/x86/kernel/vsmp_64.c | 25 +++++++++++++++++++++----
1 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/vsmp_64.c b/arch/x86/kernel/vsmp_64.c
index 59eea85..a24cf34 100644
--- a/arch/x86/kernel/vsmp_64.c
+++ b/arch/x86/kernel/vsmp_64.c
@@ -16,6 +16,7 @@
#include <linux/pci_ids.h>
#include <linux/pci_regs.h>
#include <linux/smp.h>
+#include <linux/irq.h>

#include <asm/apic.h>
#include <asm/pci-direct.h>
@@ -95,6 +96,13 @@ static void __init set_vsmp_pv_ops(void)
ctl = readl(address + 4);
printk(KERN_INFO "vSMP CTL: capabilities:0x%08x control:0x%08x\n",
cap, ctl);
+
+ /* If possible, let the vSMP foundation route the interrupt optimally */
+ if (cap & ctl & BIT(8)) {
+ ctl &= ~BIT(8);
+ no_irq_affinity = 1;
+ }
+
if (cap & ctl & (1 << 4)) {
/* Setup irq ops and turn on vSMP IRQ fastpath handling */
pv_irq_ops.irq_disable = PV_CALLEE_SAVE(vsmp_irq_disable);
@@ -102,12 +110,11 @@ static void __init set_vsmp_pv_ops(void)
pv_irq_ops.save_fl = PV_CALLEE_SAVE(vsmp_save_fl);
pv_irq_ops.restore_fl = PV_CALLEE_SAVE(vsmp_restore_fl);
pv_init_ops.patch = vsmp_patch;
-
ctl &= ~(1 << 4);
- writel(ctl, address + 4);
- ctl = readl(address + 4);
- printk(KERN_INFO "vSMP CTL: control set to:0x%08x\n", ctl);
}
+ writel(ctl, address + 4);
+ ctl = readl(address + 4);
+ pr_info("vSMP CTL: control set to:0x%08x\n", ctl);

early_iounmap(address, 8);
}
@@ -192,10 +199,20 @@ static int apicid_phys_pkg_id(int initial_apic_id, int index_msb)
return hard_smp_processor_id() >> index_msb;
}

+/*
+ * In vSMP, all cpus should be capable of handling interrupts, regardless of
+ * the apic used.
+ */
+static void fill_vector_allocation_domain(int cpu, struct cpumask *retmask)
+{
+ cpumask_setall(retmask);
+}
+
static void vsmp_apic_post_init(void)
{
/* need to update phys_pkg_id */
apic->phys_pkg_id = apicid_phys_pkg_id;
+ apic->vector_allocation_domain = fill_vector_allocation_domain;
}

void __init vsmp_init(void)
--
1.7.7.6

2012-06-06 15:01:40

by Ido Yariv

[permalink] [raw]
Subject: [tip:x86/platform] x86/platform: Introduce APIC post-initialization callback

Commit-ID: 7db971b235480849aa5b9209b67b62e987b3181b
Gitweb: http://git.kernel.org/tip/7db971b235480849aa5b9209b67b62e987b3181b
Author: Ido Yariv <[email protected]>
AuthorDate: Sun, 3 Jun 2012 01:11:34 +0300
Committer: Ingo Molnar <[email protected]>
CommitDate: Wed, 6 Jun 2012 09:06:19 +0200

x86/platform: Introduce APIC post-initialization callback

Some subarchitectures (such as vSMP) need to slightly adjust the
underlying APIC structure. Add an APIC post-initialization callback
to 'struct x86_platform_ops' for this purpose and use it for
adjusting the APIC structure on vSMP systems.

Signed-off-by: Ido Yariv <[email protected]>
Acked-by: Shai Fultheim <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Ingo Molnar <[email protected]>
---
arch/x86/include/asm/x86_init.h | 2 ++
arch/x86/kernel/apic/probe_32.c | 3 +++
arch/x86/kernel/apic/probe_64.c | 11 ++---------
arch/x86/kernel/vsmp_64.c | 13 +++++++++++++
4 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/arch/x86/include/asm/x86_init.h b/arch/x86/include/asm/x86_init.h
index c090af1..c377d9c 100644
--- a/arch/x86/include/asm/x86_init.h
+++ b/arch/x86/include/asm/x86_init.h
@@ -164,6 +164,7 @@ struct x86_cpuinit_ops {
* @i8042_detect pre-detect if i8042 controller exists
* @save_sched_clock_state: save state for sched_clock() on suspend
* @restore_sched_clock_state: restore state for sched_clock() on resume
+ * @apic_post_init: adjust apic if neeeded
*/
struct x86_platform_ops {
unsigned long (*calibrate_tsc)(void);
@@ -177,6 +178,7 @@ struct x86_platform_ops {
int (*i8042_detect)(void);
void (*save_sched_clock_state)(void);
void (*restore_sched_clock_state)(void);
+ void (*apic_post_init)(void);
};

struct pci_dev;
diff --git a/arch/x86/kernel/apic/probe_32.c b/arch/x86/kernel/apic/probe_32.c
index 1b291da..8616d51 100644
--- a/arch/x86/kernel/apic/probe_32.c
+++ b/arch/x86/kernel/apic/probe_32.c
@@ -208,6 +208,9 @@ void __init default_setup_apic_routing(void)

if (apic->setup_apic_routing)
apic->setup_apic_routing();
+
+ if (x86_platform.apic_post_init)
+ x86_platform.apic_post_init();
}

void __init generic_apic_probe(void)
diff --git a/arch/x86/kernel/apic/probe_64.c b/arch/x86/kernel/apic/probe_64.c
index 3fe9866..1793dba 100644
--- a/arch/x86/kernel/apic/probe_64.c
+++ b/arch/x86/kernel/apic/probe_64.c
@@ -23,11 +23,6 @@
#include <asm/ipi.h>
#include <asm/setup.h>

-static int apicid_phys_pkg_id(int initial_apic_id, int index_msb)
-{
- return hard_smp_processor_id() >> index_msb;
-}
-
/*
* Check the APIC IDs in bios_cpu_apicid and choose the APIC mode.
*/
@@ -48,10 +43,8 @@ void __init default_setup_apic_routing(void)
}
}

- if (is_vsmp_box()) {
- /* need to update phys_pkg_id */
- apic->phys_pkg_id = apicid_phys_pkg_id;
- }
+ if (x86_platform.apic_post_init)
+ x86_platform.apic_post_init();
}

/* Same for both flat and physical. */
diff --git a/arch/x86/kernel/vsmp_64.c b/arch/x86/kernel/vsmp_64.c
index 8eeb55a..59eea85 100644
--- a/arch/x86/kernel/vsmp_64.c
+++ b/arch/x86/kernel/vsmp_64.c
@@ -187,12 +187,25 @@ static void __init vsmp_cap_cpus(void)
#endif
}

+static int apicid_phys_pkg_id(int initial_apic_id, int index_msb)
+{
+ return hard_smp_processor_id() >> index_msb;
+}
+
+static void vsmp_apic_post_init(void)
+{
+ /* need to update phys_pkg_id */
+ apic->phys_pkg_id = apicid_phys_pkg_id;
+}
+
void __init vsmp_init(void)
{
detect_vsmp_box();
if (!is_vsmp_box())
return;

+ x86_platform.apic_post_init = vsmp_apic_post_init;
+
vsmp_cap_cpus();

set_vsmp_pv_ops();

Subject: [tip:x86/platform] x86/vsmp: Ignore IOAPIC IRQ affinity if possible

Commit-ID: 3aabb53ce5849605cee731bbc32f37120b4c4ceb
Gitweb: http://git.kernel.org/tip/3aabb53ce5849605cee731bbc32f37120b4c4ceb
Author: Ravikiran Thirumalai <[email protected]>
AuthorDate: Sun, 3 Jun 2012 01:11:35 +0300
Committer: Ingo Molnar <[email protected]>
CommitDate: Wed, 6 Jun 2012 09:06:20 +0200

x86/vsmp: Ignore IOAPIC IRQ affinity if possible

vSMP can route interrupts more optimally based on internal
knowledge the OS does not have. In order to support this
optimization, all CPUs must be able to handle all possible
IOAPIC interrupts.

Fix this by setting the vector allocation domain for all CPUs
and by enabling this feature in vSMP.

Signed-off-by: Ravikiran Thirumalai <[email protected]>
Signed-off-by: Shai Fultheim <[email protected]>
[ rebased, simplified, and reworded the commit message ]
Signed-off-by: Ido Yariv <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Ingo Molnar <[email protected]>
---
arch/x86/kernel/vsmp_64.c | 25 +++++++++++++++++++++----
1 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/vsmp_64.c b/arch/x86/kernel/vsmp_64.c
index 59eea85..c72c0d3 100644
--- a/arch/x86/kernel/vsmp_64.c
+++ b/arch/x86/kernel/vsmp_64.c
@@ -16,6 +16,7 @@
#include <linux/pci_ids.h>
#include <linux/pci_regs.h>
#include <linux/smp.h>
+#include <linux/irq.h>

#include <asm/apic.h>
#include <asm/pci-direct.h>
@@ -95,6 +96,13 @@ static void __init set_vsmp_pv_ops(void)
ctl = readl(address + 4);
printk(KERN_INFO "vSMP CTL: capabilities:0x%08x control:0x%08x\n",
cap, ctl);
+
+ /* If possible, let the vSMP foundation route the interrupt optimally */
+ if (cap & ctl & BIT(8)) {
+ ctl &= ~BIT(8);
+ no_irq_affinity = 1;
+ }
+
if (cap & ctl & (1 << 4)) {
/* Setup irq ops and turn on vSMP IRQ fastpath handling */
pv_irq_ops.irq_disable = PV_CALLEE_SAVE(vsmp_irq_disable);
@@ -102,12 +110,11 @@ static void __init set_vsmp_pv_ops(void)
pv_irq_ops.save_fl = PV_CALLEE_SAVE(vsmp_save_fl);
pv_irq_ops.restore_fl = PV_CALLEE_SAVE(vsmp_restore_fl);
pv_init_ops.patch = vsmp_patch;
-
ctl &= ~(1 << 4);
- writel(ctl, address + 4);
- ctl = readl(address + 4);
- printk(KERN_INFO "vSMP CTL: control set to:0x%08x\n", ctl);
}
+ writel(ctl, address + 4);
+ ctl = readl(address + 4);
+ pr_info("vSMP CTL: control set to:0x%08x\n", ctl);

early_iounmap(address, 8);
}
@@ -192,10 +199,20 @@ static int apicid_phys_pkg_id(int initial_apic_id, int index_msb)
return hard_smp_processor_id() >> index_msb;
}

+/*
+ * In vSMP, all cpus should be capable of handling interrupts, regardless of
+ * the APIC used.
+ */
+static void fill_vector_allocation_domain(int cpu, struct cpumask *retmask)
+{
+ cpumask_setall(retmask);
+}
+
static void vsmp_apic_post_init(void)
{
/* need to update phys_pkg_id */
apic->phys_pkg_id = apicid_phys_pkg_id;
+ apic->vector_allocation_domain = fill_vector_allocation_domain;
}

void __init vsmp_init(void)

2012-06-08 08:52:39

by Ingo Molnar

[permalink] [raw]
Subject: Re: [tip:x86/platform] x86/vsmp: Ignore IOAPIC IRQ affinity if possible


* tip-bot for Ravikiran Thirumalai <[email protected]> wrote:

> Commit-ID: 3aabb53ce5849605cee731bbc32f37120b4c4ceb
> Gitweb: http://git.kernel.org/tip/3aabb53ce5849605cee731bbc32f37120b4c4ceb
> Author: Ravikiran Thirumalai <[email protected]>
> AuthorDate: Sun, 3 Jun 2012 01:11:35 +0300
> Committer: Ingo Molnar <[email protected]>
> CommitDate: Wed, 6 Jun 2012 09:06:20 +0200
>
> x86/vsmp: Ignore IOAPIC IRQ affinity if possible

This patch causes the following build failure:

(.init.text+0x10626): undefined reference to `no_irq_affinity'

on UP kernels.

Thanks,

Ingo

2012-06-08 15:43:14

by Ido Yariv

[permalink] [raw]
Subject: Re: [tip:x86/platform] x86/vsmp: Ignore IOAPIC IRQ affinity if possible

Hi Ingo,

On Fri, Jun 08, 2012 at 10:52:29AM +0200, Ingo Molnar wrote:
>
> * tip-bot for Ravikiran Thirumalai <[email protected]> wrote:
>
> > Commit-ID: 3aabb53ce5849605cee731bbc32f37120b4c4ceb
> > Gitweb: http://git.kernel.org/tip/3aabb53ce5849605cee731bbc32f37120b4c4ceb
> > Author: Ravikiran Thirumalai <[email protected]>
> > AuthorDate: Sun, 3 Jun 2012 01:11:35 +0300
> > Committer: Ingo Molnar <[email protected]>
> > CommitDate: Wed, 6 Jun 2012 09:06:20 +0200
> >
> > x86/vsmp: Ignore IOAPIC IRQ affinity if possible
>
> This patch causes the following build failure:
>
> (.init.text+0x10626): undefined reference to `no_irq_affinity'
>
> on UP kernels.

This seems to break when CONFIG_PARAVIRT is set but CONFIG_SMP isn't.
Since there's little point in optimizing IOAPIC routing for UP kernels,
how about the following fix?

Thanks,
Ido.

>From a67a9f5b0efa20c4da0fa85848d6c9e46f8f2665 Mon Sep 17 00:00:00 2001
From: Ravikiran Thirumalai <[email protected]>
Date: Sun, 3 Jun 2012 01:11:35 +0300
Subject: [PATCH] x86/vsmp: Ignore IOAPIC IRQ affinity if possible

vSMP can route interrupts more optimally based on internal
knowledge the OS does not have. In order to support this
optimization, all CPUs must be able to handle all possible
IOAPIC interrupts.

Fix this by setting the vector allocation domain for all CPUs
and by enabling this feature in vSMP.

Signed-off-by: Ravikiran Thirumalai <[email protected]>
Signed-off-by: Shai Fultheim <[email protected]>
[[email protected]: rebased, simplified, and reworded the commit message]
Signed-off-by: Ido Yariv <[email protected]>
---
arch/x86/kernel/vsmp_64.c | 27 +++++++++++++++++++++++----
1 file changed, 23 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/vsmp_64.c b/arch/x86/kernel/vsmp_64.c
index 59eea85..6b96a73 100644
--- a/arch/x86/kernel/vsmp_64.c
+++ b/arch/x86/kernel/vsmp_64.c
@@ -16,6 +16,7 @@
#include <linux/pci_ids.h>
#include <linux/pci_regs.h>
#include <linux/smp.h>
+#include <linux/irq.h>

#include <asm/apic.h>
#include <asm/pci-direct.h>
@@ -95,6 +96,15 @@ static void __init set_vsmp_pv_ops(void)
ctl = readl(address + 4);
printk(KERN_INFO "vSMP CTL: capabilities:0x%08x control:0x%08x\n",
cap, ctl);
+
+ /* If possible, let the vSMP foundation route the interrupt optimally */
+#ifdef CONFIG_SMP
+ if (cap & ctl & BIT(8)) {
+ ctl &= ~BIT(8);
+ no_irq_affinity = 1;
+ }
+#endif
+
if (cap & ctl & (1 << 4)) {
/* Setup irq ops and turn on vSMP IRQ fastpath handling */
pv_irq_ops.irq_disable = PV_CALLEE_SAVE(vsmp_irq_disable);
@@ -102,12 +112,11 @@ static void __init set_vsmp_pv_ops(void)
pv_irq_ops.save_fl = PV_CALLEE_SAVE(vsmp_save_fl);
pv_irq_ops.restore_fl = PV_CALLEE_SAVE(vsmp_restore_fl);
pv_init_ops.patch = vsmp_patch;
-
ctl &= ~(1 << 4);
- writel(ctl, address + 4);
- ctl = readl(address + 4);
- printk(KERN_INFO "vSMP CTL: control set to:0x%08x\n", ctl);
}
+ writel(ctl, address + 4);
+ ctl = readl(address + 4);
+ pr_info("vSMP CTL: control set to:0x%08x\n", ctl);

early_iounmap(address, 8);
}
@@ -192,10 +201,20 @@ static int apicid_phys_pkg_id(int initial_apic_id, int index_msb)
return hard_smp_processor_id() >> index_msb;
}

+/*
+ * In vSMP, all cpus should be capable of handling interrupts, regardless of
+ * the APIC used.
+ */
+static void fill_vector_allocation_domain(int cpu, struct cpumask *retmask)
+{
+ cpumask_setall(retmask);
+}
+
static void vsmp_apic_post_init(void)
{
/* need to update phys_pkg_id */
apic->phys_pkg_id = apicid_phys_pkg_id;
+ apic->vector_allocation_domain = fill_vector_allocation_domain;
}

void __init vsmp_init(void)
--
1.7.10.2

2012-06-11 08:58:50

by Ingo Molnar

[permalink] [raw]
Subject: Re: [tip:x86/platform] x86/vsmp: Ignore IOAPIC IRQ affinity if possible


* Ido Yariv <[email protected]> wrote:

> This seems to break when CONFIG_PARAVIRT is set but CONFIG_SMP
> isn't. Since there's little point in optimizing IOAPIC routing
> for UP kernels, how about the following fix?

Looks good to me, I'll give it some testing.

Thanks,

Ingo

2012-06-11 19:41:28

by Ido Yariv

[permalink] [raw]
Subject: Re: [tip:x86/platform] x86/vsmp: Ignore IOAPIC IRQ affinity if possible

Hi Ingo,

On Mon, Jun 11, 2012 at 10:58:43AM +0200, Ingo Molnar wrote:
>
> * Ido Yariv <[email protected]> wrote:
>
> > This seems to break when CONFIG_PARAVIRT is set but CONFIG_SMP
> > isn't. Since there's little point in optimizing IOAPIC routing
> > for UP kernels, how about the following fix?
>
> Looks good to me, I'll give it some testing.

I'm afraid this patch is missing one more config dependency. Since
no_irq_affinity is declared in proc.c, we need to verify that
CONFIG_PROC_FS is set before referencing this variable. The below patch
fixes this.

Thanks to Fengguang Wu for reporting this.

Thanks,
Ido.

>From 2a865725447a84a7e3921b01b3683b665e896971 Mon Sep 17 00:00:00 2001
From: Ravikiran Thirumalai <[email protected]>
Date: Sun, 3 Jun 2012 01:11:35 +0300
Subject: [PATCH] x86/vsmp: Ignore IOAPIC IRQ affinity if possible

vSMP can route interrupts more optimally based on internal
knowledge the OS does not have. In order to support this
optimization, all CPUs must be able to handle all possible
IOAPIC interrupts.

Fix this by setting the vector allocation domain for all CPUs
and by enabling this feature in vSMP.

Signed-off-by: Ravikiran Thirumalai <[email protected]>
Signed-off-by: Shai Fultheim <[email protected]>
[[email protected]: rebased, simplified, and reworded the commit message]
Signed-off-by: Ido Yariv <[email protected]>
---
arch/x86/kernel/vsmp_64.c | 30 ++++++++++++++++++++++++++----
1 file changed, 26 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/vsmp_64.c b/arch/x86/kernel/vsmp_64.c
index 59eea85..3f0285a 100644
--- a/arch/x86/kernel/vsmp_64.c
+++ b/arch/x86/kernel/vsmp_64.c
@@ -16,6 +16,7 @@
#include <linux/pci_ids.h>
#include <linux/pci_regs.h>
#include <linux/smp.h>
+#include <linux/irq.h>

#include <asm/apic.h>
#include <asm/pci-direct.h>
@@ -95,6 +96,18 @@ static void __init set_vsmp_pv_ops(void)
ctl = readl(address + 4);
printk(KERN_INFO "vSMP CTL: capabilities:0x%08x control:0x%08x\n",
cap, ctl);
+
+ /* If possible, let the vSMP foundation route the interrupt optimally */
+#ifdef CONFIG_SMP
+ if (cap & ctl & BIT(8)) {
+ ctl &= ~BIT(8);
+#ifdef CONFIG_PROC_FS
+ /* Don't let users change irq affinity via procfs */
+ no_irq_affinity = 1;
+#endif
+ }
+#endif
+
if (cap & ctl & (1 << 4)) {
/* Setup irq ops and turn on vSMP IRQ fastpath handling */
pv_irq_ops.irq_disable = PV_CALLEE_SAVE(vsmp_irq_disable);
@@ -102,12 +115,11 @@ static void __init set_vsmp_pv_ops(void)
pv_irq_ops.save_fl = PV_CALLEE_SAVE(vsmp_save_fl);
pv_irq_ops.restore_fl = PV_CALLEE_SAVE(vsmp_restore_fl);
pv_init_ops.patch = vsmp_patch;
-
ctl &= ~(1 << 4);
- writel(ctl, address + 4);
- ctl = readl(address + 4);
- printk(KERN_INFO "vSMP CTL: control set to:0x%08x\n", ctl);
}
+ writel(ctl, address + 4);
+ ctl = readl(address + 4);
+ pr_info("vSMP CTL: control set to:0x%08x\n", ctl);

early_iounmap(address, 8);
}
@@ -192,10 +204,20 @@ static int apicid_phys_pkg_id(int initial_apic_id, int index_msb)
return hard_smp_processor_id() >> index_msb;
}

+/*
+ * In vSMP, all cpus should be capable of handling interrupts, regardless of
+ * the APIC used.
+ */
+static void fill_vector_allocation_domain(int cpu, struct cpumask *retmask)
+{
+ cpumask_setall(retmask);
+}
+
static void vsmp_apic_post_init(void)
{
/* need to update phys_pkg_id */
apic->phys_pkg_id = apicid_phys_pkg_id;
+ apic->vector_allocation_domain = fill_vector_allocation_domain;
}

void __init vsmp_init(void)
--
1.7.10.2

2012-06-14 10:51:39

by Ingo Molnar

[permalink] [raw]
Subject: Re: [tip:x86/platform] x86/vsmp: Ignore IOAPIC IRQ affinity if possible


* Ido Yariv <[email protected]> wrote:

> Hi Ingo,
>
> On Mon, Jun 11, 2012 at 10:58:43AM +0200, Ingo Molnar wrote:
> >
> > * Ido Yariv <[email protected]> wrote:
> >
> > > This seems to break when CONFIG_PARAVIRT is set but CONFIG_SMP
> > > isn't. Since there's little point in optimizing IOAPIC routing
> > > for UP kernels, how about the following fix?
> >
> > Looks good to me, I'll give it some testing.
>
> I'm afraid this patch is missing one more config dependency. Since
> no_irq_affinity is declared in proc.c, we need to verify that
> CONFIG_PROC_FS is set before referencing this variable. The below patch
> fixes this.
>
> Thanks to Fengguang Wu for reporting this.

Please send a delta fix patch on top of latest -tip to fix this.

Thanks,

Ingo

2012-06-14 15:43:38

by Ido Yariv

[permalink] [raw]
Subject: [PATCH] x86/vsmp: Fix linker error when CONFIG_PROC_FS is not set

set_vsmp_pv_ops() references no_irq_affinity which is undeclared if
CONFIG_PROC_FS isn't set. Fix this by adding an ifdef around this
variable's access.

Reported-by: Fengguang Wu <[email protected]>
Signed-off-by: Ido Yariv <[email protected]>
Acked-by: Shai Fultheim <[email protected]>
---
arch/x86/kernel/vsmp_64.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/arch/x86/kernel/vsmp_64.c b/arch/x86/kernel/vsmp_64.c
index 6b96a73..3f0285a 100644
--- a/arch/x86/kernel/vsmp_64.c
+++ b/arch/x86/kernel/vsmp_64.c
@@ -101,7 +101,10 @@ static void __init set_vsmp_pv_ops(void)
#ifdef CONFIG_SMP
if (cap & ctl & BIT(8)) {
ctl &= ~BIT(8);
+#ifdef CONFIG_PROC_FS
+ /* Don't let users change irq affinity via procfs */
no_irq_affinity = 1;
+#endif
}
#endif

--
1.7.10.2

2012-06-14 17:34:18

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [tip:x86/platform] x86/platform: Introduce APIC post-initialization callback

On 06/06/2012 08:01 AM, tip-bot for Ido Yariv wrote:
> Some subarchitectures (such as vSMP)

Ido, could you please stop using this formula unless you actually have
evidence of anything other than vSMP having use for it? It chronically
comes across as dishonest.

-hpa

--
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel. I don't speak on their behalf.

2012-06-15 14:22:38

by Ido Yariv

[permalink] [raw]
Subject: [tip:x86/platform] x86/vsmp: Fix linker error when CONFIG_PROC_FS is not set

Commit-ID: d48daf37a3d2e2b28a61e615c0fc538301edb0dd
Gitweb: http://git.kernel.org/tip/d48daf37a3d2e2b28a61e615c0fc538301edb0dd
Author: Ido Yariv <[email protected]>
AuthorDate: Thu, 14 Jun 2012 18:43:08 +0300
Committer: Ingo Molnar <[email protected]>
CommitDate: Fri, 15 Jun 2012 13:54:11 +0200

x86/vsmp: Fix linker error when CONFIG_PROC_FS is not set

set_vsmp_pv_ops() references no_irq_affinity which is undeclared
if CONFIG_PROC_FS isn't set. Fix this by adding an #ifdef around
this variable's access.

Reported-by: Fengguang Wu <[email protected]>
Signed-off-by: Ido Yariv <[email protected]>
Acked-by: Shai Fultheim <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Ingo Molnar <[email protected]>
---
arch/x86/kernel/vsmp_64.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/vsmp_64.c b/arch/x86/kernel/vsmp_64.c
index 6b96a73..3f0285a 100644
--- a/arch/x86/kernel/vsmp_64.c
+++ b/arch/x86/kernel/vsmp_64.c
@@ -101,7 +101,10 @@ static void __init set_vsmp_pv_ops(void)
#ifdef CONFIG_SMP
if (cap & ctl & BIT(8)) {
ctl &= ~BIT(8);
+#ifdef CONFIG_PROC_FS
+ /* Don't let users change irq affinity via procfs */
no_irq_affinity = 1;
+#endif
}
#endif

2012-06-15 14:33:11

by Ido Yariv

[permalink] [raw]
Subject: Re: [tip:x86/platform] x86/platform: Introduce APIC post-initialization callback

Hi Peter,

On Thu, Jun 14, 2012 at 10:34:03AM -0700, H. Peter Anvin wrote:
> On 06/06/2012 08:01 AM, tip-bot for Ido Yariv wrote:
> > Some subarchitectures (such as vSMP)
>
> Ido, could you please stop using this formula unless you actually have
> evidence of anything other than vSMP having use for it? It chronically
> comes across as dishonest.

It was not my intention to disguise vSMP specific features as generic
ones, and I'm sorry if it seemed that way. I assumed other
virtualization architectures might benefit from these features, but at
least initially vSMP is the sole user.

Having said that, I'll be more specific in the future.

Thanks,
Ido.