Re: [PATCH] Remove support for ThumbEE

[Julien Grall <julien@xxxxxxx>]

On 13/04/2021 10:32, Michal Orzel wrote:
> Hi Julien,
>
> On 13.04.2021 11:07, Julien Grall wrote:
> > Hi Michal,
> >
> > On 13/04/2021 09:24, Michal Orzel wrote:
> > > ThumbEE(T32EE) was introduced in ARMv7 and removed in ARMv8.
> > > In 2011 ARM deprecated any use of the ThumbEE instruction set.
> > This doesn't mean this is not present in any CPU. In fact, in the same section 
> > (see A2.10 in ARM DDI 0406C.d):
> >
> > "ThumbEE is both the name of the instruction set and the name of the extension 
> > that provides support for that
> > instruction set. The ThumbEE Extension is:
> >    - Required in implementations of the ARMv7-A profile.
> >    - Optional in implementations of the ARMv7-R profile.
> > "
> >
> > > This feature is untested and as per my understanding
> > > there are no reported users for it. >
> > > Remove all the bits related to it.
> > >
> > > Signed-off-by: Michal Orzel <michal.orzel@xxxxxxx> > ---
> > >    xen/arch/arm/cpufeature.c        |  3 +++
> > >    xen/arch/arm/domain.c            | 12 ------------
> > >    xen/arch/arm/setup.c             |  3 +--
> > >    xen/include/asm-arm/cpregs.h     |  6 ------
> > >    xen/include/asm-arm/cpufeature.h |  1 -
> > >    xen/include/asm-arm/domain.h     |  1 -
> > >    6 files changed, 4 insertions(+), 22 deletions(-)
> > >
> > > diff --git a/xen/arch/arm/cpufeature.c b/xen/arch/arm/cpufeature.c
> > > index 1d88783809..82265a72f4 100644
> > > --- a/xen/arch/arm/cpufeature.c
> > > +++ b/xen/arch/arm/cpufeature.c
> > > @@ -209,6 +209,9 @@ static int __init create_guest_cpuinfo(void)
> > >        guest_cpuinfo.pfr32.ras = 0;
> > >        guest_cpuinfo.pfr32.ras_frac = 0;
> > >    +    /* Hide ThumbEE support */
> > > +    guest_cpuinfo.pfr32.thumbee = 0;
> > Even if you hide the feature from the guest, the registers are still 
> > accessible. So you are not removing support but just opening a potential 
> > security hole as the registers now gets shared...
> >
> > Looking at the spec, it doesn't look like it is possible to trap them.
> Looking at the spec for ARMv7A/R:
> https://developer.arm.com/documentation/ddi0406/c/System-Level-Architecture/System-Control-Registers-in-a-VMSA-implementation/VMSA-System-control-registers-descriptions--in-register-order/HSTR--Hyp-System-Trap-Register--Virtualization-Extensions
> we can trap Thumbee operations.
> This means that we will not open the security hole.
That's for pointing that out. However, I am not still not sure why you 
want to drop the code. Yes this is technically untested, but:
  1) The change is very small
  2) There are CPU out there supporting it (it is mandated after all).

--
Julien Grall