LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* RE: [PATCH 1/4] x86_64 io_apic: Implement remove_pin_to_irq
@ 2007-01-08 21:46 Lu, Yinghai
  2007-01-08 22:50 ` [PATCH] x86_64 ioapic: check_timer_pin Don't add_pin_to_irq if it is already there Eric W. Biederman
  0 siblings, 1 reply; 5+ messages in thread
From: Lu, Yinghai @ 2007-01-08 21:46 UTC (permalink / raw)
  To: ebiederm
  Cc: Linus Torvalds, Tobias Diedrich, Andrew Morton, Adrian Bunk,
	Andi Kleen, Linux Kernel Mailing List



-----Original Message-----
From: ebiederm@xmission.com [mailto:ebiederm@xmission.com] 
Sent: Monday, January 08, 2007 1:31 PM
To: Lu, Yinghai
Cc: Linus Torvalds; Tobias Diedrich; Andrew Morton; Adrian Bunk; Andi
Kleen; Linux Kernel Mailing List
Subject: Re: [PATCH 1/4] x86_64 io_apic: Implement remove_pin_to_irq

>Any updates to add_pin_to_irq are wrong.  It works fine.  If there
>is something wrong we need to fix remove_pin_to_irq.

>What is the problem you see?  Sorry I'm dense at the moment.

+static int check_timer_pin(int apic, int pin) {
+	int irq, idx;
+	/* 
+	 * Test the architecture default i8254 timer pin
+	 * of apic 0 pin 2.
+	 */
+
+
+	/* If the apic pin pair is in use by another irq fail */
+	irq = irq_from_pin(apic, pin);
+	if ((irq != -1) && (irq != 0)) {
+		apic_printk(APIC_VERBOSE,KERN_INFO "...apic %d pin % in
use by irq %d\n",
+			apic, pin, irq);
+		return 0; 
+	}
+
+	/* Add an entry in mp_irqs for irq 0 */
+	idx = update_irq0_entry(apic, pin);
+
+	/* Add an entry in irq_to_pin */
+	add_pin_to_irq(0, apic, pin);
+
+	/* Now setup the irq */
+	setup_IO_APIC_irq(apic, pin, idx, 0);
+
+	/* And finally check to see if the irq works */
+	return do_check_timer_pin(apic, pin);
+}
+

In the check_timer_pin, irq_from_pin could return 0, it mean some entry
is for IRQ0 already.
The add_pin_to_irq could add another same entry for it again.

YH



^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH] x86_64 ioapic: check_timer_pin Don't add_pin_to_irq if it is already there.
  2007-01-08 21:46 [PATCH 1/4] x86_64 io_apic: Implement remove_pin_to_irq Lu, Yinghai
@ 2007-01-08 22:50 ` Eric W. Biederman
  0 siblings, 0 replies; 5+ messages in thread
From: Eric W. Biederman @ 2007-01-08 22:50 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Linus Torvalds, Tobias Diedrich, Adrian Bunk, Andi Kleen,
	Linux Kernel Mailing List, Lu, Yinghai

"Lu, Yinghai" <yinghai.lu@amd.com> writes:

>>Any updates to add_pin_to_irq are wrong.  It works fine.  If there
>>is something wrong we need to fix remove_pin_to_irq.
>
>>What is the problem you see?  Sorry I'm dense at the moment.

> In the check_timer_pin, irq_from_pin could return 0, it mean some entry
> is for IRQ0 already.
> The add_pin_to_irq could add another same entry for it again.

Yep.  My oversight.  Here is the trivial patch to fix it.  I don't
see how we could hit this case but if we are going to allow for it
we should handle it correctly.

Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---
 arch/x86_64/kernel/io_apic.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/arch/x86_64/kernel/io_apic.c b/arch/x86_64/kernel/io_apic.c
index 4891959..cc8e9a4 100644
--- a/arch/x86_64/kernel/io_apic.c
+++ b/arch/x86_64/kernel/io_apic.c
@@ -1687,7 +1687,8 @@ static int check_timer_pin(int apic, int pin)
 	idx = update_irq0_entry(apic, pin);
 
 	/* Add an entry in irq_to_pin */
-	add_pin_to_irq(0, apic, pin);
+	if (irq != 0)
+		add_pin_to_irq(0, apic, pin);
 
 	/* Now setup the irq */
 	setup_IO_APIC_irq(apic, pin, idx, 0);
-- 
1.4.4.1.g278f



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/4] x86_64 io_apic: Implement remove_pin_to_irq
  2007-01-08 21:09 [PATCH 1/4] x86_64 io_apic: Implement remove_pin_to_irq Lu, Yinghai
@ 2007-01-08 21:30 ` Eric W. Biederman
  0 siblings, 0 replies; 5+ messages in thread
From: Eric W. Biederman @ 2007-01-08 21:30 UTC (permalink / raw)
  To: Lu, Yinghai
  Cc: Linus Torvalds, Tobias Diedrich, Andrew Morton, Adrian Bunk,
	Andi Kleen, Linux Kernel Mailing List

"Lu, Yinghai" <yinghai.lu@amd.com> writes:

> -----Original Message-----
> From: ebiederm@xmission.com [mailto:ebiederm@xmission.com] 
> Sent: Monday, January 08, 2007 7:50 AM
> To: Linus Torvalds
> Cc: Tobias Diedrich; Lu, Yinghai; Andrew Morton; Adrian Bunk; Andi
> Kleen; Linux Kernel Mailing List
> Subject: [PATCH 1/4] x86_64 io_apic: Implement remove_pin_to_irq
>
> +static void remove_pin_to_irq(unsigned int irq, int apic, int pin)
> +{
> +	struct irq_pin_list *entry = irq_2_pin + irq;
>
> You may need to update add_pin_to_irq to avoid multi entries for irq 0.

Any updates to add_pin_to_irq are wrong.  It works fine.  If there
is something wrong we need to fix remove_pin_to_irq.

What is the problem you see?  Sorry I'm dense at the moment.

I preserve the invariant that irq_2_pin + irq is always the first
entry in the chain.  I do this when I delete a multi chain entry
by copying the next entry over the current entry, and then freeing
(and leaking) the second entry in the chain.

Is there something wrong with that?  I came within an inch of deleting
this multiple apic, pin to irq mapping code but the comments said it
is needed for some ioapic case.  So in resurrecting this variant I may
have goofed somewhere.

Eric

^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [PATCH 1/4] x86_64 io_apic: Implement remove_pin_to_irq
@ 2007-01-08 21:09 Lu, Yinghai
  2007-01-08 21:30 ` Eric W. Biederman
  0 siblings, 1 reply; 5+ messages in thread
From: Lu, Yinghai @ 2007-01-08 21:09 UTC (permalink / raw)
  To: ebiederm, Linus Torvalds
  Cc: Tobias Diedrich, Andrew Morton, Adrian Bunk, Andi Kleen,
	Linux Kernel Mailing List



-----Original Message-----
From: ebiederm@xmission.com [mailto:ebiederm@xmission.com] 
Sent: Monday, January 08, 2007 7:50 AM
To: Linus Torvalds
Cc: Tobias Diedrich; Lu, Yinghai; Andrew Morton; Adrian Bunk; Andi
Kleen; Linux Kernel Mailing List
Subject: [PATCH 1/4] x86_64 io_apic: Implement remove_pin_to_irq

+static void remove_pin_to_irq(unsigned int irq, int apic, int pin)
+{
+	struct irq_pin_list *entry = irq_2_pin + irq;

You may need to update add_pin_to_irq to avoid multi entries for irq 0.

YH



^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/4] x86_64 io_apic: Implement remove_pin_to_irq
  2007-01-08  1:09     ` Linus Torvalds
@ 2007-01-08 15:49       ` Eric W. Biederman
  0 siblings, 0 replies; 5+ messages in thread
From: Eric W. Biederman @ 2007-01-08 15:49 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Tobias Diedrich, Yinghai Lu, Andrew Morton, Adrian Bunk,
	Andi Kleen, Linux Kernel Mailing List


To try different irq routing combinations so we can make an informed
guess as to how to route irqs when the BIOS gets it wrong we need
the ability to modify our irq routing data structures.

This adds remove_pin_to_irq which removes the mapping from and apic pin
to an irq.

Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---
 arch/x86_64/kernel/io_apic.c |   23 +++++++++++++++++++++++
 1 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/arch/x86_64/kernel/io_apic.c b/arch/x86_64/kernel/io_apic.c
index 2a1dcd5..7365f5f 100644
--- a/arch/x86_64/kernel/io_apic.c
+++ b/arch/x86_64/kernel/io_apic.c
@@ -286,6 +286,29 @@ static void add_pin_to_irq(unsigned int irq, int apic, int pin)
 	entry->pin = pin;
 }
 
+static void remove_pin_to_irq(unsigned int irq, int apic, int pin)
+{
+	struct irq_pin_list *entry = irq_2_pin + irq;
+
+	BUG_ON(irq >= NR_IRQS);
+
+	while (entry->next && ((entry->apic != apic) || (entry->pin != pin)))
+		entry = irq_2_pin + entry->next;
+
+	if (entry->pin == apic && entry->pin == pin) {
+		if (entry->next) {
+			struct irq_pin_list *next = irq_2_pin + entry->next;
+			*entry = *next;
+			next->pin = -1;
+			next->apic = -1;
+			next->next = 0;
+		} else {
+			entry->pin = -1;
+			entry->apic = -1;
+		}
+	}
+}
+
 
 #define DO_ACTION(name,R,ACTION, FINAL)					\
 									\
-- 
1.4.4.1.g278f


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2007-01-08 22:50 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-01-08 21:46 [PATCH 1/4] x86_64 io_apic: Implement remove_pin_to_irq Lu, Yinghai
2007-01-08 22:50 ` [PATCH] x86_64 ioapic: check_timer_pin Don't add_pin_to_irq if it is already there Eric W. Biederman
  -- strict thread matches above, loose matches on Subject: below --
2007-01-08 21:09 [PATCH 1/4] x86_64 io_apic: Implement remove_pin_to_irq Lu, Yinghai
2007-01-08 21:30 ` Eric W. Biederman
     [not found] <5986589C150B2F49A46483AC44C7BCA490733F@ssvlexmb2.amd.com>
2007-01-03  6:23 ` 2.6.20-rc3: known unfixed regressions - x86_64 boot failure: "IO-APIC + timer doesn't work" Yinghai Lu
2007-01-08  0:55   ` Tobias Diedrich
2007-01-08  1:09     ` Linus Torvalds
2007-01-08 15:49       ` [PATCH 1/4] x86_64 io_apic: Implement remove_pin_to_irq Eric W. Biederman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).