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

Re: [Xen-devel] [PATCH 5/5] xen/arm64: Implement branch predictor hardening for affected Cortex-A CPUs



Hi,

On 17/01/18 17:11, Stefano Stabellini wrote:
On Wed, 17 Jan 2018, Julien Grall wrote:
Hi Stefano,

On 17/01/18 00:42, Stefano Stabellini wrote:
On Tue, 16 Jan 2018, Julien Grall wrote:
   #define MIDR_RANGE(model, min, max)     \
@@ -205,6 +232,28 @@ static const struct arm_cpu_capabilities arm_errata[]
= {
                      (1 << MIDR_VARIANT_SHIFT) | 2),
       },
   #endif
+#ifdef CONFIG_ARM64_HARDEN_BRANCH_PREDICTOR
+    {
+        .capability = ARM_HARDEN_BRANCH_PREDICTOR,
+        MIDR_ALL_VERSIONS(MIDR_CORTEX_A57),
+        .enable = enable_psci_bp_hardening,
+    },
+    {
+        .capability = ARM_HARDEN_BRANCH_PREDICTOR,
+        MIDR_ALL_VERSIONS(MIDR_CORTEX_A72),
+        .enable = enable_psci_bp_hardening,
+    },
+    {
+        .capability = ARM_HARDEN_BRANCH_PREDICTOR,
+        MIDR_ALL_VERSIONS(MIDR_CORTEX_A73),
+        .enable = enable_psci_bp_hardening,
+    },
+    {
+        .capability = ARM_HARDEN_BRANCH_PREDICTOR,
+        MIDR_ALL_VERSIONS(MIDR_CORTEX_A75),
+        .enable = enable_psci_bp_hardening,
+    },

We need to add a basic description in the desc field as it is printed by
update_cpu_capabilities.

desc field is not mandatory, and in that case I think the print would be
confusing. At the difference of the other errata, we have more check in
install_bp_hardening_vec that may result to skip the hardening.

The errata here is covering all variant/revision of A75 models for safety
reason. Arm has introduced a new field ID_AA64PFR0_EL1.CSV2 to tell whether a
branch predictor trained in one context will affect speculative execution in
another context. This field is checked in install_bp_hardening_vec so you
avoid to harden the vector tables and small performance hit.

IHMO, it would be better to have a per-CPU message in install_bp_hardening_vec
announcing whether the vector tables has been harden and the kind of
hardening.

What do you think?

I understand what you are saying and I agree with you. Maybe we should
change update_cpu_capabilities to print "detected need for workaround:
blah" but you don't have to do it in this series.

"need for workaround..." is not entirely true for the branch predictor hardening. It is more a "may need". I still prefer the per-CPU message
"CPU%u will %s on guest exit"

%u is the CPU number. %s will be the name of the call/instruction to execute.

The rationale behind is branch predictor hardening may be different on each CPU (think big.LITTLE) so code executed will be different. This differ from the other errata that will be applied for all CPUs.

Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

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