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

[Daniel Kiper <daniel.kiper@xxxxxxxxxx>]

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?

Daniel

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