Re: [Xen-devel] [PATCH 3/3] x86: check feature flags after resume

[Simon Gaiser <simon@xxxxxxxxxxxxxxxxxxxxxx>]

Simon Gaiser:
> 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 <jbeulich@xxxxxxxx>
>>
>> --- 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.

Meh, also a backport failure from me. Since e34bc403c3c7 this problem
should not appear since it does not assume a null terminated string.

Attachment: signature.asc
Description: OpenPGP digital signature

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