All of lore.kernel.org
 help / color / mirror / Atom feed
From: Catalin Marinas <catalin.marinas@arm.com>
To: Will Deacon <will@kernel.org>
Cc: linux-arm-kernel@lists.infradead.org, linux-arch@vger.kernel.org,
	Marc Zyngier <maz@kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Morten Rasmussen <morten.rasmussen@arm.com>,
	Qais Yousef <qais.yousef@arm.com>,
	Suren Baghdasaryan <surenb@google.com>,
	Quentin Perret <qperret@google.com>,
	kernel-team@android.com
Subject: Re: [PATCH v2 2/6] arm64: Allow mismatched 32-bit EL0 support
Date: Wed, 11 Nov 2020 19:10:44 +0000	[thread overview]
Message-ID: <20201111191043.GA5125@gaia> (raw)
In-Reply-To: <20201109213023.15092-3-will@kernel.org>

Hi Will,

On Mon, Nov 09, 2020 at 09:30:18PM +0000, Will Deacon wrote:
> +static bool has_32bit_el0(const struct arm64_cpu_capabilities *entry, int scope)
> +{
> +	if (!has_cpuid_feature(entry, scope))
> +		return allow_mismatched_32bit_el0;

I still don't like overriding the cpufeature mechanism in this way. What about
something like below? It still doesn't fit perfectly but at least the
capability represents what was detected in the system. We then decide in
system_supports_32bit_el0() whether to allow asymmetry. There is an
extra trick to park a non-AArch32 capable CPU in has_32bit_el0() if it
comes up late and the feature has already been advertised with
!allow_mismatched_32bit_el0.

I find it clearer, though I probably stared at it more than at your
patch ;).

diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index 97244d4feca9..0e0427997063 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -604,9 +604,13 @@ static inline bool cpu_supports_mixed_endian_el0(void)
 	return id_aa64mmfr0_mixed_endian_el0(read_cpuid(ID_AA64MMFR0_EL1));
 }
 
+extern bool allow_mismatched_32bit_el0;
+extern bool mismatched_32bit_el0;
+
 static inline bool system_supports_32bit_el0(void)
 {
-	return cpus_have_const_cap(ARM64_HAS_32BIT_EL0);
+	return cpus_have_const_cap(ARM64_HAS_32BIT_EL0) &&
+		(!mismatched_32bit_el0 || allow_mismatched_32bit_el0);
 }
 
 static inline bool system_supports_4kb_granule(void)
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index b7b6804cb931..67534327f92b 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -104,6 +104,13 @@ DECLARE_BITMAP(boot_capabilities, ARM64_NPATCHABLE);
 bool arm64_use_ng_mappings = false;
 EXPORT_SYMBOL(arm64_use_ng_mappings);
 
+/*
+ * Permit PER_LINUX32 and execve() of 32-bit binaries even if not all CPUs
+ * support it?
+ */
+bool __read_mostly allow_mismatched_32bit_el0;
+bool mismatched_32bit_el0;
+
 /*
  * Flag to indicate if we have computed the system wide
  * capabilities based on the boot time active CPUs. This
@@ -1193,6 +1200,35 @@ has_cpuid_feature(const struct arm64_cpu_capabilities *entry, int scope)
 	return feature_matches(val, entry);
 }
 
+static bool has_32bit_el0(const struct arm64_cpu_capabilities *entry, int scope)
+{
+	if (has_cpuid_feature(entry, scope))
+		return true;
+
+	if (system_capabilities_finalized() && !allow_mismatched_32bit_el0) {
+		pr_crit("CPU%d: Asymmetric AArch32 not supported\n",
+			smp_processor_id());
+		cpu_die_early();
+	}
+
+	mismatched_32bit_el0 = true;
+	return false;
+}
+
+static int __init report_32bit_el0(void)
+{
+	if (!system_supports_32bit_el0())
+		return 0;
+
+	if (mismatched_32bit_el0)
+		pr_info("detected: asymmetric 32-bit EL0 support\n");
+	else
+		pr_info("detected: 32-bit EL0 support\n");
+
+	return 0;
+}
+core_initcall(report_32bit_el0);
+
 static bool has_useable_gicv3_cpuif(const struct arm64_cpu_capabilities *entry, int scope)
 {
 	bool has_sre;
@@ -1800,10 +1836,9 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
 	},
 #endif	/* CONFIG_ARM64_VHE */
 	{
-		.desc = "32-bit EL0 Support",
 		.capability = ARM64_HAS_32BIT_EL0,
-		.type = ARM64_CPUCAP_SYSTEM_FEATURE,
-		.matches = has_cpuid_feature,
+		.type = ARM64_CPUCAP_WEAK_LOCAL_CPU_FEATURE,
+		.matches = has_32bit_el0,
 		.sys_reg = SYS_ID_AA64PFR0_EL1,
 		.sign = FTR_UNSIGNED,
 		.field_pos = ID_AA64PFR0_EL0_SHIFT,


-- 
Catalin

WARNING: multiple messages have this Message-ID (diff)
From: Catalin Marinas <catalin.marinas@arm.com>
To: Will Deacon <will@kernel.org>
Cc: linux-arch@vger.kernel.org, kernel-team@android.com,
	Quentin Perret <qperret@google.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Marc Zyngier <maz@kernel.org>, Qais Yousef <qais.yousef@arm.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Suren Baghdasaryan <surenb@google.com>,
	Morten Rasmussen <morten.rasmussen@arm.com>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v2 2/6] arm64: Allow mismatched 32-bit EL0 support
Date: Wed, 11 Nov 2020 19:10:44 +0000	[thread overview]
Message-ID: <20201111191043.GA5125@gaia> (raw)
In-Reply-To: <20201109213023.15092-3-will@kernel.org>

Hi Will,

On Mon, Nov 09, 2020 at 09:30:18PM +0000, Will Deacon wrote:
> +static bool has_32bit_el0(const struct arm64_cpu_capabilities *entry, int scope)
> +{
> +	if (!has_cpuid_feature(entry, scope))
> +		return allow_mismatched_32bit_el0;

I still don't like overriding the cpufeature mechanism in this way. What about
something like below? It still doesn't fit perfectly but at least the
capability represents what was detected in the system. We then decide in
system_supports_32bit_el0() whether to allow asymmetry. There is an
extra trick to park a non-AArch32 capable CPU in has_32bit_el0() if it
comes up late and the feature has already been advertised with
!allow_mismatched_32bit_el0.

I find it clearer, though I probably stared at it more than at your
patch ;).

diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index 97244d4feca9..0e0427997063 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -604,9 +604,13 @@ static inline bool cpu_supports_mixed_endian_el0(void)
 	return id_aa64mmfr0_mixed_endian_el0(read_cpuid(ID_AA64MMFR0_EL1));
 }
 
+extern bool allow_mismatched_32bit_el0;
+extern bool mismatched_32bit_el0;
+
 static inline bool system_supports_32bit_el0(void)
 {
-	return cpus_have_const_cap(ARM64_HAS_32BIT_EL0);
+	return cpus_have_const_cap(ARM64_HAS_32BIT_EL0) &&
+		(!mismatched_32bit_el0 || allow_mismatched_32bit_el0);
 }
 
 static inline bool system_supports_4kb_granule(void)
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index b7b6804cb931..67534327f92b 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -104,6 +104,13 @@ DECLARE_BITMAP(boot_capabilities, ARM64_NPATCHABLE);
 bool arm64_use_ng_mappings = false;
 EXPORT_SYMBOL(arm64_use_ng_mappings);
 
+/*
+ * Permit PER_LINUX32 and execve() of 32-bit binaries even if not all CPUs
+ * support it?
+ */
+bool __read_mostly allow_mismatched_32bit_el0;
+bool mismatched_32bit_el0;
+
 /*
  * Flag to indicate if we have computed the system wide
  * capabilities based on the boot time active CPUs. This
@@ -1193,6 +1200,35 @@ has_cpuid_feature(const struct arm64_cpu_capabilities *entry, int scope)
 	return feature_matches(val, entry);
 }
 
+static bool has_32bit_el0(const struct arm64_cpu_capabilities *entry, int scope)
+{
+	if (has_cpuid_feature(entry, scope))
+		return true;
+
+	if (system_capabilities_finalized() && !allow_mismatched_32bit_el0) {
+		pr_crit("CPU%d: Asymmetric AArch32 not supported\n",
+			smp_processor_id());
+		cpu_die_early();
+	}
+
+	mismatched_32bit_el0 = true;
+	return false;
+}
+
+static int __init report_32bit_el0(void)
+{
+	if (!system_supports_32bit_el0())
+		return 0;
+
+	if (mismatched_32bit_el0)
+		pr_info("detected: asymmetric 32-bit EL0 support\n");
+	else
+		pr_info("detected: 32-bit EL0 support\n");
+
+	return 0;
+}
+core_initcall(report_32bit_el0);
+
 static bool has_useable_gicv3_cpuif(const struct arm64_cpu_capabilities *entry, int scope)
 {
 	bool has_sre;
@@ -1800,10 +1836,9 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
 	},
 #endif	/* CONFIG_ARM64_VHE */
 	{
-		.desc = "32-bit EL0 Support",
 		.capability = ARM64_HAS_32BIT_EL0,
-		.type = ARM64_CPUCAP_SYSTEM_FEATURE,
-		.matches = has_cpuid_feature,
+		.type = ARM64_CPUCAP_WEAK_LOCAL_CPU_FEATURE,
+		.matches = has_32bit_el0,
 		.sys_reg = SYS_ID_AA64PFR0_EL1,
 		.sign = FTR_UNSIGNED,
 		.field_pos = ID_AA64PFR0_EL0_SHIFT,


-- 
Catalin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2020-11-11 19:10 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-09 21:30 [PATCH v2 0/6] An alternative series for asymmetric AArch32 systems Will Deacon
2020-11-09 21:30 ` Will Deacon
2020-11-09 21:30 ` [PATCH v2 1/6] arm64: cpuinfo: Split AArch32 registers out into a separate struct Will Deacon
2020-11-09 21:30   ` Will Deacon
2020-11-09 21:30 ` [PATCH v2 2/6] arm64: Allow mismatched 32-bit EL0 support Will Deacon
2020-11-09 21:30   ` Will Deacon
2020-11-11 19:10   ` Catalin Marinas [this message]
2020-11-11 19:10     ` Catalin Marinas
2020-11-13  9:36     ` Will Deacon
2020-11-13  9:36       ` Will Deacon
2020-11-13 10:26       ` Catalin Marinas
2020-11-13 10:26         ` Catalin Marinas
2020-11-09 21:30 ` [PATCH v2 3/6] KVM: arm64: Kill 32-bit vCPUs on systems with mismatched " Will Deacon
2020-11-09 21:30   ` Will Deacon
2020-11-10  9:33   ` Marc Zyngier
2020-11-10  9:33     ` Marc Zyngier
2020-11-09 21:30 ` [PATCH v2 4/6] arm64: Kill 32-bit applications scheduled on 64-bit-only CPUs Will Deacon
2020-11-09 21:30   ` Will Deacon
2020-11-09 21:30 ` [PATCH v2 5/6] arm64: Advertise CPUs capable of running 32-bit applications in sysfs Will Deacon
2020-11-09 21:30   ` Will Deacon
2020-11-10  7:04   ` Greg Kroah-Hartman
2020-11-10  7:04     ` Greg Kroah-Hartman
2020-11-10  9:28     ` Catalin Marinas
2020-11-10  9:28       ` Catalin Marinas
2020-11-10  9:36       ` Greg Kroah-Hartman
2020-11-10  9:36         ` Greg Kroah-Hartman
2020-11-10  9:53         ` Marc Zyngier
2020-11-10  9:53           ` Marc Zyngier
2020-11-10 10:10           ` Greg Kroah-Hartman
2020-11-10 10:10             ` Greg Kroah-Hartman
2020-11-10 10:46             ` Marc Zyngier
2020-11-10 10:46               ` Marc Zyngier
2020-11-10 10:57         ` Catalin Marinas
2020-11-10 10:57           ` Catalin Marinas
2020-11-09 21:30 ` [PATCH v2 6/6] arm64: Hook up cmdline parameter to allow mismatched 32-bit EL0 Will Deacon
2020-11-09 21:30   ` 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=20201111191043.GA5125@gaia \
    --to=catalin.marinas@arm.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=kernel-team@android.com \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=maz@kernel.org \
    --cc=morten.rasmussen@arm.com \
    --cc=peterz@infradead.org \
    --cc=qais.yousef@arm.com \
    --cc=qperret@google.com \
    --cc=surenb@google.com \
    --cc=will@kernel.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.