| Message ID | 147986056324.13790.12670822944798392730.stgit@warthog.procyon.org.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
David Howells <dhowells@redhat.com> wrote:
> +#define efi_call_runtime(f, ...) sys_table_arg->runtime->f(__VA_ARGS__)
Turns out it's not that simple - of course. runtime->get_variable is just a
void pointer. The old arm stub was casting it by virtue of assignment to a
function pointer variable.
The x86_64 appears to be doing bypassing all the compile-time type checking by
passing the arguments through an ellipsis and then fixing up the argument list
in the ->call() function.
What I've changed the ARM and ARM64 things to is:
#define efi_call_runtime(f, ...) ((efi_##f##_t *)sys_table_arg->runtime->f)(__VA_ARGS__)
David
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, Any reason to not Cc LAKML? On Wed, Nov 23, 2016 at 12:22:43AM +0000, David Howells wrote: > Provide the ability to perform mixed-mode runtime service calls for arm in > the same way that commit 0a637ee61247bd4bed9b2a07568ef7a1cfc76187 > ("x86/efi: Allow invocation of arbitrary boot services") provides the > ability to invoke arbitrary boot services. I'm not sure I understand. On arm/arm64, "mixed-mode" simply isn't possible. I see we already call runtime services directly in efi_get_secureboot() in drivers/firmware/efi/libstub/arm-stub.c. If this is just to provide a consistent API for the stub, please note that. > Suggested-by: Lukas Wunner <lukas@wunner.de> > Signed-off-by: David Howells <dhowells@redhat.com> > --- > > arch/arm/include/asm/efi.h | 1 + > arch/arm64/include/asm/efi.h | 1 + > 2 files changed, 2 insertions(+) > > diff --git a/arch/arm/include/asm/efi.h b/arch/arm/include/asm/efi.h > index 0b06f5341b45..e4e6a9d6a825 100644 > --- a/arch/arm/include/asm/efi.h > +++ b/arch/arm/include/asm/efi.h > @@ -55,6 +55,7 @@ void efi_virtmap_unload(void); > > #define efi_call_early(f, ...) sys_table_arg->boottime->f(__VA_ARGS__) > #define __efi_call_early(f, ...) f(__VA_ARGS__) > +#define efi_call_runtime(f, ...) sys_table_arg->runtime->f(__VA_ARGS__) > #define efi_is_64bit() (false) > > #define efi_call_proto(protocol, f, instance, ...) \ > diff --git a/arch/arm64/include/asm/efi.h b/arch/arm64/include/asm/efi.h > index 771b3f0bc757..d74ae223d89f 100644 > --- a/arch/arm64/include/asm/efi.h > +++ b/arch/arm64/include/asm/efi.h > @@ -49,6 +49,7 @@ int efi_set_mapping_permissions(struct mm_struct *mm, efi_memory_desc_t *md); > > #define efi_call_early(f, ...) sys_table_arg->boottime->f(__VA_ARGS__) > #define __efi_call_early(f, ...) f(__VA_ARGS__) > +#define efi_call_runtime(f, ...) sys_table_arg->runtime->f(__VA_ARGS__) Given this can only work in the stub, the name is somewhat unfortunate, as it sounds like it's expected to be used at runtime (i.e. in the kernel). But I guess that's not a big problem. Other than the casting issue you noted, I think this should work, though. Thanks, Mark. -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 23 November 2016 at 09:34, David Howells <dhowells@redhat.com> wrote: > David Howells <dhowells@redhat.com> wrote: > >> +#define efi_call_runtime(f, ...) sys_table_arg->runtime->f(__VA_ARGS__) > > Turns out it's not that simple - of course. runtime->get_variable is just a > void pointer. The old arm stub was casting it by virtue of assignment to a > function pointer variable. > > The x86_64 appears to be doing bypassing all the compile-time type checking by > passing the arguments through an ellipsis and then fixing up the argument list > in the ->call() function. > > What I've changed the ARM and ARM64 things to is: > > #define efi_call_runtime(f, ...) ((efi_##f##_t *)sys_table_arg->runtime->f)(__VA_ARGS__) > Could we please instead fix the definition of efi_runtime_services_t, given that we have typedefs already for all its members? -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Mark Rutland <mark.rutland@arm.com> wrote: > Any reason to not Cc LAKML? Probably not. > On Wed, Nov 23, 2016 at 12:22:43AM +0000, David Howells wrote: > > Provide the ability to perform mixed-mode runtime service calls for arm in > > the same way that commit 0a637ee61247bd4bed9b2a07568ef7a1cfc76187 > > ("x86/efi: Allow invocation of arbitrary boot services") provides the > > ability to invoke arbitrary boot services. > > I'm not sure I understand. On arm/arm64, "mixed-mode" simply isn't possible. > > I see we already call runtime services directly in efi_get_secureboot() > in drivers/firmware/efi/libstub/arm-stub.c. > > If this is just to provide a consistent API for the stub, please note > that. How about: arm/efi: Allow invocation of arbitrary runtime services efi_call_runtime() is provided for x86 to be able abstract mixed mode support. Provide this for ARM also so that common code work in mixed mode also. David -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > > What I've changed the ARM and ARM64 things to is: > > > > #define efi_call_runtime(f, ...) ((efi_##f##_t *)sys_table_arg->runtime->f)(__VA_ARGS__) > > > > Could we please instead fix the definition of efi_runtime_services_t, > given that we have typedefs already for all its members? Okay, I've pulled in your patch and removed the cast. I would like to provide wrapper static inlines for things like efi_get_variable() to get the parameter checking, but the sys_table_arg behind-the-scenes parameter is tricky to deal with in that case. David -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Nov 23, 2016 at 11:46:38AM +0000, David Howells wrote: > Mark Rutland <mark.rutland@arm.com> wrote: > > On Wed, Nov 23, 2016 at 12:22:43AM +0000, David Howells wrote: > > > Provide the ability to perform mixed-mode runtime service calls for arm in > > > the same way that commit 0a637ee61247bd4bed9b2a07568ef7a1cfc76187 > > > ("x86/efi: Allow invocation of arbitrary boot services") provides the > > > ability to invoke arbitrary boot services. > > > > I'm not sure I understand. On arm/arm64, "mixed-mode" simply isn't possible. > > > > I see we already call runtime services directly in efi_get_secureboot() > > in drivers/firmware/efi/libstub/arm-stub.c. > > > > If this is just to provide a consistent API for the stub, please note > > that. > > How about: > > arm/efi: Allow invocation of arbitrary runtime services > > efi_call_runtime() is provided for x86 to be able abstract mixed mode > support. Provide this for ARM also so that common code work in mixed mode > also. That sounds good to me. Thanks, Mark. -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/arm/include/asm/efi.h b/arch/arm/include/asm/efi.h index 0b06f5341b45..e4e6a9d6a825 100644 --- a/arch/arm/include/asm/efi.h +++ b/arch/arm/include/asm/efi.h @@ -55,6 +55,7 @@ void efi_virtmap_unload(void); #define efi_call_early(f, ...) sys_table_arg->boottime->f(__VA_ARGS__) #define __efi_call_early(f, ...) f(__VA_ARGS__) +#define efi_call_runtime(f, ...) sys_table_arg->runtime->f(__VA_ARGS__) #define efi_is_64bit() (false) #define efi_call_proto(protocol, f, instance, ...) \ diff --git a/arch/arm64/include/asm/efi.h b/arch/arm64/include/asm/efi.h index 771b3f0bc757..d74ae223d89f 100644 --- a/arch/arm64/include/asm/efi.h +++ b/arch/arm64/include/asm/efi.h @@ -49,6 +49,7 @@ int efi_set_mapping_permissions(struct mm_struct *mm, efi_memory_desc_t *md); #define efi_call_early(f, ...) sys_table_arg->boottime->f(__VA_ARGS__) #define __efi_call_early(f, ...) f(__VA_ARGS__) +#define efi_call_runtime(f, ...) sys_table_arg->runtime->f(__VA_ARGS__) #define efi_is_64bit() (true) #define efi_call_proto(protocol, f, instance, ...) \
Provide the ability to perform mixed-mode runtime service calls for arm in the same way that commit 0a637ee61247bd4bed9b2a07568ef7a1cfc76187 ("x86/efi: Allow invocation of arbitrary boot services") provides the ability to invoke arbitrary boot services. Suggested-by: Lukas Wunner <lukas@wunner.de> Signed-off-by: David Howells <dhowells@redhat.com> --- arch/arm/include/asm/efi.h | 1 + arch/arm64/include/asm/efi.h | 1 + 2 files changed, 2 insertions(+) -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html