On Wed, Jan 07, 2015 at 12:16:02PM +0000, Ard Biesheuvel wrote:
> >> diff --git a/arch/arm64/include/asm/efi.h b/arch/arm64/include/asm/efi.h
> >> index 71291253114f..6cc668a378c5 100644
> >> --- a/arch/arm64/include/asm/efi.h
> >> +++ b/arch/arm64/include/asm/efi.h
> >> @@ -7,28 +7,36 @@
> >> #ifdef CONFIG_EFI
> >> extern void efi_init(void);
> >> extern void efi_idmap_init(void);
> >> +extern void efi_virtmap_init(void);
> >> #else
> >> #define efi_init()
> >> #define efi_idmap_init()
> >> +#define efi_virtmap_init
> >> #endif
> >>
> >> #define efi_call_virt(f, ...) \
> >> ({ \
> >> - efi_##f##_t *__f = efi.systab->runtime->f; \
> >> + efi_##f##_t *__f; \
> >> efi_status_t __s; \
> >> \
> >> - kernel_neon_begin(); \
> >> + kernel_neon_begin(); /* disables preemption */ \
> >
> > Nitpick: adding comment to otherwise untouched source line.
> >
> >> + efi_virtmap_load(); \
> >> + __f = efi.systab->runtime->f; \
> >> __s = __f(__VA_ARGS__); \
> >> + efi_virtmap_unload(); \
> >> kernel_neon_end(); \
> >> __s; \
> >> })
> >>
> >> #define __efi_call_virt(f, ...) \
> >> ({ \
> >> - efi_##f##_t *__f = efi.systab->runtime->f; \
> >> + efi_##f##_t *__f; \
> >> \
> >> - kernel_neon_begin(); \
> >> + kernel_neon_begin(); /* disables preemption */ \
> >
> > Same nitpick.
> >
>
> Is there anything wrong with that?
I said nitpick.
My (very minor) objection is that a (very reasonable) comment is added
to existing functionality by a patch that adds new functionality. It
makes the git blame/praise output less clear.
> Would you prefer the comment to be on a separate line?
I would _prefer_ the comments to be a separate patch.
But again, a nitpick.
> >> diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
> >> index b80991166754..d8390f507da0 100644
> >> --- a/arch/arm64/kernel/setup.c
> >> +++ b/arch/arm64/kernel/setup.c
> >> @@ -402,6 +402,7 @@ void __init setup_arch(char **cmdline_p)
> >> request_standard_resources();
> >>
> >> efi_idmap_init();
> >> + efi_virtmap_init();
> >
> > Could these two be merged together into one function?
> > Say efi_memmap_init()?
> >
>
> Well, I decided to do it like this because efi_idmap_init() gets
> removed in its entirety (including this invocation) in a subsequent
> patch.
Ah yeah. I knew that yesterday. Ignore me. More coffee.
/
Leif
--
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
