Re: [Xen-devel] [PATCH v3 5/5] fix: add multiboot2 protocol support for EFI platforms

[Doug Goldstein <cardoe@xxxxxxxxxx>]

On 1/18/17 8:41 AM, Jan Beulich wrote:
>>>> On 18.01.17 at 14:33, <cardoe@xxxxxxxxxx> wrote:
>> On 1/18/17 4:52 AM, Jan Beulich wrote:
>>>>>> On 17.01.17 at 21:07, <cardoe@xxxxxxxxxx> wrote:
>>>> @@ -686,6 +683,10 @@ paddr_t __init efi_multiboot2(EFI_HANDLE ImageHandle, 
>>>> EFI_SYSTEM_TABLE *SystemTa
>>>>      setup_efi_pci();
>>>>      efi_variables();
>>>>  
>>>> +    /* This is the maximum size of our trampoline + our low memory stack 
>>>> */
>>>> +    cfg.size = max_t(UINTN, 64 << 10,
>>>> +            (trampoline_end - trampoline_start) + 4096);
>>>
>>> Considering the consuming code further up, I don't understand the
>>> reason for the addition of 4096 here. And if there is a reason, I'm
>>> pretty sure you actually mean PAGE_SIZE.
>>
>> You are correct. Given that the assembly is hardcoded at 64k there is no
>> reason for this. I had kicked around doing a similar max() call in
>> assembly instead of hardcoding the value but didn't do it. So I should
>> just remove this.
> 
> Well, I don't mind the max() (albeit it's not very useful, especially
> if trampoline size would ever get pretty close to 64k). And as said
> in reply to the earlier version - I think this would better be checked
> at build time (see the various ASSERT()s at the end of xen.lds.S).
> 
> Jan
> 
Thank you. I had looked for BUILD_ASSERT() and didn't see it so I didn't
know what mechanism we had which is why I didn't add it.

-- 
Doug Goldstein

Attachment: signature.asc
Description: OpenPGP digital signature

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