Re: [PATCH v10 04/11] x86/entry/64: Adapt assembly for PIE support

[Peter Zijlstra]

On Thu, Dec 05, 2019 at 09:01:50AM -0800, Thomas Garnier wrote:
> On Thu, Dec 5, 2019 at 1:04 AM Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
> > On Wed, Dec 04, 2019 at 04:09:41PM -0800, Thomas Garnier wrote:
> >
> > > @@ -1625,7 +1627,11 @@ first_nmi:
> > >       addq    $8, (%rsp)      /* Fix up RSP */
> > >       pushfq                  /* RFLAGS */
> > >       pushq   $__KERNEL_CS    /* CS */
> > > -     pushq   $1f             /* RIP */
> > > +     pushq   $0              /* Future return address */
> >
> > We're building an IRET frame, the IRET frame does not have a 'future
> > return address' field.
> 
> I assumed that's the target RIP after iretq.

It is. But it's still the (R)IP field of the IRET frame. Calling it
anything else is just confusing. The frame is 5 words: SS, (R)SP, (R)FLAGS,
CS, (R)IP.

> > > +     pushq   %rdx            /* Save RAX */
> > > +     leaq    1f(%rip), %rdx  /* RIP */
> >
> > nonsensical comment
> 
> That was the same comment from the push $1f that I changed.

Yes, but there it made sense since the PUSH actually created that field
of the frame, here it is nonsensical. What this instruction does is put
the address of the '1f' label into RDX, which is then stuck into the
(R)IP field on the next instruction.

> > > +     movq    %rdx, 8(%rsp)   /* Put 1f on return address */
> > > +     popq    %rdx            /* Restore RAX */