Re: [PATCH v3 4/5] xen/x86: add some addresses to the Multiboot2 header

[Bob Eshleman <bobbyeshleman@gmail.com>]

On 2/23/21 1:04 AM, Roger Pau Monné wrote:
> On Thu, Jan 21, 2021 at 04:51:43PM -0800, Bobby Eshleman wrote:
>> From: Daniel Kiper <daniel.kiper@oracle.com>
>>
>> In comparison to ELF the PE format is not supported by the Multiboot2
>> protocol. So, if we wish to load xen.mb.efi using this protocol we have
>> to add MULTIBOOT2_HEADER_TAG_ADDRESS and MULTIBOOT2_HEADER_TAG_ENTRY_ADDRESS
>> tags into Multiboot2 header.
>>
>> Additionally, put MULTIBOOT2_HEADER_TAG_ENTRY_ADDRESS and
>> MULTIBOOT2_HEADER_TAG_ENTRY_ADDRESS_EFI64 tags close to each
>> other to make the header more readable.
>>
>> The Multiboot2 protocol spec can be found at
>>   https://www.gnu.org/software/grub/manual/multiboot2/
>>
>> Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
>> Signed-off-by: Bobby Eshleman <bobbyeshleman@gmail.com>
>> ---
>>  xen/arch/x86/boot/head.S | 19 +++++++++++++++----
>>  1 file changed, 15 insertions(+), 4 deletions(-)
>>
>> diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
>> index 189d91a872..f2edd182a5 100644
>> --- a/xen/arch/x86/boot/head.S
>> +++ b/xen/arch/x86/boot/head.S
>> @@ -94,6 +94,13 @@ multiboot2_header:
>>          /* Align modules at page boundry. */
>>          mb2ht_init MB2_HT(MODULE_ALIGN), MB2_HT(REQUIRED)
>>  
>> +        /* The address tag. */
>> +        mb2ht_init MB2_HT(ADDRESS), MB2_HT(REQUIRED), \
>> +                   sym_offs(multiboot2_header), /* header_addr */ \
>> +                   sym_offs(start),             /* load_addr */ \
>> +                   sym_offs(__bss_start),       /* load_end_addr */ \
>> +                   sym_offs(__2M_rwdata_end)    /* bss_end_addr */
> 
> Shouldn't this only be present when a PE binary is built?
> 
> You seem to unconditionally add this to the header, even when the
> resulting binary will be in ELF format?
> 
> According to the spec: "This information does not need to be provided
> if the kernel image is in ELF format", and hence Xen shouldn't require
> the loader to understand this tag unless it's strictly required, as
> the presence of the tag forces the bootloader to use the presented
> information in order to load the kernel, regardless of the underlying
> binary format.
> 
> Thanks, Roger.
> 

Ah yes, this is true.  It may have made more sense to do this with v2 trying
to step us in the direction of a single unified binary, but it certainly isn't
required with v3.

Thanks,
Bob