From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755442AbcGHNxR (ORCPT ); Fri, 8 Jul 2016 09:53:17 -0400 Received: from smtp02.citrix.com ([66.165.176.63]:15756 "EHLO SMTP02.CITRIX.COM" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755073AbcGHNxN (ORCPT ); Fri, 8 Jul 2016 09:53:13 -0400 X-IronPort-AV: E=Sophos;i="5.28,330,1464652800"; d="scan'208";a="372023589" Subject: Re: [Xen-devel] [PATCH] xen/acpi: allow xen-acpi-processor driver to load on Xen 4.7 To: Jan Beulich , David Vrabel , , Juergen Gross References: <577FB56B02000078000FC9DB@prv-mh.provo.novell.com> <577F9CB2.5050406@citrix.com> <577FBE6702000078000FCA81@prv-mh.provo.novell.com> CC: , From: David Vrabel Message-ID: <577FB03F.9080108@citrix.com> Date: Fri, 8 Jul 2016 14:53:03 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Icedove/38.5.0 MIME-Version: 1.0 In-Reply-To: <577FBE6702000078000FCA81@prv-mh.provo.novell.com> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit X-DLP: MIA1 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 08/07/16 13:53, Jan Beulich wrote: >>>> On 08.07.16 at 14:29, 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