Loading microcode can cause new features to appear.  This has happened
routinely since Spectre/Meltdown, and even the presence of new status bits can
sometimes mean the administrator has no further actions to perform.

Conversely, loading microcode can occasionally cause features to disappear.
As with livepatching, it is very much the administrators responsibility to
confirm that a late microcode load is safe on the intended system before
rolling it out in production.

Refresh the raw CPU policy after late microcode load appears to have done
something, so xen-cpuid can reflect the updated state of the system.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>

This is also the first step of being able to livepatch support for new
functionality in microcode.

v2:
 * Discuss disappearing features in the commit message
---
 xen/arch/x86/cpu-policy.c             | 6 +++---
 xen/arch/x86/cpu/microcode/core.c     | 4 ++++
 xen/arch/x86/include/asm/cpu-policy.h | 6 ++++++
 3 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/cpu-policy.c b/xen/arch/x86/cpu-policy.c
index a58bf6cad54e..ef6a2d0d180a 100644
--- a/xen/arch/x86/cpu-policy.c
+++ b/xen/arch/x86/cpu-policy.c
@@ -15,7 +15,7 @@
 #include <asm/setup.h>
 #include <asm/xstate.h>
 
-struct cpu_policy __ro_after_init     raw_cpu_policy;
+struct cpu_policy __read_mostly       raw_cpu_policy;
 struct cpu_policy __ro_after_init    host_cpu_policy;
 #ifdef CONFIG_PV
 struct cpu_policy __ro_after_init  pv_max_cpu_policy;
@@ -343,7 +343,7 @@ static void recalculate_misc(struct cpu_policy *p)
     }
 }
 
-static void __init calculate_raw_policy(void)
+void calculate_raw_cpu_policy(void)
 {
     struct cpu_policy *p = &raw_cpu_policy;
 
@@ -655,7 +655,7 @@ static void __init calculate_hvm_def_policy(void)
 
 void __init init_guest_cpu_policies(void)
 {
-    calculate_raw_policy();
+    calculate_raw_cpu_policy();
     calculate_host_policy();
 
     if ( IS_ENABLED(CONFIG_PV) )
diff --git a/xen/arch/x86/cpu/microcode/core.c b/xen/arch/x86/cpu/microcode/core.c
index 61cd36d601d6..cd456c476fbf 100644
--- a/xen/arch/x86/cpu/microcode/core.c
+++ b/xen/arch/x86/cpu/microcode/core.c
@@ -34,6 +34,7 @@
 #include <xen/watchdog.h>
 
 #include <asm/apic.h>
+#include <asm/cpu-policy.h>
 #include <asm/delay.h>
 #include <asm/nmi.h>
 #include <asm/processor.h>
@@ -677,6 +678,9 @@ static long cf_check microcode_update_helper(void *data)
         spin_lock(&microcode_mutex);
         microcode_update_cache(patch);
         spin_unlock(&microcode_mutex);
+
+        /* Refresh the raw CPU policy, in case the features have changed. */
+        calculate_raw_cpu_policy();
     }
     else
         microcode_free_patch(patch);
diff --git a/xen/arch/x86/include/asm/cpu-policy.h b/xen/arch/x86/include/asm/cpu-policy.h
index b361537a602b..99d5a8e67eeb 100644
--- a/xen/arch/x86/include/asm/cpu-policy.h
+++ b/xen/arch/x86/include/asm/cpu-policy.h
@@ -24,4 +24,10 @@ void init_dom0_cpuid_policy(struct domain *d);
 /* Clamp the CPUID policy to reality. */
 void recalculate_cpuid_policy(struct domain *d);
 
+/*
+ * Collect the raw CPUID and MSR values.  Called during boot, and after late
+ * microcode loading.
+ */
+void calculate_raw_cpu_policy(void);
+
 #endif /* X86_CPU_POLICY_H */
-- 
2.30.2

On Thu, May 04, 2023 at 11:26:07AM +0100, Andrew Cooper wrote:
> Loading microcode can cause new features to appear.  This has happened
> routinely since Spectre/Meltdown, and even the presence of new status bits can
> sometimes mean the administrator has no further actions to perform.
> 
> Conversely, loading microcode can occasionally cause features to disappear.
> As with livepatching, it is very much the administrators responsibility to
> confirm that a late microcode load is safe on the intended system before
> rolling it out in production.
> 
> Refresh the raw CPU policy after late microcode load appears to have done
> something, so xen-cpuid can reflect the updated state of the system.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Roger Pau Monné <roger.pau@citrix.com>

I'm not fully sure it's worth calling calculate_raw_cpu_policy() if
updated != nr_cores, as it's possible the current CPU where the policy
is regenerated hasn't been updated.

Thanks, Roger.

On 04/05/2023 11:47 am, Roger Pau Monné wrote:
> On Thu, May 04, 2023 at 11:26:07AM +0100, Andrew Cooper wrote:
>> Loading microcode can cause new features to appear.  This has happened
>> routinely since Spectre/Meltdown, and even the presence of new status bits can
>> sometimes mean the administrator has no further actions to perform.
>>
>> Conversely, loading microcode can occasionally cause features to disappear.
>> As with livepatching, it is very much the administrators responsibility to
>> confirm that a late microcode load is safe on the intended system before
>> rolling it out in production.
>>
>> Refresh the raw CPU policy after late microcode load appears to have done
>> something, so xen-cpuid can reflect the updated state of the system.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> Acked-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks.

> I'm not fully sure it's worth calling calculate_raw_cpu_policy() if
> updated != nr_cores, as it's possible the current CPU where the policy
> is regenerated hasn't been updated.

As said previously, updated != nr_cores may exist in principle, but it
is not a corner case we should care about.

If the system really is asymmetric after a late load, then it is
probably not long for the world.  There is nothing Xen can do to recover.


It is awkward that we're not on the BSP when collecting.  I think that
part of the logic didn't survive the rebase over switching to
stop-machine.  I'll add it to the gitlab ticket of minor ucode work,
because it's not the end of the world.

~Andrew