This fixes two minor issues with the handling of .init data in the stub (#1, #2)
Patch #3 replaces some instances of early_memremap() with early_memremap_ro()
where the region in question is only accessed in r/o mode.
Ard Biesheuvel (3):
arm64: efistub: drop __init annotation from handle_kernel_image()
arm64: vmlinux.lds.S: handle .init.rodata.xxx and .init.bss sections
efi: arm-init: use read-only early mappings
arch/arm64/kernel/vmlinux.lds.S | 1 +
drivers/firmware/efi/arm-init.c | 14 +++++++-------
drivers/firmware/efi/libstub/arm64-stub.c | 14 +++++++-------
3 files changed, 15 insertions(+), 14 deletions(-)
--
2.5.0
After moving arm64-stub.c to libstub/, all of its sections are emitted
as .init.xxx sections automatically, and the __init annotation of
handle_kernel_image() causes it to end up in .init.init.text, which is
not recognized as an __init section by the linker scripts. So drop the
annotation.
Signed-off-by: Ard Biesheuvel <redacted>
---
drivers/firmware/efi/libstub/arm64-stub.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
The EFI stub is typically built into the decompressor (x86, ARM) so none
of its symbols are annotated as __init. However, on arm64, the stub is
linked into the kernel proper, and the code is __init annotated at the
section level by prepending all names of SHF_ALLOC sections with '.init'.
This results in section names like .init.rodata.str1.8 (for string literals)
and .init.bss (which is tiny), both of which can be moved into the .init.data
output section.
Signed-off-by: Ard Biesheuvel <redacted>
---
arch/arm64/kernel/vmlinux.lds.S | 1 +
1 file changed, 1 insertion(+)
The early mappings of the EFI system table contents and the UEFI memory
map are read-only from the OS point of view. So map them read-only to
protect them from inadvertent modification.
Signed-off-by: Ard Biesheuvel <redacted>
---
drivers/firmware/efi/arm-init.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
@@ -61,8 +61,8 @@ static int __init uefi_init(void)charvendor[100]="unknown";inti,retval;-efi.systab=early_memremap(efi_system_table,-sizeof(efi_system_table_t));+efi.systab=early_memremap_ro(efi_system_table,+sizeof(efi_system_table_t));if(efi.systab==NULL){pr_warn("Unable to map EFI system table.\n");return-ENOMEM;
@@ -86,8 +86,8 @@ static int __init uefi_init(void)efi.systab->hdr.revision&0xffff);/* Show what we know for posterity */-c16=early_memremap(efi_to_phys(efi.systab->fw_vendor),-sizeof(vendor)*sizeof(efi_char16_t));+c16=early_memremap_ro(efi_to_phys(efi.systab->fw_vendor),+sizeof(vendor)*sizeof(efi_char16_t));if(c16){for(i=0;i<(int)sizeof(vendor)-1&&*c16;++i)vendor[i]=c16[i];
@@ -100,8 +100,8 @@ static int __init uefi_init(void)efi.systab->hdr.revision&0xffff,vendor);table_size=sizeof(efi_config_table_64_t)*efi.systab->nr_tables;-config_tables=early_memremap(efi_to_phys(efi.systab->tables),-table_size);+config_tables=early_memremap_ro(efi_to_phys(efi.systab->tables),+table_size);if(config_tables==NULL){pr_warn("Unable to map EFI config table array.\n");retval=-ENOMEM;
On Thu, Jan 28, 2016 at 12:07:31PM +0100, Ard Biesheuvel wrote:
This fixes two minor issues with the handling of .init data in the stub (#1, #2)
Patch #3 replaces some instances of early_memremap() with early_memremap_ro()
where the region in question is only accessed in r/o mode.
Nice!
The series looks sensible to me, and having given it a spin on Juno
(atop of v4.5-rc1 defconfig) everything seems to work, so FWIW:
Acked-by: Mark Rutland <mark.rutland@arm.com>
Tested-by: Mark Rutland <mark.rutland@arm.com>
Mark.
Ard Biesheuvel (3):
arm64: efistub: drop __init annotation from handle_kernel_image()
arm64: vmlinux.lds.S: handle .init.rodata.xxx and .init.bss sections
efi: arm-init: use read-only early mappings
arch/arm64/kernel/vmlinux.lds.S | 1 +
drivers/firmware/efi/arm-init.c | 14 +++++++-------
drivers/firmware/efi/libstub/arm64-stub.c | 14 +++++++-------
3 files changed, 15 insertions(+), 14 deletions(-)
--
2.5.0
From: Will Deacon <hidden> Date: 2016-01-28 15:56:11
On Thu, Jan 28, 2016 at 12:07:32PM +0100, Ard Biesheuvel wrote:
After moving arm64-stub.c to libstub/, all of its sections are emitted
as .init.xxx sections automatically, and the __init annotation of
handle_kernel_image() causes it to end up in .init.init.text, which is
not recognized as an __init section by the linker scripts. So drop the
annotation.
Signed-off-by: Ard Biesheuvel <redacted>
---
drivers/firmware/efi/libstub/arm64-stub.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
From: Will Deacon <hidden> Date: 2016-01-28 15:57:29
On Thu, Jan 28, 2016 at 12:07:33PM +0100, Ard Biesheuvel wrote:
The EFI stub is typically built into the decompressor (x86, ARM) so none
of its symbols are annotated as __init. However, on arm64, the stub is
linked into the kernel proper, and the code is __init annotated at the
section level by prepending all names of SHF_ALLOC sections with '.init'.
This results in section names like .init.rodata.str1.8 (for string literals)
and .init.bss (which is tiny), both of which can be moved into the .init.data
output section.
Signed-off-by: Ard Biesheuvel <redacted>
---
arch/arm64/kernel/vmlinux.lds.S | 1 +
1 file changed, 1 insertion(+)
From: Matt Fleming <hidden> Date: 2016-01-28 22:58:10
On Thu, 28 Jan, at 12:07:32PM, Ard Biesheuvel wrote:
quoted hunk
After moving arm64-stub.c to libstub/, all of its sections are emitted
as .init.xxx sections automatically, and the __init annotation of
handle_kernel_image() causes it to end up in .init.init.text, which is
not recognized as an __init section by the linker scripts. So drop the
annotation.
Signed-off-by: Ard Biesheuvel <redacted>
---
drivers/firmware/efi/libstub/arm64-stub.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
Would it make more sense to #undef __init in one of the arm64 efistub
header files? I'm thinking of the case where some poor unsuspecting
developer writes a new function and marks it as __init, and we miss it
during review.
At least if we do the #undef we can document why __init makes no sense
in the EFI stub, and at the same time automatically fix things up.
An alternative would be to write some Kbuild magic that complains if
it sees __init, but that seems like more work than is reasonable.
From: Matt Fleming <hidden> Date: 2016-01-28 22:59:00
On Thu, 28 Jan, at 12:07:33PM, Ard Biesheuvel wrote:
quoted hunk
The EFI stub is typically built into the decompressor (x86, ARM) so none
of its symbols are annotated as __init. However, on arm64, the stub is
linked into the kernel proper, and the code is __init annotated at the
section level by prepending all names of SHF_ALLOC sections with '.init'.
This results in section names like .init.rodata.str1.8 (for string literals)
and .init.bss (which is tiny), both of which can be moved into the .init.data
output section.
Signed-off-by: Ard Biesheuvel <redacted>
---
arch/arm64/kernel/vmlinux.lds.S | 1 +
1 file changed, 1 insertion(+)
On 28 January 2016 at 23:58, Matt Fleming [off-list ref] wrote:
On Thu, 28 Jan, at 12:07:32PM, Ard Biesheuvel wrote:
quoted
After moving arm64-stub.c to libstub/, all of its sections are emitted
as .init.xxx sections automatically, and the __init annotation of
handle_kernel_image() causes it to end up in .init.init.text, which is
not recognized as an __init section by the linker scripts. So drop the
annotation.
Signed-off-by: Ard Biesheuvel <redacted>
---
drivers/firmware/efi/libstub/arm64-stub.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
Would it make more sense to #undef __init in one of the arm64 efistub
header files? I'm thinking of the case where some poor unsuspecting
developer writes a new function and marks it as __init, and we miss it
during review.
Yes, I can add it to efistub.h, and make sure it gets included in all the files
Should we #undef it and #define it to a string that is easily grep'ed
for, so it is easy to find the explanatory comment that goes along
with it?
E.g.,
#define __init __init_not_supported_in_efi_stub
At least if we do the #undef we can document why __init makes no sense
in the EFI stub, and at the same time automatically fix things up.
An alternative would be to write some Kbuild magic that complains if
it sees __init, but that seems like more work than is reasonable.
From: Matt Fleming <hidden> Date: 2016-01-29 16:00:06
On Fri, 29 Jan, at 10:36:03AM, Ard Biesheuvel wrote:
On 28 January 2016 at 23:58, Matt Fleming [off-list ref] wrote:
quoted
Would it make more sense to #undef __init in one of the arm64 efistub
header files? I'm thinking of the case where some poor unsuspecting
developer writes a new function and marks it as __init, and we miss it
during review.
Yes, I can add it to efistub.h, and make sure it gets included in all the files
Should we #undef it and #define it to a string that is easily grep'ed
for, so it is easy to find the explanatory comment that goes along
with it?
E.g.,
#define __init __init_not_supported_in_efi_stub
This would produce a compilation failure if someone tags something as
__init right? Looks fine to me.
On 29 January 2016 at 17:00, Matt Fleming [off-list ref] wrote:
On Fri, 29 Jan, at 10:36:03AM, Ard Biesheuvel wrote:
quoted
On 28 January 2016 at 23:58, Matt Fleming [off-list ref] wrote:
quoted
Would it make more sense to #undef __init in one of the arm64 efistub
header files? I'm thinking of the case where some poor unsuspecting
developer writes a new function and marks it as __init, and we miss it
during review.
Yes, I can add it to efistub.h, and make sure it gets included in all the files
Should we #undef it and #define it to a string that is easily grep'ed
for, so it is easy to find the explanatory comment that goes along
with it?
E.g.,
#define __init __init_not_supported_in_efi_stub
This would produce a compilation failure if someone tags something as
__init right? Looks fine to me.
Yes, but it makes the error message difficult to decipher, since
__init is printed and not what it resolves to. Locally, I tried this,
which seems to work:
#undef __init
#define __init __attribute__((__init_not_supported_in_efi_stub))
#pragma GCC diagnostic error "-Wattributes"
which produces
CC drivers/firmware/efi/libstub/arm-stub.o
/home/ard/linux-2.6/drivers/firmware/efi/libstub/arm-stub.c:24:1:
error: ?__init_not_supported_in_efi_stub? attribute directive ignored
[-Werror=attributes]
{
^
cc1: some warnings being treated as errors
which is slightly dodgy, but at least puts the string right in the error message
From: Matt Fleming <hidden> Date: 2016-02-02 11:08:21
On Fri, 29 Jan, at 05:03:16PM, Ard Biesheuvel wrote:
On 29 January 2016 at 17:00, Matt Fleming [off-list ref] wrote:
quoted
On Fri, 29 Jan, at 10:36:03AM, Ard Biesheuvel wrote:
quoted
On 28 January 2016 at 23:58, Matt Fleming [off-list ref] wrote:
quoted
Would it make more sense to #undef __init in one of the arm64 efistub
header files? I'm thinking of the case where some poor unsuspecting
developer writes a new function and marks it as __init, and we miss it
during review.
Yes, I can add it to efistub.h, and make sure it gets included in all the files
Should we #undef it and #define it to a string that is easily grep'ed
for, so it is easy to find the explanatory comment that goes along
with it?
E.g.,
#define __init __init_not_supported_in_efi_stub
This would produce a compilation failure if someone tags something as
__init right? Looks fine to me.
Yes, but it makes the error message difficult to decipher, since
__init is printed and not what it resolves to. Locally, I tried this,
which seems to work:
#undef __init
#define __init __attribute__((__init_not_supported_in_efi_stub))
#pragma GCC diagnostic error "-Wattributes"
which produces
CC drivers/firmware/efi/libstub/arm-stub.o
/home/ard/linux-2.6/drivers/firmware/efi/libstub/arm-stub.c:24:1:
error: ?__init_not_supported_in_efi_stub? attribute directive ignored
[-Werror=attributes]
{
^
cc1: some warnings being treated as errors
which is slightly dodgy, but at least puts the string right in the error message
What about,
#define __init __compiletime_error("__init not supported in EFI boot stub")
?
On 2 February 2016 at 12:08, Matt Fleming [off-list ref] wrote:
On Fri, 29 Jan, at 05:03:16PM, Ard Biesheuvel wrote:
quoted
On 29 January 2016 at 17:00, Matt Fleming [off-list ref] wrote:
quoted
On Fri, 29 Jan, at 10:36:03AM, Ard Biesheuvel wrote:
quoted
On 28 January 2016 at 23:58, Matt Fleming [off-list ref] wrote:
quoted
Would it make more sense to #undef __init in one of the arm64 efistub
header files? I'm thinking of the case where some poor unsuspecting
developer writes a new function and marks it as __init, and we miss it
during review.
Yes, I can add it to efistub.h, and make sure it gets included in all the files
Should we #undef it and #define it to a string that is easily grep'ed
for, so it is easy to find the explanatory comment that goes along
with it?
E.g.,
#define __init __init_not_supported_in_efi_stub
This would produce a compilation failure if someone tags something as
__init right? Looks fine to me.
Yes, but it makes the error message difficult to decipher, since
__init is printed and not what it resolves to. Locally, I tried this,
which seems to work:
#undef __init
#define __init __attribute__((__init_not_supported_in_efi_stub))
#pragma GCC diagnostic error "-Wattributes"
which produces
CC drivers/firmware/efi/libstub/arm-stub.o
/home/ard/linux-2.6/drivers/firmware/efi/libstub/arm-stub.c:24:1:
error: ?__init_not_supported_in_efi_stub? attribute directive ignored
[-Werror=attributes]
{
^
cc1: some warnings being treated as errors
which is slightly dodgy, but at least puts the string right in the error message
What about,
#define __init __compiletime_error("__init not supported in EFI boot stub")
That only works for invocations, i.e., it needs to be used in header
files, and will trigger the error if a call to the function remains
after optimization. We want it at function definition time instead.
From: Matt Fleming <hidden> Date: 2016-02-03 15:19:27
On Tue, 02 Feb, at 12:09:41PM, Ard Biesheuvel wrote:
On 2 February 2016 at 12:08, Matt Fleming [off-list ref] wrote:
quoted
What about,
#define __init __compiletime_error("__init not supported in EFI boot stub")
That only works for invocations, i.e., it needs to be used in header
files, and will trigger the error if a call to the function remains
after optimization. We want it at function definition time instead.
Good point.
OK, how about we just do the #undef, call it good, and I add the task
of printing some helpful error message to my growing TODO list?
On 3 February 2016 at 16:19, Matt Fleming [off-list ref] wrote:
On Tue, 02 Feb, at 12:09:41PM, Ard Biesheuvel wrote:
quoted
On 2 February 2016 at 12:08, Matt Fleming [off-list ref] wrote:
quoted
What about,
#define __init __compiletime_error("__init not supported in EFI boot stub")
That only works for invocations, i.e., it needs to be used in header
files, and will trigger the error if a call to the function remains
after optimization. We want it at function definition time instead.
Good point.
OK, how about we just do the #undef, call it good, and I add the task
of printing some helpful error message to my growing TODO list?
I take it you didn't like my #pragma then? :-)
In any case, I don't care deeply about this, so just #undef'ing it is fine by me