[patch 1/3] x86, intr-remap: set redirection hint in the IRTE

[patch 1/3] x86, intr-remap: set redirection hint in the IRTE

[Suresh Siddha]

[-- Attachment #1: set_redir_hint_for_irte.patch --]
[-- Type: text/plain, Size: 1843 bytes --]

Currently the redirection hint in the interrupt-remapping table entry
is set to 0, which means the remapped interrupt is directed to the
processors listed in the destination. So in logical flat mode
in the presence of intr-remapping, this results in a single
interrupt multi-casted to multiple cpu's as specified by the destination
bit mask. But what we really want is to send that interrupt to one of the cpus
based on the lowest priority delivery mode.

Set the redirection hint in the IRTE to '1' to indicate that we want
the remapped interrupt to be directed to only one of the processors
listed in the destination.

This fixes the issue of same interrupt getting delivered to multiple cpu's
in the logical flat mode in the presence of interrupt-remapping. While
there is no functional issue observed with this behavior, this will
impact performance of such configurations (<=8 cpu's using logical flat
mode in the presence of interrupt-remapping)

Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
Cc: Weidong Han <weidong.han@intel.com>
Cc: <stable@kernel.org> # [v2.6.32+]
---

diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index f1efeba..90f8a75 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -1392,6 +1392,7 @@ int setup_ioapic_entry(int apic_id, int irq,
 		irte.dlvry_mode = apic->irq_delivery_mode;
 		irte.vector = vector;
 		irte.dest_id = IRTE_DEST(destination);
+		irte.redir_hint = 1;
 
 		/* Set source-id of interrupt request */
 		set_ioapic_sid(&irte, apic_id);
@@ -3343,6 +3344,7 @@ static int msi_compose_msg(struct pci_dev *pdev, unsigned int irq,
 		irte.dlvry_mode = apic->irq_delivery_mode;
 		irte.vector = cfg->vector;
 		irte.dest_id = IRTE_DEST(dest);
+		irte.redir_hint = 1;
 
 		/* Set source-id of interrupt request */
 		if (pdev)

[patch 2/3] x86, intr-remap: remove IRTE setup duplicate code

[Suresh Siddha]

[-- Attachment #1: cleanup_irte_setup.patch --]
[-- Type: text/plain, Size: 3051 bytes --]

Remove IRTE setup duplicate code with prepare_irte()

Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
---
 arch/x86/include/asm/irq_remapping.h |   27 +++++++++++++++++++++++++++
 arch/x86/kernel/apic/io_apic.c       |   27 ++-------------------------
 2 files changed, 29 insertions(+), 25 deletions(-)

Index: tree/arch/x86/kernel/apic/io_apic.c
===================================================================
--- tree.orig/arch/x86/kernel/apic/io_apic.c
+++ tree/arch/x86/kernel/apic/io_apic.c
@@ -1377,22 +1377,7 @@ int setup_ioapic_entry(int apic_id, int 
 		if (index < 0)
 			panic("Failed to allocate IRTE for ioapic %d\n", apic_id);
 
-		memset(&irte, 0, sizeof(irte));
-
-		irte.present = 1;
-		irte.dst_mode = apic->irq_dest_mode;
-		/*
-		 * Trigger mode in the IRTE will always be edge, and the
-		 * actual level or edge trigger will be setup in the IO-APIC
-		 * RTE. This will help simplify level triggered irq migration.
-		 * For more details, see the comments above explainig IO-APIC
-		 * irq migration in the presence of interrupt-remapping.
-		 */
-		irte.trigger_mode = 0;
-		irte.dlvry_mode = apic->irq_delivery_mode;
-		irte.vector = vector;
-		irte.dest_id = IRTE_DEST(destination);
-		irte.redir_hint = 1;
+		prepare_irte(&irte, vector, destination);
 
 		/* Set source-id of interrupt request */
 		set_ioapic_sid(&irte, apic_id);
@@ -3336,15 +3321,7 @@ static int msi_compose_msg(struct pci_de
 		ir_index = map_irq_to_irte_handle(irq, &sub_handle);
 		BUG_ON(ir_index == -1);
 
-		memset (&irte, 0, sizeof(irte));
-
-		irte.present = 1;
-		irte.dst_mode = apic->irq_dest_mode;
-		irte.trigger_mode = 0; /* edge */
-		irte.dlvry_mode = apic->irq_delivery_mode;
-		irte.vector = cfg->vector;
-		irte.dest_id = IRTE_DEST(dest);
-		irte.redir_hint = 1;
+		prepare_irte(&irte, cfg->vector, dest);
 
 		/* Set source-id of interrupt request */
 		if (pdev)
Index: tree/arch/x86/include/asm/irq_remapping.h
===================================================================
--- tree.orig/arch/x86/include/asm/irq_remapping.h
+++ tree/arch/x86/include/asm/irq_remapping.h
@@ -3,4 +3,31 @@
 
 #define IRTE_DEST(dest) ((x2apic_mode) ? dest : dest << 8)
 
+#ifdef CONFIG_INTR_REMAP
+static inline void prepare_irte(struct irte *irte, int vector,
+			        unsigned int dest)
+{
+	memset(irte, 0, sizeof(*irte));
+
+	irte->present = 1;
+	irte->dst_mode = apic->irq_dest_mode;
+	/*
+	 * Trigger mode in the IRTE will always be edge, and for IO-APIC, the
+	 * actual level or edge trigger will be setup in the IO-APIC
+	 * RTE. This will help simplify level triggered irq migration.
+	 * For more details, see the comments (in io_apic.c) explainig IO-APIC
+	 * irq migration in the presence of interrupt-remapping.
+	*/
+	irte->trigger_mode = 0;
+	irte->dlvry_mode = apic->irq_delivery_mode;
+	irte->vector = vector;
+	irte->dest_id = IRTE_DEST(dest);
+	irte->redir_hint = 1;
+}
+#else
+static void prepare_irte(struct irte *irte, int vector, unsigned int dest)
+{
+}
+#endif
+
 #endif	/* _ASM_X86_IRQ_REMAPPING_H */

[tip:x86/x2apic] x86, intr-remap: Remove IRTE setup duplicate code

[tip-bot for Suresh Siddha]

Commit-ID:  62a92f4c69cd1d9361ad8c16be1dd16e6821bc15
Gitweb:     http://git.kernel.org/tip/62a92f4c69cd1d9361ad8c16be1dd16e6821bc15
Author:     Suresh Siddha <suresh.b.siddha@intel.com>
AuthorDate: Fri, 27 Aug 2010 11:09:49 -0700
Committer:  H. Peter Anvin <hpa@linux.intel.com>
CommitDate: Wed, 15 Sep 2010 17:36:45 -0700

x86, intr-remap: Remove IRTE setup duplicate code

Remove IRTE setup duplicate code with prepare_irte().

Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
LKML-Reference: <20100827181049.095067319@sbsiddha-MOBL3.sc.intel.com>
Signed-off-by: H. Peter Anvin <hpa@linux.intel.com>
---
 arch/x86/include/asm/irq_remapping.h |   27 +++++++++++++++++++++++++++
 arch/x86/kernel/apic/io_apic.c       |   27 ++-------------------------
 2 files changed, 29 insertions(+), 25 deletions(-)

diff --git a/arch/x86/include/asm/irq_remapping.h b/arch/x86/include/asm/irq_remapping.h
index f275e22..8d84150 100644
--- a/arch/x86/include/asm/irq_remapping.h
+++ b/arch/x86/include/asm/irq_remapping.h
@@ -3,4 +3,31 @@
 
 #define IRTE_DEST(dest) ((x2apic_mode) ? dest : dest << 8)
 
+#ifdef CONFIG_INTR_REMAP
+static inline void prepare_irte(struct irte *irte, int vector,
+			        unsigned int dest)
+{
+	memset(irte, 0, sizeof(*irte));
+
+	irte->present = 1;
+	irte->dst_mode = apic->irq_dest_mode;
+	/*
+	 * Trigger mode in the IRTE will always be edge, and for IO-APIC, the
+	 * actual level or edge trigger will be setup in the IO-APIC
+	 * RTE. This will help simplify level triggered irq migration.
+	 * For more details, see the comments (in io_apic.c) explainig IO-APIC
+	 * irq migration in the presence of interrupt-remapping.
+	*/
+	irte->trigger_mode = 0;
+	irte->dlvry_mode = apic->irq_delivery_mode;
+	irte->vector = vector;
+	irte->dest_id = IRTE_DEST(dest);
+	irte->redir_hint = 1;
+}
+#else
+static void prepare_irte(struct irte *irte, int vector, unsigned int dest)
+{
+}
+#endif
+
 #endif	/* _ASM_X86_IRQ_REMAPPING_H */
diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 90f8a75..e8c95a2 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -1377,22 +1377,7 @@ int setup_ioapic_entry(int apic_id, int irq,
 		if (index < 0)
 			panic("Failed to allocate IRTE for ioapic %d\n", apic_id);
 
-		memset(&irte, 0, sizeof(irte));
-
-		irte.present = 1;
-		irte.dst_mode = apic->irq_dest_mode;
-		/*
-		 * Trigger mode in the IRTE will always be edge, and the
-		 * actual level or edge trigger will be setup in the IO-APIC
-		 * RTE. This will help simplify level triggered irq migration.
-		 * For more details, see the comments above explainig IO-APIC
-		 * irq migration in the presence of interrupt-remapping.
-		 */
-		irte.trigger_mode = 0;
-		irte.dlvry_mode = apic->irq_delivery_mode;
-		irte.vector = vector;
-		irte.dest_id = IRTE_DEST(destination);
-		irte.redir_hint = 1;
+		prepare_irte(&irte, vector, destination);
 
 		/* Set source-id of interrupt request */
 		set_ioapic_sid(&irte, apic_id);
@@ -3336,15 +3321,7 @@ static int msi_compose_msg(struct pci_dev *pdev, unsigned int irq,
 		ir_index = map_irq_to_irte_handle(irq, &sub_handle);
 		BUG_ON(ir_index == -1);
 
-		memset (&irte, 0, sizeof(irte));
-
-		irte.present = 1;
-		irte.dst_mode = apic->irq_dest_mode;
-		irte.trigger_mode = 0; /* edge */
-		irte.dlvry_mode = apic->irq_delivery_mode;
-		irte.vector = cfg->vector;
-		irte.dest_id = IRTE_DEST(dest);
-		irte.redir_hint = 1;
+		prepare_irte(&irte, cfg->vector, dest);
 
 		/* Set source-id of interrupt request */
 		if (pdev)

[patch 3/3] x86, x2apic: simplify apic init in SMP and UP builds

[Suresh Siddha]

[-- Attachment #1: cleanup_enable_IR_seq.patch --]
[-- Type: text/plain, Size: 2025 bytes --]

Move enable_IR_x2apic() inside the default_setup_apic_routing()

And for smp platforms, move the default_setup_apic_routing() after
smp_sanity_check(). This cleans up the code that tries to avoid multiple
calls to default_setup_apic_routing() when smp_sanity_check() fails (which
goes through the APIC_init_uniprocessor() path)

Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
---
 arch/x86/kernel/apic/apic.c     |    3 ---
 arch/x86/kernel/apic/probe_64.c |    3 +++
 arch/x86/kernel/smpboot.c       |    4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)

Index: tip/arch/x86/kernel/apic/apic.c
===================================================================
--- tip.orig/arch/x86/kernel/apic/apic.c
+++ tip/arch/x86/kernel/apic/apic.c
@@ -1665,10 +1665,7 @@ int __init APIC_init_uniprocessor(void)
 	}
 #endif
 
-#ifndef CONFIG_SMP
-	enable_IR_x2apic();
 	default_setup_apic_routing();
-#endif
 
 	verify_local_APIC();
 	connect_bsp_APIC();
Index: tip/arch/x86/kernel/apic/probe_64.c
===================================================================
--- tip.orig/arch/x86/kernel/apic/probe_64.c
+++ tip/arch/x86/kernel/apic/probe_64.c
@@ -54,6 +54,9 @@ static int apicid_phys_pkg_id(int initia
  */
 void __init default_setup_apic_routing(void)
 {
+
+	enable_IR_x2apic();
+
 #ifdef CONFIG_X86_X2APIC
 	if (x2apic_mode
 #ifdef CONFIG_X86_UV
Index: tip/arch/x86/kernel/smpboot.c
===================================================================
--- tip.orig/arch/x86/kernel/smpboot.c
+++ tip/arch/x86/kernel/smpboot.c
@@ -1107,8 +1107,6 @@ void __init native_smp_prepare_cpus(unsi
 	}
 	set_cpu_sibling_map(0);
 
-	enable_IR_x2apic();
-	default_setup_apic_routing();
 
 	if (smp_sanity_check(max_cpus) < 0) {
 		printk(KERN_INFO "SMP disabled\n");
@@ -1116,6 +1114,8 @@ void __init native_smp_prepare_cpus(unsi
 		goto out;
 	}
 
+	default_setup_apic_routing();
+
 	preempt_disable();
 	if (read_apic_id() != boot_cpu_physical_apicid) {
 		panic("Boot APIC ID in local APIC unexpected (%d vs %d)",

[tip:x86/x2apic] x86, x2apic: Simplify apic init in SMP and UP builds

[tip-bot for Suresh Siddha]

Commit-ID:  fa47f7e52874683a9659df2f1f143105f676dc0f
Gitweb:     http://git.kernel.org/tip/fa47f7e52874683a9659df2f1f143105f676dc0f
Author:     Suresh Siddha <suresh.b.siddha@intel.com>
AuthorDate: Fri, 27 Aug 2010 11:09:50 -0700
Committer:  H. Peter Anvin <hpa@linux.intel.com>
CommitDate: Wed, 15 Sep 2010 17:37:10 -0700

x86, x2apic: Simplify apic init in SMP and UP builds

Move enable_IR_x2apic() inside the default_setup_apic_routing(),
and for SMP platforms, move the default_setup_apic_routing() after
smp_sanity_check(). This cleans up the code that tries to avoid multiple
calls to default_setup_apic_routing() when smp_sanity_check() fails (which
goes through the APIC_init_uniprocessor() path).

Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
LKML-Reference: <20100827181049.173087246@sbsiddha-MOBL3.sc.intel.com>
Signed-off-by: H. Peter Anvin <hpa@linux.intel.com>
---
 arch/x86/kernel/apic/apic.c     |    3 ---
 arch/x86/kernel/apic/probe_64.c |    3 +++
 arch/x86/kernel/smpboot.c       |    4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index e3b534c..8cf86fb 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -1665,10 +1665,7 @@ int __init APIC_init_uniprocessor(void)
 	}
 #endif
 
-#ifndef CONFIG_SMP
-	enable_IR_x2apic();
 	default_setup_apic_routing();
-#endif
 
 	verify_local_APIC();
 	connect_bsp_APIC();
diff --git a/arch/x86/kernel/apic/probe_64.c b/arch/x86/kernel/apic/probe_64.c
index 83e9be4..f9e4e6a 100644
--- a/arch/x86/kernel/apic/probe_64.c
+++ b/arch/x86/kernel/apic/probe_64.c
@@ -54,6 +54,9 @@ static int apicid_phys_pkg_id(int initial_apic_id, int index_msb)
  */
 void __init default_setup_apic_routing(void)
 {
+
+	enable_IR_x2apic();
+
 #ifdef CONFIG_X86_X2APIC
 	if (x2apic_mode
 #ifdef CONFIG_X86_UV
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 8b3bfc4..87a8c6b 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -1109,8 +1109,6 @@ void __init native_smp_prepare_cpus(unsigned int max_cpus)
 	}
 	set_cpu_sibling_map(0);
 
-	enable_IR_x2apic();
-	default_setup_apic_routing();
 
 	if (smp_sanity_check(max_cpus) < 0) {
 		printk(KERN_INFO "SMP disabled\n");
@@ -1118,6 +1116,8 @@ void __init native_smp_prepare_cpus(unsigned int max_cpus)
 		goto out;
 	}
 
+	default_setup_apic_routing();
+
 	preempt_disable();
 	if (read_apic_id() != boot_cpu_physical_apicid) {
 		panic("Boot APIC ID in local APIC unexpected (%d vs %d)",

Re: [patch 1/3] x86, intr-remap: set redirection hint in the IRTE

[Weidong Han]

Siddha, Suresh B wrote:
> Currently the redirection hint in the interrupt-remapping table entry
> is set to 0, which means the remapped interrupt is directed to the
> processors listed in the destination. So in logical flat mode
> in the presence of intr-remapping, this results in a single
> interrupt multi-casted to multiple cpu's as specified by the destination
> bit mask. But what we really want is to send that interrupt to one of the cpus
> based on the lowest priority delivery mode.
>
> Set the redirection hint in the IRTE to '1' to indicate that we want
> the remapped interrupt to be directed to only one of the processors
> listed in the destination.
>
> This fixes the issue of same interrupt getting delivered to multiple cpu's
> in the logical flat mode in the presence of interrupt-remapping. While
> there is no functional issue observed with this behavior, this will
> impact performance of such configurations (<=8 cpu's using logical flat
> mode in the presence of interrupt-remapping)
>
> Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
> Cc: Weidong Han <weidong.han@intel.com>
> Cc: <stable@kernel.org> # [v2.6.32+]
> ---
>
> diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
> index f1efeba..90f8a75 100644
> --- a/arch/x86/kernel/apic/io_apic.c
> +++ b/arch/x86/kernel/apic/io_apic.c
> @@ -1392,6 +1392,7 @@ int setup_ioapic_entry(int apic_id, int irq,
>  		irte.dlvry_mode = apic->irq_delivery_mode;
>  		irte.vector = vector;
>  		irte.dest_id = IRTE_DEST(destination);
> +		irte.redir_hint = 1;
>  
>  		/* Set source-id of interrupt request */
>  		set_ioapic_sid(&irte, apic_id);
> @@ -3343,6 +3344,7 @@ static int msi_compose_msg(struct pci_dev *pdev, unsigned int irq,
>  		irte.dlvry_mode = apic->irq_delivery_mode;
>  		irte.vector = cfg->vector;
>  		irte.dest_id = IRTE_DEST(dest);
> +		irte.redir_hint = 1;
>  
>  		/* Set source-id of interrupt request */
>  		if (pdev)
>
>
>   
Hi Suresh,

Your patch always sets redir_hint to 1 for all delivery modes. Why not 
check if it's lowest priority delivery mode before set redit_hint to 1?

Regards,
Weidong

Re: [patch 1/3] x86, intr-remap: set redirection hint in the IRTE

[Weidong Han]

Siddha, Suresh B wrote:
> Currently the redirection hint in the interrupt-remapping table entry
> is set to 0, which means the remapped interrupt is directed to the
> processors listed in the destination. So in logical flat mode
> in the presence of intr-remapping, this results in a single
> interrupt multi-casted to multiple cpu's as specified by the destination
> bit mask. But what we really want is to send that interrupt to one of the cpus
> based on the lowest priority delivery mode.
>
> Set the redirection hint in the IRTE to '1' to indicate that we want
> the remapped interrupt to be directed to only one of the processors
> listed in the destination.
>
> This fixes the issue of same interrupt getting delivered to multiple cpu's
> in the logical flat mode in the presence of interrupt-remapping. While
> there is no functional issue observed with this behavior, this will
> impact performance of such configurations (<=8 cpu's using logical flat
> mode in the presence of interrupt-remapping)
>
> Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
> Cc: Weidong Han <weidong.han@intel.com>
> Cc: <stable@kernel.org> # [v2.6.32+]
> ---
>
> diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
> index f1efeba..90f8a75 100644
> --- a/arch/x86/kernel/apic/io_apic.c
> +++ b/arch/x86/kernel/apic/io_apic.c
> @@ -1392,6 +1392,7 @@ int setup_ioapic_entry(int apic_id, int irq,
>  		irte.dlvry_mode = apic->irq_delivery_mode;
>  		irte.vector = vector;
>  		irte.dest_id = IRTE_DEST(destination);
> +		irte.redir_hint = 1;
>  
>  		/* Set source-id of interrupt request */
>  		set_ioapic_sid(&irte, apic_id);
> @@ -3343,6 +3344,7 @@ static int msi_compose_msg(struct pci_dev *pdev, unsigned int irq,
>  		irte.dlvry_mode = apic->irq_delivery_mode;
>  		irte.vector = cfg->vector;
>  		irte.dest_id = IRTE_DEST(dest);
> +		irte.redir_hint = 1;
>  
>  		/* Set source-id of interrupt request */
>  		if (pdev)
>
>
>   
hi Suresh,

I think it needs to set redir_hint only for lowest priority delivery 
mode. Why set it for all delivery modes?

Regards,
Weidong

Re: [patch 1/3] x86, intr-remap: set redirection hint in the IRTE

[Suresh Siddha]

On Sun, 2010-08-29 at 18:47 -0700, Han, Weidong wrote:
> Siddha, Suresh B wrote:
> > Currently the redirection hint in the interrupt-remapping table entry
> > is set to 0, which means the remapped interrupt is directed to the
> > processors listed in the destination. So in logical flat mode
> > in the presence of intr-remapping, this results in a single
> > interrupt multi-casted to multiple cpu's as specified by the destination
> > bit mask. But what we really want is to send that interrupt to one of the cpus
> > based on the lowest priority delivery mode.
> >
> > Set the redirection hint in the IRTE to '1' to indicate that we want
> > the remapped interrupt to be directed to only one of the processors
> > listed in the destination.
> >
> > This fixes the issue of same interrupt getting delivered to multiple cpu's
> > in the logical flat mode in the presence of interrupt-remapping. While
> > there is no functional issue observed with this behavior, this will
> > impact performance of such configurations (<=8 cpu's using logical flat
> > mode in the presence of interrupt-remapping)
> >
> > Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
> > Cc: Weidong Han <weidong.han@intel.com>
> > Cc: <stable@kernel.org> # [v2.6.32+]
> > ---
> >
> > diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
> > index f1efeba..90f8a75 100644
> > --- a/arch/x86/kernel/apic/io_apic.c
> > +++ b/arch/x86/kernel/apic/io_apic.c
> > @@ -1392,6 +1392,7 @@ int setup_ioapic_entry(int apic_id, int irq,
> >  		irte.dlvry_mode = apic->irq_delivery_mode;
> >  		irte.vector = vector;
> >  		irte.dest_id = IRTE_DEST(destination);
> > +		irte.redir_hint = 1;
> >  
> >  		/* Set source-id of interrupt request */
> >  		set_ioapic_sid(&irte, apic_id);
> > @@ -3343,6 +3344,7 @@ static int msi_compose_msg(struct pci_dev *pdev, unsigned int irq,
> >  		irte.dlvry_mode = apic->irq_delivery_mode;
> >  		irte.vector = cfg->vector;
> >  		irte.dest_id = IRTE_DEST(dest);
> > +		irte.redir_hint = 1;
> >  
> >  		/* Set source-id of interrupt request */
> >  		if (pdev)
> >
> >
> >   
> hi Suresh,
> 
> I think it needs to set redir_hint only for lowest priority delivery 
> mode. Why set it for all delivery modes?

Weidong, For all the IRQ types (irrespective of lowest priority/fixed
mode etc) x86 linux kernel doesn't use irq multi-cast model (which is
what irte.redir_hint == 0 means). So as it is not particularly tied with
lowest priority mode (irte.redir_hint is equally applicable for fixed
mode though we don't use it currently), it makes sense to set it
unconditionally for the current usages.

thanks,
suresh

Re: [patch 1/3] x86, intr-remap: set redirection hint in the IRTE

[Weidong Han]

Siddha, Suresh B wrote:
> On Sun, 2010-08-29 at 18:47 -0700, Han, Weidong wrote:
>   
>> Siddha, Suresh B wrote:
>>     
>>> Currently the redirection hint in the interrupt-remapping table entry
>>> is set to 0, which means the remapped interrupt is directed to the
>>> processors listed in the destination. So in logical flat mode
>>> in the presence of intr-remapping, this results in a single
>>> interrupt multi-casted to multiple cpu's as specified by the destination
>>> bit mask. But what we really want is to send that interrupt to one of the cpus
>>> based on the lowest priority delivery mode.
>>>
>>> Set the redirection hint in the IRTE to '1' to indicate that we want
>>> the remapped interrupt to be directed to only one of the processors
>>> listed in the destination.
>>>
>>> This fixes the issue of same interrupt getting delivered to multiple cpu's
>>> in the logical flat mode in the presence of interrupt-remapping. While
>>> there is no functional issue observed with this behavior, this will
>>> impact performance of such configurations (<=8 cpu's using logical flat
>>> mode in the presence of interrupt-remapping)
>>>
>>> Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
>>> Cc: Weidong Han <weidong.han@intel.com>
>>> Cc: <stable@kernel.org> # [v2.6.32+]
>>> ---
>>>
>>> diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
>>> index f1efeba..90f8a75 100644
>>> --- a/arch/x86/kernel/apic/io_apic.c
>>> +++ b/arch/x86/kernel/apic/io_apic.c
>>> @@ -1392,6 +1392,7 @@ int setup_ioapic_entry(int apic_id, int irq,
>>>  		irte.dlvry_mode = apic->irq_delivery_mode;
>>>  		irte.vector = vector;
>>>  		irte.dest_id = IRTE_DEST(destination);
>>> +		irte.redir_hint = 1;
>>>  
>>>  		/* Set source-id of interrupt request */
>>>  		set_ioapic_sid(&irte, apic_id);
>>> @@ -3343,6 +3344,7 @@ static int msi_compose_msg(struct pci_dev *pdev, unsigned int irq,
>>>  		irte.dlvry_mode = apic->irq_delivery_mode;
>>>  		irte.vector = cfg->vector;
>>>  		irte.dest_id = IRTE_DEST(dest);
>>> +		irte.redir_hint = 1;
>>>  
>>>  		/* Set source-id of interrupt request */
>>>  		if (pdev)
>>>
>>>
>>>   
>>>       
>> hi Suresh,
>>
>> I think it needs to set redir_hint only for lowest priority delivery 
>> mode. Why set it for all delivery modes?
>>     
>
> Weidong, For all the IRQ types (irrespective of lowest priority/fixed
> mode etc) x86 linux kernel doesn't use irq multi-cast model (which is
> what irte.redir_hint == 0 means). So as it is not particularly tied with
> lowest priority mode (irte.redir_hint is equally applicable for fixed
> mode though we don't use it currently), it makes sense to set it
> unconditionally for the current usages.
>
>
>   
Thanks for your explanation. It's ok.

Regards,
Weidong

[tip:x86/x2apic] x86, intr-remap: Set redirection hint in the IRTE

[tip-bot for Suresh Siddha]

Commit-ID:  75e3cfbed6f71a8f151dc6e413b6ce3c390030cb
Gitweb:     http://git.kernel.org/tip/75e3cfbed6f71a8f151dc6e413b6ce3c390030cb
Author:     Suresh Siddha <suresh.b.siddha@intel.com>
AuthorDate: Fri, 27 Aug 2010 11:09:48 -0700
Committer:  H. Peter Anvin <hpa@linux.intel.com>
CommitDate: Wed, 15 Sep 2010 17:36:37 -0700

x86, intr-remap: Set redirection hint in the IRTE

Currently the redirection hint in the interrupt-remapping table entry
is set to 0, which means the remapped interrupt is directed to the
processors listed in the destination. So in logical flat mode
in the presence of intr-remapping, this results in a single
interrupt multi-casted to multiple cpu's as specified by the destination
bit mask. But what we really want is to send that interrupt to one of the cpus
based on the lowest priority delivery mode.

Set the redirection hint in the IRTE to '1' to indicate that we want
the remapped interrupt to be directed to only one of the processors
listed in the destination.

This fixes the issue of same interrupt getting delivered to multiple cpu's
in the logical flat mode in the presence of interrupt-remapping. While
there is no functional issue observed with this behavior, this will
impact performance of such configurations (<=8 cpu's using logical flat
mode in the presence of interrupt-remapping)

Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
LKML-Reference: <20100827181049.013051492@sbsiddha-MOBL3.sc.intel.com>
Cc: Weidong Han <weidong.han@intel.com>
Cc: <stable@kernel.org> # [v2.6.32+]
Signed-off-by: H. Peter Anvin <hpa@linux.intel.com>
---
 arch/x86/kernel/apic/io_apic.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index f1efeba..90f8a75 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -1392,6 +1392,7 @@ int setup_ioapic_entry(int apic_id, int irq,
 		irte.dlvry_mode = apic->irq_delivery_mode;
 		irte.vector = vector;
 		irte.dest_id = IRTE_DEST(destination);
+		irte.redir_hint = 1;
 
 		/* Set source-id of interrupt request */
 		set_ioapic_sid(&irte, apic_id);
@@ -3343,6 +3344,7 @@ static int msi_compose_msg(struct pci_dev *pdev, unsigned int irq,
 		irte.dlvry_mode = apic->irq_delivery_mode;
 		irte.vector = cfg->vector;
 		irte.dest_id = IRTE_DEST(dest);
+		irte.redir_hint = 1;
 
 		/* Set source-id of interrupt request */
 		if (pdev)