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

[David Vrabel]

On 08/07/16 13:53, Jan Beulich wrote:
>>>> 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?

It's also possible that I'm misremembering why we went with the above hack.

I've applied your patch to for-linus-3.7b, thanks.

David