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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <577FBE6702000078000FCA81@prv-mh.provo.novell.com>
Date:	Fri, 08 Jul 2016 06:53:27 -0600
From:	"Jan Beulich" <JBeulich@...e.com>
To:	"David Vrabel" <david.vrabel@...rix.com>,
	<boris.ostrovsky@...cle.com>, "Juergen Gross" <JGross@...e.com>
Cc:	<xen-devel@...ts.xenproject.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] xen/acpi: allow xen-acpi-processor driver to load
 on Xen 4.7

>>> On 08.07.16 at 14:29, <david.vrabel@...rix.com> 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