Re: [PATCH v6 04/12] microcode: introduce a global cache of ucode patch

From: Sergey Dyasli <sergey.dyasli@citrix.com>
To: Chao Gao <chao.gao@intel.com>, xen-devel@lists.xenproject.org
Cc: "sergey.dyasli@citrix.com >> Sergey Dyasli"
	<sergey.dyasli@citrix.com>, "Wei Liu" <wei.liu2@citrix.com>,
	"Ashok Raj" <ashok.raj@intel.com>,
	"Andrew Cooper" <andrew.cooper3@citrix.com>,
	"Jan Beulich" <jbeulich@suse.com>,
	"Roger Pau Monné" <roger.pau@citrix.com>
Subject: Re: [PATCH v6 04/12] microcode: introduce a global cache of ucode patch
Date: Wed, 13 Mar 2019 16:36:50 +0000	[thread overview]
Message-ID: <b761d5b1-3dc6-0e0a-e81d-b69cdcc7166d@citrix.com> (raw)
In-Reply-To: <1552291056-20286-5-git-send-email-chao.gao@intel.com>

On 11/03/2019 07:57, Chao Gao wrote:
> to replace the current per-cpu cache 'uci->mc'.
> 
> Compared to the current per-cpu cache, the benefits of the global
> microcode cache are:
> 1. It reduces the work that need to be done on each CPU. Parsing ucode
> file is done once on one CPU. Other CPUs needn't parse ucode file.
> Instead, they can find out and load the newest patch from the global
> cache.
> 2. It reduces the memory consumption on a system with many CPU cores.
> 
> Two functions, microcode_save_patch() and microcode_find_patch() are
> introduced. The former adds one given patch to the global cache. The
> latter gets a newer and matched ucode patch from the global cache.
> All operations on the cache is expected to be done with the
> 'microcode_mutex' hold.
> 
> Note that I deliberately avoid touching 'uci->mc' as I am going to
> remove it completely in the next patch.
> 
> Signed-off-by: Chao Gao <chao.gao@intel.com>
> ---
> Changes in v6:
>  - constify local variables and function parameters if possible
>  - comment that the global cache is protected by 'microcode_mutex'.
>    and add assertions to catch violations in microcode_{save/find}_patch()
> 
> Changes in v5:
>  - reword the commit description
>  - find_patch() and save_patch() are abstracted into common functions
>    with some hooks for AMD and Intel
> ---
>  xen/arch/x86/microcode.c        | 60 +++++++++++++++++++++++++++
>  xen/arch/x86/microcode_amd.c    | 91 +++++++++++++++++++++++++++++++++++++----
>  xen/arch/x86/microcode_intel.c  | 66 ++++++++++++++++++++++++++----
>  xen/include/asm-x86/microcode.h | 13 ++++++
>  4 files changed, 215 insertions(+), 15 deletions(-)

This needs the following change for the AMD part:

diff --git a/xen/arch/x86/microcode_amd.c b/xen/arch/x86/microcode_amd.c
index 4aa8fdca38..358f3c44e3 100644
--- a/xen/arch/x86/microcode_amd.c
+++ b/xen/arch/x86/microcode_amd.c
@@ -249,6 +249,7 @@ static enum microcode_match_result compare_patch(
 static int apply_microcode(void)
 {
     uint32_t rev;
+    struct microcode_amd *mc_amd;
     const struct microcode_header_amd *hdr;
     const struct microcode_patch *patch;
     int hw_err;
@@ -259,7 +260,8 @@ static int apply_microcode(void)
     if ( patch == NULL )
         return -EINVAL;

-    hdr = patch->data;
+    mc_amd = patch->data;
+    hdr = mc_amd->mpb;

     BUG_ON(local_irq_is_enabled());


Another thing to mention is that even with this fix applied, I get
early ucode update only on CPU0. With the whole series applied, secondary
CPUs also start doing early ucode update.

--
Thanks,
Sergey

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