Messages in this thread | | | From | Brian Gerst <> | Date | Wed, 26 Feb 2020 16:54:33 -0500 | Subject | Re: [patch 01/15] x86/irq: Convey vector as argument and not in ptregs |
| |
On Wed, Feb 26, 2020 at 3:13 PM Thomas Gleixner <tglx@linutronix.de> wrote: > > Brian Gerst <brgerst@gmail.com> writes: > > > On Tue, Feb 25, 2020 at 6:26 PM Thomas Gleixner <tglx@linutronix.de> wrote: > >> > >> Device interrupts which go through do_IRQ() or the spurious interrupt > >> handler have their separate entry code on 64 bit for no good reason. > >> > >> Both 32 and 64 bit transport the vector number through ORIG_[RE]AX in > >> pt_regs. Further the vector number is forced to fit into an u8 and is > >> complemented and offset by 0x80 for historical reasons. > > > > The reason for the 0x80 offset is so that the push instruction only > > takes two bytes. This allows each entry stub to be packed into a > > fixed 8 bytes. idt_setup_apic_and_irq_gates() assumes this 8-byte > > fixed length for the stubs, so now every odd vector after 0x80 is > > broken. > > > > 508: 6a 7f pushq $0x7f > > 50a: e9 f1 08 00 00 jmpq e00 <common_interrupt> > > 50f: 90 nop > > 510: 68 80 00 00 00 pushq $0x80 > > 515: e9 e6 08 00 00 jmpq e00 <common_interrupt> > > 51a: 66 0f 1f 44 00 00 nopw 0x0(%rax,%rax,1) > > 520: 68 81 00 00 00 pushq $0x81 > > 525: e9 d6 08 00 00 jmpq e00 <common_interrupt> > > 52a: 66 0f 1f 44 00 00 nopw 0x0(%rax,%rax,1) > > > > The 0x81 vector should start at 0x518, not 0x520. > > Bah, I somehow missed that big fat comment explaining it. :) > > Thanks for catching it. So my testing just has been lucky to not hit one > of those. > > Now the question is whether we care about the packed stubs or just make > them larger by using alignment to get rid of this silly +0x80 and > ~vector fixup later on. The straight forward thing clearly has its charm > and I doubt it matters in measurable ways.
I think we can get rid of the inversion. That was done so orig_ax had a negative number (signifying it's not a syscall), but if you replace it with -1 that isn't necessary. A simple -0x80 offset should be sufficient.
I think it's a worthy optimization to keep. There are 240 of these stubs, so increasing the allocation to 16 bytes would add 1920 bytes to the kernel text.
-- Brian Gerst
| |