Messages in this thread | | | Date | Wed, 11 Feb 2015 21:17:07 -0500 | From | Steven Rostedt <> | Subject | Re: [PATCH 08/11] x86: entry_64.S: fold test_in_nmi macro into its only user |
| |
On Wed, 11 Feb 2015 12:40:36 -0800 Andy Lutomirski <luto@amacapital.net> wrote:
> On Wed, Jan 14, 2015 at 1:48 PM, Denys Vlasenko <dvlasenk@redhat.com> wrote: > > No code changes. > > Steven, is this okay with you?
Not as is. The reason I created the macro in the first place was to give names to values and labels. That is:
- .macro test_in_nmi reg stack nmi_ret normal_ret - cmpq %\reg, \stack - ja \normal_ret - subq $EXCEPTION_STKSZ, %\reg - cmpq %\reg, \stack - jb \normal_ret - jmp \nmi_ret
Shows you that we are comparing the reg to a given stack. And it also shows us where to jump as a result (normal_ret vs nmi_ret).
We lose that with:
+ cmpq %rdx, 4*8(%rsp) + ja first_nmi + subq $EXCEPTION_STKSZ, %rdx + cmpq %rdx, 4*8(%rsp) + jb first_nmi + jmp nested_nmi
I'm not against the change. I would like to see some descriptive comments along with it. Like adding:
/* Compare the NMI stack (rdx) with the stack we came from (4*8(%rsp)) */ cmpq %rdx, 4*8(%rsp) /* If the stack pointer is above the NMI stack, this is a normal NMI */ ja first_nmi subq $EXCEPTION_STKSZ, %rdx cmpq %rdx, 4*8(%rsp) /* If it is below the NMI stack, it is a normal NMI */ jb first_nmi /* Ah, it is within the NMI stack, treat it as nested */ jmp nested_nmi
-- Steve
| |