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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 24.11.16 at 22:44, <daniel.kiper@xxxxxxxxxx> wrote:
> 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.

Which by itself is suspicious, as the dependency issue had been
introduced only in (iirc) Pentium4. And anyway, this is a general
recommendation of theirs, i.e. something to consider when
intending to run well everywhere. Such general recommendation
by their very nature don't consider model specific differences.

> 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.

I agree, and I hope we can get Andrew to agree too.

Jan


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