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

Re: [Xen-devel] [PATCH] x86/mwait-idle: Tweak reporting when MONITOR is not available



>>> On 27.09.18 at 13:12, <andrew.cooper3@xxxxxxxxxx> wrote:
> Currently, booting Xen as a PVH guest yields:
> 
>   (d10) (XEN) mwait-idle: does not run on family 6 model 60
> 
> which is inaccurate.
> 
> The problem is the lack of monitor, rather than the family/model.  Combine the
> two CPUID checks and skip the list search in the case that is is going to fail
> for feature reasons.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

For the specific case here and at this point in time, the change is acceptable,
so feel free to use
Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
However,

> --- a/xen/arch/x86/cpu/mwait-idle.c
> +++ b/xen/arch/x86/cpu/mwait-idle.c
> @@ -1113,17 +1113,17 @@ static void __init mwait_idle_state_table_update(void)
>  static int __init mwait_idle_probe(void)
>  {
>       unsigned int eax, ebx, ecx;
> -     const struct x86_cpu_id *id = x86_match_cpu(intel_idle_ids);
> +     const struct x86_cpu_id *id;
>  
> -     if (!id) {
> +     if (!cpu_has_monitor || boot_cpu_data.cpuid_level < CPUID_MWAIT_LEAF)
> +             return -ENODEV;
> +
> +     if (!(id = x86_match_cpu(intel_idle_ids))) {
>               pr_debug(PREFIX "does not run on family %d model %d\n",
>                        boot_cpu_data.x86, boot_cpu_data.x86_model);
>               return -ENODEV;
>       }

... the cpu_has_monitor check is (here) redundant with what intel_idle_ids[]
says, and in the more general case (including possibly here, going forward)
there might be entries in such a table with differing feature flag requests.
Therefore I would prefer if the redundancy was not introduced (also to
avoid others [blindly] cloning this code), and instead the log message be
made less misleading in the PVH case. The moving ahead of the CPUID level
check, otoh, is fine with me (but won't help your case at all afaict), since
new (future) feature dependencies are rather unlikely to be towards lower
levels.

Jan



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

 


Rackspace

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