[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

>>> On 11.04.18 at 22:21, <simon@xxxxxxxxxxxxxxxxxxxxxx> 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).


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.