[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v3 1/2] x86/efi: move the logic to detect PE build support



>>> On 25.07.18 at 15:38, <roger.pau@xxxxxxxxxx> wrote:
> On Wed, Jul 25, 2018 at 02:48:28PM +0200, Daniel Kiper wrote:
>> On Wed, Jul 18, 2018 at 12:27:32PM +0200, Roger Pau Monne wrote:
>> > So that it can be used by other components apart from the efi specific
>> > code. By moving the detection code creating a dummy efi/disabled file
>> > can be avoided.
>> >
>> > This is required so that the conditional used to define the efi symbol
>> > in the linker script can be removed and instead the definition of the
>> > efi symbol can be guarded using the preprocessor.
>> >
>> > The motivation behind this change is to be able to build Xen using lld
>> > (the LLVM linker), that at least on version 6.0.0 doesn't work
>> > properly with a DEFINED being used in a conditional expression:
>> >
>> > ld    -melf_x86_64_fbsd  -T xen.lds -N prelink.o --build-id=sha1 \
>> >     /root/src/xen/xen/common/symbols-dummy.o -o 
>> > /root/src/xen/xen/.xen-syms.0
>> > ld: error: xen.lds:233: symbol not found: efi
>> >
>> > Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
>> > Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
>> 
>> This patch does more than subject says. So, please split this patch into
>> 2 parts. One which really moves the detection logic and the second which
>> replaces DEFINED() with #ifdef.
> 
> It's not possible to place the whole contents of the patch in a
> subject line, that's why there's also a commit message that I think
> correctly explains everything done in the patch.
> 
> IMO just having a patch to change the DEFINED to an ifdef is quite
> pointless, it's better to switch it here together with the change that
> actually introduces the define itself.

FTR I'm fine with the patch as is, as indicated by my R-b.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.