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. -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
