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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 28.09.16 at 11:39, <daniel.kiper@xxxxxxxxxx> wrote:
> On Wed, Sep 28, 2016 at 02:57:10AM -0600, Jan Beulich wrote:
>> >>> On 27.09.16 at 20:11, <daniel.kiper@xxxxxxxxxx> wrote:
>> > On Mon, Sep 26, 2016 at 07:47:42AM -0600, Jan Beulich wrote:
>> >> >>> On 23.09.16 at 23:47, <daniel.kiper@xxxxxxxxxx> 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
>> >> > +        shr     $3,%ecx
>> >> > +        xor     %eax,%eax
>> >> > +        rep stosq
>> >> > +
>> >> > +        pop     %rdi
>> >> > +
>> >> > +        /*
>> >> > +         * efi_multiboot2() is called according to System V AMD64 ABI:
>> >> > +         *   - IN: %rdi - EFI ImageHandle, %rsi - EFI SystemTable,
>> >> > +         *   - OUT: %rax - highest usable memory address below 1 MiB;
>> >> > +         *                 memory above this address is reserved for 
>> >> > trampoline;
>> >> > +         *                 memory below this address is used for stack 
>> >> > and as
>> >> > +         *                 a storage for boot data.
>> >>
>> >> How can you validly use memory below this address? (And I'd like to
>> >
>> > Right, I should not do that blindly. As quick fix we can check in
>> > efi_arch_process_memory_map()
>> > that chosen memory region has size cfg.size plus let's say 64 KiB. Is it 
>> > sufficient
>> > for you?
>>
>> Depends. Part of my problem here is that you convert, in your answer,
>> my "validly" to "blindly". And then I'd like to see especially "storage for
> 
> I am not sure why the difference, IMO minor, is important for you here.

Because I questioned any access to that area, yet you suggested
accesses are okay with a little more checking.

>> boot data" better qualified: What exactly is it that you mean to store
>> there?
> 
> mbi struct created in reloc.c from original multiboot(2) data passed by boot 
> loader.

Ah, right. Please make the comment less vague then. And this then
also gives you/us a means to determine what size would need to be
available (64k looks to be way too much for mb1, but mb2 of course
is much less bounded).

Jan


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