[src/trunk]: src/sys/arch/x86/x86 xc_wait() does not wait for all cpus to finish

[cegger <cegger%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/6610297bcdcd
branches:  trunk
changeset: 779209:6610297bcdcd
user:      cegger <cegger%NetBSD.org@localhost>
date:      Thu May 10 12:35:53 2012 +0000

description:
xc_wait() does not wait for all cpus to finish
their callback. That means the ucode buffer is released while still in use
and this causes a crash.
Quick fix: check if the ucode buffer has been freed and abort.
You may need to run 'cpuctl ucode' twice to apply it to all cpus.

Per discussion with rmind@ use low priority xcalls and splhigh.

diffstat:

 sys/arch/x86/x86/cpu_ucode_amd.c |  19 +++++++++++++++----
 1 files changed, 15 insertions(+), 4 deletions(-)

diffs (68 lines):

diff -r 657aac0c9e76 -r 6610297bcdcd sys/arch/x86/x86/cpu_ucode_amd.c
--- a/sys/arch/x86/x86/cpu_ucode_amd.c  Thu May 10 11:47:09 2012 +0000
+++ b/sys/arch/x86/x86/cpu_ucode_amd.c  Thu May 10 12:35:53 2012 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: cpu_ucode_amd.c,v 1.2 2012/05/09 13:58:09 cegger Exp $ */
+/* $NetBSD: cpu_ucode_amd.c,v 1.3 2012/05/10 12:35:53 cegger Exp $ */
 /*
  * Copyright (c) 2012 The NetBSD Foundation, Inc.
  * All rights reserved.
@@ -29,7 +29,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: cpu_ucode_amd.c,v 1.2 2012/05/09 13:58:09 cegger Exp $");
+__KERNEL_RCSID(0, "$NetBSD: cpu_ucode_amd.c,v 1.3 2012/05/10 12:35:53 cegger Exp $");
 
 #include "opt_xen.h"
 #include "opt_cpu_ucode.h"
@@ -150,12 +150,14 @@
        struct microcode_amd mc_amd;
        struct mc_buf mc;
        device_t dev;
+       int s;
 
        memcpy(&mc, arg1, sizeof(mc));
        mc_amd.mpb = mc.mc_amd->mpb;
        mc_amd.mpb_size = mc.mc_amd->mpb_size;
 
        dev = curcpu()->ci_dev;
+       s = splhigh();
 
        do {
                uint64_t patchlevel;
@@ -196,8 +198,16 @@
                }
 
 next:
-               if (mc.mc_mpbuf == NULL)
+               /* Check for race:
+                * When we booted with -x a cpu might already finished
+                * (why doesn't xc_wait() wait for *all* cpus?)
+                * and sc->sc_blob is already freed.
+                * In this case the calculation below touches
+                * non-allocated memory and results in a crash.
+                */
+               if (sc->sc_blob == NULL)
                        break;
+
                mc.mc_buf += mc.mc_mpbuf->mpb_len +
                    sizeof(mc.mc_mpbuf->mpb_type) +
                    sizeof(mc.mc_mpbuf->mpb_len);
@@ -213,6 +223,7 @@
 out:
        if (error)
                ((struct mc_buf *)(arg1))->mc_error = error;
+       splx(s);
 }
 
 int
@@ -279,7 +290,7 @@
 
        /* Apply it on all cpus */
        mc.mc_error = 0;
-       where = xc_broadcast(XC_HIGHPRI, cpu_apply_cb, sc, &mc);
+       where = xc_broadcast(0, cpu_apply_cb, sc, &mc);
 
        /* Wait for completion */
        xc_wait(where);