Re: [Xen-devel] [PATCH v7 08/14] x86: add multiboot2 protocol support for EFI platforms

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 27.09.16 at 20:21, <daniel.kiper@xxxxxxxxxx> wrote:
> On Mon, Sep 26, 2016 at 09:12:40AM -0600, Jan Beulich wrote:
>> >>> On 26.09.16 at 16:40, <andrew.cooper3@xxxxxxxxxx> wrote:
>> > On 26/09/16 15:33, Jan Beulich wrote:
>> >>>>> On 26.09.16 at 16:19, <andrew.cooper3@xxxxxxxxxx> wrote:
>> >>> On 23/09/16 22:47, Daniel Kiper wrote:
>> >>>> +        /*
>> >>>> +         * Initialize BSS (no nasty surprises!).
>> >>>> +         * It must be done earlier than in BIOS case
>> >>>> +         * because efi_multiboot2() touches it.
>> >>>> +         */
>> >>>> +        lea     .startof.(.bss)(%rip),%edi
>> >>>> +        mov     $.sizeof.(.bss),%ecx
>> >>> Sorry, but you cannot use this syntax, for the same reasons why I won't
>> >>> accept Jan's patch making similar changes elsewhere.
>> >>>
>> >>> Amongst other issues, you will break the Clang build with it.
>> >> Did you verify meanwhile that this doesn't work with llvm?
>> >
>> > Yes.
>> >
>> > andrewcoop@andrewcoop:/local/xen.git/xen$ cat foo.c
>> > #include <stdio.h>
>> >
>> > static unsigned int x;
>> >
>> > int main(void)
>> > {
>> >     asm volatile("lea .startof.(.bss)(%%rip), %%rdi;"
>> >                  "mov $.sizeof.(.bss), %%rcx;"
>> >                  "mov $-1, %%rax;"
>> >                  "rep stosb;"
>> >                  ::: "memory", "rax", "rcx", "rdi");
>> >     printf("x: %#x\n", x);
>> > }
>> > andrewcoop@andrewcoop:/local/xen.git/xen$ gcc foo.c -o foo && ./foo
>> > x: 0xffffffff
>> > andrewcoop@andrewcoop:/local/xen.git/xen$ clang-3.8 foo.c -o foo && ./foo
>> > foo.c:7:18: error: unexpected token in memory operand
>> >     asm volatile("lea .startof.(.bss)(%%rip), %%rdi;"
>> >                  ^
>> > <inline asm>:1:16: note: instantiated into assembly here
>> >         lea .startof.(.bss)(%rip), %rdi;mov $.sizeof.(.bss), %rcx;mov
>> > $-1, %rax;rep stosb;
>> >                       ^
>> > foo.c:7:18: error: unexpected token in argument list
>> >     asm volatile("lea .startof.(.bss)(%%rip), %%rdi;"
>> >                  ^
>> > <inline asm>:1:47: note: instantiated into assembly here
>> >         lea .startof.(.bss)(%rip), %rdi;mov $.sizeof.(.bss), %rcx;mov
>> > $-1, %rax;rep stosb;
>> >                                                      ^
>> > 2 errors generated.
>>
>> No, that's inline assembly, which does not match Daniel's use
>> case. That's why I referred to llvm, as with assembly sources it
>> should only be the linker which gets to see the symbols generated
>> from those constructs (and if llvm supported them I'd then consider
>> breaking out the assembly file changes from that patch of mine).
> 
> Guys, may I suggest using sym_phys(__bss_start)/sym_phys(__bss_end) as
> it is in head.S right now? If .startof.()/.sizeof.() works as expected
> and both of you accept it then later we can move to new approach. Does
> it make sense?

While I dislike taking this step backwards, I guess that's the only
viable option to move this patch closer to being ready to go in.

Jan


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