Re: [PATCH v2 2/2] x86: correct ordering of operations during S3 resume

["Jan Beulich" <JBeulich@suse.com>]

>>> On 11.04.18 at 22:21, <simon@invisiblethingslab.com> wrote:
> @@ -243,17 +244,21 @@ static int enter_state(u32 state)
>      if ( (state == ACPI_STATE_S3) && error )
>          tboot_s3_error(error);
>  
> +    console_end_sync();
> +
> +    error = microcode_resume_cpu(0);
> +    if (error && error != -ENOENT)

Missing blanks.

> +        panic("Could not restore microcode on boot cpu (%d)", error);

Andrew, you've suggested the panic() here, but I'm not convinced
this is strictly necessary. For most ucode updates we're fine without,
and could accept them being re-applied post-resume. That'll make
a lot of false positive panics. Furthermore, in case there are really
mixed steppings, receiving -ENOENT here still means we're going to
die soon after. I.e. (rare) false negatives are possible as well.

Instead what I think we want is a feature comparison after resume:
Any feature we (or any alive guests) have in active use needs to be
available. That is, host_cpuid_policy must not have changed (the
two {hvm,pv}_max_cpuid_policy are only derived, and hence won't
need separate checking afaict; without that checking
host_cpuid_policy might be too strict, but then again compiling a
list of features we actually use would be prone to go stale very
quickly once use of new features is introduced, unless we tied this
into cpu_has_* checks, implying that any such check means the
feature is used if available, yet that would in turn have issues in
particular with the uses in the insn emulator).

Jan


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