Re: [PATCH 0/2] Honey, I shrunk the EFI stub

[Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org>]

On Mon, 07 Nov, at 12:17:00PM, Lukas Wunner wrote:
> Demonstrate the code reduction attainable by efi_call_proto()
> which was proffered in a patch I've posted a few minutes ago.
> 
> For this to work, all three protocol variants (_32_t and _64_t for x86
> and _t for ARM) need to be declared as typedefs.  The declaration and
> naming of protocols in include/linux/efi.h currently isn't consistent,
> some are declared as typedefs and some aren't, some use a "_t" suffix
> and some don't.  These inconsistencies need to be straightened out
> when converting to efi_call_proto().  It should be noted that checkpatch
> complains about newly introduced typedefs.  It would be possible to
> retool efi_call_proto() to work without typedef declarations as long
> as it's done consistently.
 
This is probably v4.11 material. We *may* be able to get this into
v4.10 if I review and merge this soon, but it definitely isn't going
to be included in the imminent pull request.

I do like the general idea though.

> In __file_size32() all protocol calls are currently cast to unsigned long,
> which is 64 bit when compiled on x86_64.  Matt has said that the register
> needs to be loaded with a 32 bit address, so it looks to me like this is
> currently broken for mixed-mode.  Patch [1/2] should fix this.  E.g.:
> 
> 	efi_file_handle_32_t *h, *fh = __fh;
> [...]
> 	status = efi_early->call((unsigned long)h->get_info, h, &info_guid,
> 				 &info_sz, NULL);

There's a subtle distinction here between 32-bit address and 32-bit
value. A 64-bit value can be a valid 32-bit address, provided that the
upper 32-bits are zero, e.g. 0x00000000ffffffff.

So when I say "32-bit address" I really just mean some value where
only the lower 32-bits are important.

That is why using unsigned long in mixed-mode is OK for the early call
code.
 
> Another oddity is that info_sz is declared u32 in __file_size32(),
> yet the spec says that the third argument to EFI_FILE_PROTOCOL.GetInfo()
> is of type UINTN, which I assume is 64 bit regardless of mixed-mode,
> or am I missing something?  Patch [1/2] uses an unsigned long instead.

UINTN is an unsigned value of native width as seen by the firmware. On
32-bit firmware that's 32-bits and 64-bit firmware 64-bits.

Using 'u32' in __file_size32() is correct, unsigned long is not.