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

Re: [PATCH] xen/arm: Warn user on cpu errata 832075

Hi Bertrand,

On 14/10/2020 17:03, Bertrand Marquis wrote:

On 14 Oct 2020, at 12:35, Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:

On 14/10/2020 11:41, Bertrand Marquis wrote:
When a Cortex A57 processor is affected by CPU errata 832075, a guest
not implementing the workaround for it could deadlock the system.
Add a warning during boot informing the user that only trusted guests
should be executed on the system.
An equivalent warning is already given to the user by KVM on cores
affected by this errata.

Signed-off-by: Bertrand Marquis <bertrand.marquis@xxxxxxx>
xen/arch/arm/cpuerrata.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)

diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c
index 6c09017515..8f9ab6dde1 100644
--- a/xen/arch/arm/cpuerrata.c
+++ b/xen/arch/arm/cpuerrata.c
@@ -240,6 +240,26 @@ static int enable_ic_inv_hardening(void *data)


+#ifdef CONFIG_ARM64_ERRATUM_832075
+static int warn_device_load_acquire_errata(void *data)
+    static bool warned = false;
+    if ( !warned )
+    {
+        warning_add("This CPU is affected by the errata 832075.\n"
+                    "Guests without required CPU erratum workarounds\n"
+                    "can deadlock the system!\n"
+                    "Only trusted guests should be used on this system.\n");
+        warned = true;

This is an antipattern, which probably wants fixing elsewhere as well.

warning_add() is __init.  It's not legitimate to call from a non-init
function, and a less useless build system would have modpost to object.

The ARM_SMCCC_ARCH_WORKAROUND_1 instance asserts based on system state,
but this provides no safety at all.

What warning_add() actually does is queue messages for some point near
the end of boot.  It's not clear that this is even a clever thing to do.

I'm very tempted to suggest a blanket change to printk_once().

If this is needed then this could be done in an other serie ?

The callback ->enable() will be called when a CPU is onlined/offlined. So this is going to require if you plan to support CPU hotplugs or suspend resume.

Would be good to keep this patch as purely handling the errata.

In the case of this patch, how about moving the warning_add() in enable_errata_workarounds()?

By then we should now all the errata present on your platform. All CPUs onlined afterwards (i.e. runtime) should always abide to the set discover during boot.


Julien Grall



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