[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] Remove support for ThumbEE





On 13/04/2021 10:59, Michal Orzel wrote:


On 13.04.2021 11:42, Julien Grall wrote:


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).

I wanted to drop the support for thumbee due to the following reasons:
-it is untested
-it is deprecated

Deprecated only means new code should avoid to use the instruction set. But it is still present for existing code until they are migrated to a newer instruction set.

However, as I pointed out only the instruction set is deprecated. The extension itself is mandated by Armv7-A.

-no reported users for this feature
We probably only see the tip of the iceberg in term of the users. If the feature/pain is quite small to the community, then we should try to not drop feature.

-KVM did that

Well, KVM fully dropped 32-bit support. But I am not yet in favor of that in Xen.


If you think we should not do this, then I am ok to abandon this patch.

Based on the information you provided, I think we should keep support.

Cheers,

--
Julien Grall



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.