From: Andre Przywara <andre.przywara@arm.com> To: Will Deacon <will.deacon@arm.com> Cc: Jeremy Linton <jeremy.linton@arm.com>, <linux-arm-kernel@lists.infradead.org>, <catalin.marinas@arm.com>, <marc.zyngier@arm.com>, <suzuki.poulose@arm.com>, <Dave.Martin@arm.com>, <shankerd@codeaurora.org>, <julien.thierry@arm.com>, <mlangsdo@redhat.com>, <stefan.wahren@i2e.com>, <linux-kernel@vger.kernel.org>, Stefan Wahren <stefan.wahren@i2se.com> Subject: Re: [PATCH v6 09/10] arm64: add sysfs vulnerability show for speculative store bypass Date: Fri, 5 Apr 2019 11:10:22 +0100 [thread overview] Message-ID: <20190405111022.6ef0b7bb@donnerap.cambridge.arm.com> (raw) In-Reply-To: <20190403165005.GA17500@fuggles.cambridge.arm.com> On Wed, 3 Apr 2019 17:50:05 +0100 Will Deacon <will.deacon@arm.com> wrote: > Hi Jeremy, > > On Thu, Mar 21, 2019 at 06:05:56PM -0500, Jeremy Linton wrote: > > Return status based on ssbd_state and the arm64 SSBS feature. If > > the mitigation is disabled, or the firmware isn't responding then > > return the expected machine state based on a new blacklist of known > > vulnerable cores. > > > > Signed-off-by: Jeremy Linton <jeremy.linton@arm.com> > > Reviewed-by: Andre Przywara <andre.przywara@arm.com> > > Tested-by: Stefan Wahren <stefan.wahren@i2se.com> > > --- > > arch/arm64/kernel/cpu_errata.c | 44 ++++++++++++++++++++++++++++++++++ > > 1 file changed, 44 insertions(+) > > > > diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c > > index 6958dcdabf7d..172ffbabd597 100644 > > --- a/arch/arm64/kernel/cpu_errata.c > > +++ b/arch/arm64/kernel/cpu_errata.c > > @@ -278,6 +278,7 @@ static int detect_harden_bp_fw(void) > > DEFINE_PER_CPU_READ_MOSTLY(u64, arm64_ssbd_callback_required); > > > > int ssbd_state __read_mostly = ARM64_SSBD_KERNEL; > > +static bool __ssb_safe = true; > > > > static const struct ssbd_options { > > const char *str; > > @@ -386,6 +387,9 @@ static bool has_ssbd_mitigation(const struct arm64_cpu_capabilities *entry, > > > > WARN_ON(scope != SCOPE_LOCAL_CPU || preemptible()); > > > > + if (is_midr_in_range_list(read_cpuid_id(), entry->midr_range_list)) > > + __ssb_safe = false; > > + > > Does this mean that we assume that CPUs not present in our table are not > affected by speculative store bypass? No, not affected are only those where we either have SSBS or the firmware explicitly returns SMCCC_RET_NOT_REQUIRED. This is governed by ssbd_state. So this doesn't affect correctness. __ssb_safe is an additional state just used for the sysfs output. But indeed it looks like this is wrong if the CPU is both not listed and the system doesn't provide the firmware interface: AFAICS we would report "Not affected" in this case. > I don't think that's a good > assumption, because we don't necessary have knowledge about partner or > future CPU implementations, so I think any CPU lists really have to be > whitelists like they are for the other vulnerabilities. I think the idea was to cover all those "legacy" systems which have older cores (no SSBS), but didn't get an firmware update. So your old Seattle would truthfully report "Vulnerable", but any random A53 device would report "Not affected", even with ancient firmware. Cheers, Andre. > > if (this_cpu_has_cap(ARM64_SSBS)) { > > required = false; > > goto out_printmsg; > > @@ -419,12 +423,14 @@ static bool has_ssbd_mitigation(const struct arm64_cpu_capabilities *entry, > > ssbd_state = ARM64_SSBD_UNKNOWN; > > return false; > > > > + /* machines with mixed mitigation requirements must not return this */ > > case SMCCC_RET_NOT_REQUIRED: > > pr_info_once("%s mitigation not required\n", entry->desc); > > ssbd_state = ARM64_SSBD_MITIGATED; > > return false; > > > > case SMCCC_RET_SUCCESS: > > + __ssb_safe = false; > > required = true; > > break; > > > > @@ -474,6 +480,16 @@ static bool has_ssbd_mitigation(const struct arm64_cpu_capabilities *entry, > > return required; > > } > > > > +/* known vulnerable cores */ > > +static const struct midr_range arm64_ssb_cpus[] = { > > + MIDR_ALL_VERSIONS(MIDR_CORTEX_A57), > > + MIDR_ALL_VERSIONS(MIDR_CORTEX_A72), > > + MIDR_ALL_VERSIONS(MIDR_CORTEX_A73), > > + MIDR_ALL_VERSIONS(MIDR_CORTEX_A75), > > + MIDR_ALL_VERSIONS(MIDR_CORTEX_A76), > > + {}, > > +}; > > + > > static void __maybe_unused > > cpu_enable_cache_maint_trap(const struct arm64_cpu_capabilities *__unused) > > { > > @@ -769,6 +785,7 @@ const struct arm64_cpu_capabilities arm64_errata[] = { > > .capability = ARM64_SSBD, > > .type = ARM64_CPUCAP_LOCAL_CPU_ERRATUM, > > .matches = has_ssbd_mitigation, > > + .midr_range_list = arm64_ssb_cpus, > > }, > > #ifdef CONFIG_ARM64_ERRATUM_1188873 > > { > > @@ -807,3 +824,30 @@ ssize_t cpu_show_spectre_v2(struct device *dev, struct device_attribute *attr, > > > > return sprintf(buf, "Vulnerable\n"); > > } > > + > > +ssize_t cpu_show_spec_store_bypass(struct device *dev, > > + struct device_attribute *attr, char *buf) > > +{ > > + /* > > + * Two assumptions: First, ssbd_state reflects the worse case > > + * for heterogeneous machines, and that if SSBS is supported its > > + * supported by all cores. > > + */ > > + switch (ssbd_state) { > > + case ARM64_SSBD_MITIGATED: > > + return sprintf(buf, "Not affected\n"); > > + > > + case ARM64_SSBD_KERNEL: > > + case ARM64_SSBD_FORCE_ENABLE: > > + if (cpus_have_cap(ARM64_SSBS)) > > + return sprintf(buf, "Not affected\n"); > > + if (IS_ENABLED(CONFIG_ARM64_SSBD)) > > + return sprintf(buf, > > + "Mitigation: Speculative Store Bypass disabled\n"); > > x86 has a message about the prctl(), which we also support. > > Will
WARNING: multiple messages have this Message-ID (diff)
From: Andre Przywara <andre.przywara@arm.com> To: Will Deacon <will.deacon@arm.com> Cc: Stefan Wahren <stefan.wahren@i2se.com>, mlangsdo@redhat.com, suzuki.poulose@arm.com, marc.zyngier@arm.com, catalin.marinas@arm.com, julien.thierry@arm.com, linux-kernel@vger.kernel.org, Jeremy Linton <jeremy.linton@arm.com>, stefan.wahren@i2e.com, shankerd@codeaurora.org, Dave.Martin@arm.com, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v6 09/10] arm64: add sysfs vulnerability show for speculative store bypass Date: Fri, 5 Apr 2019 11:10:22 +0100 [thread overview] Message-ID: <20190405111022.6ef0b7bb@donnerap.cambridge.arm.com> (raw) In-Reply-To: <20190403165005.GA17500@fuggles.cambridge.arm.com> On Wed, 3 Apr 2019 17:50:05 +0100 Will Deacon <will.deacon@arm.com> wrote: > Hi Jeremy, > > On Thu, Mar 21, 2019 at 06:05:56PM -0500, Jeremy Linton wrote: > > Return status based on ssbd_state and the arm64 SSBS feature. If > > the mitigation is disabled, or the firmware isn't responding then > > return the expected machine state based on a new blacklist of known > > vulnerable cores. > > > > Signed-off-by: Jeremy Linton <jeremy.linton@arm.com> > > Reviewed-by: Andre Przywara <andre.przywara@arm.com> > > Tested-by: Stefan Wahren <stefan.wahren@i2se.com> > > --- > > arch/arm64/kernel/cpu_errata.c | 44 ++++++++++++++++++++++++++++++++++ > > 1 file changed, 44 insertions(+) > > > > diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c > > index 6958dcdabf7d..172ffbabd597 100644 > > --- a/arch/arm64/kernel/cpu_errata.c > > +++ b/arch/arm64/kernel/cpu_errata.c > > @@ -278,6 +278,7 @@ static int detect_harden_bp_fw(void) > > DEFINE_PER_CPU_READ_MOSTLY(u64, arm64_ssbd_callback_required); > > > > int ssbd_state __read_mostly = ARM64_SSBD_KERNEL; > > +static bool __ssb_safe = true; > > > > static const struct ssbd_options { > > const char *str; > > @@ -386,6 +387,9 @@ static bool has_ssbd_mitigation(const struct arm64_cpu_capabilities *entry, > > > > WARN_ON(scope != SCOPE_LOCAL_CPU || preemptible()); > > > > + if (is_midr_in_range_list(read_cpuid_id(), entry->midr_range_list)) > > + __ssb_safe = false; > > + > > Does this mean that we assume that CPUs not present in our table are not > affected by speculative store bypass? No, not affected are only those where we either have SSBS or the firmware explicitly returns SMCCC_RET_NOT_REQUIRED. This is governed by ssbd_state. So this doesn't affect correctness. __ssb_safe is an additional state just used for the sysfs output. But indeed it looks like this is wrong if the CPU is both not listed and the system doesn't provide the firmware interface: AFAICS we would report "Not affected" in this case. > I don't think that's a good > assumption, because we don't necessary have knowledge about partner or > future CPU implementations, so I think any CPU lists really have to be > whitelists like they are for the other vulnerabilities. I think the idea was to cover all those "legacy" systems which have older cores (no SSBS), but didn't get an firmware update. So your old Seattle would truthfully report "Vulnerable", but any random A53 device would report "Not affected", even with ancient firmware. Cheers, Andre. > > if (this_cpu_has_cap(ARM64_SSBS)) { > > required = false; > > goto out_printmsg; > > @@ -419,12 +423,14 @@ static bool has_ssbd_mitigation(const struct arm64_cpu_capabilities *entry, > > ssbd_state = ARM64_SSBD_UNKNOWN; > > return false; > > > > + /* machines with mixed mitigation requirements must not return this */ > > case SMCCC_RET_NOT_REQUIRED: > > pr_info_once("%s mitigation not required\n", entry->desc); > > ssbd_state = ARM64_SSBD_MITIGATED; > > return false; > > > > case SMCCC_RET_SUCCESS: > > + __ssb_safe = false; > > required = true; > > break; > > > > @@ -474,6 +480,16 @@ static bool has_ssbd_mitigation(const struct arm64_cpu_capabilities *entry, > > return required; > > } > > > > +/* known vulnerable cores */ > > +static const struct midr_range arm64_ssb_cpus[] = { > > + MIDR_ALL_VERSIONS(MIDR_CORTEX_A57), > > + MIDR_ALL_VERSIONS(MIDR_CORTEX_A72), > > + MIDR_ALL_VERSIONS(MIDR_CORTEX_A73), > > + MIDR_ALL_VERSIONS(MIDR_CORTEX_A75), > > + MIDR_ALL_VERSIONS(MIDR_CORTEX_A76), > > + {}, > > +}; > > + > > static void __maybe_unused > > cpu_enable_cache_maint_trap(const struct arm64_cpu_capabilities *__unused) > > { > > @@ -769,6 +785,7 @@ const struct arm64_cpu_capabilities arm64_errata[] = { > > .capability = ARM64_SSBD, > > .type = ARM64_CPUCAP_LOCAL_CPU_ERRATUM, > > .matches = has_ssbd_mitigation, > > + .midr_range_list = arm64_ssb_cpus, > > }, > > #ifdef CONFIG_ARM64_ERRATUM_1188873 > > { > > @@ -807,3 +824,30 @@ ssize_t cpu_show_spectre_v2(struct device *dev, struct device_attribute *attr, > > > > return sprintf(buf, "Vulnerable\n"); > > } > > + > > +ssize_t cpu_show_spec_store_bypass(struct device *dev, > > + struct device_attribute *attr, char *buf) > > +{ > > + /* > > + * Two assumptions: First, ssbd_state reflects the worse case > > + * for heterogeneous machines, and that if SSBS is supported its > > + * supported by all cores. > > + */ > > + switch (ssbd_state) { > > + case ARM64_SSBD_MITIGATED: > > + return sprintf(buf, "Not affected\n"); > > + > > + case ARM64_SSBD_KERNEL: > > + case ARM64_SSBD_FORCE_ENABLE: > > + if (cpus_have_cap(ARM64_SSBS)) > > + return sprintf(buf, "Not affected\n"); > > + if (IS_ENABLED(CONFIG_ARM64_SSBD)) > > + return sprintf(buf, > > + "Mitigation: Speculative Store Bypass disabled\n"); > > x86 has a message about the prctl(), which we also support. > > Will _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2019-04-05 10:10 UTC|newest] Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-03-21 23:05 [PATCH v6 00/10] arm64: add system vulnerability sysfs entries Jeremy Linton 2019-03-21 23:05 ` Jeremy Linton 2019-03-21 23:05 ` [PATCH v6 01/10] arm64: Provide a command line to disable spectre_v2 mitigation Jeremy Linton 2019-03-21 23:05 ` Jeremy Linton 2019-03-21 23:05 ` [PATCH v6 02/10] arm64: add sysfs vulnerability show for spectre v1 Jeremy Linton 2019-03-21 23:05 ` Jeremy Linton 2019-03-21 23:05 ` [PATCH v6 03/10] arm64: add sysfs vulnerability show for meltdown Jeremy Linton 2019-03-21 23:05 ` Jeremy Linton 2019-03-25 10:32 ` Andre Przywara 2019-03-25 10:32 ` Andre Przywara 2019-03-21 23:05 ` [PATCH v6 04/10] arm64: Advertise mitigation of Spectre-v2, or lack thereof Jeremy Linton 2019-03-21 23:05 ` Jeremy Linton 2019-03-25 10:59 ` Suzuki K Poulose 2019-03-25 10:59 ` Suzuki K Poulose 2019-03-21 23:05 ` [PATCH v6 05/10] arm64: Use firmware to detect CPUs that are not affected by Spectre-v2 Jeremy Linton 2019-03-21 23:05 ` Jeremy Linton 2019-03-21 23:05 ` [PATCH v6 06/10] arm64: Always enable spectrev2 vulnerability detection Jeremy Linton 2019-03-21 23:05 ` Jeremy Linton 2019-03-21 23:05 ` [PATCH v6 07/10] arm64: add sysfs vulnerability show for spectre v2 Jeremy Linton 2019-03-21 23:05 ` Jeremy Linton 2019-03-21 23:05 ` [PATCH v6 08/10] arm64: Always enable ssb vulnerability detection Jeremy Linton 2019-03-21 23:05 ` Jeremy Linton 2019-03-21 23:05 ` [PATCH v6 09/10] arm64: add sysfs vulnerability show for speculative store bypass Jeremy Linton 2019-03-21 23:05 ` Jeremy Linton 2019-04-03 16:50 ` Will Deacon 2019-04-03 16:50 ` Will Deacon 2019-04-05 10:10 ` Andre Przywara [this message] 2019-04-05 10:10 ` Andre Przywara 2019-04-05 14:43 ` Will Deacon 2019-04-05 14:43 ` Will Deacon 2019-04-05 15:18 ` Andre Przywara 2019-04-05 15:18 ` Andre Przywara 2019-04-05 16:01 ` Jeremy Linton 2019-04-05 16:01 ` Jeremy Linton 2019-03-21 23:05 ` [PATCH v6 10/10] arm64: enable generic CPU vulnerabilites support Jeremy Linton 2019-03-21 23:05 ` Jeremy Linton 2019-03-22 17:49 ` Stefan Wahren 2019-03-22 17:49 ` Stefan Wahren 2019-03-25 10:33 ` [PATCH v6 00/10] arm64: add system vulnerability sysfs entries Andre Przywara 2019-03-25 10:33 ` Andre Przywara 2019-03-25 12:22 ` Catalin Marinas 2019-03-25 12:22 ` Catalin Marinas
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=20190405111022.6ef0b7bb@donnerap.cambridge.arm.com \ --to=andre.przywara@arm.com \ --cc=Dave.Martin@arm.com \ --cc=catalin.marinas@arm.com \ --cc=jeremy.linton@arm.com \ --cc=julien.thierry@arm.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=marc.zyngier@arm.com \ --cc=mlangsdo@redhat.com \ --cc=shankerd@codeaurora.org \ --cc=stefan.wahren@i2e.com \ --cc=stefan.wahren@i2se.com \ --cc=suzuki.poulose@arm.com \ --cc=will.deacon@arm.com \ /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: linkBe 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.