All of lore.kernel.org
 help / color / mirror / Atom feed
From: will.deacon@arm.com (Will Deacon)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC PATCH v3 8/8] arm64: Ensure the secondary CPUs have safe ASIDBits size
Date: Tue, 15 Dec 2015 12:02:30 +0000	[thread overview]
Message-ID: <20151215120230.GF9452@arm.com> (raw)
In-Reply-To: <1449655039-22022-9-git-send-email-suzuki.poulose@arm.com>

On Wed, Dec 09, 2015 at 09:57:19AM +0000, Suzuki K. Poulose wrote:
> The ID_AA64MMFR0_EL1:ASIDBits determines the size of the mm context
> id and is used in the early boot to make decisions. The value is
> picked up from the Boot CPU and cannot be delayed until other CPUs
> are up. If a secondary CPU has a smaller size than that of the Boot
> CPU, things will break horribly and the usual SANITY check is not good
> enough to prevent the system from crashing. So, crash the system with
> enough information.
> 
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Signed-off-by: Suzuki K. Poulose <suzuki.poulose@arm.com>
> ---
>  arch/arm64/include/asm/mmu_context.h |    2 ++
>  arch/arm64/kernel/cpufeature.c       |    2 ++
>  arch/arm64/mm/context.c              |   16 ++++++++++++++++
>  3 files changed, 20 insertions(+)
> 
> diff --git a/arch/arm64/include/asm/mmu_context.h b/arch/arm64/include/asm/mmu_context.h
> index 2416578..bd8a0b9 100644
> --- a/arch/arm64/include/asm/mmu_context.h
> +++ b/arch/arm64/include/asm/mmu_context.h
> @@ -147,4 +147,6 @@ switch_mm(struct mm_struct *prev, struct mm_struct *next,
>  #define deactivate_mm(tsk,mm)	do { } while (0)
>  #define activate_mm(prev,next)	switch_mm(prev, next, NULL)
>  
> +void verify_cpu_asid_bits(void);
> +
>  #endif
> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index e63da0f..f7bcd30 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -24,6 +24,7 @@
>  #include <asm/cpu.h>
>  #include <asm/cpufeature.h>
>  #include <asm/cpu_ops.h>
> +#include <asm/mmu_context.h>
>  #include <asm/processor.h>
>  #include <asm/sysreg.h>
>  
> @@ -829,6 +830,7 @@ static u64 __raw_read_system_reg(u32 sys_id)
>   */
>  static void check_early_cpu_features(void)
>  {
> +	verify_cpu_asid_bits();
>  }
>  
>  /*
> diff --git a/arch/arm64/mm/context.c b/arch/arm64/mm/context.c
> index 643bf4b..b945bb4 100644
> --- a/arch/arm64/mm/context.c
> +++ b/arch/arm64/mm/context.c
> @@ -24,6 +24,7 @@
>  
>  #include <asm/cpufeature.h>
>  #include <asm/mmu_context.h>
> +#include <asm/smp.h>
>  #include <asm/tlbflush.h>
>  
>  static u32 asid_bits;
> @@ -62,6 +63,21 @@ static u32 get_cpu_asid_bits(void)
>  	return asid;
>  }
>  
> +/* Check if the current cpu's ASIDBits is compatible with asid_bits */
> +void verify_cpu_asid_bits(void)
> +{
> +	u32 asid = get_cpu_asid_bits();
> +
> +	if (asid < asid_bits) {
> +		/* Differing ASIDBits is dangerous, crash the system */

This comment isn't much use. How about:

  /*
   * We cannot decrease the ASID size at runtime, so panic if we support
   * fewer ASID bits than the boot CPU.
   */

> +		pr_crit("CPU%d: has smaller ASIDBits(%u) than the one in use (%u)\n",
> +				smp_processor_id(), asid, asid_bits);

"CPU%d: smaller ASID size (%d) than boot CPU (%u)\n"

> +		update_cpu_boot_status(CPU_PANIC_KERNEL);
> +		isb();

Why?

> +		while(1);

We probably want the usuale wfe/wfi trick here. Perhaps we should have
an inline function for that.

Will

WARNING: multiple messages have this Message-ID (diff)
From: Will Deacon <will.deacon@arm.com>
To: "Suzuki K. Poulose" <suzuki.poulose@arm.com>
Cc: linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, mark.rutland@arm.com,
	catalin.marinas@arm.com, marc.zyngier@arm.com
Subject: Re: [RFC PATCH v3 8/8] arm64: Ensure the secondary CPUs have safe ASIDBits size
Date: Tue, 15 Dec 2015 12:02:30 +0000	[thread overview]
Message-ID: <20151215120230.GF9452@arm.com> (raw)
In-Reply-To: <1449655039-22022-9-git-send-email-suzuki.poulose@arm.com>

On Wed, Dec 09, 2015 at 09:57:19AM +0000, Suzuki K. Poulose wrote:
> The ID_AA64MMFR0_EL1:ASIDBits determines the size of the mm context
> id and is used in the early boot to make decisions. The value is
> picked up from the Boot CPU and cannot be delayed until other CPUs
> are up. If a secondary CPU has a smaller size than that of the Boot
> CPU, things will break horribly and the usual SANITY check is not good
> enough to prevent the system from crashing. So, crash the system with
> enough information.
> 
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Signed-off-by: Suzuki K. Poulose <suzuki.poulose@arm.com>
> ---
>  arch/arm64/include/asm/mmu_context.h |    2 ++
>  arch/arm64/kernel/cpufeature.c       |    2 ++
>  arch/arm64/mm/context.c              |   16 ++++++++++++++++
>  3 files changed, 20 insertions(+)
> 
> diff --git a/arch/arm64/include/asm/mmu_context.h b/arch/arm64/include/asm/mmu_context.h
> index 2416578..bd8a0b9 100644
> --- a/arch/arm64/include/asm/mmu_context.h
> +++ b/arch/arm64/include/asm/mmu_context.h
> @@ -147,4 +147,6 @@ switch_mm(struct mm_struct *prev, struct mm_struct *next,
>  #define deactivate_mm(tsk,mm)	do { } while (0)
>  #define activate_mm(prev,next)	switch_mm(prev, next, NULL)
>  
> +void verify_cpu_asid_bits(void);
> +
>  #endif
> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index e63da0f..f7bcd30 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -24,6 +24,7 @@
>  #include <asm/cpu.h>
>  #include <asm/cpufeature.h>
>  #include <asm/cpu_ops.h>
> +#include <asm/mmu_context.h>
>  #include <asm/processor.h>
>  #include <asm/sysreg.h>
>  
> @@ -829,6 +830,7 @@ static u64 __raw_read_system_reg(u32 sys_id)
>   */
>  static void check_early_cpu_features(void)
>  {
> +	verify_cpu_asid_bits();
>  }
>  
>  /*
> diff --git a/arch/arm64/mm/context.c b/arch/arm64/mm/context.c
> index 643bf4b..b945bb4 100644
> --- a/arch/arm64/mm/context.c
> +++ b/arch/arm64/mm/context.c
> @@ -24,6 +24,7 @@
>  
>  #include <asm/cpufeature.h>
>  #include <asm/mmu_context.h>
> +#include <asm/smp.h>
>  #include <asm/tlbflush.h>
>  
>  static u32 asid_bits;
> @@ -62,6 +63,21 @@ static u32 get_cpu_asid_bits(void)
>  	return asid;
>  }
>  
> +/* Check if the current cpu's ASIDBits is compatible with asid_bits */
> +void verify_cpu_asid_bits(void)
> +{
> +	u32 asid = get_cpu_asid_bits();
> +
> +	if (asid < asid_bits) {
> +		/* Differing ASIDBits is dangerous, crash the system */

This comment isn't much use. How about:

  /*
   * We cannot decrease the ASID size at runtime, so panic if we support
   * fewer ASID bits than the boot CPU.
   */

> +		pr_crit("CPU%d: has smaller ASIDBits(%u) than the one in use (%u)\n",
> +				smp_processor_id(), asid, asid_bits);

"CPU%d: smaller ASID size (%d) than boot CPU (%u)\n"

> +		update_cpu_boot_status(CPU_PANIC_KERNEL);
> +		isb();

Why?

> +		while(1);

We probably want the usuale wfe/wfi trick here. Perhaps we should have
an inline function for that.

Will

  reply	other threads:[~2015-12-15 12:02 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-12-09  9:57 [RFC PATCH v3 0/8] arm64: Verify early CPU features Suzuki K. Poulose
2015-12-09  9:57 ` Suzuki K. Poulose
2015-12-09  9:57 ` [RFC PATCH v3 1/8] arm64: Introduce cpu_die_early Suzuki K. Poulose
2015-12-09  9:57   ` Suzuki K. Poulose
2015-12-09  9:57 ` [RFC PATCH v3 2/8] arm64: Move cpu_die_early to smp.c Suzuki K. Poulose
2015-12-09  9:57   ` Suzuki K. Poulose
2015-12-15 11:23   ` Will Deacon
2015-12-15 11:23     ` Will Deacon
2015-12-15 11:26     ` Mark Rutland
2015-12-15 11:26       ` Mark Rutland
2015-12-09  9:57 ` [RFC PATCH v3 3/8] arm64: head.S : Change register usage Suzuki K. Poulose
2015-12-09  9:57   ` Suzuki K. Poulose
2015-12-09  9:57 ` [RFC PATCH v3 4/8] arm64: Handle early CPU boot failures Suzuki K. Poulose
2015-12-09  9:57   ` Suzuki K. Poulose
2015-12-15 11:37   ` Suzuki K. Poulose
2015-12-15 11:37     ` Suzuki K. Poulose
2015-12-15 11:55   ` Will Deacon
2015-12-15 11:55     ` Will Deacon
2015-12-16 10:39     ` Suzuki K. Poulose
2015-12-16 10:39       ` Suzuki K. Poulose
2015-12-16 11:29       ` Will Deacon
2015-12-16 11:29         ` Will Deacon
2015-12-16 11:36         ` Mark Rutland
2015-12-16 11:36           ` Mark Rutland
2015-12-09  9:57 ` [RFC PATCH v3 5/8] arm64: Enable CPU capability verification unconditionally Suzuki K. Poulose
2015-12-09  9:57   ` Suzuki K. Poulose
2015-12-09  9:57 ` [RFC PATCH v3 6/8] arm64: Add hook for checking early CPU features Suzuki K. Poulose
2015-12-09  9:57   ` Suzuki K. Poulose
2015-12-09  9:57 ` [RFC PATCH v3 7/8] arm64: Add helper for extracting ASIDBits Suzuki K. Poulose
2015-12-09  9:57   ` Suzuki K. Poulose
2015-12-09  9:57 ` [RFC PATCH v3 8/8] arm64: Ensure the secondary CPUs have safe ASIDBits size Suzuki K. Poulose
2015-12-09  9:57   ` Suzuki K. Poulose
2015-12-15 12:02   ` Will Deacon [this message]
2015-12-15 12:02     ` Will Deacon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20151215120230.GF9452@arm.com \
    --to=will.deacon@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.