On Thu, 10 Mar, at 12:40:02PM, Ard Biesheuvel wrote: > The Graphics Output Protocol code executes in the stub, so create a generic > version based on the x86 version in libstub so that we can move all archs > to it in subsequent patches. The new source file gop.c is added to the > libstub build for all architectures, but is not actually included in any of > the final images, since this patch does not wire it up yet. > > Note that the generic version uses slightly different ways of casting the > protocol methods and some other variables to the correct types, since such > method calls are not loosely typed on ARM and arm64 as they are on x86. > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx> > --- > arch/arm/include/asm/efi.h | 4 +- > arch/arm64/include/asm/efi.h | 4 +- > arch/x86/boot/compressed/eboot.h | 74 ---- > arch/x86/include/asm/efi.h | 5 + > drivers/firmware/efi/libstub/Makefile | 2 +- > drivers/firmware/efi/libstub/gop.c | 354 ++++++++++++++++++++ > include/linux/efi.h | 87 ++++- > 7 files changed, 451 insertions(+), 79 deletions(-) I can totally appreciate why you did the gop code move in stages, but I'd prefer to see the move (deletion from arch/x86 and addition to drivers/firmware/efi) done as a single patch. The reason is that git will produce a very nice diff stat that shows the number of lines that were simply moved from one file to another without modification. Which makes it much easier to see which lines *did* change. As a very quick and dirty example (but this is not what I'm suggesting you do because it'd be too many changes in one patch) I merged this patch with the next one, which produces the following diff stat. The important point are the insertions, : $ git show --shortstat --summary -C10% e63c02a98c3c : : 8 files changed, 133 insertions(+), 1606 deletions(-) : copy arch/x86/boot/compressed/eboot.c => drivers/firmware/efi/libstub/gop.c (20%) > diff --git a/arch/arm/include/asm/efi.h b/arch/arm/include/asm/efi.h > index e0eea72deb87..daebcfe6c35a 100644 > --- a/arch/arm/include/asm/efi.h > +++ b/arch/arm/include/asm/efi.h > @@ -59,7 +59,9 @@ void efi_virtmap_unload(void); > > /* arch specific definitions used by the stub code */ > > -#define efi_call_early(f, ...) sys_table_arg->boottime->f(__VA_ARGS__) > +#define efi_call_early(f, ...) sys_table_arg->boottime->f(__VA_ARGS__) > +#define __efi_call_early(f, ...) f(__VA_ARGS__) > +#define efi_is_64bit() (0) Is there a specific reason that these can't be boolean? I know some people hate them, but I've always been a pretty big fan when the circumstances are right. Plus it is bool on x86. -- 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