Mailing List Archive: [PATCH] xen/arm: do not read MVFR2 on arm32

MVFR2 is not available on arm32. Don't try to read it. Make MVFR2 arm64
only.

Fixes: 9cfdb489af81 ("xen/arm: Add ID registers and complete cpuinfo")
Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
---
xen/arch/arm/cpufeature.c | 2 ++
xen/include/asm-arm/cpufeature.h | 2 +-
2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/cpufeature.c b/xen/arch/arm/cpufeature.c
index 1f6a85aafe..9e3377eae3 100644
--- a/xen/arch/arm/cpufeature.c
+++ b/xen/arch/arm/cpufeature.c
@@ -150,7 +150,9 @@ void identify_cpu(struct cpuinfo_arm *c)

c->mvfr.bits[0] = READ_SYSREG(MVFR0_EL1);
c->mvfr.bits[1] = READ_SYSREG(MVFR1_EL1);
+#ifdef CONFIG_ARM_64
c->mvfr.bits[2] = READ_SYSREG(MVFR2_EL1);
+#endif
}

/*
diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h
index 6058744c18..fa20cb493a 100644
--- a/xen/include/asm-arm/cpufeature.h
+++ b/xen/include/asm-arm/cpufeature.h
@@ -272,7 +272,7 @@ struct cpuinfo_arm {
} isa32;

struct {
- register_t bits[3];
+ register_t bits[2 + IS_ENABLED(CONFIG_ARM_64)];
} mvfr;
};

--
2.17.1

On 05/01/2021 19:05, Stefano Stabellini wrote:
> MVFR2 is not available on arm32. Don't try to read it. Make MVFR2 arm64
> only.

Not really, MVFR2 is allocated when running in AArch32 mode on Armv8. It
just doesn't exist on Armv7. See my answer your previous e-mail for more
details.

>
> Fixes: 9cfdb489af81 ("xen/arm: Add ID registers and complete cpuinfo")
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
> ---
> xen/arch/arm/cpufeature.c | 2 ++
> xen/include/asm-arm/cpufeature.h | 2 +-
> 2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/xen/arch/arm/cpufeature.c b/xen/arch/arm/cpufeature.c
> index 1f6a85aafe..9e3377eae3 100644
> --- a/xen/arch/arm/cpufeature.c
> +++ b/xen/arch/arm/cpufeature.c
> @@ -150,7 +150,9 @@ void identify_cpu(struct cpuinfo_arm *c)
>
> c->mvfr.bits[0] = READ_SYSREG(MVFR0_EL1);
> c->mvfr.bits[1] = READ_SYSREG(MVFR1_EL1);
> +#ifdef CONFIG_ARM_64
> c->mvfr.bits[2] = READ_SYSREG(MVFR2_EL1);
> +#endif
> }
>
> /*
> diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h
> index 6058744c18..fa20cb493a 100644
> --- a/xen/include/asm-arm/cpufeature.h
> +++ b/xen/include/asm-arm/cpufeature.h
> @@ -272,7 +272,7 @@ struct cpuinfo_arm {
> } isa32;
>
> struct {
> - register_t bits[3];
> + register_t bits[2 + IS_ENABLED(CONFIG_ARM_64)];

mvfr.bits[2] will be accessed from arch/arm/vcpreg.c, so you will
provide garbagge to guest if the user happen to run Xen on Arm32 on Armv8.

Given that MVFR2 exists, I think it would be best to keep the definition
in cpuinfo_arm around and only hardcode the value in cpufeature.c.

Please see my previous e-mail for more rationale on this approach.

Cheers,

--
Julien Grall