Big endian modifications

Big endian modifications

[Rory Bolt]

Hello,

I have been modifying GRUB on Das U-boot to allow it to boot aarch64_be big endian kernels on the RockPro64 board. Since the RockPro64 has a PCIe (albeit 2.0) slot, it provides a low cost/reasonable performance (vs QEMU) system to test our devices, drivers and software stack on a big endian system. I am in the process of publishing the required changes to GRUB, however there are two minor changes made to the kernel itself. Arguably the modification to head.S could be moved to grub itself, however it is in the defensive spirit of not assuming the boot loader has set up everything correctly and is very similar to the startup of NetBSD which contains similar code in its startup. The second change to Kconfig allows a EFI/PE/COFF header to be added to a big endian kernel (although unfortunately it also adds a useless big endian efistub to the kernel too). With the following two changes the modified GRUB can boot either big endian or little endian Arm64 images.

Please consider the following two modifications for inclusion:

diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index 6a98f1a38c29..40a18d767d15 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -89,6 +89,24 @@
         *  x24        __primary_switch() .. relocate_kernel()  current RELR displacement
         */
 SYM_CODE_START(primary_entry)
+#ifdef CONFIG_CPU_BIG_ENDIAN
+        mrs     x21, CurrentEL
+        lsr     x21, x21, #2
+        cmp     x21, #0x2
+        b.lo    1f
+
+        mrs     x21, sctlr_el2
+        orr     x21, x21, #SCTLR_ELx_EE /* set: Big Endian */
+        msr     sctlr_el2, x21
+        isb
+
+1:
+        mrs     x21, sctlr_el1
+        orr     x21, x21, #(SCTLR_ELx_EE | SCTLR_EL1_E0E)       /* set: Big Endian */
+        msr     sctlr_el1, x21
+        isb
+
+#endif
        bl      preserve_boot_args
        bl      init_kernel_el                  // w0=cpu_boot_mode
        adrp    x23, __PHYS_OFFSET


diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index c4207cf9bb17..a9ccbeb75ea7 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -1997,7 +1997,7 @@ config EFI_STUB

 config EFI
        bool "UEFI runtime support"
-       depends on OF && !CPU_BIG_ENDIAN
+       depends on OF
        depends on KERNEL_MODE_NEON
        select ARCH_SUPPORTS_ACPI
        select LIBFDT

Rory Bolt

KIOXIA America, Inc. | formerly Toshiba Memory America, Inc.






_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: Big endian modifications

[Mark Rutland]

Hi,

On Thu, Feb 03, 2022 at 06:48:43PM +0000, Rory Bolt wrote:
> Hello,
> 
> I have been modifying GRUB on Das U-boot to allow it to boot aarch64_be big
> endian kernels on the RockPro64 board. Since the RockPro64 has a PCIe (albeit
> 2.0) slot, it provides a low cost/reasonable performance (vs QEMU) system to
> test our devices, drivers and software stack on a big endian system. I am in
> the process of publishing the required changes to GRUB, however there are two
> minor changes made to the kernel itself.

Kernel patches need to be submitted in a particular format, with a commit
title, commit message, and DCO (Signed-off-by) lines. Please see:

  https://docs.kernel.org/process/submitting-patches.html

  https://docs.kernel.org/process/development-process.html

It's also worthwhile looking at the MAINTAINERS file to Cc relevant people. For
example, Catalin and Will are the maintainers of the arm64 port, and Ard, Marc,
and I have made various changes to this code.

> Arguably the modification to head.S could be moved to grub itself, however it
> is in the defensive spirit of not assuming the boot loader has set up
> everything correctly and is very similar to the startup of NetBSD which
> contains similar code in its startup.

Hmm... big-endian boot *should* already work. The kernel configures its
endianness in init_kernel_el(), and from testing a BE v5.17-rc2 kernel just now
with a FVP Base model and the boot-wrapper (which would leave the CPU in a
little-endian state when jumping to the kernel) things seem to work.

Which kernel version are you testing with, and how *exactly* is the kernel
being entered?

Is it being entered directly by the protocol defined at:

  https://docs.kernel.org/arm64/booting.html

... or some other way (e.g. via the EFI application entry point?).

> The second change to Kconfig allows a EFI/PE/COFF header to be added to a big
> endian kernel (although unfortunately it also adds a useless big endian
> efistub to the kernel too). With the following two changes the modified GRUB
> can boot either big endian or little endian Arm64 images.

This implies that grub is trying to enter the EFI entry point, and a bunch of
things aren't going to work thereafter.

What have you changed on the GRUB side? Because I don't understand why it would
*need* changes, if the PE/COFF header and EFI stub handled being entered in LE,
then switching to BE. That and it should be possible to boot a BE kernel
without neding a PE/COFF header, so something doesn't add up.

> Please consider the following two modifications for inclusion:

As above, we can't take this as-is, but if you've found a regression I'm more
than happy to try to get that fixed. If we *really* need to be able to boot
BE kernels from EFI, there might be things that we can do, but I don't think we
can just remove the CONFIG_EFI dependency on !CPU_BIG_ENDIAN -- the runtime
bits *certainly* won't work, at minimum the stub needs to be built LE, etc.

Also, have you considered running your kernel under KVM, with devices assigned
to it? That might be significantly easier than having a BE kernel that handles
EFI.

Thanks,
Mark.

> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
> index 6a98f1a38c29..40a18d767d15 100644
> --- a/arch/arm64/kernel/head.S
> +++ b/arch/arm64/kernel/head.S
> @@ -89,6 +89,24 @@
>          *  x24        __primary_switch() .. relocate_kernel()  current RELR displacement
>          */
>  SYM_CODE_START(primary_entry)
> +#ifdef CONFIG_CPU_BIG_ENDIAN
> +        mrs     x21, CurrentEL
> +        lsr     x21, x21, #2
> +        cmp     x21, #0x2
> +        b.lo    1f
> +
> +        mrs     x21, sctlr_el2
> +        orr     x21, x21, #SCTLR_ELx_EE /* set: Big Endian */
> +        msr     sctlr_el2, x21
> +        isb
> +
> +1:
> +        mrs     x21, sctlr_el1
> +        orr     x21, x21, #(SCTLR_ELx_EE | SCTLR_EL1_E0E)       /* set: Big Endian */
> +        msr     sctlr_el1, x21
> +        isb
> +
> +#endif
>         bl      preserve_boot_args
>         bl      init_kernel_el                  // w0=cpu_boot_mode
>         adrp    x23, __PHYS_OFFSET
> 
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index c4207cf9bb17..a9ccbeb75ea7 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -1997,7 +1997,7 @@ config EFI_STUB
> 
>  config EFI
>         bool "UEFI runtime support"
> -       depends on OF && !CPU_BIG_ENDIAN
> +       depends on OF
>         depends on KERNEL_MODE_NEON
>         select ARCH_SUPPORTS_ACPI
>         select LIBFDT
> 
> Rory Bolt
> 
> KIOXIA America, Inc. | formerly Toshiba Memory America, Inc.
> 
> 
> 
> 
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: Big endian modifications

[Robin Murphy]

Hi Rory,

On 2022-02-03 18:48, Rory Bolt wrote:
> Hello,
> 
> I have been modifying GRUB on Das U-boot to allow it to boot aarch64_be big endian kernels on the RockPro64 board. Since the RockPro64 has a PCIe (albeit 2.0) slot, it provides a low cost/reasonable performance (vs QEMU) system to test our devices, drivers and software stack on a big endian system. I am in the process of publishing the required changes to GRUB, however there are two minor changes made to the kernel itself. Arguably the modification to head.S could be moved to grub itself, however it is in the defensive spirit of not assuming the boot loader has set up everything correctly and is very similar to the startup of NetBSD which contains similar code in its startup. The second change to Kconfig allows a EFI/PE/COFF header to be added to a big endian kernel (although unfortunately it also adds a useless big endian efistub to the kernel too). With the following two changes the modified GRUB can boot either big endian or little endian Arm64 images.

FWIW I believe the previous thinking was that teaching GRUB to boot 
non-EFI images via the original arm64 boot protocol would be the most 
useful thing to do, since that would increase coverage and flexibility 
regardless of endianness.

However if you're using U-Boot, then booti ought to do exactly that with 
a standard BE image already, so the pragmatic option might be to just 
avoid the detour through EFI altogether.

> Please consider the following two modifications for inclusion:
> 
> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
> index 6a98f1a38c29..40a18d767d15 100644
> --- a/arch/arm64/kernel/head.S
> +++ b/arch/arm64/kernel/head.S
> @@ -89,6 +89,24 @@
>           *  x24        __primary_switch() .. relocate_kernel()  current RELR displacement
>           */
>   SYM_CODE_START(primary_entry)
> +#ifdef CONFIG_CPU_BIG_ENDIAN
> +        mrs     x21, CurrentEL
> +        lsr     x21, x21, #2
> +        cmp     x21, #0x2
> +        b.lo    1f
> +
> +        mrs     x21, sctlr_el2
> +        orr     x21, x21, #SCTLR_ELx_EE /* set: Big Endian */
> +        msr     sctlr_el2, x21
> +        isb
> +
> +1:
> +        mrs     x21, sctlr_el1
> +        orr     x21, x21, #(SCTLR_ELx_EE | SCTLR_EL1_E0E)       /* set: Big Endian */
> +        msr     sctlr_el1, x21
> +        isb
> +
> +#endif
>          bl      preserve_boot_args
>          bl      init_kernel_el                  // w0=cpu_boot_mode

Pretty much the first thing that init_kernel_el does is to install the 
appropriate INIT_SCTLR_EL[12]_MMU_OFF value which already contains the 
relevant ENDIAN_SET_EL* combinations, therefore this change looks like 
it shouldn't be needed. Have you seen an actual issue here, other than 
anything stemming from the completely broken and 
illegal-per-the-UEFI-spec big-endian EFI config?

It's been a while since I last tried booting a BE kernel (especially in 
bare-metal EL2 rather than a VM), but it definitely used to work fine.

Robin.

>          adrp    x23, __PHYS_OFFSET
> 
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index c4207cf9bb17..a9ccbeb75ea7 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -1997,7 +1997,7 @@ config EFI_STUB
> 
>   config EFI
>          bool "UEFI runtime support"
> -       depends on OF && !CPU_BIG_ENDIAN
> +       depends on OF
>          depends on KERNEL_MODE_NEON
>          select ARCH_SUPPORTS_ACPI
>          select LIBFDT
> 
> Rory Bolt
> 
> KIOXIA America, Inc. | formerly Toshiba Memory America, Inc.
> 
> 
> 
> 
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: Big endian modifications

[Robin Murphy]

On 2022-02-03 20:10, Mark Rutland wrote:
> Hi,
> 
> On Thu, Feb 03, 2022 at 06:48:43PM +0000, Rory Bolt wrote:
>> Hello,
>>
>> I have been modifying GRUB on Das U-boot to allow it to boot aarch64_be big
>> endian kernels on the RockPro64 board. Since the RockPro64 has a PCIe (albeit
>> 2.0) slot, it provides a low cost/reasonable performance (vs QEMU) system to
>> test our devices, drivers and software stack on a big endian system. I am in
>> the process of publishing the required changes to GRUB, however there are two
>> minor changes made to the kernel itself.
> 
> Kernel patches need to be submitted in a particular format, with a commit
> title, commit message, and DCO (Signed-off-by) lines. Please see:
> 
>    https://docs.kernel.org/process/submitting-patches.html
> 
>    https://docs.kernel.org/process/development-process.html
> 
> It's also worthwhile looking at the MAINTAINERS file to Cc relevant people. For
> example, Catalin and Will are the maintainers of the arm64 port, and Ard, Marc,
> and I have made various changes to this code.
> 
>> Arguably the modification to head.S could be moved to grub itself, however it
>> is in the defensive spirit of not assuming the boot loader has set up
>> everything correctly and is very similar to the startup of NetBSD which
>> contains similar code in its startup.
> 
> Hmm... big-endian boot *should* already work. The kernel configures its
> endianness in init_kernel_el(), and from testing a BE v5.17-rc2 kernel just now
> with a FVP Base model and the boot-wrapper (which would leave the CPU in a
> little-endian state when jumping to the kernel) things seem to work.
> 
> Which kernel version are you testing with, and how *exactly* is the kernel
> being entered?
> 
> Is it being entered directly by the protocol defined at:
> 
>    https://docs.kernel.org/arm64/booting.html
> 
> ... or some other way (e.g. via the EFI application entry point?).
> 
>> The second change to Kconfig allows a EFI/PE/COFF header to be added to a big
>> endian kernel (although unfortunately it also adds a useless big endian
>> efistub to the kernel too). With the following two changes the modified GRUB
>> can boot either big endian or little endian Arm64 images.
> 
> This implies that grub is trying to enter the EFI entry point, and a bunch of
> things aren't going to work thereafter.
> 
> What have you changed on the GRUB side? Because I don't understand why it would
> *need* changes, if the PE/COFF header and EFI stub handled being entered in LE,
> then switching to BE. That and it should be possible to boot a BE kernel
> without neding a PE/COFF header, so something doesn't add up.

AFAIK arm64 GRUB only supports calling Linux as an EFI application.

>> Please consider the following two modifications for inclusion:
> 
> As above, we can't take this as-is, but if you've found a regression I'm more
> than happy to try to get that fixed. If we *really* need to be able to boot
> BE kernels from EFI, there might be things that we can do, but I don't think we
> can just remove the CONFIG_EFI dependency on !CPU_BIG_ENDIAN -- the runtime
> bits *certainly* won't work, at minimum the stub needs to be built LE, etc.
> 
> Also, have you considered running your kernel under KVM, with devices assigned
> to it? That might be significantly easier than having a BE kernel that handles
> EFI.

Sadly that's not an option on RockPro64 since there's no suitable IOMMU. 
The number of affordable AArch64 platforms actually capable of device 
assignment is still disappointingly close to none :(

Robin.

> 
> Thanks,
> Mark.
> 
>> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
>> index 6a98f1a38c29..40a18d767d15 100644
>> --- a/arch/arm64/kernel/head.S
>> +++ b/arch/arm64/kernel/head.S
>> @@ -89,6 +89,24 @@
>>           *  x24        __primary_switch() .. relocate_kernel()  current RELR displacement
>>           */
>>   SYM_CODE_START(primary_entry)
>> +#ifdef CONFIG_CPU_BIG_ENDIAN
>> +        mrs     x21, CurrentEL
>> +        lsr     x21, x21, #2
>> +        cmp     x21, #0x2
>> +        b.lo    1f
>> +
>> +        mrs     x21, sctlr_el2
>> +        orr     x21, x21, #SCTLR_ELx_EE /* set: Big Endian */
>> +        msr     sctlr_el2, x21
>> +        isb
>> +
>> +1:
>> +        mrs     x21, sctlr_el1
>> +        orr     x21, x21, #(SCTLR_ELx_EE | SCTLR_EL1_E0E)       /* set: Big Endian */
>> +        msr     sctlr_el1, x21
>> +        isb
>> +
>> +#endif
>>          bl      preserve_boot_args
>>          bl      init_kernel_el                  // w0=cpu_boot_mode
>>          adrp    x23, __PHYS_OFFSET
>>
>>
>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>> index c4207cf9bb17..a9ccbeb75ea7 100644
>> --- a/arch/arm64/Kconfig
>> +++ b/arch/arm64/Kconfig
>> @@ -1997,7 +1997,7 @@ config EFI_STUB
>>
>>   config EFI
>>          bool "UEFI runtime support"
>> -       depends on OF && !CPU_BIG_ENDIAN
>> +       depends on OF
>>          depends on KERNEL_MODE_NEON
>>          select ARCH_SUPPORTS_ACPI
>>          select LIBFDT
>>
>> Rory Bolt
>>
>> KIOXIA America, Inc. | formerly Toshiba Memory America, Inc.
>>
>>
>>
>>
>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: Big endian modifications

[Rory Bolt]

Mark,
Thank you for your prompt and valuable feedback. If I end up submitting changes I will be sure to follow the guidelines you provided.

I missed the fact that init_kernel_el() is already setting the endian flag via the INIT_SCTLR_ELx_MMU_OFF constant, so you are correct that my change to head.S should not be needed. I will remove it, retest, and follow up if there is any issue (which I doubt).

I am testing with 5.16.5.

I extracted the linux efistub, recompiled it little endian, and linked it into the grub standalone executable. The boot path is U-boot->grub->external_little_endian_efi_stub->primary_entry. Since it is the same efistub that is embedded normally for Linux, it performs all the necessary initialization.

The system is working well, albeit EFI runtime services are not available (as is expected).

Unfortunately PCIe passthrough is not an option due to hardware constraints for this platform. (e.g. IOMMU limitations).

Unless I have made (another) oversight, GRUB only supports Linux as an EFI application, so the modification I have made are to move the EFIstub functionality from within the kernel to being part of grub.

Thank you again,
-Rory

-----Original Message-----
From: Mark Rutland <mark.rutland@arm.com> 
Sent: Thursday, February 3, 2022 12:11 PM
To: Rory Bolt <Rory.Bolt@kioxia.com>
Cc: linux-arm-kernel@lists.infradead.org; ardb@kernel.org; maz@kernel.org; will@kernel.org; catalin.marinas@arm.com
Subject: Re: Big endian modifications

Hi,

On Thu, Feb 03, 2022 at 06:48:43PM +0000, Rory Bolt wrote:
> Hello,
> 
> I have been modifying GRUB on Das U-boot to allow it to boot 
> aarch64_be big endian kernels on the RockPro64 board. Since the 
> RockPro64 has a PCIe (albeit
> 2.0) slot, it provides a low cost/reasonable performance (vs QEMU) 
> system to test our devices, drivers and software stack on a big endian 
> system. I am in the process of publishing the required changes to 
> GRUB, however there are two minor changes made to the kernel itself.

Kernel patches need to be submitted in a particular format, with a commit title, commit message, and DCO (Signed-off-by) lines. Please see:

  https://docs.kernel.org/process/submitting-patches.html

  https://docs.kernel.org/process/development-process.html

It's also worthwhile looking at the MAINTAINERS file to Cc relevant people. For example, Catalin and Will are the maintainers of the arm64 port, and Ard, Marc, and I have made various changes to this code.

> Arguably the modification to head.S could be moved to grub itself, 
> however it is in the defensive spirit of not assuming the boot loader 
> has set up everything correctly and is very similar to the startup of 
> NetBSD which contains similar code in its startup.

Hmm... big-endian boot *should* already work. The kernel configures its endianness in init_kernel_el(), and from testing a BE v5.17-rc2 kernel just now with a FVP Base model and the boot-wrapper (which would leave the CPU in a little-endian state when jumping to the kernel) things seem to work.

Which kernel version are you testing with, and how *exactly* is the kernel being entered?

Is it being entered directly by the protocol defined at:

  https://docs.kernel.org/arm64/booting.html

... or some other way (e.g. via the EFI application entry point?).

> The second change to Kconfig allows a EFI/PE/COFF header to be added 
> to a big endian kernel (although unfortunately it also adds a useless 
> big endian efistub to the kernel too). With the following two changes 
> the modified GRUB can boot either big endian or little endian Arm64 images.

This implies that grub is trying to enter the EFI entry point, and a bunch of things aren't going to work thereafter.

What have you changed on the GRUB side? Because I don't understand why it would
*need* changes, if the PE/COFF header and EFI stub handled being entered in LE, then switching to BE. That and it should be possible to boot a BE kernel without neding a PE/COFF header, so something doesn't add up.

> Please consider the following two modifications for inclusion:

As above, we can't take this as-is, but if you've found a regression I'm more than happy to try to get that fixed. If we *really* need to be able to boot BE kernels from EFI, there might be things that we can do, but I don't think we can just remove the CONFIG_EFI dependency on !CPU_BIG_ENDIAN -- the runtime bits *certainly* won't work, at minimum the stub needs to be built LE, etc.

Also, have you considered running your kernel under KVM, with devices assigned to it? That might be significantly easier than having a BE kernel that handles EFI.

Thanks,
Mark.

> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S index 
> 6a98f1a38c29..40a18d767d15 100644
> --- a/arch/arm64/kernel/head.S
> +++ b/arch/arm64/kernel/head.S
> @@ -89,6 +89,24 @@
>          *  x24        __primary_switch() .. relocate_kernel()  current RELR displacement
>          */
>  SYM_CODE_START(primary_entry)
> +#ifdef CONFIG_CPU_BIG_ENDIAN
> +        mrs     x21, CurrentEL
> +        lsr     x21, x21, #2
> +        cmp     x21, #0x2
> +        b.lo    1f
> +
> +        mrs     x21, sctlr_el2
> +        orr     x21, x21, #SCTLR_ELx_EE /* set: Big Endian */
> +        msr     sctlr_el2, x21
> +        isb
> +
> +1:
> +        mrs     x21, sctlr_el1
> +        orr     x21, x21, #(SCTLR_ELx_EE | SCTLR_EL1_E0E)       /* set: Big Endian */
> +        msr     sctlr_el1, x21
> +        isb
> +
> +#endif
>         bl      preserve_boot_args
>         bl      init_kernel_el                  // w0=cpu_boot_mode
>         adrp    x23, __PHYS_OFFSET
> 
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index 
> c4207cf9bb17..a9ccbeb75ea7 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -1997,7 +1997,7 @@ config EFI_STUB
> 
>  config EFI
>         bool "UEFI runtime support"
> -       depends on OF && !CPU_BIG_ENDIAN
> +       depends on OF
>         depends on KERNEL_MODE_NEON
>         select ARCH_SUPPORTS_ACPI
>         select LIBFDT
> 
> Rory Bolt
> 
> KIOXIA America, Inc. | formerly Toshiba Memory America, Inc.
> 
> 
> 
> 
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: Big endian modifications

[Rory Bolt]

Wow. I am unfamiliar with booti - I will investigate. Thank you for the pointer! Believe me when I say the detour through EFI was interesting...

-Rory

-----Original Message-----
From: Robin Murphy <robin.murphy@arm.com> 
Sent: Thursday, February 3, 2022 12:46 PM
To: Rory Bolt <Rory.Bolt@kioxia.com>; linux-arm-kernel@lists.infradead.org
Subject: Re: Big endian modifications

Hi Rory,

On 2022-02-03 18:48, Rory Bolt wrote:
> Hello,
> 
> I have been modifying GRUB on Das U-boot to allow it to boot aarch64_be big endian kernels on the RockPro64 board. Since the RockPro64 has a PCIe (albeit 2.0) slot, it provides a low cost/reasonable performance (vs QEMU) system to test our devices, drivers and software stack on a big endian system. I am in the process of publishing the required changes to GRUB, however there are two minor changes made to the kernel itself. Arguably the modification to head.S could be moved to grub itself, however it is in the defensive spirit of not assuming the boot loader has set up everything correctly and is very similar to the startup of NetBSD which contains similar code in its startup. The second change to Kconfig allows a EFI/PE/COFF header to be added to a big endian kernel (although unfortunately it also adds a useless big endian efistub to the kernel too). With the following two changes the modified GRUB can boot either big endian or little endian Arm64 images.

FWIW I believe the previous thinking was that teaching GRUB to boot non-EFI images via the original arm64 boot protocol would be the most useful thing to do, since that would increase coverage and flexibility regardless of endianness.

However if you're using U-Boot, then booti ought to do exactly that with a standard BE image already, so the pragmatic option might be to just avoid the detour through EFI altogether.

> Please consider the following two modifications for inclusion:
> 
> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S index 
> 6a98f1a38c29..40a18d767d15 100644
> --- a/arch/arm64/kernel/head.S
> +++ b/arch/arm64/kernel/head.S
> @@ -89,6 +89,24 @@
>           *  x24        __primary_switch() .. relocate_kernel()  current RELR displacement
>           */
>   SYM_CODE_START(primary_entry)
> +#ifdef CONFIG_CPU_BIG_ENDIAN
> +        mrs     x21, CurrentEL
> +        lsr     x21, x21, #2
> +        cmp     x21, #0x2
> +        b.lo    1f
> +
> +        mrs     x21, sctlr_el2
> +        orr     x21, x21, #SCTLR_ELx_EE /* set: Big Endian */
> +        msr     sctlr_el2, x21
> +        isb
> +
> +1:
> +        mrs     x21, sctlr_el1
> +        orr     x21, x21, #(SCTLR_ELx_EE | SCTLR_EL1_E0E)       /* set: Big Endian */
> +        msr     sctlr_el1, x21
> +        isb
> +
> +#endif
>          bl      preserve_boot_args
>          bl      init_kernel_el                  // w0=cpu_boot_mode

Pretty much the first thing that init_kernel_el does is to install the appropriate INIT_SCTLR_EL[12]_MMU_OFF value which already contains the relevant ENDIAN_SET_EL* combinations, therefore this change looks like it shouldn't be needed. Have you seen an actual issue here, other than anything stemming from the completely broken and illegal-per-the-UEFI-spec big-endian EFI config?

It's been a while since I last tried booting a BE kernel (especially in bare-metal EL2 rather than a VM), but it definitely used to work fine.

Robin.

>          adrp    x23, __PHYS_OFFSET
> 
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index 
> c4207cf9bb17..a9ccbeb75ea7 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -1997,7 +1997,7 @@ config EFI_STUB
> 
>   config EFI
>          bool "UEFI runtime support"
> -       depends on OF && !CPU_BIG_ENDIAN
> +       depends on OF
>          depends on KERNEL_MODE_NEON
>          select ARCH_SUPPORTS_ACPI
>          select LIBFDT
> 
> Rory Bolt
> 
> KIOXIA America, Inc. | formerly Toshiba Memory America, Inc.
> 
> 
> 
> 
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: Big endian modifications

[Rory Bolt]

Mark,

I tested without my patch and the kernel does run big endian, however:

The reason I thought my patch was necessary in the first place was that upon kernel entry, before the call to init_kernel_el(), the first call is to preserve_boot_args() which writes the contents of x0..x3 to memory... which will be done in little endian format. Fortunately, no one currently uses the values stored in boot_args for anything other than a check to see that x1..x3 were set to zero since they are reserved for future use. This works now, since they are reserved and set to zero which is not endian sensitive, however in the future if any additional boot args are passed in x1..x3 they will not be stored in the correct byte order.

Should this be addressed now, or in the future?

-Rory

-----Original Message-----
From: Mark Rutland <mark.rutland@arm.com> 
Sent: Thursday, February 3, 2022 12:11 PM
To: Rory Bolt <Rory.Bolt@kioxia.com>
Cc: linux-arm-kernel@lists.infradead.org; ardb@kernel.org; maz@kernel.org; will@kernel.org; catalin.marinas@arm.com
Subject: Re: Big endian modifications

Hi,

On Thu, Feb 03, 2022 at 06:48:43PM +0000, Rory Bolt wrote:
> Hello,
> 
> I have been modifying GRUB on Das U-boot to allow it to boot 
> aarch64_be big endian kernels on the RockPro64 board. Since the 
> RockPro64 has a PCIe (albeit
> 2.0) slot, it provides a low cost/reasonable performance (vs QEMU) 
> system to test our devices, drivers and software stack on a big endian 
> system. I am in the process of publishing the required changes to 
> GRUB, however there are two minor changes made to the kernel itself.

Kernel patches need to be submitted in a particular format, with a commit title, commit message, and DCO (Signed-off-by) lines. Please see:

  https://docs.kernel.org/process/submitting-patches.html

  https://docs.kernel.org/process/development-process.html

It's also worthwhile looking at the MAINTAINERS file to Cc relevant people. For example, Catalin and Will are the maintainers of the arm64 port, and Ard, Marc, and I have made various changes to this code.

> Arguably the modification to head.S could be moved to grub itself, 
> however it is in the defensive spirit of not assuming the boot loader 
> has set up everything correctly and is very similar to the startup of 
> NetBSD which contains similar code in its startup.

Hmm... big-endian boot *should* already work. The kernel configures its endianness in init_kernel_el(), and from testing a BE v5.17-rc2 kernel just now with a FVP Base model and the boot-wrapper (which would leave the CPU in a little-endian state when jumping to the kernel) things seem to work.

Which kernel version are you testing with, and how *exactly* is the kernel being entered?

Is it being entered directly by the protocol defined at:

  https://docs.kernel.org/arm64/booting.html

... or some other way (e.g. via the EFI application entry point?).

> The second change to Kconfig allows a EFI/PE/COFF header to be added 
> to a big endian kernel (although unfortunately it also adds a useless 
> big endian efistub to the kernel too). With the following two changes 
> the modified GRUB can boot either big endian or little endian Arm64 images.

This implies that grub is trying to enter the EFI entry point, and a bunch of things aren't going to work thereafter.

What have you changed on the GRUB side? Because I don't understand why it would
*need* changes, if the PE/COFF header and EFI stub handled being entered in LE, then switching to BE. That and it should be possible to boot a BE kernel without neding a PE/COFF header, so something doesn't add up.

> Please consider the following two modifications for inclusion:

As above, we can't take this as-is, but if you've found a regression I'm more than happy to try to get that fixed. If we *really* need to be able to boot BE kernels from EFI, there might be things that we can do, but I don't think we can just remove the CONFIG_EFI dependency on !CPU_BIG_ENDIAN -- the runtime bits *certainly* won't work, at minimum the stub needs to be built LE, etc.

Also, have you considered running your kernel under KVM, with devices assigned to it? That might be significantly easier than having a BE kernel that handles EFI.

Thanks,
Mark.

> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S index 
> 6a98f1a38c29..40a18d767d15 100644
> --- a/arch/arm64/kernel/head.S
> +++ b/arch/arm64/kernel/head.S
> @@ -89,6 +89,24 @@
>          *  x24        __primary_switch() .. relocate_kernel()  current RELR displacement
>          */
>  SYM_CODE_START(primary_entry)
> +#ifdef CONFIG_CPU_BIG_ENDIAN
> +        mrs     x21, CurrentEL
> +        lsr     x21, x21, #2
> +        cmp     x21, #0x2
> +        b.lo    1f
> +
> +        mrs     x21, sctlr_el2
> +        orr     x21, x21, #SCTLR_ELx_EE /* set: Big Endian */
> +        msr     sctlr_el2, x21
> +        isb
> +
> +1:
> +        mrs     x21, sctlr_el1
> +        orr     x21, x21, #(SCTLR_ELx_EE | SCTLR_EL1_E0E)       /* set: Big Endian */
> +        msr     sctlr_el1, x21
> +        isb
> +
> +#endif
>         bl      preserve_boot_args
>         bl      init_kernel_el                  // w0=cpu_boot_mode
>         adrp    x23, __PHYS_OFFSET
> 
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index 
> c4207cf9bb17..a9ccbeb75ea7 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -1997,7 +1997,7 @@ config EFI_STUB
> 
>  config EFI
>         bool "UEFI runtime support"
> -       depends on OF && !CPU_BIG_ENDIAN
> +       depends on OF
>         depends on KERNEL_MODE_NEON
>         select ARCH_SUPPORTS_ACPI
>         select LIBFDT
> 
> Rory Bolt
> 
> KIOXIA America, Inc. | formerly Toshiba Memory America, Inc.
> 
> 
> 
> 
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: Big endian modifications

[Rory Bolt]

Slight correction: the boot args will be stored in whatever endianness the boot loader left the machine in, not always little endian. The point is, if they are ever needed in the future they will be in stored in an order that may not match the running kernel.

-----Original Message-----
From: linux-arm-kernel <linux-arm-kernel-bounces@lists.infradead.org> On Behalf Of Rory Bolt
Sent: Friday, February 4, 2022 9:59 AM
To: Mark Rutland <mark.rutland@arm.com>
Cc: linux-arm-kernel@lists.infradead.org; ardb@kernel.org; maz@kernel.org; will@kernel.org; catalin.marinas@arm.com
Subject: RE: Big endian modifications

Mark,

I tested without my patch and the kernel does run big endian, however:

The reason I thought my patch was necessary in the first place was that upon kernel entry, before the call to init_kernel_el(), the first call is to preserve_boot_args() which writes the contents of x0..x3 to memory... which will be done in little endian format. Fortunately, no one currently uses the values stored in boot_args for anything other than a check to see that x1..x3 were set to zero since they are reserved for future use. This works now, since they are reserved and set to zero which is not endian sensitive, however in the future if any additional boot args are passed in x1..x3 they will not be stored in the correct byte order.

Should this be addressed now, or in the future?

-Rory

-----Original Message-----
From: Mark Rutland <mark.rutland@arm.com>
Sent: Thursday, February 3, 2022 12:11 PM
To: Rory Bolt <Rory.Bolt@kioxia.com>
Cc: linux-arm-kernel@lists.infradead.org; ardb@kernel.org; maz@kernel.org; will@kernel.org; catalin.marinas@arm.com
Subject: Re: Big endian modifications

Hi,

On Thu, Feb 03, 2022 at 06:48:43PM +0000, Rory Bolt wrote:
> Hello,
> 
> I have been modifying GRUB on Das U-boot to allow it to boot 
> aarch64_be big endian kernels on the RockPro64 board. Since the
> RockPro64 has a PCIe (albeit
> 2.0) slot, it provides a low cost/reasonable performance (vs QEMU) 
> system to test our devices, drivers and software stack on a big endian 
> system. I am in the process of publishing the required changes to 
> GRUB, however there are two minor changes made to the kernel itself.

Kernel patches need to be submitted in a particular format, with a commit title, commit message, and DCO (Signed-off-by) lines. Please see:

  https://docs.kernel.org/process/submitting-patches.html

  https://docs.kernel.org/process/development-process.html

It's also worthwhile looking at the MAINTAINERS file to Cc relevant people. For example, Catalin and Will are the maintainers of the arm64 port, and Ard, Marc, and I have made various changes to this code.

> Arguably the modification to head.S could be moved to grub itself, 
> however it is in the defensive spirit of not assuming the boot loader 
> has set up everything correctly and is very similar to the startup of 
> NetBSD which contains similar code in its startup.

Hmm... big-endian boot *should* already work. The kernel configures its endianness in init_kernel_el(), and from testing a BE v5.17-rc2 kernel just now with a FVP Base model and the boot-wrapper (which would leave the CPU in a little-endian state when jumping to the kernel) things seem to work.

Which kernel version are you testing with, and how *exactly* is the kernel being entered?

Is it being entered directly by the protocol defined at:

  https://docs.kernel.org/arm64/booting.html

... or some other way (e.g. via the EFI application entry point?).

> The second change to Kconfig allows a EFI/PE/COFF header to be added 
> to a big endian kernel (although unfortunately it also adds a useless 
> big endian efistub to the kernel too). With the following two changes 
> the modified GRUB can boot either big endian or little endian Arm64 images.

This implies that grub is trying to enter the EFI entry point, and a bunch of things aren't going to work thereafter.

What have you changed on the GRUB side? Because I don't understand why it would
*need* changes, if the PE/COFF header and EFI stub handled being entered in LE, then switching to BE. That and it should be possible to boot a BE kernel without neding a PE/COFF header, so something doesn't add up.

> Please consider the following two modifications for inclusion:

As above, we can't take this as-is, but if you've found a regression I'm more than happy to try to get that fixed. If we *really* need to be able to boot BE kernels from EFI, there might be things that we can do, but I don't think we can just remove the CONFIG_EFI dependency on !CPU_BIG_ENDIAN -- the runtime bits *certainly* won't work, at minimum the stub needs to be built LE, etc.

Also, have you considered running your kernel under KVM, with devices assigned to it? That might be significantly easier than having a BE kernel that handles EFI.

Thanks,
Mark.

> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S index
> 6a98f1a38c29..40a18d767d15 100644
> --- a/arch/arm64/kernel/head.S
> +++ b/arch/arm64/kernel/head.S
> @@ -89,6 +89,24 @@
>          *  x24        __primary_switch() .. relocate_kernel()  current RELR displacement
>          */
>  SYM_CODE_START(primary_entry)
> +#ifdef CONFIG_CPU_BIG_ENDIAN
> +        mrs     x21, CurrentEL
> +        lsr     x21, x21, #2
> +        cmp     x21, #0x2
> +        b.lo    1f
> +
> +        mrs     x21, sctlr_el2
> +        orr     x21, x21, #SCTLR_ELx_EE /* set: Big Endian */
> +        msr     sctlr_el2, x21
> +        isb
> +
> +1:
> +        mrs     x21, sctlr_el1
> +        orr     x21, x21, #(SCTLR_ELx_EE | SCTLR_EL1_E0E)       /* set: Big Endian */
> +        msr     sctlr_el1, x21
> +        isb
> +
> +#endif
>         bl      preserve_boot_args
>         bl      init_kernel_el                  // w0=cpu_boot_mode
>         adrp    x23, __PHYS_OFFSET
> 
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index
> c4207cf9bb17..a9ccbeb75ea7 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -1997,7 +1997,7 @@ config EFI_STUB
> 
>  config EFI
>         bool "UEFI runtime support"
> -       depends on OF && !CPU_BIG_ENDIAN
> +       depends on OF
>         depends on KERNEL_MODE_NEON
>         select ARCH_SUPPORTS_ACPI
>         select LIBFDT
> 
> Rory Bolt
> 
> KIOXIA America, Inc. | formerly Toshiba Memory America, Inc.
> 
> 
> 
> 
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel