Re: [Xen-devel] [PATCH RFC 15/19] Add PE/COFF header in head.S

[Roy Franz <roy.franz@xxxxxxxxxx>]

On Wed, Jul 2, 2014 at 5:02 AM, Ian Campbell <Ian.Campbell@xxxxxxxxxx> wrote:
> On Sat, 2014-06-28 at 02:25 +0100, Roy Franz wrote:
>> This patch adds a simple PE/COFF header in head.S.  PE/COFF linker support
>> is not available for arm64, so a native build is not possible.  Also, this
>> allows the binary to be both a PE/COFF EFI application, and a normal Image
>> file bootable like a Linux kernel.  The arm and arm64 Linux kernels use the
>> same methodology to create a single image that is both an EFI application and
>> a zImage/Image file.
>
> This looks good to me. A few minor comments:
>
> Some of the whitespace is off (please use space and not hard tabs
> throughout)
I'll clean that up.

>
> I think we could perhaps get rid of CONFIG_EFI_STUB at least at the
> arm64 subarch level. We don't in general support the same level of
> configurability in Xen as Linux -- it's mostly just a single
> configuration for everyone. It'd need to be retain at the arch level to
> cope with arm32 not doing efi, but removing it here would make things
> simpler I think?
I agree. I don't think the arm64 stub needs to be configurable, as it
should not
affect non-stub users when it is enabled, and should be a fairly small amount
of code.  It will need to be disabled for arm32.

>
> Would it be possible to find sort of link/reference to a spec for the
> header structs which you are adding and stick them in comments before
> each one please.
I will try to track down some documentation to link to.  It may end up
being a single
link to a PDF (or even a microsoft Word document, since it is a Microsoft spec.)

>
>>
>> Signed-off-by: Roy Franz <roy.franz@xxxxxxxxxx>
>> ---
>>  xen/arch/arm/arm64/head.S | 118 
>> +++++++++++++++++++++++++++++++++++++++++++++-
>>  xen/arch/arm/xen.lds.S    |   1 +
>>  2 files changed, 117 insertions(+), 2 deletions(-)
>>
>> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
>> index 2a13527..9b88aeb 100644
>> --- a/xen/arch/arm/arm64/head.S
>> +++ b/xen/arch/arm/arm64/head.S
>> @@ -84,7 +84,7 @@
>>  #endif /* !CONFIG_EARLY_PRINTK */
>>
>>          /*.aarch64*/
>> -
>> +#define CONFIG_EFI_STUB
>>          /*
>>           * Kernel startup entry point.
>>           * ---------------------------
>> @@ -100,12 +100,24 @@
>>           */
>>
>>          .global start
>> +efi_stub_entry:  /* Dummy symbol so we can compile before actual stub added 
>> */
>>  start:
>> +#ifdef CONFIG_EFI_STUB
>>          /*
>>           * DO NOT MODIFY. Image header expected by Linux boot-loaders.
>>           */
>> -        b       real_start           /* branch to kernel start, magic */
>> +efi_head:
>> +        /*
>> +         * This add instruction has no meaningful effect except that
>> +         * its opcode forms the magic "MZ" signature of a PE/COFF file
>> +      * that is required for UEFI applications.
>> +         */
>> +        add     x13, x18, #0x16
>
> I love this hack ;-)
>
> Ian.
>

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