linux-kernel - Re: [PATCH 03/17] Add system pointer argument to shared EFI stub related functions so they no longer use global system table pointer as they did when part of eboot.c.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAFECyb8NbZ=hrAGyoV-UPUhh-Jst6ZVRk9RUQj_Kb1qq_xuUAg@mail.gmail.com>
Date:	Wed, 7 Aug 2013 10:10:54 -0700
From:	Roy Franz <roy.franz@...aro.org>
To:	Matt Fleming <matt@...sole-pimps.org>
Cc:	linux-kernel@...r.kernel.org, linux-efi@...r.kernel.org,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>, matt.fleming@...el.com,
	Russell King - ARM Linux <linux@....linux.org.uk>,
	Leif Lindholm <leif.lindholm@...aro.org>,
	Dave Martin <dave.martin@....com>
Subject: Re: [PATCH 03/17] Add system pointer argument to shared EFI stub
 related functions so they no longer use global system table pointer as they
 did when part of eboot.c.

On Wed, Aug 7, 2013 at 6:08 AM, Matt Fleming <matt@...sole-pimps.org> wrote:
> On Tue, 06 Aug, at 08:44:59PM, Roy Franz wrote:
>> Signed-off-by: Roy Franz <roy.franz@...aro.org>
>> ---
>>  arch/x86/boot/compressed/eboot.c       |   38 +++++++------
>>  drivers/firmware/efi/efi-stub-helper.c |   96 +++++++++++++++++---------------
>>  2 files changed, 72 insertions(+), 62 deletions(-)
>
> For future reference you should really use a shorter first line in your
> git commit message, which would produe a shorter subject when mailing
> your patches.
>
> I'll fix up the commit messages when I apply these patches, so don't
> worry about it for now.
>
> [...]
>
>> @@ -19,15 +19,16 @@ struct initrd {
>>
>>
>>
>> -static void efi_char16_printk(efi_char16_t *str)
>> +static void efi_char16_printk(efi_system_table_t *sys_table_arg,
>> +                           efi_char16_t *str)
>>  {
>>       struct efi_simple_text_output_protocol *out;
>>
>> -     out = (struct efi_simple_text_output_protocol *)sys_table->con_out;
>> +     out = (struct efi_simple_text_output_protocol *)sys_table_arg->con_out;
>>       efi_call_phys2(out->output_string, out, str);
>>  }
>>
>> -static void efi_printk(char *str)
>> +static void efi_printk(efi_system_table_t *sys_table_arg, char *str)
>>  {
>>       char *s8;
>>
>
> Hmm... I'm not necessarily convinced this is an improvement over using
> some kind of a global pointer to the EFI System Table.
>
> Parameterizing stuff like this is useful when the argument changes at
> runtime from call to call, but that isn't the case for the boot stubs. I
> don't think there's anything wrong with a global in this scenario, and
> this patch is a fair amount of churn for no real improvement.
>
> --
> Matt Fleming, Intel Open Source Technology Center

Hi Matt,

I went this way since the shared code is in a separate file - I really
didn't like using a global variable as part of the interface to
the shared code.  This has the nice side benefit of allowing the ARM
stub to not use any global variables, so we don't have to do
any GOT fixups to relocate the code - it is position independent if we
don't use global variables.

Roy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/