[RFC PATCH 2/3] arm64: Add support for asymmetric AArch32 EL0 configurations

[Catalin Marinas]

On Fri, Oct 09, 2020 at 08:13:56AM +0200, Greg Kroah-Hartman wrote:
> On Thu, Oct 08, 2020 at 07:16:40PM +0100, Qais Yousef wrote:
> > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> > index 6d232837cbee..591853504dc4 100644
> > --- a/arch/arm64/Kconfig
> > +++ b/arch/arm64/Kconfig
> > @@ -1868,6 +1868,20 @@ config DMI
> >  
> >  endmenu
> >  
> > +config ASYMMETRIC_AARCH32
> > +	bool "Allow support for asymmetric AArch32 support"
> > +	depends on COMPAT && EXPERT
> 
> Why EXPERT?  You don't want this able to be enabled by anyone?

Exactly ;). Anyway, depending on how user-friendly the feature becomes
(like the kernel transparently handling task placement), we may drop
this condition or replace it with a command line option. By default
current kernels block ELF32 processes on such platform.

> > +	help
> > +	  Enable this option to allow support for asymmetric AArch32 EL0
> > +	  CPU configurations. Once the AArch32 EL0 support is detected
> > +	  on a CPU, the feature is made available to user space to allow
> > +	  the execution of 32-bit (compat) applications. If the affinity
> > +	  of the 32-bit application contains a non-AArch32 capable CPU
> > +	  or the last AArch32 capable CPU is offlined, the application
> > +	  will be killed.
> > +
> > +	  If unsure say N.
> > +
> >  config SYSVIPC_COMPAT
> >  	def_bool y
> >  	depends on COMPAT && SYSVIPC
> > diff --git a/arch/arm64/include/asm/cpu.h b/arch/arm64/include/asm/cpu.h
> > index 7faae6ff3ab4..c920fa45e502 100644
> > --- a/arch/arm64/include/asm/cpu.h
> > +++ b/arch/arm64/include/asm/cpu.h
> > @@ -15,6 +15,7 @@
> >  struct cpuinfo_arm64 {
> >  	struct cpu	cpu;
> >  	struct kobject	kobj;
> > +	bool		aarch32_valid;
> 
> Do you mean to cause holes in this structure?  :)

No matter where you put it, there's still a hole (could move the hole at
the end though), unless we make it a 32-bit value.

Thinking about this, since we only check it on the boot_cpu_data
structure, we could probably make it a stand-alone variable or drop it
altogether (see below).

> Isn't "valid" the common thing?  Do you now have to explicitly enable
> this everywhere instead of just dealing with the uncommon case of this
> cpu variant?

We have a whole infrastructure for dealing with asymmetric features and
in most cases we only want the common functionality. The CPUID register
fields across all CPUs are normally sanitised to the lowest common
value. For some secondary CPU feature not matching the boot CPU we taint
the kernel.

In the original patch (that ended up on some Google gerrit), we relaxed
the 32-bit support checking so that the sanitised register contains the
highest value. However, if booting on a (big) CPU that did not have
32-bit, the kernel would end up tainted. The reason for aarch32_valid
was to delay populating the boot CPU information until a secondary CPU
comes up with 32-bit support and subsequently avoid the tainting.

With a last minute change (yesterday), we reverted the sanitised 32-bit
support field to the lowest value and introduced a new feature check
that's enabled when at least one of the CPUs has 32-bit support (we do
something similar for errata detection). With this in place, I think the
aarch32_valid setting/checking and delayed boot_cpu_data update can go.

> I don't see this information being exported to userspace anywhere.  I
> know Intel has submitted a patch to export this "type" of thing to the
> cpu sysfs directories, can you do the same thing here?
> 
> Otherwise, how is userspace supposed to know where to place programs
> that are 32bit?

In this series, we tried to avoid this by introducing an automatic
affinity setting/hacking (patch 3). So it's entirely transparent to the
user, it doesn't need to explicitly ask for specific task placement.

Given that Peter is against overriding the user cpumask and that a void
intersection between the 32-bit cpumask and the user one would lead to
SIGKILL, we probably have to expose the information somewhere. Currently
we have the midr_el1 register exposed in sysfs and this tells the
specific CPU implementation. It doesn't, however, state whether 32-bit
is supported unless one checks the specifications. I'd prefer to extend
the current arm64 sysfs cpu interface to include the rest of the id_*
registers, unless we find a way to create a common interface with what
the x86 guys are doing. But I'm slightly doubtful that we can find a
common interface. While 32-bit is common across other architectures, we
may want this for other features which don't have any correspondent.

-- 
Catalin