Re: [PATCH] Remove support for ThumbEE

[Michal Orzel <michal.orzel@xxxxxxx>]

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.

> 
> In any case, the number of registers to save/restore is pretty limited. So I 
> don't see the problem to keep the code around. It doesn't mean the feature is 
> working, it just means we properly keep the domain isolated from each other.
> 
> Cheers,
> 

Cheers,
Michal