From: H. Peter Anvin on 8 Jan 2010 21:30 On 01/08/2010 06:09 PM, Suresh Siddha wrote: > > So change the IRQ_MOVE_CLEANUP_VECTOR to 0x20 and allow 0x21-0x2f to be used > for device interrupts. 0x30-0x3f will be used for ISA interrupts (these > also can be migrated in the context of IOAPIC and hence need to be at a higher > priority level than IRQ_MOVE_CLEANUP_VECTOR). > You're referring to when they're accessed as IOAPIC interrupts as opposed to ExtInt interrupts? > > -/* > - * First APIC vector available to drivers: (vectors 0x30-0xee). We > - * start allocating at 0x31 to spread out vectors evenly between > - * priority levels. (0x80 is the syscall vector) > - */ > -#define FIRST_DEVICE_VECTOR (IRQ15_VECTOR + 1) > -#define VECTOR_OFFSET_START 1 > - > #define NR_VECTORS 256 > > #define FPU_IRQ 13 > diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c > index d5bfa29..5c090a1 100644 > --- a/arch/x86/kernel/apic/io_apic.c > +++ b/arch/x86/kernel/apic/io_apic.c > @@ -1162,8 +1162,8 @@ __assign_irq_vector(int irq, struct irq_cfg *cfg, const struct cpumask *mask) > * Also, we've got to be careful not to trash gate > * 0x80, because int 0x80 is hm, kind of importantish. ;) > */ > - static int current_vector = FIRST_DEVICE_VECTOR + VECTOR_OFFSET_START; > - static int current_offset = VECTOR_OFFSET_START % 8; > + static int current_vector = FIRST_DEVICE_VECTOR; > + static int current_offset = 0; > unsigned int old_vector; > int cpu, err; > cpumask_var_t tmp_mask; > I'm not entirely sure I like losing this bit, even though it isn't really necessary with your changes (VECTOR_OFFSET_START would be 0). I'm afraid we might end up with the same buglet being "reinvented" later. However, my most serious concern with this patch is that there is a fairly significant change due to this patch, which is that the legacy IRQ vectors now fall *inside* the FIRST_DEVICE_VECTOR range. This isn't a bad thing -- in fact, it is fundamentally the right thing to do especially once we consider platforms which *don't* have the legacy IRQs -- but it makes me scared of unexpected behavior changes as a result. If you feel confident that that is not the case, could you outline why it shouldn't be a problem? -hpa -- H. Peter Anvin, Intel Open Source Technology Center I work for Intel. I don't speak on their behalf. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: H. Peter Anvin on 8 Jan 2010 22:40 On 01/08/2010 06:09 PM, Suresh Siddha wrote: > From: Suresh Siddha <suresh.b.siddha(a)intel.com> > Subject: x86, apic: use 0x20 for the IRQ_MOVE_CLEANUP_VECTOR instead of 0x1f > > After talking to some more folks inside intel (Peter Anvin, Asit Mallick), > the safest option (for future compatibility etc) seen was to use vector 0x20 > for IRQ_MOVE_CLEANUP_VECTOR instead of using vector 0x1f (which is documented as > reserved vector in the Intel IA32 manuals). > > Also we don't need to reserve the entire privilege level (all 16 vectors in > the priority bucket that IRQ_MOVE_CLEANUP_VECTOR falls into), as the > x86 architecture (section 10.9.3 in SDM Vol3a) specifies that with in the > priority level, the higher the vector number the higher the priority. > And hence we don't need to reserve the complete priority level 0x20-0x2f for > the IRQ migration cleanup logic. > > So change the IRQ_MOVE_CLEANUP_VECTOR to 0x20 and allow 0x21-0x2f to be used > for device interrupts. 0x30-0x3f will be used for ISA interrupts (these > also can be migrated in the context of IOAPIC and hence need to be at a higher > priority level than IRQ_MOVE_CLEANUP_VECTOR). > I guess another thing is whether or not we can allocate 0x20 as a normal vector if we don't need IRQ_MOVE_CLEANUP_VECTORS, and rely on the used_vectors for that one as well. -hpa -- H. Peter Anvin, Intel Open Source Technology Center I work for Intel. I don't speak on their behalf. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: H. Peter Anvin on 11 Jan 2010 18:00 On 01/11/2010 02:53 PM, Suresh Siddha wrote: > >> However, my most serious concern with this patch is that there is a >> fairly significant change due to this patch, which is that the legacy >> IRQ vectors now fall *inside* the FIRST_DEVICE_VECTOR range. This isn't >> a bad thing -- in fact, it is fundamentally the right thing to do >> especially once we consider platforms which *don't* have the legacy IRQs >> -- but it makes me scared of unexpected behavior changes as a result. >> If you feel confident that that is not the case, could you outline why >> it shouldn't be a problem? > > In irqinit.c, we statically pre-assign the per-cpu vector to irq > mappings (vector_irq) for all the legacy IRQ vectors. Similarly irq_cfg > is statically initialized for legacy IRQ's in io_apic.c. So we won't be > able to use this space for anything else. > What enforces that, though? The used_vector bitmap? In the past it was enforced simply by being < FIRST_DEVICE_VECTOR. -hpa -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: H. Peter Anvin on 11 Jan 2010 18:10 On 01/11/2010 03:00 PM, Eric W. Biederman wrote: >> >> In irqinit.c, we statically pre-assign the per-cpu vector to irq >> mappings (vector_irq) for all the legacy IRQ vectors. Similarly irq_cfg >> is statically initialized for legacy IRQ's in io_apic.c. So we won't be >> able to use this space for anything else. > > Last I looked when we switched legacy irqs irqs from pic mode to io_apic mode > we continue to use the same vector. > > Which means we should not be wasting those vectors and that it is dangerous > to play lowest priority irq games in that range. > This statement made no sense to me? -hpa -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: H. Peter Anvin on 11 Jan 2010 18:20 On 01/11/2010 03:10 PM, Eric W. Biederman wrote: > "H. Peter Anvin" <hpa(a)zytor.com> writes: > >> On 01/11/2010 02:53 PM, Suresh Siddha wrote: >>> >>>> However, my most serious concern with this patch is that there is a >>>> fairly significant change due to this patch, which is that the legacy >>>> IRQ vectors now fall *inside* the FIRST_DEVICE_VECTOR range. This isn't >>>> a bad thing -- in fact, it is fundamentally the right thing to do >>>> especially once we consider platforms which *don't* have the legacy IRQs >>>> -- but it makes me scared of unexpected behavior changes as a result. >>>> If you feel confident that that is not the case, could you outline why >>>> it shouldn't be a problem? >>> >>> In irqinit.c, we statically pre-assign the per-cpu vector to irq >>> mappings (vector_irq) for all the legacy IRQ vectors. Similarly irq_cfg >>> is statically initialized for legacy IRQ's in io_apic.c. So we won't be >>> able to use this space for anything else. >>> >> >> What enforces that, though? The used_vector bitmap? In the past it was >> enforced simply by being < FIRST_DEVICE_VECTOR. > > I believe historically it was simply that we did not loop over that set of vectors, > in assign_irq_vector. > Yes, that's what I said. My question was to Suresh what enforces that in the case of his patch, which moves the legacy range into the middle of the device vectors. -hpa -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
|
Next
|
Last
Pages: 1 2 3 Prev: [PATCH 1/6] NOMMU: Fix SYSV SHM for NOMMU Next: trouble compiling dwalker's for-next tree |