Re: [RFC PATCH 1/2] x86: Fix rbp saving in pt_regs on irq entry

Frederic Weisbecker
2011-01-06T17:12:42Z

On Thu, Jan 06, 2011 at 04:58:54PM +0000, Jan Beulich wrote:
> >>> On 06.01.11 at 17:54, Frederic Weisbecker <fweisbec@gmail.com> wrote:
> > On Thu, Jan 06, 2011 at 04:39:39PM +0000, Jan Beulich wrote:
> >> >>> On 06.01.11 at 17:22, Frederic Weisbecker <fweisbec@gmail.com> wrote:
> >> > On Thu, Jan 06, 2011 at 04:10:55PM +0000, Jan Beulich wrote:
> >> >> >>> On 06.01.11 at 16:45, Frederic Weisbecker <fweisbec@gmail.com> wrote:
> >> >> > Before we had:
> >> >> >
> >> >> >
> >> >> > leaveq
> >> >> >
> >> >> > CFI_RESTORE rbp
> >> >> > CFI_DEF_CFA_REGISTER rsp
> >> >> > CFI_ADJUST_CFA_OFFSET -8
> >> >> >
> >> >> > So CFI_RESTORE means rbp has now the value of the base frame of
> >> >> > the calling frame (the base frame pointer of the interrupted proc) ?
> >> >>
> >> >> No - all it means is that %rbp now has its original (caller or
> >> >> interrupted procedure) value again (i.e. an unwinder should not
> >> >> try to read it from the stack [or other previously recorded
> >> >> location] anymore).
> >> >>
> >> >> > And what follows means that rsp-8 points to the return address?
> >> >>
> >> >> No - .cfi_def_cfa_register says which register serves as the frame
> >> >> pointer, and .cfi_adjust_cfa_offset says to adjust the offset from
> >> >> the frame pointer to the top [or bottom] of frame. At any time
> >> >>
> >> >> CFA = cfa_register + cfa_offset
> >> >>
> >> >> with CFA being what all locations on the stack are expressed
> >> >> relative to.
> >> >
> >> > Ok.
> >> >
> >> > So here rsp points to pt_regs::r11
> >> >
> >> > I don't understand why locations relative to the stack must be
> >> > expressed here by taking rsp - 8 as a base.
> >>
> >> Nothing says rsp-8. The annotations merely say to set the base
> >> register to rsp and to *adjust* the offset by -8 (after all, that's
> >> what the leaveq instruction does).
> >
> > Ah! So CFA acts like a virtual frame base pointer right?
>
> Correct.

Ah great. I was starting to prepare for the case you come to stab me :)

So what do you think about that:

leaveq

CFI_RESTORE rbp
CFI_DEF_CFA_REGISTER rsp
CFI_ADJUST_CFA_OFFSET -8

/* we did not save rbx, restore only from ARGOFFSET */
addq $8, %rsp
CFI_ADJUST_CFA_OFFSET -16

Does that look correct to you? We increased rsp to start recovering
the regs from the right place, but the frame pointer of the current
proc must stay what it was.

Now I don't understand how this is all useful as this is not a normal
proc but an interruption. We can't get back the return address from
the CFA. Or am I missing something?