Jan Beulich: > Make sure no previously present features are missing after resume (and > the re-loading of microcode), to avoid later crashes or (likely silent) > hangs / live locks. This doesn't go beyond checking x86_capability[], > but this should be good enough for the immediate need of making sure > that the BIT mitigation MSRs are still available. > > Signed-off-by: Jan Beulich <[email protected]> > > --- a/xen/arch/x86/acpi/power.c > +++ b/xen/arch/x86/acpi/power.c > @@ -254,6 +254,9 @@ static int enter_state(u32 state) > > microcode_resume_cpu(0); > > + if ( !recheck_cpu_features(0) ) > + panic("Missing previously available feature(s)."); > + > ci->bti_ist_info = default_bti_ist_info; > asm volatile (ALTERNATIVE("", "wrmsr", X86_FEATURE_XEN_IBRS_SET) > :: "a" (SPEC_CTRL_IBRS), "c" (MSR_SPEC_CTRL), "d" (0) > --- a/xen/arch/x86/cpu/common.c > +++ b/xen/arch/x86/cpu/common.c > @@ -501,6 +501,9 @@ void identify_cpu(struct cpuinfo_x86 *c) > printk("\n"); > #endif > > + if (system_state == SYS_STATE_resume) > + return; > + > /* > * On SMP, boot_cpu_data holds the common feature set between > * all CPUs; so make sure that we indicate which features are > --- a/xen/arch/x86/cpuid.c > +++ b/xen/arch/x86/cpuid.c > @@ -473,6 +473,28 @@ void __init init_guest_cpuid(void) > calculate_hvm_max_policy(); > } > > +bool recheck_cpu_features(unsigned int cpu) > +{ > + bool okay = true; > + struct cpuinfo_x86 c; > + const struct cpuinfo_x86 *bsp = &boot_cpu_data; > + unsigned int i; > + > + identify_cpu(&c);
This runs into a bug in identify_cpu(). x86_vendor_id does not get
zeroed, so the x86_vendor_id is not null terminated and the vendor
identification fails.
diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c
index 4feaa2ceb6..5750d26216 100644
--- a/xen/arch/x86/cpu/common.c
+++ b/xen/arch/x86/cpu/common.c
@@ -366,8 +366,8 @@ void identify_cpu(struct cpuinfo_x86 *c)
c->x86_vendor = X86_VENDOR_UNKNOWN;
c->cpuid_level = -1; /* CPUID not detected */
c->x86_model = c->x86_mask = 0; /* So far unknown... */
- c->x86_vendor_id[0] = '\0'; /* Unset */
- c->x86_model_id[0] = '\0'; /* Unset */
+ memset(&c->x86_vendor_id, 0, sizeof(c->x86_vendor_id));
+ memset(&c->x86_model_id, 0, sizeof(c->x86_model_id));
c->x86_max_cores = 1;
c->x86_num_siblings = 1;
c->x86_clflush_size = 0;
With this patch it works for me.
> +
> + for ( i = 0; i < NCAPINTS; ++i )
> + {
> + if ( !(~c.x86_capability[i] & bsp->x86_capability[i]) )
> + continue;
> +
> + printk(XENLOG_ERR "CPU%u: cap[%2u] is %08x (expected %08x)\n",
> + cpu, i, c.x86_capability[i], bsp->x86_capability[i]);
> + okay = false;
> + }
> +
> + return okay;
> +}
> +
> const uint32_t *lookup_deep_deps(uint32_t feature)
> {
> static const struct {
> --- a/xen/arch/x86/smpboot.c
> +++ b/xen/arch/x86/smpboot.c
> @@ -90,11 +90,14 @@ void initialize_cpu_data(unsigned int cp
> cpu_data[cpu] = boot_cpu_data;
> }
>
> -static void smp_store_cpu_info(int id)
> +static bool smp_store_cpu_info(unsigned int id)
> {
> unsigned int socket;
>
> - identify_cpu(&cpu_data[id]);
> + if ( system_state != SYS_STATE_resume )
> + identify_cpu(&cpu_data[id]);
> + else if ( !recheck_cpu_features(id) )
> + return false;
>
> socket = cpu_to_socket(id);
> if ( !socket_cpumask[socket] )
> @@ -102,6 +105,8 @@ static void smp_store_cpu_info(int id)
> socket_cpumask[socket] = secondary_socket_cpumask;
> secondary_socket_cpumask = NULL;
> }
> +
> + return true;
> }
>
> /*
> @@ -187,12 +192,19 @@ static void smp_callin(void)
> setup_local_APIC();
>
> /* Save our processor parameters. */
> - smp_store_cpu_info(cpu);
> + if ( !smp_store_cpu_info(cpu) )
> + {
> + printk("CPU%u: Failed to validate features - not coming back
> online\n",
> + cpu);
> + cpu_error = -ENXIO;
> + goto halt;
> + }
>
> if ( (rc = hvm_cpu_up()) != 0 )
> {
> printk("CPU%d: Failed to initialise HVM. Not coming online.\n", cpu);
> cpu_error = rc;
> + halt:
> clear_local_APIC();
> spin_debug_enable();
> cpu_exit_clear(cpu);
> --- a/xen/include/asm-x86/cpuid.h
> +++ b/xen/include/asm-x86/cpuid.h
> @@ -253,6 +253,9 @@ static inline void cpuid_featureset_to_p
> extern struct cpuid_policy raw_cpuid_policy, host_cpuid_policy,
> pv_max_cpuid_policy, hvm_max_cpuid_policy;
>
> +/* Check that all previously present features are still available. */
> +bool recheck_cpu_features(unsigned int cpu);
> +
> /* Allocate and initialise a CPUID policy suitable for the domain. */
> int init_domain_cpuid_policy(struct domain *d);
>
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Xen-devel mailing list [email protected] https://lists.xenproject.org/mailman/listinfo/xen-devel
