2012-05-07 14:13:46

by Joerg Roedel

[permalink] [raw]
Subject: [git pull] irq remapping ops for x86

(again as a signed email)

Hi Ingo,

The following changes since commit d48b97b403d23f6df0b990cee652bdf9a52337a3:

Linux 3.4-rc6 (2012-05-06 15:07:32 -0700)

are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git intr-remapping-ops

or as a signed tag at:
git://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git intr-remapping-ops-for-ingo

Joerg Roedel (8):
iommu: Rename intr_remapping files to intel_intr_remapping
iommu/vt-d: Make intr-remapping initialization generic
iommu/vt-d: Convert missing apic.c intr-remapping call to remap_ops
iommu/vt-d: Convert IR ioapic-setup to use remap_ops
iommu/vt-d: Convert IR set_affinity function to remap_ops
iommu/vt-d: Convert free_irte into a remap_ops callback
iommu/vt-d: Convert MSI remapping setup to remap_ops
x86, iommu/vt-d: Clean up interfaces for interrupt remapping

Suresh Siddha (2):
iommu: rename intr_remapping references to irq_remapping
iommu: rename intr_remapping.[ch] to irq_remapping.[ch]

arch/ia64/include/asm/irq_remapping.h | 4 +
arch/x86/include/asm/irq_remapping.h | 120 +++++--
arch/x86/kernel/apic/apic.c | 30 +-
arch/x86/kernel/apic/io_apic.c | 297 ++++-------------
drivers/iommu/Makefile | 2 +-
drivers/iommu/dmar.c | 9 +-
drivers/iommu/intel-iommu.c | 3 +-
.../{intr_remapping.c => intel_irq_remapping.c} | 355 ++++++++++++++++----
drivers/iommu/intr_remapping.h | 17 -
drivers/iommu/irq_remapping.c | 164 +++++++++
drivers/iommu/irq_remapping.h | 88 +++++
include/linux/dmar.h | 85 -----
12 files changed, 724 insertions(+), 450 deletions(-)
create mode 100644 arch/ia64/include/asm/irq_remapping.h
rename drivers/iommu/{intr_remapping.c => intel_irq_remapping.c} (66%)
delete mode 100644 drivers/iommu/intr_remapping.h
create mode 100644 drivers/iommu/irq_remapping.c
create mode 100644 drivers/iommu/irq_remapping.h

This patchset introduces a generic ops-interface for
accessing interrupt remapping hardware on x86. It factors
out the VT-d specific code from io_apic.c and moves it to
drivers/iommu. These changes will be used to add support for
AMD interrupt remapping hardware.

Please pull.

Thanks,

Joerg


Attachments:
(No filename) (2.44 kB)
signature.asc (836.00 B)
Digital signature
Download all attachments

2012-05-08 03:52:12

by Ingo Molnar

[permalink] [raw]
Subject: Re: [git pull] irq remapping ops for x86


* Joerg Roedel <[email protected]> wrote:

> (again as a signed email)
>
> Hi Ingo,
>
> The following changes since commit d48b97b403d23f6df0b990cee652bdf9a52337a3:
>
> Linux 3.4-rc6 (2012-05-06 15:07:32 -0700)
>
> are available in the git repository at:
> git://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git intr-remapping-ops
>
> or as a signed tag at:
> git://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git intr-remapping-ops-for-ingo
>
> Joerg Roedel (8):
> iommu: Rename intr_remapping files to intel_intr_remapping
> iommu/vt-d: Make intr-remapping initialization generic
> iommu/vt-d: Convert missing apic.c intr-remapping call to remap_ops
> iommu/vt-d: Convert IR ioapic-setup to use remap_ops
> iommu/vt-d: Convert IR set_affinity function to remap_ops
> iommu/vt-d: Convert free_irte into a remap_ops callback
> iommu/vt-d: Convert MSI remapping setup to remap_ops
> x86, iommu/vt-d: Clean up interfaces for interrupt remapping
>
> Suresh Siddha (2):
> iommu: rename intr_remapping references to irq_remapping
> iommu: rename intr_remapping.[ch] to irq_remapping.[ch]
>
> arch/ia64/include/asm/irq_remapping.h | 4 +
> arch/x86/include/asm/irq_remapping.h | 120 +++++--
> arch/x86/kernel/apic/apic.c | 30 +-
> arch/x86/kernel/apic/io_apic.c | 297 ++++-------------
> drivers/iommu/Makefile | 2 +-
> drivers/iommu/dmar.c | 9 +-
> drivers/iommu/intel-iommu.c | 3 +-
> .../{intr_remapping.c => intel_irq_remapping.c} | 355 ++++++++++++++++----
> drivers/iommu/intr_remapping.h | 17 -
> drivers/iommu/irq_remapping.c | 164 +++++++++
> drivers/iommu/irq_remapping.h | 88 +++++
> include/linux/dmar.h | 85 -----
> 12 files changed, 724 insertions(+), 450 deletions(-)
> create mode 100644 arch/ia64/include/asm/irq_remapping.h
> rename drivers/iommu/{intr_remapping.c => intel_irq_remapping.c} (66%)
> delete mode 100644 drivers/iommu/intr_remapping.h
> create mode 100644 drivers/iommu/irq_remapping.c
> create mode 100644 drivers/iommu/irq_remapping.h
>
> This patchset introduces a generic ops-interface for
> accessing interrupt remapping hardware on x86. It factors
> out the VT-d specific code from io_apic.c and moves it to
> drivers/iommu. These changes will be used to add support for
> AMD interrupt remapping hardware.
>
> Please pull.

I've pulled it, but note that there's an UP build failure:

drivers/iommu/intel_irq_remapping.c:955:19: error: ‘struct irq_data’ has no member named ‘affinity’

config attached. This has to be fixed before I can push this
towards linux-next.

Thanks,

Ingo


Attachments:
(No filename) (2.83 kB)
config (74.04 kB)
Download all attachments

2012-05-08 07:05:39

by Suresh Siddha

[permalink] [raw]
Subject: [PATCH 1/3] irq_remap: fix compiler warning with !CONFIG_IRQ_REMAP

Fix the below compiler warning:
arch/x86/include/asm/irq_remapping.h:72:19: warning: ‘struct IO_APIC_route_entry’ declared inside parameter list [enabled by default]

Signed-off-by: Suresh Siddha <[email protected]>
Cc: Joerg Roedel <[email protected]>
---
arch/x86/include/asm/irq_remapping.h | 6 ++----
1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/irq_remapping.h b/arch/x86/include/asm/irq_remapping.h
index dcb0c72..5fb9bbb 100644
--- a/arch/x86/include/asm/irq_remapping.h
+++ b/arch/x86/include/asm/irq_remapping.h
@@ -22,11 +22,9 @@
#ifndef __X86_IRQ_REMAPPING_H
#define __X86_IRQ_REMAPPING_H

-#ifdef CONFIG_IRQ_REMAP
+#include <asm/io_apic.h>

-struct IO_APIC_route_entry;
-struct io_apic_irq_attr;
-struct pci_dev;
+#ifdef CONFIG_IRQ_REMAP

extern int irq_remapping_enabled;

--
1.7.6.5

2012-05-08 07:06:01

by Suresh Siddha

[permalink] [raw]
Subject: [PATCH 3/3] irq_remap: fix the 'sub_handle' uninitialized warning

Fix the uninitialized warning:
drivers/iommu/intel_irq_remapping.c:986:12: warning: ‘sub_handle’ may be used uninitialized in this function [-Wuninitialized]

Signed-off-by: Suresh Siddha <[email protected]>
Cc: Joerg Roedel <[email protected]>
---
drivers/iommu/intel_irq_remapping.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/iommu/intel_irq_remapping.c b/drivers/iommu/intel_irq_remapping.c
index 1c0255e..6d34706 100644
--- a/drivers/iommu/intel_irq_remapping.c
+++ b/drivers/iommu/intel_irq_remapping.c
@@ -964,7 +964,7 @@ static void intel_compose_msi_msg(struct pci_dev *pdev,
{
struct irq_cfg *cfg;
struct irte irte;
- u16 sub_handle;
+ u16 sub_handle = 0;
int ir_index;

cfg = irq_get_chip_data(irq);
--
1.7.6.5

2012-05-08 07:05:58

by Suresh Siddha

[permalink] [raw]
Subject: [PATCH 2/3] irq_remap: fix the UP build failure

Fix the below UP build failure with CONFIG_IRQ_REMAP enabled.

drivers/iommu/intel_irq_remapping.c:955:19: error: ‘struct irq_data’ has no member named ‘affinity’

Reported-by: Ingo Molnar <[email protected]>
Signed-off-by: Suresh Siddha <[email protected]>
Cc: Joerg Roedel <[email protected]>
---
drivers/iommu/intel_irq_remapping.c | 4 ++++
drivers/iommu/irq_remapping.c | 2 ++
drivers/iommu/irq_remapping.h | 2 ++
3 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/drivers/iommu/intel_irq_remapping.c b/drivers/iommu/intel_irq_remapping.c
index b4d3950..1c0255e 100644
--- a/drivers/iommu/intel_irq_remapping.c
+++ b/drivers/iommu/intel_irq_remapping.c
@@ -902,6 +902,7 @@ static int intel_setup_ioapic_entry(int irq,
return 0;
}

+#ifdef CONFIG_SMP
/*
* Migrate the IO-APIC irq in the presence of intr-remapping.
*
@@ -955,6 +956,7 @@ intel_ioapic_set_affinity(struct irq_data *data, const struct cpumask *mask,
cpumask_copy(data->affinity, mask);
return 0;
}
+#endif

static void intel_compose_msi_msg(struct pci_dev *pdev,
unsigned int irq, unsigned int dest,
@@ -1056,7 +1058,9 @@ struct irq_remap_ops intel_irq_remap_ops = {
.reenable = reenable_irq_remapping,
.enable_faulting = enable_drhd_fault_handling,
.setup_ioapic_entry = intel_setup_ioapic_entry,
+#ifdef CONFIG_SMP
.set_affinity = intel_ioapic_set_affinity,
+#endif
.free_irq = free_irte,
.compose_msi_msg = intel_compose_msi_msg,
.msi_alloc_irq = intel_msi_alloc_irq,
diff --git a/drivers/iommu/irq_remapping.c b/drivers/iommu/irq_remapping.c
index 1cf350e..40cda8e 100644
--- a/drivers/iommu/irq_remapping.c
+++ b/drivers/iommu/irq_remapping.c
@@ -111,6 +111,7 @@ int setup_ioapic_remapped_entry(int irq,
vector, attr);
}

+#ifdef CONFIG_SMP
int set_remapped_irq_affinity(struct irq_data *data, const struct cpumask *mask,
bool force)
{
@@ -119,6 +120,7 @@ int set_remapped_irq_affinity(struct irq_data *data, const struct cpumask *mask,

return remap_ops->set_affinity(data, mask, force);
}
+#endif

void free_remapped_irq(int irq)
{
diff --git a/drivers/iommu/irq_remapping.h b/drivers/iommu/irq_remapping.h
index b12974c..be9d729 100644
--- a/drivers/iommu/irq_remapping.h
+++ b/drivers/iommu/irq_remapping.h
@@ -59,9 +59,11 @@ struct irq_remap_ops {
unsigned int, int,
struct io_apic_irq_attr *);

+#ifdef CONFIG_SMP
/* Set the CPU affinity of a remapped interrupt */
int (*set_affinity)(struct irq_data *data, const struct cpumask *mask,
bool force);
+#endif

/* Free an IRQ */
int (*free_irq)(int);
--
1.7.6.5

2012-05-08 09:09:55

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 2/3] irq_remap: fix the UP build failure


* Suresh Siddha <[email protected]> wrote:

> Fix the below UP build failure with CONFIG_IRQ_REMAP enabled.
>
> drivers/iommu/intel_irq_remapping.c:955:19: error: ‘struct irq_data’ has no member named ‘affinity’

hm:

> +++ b/drivers/iommu/intel_irq_remapping.c
> +#ifdef CONFIG_SMP
> +#endif
> +#ifdef CONFIG_SMP
> +#endif
> +#ifdef CONFIG_SMP
> +#endif
> +#ifdef CONFIG_SMP
> +#endif

Adding this many #ifdefs is a bit sad. Could we not make the UP
side have the (supposedly zero length!) affinity cpumask
instead, or so, and make sure that the SMP functions compile to
something sensible on UP?

Thanks,

Ingo

2012-05-08 09:15:33

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 3/3] irq_remap: fix the 'sub_handle' uninitialized warning


another problem is that drivers/iommu/intel_irq_remapping.c is
supplied with comments rather poorly - I have to page down
almost 300 lines to see the first substantial comment! Copyright
notices would be nice as well.

So lets do a better job and document this thing a bit better.
Please also align structure definitions vertically, like we
typically to do in new code.

The file names should probably also be standardized (but that
should come via a Git space pull request, using git mv and
such). What we have currently is pretty random: 'dmar.c' should
probably be dma_remap.c, and *irq_remapping.c should probably be
*irq_remap.c.

Thanks,

Ingo

2012-05-08 10:01:58

by Suresh Siddha

[permalink] [raw]
Subject: [tip:core/iommu] irq_remap: Fix compiler warning with CONFIG_IRQ_REMAP=y

Commit-ID: 399988eea194a8453e283fdd2da968d1fd39a7cf
Gitweb: http://git.kernel.org/tip/399988eea194a8453e283fdd2da968d1fd39a7cf
Author: Suresh Siddha <[email protected]>
AuthorDate: Tue, 8 May 2012 00:08:52 -0700
Committer: Ingo Molnar <[email protected]>
CommitDate: Tue, 8 May 2012 11:17:29 +0200

irq_remap: Fix compiler warning with CONFIG_IRQ_REMAP=y

Fix the below compiler warning:

arch/x86/include/asm/irq_remapping.h:72:19: warning: ‘struct IO_APIC_route_entry’ declared inside parameter list [enabled by default]

Signed-off-by: Suresh Siddha <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: Joerg Roedel <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Ingo Molnar <[email protected]>
---
arch/x86/include/asm/irq_remapping.h | 6 ++----
1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/irq_remapping.h b/arch/x86/include/asm/irq_remapping.h
index dcb0c72..5fb9bbb 100644
--- a/arch/x86/include/asm/irq_remapping.h
+++ b/arch/x86/include/asm/irq_remapping.h
@@ -22,11 +22,9 @@
#ifndef __X86_IRQ_REMAPPING_H
#define __X86_IRQ_REMAPPING_H

-#ifdef CONFIG_IRQ_REMAP
+#include <asm/io_apic.h>

-struct IO_APIC_route_entry;
-struct io_apic_irq_attr;
-struct pci_dev;
+#ifdef CONFIG_IRQ_REMAP

extern int irq_remapping_enabled;

2012-05-08 10:02:49

by Suresh Siddha

[permalink] [raw]
Subject: [tip:core/iommu] irq_remap: Fix UP build failure

Commit-ID: 82b481e80d8a17720b5805393684a95184cfb6bb
Gitweb: http://git.kernel.org/tip/82b481e80d8a17720b5805393684a95184cfb6bb
Author: Suresh Siddha <[email protected]>
AuthorDate: Tue, 8 May 2012 00:08:53 -0700
Committer: Ingo Molnar <[email protected]>
CommitDate: Tue, 8 May 2012 11:17:30 +0200

irq_remap: Fix UP build failure

Fix the below UP build failure with CONFIG_IRQ_REMAP enabled.

drivers/iommu/intel_irq_remapping.c:955:19: error: ‘struct irq_data’ has no member named ‘affinity’

Reported-by: Ingo Molnar <[email protected]>
Signed-off-by: Suresh Siddha <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: Joerg Roedel <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Ingo Molnar <[email protected]>
---
drivers/iommu/intel_irq_remapping.c | 4 ++++
drivers/iommu/irq_remapping.c | 2 ++
drivers/iommu/irq_remapping.h | 2 ++
3 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/drivers/iommu/intel_irq_remapping.c b/drivers/iommu/intel_irq_remapping.c
index b4d3950..1c0255e 100644
--- a/drivers/iommu/intel_irq_remapping.c
+++ b/drivers/iommu/intel_irq_remapping.c
@@ -902,6 +902,7 @@ static int intel_setup_ioapic_entry(int irq,
return 0;
}

+#ifdef CONFIG_SMP
/*
* Migrate the IO-APIC irq in the presence of intr-remapping.
*
@@ -955,6 +956,7 @@ intel_ioapic_set_affinity(struct irq_data *data, const struct cpumask *mask,
cpumask_copy(data->affinity, mask);
return 0;
}
+#endif

static void intel_compose_msi_msg(struct pci_dev *pdev,
unsigned int irq, unsigned int dest,
@@ -1056,7 +1058,9 @@ struct irq_remap_ops intel_irq_remap_ops = {
.reenable = reenable_irq_remapping,
.enable_faulting = enable_drhd_fault_handling,
.setup_ioapic_entry = intel_setup_ioapic_entry,
+#ifdef CONFIG_SMP
.set_affinity = intel_ioapic_set_affinity,
+#endif
.free_irq = free_irte,
.compose_msi_msg = intel_compose_msi_msg,
.msi_alloc_irq = intel_msi_alloc_irq,
diff --git a/drivers/iommu/irq_remapping.c b/drivers/iommu/irq_remapping.c
index 1cf350e..40cda8e 100644
--- a/drivers/iommu/irq_remapping.c
+++ b/drivers/iommu/irq_remapping.c
@@ -111,6 +111,7 @@ int setup_ioapic_remapped_entry(int irq,
vector, attr);
}

+#ifdef CONFIG_SMP
int set_remapped_irq_affinity(struct irq_data *data, const struct cpumask *mask,
bool force)
{
@@ -119,6 +120,7 @@ int set_remapped_irq_affinity(struct irq_data *data, const struct cpumask *mask,

return remap_ops->set_affinity(data, mask, force);
}
+#endif

void free_remapped_irq(int irq)
{
diff --git a/drivers/iommu/irq_remapping.h b/drivers/iommu/irq_remapping.h
index b12974c..be9d729 100644
--- a/drivers/iommu/irq_remapping.h
+++ b/drivers/iommu/irq_remapping.h
@@ -59,9 +59,11 @@ struct irq_remap_ops {
unsigned int, int,
struct io_apic_irq_attr *);

+#ifdef CONFIG_SMP
/* Set the CPU affinity of a remapped interrupt */
int (*set_affinity)(struct irq_data *data, const struct cpumask *mask,
bool force);
+#endif

/* Free an IRQ */
int (*free_irq)(int);

2012-05-08 10:03:36

by Suresh Siddha

[permalink] [raw]
Subject: [tip:core/iommu] irq_remap: Fix the 'sub_handle' uninitialized warning

Commit-ID: c558df4a01a9ec7b02303aba808d1e5044822add
Gitweb: http://git.kernel.org/tip/c558df4a01a9ec7b02303aba808d1e5044822add
Author: Suresh Siddha <[email protected]>
AuthorDate: Tue, 8 May 2012 00:08:54 -0700
Committer: Ingo Molnar <[email protected]>
CommitDate: Tue, 8 May 2012 11:17:30 +0200

irq_remap: Fix the 'sub_handle' uninitialized warning

Fix this uninitialized variable warning:

drivers/iommu/intel_irq_remapping.c:986:12: warning: ‘sub_handle’ may be used uninitialized in this function [-Wuninitialized]

GCC is wrong, help it out.

Signed-off-by: Suresh Siddha <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: Joerg Roedel <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Ingo Molnar <[email protected]>
---
drivers/iommu/intel_irq_remapping.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/iommu/intel_irq_remapping.c b/drivers/iommu/intel_irq_remapping.c
index 1c0255e..6d34706 100644
--- a/drivers/iommu/intel_irq_remapping.c
+++ b/drivers/iommu/intel_irq_remapping.c
@@ -964,7 +964,7 @@ static void intel_compose_msi_msg(struct pci_dev *pdev,
{
struct irq_cfg *cfg;
struct irte irte;
- u16 sub_handle;
+ u16 sub_handle = 0;
int ir_index;

cfg = irq_get_chip_data(irq);

2012-05-09 02:33:47

by Suresh Siddha

[permalink] [raw]
Subject: Re: [PATCH 2/3] irq_remap: fix the UP build failure

On Tue, 2012-05-08 at 11:09 +0200, Ingo Molnar wrote:
> * Suresh Siddha <[email protected]> wrote:
>
> > Fix the below UP build failure with CONFIG_IRQ_REMAP enabled.
> >
> > drivers/iommu/intel_irq_remapping.c:955:19: error: ‘struct irq_data’ has no member named ‘affinity’
>
> hm:
>
> > +++ b/drivers/iommu/intel_irq_remapping.c
> > +#ifdef CONFIG_SMP
> > +#endif
> > +#ifdef CONFIG_SMP
> > +#endif
> > +#ifdef CONFIG_SMP
> > +#endif
> > +#ifdef CONFIG_SMP
> > +#endif
>
> Adding this many #ifdefs is a bit sad. Could we not make the UP
> side have the (supposedly zero length!) affinity cpumask
> instead, or so, and make sure that the SMP functions compile to
> something sensible on UP?
>

How about using config_enabled() to clean this up? Something like the
appended?

I first tried config_enabled(SMP) with out closely looking at the macro
definition and didn't work. I had to use config_enabled(CONFIG_SMP) to
really get this working. So in the appended patch I fixed config_enabled
macro to accept config_enabled(SMP). If this all sounds ok, then I can
split the appended patch into multiple patches.

thanks.
---
From: Suresh Siddha <[email protected]>
Subject: change config_enabled() and use it to cleanup #ifdef CONFIG_SMP

change config_enabled() to accept config_enabled(SMP) instead of
config_enabled(CONFIG_SMP).

Use this to cleanup the irq_chip smp affinity routines in io_apic and
irq_remapping subsystems.

Signed-off-by: Suresh Siddha <[email protected]>
---
arch/x86/kernel/apic/io_apic.c | 173 +++++++++++++++++------------------
drivers/iommu/intel_irq_remapping.c | 7 +-
drivers/iommu/irq_remapping.c | 5 +-
drivers/iommu/irq_remapping.h | 2 -
include/linux/irq.h | 2 -
include/linux/kconfig.h | 15 ++--
6 files changed, 97 insertions(+), 107 deletions(-)

diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index ffdc152..f8f3469 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -2221,71 +2221,6 @@ void send_cleanup_vector(struct irq_cfg *cfg)
cfg->move_in_progress = 0;
}

-static void __target_IO_APIC_irq(unsigned int irq, unsigned int dest, struct irq_cfg *cfg)
-{
- int apic, pin;
- struct irq_pin_list *entry;
- u8 vector = cfg->vector;
-
- for_each_irq_pin(entry, cfg->irq_2_pin) {
- unsigned int reg;
-
- apic = entry->apic;
- pin = entry->pin;
- /*
- * With interrupt-remapping, destination information comes
- * from interrupt-remapping table entry.
- */
- if (!irq_remapped(cfg))
- io_apic_write(apic, 0x11 + pin*2, dest);
- reg = io_apic_read(apic, 0x10 + pin*2);
- reg &= ~IO_APIC_REDIR_VECTOR_MASK;
- reg |= vector;
- io_apic_modify(apic, 0x10 + pin*2, reg);
- }
-}
-
-/*
- * Either sets data->affinity to a valid value, and returns
- * ->cpu_mask_to_apicid of that in dest_id, or returns -1 and
- * leaves data->affinity untouched.
- */
-int __ioapic_set_affinity(struct irq_data *data, const struct cpumask *mask,
- unsigned int *dest_id)
-{
- struct irq_cfg *cfg = data->chip_data;
-
- if (!cpumask_intersects(mask, cpu_online_mask))
- return -1;
-
- if (assign_irq_vector(data->irq, data->chip_data, mask))
- return -1;
-
- cpumask_copy(data->affinity, mask);
-
- *dest_id = apic->cpu_mask_to_apicid_and(mask, cfg->domain);
- return 0;
-}
-
-static int
-ioapic_set_affinity(struct irq_data *data, const struct cpumask *mask,
- bool force)
-{
- unsigned int dest, irq = data->irq;
- unsigned long flags;
- int ret;
-
- raw_spin_lock_irqsave(&ioapic_lock, flags);
- ret = __ioapic_set_affinity(data, mask, &dest);
- if (!ret) {
- /* Only the high 8 bits are valid. */
- dest = SET_APIC_LOGICAL_ID(dest);
- __target_IO_APIC_irq(irq, dest, data->chip_data);
- }
- raw_spin_unlock_irqrestore(&ioapic_lock, flags);
- return ret;
-}
-
asmlinkage void smp_irq_move_cleanup_interrupt(void)
{
unsigned vector, me;
@@ -2373,6 +2308,78 @@ void irq_force_complete_move(int irq)
static inline void irq_complete_move(struct irq_cfg *cfg) { }
#endif

+static void __target_IO_APIC_irq(unsigned int irq, unsigned int dest, struct irq_cfg *cfg)
+{
+ int apic, pin;
+ struct irq_pin_list *entry;
+ u8 vector = cfg->vector;
+
+ for_each_irq_pin(entry, cfg->irq_2_pin) {
+ unsigned int reg;
+
+ apic = entry->apic;
+ pin = entry->pin;
+ /*
+ * With interrupt-remapping, destination information comes
+ * from interrupt-remapping table entry.
+ */
+ if (!irq_remapped(cfg))
+ io_apic_write(apic, 0x11 + pin*2, dest);
+ reg = io_apic_read(apic, 0x10 + pin*2);
+ reg &= ~IO_APIC_REDIR_VECTOR_MASK;
+ reg |= vector;
+ io_apic_modify(apic, 0x10 + pin*2, reg);
+ }
+}
+
+/*
+ * Either sets data->affinity to a valid value, and returns
+ * ->cpu_mask_to_apicid of that in dest_id, or returns -1 and
+ * leaves data->affinity untouched.
+ */
+int __ioapic_set_affinity(struct irq_data *data, const struct cpumask *mask,
+ unsigned int *dest_id)
+{
+ struct irq_cfg *cfg = data->chip_data;
+
+ if (!config_enabled(SMP))
+ return -1;
+
+ if (!cpumask_intersects(mask, cpu_online_mask))
+ return -1;
+
+ if (assign_irq_vector(data->irq, data->chip_data, mask))
+ return -1;
+
+ cpumask_copy(data->affinity, mask);
+
+ *dest_id = apic->cpu_mask_to_apicid_and(mask, cfg->domain);
+ return 0;
+}
+
+static int
+ioapic_set_affinity(struct irq_data *data, const struct cpumask *mask,
+ bool force)
+{
+ unsigned int dest, irq = data->irq;
+ unsigned long flags;
+ int ret;
+
+ if (!config_enabled(SMP))
+ return -1;
+
+ raw_spin_lock_irqsave(&ioapic_lock, flags);
+ ret = __ioapic_set_affinity(data, mask, &dest);
+ if (!ret) {
+ /* Only the high 8 bits are valid. */
+ dest = SET_APIC_LOGICAL_ID(dest);
+ __target_IO_APIC_irq(irq, dest, data->chip_data);
+ }
+ raw_spin_unlock_irqrestore(&ioapic_lock, flags);
+ return ret;
+}
+
+
static void ack_apic_edge(struct irq_data *data)
{
irq_complete_move(data->chip_data);
@@ -2552,9 +2559,7 @@ static void irq_remap_modify_chip_defaults(struct irq_chip *chip)
chip->irq_ack = ir_ack_apic_edge;
chip->irq_eoi = ir_ack_apic_level;

-#ifdef CONFIG_SMP
chip->irq_set_affinity = set_remapped_irq_affinity;
-#endif
}
#endif /* CONFIG_IRQ_REMAP */

@@ -2565,9 +2570,7 @@ static struct irq_chip ioapic_chip __read_mostly = {
.irq_unmask = unmask_ioapic_irq,
.irq_ack = ack_apic_edge,
.irq_eoi = ack_apic_level,
-#ifdef CONFIG_SMP
.irq_set_affinity = ioapic_set_affinity,
-#endif
.irq_retrigger = ioapic_retrigger_irq,
};

@@ -3083,7 +3086,6 @@ static int msi_compose_msg(struct pci_dev *pdev, unsigned int irq,
return err;
}

-#ifdef CONFIG_SMP
static int
msi_set_affinity(struct irq_data *data, const struct cpumask *mask, bool force)
{
@@ -3091,6 +3093,9 @@ msi_set_affinity(struct irq_data *data, const struct cpumask *mask, bool force)
struct msi_msg msg;
unsigned int dest;

+ if (!config_enabled(SMP))
+ return -1;
+
if (__ioapic_set_affinity(data, mask, &dest))
return -1;

@@ -3105,7 +3110,6 @@ msi_set_affinity(struct irq_data *data, const struct cpumask *mask, bool force)

return 0;
}
-#endif /* CONFIG_SMP */

/*
* IRQ Chip for MSI PCI/PCI-X/PCI-Express Devices,
@@ -3116,9 +3120,7 @@ static struct irq_chip msi_chip = {
.irq_unmask = unmask_msi_irq,
.irq_mask = mask_msi_irq,
.irq_ack = ack_apic_edge,
-#ifdef CONFIG_SMP
.irq_set_affinity = msi_set_affinity,
-#endif
.irq_retrigger = ioapic_retrigger_irq,
};

@@ -3203,7 +3205,6 @@ void native_teardown_msi_irq(unsigned int irq)
}

#ifdef CONFIG_DMAR_TABLE
-#ifdef CONFIG_SMP
static int
dmar_msi_set_affinity(struct irq_data *data, const struct cpumask *mask,
bool force)
@@ -3212,6 +3213,9 @@ dmar_msi_set_affinity(struct irq_data *data, const struct cpumask *mask,
unsigned int dest, irq = data->irq;
struct msi_msg msg;

+ if (!config_enabled(SMP))
+ return -1;
+
if (__ioapic_set_affinity(data, mask, &dest))
return -1;

@@ -3228,16 +3232,12 @@ dmar_msi_set_affinity(struct irq_data *data, const struct cpumask *mask,
return 0;
}

-#endif /* CONFIG_SMP */
-
static struct irq_chip dmar_msi_type = {
.name = "DMAR_MSI",
.irq_unmask = dmar_msi_unmask,
.irq_mask = dmar_msi_mask,
.irq_ack = ack_apic_edge,
-#ifdef CONFIG_SMP
.irq_set_affinity = dmar_msi_set_affinity,
-#endif
.irq_retrigger = ioapic_retrigger_irq,
};

@@ -3258,7 +3258,6 @@ int arch_setup_dmar_msi(unsigned int irq)

#ifdef CONFIG_HPET_TIMER

-#ifdef CONFIG_SMP
static int hpet_msi_set_affinity(struct irq_data *data,
const struct cpumask *mask, bool force)
{
@@ -3266,6 +3265,9 @@ static int hpet_msi_set_affinity(struct irq_data *data,
struct msi_msg msg;
unsigned int dest;

+ if (!config_enabled(SMP))
+ return -1;
+
if (__ioapic_set_affinity(data, mask, &dest))
return -1;

@@ -3281,16 +3283,12 @@ static int hpet_msi_set_affinity(struct irq_data *data,
return 0;
}

-#endif /* CONFIG_SMP */
-
static struct irq_chip hpet_msi_type = {
.name = "HPET_MSI",
.irq_unmask = hpet_msi_unmask,
.irq_mask = hpet_msi_mask,
.irq_ack = ack_apic_edge,
-#ifdef CONFIG_SMP
.irq_set_affinity = hpet_msi_set_affinity,
-#endif
.irq_retrigger = ioapic_retrigger_irq,
};

@@ -3325,8 +3323,6 @@ int arch_setup_hpet_msi(unsigned int irq, unsigned int id)
*/
#ifdef CONFIG_HT_IRQ

-#ifdef CONFIG_SMP
-
static void target_ht_irq(unsigned int irq, unsigned int dest, u8 vector)
{
struct ht_irq_msg msg;
@@ -3347,6 +3343,9 @@ ht_set_affinity(struct irq_data *data, const struct cpumask *mask, bool force)
struct irq_cfg *cfg = data->chip_data;
unsigned int dest;

+ if (!config_enabled(SMP))
+ return -1;
+
if (__ioapic_set_affinity(data, mask, &dest))
return -1;

@@ -3354,16 +3353,12 @@ ht_set_affinity(struct irq_data *data, const struct cpumask *mask, bool force)
return 0;
}

-#endif
-
static struct irq_chip ht_irq_chip = {
.name = "PCI-HT",
.irq_mask = mask_ht_irq,
.irq_unmask = unmask_ht_irq,
.irq_ack = ack_apic_edge,
-#ifdef CONFIG_SMP
.irq_set_affinity = ht_set_affinity,
-#endif
.irq_retrigger = ioapic_retrigger_irq,
};

diff --git a/drivers/iommu/intel_irq_remapping.c b/drivers/iommu/intel_irq_remapping.c
index 6d34706..b3481cf 100644
--- a/drivers/iommu/intel_irq_remapping.c
+++ b/drivers/iommu/intel_irq_remapping.c
@@ -902,7 +902,6 @@ static int intel_setup_ioapic_entry(int irq,
return 0;
}

-#ifdef CONFIG_SMP
/*
* Migrate the IO-APIC irq in the presence of intr-remapping.
*
@@ -925,6 +924,9 @@ intel_ioapic_set_affinity(struct irq_data *data, const struct cpumask *mask,
unsigned int dest, irq = data->irq;
struct irte irte;

+ if (!config_enabled(CONFIG_SMP))
+ return -EINVAL;
+
if (!cpumask_intersects(mask, cpu_online_mask))
return -EINVAL;

@@ -956,7 +958,6 @@ intel_ioapic_set_affinity(struct irq_data *data, const struct cpumask *mask,
cpumask_copy(data->affinity, mask);
return 0;
}
-#endif

static void intel_compose_msi_msg(struct pci_dev *pdev,
unsigned int irq, unsigned int dest,
@@ -1058,9 +1059,7 @@ struct irq_remap_ops intel_irq_remap_ops = {
.reenable = reenable_irq_remapping,
.enable_faulting = enable_drhd_fault_handling,
.setup_ioapic_entry = intel_setup_ioapic_entry,
-#ifdef CONFIG_SMP
.set_affinity = intel_ioapic_set_affinity,
-#endif
.free_irq = free_irte,
.compose_msi_msg = intel_compose_msi_msg,
.msi_alloc_irq = intel_msi_alloc_irq,
diff --git a/drivers/iommu/irq_remapping.c b/drivers/iommu/irq_remapping.c
index 40cda8e..1d29b1c 100644
--- a/drivers/iommu/irq_remapping.c
+++ b/drivers/iommu/irq_remapping.c
@@ -111,16 +111,15 @@ int setup_ioapic_remapped_entry(int irq,
vector, attr);
}

-#ifdef CONFIG_SMP
int set_remapped_irq_affinity(struct irq_data *data, const struct cpumask *mask,
bool force)
{
- if (!remap_ops || !remap_ops->set_affinity)
+ if (!config_enabled(CONFIG_SMP) || !remap_ops ||
+ !remap_ops->set_affinity)
return 0;

return remap_ops->set_affinity(data, mask, force);
}
-#endif

void free_remapped_irq(int irq)
{
diff --git a/drivers/iommu/irq_remapping.h b/drivers/iommu/irq_remapping.h
index be9d729..b12974c 100644
--- a/drivers/iommu/irq_remapping.h
+++ b/drivers/iommu/irq_remapping.h
@@ -59,11 +59,9 @@ struct irq_remap_ops {
unsigned int, int,
struct io_apic_irq_attr *);

-#ifdef CONFIG_SMP
/* Set the CPU affinity of a remapped interrupt */
int (*set_affinity)(struct irq_data *data, const struct cpumask *mask,
bool force);
-#endif

/* Free an IRQ */
int (*free_irq)(int);
diff --git a/include/linux/irq.h b/include/linux/irq.h
index b27cfcf..008d4ec 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -150,9 +150,7 @@ struct irq_data {
void *handler_data;
void *chip_data;
struct msi_desc *msi_desc;
-#ifdef CONFIG_SMP
cpumask_var_t affinity;
-#endif
};

/*
diff --git a/include/linux/kconfig.h b/include/linux/kconfig.h
index be342b9..54c1c4e 100644
--- a/include/linux/kconfig.h
+++ b/include/linux/kconfig.h
@@ -17,10 +17,11 @@
* the last step cherry picks the 2nd arg, we get a zero.
*/
#define __ARG_PLACEHOLDER_1 0,
-#define config_enabled(cfg) _config_enabled(cfg)
-#define _config_enabled(value) __config_enabled(__ARG_PLACEHOLDER_##value)
-#define __config_enabled(arg1_or_junk) ___config_enabled(arg1_or_junk 1, 0)
-#define ___config_enabled(__ignored, val, ...) val
+#define config_enabled(cfg) _config_enabled(CONFIG_##cfg)
+#define _config_enabled(cfg) __config_enabled(cfg)
+#define __config_enabled(value) ___config_enabled(__ARG_PLACEHOLDER_##value)
+#define ___config_enabled(arg1_or_junk) ____config_enabled(arg1_or_junk 1, 0)
+#define ____config_enabled(__ignored, val, ...) val

/*
* IS_ENABLED(CONFIG_FOO) evaluates to 1 if CONFIG_FOO is set to 'y' or 'm',
@@ -28,19 +29,19 @@
*
*/
#define IS_ENABLED(option) \
- (config_enabled(option) || config_enabled(option##_MODULE))
+ (_config_enabled(option) || _config_enabled(option##_MODULE))

/*
* IS_BUILTIN(CONFIG_FOO) evaluates to 1 if CONFIG_FOO is set to 'y', 0
* otherwise. For boolean options, this is equivalent to
* IS_ENABLED(CONFIG_FOO).
*/
-#define IS_BUILTIN(option) config_enabled(option)
+#define IS_BUILTIN(option) _config_enabled(option)

/*
* IS_MODULE(CONFIG_FOO) evaluates to 1 if CONFIG_FOO is set to 'm', 0
* otherwise.
*/
-#define IS_MODULE(option) config_enabled(option##_MODULE)
+#define IS_MODULE(option) _config_enabled(option##_MODULE)

#endif /* __LINUX_KCONFIG_H */

2012-05-09 09:30:09

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 2/3] irq_remap: fix the UP build failure


* Suresh Siddha <[email protected]> wrote:

> On Tue, 2012-05-08 at 11:09 +0200, Ingo Molnar wrote:
> > * Suresh Siddha <[email protected]> wrote:
> >
> > > Fix the below UP build failure with CONFIG_IRQ_REMAP enabled.
> > >
> > > drivers/iommu/intel_irq_remapping.c:955:19: error: ‘struct irq_data’ has no member named ‘affinity’
> >
> > hm:
> >
> > > +++ b/drivers/iommu/intel_irq_remapping.c
> > > +#ifdef CONFIG_SMP
> > > +#endif
> > > +#ifdef CONFIG_SMP
> > > +#endif
> > > +#ifdef CONFIG_SMP
> > > +#endif
> > > +#ifdef CONFIG_SMP
> > > +#endif
> >
> > Adding this many #ifdefs is a bit sad. Could we not make the UP
> > side have the (supposedly zero length!) affinity cpumask
> > instead, or so, and make sure that the SMP functions compile to
> > something sensible on UP?
> >
>
> How about using config_enabled() to clean this up? Something
> like the appended?
>
> I first tried config_enabled(SMP) with out closely looking at
> the macro definition and didn't work. I had to use
> config_enabled(CONFIG_SMP) to really get this working. So in
> the appended patch I fixed config_enabled macro to accept
> config_enabled(SMP). If this all sounds ok, then I can split
> the appended patch into multiple patches.

Looks a lot cleaner!

Please split out the config_enabled() change into a separate
patch and Cc: Linus on the resend.

I like your idea of allowing config_enabled(SMP) as well,
there's no reason to say 'config' twice. It's too easy to get
this wrong and AFAICS there's no build error if we are using a
non-existent config flag, right?

Thanks,

Ingo

2012-05-09 18:43:30

by Suresh Siddha

[permalink] [raw]
Subject: [PATCH 1/2] kconfig: change config_enabled() to accept X instead of CONFIG_X

change config_enabled() to use it as config_enabled(SMP) instead of
config_enabled(CONFIG_SMP).

Signed-off-by: Suresh Siddha <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Paul Gortmaker <[email protected]>
---
include/linux/kconfig.h | 15 ++++++++-------
1 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/include/linux/kconfig.h b/include/linux/kconfig.h
index be342b9..54c1c4e 100644
--- a/include/linux/kconfig.h
+++ b/include/linux/kconfig.h
@@ -17,10 +17,11 @@
* the last step cherry picks the 2nd arg, we get a zero.
*/
#define __ARG_PLACEHOLDER_1 0,
-#define config_enabled(cfg) _config_enabled(cfg)
-#define _config_enabled(value) __config_enabled(__ARG_PLACEHOLDER_##value)
-#define __config_enabled(arg1_or_junk) ___config_enabled(arg1_or_junk 1, 0)
-#define ___config_enabled(__ignored, val, ...) val
+#define config_enabled(cfg) _config_enabled(CONFIG_##cfg)
+#define _config_enabled(cfg) __config_enabled(cfg)
+#define __config_enabled(value) ___config_enabled(__ARG_PLACEHOLDER_##value)
+#define ___config_enabled(arg1_or_junk) ____config_enabled(arg1_or_junk 1, 0)
+#define ____config_enabled(__ignored, val, ...) val

/*
* IS_ENABLED(CONFIG_FOO) evaluates to 1 if CONFIG_FOO is set to 'y' or 'm',
@@ -28,19 +29,19 @@
*
*/
#define IS_ENABLED(option) \
- (config_enabled(option) || config_enabled(option##_MODULE))
+ (_config_enabled(option) || _config_enabled(option##_MODULE))

/*
* IS_BUILTIN(CONFIG_FOO) evaluates to 1 if CONFIG_FOO is set to 'y', 0
* otherwise. For boolean options, this is equivalent to
* IS_ENABLED(CONFIG_FOO).
*/
-#define IS_BUILTIN(option) config_enabled(option)
+#define IS_BUILTIN(option) _config_enabled(option)

/*
* IS_MODULE(CONFIG_FOO) evaluates to 1 if CONFIG_FOO is set to 'm', 0
* otherwise.
*/
-#define IS_MODULE(option) config_enabled(option##_MODULE)
+#define IS_MODULE(option) _config_enabled(option##_MODULE)

#endif /* __LINUX_KCONFIG_H */
--
1.7.6.5

2012-05-09 18:43:34

by Suresh Siddha

[permalink] [raw]
Subject: [PATCH 2/2] irq: use config_enabled(SMP) checks to cleanup irq_set_affinity() for UP

Use config_enabled(SMP) checks for cleaning up the ifdef CONFIG_SMP around
irq_set_affinity routines in io_apic and irq_remapping subsystems.

Signed-off-by: Suresh Siddha <[email protected]>
---
arch/x86/kernel/apic/io_apic.c | 173 +++++++++++++++++------------------
drivers/iommu/intel_irq_remapping.c | 7 +-
drivers/iommu/irq_remapping.c | 5 +-
drivers/iommu/irq_remapping.h | 2 -
include/linux/irq.h | 2 -
5 files changed, 89 insertions(+), 100 deletions(-)

diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index ffdc152..f8f3469 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -2221,71 +2221,6 @@ void send_cleanup_vector(struct irq_cfg *cfg)
cfg->move_in_progress = 0;
}

-static void __target_IO_APIC_irq(unsigned int irq, unsigned int dest, struct irq_cfg *cfg)
-{
- int apic, pin;
- struct irq_pin_list *entry;
- u8 vector = cfg->vector;
-
- for_each_irq_pin(entry, cfg->irq_2_pin) {
- unsigned int reg;
-
- apic = entry->apic;
- pin = entry->pin;
- /*
- * With interrupt-remapping, destination information comes
- * from interrupt-remapping table entry.
- */
- if (!irq_remapped(cfg))
- io_apic_write(apic, 0x11 + pin*2, dest);
- reg = io_apic_read(apic, 0x10 + pin*2);
- reg &= ~IO_APIC_REDIR_VECTOR_MASK;
- reg |= vector;
- io_apic_modify(apic, 0x10 + pin*2, reg);
- }
-}
-
-/*
- * Either sets data->affinity to a valid value, and returns
- * ->cpu_mask_to_apicid of that in dest_id, or returns -1 and
- * leaves data->affinity untouched.
- */
-int __ioapic_set_affinity(struct irq_data *data, const struct cpumask *mask,
- unsigned int *dest_id)
-{
- struct irq_cfg *cfg = data->chip_data;
-
- if (!cpumask_intersects(mask, cpu_online_mask))
- return -1;
-
- if (assign_irq_vector(data->irq, data->chip_data, mask))
- return -1;
-
- cpumask_copy(data->affinity, mask);
-
- *dest_id = apic->cpu_mask_to_apicid_and(mask, cfg->domain);
- return 0;
-}
-
-static int
-ioapic_set_affinity(struct irq_data *data, const struct cpumask *mask,
- bool force)
-{
- unsigned int dest, irq = data->irq;
- unsigned long flags;
- int ret;
-
- raw_spin_lock_irqsave(&ioapic_lock, flags);
- ret = __ioapic_set_affinity(data, mask, &dest);
- if (!ret) {
- /* Only the high 8 bits are valid. */
- dest = SET_APIC_LOGICAL_ID(dest);
- __target_IO_APIC_irq(irq, dest, data->chip_data);
- }
- raw_spin_unlock_irqrestore(&ioapic_lock, flags);
- return ret;
-}
-
asmlinkage void smp_irq_move_cleanup_interrupt(void)
{
unsigned vector, me;
@@ -2373,6 +2308,78 @@ void irq_force_complete_move(int irq)
static inline void irq_complete_move(struct irq_cfg *cfg) { }
#endif

+static void __target_IO_APIC_irq(unsigned int irq, unsigned int dest, struct irq_cfg *cfg)
+{
+ int apic, pin;
+ struct irq_pin_list *entry;
+ u8 vector = cfg->vector;
+
+ for_each_irq_pin(entry, cfg->irq_2_pin) {
+ unsigned int reg;
+
+ apic = entry->apic;
+ pin = entry->pin;
+ /*
+ * With interrupt-remapping, destination information comes
+ * from interrupt-remapping table entry.
+ */
+ if (!irq_remapped(cfg))
+ io_apic_write(apic, 0x11 + pin*2, dest);
+ reg = io_apic_read(apic, 0x10 + pin*2);
+ reg &= ~IO_APIC_REDIR_VECTOR_MASK;
+ reg |= vector;
+ io_apic_modify(apic, 0x10 + pin*2, reg);
+ }
+}
+
+/*
+ * Either sets data->affinity to a valid value, and returns
+ * ->cpu_mask_to_apicid of that in dest_id, or returns -1 and
+ * leaves data->affinity untouched.
+ */
+int __ioapic_set_affinity(struct irq_data *data, const struct cpumask *mask,
+ unsigned int *dest_id)
+{
+ struct irq_cfg *cfg = data->chip_data;
+
+ if (!config_enabled(SMP))
+ return -1;
+
+ if (!cpumask_intersects(mask, cpu_online_mask))
+ return -1;
+
+ if (assign_irq_vector(data->irq, data->chip_data, mask))
+ return -1;
+
+ cpumask_copy(data->affinity, mask);
+
+ *dest_id = apic->cpu_mask_to_apicid_and(mask, cfg->domain);
+ return 0;
+}
+
+static int
+ioapic_set_affinity(struct irq_data *data, const struct cpumask *mask,
+ bool force)
+{
+ unsigned int dest, irq = data->irq;
+ unsigned long flags;
+ int ret;
+
+ if (!config_enabled(SMP))
+ return -1;
+
+ raw_spin_lock_irqsave(&ioapic_lock, flags);
+ ret = __ioapic_set_affinity(data, mask, &dest);
+ if (!ret) {
+ /* Only the high 8 bits are valid. */
+ dest = SET_APIC_LOGICAL_ID(dest);
+ __target_IO_APIC_irq(irq, dest, data->chip_data);
+ }
+ raw_spin_unlock_irqrestore(&ioapic_lock, flags);
+ return ret;
+}
+
+
static void ack_apic_edge(struct irq_data *data)
{
irq_complete_move(data->chip_data);
@@ -2552,9 +2559,7 @@ static void irq_remap_modify_chip_defaults(struct irq_chip *chip)
chip->irq_ack = ir_ack_apic_edge;
chip->irq_eoi = ir_ack_apic_level;

-#ifdef CONFIG_SMP
chip->irq_set_affinity = set_remapped_irq_affinity;
-#endif
}
#endif /* CONFIG_IRQ_REMAP */

@@ -2565,9 +2570,7 @@ static struct irq_chip ioapic_chip __read_mostly = {
.irq_unmask = unmask_ioapic_irq,
.irq_ack = ack_apic_edge,
.irq_eoi = ack_apic_level,
-#ifdef CONFIG_SMP
.irq_set_affinity = ioapic_set_affinity,
-#endif
.irq_retrigger = ioapic_retrigger_irq,
};

@@ -3083,7 +3086,6 @@ static int msi_compose_msg(struct pci_dev *pdev, unsigned int irq,
return err;
}

-#ifdef CONFIG_SMP
static int
msi_set_affinity(struct irq_data *data, const struct cpumask *mask, bool force)
{
@@ -3091,6 +3093,9 @@ msi_set_affinity(struct irq_data *data, const struct cpumask *mask, bool force)
struct msi_msg msg;
unsigned int dest;

+ if (!config_enabled(SMP))
+ return -1;
+
if (__ioapic_set_affinity(data, mask, &dest))
return -1;

@@ -3105,7 +3110,6 @@ msi_set_affinity(struct irq_data *data, const struct cpumask *mask, bool force)

return 0;
}
-#endif /* CONFIG_SMP */

/*
* IRQ Chip for MSI PCI/PCI-X/PCI-Express Devices,
@@ -3116,9 +3120,7 @@ static struct irq_chip msi_chip = {
.irq_unmask = unmask_msi_irq,
.irq_mask = mask_msi_irq,
.irq_ack = ack_apic_edge,
-#ifdef CONFIG_SMP
.irq_set_affinity = msi_set_affinity,
-#endif
.irq_retrigger = ioapic_retrigger_irq,
};

@@ -3203,7 +3205,6 @@ void native_teardown_msi_irq(unsigned int irq)
}

#ifdef CONFIG_DMAR_TABLE
-#ifdef CONFIG_SMP
static int
dmar_msi_set_affinity(struct irq_data *data, const struct cpumask *mask,
bool force)
@@ -3212,6 +3213,9 @@ dmar_msi_set_affinity(struct irq_data *data, const struct cpumask *mask,
unsigned int dest, irq = data->irq;
struct msi_msg msg;

+ if (!config_enabled(SMP))
+ return -1;
+
if (__ioapic_set_affinity(data, mask, &dest))
return -1;

@@ -3228,16 +3232,12 @@ dmar_msi_set_affinity(struct irq_data *data, const struct cpumask *mask,
return 0;
}

-#endif /* CONFIG_SMP */
-
static struct irq_chip dmar_msi_type = {
.name = "DMAR_MSI",
.irq_unmask = dmar_msi_unmask,
.irq_mask = dmar_msi_mask,
.irq_ack = ack_apic_edge,
-#ifdef CONFIG_SMP
.irq_set_affinity = dmar_msi_set_affinity,
-#endif
.irq_retrigger = ioapic_retrigger_irq,
};

@@ -3258,7 +3258,6 @@ int arch_setup_dmar_msi(unsigned int irq)

#ifdef CONFIG_HPET_TIMER

-#ifdef CONFIG_SMP
static int hpet_msi_set_affinity(struct irq_data *data,
const struct cpumask *mask, bool force)
{
@@ -3266,6 +3265,9 @@ static int hpet_msi_set_affinity(struct irq_data *data,
struct msi_msg msg;
unsigned int dest;

+ if (!config_enabled(SMP))
+ return -1;
+
if (__ioapic_set_affinity(data, mask, &dest))
return -1;

@@ -3281,16 +3283,12 @@ static int hpet_msi_set_affinity(struct irq_data *data,
return 0;
}

-#endif /* CONFIG_SMP */
-
static struct irq_chip hpet_msi_type = {
.name = "HPET_MSI",
.irq_unmask = hpet_msi_unmask,
.irq_mask = hpet_msi_mask,
.irq_ack = ack_apic_edge,
-#ifdef CONFIG_SMP
.irq_set_affinity = hpet_msi_set_affinity,
-#endif
.irq_retrigger = ioapic_retrigger_irq,
};

@@ -3325,8 +3323,6 @@ int arch_setup_hpet_msi(unsigned int irq, unsigned int id)
*/
#ifdef CONFIG_HT_IRQ

-#ifdef CONFIG_SMP
-
static void target_ht_irq(unsigned int irq, unsigned int dest, u8 vector)
{
struct ht_irq_msg msg;
@@ -3347,6 +3343,9 @@ ht_set_affinity(struct irq_data *data, const struct cpumask *mask, bool force)
struct irq_cfg *cfg = data->chip_data;
unsigned int dest;

+ if (!config_enabled(SMP))
+ return -1;
+
if (__ioapic_set_affinity(data, mask, &dest))
return -1;

@@ -3354,16 +3353,12 @@ ht_set_affinity(struct irq_data *data, const struct cpumask *mask, bool force)
return 0;
}

-#endif
-
static struct irq_chip ht_irq_chip = {
.name = "PCI-HT",
.irq_mask = mask_ht_irq,
.irq_unmask = unmask_ht_irq,
.irq_ack = ack_apic_edge,
-#ifdef CONFIG_SMP
.irq_set_affinity = ht_set_affinity,
-#endif
.irq_retrigger = ioapic_retrigger_irq,
};

diff --git a/drivers/iommu/intel_irq_remapping.c b/drivers/iommu/intel_irq_remapping.c
index 6d34706..b3481cf 100644
--- a/drivers/iommu/intel_irq_remapping.c
+++ b/drivers/iommu/intel_irq_remapping.c
@@ -902,7 +902,6 @@ static int intel_setup_ioapic_entry(int irq,
return 0;
}

-#ifdef CONFIG_SMP
/*
* Migrate the IO-APIC irq in the presence of intr-remapping.
*
@@ -925,6 +924,9 @@ intel_ioapic_set_affinity(struct irq_data *data, const struct cpumask *mask,
unsigned int dest, irq = data->irq;
struct irte irte;

+ if (!config_enabled(CONFIG_SMP))
+ return -EINVAL;
+
if (!cpumask_intersects(mask, cpu_online_mask))
return -EINVAL;

@@ -956,7 +958,6 @@ intel_ioapic_set_affinity(struct irq_data *data, const struct cpumask *mask,
cpumask_copy(data->affinity, mask);
return 0;
}
-#endif

static void intel_compose_msi_msg(struct pci_dev *pdev,
unsigned int irq, unsigned int dest,
@@ -1058,9 +1059,7 @@ struct irq_remap_ops intel_irq_remap_ops = {
.reenable = reenable_irq_remapping,
.enable_faulting = enable_drhd_fault_handling,
.setup_ioapic_entry = intel_setup_ioapic_entry,
-#ifdef CONFIG_SMP
.set_affinity = intel_ioapic_set_affinity,
-#endif
.free_irq = free_irte,
.compose_msi_msg = intel_compose_msi_msg,
.msi_alloc_irq = intel_msi_alloc_irq,
diff --git a/drivers/iommu/irq_remapping.c b/drivers/iommu/irq_remapping.c
index 40cda8e..1d29b1c 100644
--- a/drivers/iommu/irq_remapping.c
+++ b/drivers/iommu/irq_remapping.c
@@ -111,16 +111,15 @@ int setup_ioapic_remapped_entry(int irq,
vector, attr);
}

-#ifdef CONFIG_SMP
int set_remapped_irq_affinity(struct irq_data *data, const struct cpumask *mask,
bool force)
{
- if (!remap_ops || !remap_ops->set_affinity)
+ if (!config_enabled(CONFIG_SMP) || !remap_ops ||
+ !remap_ops->set_affinity)
return 0;

return remap_ops->set_affinity(data, mask, force);
}
-#endif

void free_remapped_irq(int irq)
{
diff --git a/drivers/iommu/irq_remapping.h b/drivers/iommu/irq_remapping.h
index be9d729..b12974c 100644
--- a/drivers/iommu/irq_remapping.h
+++ b/drivers/iommu/irq_remapping.h
@@ -59,11 +59,9 @@ struct irq_remap_ops {
unsigned int, int,
struct io_apic_irq_attr *);

-#ifdef CONFIG_SMP
/* Set the CPU affinity of a remapped interrupt */
int (*set_affinity)(struct irq_data *data, const struct cpumask *mask,
bool force);
-#endif

/* Free an IRQ */
int (*free_irq)(int);
diff --git a/include/linux/irq.h b/include/linux/irq.h
index b27cfcf..008d4ec 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -150,9 +150,7 @@ struct irq_data {
void *handler_data;
void *chip_data;
struct msi_desc *msi_desc;
-#ifdef CONFIG_SMP
cpumask_var_t affinity;
-#endif
};

/*
--
1.7.6.5

2012-05-09 18:56:12

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [PATCH 1/2] kconfig: change config_enabled() to accept X instead of CONFIG_X

On Wed, May 09, 2012 at 11:46:35AM -0700, Suresh Siddha wrote:
> change config_enabled() to use it as config_enabled(SMP) instead of
> config_enabled(CONFIG_SMP).

You are breaking all existing users.
And CONFIG_ was included to allow better grepability.

If you insist on having a CONFIG less variant
introduce a new variant for this.

Sam

2012-05-09 19:05:49

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH 1/2] kconfig: change config_enabled() to accept X instead of CONFIG_X

On Wed, May 9, 2012 at 11:46 AM, Suresh Siddha
<[email protected]> wrote:
> change config_enabled() to use it as config_enabled(SMP) instead of
> config_enabled(CONFIG_SMP).

People hated this when I suggested it, and I kind of understand why.

It breaks greppability of CONFIG_xyz.

So I think I'd have to NAK this.

Linus

2012-05-09 19:25:34

by Suresh Siddha

[permalink] [raw]
Subject: Re: [PATCH 1/2] kconfig: change config_enabled() to accept X instead of CONFIG_X

On Wed, 2012-05-09 at 20:56 +0200, Sam Ravnborg wrote:
> On Wed, May 09, 2012 at 11:46:35AM -0700, Suresh Siddha wrote:
> > change config_enabled() to use it as config_enabled(SMP) instead of
> > config_enabled(CONFIG_SMP).
>
> You are breaking all existing users.

I checked the mainline and there were no users as far as I checked.
Referring to some upcoming code? Just curious to see why I missed.

> And CONFIG_ was included to allow better grepability.

Ok.

> If you insist on having a CONFIG less variant
> introduce a new variant for this.

I will drop this for now.

thanks,
suresh

2012-05-09 19:55:13

by Suresh Siddha

[permalink] [raw]
Subject: Re: [PATCH 2/2] irq: use config_enabled(SMP) checks to cleanup irq_set_affinity() for UP

On Wed, 2012-05-09 at 11:46 -0700, Suresh Siddha wrote:
> Use config_enabled(SMP) checks for cleaning up the ifdef CONFIG_SMP around
> irq_set_affinity routines in io_apic and irq_remapping subsystems.
>

Switched to IS_BUILTIN() instead. Thanks.
---

From: Suresh Siddha <[email protected]>
Subject: irq: use IS_BUILTIN(CONFIG_SMP) checks to cleanup irq_set_affinity() for UP

Use IS_BUILTIN(CONFIG_SMP) checks for cleaning up the ifdef CONFIG_SMP
around irq_set_affinity routines in io_apic and irq_remapping subsystems.

Signed-off-by: Suresh Siddha <[email protected]>
---
arch/x86/kernel/apic/io_apic.c | 173 +++++++++++++++++------------------
drivers/iommu/intel_irq_remapping.c | 7 +-
drivers/iommu/irq_remapping.c | 5 +-
drivers/iommu/irq_remapping.h | 2 -
include/linux/irq.h | 2 -
5 files changed, 89 insertions(+), 100 deletions(-)

diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index ffdc152..f8f3469 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -2221,71 +2221,6 @@ void send_cleanup_vector(struct irq_cfg *cfg)
cfg->move_in_progress = 0;
}

-static void __target_IO_APIC_irq(unsigned int irq, unsigned int dest, struct irq_cfg *cfg)
-{
- int apic, pin;
- struct irq_pin_list *entry;
- u8 vector = cfg->vector;
-
- for_each_irq_pin(entry, cfg->irq_2_pin) {
- unsigned int reg;
-
- apic = entry->apic;
- pin = entry->pin;
- /*
- * With interrupt-remapping, destination information comes
- * from interrupt-remapping table entry.
- */
- if (!irq_remapped(cfg))
- io_apic_write(apic, 0x11 + pin*2, dest);
- reg = io_apic_read(apic, 0x10 + pin*2);
- reg &= ~IO_APIC_REDIR_VECTOR_MASK;
- reg |= vector;
- io_apic_modify(apic, 0x10 + pin*2, reg);
- }
-}
-
-/*
- * Either sets data->affinity to a valid value, and returns
- * ->cpu_mask_to_apicid of that in dest_id, or returns -1 and
- * leaves data->affinity untouched.
- */
-int __ioapic_set_affinity(struct irq_data *data, const struct cpumask *mask,
- unsigned int *dest_id)
-{
- struct irq_cfg *cfg = data->chip_data;
-
- if (!cpumask_intersects(mask, cpu_online_mask))
- return -1;
-
- if (assign_irq_vector(data->irq, data->chip_data, mask))
- return -1;
-
- cpumask_copy(data->affinity, mask);
-
- *dest_id = apic->cpu_mask_to_apicid_and(mask, cfg->domain);
- return 0;
-}
-
-static int
-ioapic_set_affinity(struct irq_data *data, const struct cpumask *mask,
- bool force)
-{
- unsigned int dest, irq = data->irq;
- unsigned long flags;
- int ret;
-
- raw_spin_lock_irqsave(&ioapic_lock, flags);
- ret = __ioapic_set_affinity(data, mask, &dest);
- if (!ret) {
- /* Only the high 8 bits are valid. */
- dest = SET_APIC_LOGICAL_ID(dest);
- __target_IO_APIC_irq(irq, dest, data->chip_data);
- }
- raw_spin_unlock_irqrestore(&ioapic_lock, flags);
- return ret;
-}
-
asmlinkage void smp_irq_move_cleanup_interrupt(void)
{
unsigned vector, me;
@@ -2373,6 +2308,78 @@ void irq_force_complete_move(int irq)
static inline void irq_complete_move(struct irq_cfg *cfg) { }
#endif

+static void __target_IO_APIC_irq(unsigned int irq, unsigned int dest, struct irq_cfg *cfg)
+{
+ int apic, pin;
+ struct irq_pin_list *entry;
+ u8 vector = cfg->vector;
+
+ for_each_irq_pin(entry, cfg->irq_2_pin) {
+ unsigned int reg;
+
+ apic = entry->apic;
+ pin = entry->pin;
+ /*
+ * With interrupt-remapping, destination information comes
+ * from interrupt-remapping table entry.
+ */
+ if (!irq_remapped(cfg))
+ io_apic_write(apic, 0x11 + pin*2, dest);
+ reg = io_apic_read(apic, 0x10 + pin*2);
+ reg &= ~IO_APIC_REDIR_VECTOR_MASK;
+ reg |= vector;
+ io_apic_modify(apic, 0x10 + pin*2, reg);
+ }
+}
+
+/*
+ * Either sets data->affinity to a valid value, and returns
+ * ->cpu_mask_to_apicid of that in dest_id, or returns -1 and
+ * leaves data->affinity untouched.
+ */
+int __ioapic_set_affinity(struct irq_data *data, const struct cpumask *mask,
+ unsigned int *dest_id)
+{
+ struct irq_cfg *cfg = data->chip_data;
+
+ if (!IS_BUILTIN(CONFIG_SMP))
+ return -1;
+
+ if (!cpumask_intersects(mask, cpu_online_mask))
+ return -1;
+
+ if (assign_irq_vector(data->irq, data->chip_data, mask))
+ return -1;
+
+ cpumask_copy(data->affinity, mask);
+
+ *dest_id = apic->cpu_mask_to_apicid_and(mask, cfg->domain);
+ return 0;
+}
+
+static int
+ioapic_set_affinity(struct irq_data *data, const struct cpumask *mask,
+ bool force)
+{
+ unsigned int dest, irq = data->irq;
+ unsigned long flags;
+ int ret;
+
+ if (!IS_BUILTIN(CONFIG_SMP))
+ return -1;
+
+ raw_spin_lock_irqsave(&ioapic_lock, flags);
+ ret = __ioapic_set_affinity(data, mask, &dest);
+ if (!ret) {
+ /* Only the high 8 bits are valid. */
+ dest = SET_APIC_LOGICAL_ID(dest);
+ __target_IO_APIC_irq(irq, dest, data->chip_data);
+ }
+ raw_spin_unlock_irqrestore(&ioapic_lock, flags);
+ return ret;
+}
+
+
static void ack_apic_edge(struct irq_data *data)
{
irq_complete_move(data->chip_data);
@@ -2552,9 +2559,7 @@ static void irq_remap_modify_chip_defaults(struct irq_chip *chip)
chip->irq_ack = ir_ack_apic_edge;
chip->irq_eoi = ir_ack_apic_level;

-#ifdef CONFIG_SMP
chip->irq_set_affinity = set_remapped_irq_affinity;
-#endif
}
#endif /* CONFIG_IRQ_REMAP */

@@ -2565,9 +2570,7 @@ static struct irq_chip ioapic_chip __read_mostly = {
.irq_unmask = unmask_ioapic_irq,
.irq_ack = ack_apic_edge,
.irq_eoi = ack_apic_level,
-#ifdef CONFIG_SMP
.irq_set_affinity = ioapic_set_affinity,
-#endif
.irq_retrigger = ioapic_retrigger_irq,
};

@@ -3083,7 +3086,6 @@ static int msi_compose_msg(struct pci_dev *pdev, unsigned int irq,
return err;
}

-#ifdef CONFIG_SMP
static int
msi_set_affinity(struct irq_data *data, const struct cpumask *mask, bool force)
{
@@ -3091,6 +3093,9 @@ msi_set_affinity(struct irq_data *data, const struct cpumask *mask, bool force)
struct msi_msg msg;
unsigned int dest;

+ if (!IS_BUILTIN(CONFIG_SMP))
+ return -1;
+
if (__ioapic_set_affinity(data, mask, &dest))
return -1;

@@ -3105,7 +3110,6 @@ msi_set_affinity(struct irq_data *data, const struct cpumask *mask, bool force)

return 0;
}
-#endif /* CONFIG_SMP */

/*
* IRQ Chip for MSI PCI/PCI-X/PCI-Express Devices,
@@ -3116,9 +3120,7 @@ static struct irq_chip msi_chip = {
.irq_unmask = unmask_msi_irq,
.irq_mask = mask_msi_irq,
.irq_ack = ack_apic_edge,
-#ifdef CONFIG_SMP
.irq_set_affinity = msi_set_affinity,
-#endif
.irq_retrigger = ioapic_retrigger_irq,
};

@@ -3203,7 +3205,6 @@ void native_teardown_msi_irq(unsigned int irq)
}

#ifdef CONFIG_DMAR_TABLE
-#ifdef CONFIG_SMP
static int
dmar_msi_set_affinity(struct irq_data *data, const struct cpumask *mask,
bool force)
@@ -3212,6 +3213,9 @@ dmar_msi_set_affinity(struct irq_data *data, const struct cpumask *mask,
unsigned int dest, irq = data->irq;
struct msi_msg msg;

+ if (!IS_BUILTIN(CONFIG_SMP))
+ return -1;
+
if (__ioapic_set_affinity(data, mask, &dest))
return -1;

@@ -3228,16 +3232,12 @@ dmar_msi_set_affinity(struct irq_data *data, const struct cpumask *mask,
return 0;
}

-#endif /* CONFIG_SMP */
-
static struct irq_chip dmar_msi_type = {
.name = "DMAR_MSI",
.irq_unmask = dmar_msi_unmask,
.irq_mask = dmar_msi_mask,
.irq_ack = ack_apic_edge,
-#ifdef CONFIG_SMP
.irq_set_affinity = dmar_msi_set_affinity,
-#endif
.irq_retrigger = ioapic_retrigger_irq,
};

@@ -3258,7 +3258,6 @@ int arch_setup_dmar_msi(unsigned int irq)

#ifdef CONFIG_HPET_TIMER

-#ifdef CONFIG_SMP
static int hpet_msi_set_affinity(struct irq_data *data,
const struct cpumask *mask, bool force)
{
@@ -3266,6 +3265,9 @@ static int hpet_msi_set_affinity(struct irq_data *data,
struct msi_msg msg;
unsigned int dest;

+ if (!IS_BUILTIN(CONFIG_SMP))
+ return -1;
+
if (__ioapic_set_affinity(data, mask, &dest))
return -1;

@@ -3281,16 +3283,12 @@ static int hpet_msi_set_affinity(struct irq_data *data,
return 0;
}

-#endif /* CONFIG_SMP */
-
static struct irq_chip hpet_msi_type = {
.name = "HPET_MSI",
.irq_unmask = hpet_msi_unmask,
.irq_mask = hpet_msi_mask,
.irq_ack = ack_apic_edge,
-#ifdef CONFIG_SMP
.irq_set_affinity = hpet_msi_set_affinity,
-#endif
.irq_retrigger = ioapic_retrigger_irq,
};

@@ -3325,8 +3323,6 @@ int arch_setup_hpet_msi(unsigned int irq, unsigned int id)
*/
#ifdef CONFIG_HT_IRQ

-#ifdef CONFIG_SMP
-
static void target_ht_irq(unsigned int irq, unsigned int dest, u8 vector)
{
struct ht_irq_msg msg;
@@ -3347,6 +3343,9 @@ ht_set_affinity(struct irq_data *data, const struct cpumask *mask, bool force)
struct irq_cfg *cfg = data->chip_data;
unsigned int dest;

+ if (!IS_BUILTIN(CONFIG_SMP))
+ return -1;
+
if (__ioapic_set_affinity(data, mask, &dest))
return -1;

@@ -3354,16 +3353,12 @@ ht_set_affinity(struct irq_data *data, const struct cpumask *mask, bool force)
return 0;
}

-#endif
-
static struct irq_chip ht_irq_chip = {
.name = "PCI-HT",
.irq_mask = mask_ht_irq,
.irq_unmask = unmask_ht_irq,
.irq_ack = ack_apic_edge,
-#ifdef CONFIG_SMP
.irq_set_affinity = ht_set_affinity,
-#endif
.irq_retrigger = ioapic_retrigger_irq,
};

diff --git a/drivers/iommu/intel_irq_remapping.c b/drivers/iommu/intel_irq_remapping.c
index 6d34706..b3481cf 100644
--- a/drivers/iommu/intel_irq_remapping.c
+++ b/drivers/iommu/intel_irq_remapping.c
@@ -902,7 +902,6 @@ static int intel_setup_ioapic_entry(int irq,
return 0;
}

-#ifdef CONFIG_SMP
/*
* Migrate the IO-APIC irq in the presence of intr-remapping.
*
@@ -925,6 +924,9 @@ intel_ioapic_set_affinity(struct irq_data *data, const struct cpumask *mask,
unsigned int dest, irq = data->irq;
struct irte irte;

+ if (!IS_BUILTIN(CONFIG_SMP))
+ return -EINVAL;
+
if (!cpumask_intersects(mask, cpu_online_mask))
return -EINVAL;

@@ -956,7 +958,6 @@ intel_ioapic_set_affinity(struct irq_data *data, const struct cpumask *mask,
cpumask_copy(data->affinity, mask);
return 0;
}
-#endif

static void intel_compose_msi_msg(struct pci_dev *pdev,
unsigned int irq, unsigned int dest,
@@ -1058,9 +1059,7 @@ struct irq_remap_ops intel_irq_remap_ops = {
.reenable = reenable_irq_remapping,
.enable_faulting = enable_drhd_fault_handling,
.setup_ioapic_entry = intel_setup_ioapic_entry,
-#ifdef CONFIG_SMP
.set_affinity = intel_ioapic_set_affinity,
-#endif
.free_irq = free_irte,
.compose_msi_msg = intel_compose_msi_msg,
.msi_alloc_irq = intel_msi_alloc_irq,
diff --git a/drivers/iommu/irq_remapping.c b/drivers/iommu/irq_remapping.c
index 40cda8e..1d29b1c 100644
--- a/drivers/iommu/irq_remapping.c
+++ b/drivers/iommu/irq_remapping.c
@@ -111,16 +111,15 @@ int setup_ioapic_remapped_entry(int irq,
vector, attr);
}

-#ifdef CONFIG_SMP
int set_remapped_irq_affinity(struct irq_data *data, const struct cpumask *mask,
bool force)
{
- if (!remap_ops || !remap_ops->set_affinity)
+ if (!IS_BUILTIN(CONFIG_SMP) || !remap_ops ||
+ !remap_ops->set_affinity)
return 0;

return remap_ops->set_affinity(data, mask, force);
}
-#endif

void free_remapped_irq(int irq)
{
diff --git a/drivers/iommu/irq_remapping.h b/drivers/iommu/irq_remapping.h
index be9d729..b12974c 100644
--- a/drivers/iommu/irq_remapping.h
+++ b/drivers/iommu/irq_remapping.h
@@ -59,11 +59,9 @@ struct irq_remap_ops {
unsigned int, int,
struct io_apic_irq_attr *);

-#ifdef CONFIG_SMP
/* Set the CPU affinity of a remapped interrupt */
int (*set_affinity)(struct irq_data *data, const struct cpumask *mask,
bool force);
-#endif

/* Free an IRQ */
int (*free_irq)(int);
diff --git a/include/linux/irq.h b/include/linux/irq.h
index b27cfcf..008d4ec 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -150,9 +150,7 @@ struct irq_data {
void *handler_data;
void *chip_data;
struct msi_desc *msi_desc;
-#ifdef CONFIG_SMP
cpumask_var_t affinity;
-#endif
};

/*
--
1.7.6.5


2012-05-09 20:05:31

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [PATCH 1/2] kconfig: change config_enabled() to accept X instead of CONFIG_X

On Wed, May 09, 2012 at 12:29:01PM -0700, Suresh Siddha wrote:
> On Wed, 2012-05-09 at 20:56 +0200, Sam Ravnborg wrote:
> > On Wed, May 09, 2012 at 11:46:35AM -0700, Suresh Siddha wrote:
> > > change config_enabled() to use it as config_enabled(SMP) instead of
> > > config_enabled(CONFIG_SMP).
> >
> > You are breaking all existing users.
>
> I checked the mainline and there were no users as far as I checked.
> Referring to some upcoming code? Just curious to see why I missed.
I assume that as it is in the tree someone has used it.
There are no users in the upstream kernel but I did not check -next.

Sam

2012-05-10 06:05:34

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH 1/2] kconfig: change config_enabled() to accept X instead of CONFIG_X

On Wed, May 9, 2012 at 8:56 PM, Sam Ravnborg <[email protected]> wrote:
> On Wed, May 09, 2012 at 11:46:35AM -0700, Suresh Siddha wrote:
>> change config_enabled() to use it as config_enabled(SMP) instead of
>> config_enabled(CONFIG_SMP).
>
> You are breaking all existing users.
> And CONFIG_ was included to allow better grepability.

Indeed.

> If you insist on having a CONFIG less variant
> introduce a new variant for this.

It's already bad enough that you have to use
`git grep -w FOO $(git ls-files "*Kconf*")` [*] to find the non-prefixed
symbols in the Kconfig logic, followed by `git grep -w CONFIG_FOO`
for the rest.

[*] Doh, recently I discovered that `git grep -w FOO -- "*Kconf*"' also
works, but my fingers are not yet used to it...

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

2012-05-10 07:50:38

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 2/2] irq: use config_enabled(SMP) checks to cleanup irq_set_affinity() for UP


* Suresh Siddha <[email protected]> wrote:

> On Wed, 2012-05-09 at 11:46 -0700, Suresh Siddha wrote:
> > Use config_enabled(SMP) checks for cleaning up the ifdef CONFIG_SMP around
> > irq_set_affinity routines in io_apic and irq_remapping subsystems.
> >
>
> Switched to IS_BUILTIN() instead. Thanks.

Hm, what was wrong with config_enabled(CONFIG_SMP)?

The IS_BUILTIN() macro is 1) shouting loud, needlessly 2)
somewhat confusing for the CONFIG_SMP case: of course SMP is
'built in', not modular - and the fact that 'built in' also
implies 'enabled' is lost in that naming variant.

So I think IS_BUILTIN() makes more sense for genuine tri-state
config knobs, and bools like CONFIG_SMP should se
config_enabled()..

Thanks,

Ingo

2012-05-10 18:15:42

by Suresh Siddha

[permalink] [raw]
Subject: Re: [PATCH 2/2] irq: use config_enabled(SMP) checks to cleanup irq_set_affinity() for UP

On Thu, 2012-05-10 at 09:50 +0200, Ingo Molnar wrote:
> * Suresh Siddha <[email protected]> wrote:
>
> > On Wed, 2012-05-09 at 11:46 -0700, Suresh Siddha wrote:
> > > Use config_enabled(SMP) checks for cleaning up the ifdef CONFIG_SMP around
> > > irq_set_affinity routines in io_apic and irq_remapping subsystems.
> > >
> >
> > Switched to IS_BUILTIN() instead. Thanks.
>
> Hm, what was wrong with config_enabled(CONFIG_SMP)?

For some reason, didn't like the sound of it. But I am not too picky on
this.

> The IS_BUILTIN() macro is 1) shouting loud, needlessly 2)
> somewhat confusing for the CONFIG_SMP case: of course SMP is
> 'built in', not modular - and the fact that 'built in' also
> implies 'enabled' is lost in that naming variant.
>
> So I think IS_BUILTIN() makes more sense for genuine tri-state
> config knobs, and bools like CONFIG_SMP should se
> config_enabled()..

yes was thinking about this but preferred the sound of IS_BUILTIN() than
two configs ;) Anyways I am not too insistent on this. Appended the
patch with config_enabled(CONFIG_SMP).

Thanks.
---
From: Suresh Siddha <[email protected]>
Subject: irq: use config_enabled(CONFIG_SMP) checks to cleanup irq_set_affinity() for UP

Use config_enabled(CONFIG_SMP) checks for cleaning up the ifdef CONFIG_SMP
around irq_set_affinity routines in io_apic and irq_remapping subsystems.

Signed-off-by: Suresh Siddha <[email protected]>
---
arch/x86/kernel/apic/io_apic.c | 173 +++++++++++++++++------------------
drivers/iommu/intel_irq_remapping.c | 7 +-
drivers/iommu/irq_remapping.c | 5 +-
drivers/iommu/irq_remapping.h | 2 -
include/linux/irq.h | 2 -
5 files changed, 89 insertions(+), 100 deletions(-)

diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index ffdc152..f8f3469 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -2221,71 +2221,6 @@ void send_cleanup_vector(struct irq_cfg *cfg)
cfg->move_in_progress = 0;
}

-static void __target_IO_APIC_irq(unsigned int irq, unsigned int dest, struct irq_cfg *cfg)
-{
- int apic, pin;
- struct irq_pin_list *entry;
- u8 vector = cfg->vector;
-
- for_each_irq_pin(entry, cfg->irq_2_pin) {
- unsigned int reg;
-
- apic = entry->apic;
- pin = entry->pin;
- /*
- * With interrupt-remapping, destination information comes
- * from interrupt-remapping table entry.
- */
- if (!irq_remapped(cfg))
- io_apic_write(apic, 0x11 + pin*2, dest);
- reg = io_apic_read(apic, 0x10 + pin*2);
- reg &= ~IO_APIC_REDIR_VECTOR_MASK;
- reg |= vector;
- io_apic_modify(apic, 0x10 + pin*2, reg);
- }
-}
-
-/*
- * Either sets data->affinity to a valid value, and returns
- * ->cpu_mask_to_apicid of that in dest_id, or returns -1 and
- * leaves data->affinity untouched.
- */
-int __ioapic_set_affinity(struct irq_data *data, const struct cpumask *mask,
- unsigned int *dest_id)
-{
- struct irq_cfg *cfg = data->chip_data;
-
- if (!cpumask_intersects(mask, cpu_online_mask))
- return -1;
-
- if (assign_irq_vector(data->irq, data->chip_data, mask))
- return -1;
-
- cpumask_copy(data->affinity, mask);
-
- *dest_id = apic->cpu_mask_to_apicid_and(mask, cfg->domain);
- return 0;
-}
-
-static int
-ioapic_set_affinity(struct irq_data *data, const struct cpumask *mask,
- bool force)
-{
- unsigned int dest, irq = data->irq;
- unsigned long flags;
- int ret;
-
- raw_spin_lock_irqsave(&ioapic_lock, flags);
- ret = __ioapic_set_affinity(data, mask, &dest);
- if (!ret) {
- /* Only the high 8 bits are valid. */
- dest = SET_APIC_LOGICAL_ID(dest);
- __target_IO_APIC_irq(irq, dest, data->chip_data);
- }
- raw_spin_unlock_irqrestore(&ioapic_lock, flags);
- return ret;
-}
-
asmlinkage void smp_irq_move_cleanup_interrupt(void)
{
unsigned vector, me;
@@ -2373,6 +2308,78 @@ void irq_force_complete_move(int irq)
static inline void irq_complete_move(struct irq_cfg *cfg) { }
#endif

+static void __target_IO_APIC_irq(unsigned int irq, unsigned int dest, struct irq_cfg *cfg)
+{
+ int apic, pin;
+ struct irq_pin_list *entry;
+ u8 vector = cfg->vector;
+
+ for_each_irq_pin(entry, cfg->irq_2_pin) {
+ unsigned int reg;
+
+ apic = entry->apic;
+ pin = entry->pin;
+ /*
+ * With interrupt-remapping, destination information comes
+ * from interrupt-remapping table entry.
+ */
+ if (!irq_remapped(cfg))
+ io_apic_write(apic, 0x11 + pin*2, dest);
+ reg = io_apic_read(apic, 0x10 + pin*2);
+ reg &= ~IO_APIC_REDIR_VECTOR_MASK;
+ reg |= vector;
+ io_apic_modify(apic, 0x10 + pin*2, reg);
+ }
+}
+
+/*
+ * Either sets data->affinity to a valid value, and returns
+ * ->cpu_mask_to_apicid of that in dest_id, or returns -1 and
+ * leaves data->affinity untouched.
+ */
+int __ioapic_set_affinity(struct irq_data *data, const struct cpumask *mask,
+ unsigned int *dest_id)
+{
+ struct irq_cfg *cfg = data->chip_data;
+
+ if (!config_enabled(CONFIG_SMP))
+ return -1;
+
+ if (!cpumask_intersects(mask, cpu_online_mask))
+ return -1;
+
+ if (assign_irq_vector(data->irq, data->chip_data, mask))
+ return -1;
+
+ cpumask_copy(data->affinity, mask);
+
+ *dest_id = apic->cpu_mask_to_apicid_and(mask, cfg->domain);
+ return 0;
+}
+
+static int
+ioapic_set_affinity(struct irq_data *data, const struct cpumask *mask,
+ bool force)
+{
+ unsigned int dest, irq = data->irq;
+ unsigned long flags;
+ int ret;
+
+ if (!config_enabled(CONFIG_SMP))
+ return -1;
+
+ raw_spin_lock_irqsave(&ioapic_lock, flags);
+ ret = __ioapic_set_affinity(data, mask, &dest);
+ if (!ret) {
+ /* Only the high 8 bits are valid. */
+ dest = SET_APIC_LOGICAL_ID(dest);
+ __target_IO_APIC_irq(irq, dest, data->chip_data);
+ }
+ raw_spin_unlock_irqrestore(&ioapic_lock, flags);
+ return ret;
+}
+
+
static void ack_apic_edge(struct irq_data *data)
{
irq_complete_move(data->chip_data);
@@ -2552,9 +2559,7 @@ static void irq_remap_modify_chip_defaults(struct irq_chip *chip)
chip->irq_ack = ir_ack_apic_edge;
chip->irq_eoi = ir_ack_apic_level;

-#ifdef CONFIG_SMP
chip->irq_set_affinity = set_remapped_irq_affinity;
-#endif
}
#endif /* CONFIG_IRQ_REMAP */

@@ -2565,9 +2570,7 @@ static struct irq_chip ioapic_chip __read_mostly = {
.irq_unmask = unmask_ioapic_irq,
.irq_ack = ack_apic_edge,
.irq_eoi = ack_apic_level,
-#ifdef CONFIG_SMP
.irq_set_affinity = ioapic_set_affinity,
-#endif
.irq_retrigger = ioapic_retrigger_irq,
};

@@ -3083,7 +3086,6 @@ static int msi_compose_msg(struct pci_dev *pdev, unsigned int irq,
return err;
}

-#ifdef CONFIG_SMP
static int
msi_set_affinity(struct irq_data *data, const struct cpumask *mask, bool force)
{
@@ -3091,6 +3093,9 @@ msi_set_affinity(struct irq_data *data, const struct cpumask *mask, bool force)
struct msi_msg msg;
unsigned int dest;

+ if (!config_enabled(CONFIG_SMP))
+ return -1;
+
if (__ioapic_set_affinity(data, mask, &dest))
return -1;

@@ -3105,7 +3110,6 @@ msi_set_affinity(struct irq_data *data, const struct cpumask *mask, bool force)

return 0;
}
-#endif /* CONFIG_SMP */

/*
* IRQ Chip for MSI PCI/PCI-X/PCI-Express Devices,
@@ -3116,9 +3120,7 @@ static struct irq_chip msi_chip = {
.irq_unmask = unmask_msi_irq,
.irq_mask = mask_msi_irq,
.irq_ack = ack_apic_edge,
-#ifdef CONFIG_SMP
.irq_set_affinity = msi_set_affinity,
-#endif
.irq_retrigger = ioapic_retrigger_irq,
};

@@ -3203,7 +3205,6 @@ void native_teardown_msi_irq(unsigned int irq)
}

#ifdef CONFIG_DMAR_TABLE
-#ifdef CONFIG_SMP
static int
dmar_msi_set_affinity(struct irq_data *data, const struct cpumask *mask,
bool force)
@@ -3212,6 +3213,9 @@ dmar_msi_set_affinity(struct irq_data *data, const struct cpumask *mask,
unsigned int dest, irq = data->irq;
struct msi_msg msg;

+ if (!config_enabled(CONFIG_SMP))
+ return -1;
+
if (__ioapic_set_affinity(data, mask, &dest))
return -1;

@@ -3228,16 +3232,12 @@ dmar_msi_set_affinity(struct irq_data *data, const struct cpumask *mask,
return 0;
}

-#endif /* CONFIG_SMP */
-
static struct irq_chip dmar_msi_type = {
.name = "DMAR_MSI",
.irq_unmask = dmar_msi_unmask,
.irq_mask = dmar_msi_mask,
.irq_ack = ack_apic_edge,
-#ifdef CONFIG_SMP
.irq_set_affinity = dmar_msi_set_affinity,
-#endif
.irq_retrigger = ioapic_retrigger_irq,
};

@@ -3258,7 +3258,6 @@ int arch_setup_dmar_msi(unsigned int irq)

#ifdef CONFIG_HPET_TIMER

-#ifdef CONFIG_SMP
static int hpet_msi_set_affinity(struct irq_data *data,
const struct cpumask *mask, bool force)
{
@@ -3266,6 +3265,9 @@ static int hpet_msi_set_affinity(struct irq_data *data,
struct msi_msg msg;
unsigned int dest;

+ if (!config_enabled(CONFIG_SMP))
+ return -1;
+
if (__ioapic_set_affinity(data, mask, &dest))
return -1;

@@ -3281,16 +3283,12 @@ static int hpet_msi_set_affinity(struct irq_data *data,
return 0;
}

-#endif /* CONFIG_SMP */
-
static struct irq_chip hpet_msi_type = {
.name = "HPET_MSI",
.irq_unmask = hpet_msi_unmask,
.irq_mask = hpet_msi_mask,
.irq_ack = ack_apic_edge,
-#ifdef CONFIG_SMP
.irq_set_affinity = hpet_msi_set_affinity,
-#endif
.irq_retrigger = ioapic_retrigger_irq,
};

@@ -3325,8 +3323,6 @@ int arch_setup_hpet_msi(unsigned int irq, unsigned int id)
*/
#ifdef CONFIG_HT_IRQ

-#ifdef CONFIG_SMP
-
static void target_ht_irq(unsigned int irq, unsigned int dest, u8 vector)
{
struct ht_irq_msg msg;
@@ -3347,6 +3343,9 @@ ht_set_affinity(struct irq_data *data, const struct cpumask *mask, bool force)
struct irq_cfg *cfg = data->chip_data;
unsigned int dest;

+ if (!config_enabled(CONFIG_SMP))
+ return -1;
+
if (__ioapic_set_affinity(data, mask, &dest))
return -1;

@@ -3354,16 +3353,12 @@ ht_set_affinity(struct irq_data *data, const struct cpumask *mask, bool force)
return 0;
}

-#endif
-
static struct irq_chip ht_irq_chip = {
.name = "PCI-HT",
.irq_mask = mask_ht_irq,
.irq_unmask = unmask_ht_irq,
.irq_ack = ack_apic_edge,
-#ifdef CONFIG_SMP
.irq_set_affinity = ht_set_affinity,
-#endif
.irq_retrigger = ioapic_retrigger_irq,
};

diff --git a/drivers/iommu/intel_irq_remapping.c b/drivers/iommu/intel_irq_remapping.c
index 6d34706..b3481cf 100644
--- a/drivers/iommu/intel_irq_remapping.c
+++ b/drivers/iommu/intel_irq_remapping.c
@@ -902,7 +902,6 @@ static int intel_setup_ioapic_entry(int irq,
return 0;
}

-#ifdef CONFIG_SMP
/*
* Migrate the IO-APIC irq in the presence of intr-remapping.
*
@@ -925,6 +924,9 @@ intel_ioapic_set_affinity(struct irq_data *data, const struct cpumask *mask,
unsigned int dest, irq = data->irq;
struct irte irte;

+ if (!config_enabled(CONFIG_SMP))
+ return -EINVAL;
+
if (!cpumask_intersects(mask, cpu_online_mask))
return -EINVAL;

@@ -956,7 +958,6 @@ intel_ioapic_set_affinity(struct irq_data *data, const struct cpumask *mask,
cpumask_copy(data->affinity, mask);
return 0;
}
-#endif

static void intel_compose_msi_msg(struct pci_dev *pdev,
unsigned int irq, unsigned int dest,
@@ -1058,9 +1059,7 @@ struct irq_remap_ops intel_irq_remap_ops = {
.reenable = reenable_irq_remapping,
.enable_faulting = enable_drhd_fault_handling,
.setup_ioapic_entry = intel_setup_ioapic_entry,
-#ifdef CONFIG_SMP
.set_affinity = intel_ioapic_set_affinity,
-#endif
.free_irq = free_irte,
.compose_msi_msg = intel_compose_msi_msg,
.msi_alloc_irq = intel_msi_alloc_irq,
diff --git a/drivers/iommu/irq_remapping.c b/drivers/iommu/irq_remapping.c
index 40cda8e..1d29b1c 100644
--- a/drivers/iommu/irq_remapping.c
+++ b/drivers/iommu/irq_remapping.c
@@ -111,16 +111,15 @@ int setup_ioapic_remapped_entry(int irq,
vector, attr);
}

-#ifdef CONFIG_SMP
int set_remapped_irq_affinity(struct irq_data *data, const struct cpumask *mask,
bool force)
{
- if (!remap_ops || !remap_ops->set_affinity)
+ if (!config_enabled(CONFIG_SMP) || !remap_ops ||
+ !remap_ops->set_affinity)
return 0;

return remap_ops->set_affinity(data, mask, force);
}
-#endif

void free_remapped_irq(int irq)
{
diff --git a/drivers/iommu/irq_remapping.h b/drivers/iommu/irq_remapping.h
index be9d729..b12974c 100644
--- a/drivers/iommu/irq_remapping.h
+++ b/drivers/iommu/irq_remapping.h
@@ -59,11 +59,9 @@ struct irq_remap_ops {
unsigned int, int,
struct io_apic_irq_attr *);

-#ifdef CONFIG_SMP
/* Set the CPU affinity of a remapped interrupt */
int (*set_affinity)(struct irq_data *data, const struct cpumask *mask,
bool force);
-#endif

/* Free an IRQ */
int (*free_irq)(int);
diff --git a/include/linux/irq.h b/include/linux/irq.h
index b27cfcf..008d4ec 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -150,9 +150,7 @@ struct irq_data {
void *handler_data;
void *chip_data;
struct msi_desc *msi_desc;
-#ifdef CONFIG_SMP
cpumask_var_t affinity;
-#endif
};

/*
--
1.7.6.5