]>
Commit | Line | Data |
---|---|---|
6a930a95 BS |
1 | From: Jay Lan <jlan@sgi.com> |
2 | Subject: [PATCH] Resolve KDB conflicts with UV | |
3 | References: bnc#440376 | |
4 | ||
5 | Hi Keith, | |
6 | ||
7 | On Wed, Oct 29, 2008 at 03:57:25PM +1100, Keith Owens wrote: | |
8 | > However there is a separate problem with your patch. You now wait in | |
9 | > smp_kdb_stop() until all cpus are in KDB. If any cpu is completely | |
10 | > hung so it cannot be interrupted then smp_kdb_stop() will never return | |
11 | > and KDB will now appear to hang. | |
12 | > | |
13 | > The existing code avoids this by | |
14 | > | |
15 | > kdb() -> smp_kdb_stop() - issue KDB_VECTOR as normal interrupt but do not wait for cpus | |
16 | > kdb() -> kdba_main_loop() | |
17 | > kdba_main_loop() -> kdb_save_running() | |
18 | > kdb_save_running() -> kdb_main_loop() | |
19 | > kdb_main_loop() -> kdb_wait_for_cpus() | |
20 | > | |
21 | > kdb_wait_for_cpus() waits until the other cpus are in KDB. If a cpu | |
22 | > does not respond to KDB_VECTOR after a few seconds then | |
23 | > kdb_wait_for_cpus() hits the missing cpus with NMI. | |
24 | > | |
25 | > This two step approach (send KDB_VECTOR as normal interrupt, wait then | |
26 | > send NMI) is used because NMI can be serviced at any time, even when | |
27 | > the target cpu is in the middle of servicing an interrupt. This can | |
28 | > result in incomplete register state which leads to broken backtraces. | |
29 | > IOW, sending NMI first would actually make debugging harder. | |
30 | > | |
31 | > Given the above logic, if you are going to take over an existing | |
32 | > interrupt vector then the vector needs to be acquired near the start of | |
33 | > kdb() and released near the end of kdb(), and only on the master cpu. | |
34 | > | |
35 | > Note: there is no overwhelming need for KDB_VECTOR to have a high | |
36 | > priority. As long as it is received within a few seconds then all is | |
37 | > well. | |
38 | ||
39 | Thanks for the explanation. I see your point. | |
40 | ||
41 | How about if we keep the two step approach, but take over the vector | |
42 | when we need it, in step one. Then give it back when the step two | |
43 | wait is over. | |
44 | (assuming we don't take over a vector needed for the NMI) | |
45 | ||
46 | Like this: | |
47 | ||
48 | Signed-off-by: Jay Lan <jlan@sgi.com> | |
49 | Acked-by: Bernhard Walle <bwalle@suse.de> | |
50 | ||
51 | --- | |
52 | arch/ia64/include/asm/kdb.h | 4 ++++ | |
53 | arch/x86/kdb/kdbasupport_32.c | 22 ++++++++++++++++++---- | |
54 | arch/x86/kdb/kdbasupport_64.c | 23 +++++++++++++++++++---- | |
55 | include/asm-x86/irq_vectors.h | 11 ++++++----- | |
56 | include/asm-x86/kdb.h | 4 ++++ | |
57 | kdb/kdbmain.c | 2 ++ | |
58 | 6 files changed, 53 insertions(+), 13 deletions(-) | |
59 | ||
60 | --- a/arch/ia64/include/asm/kdb.h | |
61 | +++ b/arch/ia64/include/asm/kdb.h | |
62 | @@ -42,4 +42,8 @@ kdba_funcptr_value(void *fp) | |
63 | return *(unsigned long *)fp; | |
64 | } | |
65 | ||
66 | +#ifdef CONFIG_SMP | |
67 | +#define kdba_giveback_vector(vector) (0) | |
68 | +#endif | |
69 | + | |
70 | #endif /* !_ASM_KDB_H */ | |
71 | --- a/arch/x86/kdb/kdbasupport_32.c | |
72 | +++ b/arch/x86/kdb/kdbasupport_32.c | |
73 | @@ -883,9 +883,6 @@ kdba_cpu_up(void) | |
74 | static int __init | |
75 | kdba_arch_init(void) | |
76 | { | |
77 | -#ifdef CONFIG_SMP | |
78 | - set_intr_gate(KDB_VECTOR, kdb_interrupt); | |
79 | -#endif | |
80 | set_intr_gate(KDBENTER_VECTOR, kdb_call); | |
81 | return 0; | |
82 | } | |
83 | @@ -1027,14 +1024,31 @@ kdba_verify_rw(unsigned long addr, size_ | |
84 | ||
85 | #include <mach_ipi.h> | |
86 | ||
87 | +gate_desc save_idt[NR_VECTORS]; | |
88 | + | |
89 | +void kdba_takeover_vector(int vector) | |
90 | +{ | |
91 | + memcpy(&save_idt[vector], &idt_table[vector], sizeof(gate_desc)); | |
92 | + set_intr_gate(KDB_VECTOR, kdb_interrupt); | |
93 | + return; | |
94 | +} | |
95 | + | |
96 | +void kdba_giveback_vector(int vector) | |
97 | +{ | |
98 | + native_write_idt_entry(idt_table, vector, &save_idt[vector]); | |
99 | + return; | |
100 | +} | |
101 | + | |
102 | /* When first entering KDB, try a normal IPI. That reduces backtrace problems | |
103 | * on the other cpus. | |
104 | */ | |
105 | void | |
106 | smp_kdb_stop(void) | |
107 | { | |
108 | - if (!KDB_FLAG(NOIPI)) | |
109 | + if (!KDB_FLAG(NOIPI)) { | |
110 | + kdba_takeover_vector(KDB_VECTOR); | |
111 | send_IPI_allbutself(KDB_VECTOR); | |
112 | + } | |
113 | } | |
114 | ||
115 | /* The normal KDB IPI handler */ | |
116 | --- a/arch/x86/kdb/kdbasupport_64.c | |
117 | +++ b/arch/x86/kdb/kdbasupport_64.c | |
118 | @@ -21,6 +21,7 @@ | |
119 | #include <linux/interrupt.h> | |
120 | #include <linux/module.h> | |
121 | #include <linux/kdebug.h> | |
122 | +#include <linux/cpumask.h> | |
123 | #include <asm/processor.h> | |
124 | #include <asm/msr.h> | |
125 | #include <asm/uaccess.h> | |
126 | @@ -900,9 +901,6 @@ kdba_cpu_up(void) | |
127 | static int __init | |
128 | kdba_arch_init(void) | |
129 | { | |
130 | -#ifdef CONFIG_SMP | |
131 | - set_intr_gate(KDB_VECTOR, kdb_interrupt); | |
132 | -#endif | |
133 | set_intr_gate(KDBENTER_VECTOR, kdb_call); | |
134 | return 0; | |
135 | } | |
136 | @@ -976,14 +974,31 @@ kdba_set_current_task(const struct task_ | |
137 | ||
138 | #include <mach_ipi.h> | |
139 | ||
140 | +gate_desc save_idt[NR_VECTORS]; | |
141 | + | |
142 | +void kdba_takeover_vector(int vector) | |
143 | +{ | |
144 | + memcpy(&save_idt[vector], &idt_table[vector], sizeof(gate_desc)); | |
145 | + set_intr_gate(KDB_VECTOR, kdb_interrupt); | |
146 | + return; | |
147 | +} | |
148 | + | |
149 | +void kdba_giveback_vector(int vector) | |
150 | +{ | |
151 | + native_write_idt_entry(idt_table, vector, &save_idt[vector]); | |
152 | + return; | |
153 | +} | |
154 | + | |
155 | /* When first entering KDB, try a normal IPI. That reduces backtrace problems | |
156 | * on the other cpus. | |
157 | */ | |
158 | void | |
159 | smp_kdb_stop(void) | |
160 | { | |
161 | - if (!KDB_FLAG(NOIPI)) | |
162 | + if (!KDB_FLAG(NOIPI)) { | |
163 | + kdba_takeover_vector(KDB_VECTOR); | |
164 | send_IPI_allbutself(KDB_VECTOR); | |
165 | + } | |
166 | } | |
167 | ||
168 | /* The normal KDB IPI handler */ | |
169 | --- a/include/asm-x86/irq_vectors.h | |
170 | +++ b/include/asm-x86/irq_vectors.h | |
171 | @@ -66,7 +66,6 @@ | |
172 | # define RESCHEDULE_VECTOR 0xfc | |
173 | # define CALL_FUNCTION_VECTOR 0xfb | |
174 | # define CALL_FUNCTION_SINGLE_VECTOR 0xfa | |
175 | -#define KDB_VECTOR 0xf9 | |
176 | # define THERMAL_APIC_VECTOR 0xf0 | |
177 | ||
178 | #else | |
179 | @@ -79,10 +78,6 @@ | |
180 | #define THERMAL_APIC_VECTOR 0xfa | |
181 | #define THRESHOLD_APIC_VECTOR 0xf9 | |
182 | #define UV_BAU_MESSAGE 0xf8 | |
183 | -/* Overload KDB_VECTOR with UV_BAU_MESSAGE. By the time the UV hardware is | |
184 | - * ready, we should have moved to a dynamically allocated vector scheme. | |
185 | - */ | |
186 | -#define KDB_VECTOR 0xf8 | |
187 | #define INVALIDATE_TLB_VECTOR_END 0xf7 | |
188 | #define INVALIDATE_TLB_VECTOR_START 0xf0 /* f0-f7 used for TLB flush */ | |
189 | ||
190 | @@ -91,6 +86,12 @@ | |
191 | #endif | |
192 | ||
193 | /* | |
194 | + * KDB_VECTOR will take over vector 0xfe when it is needed, as in theory | |
195 | + * it should not be used anyway. | |
196 | + */ | |
197 | +#define KDB_VECTOR 0xfe | |
198 | + | |
199 | +/* | |
200 | * Local APIC timer IRQ vector is on a different priority level, | |
201 | * to work around the 'lost local interrupt if more than 2 IRQ | |
202 | * sources per level' errata. | |
203 | --- a/include/asm-x86/kdb.h | |
204 | +++ b/include/asm-x86/kdb.h | |
205 | @@ -131,4 +131,8 @@ kdba_funcptr_value(void *fp) | |
206 | return (unsigned long)fp; | |
207 | } | |
208 | ||
209 | +#ifdef CONFIG_SMP | |
210 | +extern void kdba_giveback_vector(int); | |
211 | +#endif | |
212 | + | |
213 | #endif /* !_ASM_KDB_H */ | |
214 | --- a/kdb/kdbmain.c | |
215 | +++ b/kdb/kdbmain.c | |
216 | @@ -1666,6 +1666,8 @@ kdb_wait_for_cpus(void) | |
217 | wait == 1 ? " is" : "s are", | |
218 | wait == 1 ? "its" : "their"); | |
219 | } | |
220 | + /* give back the vector we took over in smp_kdb_stop */ | |
221 | + kdba_giveback_vector(KDB_VECTOR); | |
222 | #endif /* CONFIG_SMP */ | |
223 | } | |
224 |