Re: [PATCH] xen/acpi: allow xen-acpi-processor driver to load on Xen 4.7

[Jan Beulich]

>>> On 08.07.16 at 14:29, <david.vrabel@xxxxxxxxxx> wrote:
> On 08/07/16 13:15, Jan Beulich wrote:
>> As of Xen 4.7 PV CPUID doesn't expose either of CPUID[1].ECX[7] and
>> CPUID[0x80000007].EDX[7] anymore, causing the driver to fail to load on
>> both Intel and AMD systems. Doing any kind of hardware capability
>> checks in the driver as a prerequisite was wrong anyway: With the
>> hypervisor being in charge, all such checking should be done by it. If
>> ACPI data gets uploaded despite some missing capability, the hypervisor
>> is free to ignore part or all of that data.
>> 
>> Ditch the entire check_prereq() function, and do the only valid check
>> (xen_initial_domain()) in the caller in its place.
> 
> Thanks, but I'm not sure this is sufficient.  I think the generic ACPI
> code needs to know the full capabilities in order to generate the
> correct tables, or you won't get (for example) turbo mode working.
> 
> We had to fake the EST feature back in.
> 
> --- a/arch/x86/xen/enlighten.c
> +++ b/arch/x86/xen/enlighten.c
> @@ -448,7 +448,8 @@ static void __init xen_init_cpuid_mask(void)
>  	if ((cx & xsave_mask) != xsave_mask)
>  		cpuid_leaf1_ecx_mask &= ~xsave_mask; /* disable XSAVE & OSXSAVE */
>  	if (xen_check_mwait())
> -		cpuid_leaf1_ecx_set_mask = (1 << (X86_FEATURE_MWAIT % 32));
> +		cpuid_leaf1_ecx_set_mask = (1 << (X86_FEATURE_MWAIT % 32)
> +					   | 1 << (X86_FEATURE_EST % 32));
>  }
> 
>  static void xen_set_debugreg(int reg, unsigned long val)

Hmm, interesting. I admit I only tested on an AMD system, so I
can't exclude the above is necessary. Otoh going over generic
ACPI code the only use of X86_FEATURE_EST controls the
logging of a message. Plus there's a use in
arch_acpi_set_pdc_bits() - perhaps that's the one you mean?

There's certainly no use of X86_FEATURE_HW_PSTATE anywhere
in relevant code, so the AMD side would appear to be fine (which
matches my testing). So I think the patch is fine as is (also avoiding
cross component adjustments), and the part you suggest may then
better be a separate patch?

Jan