Re: [PATCH v2 0/6] Add ARM support for VS2017

["Gao, Liming" <liming.gao@intel.com>]

Pete:
  I get your point. I suggest to add it in VS2017 tool chain comment. To build XIP firmware image, /FILEALIGN:4096 are required. Now, SEC, PEI_CORE and PEIM module require it. 

Thanks
Liming
>-----Original Message-----
>From: Pete Batard [mailto:pete@akeo.ie]
>Sent: Thursday, December 07, 2017 11:56 PM
>To: edk2-devel@lists.01.org; Gao, Liming <liming.gao@intel.com>
>Subject: Re: [PATCH v2 0/6] Add ARM support for VS2017
>
>Hi Liming,
>
>On 2017.12.07 03:02, Gao, Liming wrote:
>>    This is the base step to enable VS2017 ARM tool chain. Then, user could
>use it and build ARM application first.
>
>Yes.
>
>>    Here, I have one question. VS2017 ARM requires 4096 alignment. So, why
>not add /FILEALIGN:4096 into tools_def.txt as the default linker option for
>ARM?
>
>I don't think we want to do that on account that this option does not
>need to be specified when building regular applications, and is
>currently only needed when building the QEMU firmware.
>
>So it's probably better to only specify it in the context where it is
>needed, rather than globally, especially as the documentation for
>/FILEALIGN [1] indicates that "By default, the linker does not use a
>fixed alignment size" which would tend to indicate that we might be
>better off letting the compiler decide what it should use on its own.
>
>Especially, forcing /FILEALIGN to 4K everywhere does appear to increase
>the size of the generated binaries. For instance, I'm seeing a RELEASE
>Shell.efi  with a size of 762 KB without /FILEALIGN, vs. 792 KB with
>/FILEALIGN:4096.
>
>Thus, I think we should be conservative with regards to the global
>compiler options we add: if an option only seems to be needed for a
>single module (and especially, if that module has not yet been patched
>and reviewed for compilation), it should probably be specified for that
>module alone.
>
>As such, unless you or other people on this list have a strong opinion
>that /FILEALIGN:4096 should be global, I am currently not planning to
>resubmit a new patch that adds that option.
>
>Please let me know if you still think the patch should be altered.
>
>Regards,
>
>/Pete
>
>[1] https://docs.microsoft.com/en-gb/cpp/build/reference/filealign