* [PATCH v2 0/2] arm64: Support HP Envy X2
@ 2018-12-23 2:52 Alexander Graf
2018-12-23 2:52 ` [PATCH v2 1/2] mkimage: Simplify header size logic Alexander Graf
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Alexander Graf @ 2018-12-23 2:52 UTC (permalink / raw)
To: grub-devel; +Cc: Leif Lindholm, Peter Jones, Jon Masters, Ard Biesheuvel
I got a new toy this week: An HP Envy X2 system. This is one of those shiny
new Qualcomm Snapdragon based Windows tablet/notebook hybrid things.
While running Windows on those is actually not a terribly bad experience now
that WSL is out, I would like to see Linux run on those as well in the future.
Unfortunately as far as I'm aware so far nobody was able to run self built
binaries on the built-in UEFI version.
Turns out, it's a problem with aligning the start of the header to 4k. Once
we do that, binaries can be loaded just fine and run.
The reason behind that is simple: Its firmware tries to ensure NX protection
flags and can do so only when the code is 4K aligned.
So to maintain compatibility with that device, this patch set just bumps the
header alignment to 4K always on arm64-efi. This way we improve overall
compatibility - there surely will be more devices coming with similar
constraints.
v1 -> v2:
- Remove explicit device wording from patch
- Use GRUB_EFI_PAGE_SIZE
Alexander Graf (2):
mkimage: Simplify header size logic
mkimage: arm64-efi: Align first section to page
util/mkimage.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
--
2.12.3
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH v2 1/2] mkimage: Simplify header size logic 2018-12-23 2:52 [PATCH v2 0/2] arm64: Support HP Envy X2 Alexander Graf @ 2018-12-23 2:52 ` Alexander Graf 2019-01-14 13:29 ` Daniel Kiper 2018-12-23 2:52 ` [PATCH v2 2/2] mkimage: arm64-efi: Align first section to page Alexander Graf 2019-01-07 9:58 ` [PATCH v2 0/2] arm64: Support HP Envy X2 Leif Lindholm 2 siblings, 1 reply; 8+ messages in thread From: Alexander Graf @ 2018-12-23 2:52 UTC (permalink / raw) To: grub-devel; +Cc: Leif Lindholm, Peter Jones, Jon Masters, Ard Biesheuvel For EFI images, we always have the following layout: [PE header] [padding] [first section (which also is the entry point)] Currently there are 2 places where we define how big header+padding are: in the .vaddr_offset member of our target image definition struct as well as in code in grub_install_generate_image(). Remove the latter, so that we only have a single place to modify if we need to change the padding. Signed-off-by: Alexander Graf <agraf@suse.de> --- util/mkimage.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/util/mkimage.c b/util/mkimage.c index 353bb1098..88b991764 100644 --- a/util/mkimage.c +++ b/util/mkimage.c @@ -1226,10 +1226,7 @@ grub_install_generate_image (const char *dir, const char *prefix, int header_size; int reloc_addr; - if (image_target->voidp_sizeof == 4) - header_size = EFI32_HEADER_SIZE; - else - header_size = EFI64_HEADER_SIZE; + header_size = image_target->vaddr_offset; reloc_addr = ALIGN_UP (header_size + core_size, image_target->section_align); -- 2.12.3 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] mkimage: Simplify header size logic 2018-12-23 2:52 ` [PATCH v2 1/2] mkimage: Simplify header size logic Alexander Graf @ 2019-01-14 13:29 ` Daniel Kiper 0 siblings, 0 replies; 8+ messages in thread From: Daniel Kiper @ 2019-01-14 13:29 UTC (permalink / raw) To: Alexander Graf; +Cc: grub-devel, Jon Masters, Leif Lindholm, Ard Biesheuvel On Sun, Dec 23, 2018 at 03:52:06AM +0100, Alexander Graf wrote: > For EFI images, we always have the following layout: > > [PE header] > [padding] > [first section (which also is the entry point)] > > Currently there are 2 places where we define how big header+padding are: > in the .vaddr_offset member of our target image definition struct as well > as in code in grub_install_generate_image(). > > Remove the latter, so that we only have a single place to modify if we > need to change the padding. > > Signed-off-by: Alexander Graf <agraf@suse.de> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com> Daniel ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 2/2] mkimage: arm64-efi: Align first section to page 2018-12-23 2:52 [PATCH v2 0/2] arm64: Support HP Envy X2 Alexander Graf 2018-12-23 2:52 ` [PATCH v2 1/2] mkimage: Simplify header size logic Alexander Graf @ 2018-12-23 2:52 ` Alexander Graf 2019-01-14 13:37 ` Daniel Kiper 2019-01-07 9:58 ` [PATCH v2 0/2] arm64: Support HP Envy X2 Leif Lindholm 2 siblings, 1 reply; 8+ messages in thread From: Alexander Graf @ 2018-12-23 2:52 UTC (permalink / raw) To: grub-devel; +Cc: Leif Lindholm, Peter Jones, Jon Masters, Ard Biesheuvel In order to enforce NX semantics on non-code pages, UEFI firmware may require that all code is EFI_PAGE_SIZE (4k) aligned. A similar change has recently been applied to edk2 to accomodate for the same fact: https://lists.01.org/pipermail/edk2-devel/2018-December/033708.html This patch adapts grub to also implement the same alignment guarantees and thus ensures we can boot even when strict permission checks are in place. Signed-off-by: Alexander Graf <agraf@suse.de> --- v1 -> v2: - Mention only NX requirement in patch description - Use GRUB_EFI_PAGE_SIZE --- util/mkimage.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/util/mkimage.c b/util/mkimage.c index 88b991764..de93c5a13 100644 --- a/util/mkimage.c +++ b/util/mkimage.c @@ -39,6 +39,7 @@ #include <string.h> #include <stdlib.h> #include <assert.h> +#include <grub/efi/memory.h> #include <grub/efi/pe32.h> #include <grub/uboot/image.h> #include <grub/arm/reloc.h> @@ -623,7 +624,7 @@ static const struct grub_install_image_target_desc image_targets[] = .decompressor_uncompressed_size = TARGET_NO_FIELD, .decompressor_uncompressed_addr = TARGET_NO_FIELD, .section_align = GRUB_PE32_SECTION_ALIGNMENT, - .vaddr_offset = EFI64_HEADER_SIZE, + .vaddr_offset = GRUB_EFI_PAGE_SIZE, .pe_target = GRUB_PE32_MACHINE_ARM64, .elf_target = EM_AARCH64, }, -- 2.12.3 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] mkimage: arm64-efi: Align first section to page 2018-12-23 2:52 ` [PATCH v2 2/2] mkimage: arm64-efi: Align first section to page Alexander Graf @ 2019-01-14 13:37 ` Daniel Kiper 2019-01-14 13:41 ` Alexander Graf 0 siblings, 1 reply; 8+ messages in thread From: Daniel Kiper @ 2019-01-14 13:37 UTC (permalink / raw) To: Alexander Graf; +Cc: grub-devel, Jon Masters, Leif Lindholm, Ard Biesheuvel On Sun, Dec 23, 2018 at 03:52:07AM +0100, Alexander Graf wrote: > In order to enforce NX semantics on non-code pages, UEFI firmware > may require that all code is EFI_PAGE_SIZE (4k) aligned. A similar > change has recently been applied to edk2 to accomodate for the same > fact: > > https://lists.01.org/pipermail/edk2-devel/2018-December/033708.html > > This patch adapts grub to also implement the same alignment guarantees > and thus ensures we can boot even when strict permission checks are in > place. > > Signed-off-by: Alexander Graf <agraf@suse.de> > > --- > > v1 -> v2: > > - Mention only NX requirement in patch description > - Use GRUB_EFI_PAGE_SIZE > --- > util/mkimage.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/util/mkimage.c b/util/mkimage.c > index 88b991764..de93c5a13 100644 > --- a/util/mkimage.c > +++ b/util/mkimage.c > @@ -39,6 +39,7 @@ > #include <string.h> > #include <stdlib.h> > #include <assert.h> > +#include <grub/efi/memory.h> > #include <grub/efi/pe32.h> > #include <grub/uboot/image.h> > #include <grub/arm/reloc.h> > @@ -623,7 +624,7 @@ static const struct grub_install_image_target_desc image_targets[] = > .decompressor_uncompressed_size = TARGET_NO_FIELD, > .decompressor_uncompressed_addr = TARGET_NO_FIELD, > .section_align = GRUB_PE32_SECTION_ALIGNMENT, > - .vaddr_offset = EFI64_HEADER_SIZE, > + .vaddr_offset = GRUB_EFI_PAGE_SIZE, Nack. I think that we will soon have the same problem on other targtes. So, I would try this: #define EFI32_HEADER_SIZE ALIGN_UP (GRUB_PE32_MSDOS_STUB_SIZE \ + GRUB_PE32_SIGNATURE_SIZE \ + sizeof (struct grub_pe32_coff_header) \ + sizeof (struct grub_pe32_optional_header) \ + 4 * sizeof (struct grub_pe32_section_table), \ ALIGN_UP (GRUB_PE32_SECTION_ALIGNMENT, GRUB_EFI_PAGE_SIZE)) #define EFI64_HEADER_SIZE ALIGN_UP (GRUB_PE32_MSDOS_STUB_SIZE \ + GRUB_PE32_SIGNATURE_SIZE \ + sizeof (struct grub_pe32_coff_header) \ + sizeof (struct grub_pe64_optional_header) \ + 4 * sizeof (struct grub_pe32_section_table), \ ALIGN_UP (GRUB_PE32_SECTION_ALIGNMENT, GRUB_EFI_PAGE_SIZE)) And there is another problem with your proposal. What will happen if EFI64_HEADER_SIZE > GRUB_EFI_PAGE_SIZE? Additionally, why arm-efi is different? Daniel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] mkimage: arm64-efi: Align first section to page 2019-01-14 13:37 ` Daniel Kiper @ 2019-01-14 13:41 ` Alexander Graf 2019-01-14 14:21 ` Leif Lindholm 0 siblings, 1 reply; 8+ messages in thread From: Alexander Graf @ 2019-01-14 13:41 UTC (permalink / raw) To: Daniel Kiper; +Cc: grub-devel, Jon Masters, Leif Lindholm, Ard Biesheuvel On 01/14/2019 02:37 PM, Daniel Kiper wrote: > On Sun, Dec 23, 2018 at 03:52:07AM +0100, Alexander Graf wrote: >> In order to enforce NX semantics on non-code pages, UEFI firmware >> may require that all code is EFI_PAGE_SIZE (4k) aligned. A similar >> change has recently been applied to edk2 to accomodate for the same >> fact: >> >> https://lists.01.org/pipermail/edk2-devel/2018-December/033708.html >> >> This patch adapts grub to also implement the same alignment guarantees >> and thus ensures we can boot even when strict permission checks are in >> place. >> >> Signed-off-by: Alexander Graf <agraf@suse.de> >> >> --- >> >> v1 -> v2: >> >> - Mention only NX requirement in patch description >> - Use GRUB_EFI_PAGE_SIZE >> --- >> util/mkimage.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/util/mkimage.c b/util/mkimage.c >> index 88b991764..de93c5a13 100644 >> --- a/util/mkimage.c >> +++ b/util/mkimage.c >> @@ -39,6 +39,7 @@ >> #include <string.h> >> #include <stdlib.h> >> #include <assert.h> >> +#include <grub/efi/memory.h> >> #include <grub/efi/pe32.h> >> #include <grub/uboot/image.h> >> #include <grub/arm/reloc.h> >> @@ -623,7 +624,7 @@ static const struct grub_install_image_target_desc image_targets[] = >> .decompressor_uncompressed_size = TARGET_NO_FIELD, >> .decompressor_uncompressed_addr = TARGET_NO_FIELD, >> .section_align = GRUB_PE32_SECTION_ALIGNMENT, >> - .vaddr_offset = EFI64_HEADER_SIZE, >> + .vaddr_offset = GRUB_EFI_PAGE_SIZE, > Nack. > > I think that we will soon have the same problem on other targtes. > So, I would try this: > > #define EFI32_HEADER_SIZE ALIGN_UP (GRUB_PE32_MSDOS_STUB_SIZE \ > + GRUB_PE32_SIGNATURE_SIZE \ > + sizeof (struct grub_pe32_coff_header) \ > + sizeof (struct grub_pe32_optional_header) \ > + 4 * sizeof (struct grub_pe32_section_table), \ > ALIGN_UP (GRUB_PE32_SECTION_ALIGNMENT, GRUB_EFI_PAGE_SIZE)) > > #define EFI64_HEADER_SIZE ALIGN_UP (GRUB_PE32_MSDOS_STUB_SIZE \ > + GRUB_PE32_SIGNATURE_SIZE \ > + sizeof (struct grub_pe32_coff_header) \ > + sizeof (struct grub_pe64_optional_header) \ > + 4 * sizeof (struct grub_pe32_section_table), \ > ALIGN_UP (GRUB_PE32_SECTION_ALIGNMENT, GRUB_EFI_PAGE_SIZE)) > > And there is another problem with your proposal. What will happen > if EFI64_HEADER_SIZE > GRUB_EFI_PAGE_SIZE? I don't think it ever can be, can it? The header size is pretty constant. > Additionally, why arm-efi is different? Mostly because in the wild I have not seen anyone on non-aarch64 pull in page alignment requirements :). But yes, I'll be happy to redo the patch and make EFI binaries always 4k aligned. I also learned in an off-list mailing list thread, that newer versions of that Qcom firmware require 4k section alignments, so I will push for that across all targets too. That way we should be aligned with the MS compiler suite. Alex ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] mkimage: arm64-efi: Align first section to page 2019-01-14 13:41 ` Alexander Graf @ 2019-01-14 14:21 ` Leif Lindholm 0 siblings, 0 replies; 8+ messages in thread From: Leif Lindholm @ 2019-01-14 14:21 UTC (permalink / raw) To: Alexander Graf; +Cc: Daniel Kiper, grub-devel, Jon Masters, Ard Biesheuvel On Mon, Jan 14, 2019 at 02:41:30PM +0100, Alexander Graf wrote: > On 01/14/2019 02:37 PM, Daniel Kiper wrote: > > On Sun, Dec 23, 2018 at 03:52:07AM +0100, Alexander Graf wrote: > > > In order to enforce NX semantics on non-code pages, UEFI firmware > > > may require that all code is EFI_PAGE_SIZE (4k) aligned. A similar > > > change has recently been applied to edk2 to accomodate for the same > > > fact: > > > > > > https://lists.01.org/pipermail/edk2-devel/2018-December/033708.html > > > > > > This patch adapts grub to also implement the same alignment guarantees > > > and thus ensures we can boot even when strict permission checks are in > > > place. > > > > > > Signed-off-by: Alexander Graf <agraf@suse.de> > > > > > > --- > > > > > > v1 -> v2: > > > > > > - Mention only NX requirement in patch description > > > - Use GRUB_EFI_PAGE_SIZE > > > --- > > > util/mkimage.c | 3 ++- > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > diff --git a/util/mkimage.c b/util/mkimage.c > > > index 88b991764..de93c5a13 100644 > > > --- a/util/mkimage.c > > > +++ b/util/mkimage.c > > > @@ -39,6 +39,7 @@ > > > #include <string.h> > > > #include <stdlib.h> > > > #include <assert.h> > > > +#include <grub/efi/memory.h> > > > #include <grub/efi/pe32.h> > > > #include <grub/uboot/image.h> > > > #include <grub/arm/reloc.h> > > > @@ -623,7 +624,7 @@ static const struct grub_install_image_target_desc image_targets[] = > > > .decompressor_uncompressed_size = TARGET_NO_FIELD, > > > .decompressor_uncompressed_addr = TARGET_NO_FIELD, > > > .section_align = GRUB_PE32_SECTION_ALIGNMENT, > > > - .vaddr_offset = EFI64_HEADER_SIZE, > > > + .vaddr_offset = GRUB_EFI_PAGE_SIZE, > > Nack. > > > > I think that we will soon have the same problem on other targtes. > > So, I would try this: > > > > #define EFI32_HEADER_SIZE ALIGN_UP (GRUB_PE32_MSDOS_STUB_SIZE \ > > + GRUB_PE32_SIGNATURE_SIZE \ > > + sizeof (struct grub_pe32_coff_header) \ > > + sizeof (struct grub_pe32_optional_header) \ > > + 4 * sizeof (struct grub_pe32_section_table), \ > > ALIGN_UP (GRUB_PE32_SECTION_ALIGNMENT, GRUB_EFI_PAGE_SIZE)) > > > > #define EFI64_HEADER_SIZE ALIGN_UP (GRUB_PE32_MSDOS_STUB_SIZE \ > > + GRUB_PE32_SIGNATURE_SIZE \ > > + sizeof (struct grub_pe32_coff_header) \ > > + sizeof (struct grub_pe64_optional_header) \ > > + 4 * sizeof (struct grub_pe32_section_table), \ > > ALIGN_UP (GRUB_PE32_SECTION_ALIGNMENT, GRUB_EFI_PAGE_SIZE)) > > > > And there is another problem with your proposal. What will happen > > if EFI64_HEADER_SIZE > GRUB_EFI_PAGE_SIZE? > > I don't think it ever can be, can it? The header size is pretty constant. > > > Additionally, why arm-efi is different? > > Mostly because in the wild I have not seen anyone on non-aarch64 pull in > page alignment requirements :). arm-efi is mainly different in that I don't expect non-embedded 32-bit devices to show up. There would be nothing wrong in applying the same change. There is an argument for i386/x86_64 to do the same as arm64, but ... > But yes, I'll be happy to redo the patch and make EFI binaries always 4k > aligned. I also learned in an off-list mailing list thread, that newer > versions of that Qcom firmware require 4k section alignments, so I will push > for that across all targets too. That way we should be aligned with the MS > compiler suite. Ouch. But, yes, that would also make sense. / Leif ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 0/2] arm64: Support HP Envy X2 2018-12-23 2:52 [PATCH v2 0/2] arm64: Support HP Envy X2 Alexander Graf 2018-12-23 2:52 ` [PATCH v2 1/2] mkimage: Simplify header size logic Alexander Graf 2018-12-23 2:52 ` [PATCH v2 2/2] mkimage: arm64-efi: Align first section to page Alexander Graf @ 2019-01-07 9:58 ` Leif Lindholm 2 siblings, 0 replies; 8+ messages in thread From: Leif Lindholm @ 2019-01-07 9:58 UTC (permalink / raw) To: Alexander Graf; +Cc: grub-devel, Peter Jones, Jon Masters, Ard Biesheuvel On Sun, Dec 23, 2018 at 03:52:05AM +0100, Alexander Graf wrote: > I got a new toy this week: An HP Envy X2 system. This is one of those shiny > new Qualcomm Snapdragon based Windows tablet/notebook hybrid things. > > While running Windows on those is actually not a terribly bad experience now > that WSL is out, I would like to see Linux run on those as well in the future. > > Unfortunately as far as I'm aware so far nobody was able to run self built > binaries on the built-in UEFI version. > > Turns out, it's a problem with aligning the start of the header to 4k. Once > we do that, binaries can be loaded just fine and run. > > The reason behind that is simple: Its firmware tries to ensure NX protection > flags and can do so only when the code is 4K aligned. > > So to maintain compatibility with that device, this patch set just bumps the > header alignment to 4K always on arm64-efi. This way we improve overall > compatibility - there surely will be more devices coming with similar > constraints. > > v1 -> v2: > > - Remove explicit device wording from patch > - Use GRUB_EFI_PAGE_SIZE Looks good to me, thanks! Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org> Tested-by: Leif Lindholm <leif.lindholm@linaro.org> > Alexander Graf (2): > mkimage: Simplify header size logic > mkimage: arm64-efi: Align first section to page > > util/mkimage.c | 8 +++----- > 1 file changed, 3 insertions(+), 5 deletions(-) > > -- > 2.12.3 > ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-01-14 14:22 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-12-23 2:52 [PATCH v2 0/2] arm64: Support HP Envy X2 Alexander Graf 2018-12-23 2:52 ` [PATCH v2 1/2] mkimage: Simplify header size logic Alexander Graf 2019-01-14 13:29 ` Daniel Kiper 2018-12-23 2:52 ` [PATCH v2 2/2] mkimage: arm64-efi: Align first section to page Alexander Graf 2019-01-14 13:37 ` Daniel Kiper 2019-01-14 13:41 ` Alexander Graf 2019-01-14 14:21 ` Leif Lindholm 2019-01-07 9:58 ` [PATCH v2 0/2] arm64: Support HP Envy X2 Leif Lindholm
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.