Re: [Xen-devel] [PATCH v4 03/11] x86/mce: handle host LMCE

["Jan Beulich" <jbeulich@xxxxxxxx>]

>>> Haozhong Zhang <haozhong.zhang@xxxxxxxxx> 06/26/17 11:17 AM >>>
>+/**
>+ * Append a telemetry of deferred MCE to a per-cpu pending list,
>+ * either @pending or @lmce_pending, according to rules below:
>+ *  - if @pending is not empty, then the new telemetry will be
>+ *    appended to @pending;
>+ *  - if @pending is empty and the new telemetry is for a deferred
>+ *    LMCE, then the new telemetry will be appended to @lmce_pending;
>+ *  - if @pending is empty and the new telemetry is for a deferred
>+ *    non-local MCE, all existing telemetries in @lmce_pending will be
>+ *    moved to @pending and then the new telemetry will be appended to
>+ *    @pending.
>+ *
>+ * This function must be called with MCIP bit set, so that it does not
>+ * need to worry about MC# re-occurring in this function.
>+ *
>+ * As a result, this function can preserve the mutual exclusivity
>+ * between @pending and @lmce_pending (see their comments in struct
>+ * mc_telem_cpu_ctl).
>+ *
>+ * Parameters:
>+ *  @cookie: telemetry of the deferred MCE
>+ *  @lmce:   indicate whether the telemetry is for LMCE
>+ */
>+void mctelem_defer(mctelem_cookie_t cookie, bool lmce)
 >{
        >struct mctelem_ent *tep = COOKIE2MCTE(cookie);
>-
>-      mctelem_xchg_head(&this_cpu(mctctl.pending), &tep->mcte_next, tep);
>+      struct mc_telem_cpu_ctl *mctctl = &this_cpu(mctctl);
>+
>+      ASSERT(mctctl->pending == NULL || mctctl->lmce_pending == NULL);
>+
>+      if (mctctl->pending)
>+              mctelem_xchg_head(&mctctl->pending, &tep->mcte_next, tep);
>+      else if (lmce)
>+              mctelem_xchg_head(&mctctl->lmce_pending, &tep->mcte_next, tep);
>+      else {
>+              if (mctctl->lmce_pending)
>+                      mctelem_xchg_head(&mctctl->lmce_pending,
>+                                        &mctctl->pending, NULL);

I don't think this is sufficiently proven to be safe: This may set ->pending to
non-NULL more than once, and while your comment above considers the
producer side, it doesn't consider the consumer(s). This is even more so that
the consumer side uses potentially stale information to tell which list head to
update.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel