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

Re: [PATCH v2] xen/arm: don't read aarch32 regs when aarch32 isn't available


  • To: Stefano Stabellini <sstabellini@xxxxxxxxxx>
  • From: Bertrand Marquis <Bertrand.Marquis@xxxxxxx>
  • Date: Wed, 13 Jan 2021 08:18:14 +0000
  • Accept-language: en-GB, en-US
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=arm.com; dmarc=pass action=none header.from=arm.com; dkim=pass header.d=arm.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=qdI9gmRr/Dm7FgdBfGdsy0yR5J3qamVE6WC24IIzGvU=; b=B7XlrV2bvdyg9Gh833ts69aZgTcKrY4/ANpJTCT6fl9WRmTGOqVA1N5e0HelGzHux2bj6syZruaEO/f7HnGsgcSzG3qCLf2wMIETvt1SPHNf184uMVXvq2qT08RnYilFG0xbOvoO7YQj+cApedYei+nMnIrB6Yc3VGDTI410eKVquN7CNY408BHrVx+lYJ+VivTCuOjw8bgHYW3yWimnG9e8uhx1oq8meRYULRFFnynhYM7rsqTVcG+S4oxLnITDGgNqIissJVf0bwBJHKVZeMVQHIu9EpElq53zwg59XNru9ypNVVDTDl+oV09mWHuH9mEtmC5pCedTG5akJ3qLGg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=hl5ZdAVGYIZbQsAZGoZM4ZV/Ph0MzxmFy4Ocqo5fx8pgbWky8UpIMRACzNfKpohR6XJD9mnBBjLJCWyamI5e3uu08k/NGZiVT47tUWSNZDFjbIr6tFsZYslBfAP7K15rHgee6CxrNFrA/8AuQ7G4Wls6FhMo8qm5eq9MWy2efKqNZEaWTGFTQSiHj6SpFF1iCKwPB5/2DcVQzaXJ0l0NDxaaV5ximkQG/4/6BIdtk9+SVa9WQev6mM6q4Drks9RQ9hya5FxA8CgrnWA9GBy9CxwaJKnMNNaSACAD0ClUUD7ueKU68/gpM6q5HSwFiYsfsIRFva76QuG26pQWmqBtpQ==
  • Authentication-results-original: kernel.org; dkim=none (message not signed) header.d=none;kernel.org; dmarc=none action=none header.from=arm.com;
  • Cc: Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>, "Volodymyr_Babchuk@xxxxxxxx" <Volodymyr_Babchuk@xxxxxxxx>, "julien@xxxxxxx" <julien@xxxxxxx>, Stefano Stabellini <stefano.stabellini@xxxxxxxxxx>
  • Delivery-date: Wed, 13 Jan 2021 08:18:31 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Nodisclaimer: true
  • Original-authentication-results: kernel.org; dkim=none (message not signed) header.d=none;kernel.org; dmarc=none action=none header.from=arm.com;
  • Thread-index: AQHW6T0cVyzUUW/j3ECSp244SRxuaKolNsuA
  • Thread-topic: [PATCH v2] xen/arm: don't read aarch32 regs when aarch32 isn't available

Hi Stefano,

> On 12 Jan 2021, at 23:44, Stefano Stabellini <sstabellini@xxxxxxxxxx> wrote:
> 
> Don't read aarch32 system registers at boot time when the aarch32 state
> is not available at EL0. They are UNKNOWN, so it is not useful to read
> them.  Moreover, on Cavium ThunderX reading ID_PFR2_EL1 causes a Xen
> crash.  Instead, only read them when aarch32 is available.

Might be a good idea to say that on Cavium ID_PFR2_EL1 is generating an
unsupported exception causing a Xen crash on boot.

> 
> Leave the corresponding fields in struct cpuinfo_arm so that they
> are read-as-zero from a guest.
> 
> Since we are editing identify_cpu, also fix the indentation: 4 spaces
> instead of 8.
> 
> Fixes: 9cfdb489af81 ("xen/arm: Add ID registers and complete cpuinfo")
> Link: 
> https://lore.kernel.org/xen-devel/f90e40ee-b042-6cc5-a08d-aef41a279527@xxxxxxxx/
> Suggested-by: Julien Grall <julien@xxxxxxx>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxx>
with this added:

Reviewed-by: Bertrand Marquis <bertrand.marquis@xxxxxxx>

Cheers
Bertrand

> ---
> Changes in v2:
> - check el0 instead of el1 for aarch32 support
> - clarify EL0 in commit message
> - remove temporary link in commit message
> - use lore.kernel.org in commit message
> - introduce cpu_feature64_has_el0_32
> - rename aarch32 to aarch32_el0
> ---
> xen/arch/arm/cpufeature.c        | 35 +++++++++++++++++++-------------
> xen/include/asm-arm/cpufeature.h |  2 ++
> 2 files changed, 23 insertions(+), 14 deletions(-)
> 
> diff --git a/xen/arch/arm/cpufeature.c b/xen/arch/arm/cpufeature.c
> index 698bfa0201..99fe4db280 100644
> --- a/xen/arch/arm/cpufeature.c
> +++ b/xen/arch/arm/cpufeature.c
> @@ -101,29 +101,35 @@ int enable_nonboot_cpu_caps(const struct 
> arm_cpu_capabilities *caps)
> 
> void identify_cpu(struct cpuinfo_arm *c)
> {
> -        c->midr.bits = READ_SYSREG(MIDR_EL1);
> -        c->mpidr.bits = READ_SYSREG(MPIDR_EL1);
> +    bool aarch32_el0 = true;
> +
> +    c->midr.bits = READ_SYSREG(MIDR_EL1);
> +    c->mpidr.bits = READ_SYSREG(MPIDR_EL1);
> 
> #ifdef CONFIG_ARM_64
> -        c->pfr64.bits[0] = READ_SYSREG(ID_AA64PFR0_EL1);
> -        c->pfr64.bits[1] = READ_SYSREG(ID_AA64PFR1_EL1);
> +    c->pfr64.bits[0] = READ_SYSREG(ID_AA64PFR0_EL1);
> +    c->pfr64.bits[1] = READ_SYSREG(ID_AA64PFR1_EL1);
> +
> +    c->dbg64.bits[0] = READ_SYSREG(ID_AA64DFR0_EL1);
> +    c->dbg64.bits[1] = READ_SYSREG(ID_AA64DFR1_EL1);
> 
> -        c->dbg64.bits[0] = READ_SYSREG(ID_AA64DFR0_EL1);
> -        c->dbg64.bits[1] = READ_SYSREG(ID_AA64DFR1_EL1);
> +    c->aux64.bits[0] = READ_SYSREG(ID_AA64AFR0_EL1);
> +    c->aux64.bits[1] = READ_SYSREG(ID_AA64AFR1_EL1);
> 
> -        c->aux64.bits[0] = READ_SYSREG(ID_AA64AFR0_EL1);
> -        c->aux64.bits[1] = READ_SYSREG(ID_AA64AFR1_EL1);
> +    c->mm64.bits[0]  = READ_SYSREG(ID_AA64MMFR0_EL1);
> +    c->mm64.bits[1]  = READ_SYSREG(ID_AA64MMFR1_EL1);
> +    c->mm64.bits[2]  = READ_SYSREG(ID_AA64MMFR2_EL1);
> 
> -        c->mm64.bits[0]  = READ_SYSREG(ID_AA64MMFR0_EL1);
> -        c->mm64.bits[1]  = READ_SYSREG(ID_AA64MMFR1_EL1);
> -        c->mm64.bits[2]  = READ_SYSREG(ID_AA64MMFR2_EL1);
> +    c->isa64.bits[0] = READ_SYSREG(ID_AA64ISAR0_EL1);
> +    c->isa64.bits[1] = READ_SYSREG(ID_AA64ISAR1_EL1);
> 
> -        c->isa64.bits[0] = READ_SYSREG(ID_AA64ISAR0_EL1);
> -        c->isa64.bits[1] = READ_SYSREG(ID_AA64ISAR1_EL1);
> +    c->zfr64.bits[0] = READ_SYSREG(ID_AA64ZFR0_EL1);
> 
> -        c->zfr64.bits[0] = READ_SYSREG(ID_AA64ZFR0_EL1);
> +    aarch32_el0 = cpu_feature64_has_el0_32(c);
> #endif
> 
> +    if ( aarch32_el0 )
> +    {
>         c->pfr32.bits[0] = READ_SYSREG(ID_PFR0_EL1);
>         c->pfr32.bits[1] = READ_SYSREG(ID_PFR1_EL1);
>         c->pfr32.bits[2] = READ_SYSREG(ID_PFR2_EL1);
> @@ -153,6 +159,7 @@ void identify_cpu(struct cpuinfo_arm *c)
> #ifndef MVFR2_MAYBE_UNDEFINED
>         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..13a2739a69 100644
> --- a/xen/include/asm-arm/cpufeature.h
> +++ b/xen/include/asm-arm/cpufeature.h
> @@ -5,6 +5,8 @@
> #define cpu_feature64(c, feat)         ((c)->pfr64.feat)
> #define boot_cpu_feature64(feat)       (boot_cpu_data.pfr64.feat)
> 
> +#define cpu_feature64_has_el0_32(c)    (cpu_feature64(c, el0) == 2)
> +
> #define cpu_has_el0_32    (boot_cpu_feature64(el0) == 2)
> #define cpu_has_el0_64    (boot_cpu_feature64(el0) >= 1)
> #define cpu_has_el1_32    (boot_cpu_feature64(el1) == 2)
> -- 
> 2.17.1
> 




 


Rackspace

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