diff mbox series

[v4,01/26] x86/boot: Align vmlinuz sections on page size

Message ID 8211c633eb5dceeabee2996a4db91cd971cf7c77.1671098103.git.baskov@ispras.ru
State New
Headers
Series x86_64: Improvements at compressed kernel stage |

Commit Message

Evgeniy Baskov Dec. 15, 2022, 12:37 p.m. UTC 
  To protect sections on page table level each section
needs to be aligned on page size (4KB).

Set sections alignment in linker script.

Tested-by: Mario Limonciello <mario.limonciello@amd.com>
Tested-by: Peter Jones <pjones@redhat.com>
Signed-off-by: Evgeniy Baskov <baskov@ispras.ru>
---
 arch/x86/boot/compressed/vmlinux.lds.S | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Ard Biesheuvel March 10, 2023, 2:43 p.m. UTC | #1 
On Thu, 15 Dec 2022 at 13:38, Evgeniy Baskov <baskov@ispras.ru> wrote:
>
> To protect sections on page table level each section
> needs to be aligned on page size (4KB).
>
> Set sections alignment in linker script.
>
> Tested-by: Mario Limonciello <mario.limonciello@amd.com>
> Tested-by: Peter Jones <pjones@redhat.com>
> Signed-off-by: Evgeniy Baskov <baskov@ispras.ru>
> ---
>  arch/x86/boot/compressed/vmlinux.lds.S | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/arch/x86/boot/compressed/vmlinux.lds.S b/arch/x86/boot/compressed/vmlinux.lds.S
> index 112b2375d021..6be90f1a1198 100644
> --- a/arch/x86/boot/compressed/vmlinux.lds.S
> +++ b/arch/x86/boot/compressed/vmlinux.lds.S
> @@ -27,21 +27,27 @@ SECTIONS
>                 HEAD_TEXT
>                 _ehead = . ;
>         }
> +       . = ALIGN(PAGE_SIZE);
>         .rodata..compressed : {
> +               _compressed = .;
>                 *(.rodata..compressed)

Can you just move this bit into the rodata section below?

> +               _ecompressed = .;
>         }
> +       . = ALIGN(PAGE_SIZE);
>         .text : {

Please use

.text : ALIGN(PAGE_SIZE) {

which marks the section as being page aligned, rather than just being
placed on a 4k boundary.

>                 _text = .;      /* Text */
>                 *(.text)
>                 *(.text.*)
>                 _etext = . ;
>         }
> +       . = ALIGN(PAGE_SIZE);
>         .rodata : {
>                 _rodata = . ;
>                 *(.rodata)       /* read-only data */
>                 *(.rodata.*)
>                 _erodata = . ;
>         }
> +       . = ALIGN(PAGE_SIZE);
>         .data : {
>                 _data = . ;
>                 *(.data)
> --
> 2.37.4
>
Evgeniy Baskov March 11, 2023, 2:30 p.m. UTC | #2 
On 2023-03-10 17:43, Ard Biesheuvel wrote:
> On Thu, 15 Dec 2022 at 13:38, Evgeniy Baskov <baskov@ispras.ru> wrote:
>> 
>> To protect sections on page table level each section
>> needs to be aligned on page size (4KB).
>> 
>> Set sections alignment in linker script.
>> 
>> Tested-by: Mario Limonciello <mario.limonciello@amd.com>
>> Tested-by: Peter Jones <pjones@redhat.com>
>> Signed-off-by: Evgeniy Baskov <baskov@ispras.ru>
>> ---
>>  arch/x86/boot/compressed/vmlinux.lds.S | 6 ++++++
>>  1 file changed, 6 insertions(+)
>> 
>> diff --git a/arch/x86/boot/compressed/vmlinux.lds.S 
>> b/arch/x86/boot/compressed/vmlinux.lds.S
>> index 112b2375d021..6be90f1a1198 100644
>> --- a/arch/x86/boot/compressed/vmlinux.lds.S
>> +++ b/arch/x86/boot/compressed/vmlinux.lds.S
>> @@ -27,21 +27,27 @@ SECTIONS
>>                 HEAD_TEXT
>>                 _ehead = . ;
>>         }
>> +       . = ALIGN(PAGE_SIZE);
>>         .rodata..compressed : {
>> +               _compressed = .;
>>                 *(.rodata..compressed)
> 
> Can you just move this bit into the rodata section below?

I don't think that easily possible, as the layout need
to stay compatible with in-place extraction for non-UEFI boot.
For that execution path the code in .head.text moves everything
behind it to the end of the extraction buffer and extraction
code overwrites compressed kernel blob progressively during
extraction. And that is why we have effectively have two code
sections...

> 
>> +               _ecompressed = .;
>>         }
>> +       . = ALIGN(PAGE_SIZE);
>>         .text : {
> 
> Please use
> 
> .text : ALIGN(PAGE_SIZE) {
> 
> which marks the section as being page aligned, rather than just being
> placed on a 4k boundary.

Will fix in v5.

> 
>>                 _text = .;      /* Text */
>>                 *(.text)
>>                 *(.text.*)
>>                 _etext = . ;
>>         }
>> +       . = ALIGN(PAGE_SIZE);
>>         .rodata : {
>>                 _rodata = . ;
>>                 *(.rodata)       /* read-only data */
>>                 *(.rodata.*)
>>                 _erodata = . ;
>>         }
>> +       . = ALIGN(PAGE_SIZE);
>>         .data : {
>>                 _data = . ;
>>                 *(.data)
>> --
>> 2.37.4
>>
Ard Biesheuvel March 11, 2023, 2:42 p.m. UTC | #3 
On Sat, 11 Mar 2023 at 15:30, Evgeniy Baskov <baskov@ispras.ru> wrote:
>
> On 2023-03-10 17:43, Ard Biesheuvel wrote:
> > On Thu, 15 Dec 2022 at 13:38, Evgeniy Baskov <baskov@ispras.ru> wrote:
> >>
> >> To protect sections on page table level each section
> >> needs to be aligned on page size (4KB).
> >>
> >> Set sections alignment in linker script.
> >>
> >> Tested-by: Mario Limonciello <mario.limonciello@amd.com>
> >> Tested-by: Peter Jones <pjones@redhat.com>
> >> Signed-off-by: Evgeniy Baskov <baskov@ispras.ru>
> >> ---
> >>  arch/x86/boot/compressed/vmlinux.lds.S | 6 ++++++
> >>  1 file changed, 6 insertions(+)
> >>
> >> diff --git a/arch/x86/boot/compressed/vmlinux.lds.S
> >> b/arch/x86/boot/compressed/vmlinux.lds.S
> >> index 112b2375d021..6be90f1a1198 100644
> >> --- a/arch/x86/boot/compressed/vmlinux.lds.S
> >> +++ b/arch/x86/boot/compressed/vmlinux.lds.S
> >> @@ -27,21 +27,27 @@ SECTIONS
> >>                 HEAD_TEXT
> >>                 _ehead = . ;
> >>         }
> >> +       . = ALIGN(PAGE_SIZE);
> >>         .rodata..compressed : {
> >> +               _compressed = .;
> >>                 *(.rodata..compressed)
> >
> > Can you just move this bit into the rodata section below?
>
> I don't think that easily possible, as the layout need
> to stay compatible with in-place extraction for non-UEFI boot.
> For that execution path the code in .head.text moves everything
> behind it to the end of the extraction buffer and extraction
> code overwrites compressed kernel blob progressively during
> extraction. And that is why we have effectively have two code
> sections...
>

A right - thanks for explaining that to me.

So in the end, I think it doesn't matter in any case if we just stick
to a single .text section with R-X attributes and a single .data
section with RW- attributes.


> >
> >> +               _ecompressed = .;
> >>         }
> >> +       . = ALIGN(PAGE_SIZE);
> >>         .text : {
> >
> > Please use
> >
> > .text : ALIGN(PAGE_SIZE) {
> >
> > which marks the section as being page aligned, rather than just being
> > placed on a 4k boundary.
>
> Will fix in v5.
>
> >
> >>                 _text = .;      /* Text */
> >>                 *(.text)
> >>                 *(.text.*)
> >>                 _etext = . ;
> >>         }
> >> +       . = ALIGN(PAGE_SIZE);
> >>         .rodata : {
> >>                 _rodata = . ;
> >>                 *(.rodata)       /* read-only data */
> >>                 *(.rodata.*)
> >>                 _erodata = . ;
> >>         }
> >> +       . = ALIGN(PAGE_SIZE);
> >>         .data : {
> >>                 _data = . ;
> >>                 *(.data)
> >> --
> >> 2.37.4
> >>
diff mbox series

Patch

diff --git a/arch/x86/boot/compressed/vmlinux.lds.S b/arch/x86/boot/compressed/vmlinux.lds.S
index 112b2375d021..6be90f1a1198 100644
--- a/arch/x86/boot/compressed/vmlinux.lds.S
+++ b/arch/x86/boot/compressed/vmlinux.lds.S
@@ -27,21 +27,27 @@  SECTIONS
 		HEAD_TEXT
 		_ehead = . ;
 	}
+	. = ALIGN(PAGE_SIZE);
 	.rodata..compressed : {
+		_compressed = .;
 		*(.rodata..compressed)
+		_ecompressed = .;
 	}
+	. = ALIGN(PAGE_SIZE);
 	.text :	{
 		_text = .; 	/* Text */
 		*(.text)
 		*(.text.*)
 		_etext = . ;
 	}
+	. = ALIGN(PAGE_SIZE);
 	.rodata : {
 		_rodata = . ;
 		*(.rodata)	 /* read-only data */
 		*(.rodata.*)
 		_erodata = . ;
 	}
+	. = ALIGN(PAGE_SIZE);
 	.data :	{
 		_data = . ;
 		*(.data)