Re: [Xen-devel] [PATCH 1 of 1] x86/kexec: Change NMI and MCE handling on kexec path

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 06.12.12 at 18:17, Mats Petersson <mats.petersson@xxxxxxxxxx> wrote:
> On 06/12/12 15:56, Andrew Cooper wrote:
>> +/* Enable NMIs.  No special register assumptions, and all preserved. */
>> +ENTRY(enable_nmis)
>> +        pushq %rax
>> +        movq  %rsp, %rax /* Grab RSP before pushing */
>> +
>> +        /* Set up stack frame */
>> +        pushq $0               /* SS */
>> +        pushq %rax             /* RSP */
>> +        pushfq                 /* RFLAGS */
>> +        pushq $__HYPERVISOR_CS /* CS */
>> +        leaq  1f(%rip),%rax
>> +        pushq %rax             /* RIP */
> I did see Jan's comment to add "trap_nop" to this section of code, and I 
> do agree that saving 14 bytes by not having a single iret in it's own 
> aligned block is a good thing, but how about:
> 
> +        /* Set up stack frame */
> +        pushq $0               /* SS */
> +        pushq %rax             /* RSP */
> +        pushfq                 /* RFLAGS */
> +        pushq $__HYPERVISOR_CS /* CS */
> +        leaq  1f(%rip),%rax
> +        pushq %rax             /* RIP */
> +        iretq /* Disable the hardware NMI latch */
> +1:
> +        popq %rax
> +        retq
> +
> +/* No op trap handler.  Required for kexec crash path.
> + * It is not used in performance critical code, and saves having a single
> + * lone aligned iret. It does not use ENTRY to declare the symbol to avoid 
> the
> + * explicit alignment. */
> +.globl trap_nop;
> +trap_nop:
> +
> +        iretq /* Disable the hardware NMI latch */
> 
> 
> We still have the benefit of not wasting 14 bytes on aligning a single 
> iretq, but the code to "do a iretq to enable nmi" is a bit cleaner.

I'd be okay with that too (with the stray blank line and ; removed),
but personally I prefer the version as sent by Andrew.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel