Re: [PATCH] Fix 32-bit ARM handling of the CTR register

[Leif Lindholm]

On Sun, May 24, 2020 at 12:32:48 +0100, Marc Zyngier wrote:
> When booting on an ARMv8 core that implements either CTR.IDC or CTR.DIC
> (indicating that some of the cache maintenance operations can be
> removed when dealing with I/D-cache coherency, GRUB dies with a
> "Unsupported cache type 0x........" message.
> 
> This is pretty likely to happen when running in a virtual machine
> hosted on an arm64 machine (I've triggered it on a system built around
> a bunch of Cortex-A55 cores, which implements CTR.IDC).
> 
> It turns out that the way GRUB deals with the CTR register is a bit
> harsh for anything from ARMv7 onwards. The layout of the register is
> backward compatible, meaning that nothing that gets added is allowed to
> break earlier behaviour. In this case, ignoring IDC is completely fine,
> and only results in unnecessary cache maintenance.
> 
> We can thus avoid being paranoid, and align the 32bit behaviour with
> its 64bit equivalent.
> 
> Signed-off-by: Marc Zyngier <address@hidden>

This patch has the added benfit that it gets rid of a (gnu-specific)
case range.

Reviewed-by: Leif Lindholm <address@hidden>
Thanks!

> ---
>  grub-core/kern/arm/cache.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/grub-core/kern/arm/cache.c b/grub-core/kern/arm/cache.c
> index af1c4bbf5..6c75193e4 100644
> --- a/grub-core/kern/arm/cache.c
> +++ b/grub-core/kern/arm/cache.c
> @@ -93,13 +93,16 @@ probe_caches (void)
>        grub_arch_cache_ilinesz = 8 << (cache_type & 3);
>        type = ARCH_ARMV6;
>        break;
> -    case 0x80 ... 0x8f:
> +    default:
> +      /*
> +       * The CTR register is pretty much unchanged from v7 onwards,
> +       * and is guaranteed to be backward compatible (the IDC/DIC bits
> +       * allow certain CMOs to be elided, but performing them is never
> +       * wrong), hence handling it like its AArch64 equivalent.
> +       */
>        grub_arch_cache_dlinesz = 4 << ((cache_type >> 16) & 0xf);
>        grub_arch_cache_ilinesz = 4 << (cache_type & 0xf);
>        type = ARCH_ARMV7;
> -      break;
> -    default:
> -      grub_fatal ("Unsupported cache type 0x%x", cache_type);
>      }
>    if (grub_arch_cache_dlinesz > grub_arch_cache_ilinesz)
>      grub_arch_cache_max_linesz = grub_arch_cache_dlinesz;
> -- 
> 2.26.2
> 
> 
> _______________________________________________
> Grub-devel mailing list
> address@hidden
> https://lists.gnu.org/mailman/listinfo/grub-devel