Re: [patch 01/15] x86/irq: Convey vector as argument and not in ptregs

From: Brian Gerst <brgerst@gmail.com>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: LKML <linux-kernel@vger.kernel.org>,
	"the arch/x86 maintainers" <x86@kernel.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	Juergen Gross <jgross@suse.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Arnd Bergmann <arnd@arndb.de>
Subject: Re: [patch 01/15] x86/irq: Convey vector as argument and not in ptregs
Date: Wed, 26 Feb 2020 16:54:33 -0500	[thread overview]
Message-ID: <CAMzpN2j7EHZ2bKg9SZ2Ri-qsmEoknAAJO6O5yoLn-fY8_h1B2A@mail.gmail.com> (raw)
In-Reply-To: <87k149p0na.fsf@nanos.tec.linutronix.de>

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