Re: [Xen-devel] [PATCH v9 07/13] x86: add multiboot2 protocol support for EFI platforms

[Daniel Kiper <daniel.kiper@xxxxxxxxxx>]

On Thu, Nov 24, 2016 at 04:08:12AM -0700, Jan Beulich wrote:
> >>> On 23.11.16 at 19:52, <andrew.cooper3@xxxxxxxxxx> wrote:
> > On 29/09/16 22:42, Daniel Kiper wrote:

[...]

> >> +.Lefi_mb2_tsize:
> >> +        /* Check Multiboot2 information total size. */
> >> +        mov     %ecx,%r8d
> >> +        sub     %ebx,%r8d
> >> +        cmp     %r8d,MB2_fixed_total_size(%rbx)
> >> +        jbe     run_bs
> >> +
> >> +        /* Are EFI boot services available? */
> >> +        cmpl    $MULTIBOOT2_TAG_TYPE_EFI_BS,MB2_tag_type(%rcx)
> >> +        jne     .Lefi_mb2_st
> >> +
> >> +        /*
> >> +         * Yes, store that info in skip_realmode variable. I agree that
> >> +         * its name is a bit unfortunate in this context, however, by the
> >> +         * way we disable real mode and other legacy stuff which should
> >> +         * not be run on EFI platforms.
> >> +         */
> >> +        incb    skip_realmode(%rip)
> >
> > Always use add/sub 1 in preference to inc and dec.  They are the same
> > length to encode in 64bit, and avoids a pipeline stall from a merge of
> > the eflags register.
>
> What you say regarding length not true - add/sub need to encode
> the immediate somewhere (even if the operand was a register,
> inc/dec would still be smaller than add/sub, just not by as much as
> in 32-bit code). And the pipeline stall, afaik, affects only rather old
> processors.

Intel 64 and IA-32 Architectures Optimization Reference Manual, section
3.5.1.1, Use of the INC and DEC Instructions says nothing about exceptions.
So, it looks that this applies to all x86 CPUs. However, as I said earlier,
I am not convinced that we should consider such nuances here. incb reads
better at least for me.

Daniel

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