RE: Big endian modifications

[Rory Bolt <Rory.Bolt@kioxia.com>]

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