[PATCH 2/4] x86/hvmloader: Don't build as PIC/PIE

Andrew Cooper posted 4 patches 3 years, 5 months ago
[PATCH 2/4] x86/hvmloader: Don't build as PIC/PIE
Posted by Andrew Cooper 3 years, 5 months ago
HVMLoader is not relocatable in memory, and 32bit PIC code has a large
overhead.  Build it as non-relocatable.

Bloat-o-meter reports a net:
  add/remove: 0/0 grow/shrink: 3/107 up/down: 14/-3370 (-3356)

No functional change.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>
---
 tools/firmware/hvmloader/Makefile | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/firmware/hvmloader/Makefile b/tools/firmware/hvmloader/Makefile
index 4f31c881613c..eb757819274b 100644
--- a/tools/firmware/hvmloader/Makefile
+++ b/tools/firmware/hvmloader/Makefile
@@ -23,7 +23,8 @@ include $(XEN_ROOT)/tools/firmware/Rules.mk
 # SMBIOS spec requires format mm/dd/yyyy
 SMBIOS_REL_DATE ?= $(shell date +%m/%d/%Y)
 
-CFLAGS += $(CFLAGS_xeninclude)
+CFLAGS += $(CFLAGS_xeninclude) -fno-pic
+$(call cc-option-add,CFLAGS,-no-pie)
 
 # We mustn't use tools-only public interfaces.
 CFLAGS += -D__XEN_INTERFACE_VERSION__=__XEN_LATEST_INTERFACE_VERSION__
-- 
2.11.0


Re: [PATCH 2/4] x86/hvmloader: Don't build as PIC/PIE
Posted by Jan Beulich 3 years, 5 months ago
On 24.08.2022 12:59, Andrew Cooper wrote:
> HVMLoader is not relocatable in memory, and 32bit PIC code has a large
> overhead.  Build it as non-relocatable.
> 
> Bloat-o-meter reports a net:
>   add/remove: 0/0 grow/shrink: 3/107 up/down: 14/-3370 (-3356)
> 
> No functional change.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Roger Pau Monné <roger.pau@citrix.com>
> CC: Wei Liu <wl@xen.org>
> ---
>  tools/firmware/hvmloader/Makefile | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/firmware/hvmloader/Makefile b/tools/firmware/hvmloader/Makefile
> index 4f31c881613c..eb757819274b 100644
> --- a/tools/firmware/hvmloader/Makefile
> +++ b/tools/firmware/hvmloader/Makefile
> @@ -23,7 +23,8 @@ include $(XEN_ROOT)/tools/firmware/Rules.mk
>  # SMBIOS spec requires format mm/dd/yyyy
>  SMBIOS_REL_DATE ?= $(shell date +%m/%d/%Y)
>  
> -CFLAGS += $(CFLAGS_xeninclude)
> +CFLAGS += $(CFLAGS_xeninclude) -fno-pic
> +$(call cc-option-add,CFLAGS,-no-pie)

This is supposed to be coming from EMBEDDED_EXTRA_CFLAGS, if only
it was spelled correctly there. See the patch just sent. This line
(see that other patch) is meaningless anyway, as we don't use
$(CFLAGS) for linking here. So with it dropped
Reviewed-by: Jan Beulich <jbeulich@suse.com>

I do think though that the description could do with some expanding,
as I don't think -fpic or -fPIC is the default normally. I suppose
it's only specific distros which make this the default.

Jan

Re: [PATCH 2/4] x86/hvmloader: Don't build as PIC/PIE
Posted by Andrew Cooper 2 years, 10 months ago
On 25/08/2022 8:20 am, Jan Beulich wrote:
> On 24.08.2022 12:59, Andrew Cooper wrote:
>> HVMLoader is not relocatable in memory, and 32bit PIC code has a large
>> overhead.  Build it as non-relocatable.
>>
>> Bloat-o-meter reports a net:
>>   add/remove: 0/0 grow/shrink: 3/107 up/down: 14/-3370 (-3356)
>>
>> No functional change.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> ---
>> CC: Jan Beulich <JBeulich@suse.com>
>> CC: Roger Pau Monné <roger.pau@citrix.com>
>> CC: Wei Liu <wl@xen.org>
>> ---
>>  tools/firmware/hvmloader/Makefile | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/firmware/hvmloader/Makefile
>> b/tools/firmware/hvmloader/Makefile
>> index 4f31c881613c..eb757819274b 100644
>> --- a/tools/firmware/hvmloader/Makefile
>> +++ b/tools/firmware/hvmloader/Makefile
>> @@ -23,7 +23,8 @@ include $(XEN_ROOT)/tools/firmware/Rules.mk
>>  # SMBIOS spec requires format mm/dd/yyyy
>>  SMBIOS_REL_DATE ?= $(shell date +%m/%d/%Y)
>>  
>> -CFLAGS += $(CFLAGS_xeninclude)
>> +CFLAGS += $(CFLAGS_xeninclude) -fno-pic
>> +$(call cc-option-add,CFLAGS,-no-pie)
>
> This is supposed to be coming from EMBEDDED_EXTRA_CFLAGS, if only
> it was spelled correctly there. See the patch just sent. This line
> (see that other patch) is meaningless anyway, as we don't use
> $(CFLAGS) for linking here. So with it dropped
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
>
> I do think though that the description could do with some expanding,
> as I don't think -fpic or -fPIC is the default normally. I suppose
> it's only specific distros which make this the default.

Yeah, for ASLR reasons, but that covers ~all of our downstream users.

I'll tweak the commit message and drop the PIE part.

~Andrew