From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58798) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1axePA-0005Ei-IK for qemu-devel@nongnu.org; Tue, 03 May 2016 13:46:47 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1axeOy-0007xn-Ri for qemu-devel@nongnu.org; Tue, 03 May 2016 13:46:35 -0400 Received: from mga04.intel.com ([192.55.52.120]:36433) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1axeOy-0007rb-IZ for qemu-devel@nongnu.org; Tue, 03 May 2016 13:46:28 -0400 References: <1461744786-47643-1-git-send-email-guangrong.xiao@linux.intel.com> <20160428173443.GA24153@thinpad.lan.raisama.net> <57284774.7010401@linux.intel.com> <20160503161131.GN4457@thinpad.lan.raisama.net> From: Xiao Guangrong Message-ID: <5728E374.4020809@linux.intel.com> Date: Wed, 4 May 2016 01:44:20 +0800 MIME-Version: 1.0 In-Reply-To: <20160503161131.GN4457@thinpad.lan.raisama.net> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] XSAVES in GET_SUPPORTED_CPUID (was Re: [PATCH] target-i386: add Skylake-Client cpu mode) List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Habkost Cc: pbonzini@redhat.com, imammedo@redhat.com, kvm@vger.kernel.org, mst@redhat.com, gleb@kernel.org, mtosatti@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com, dan.j.williams@intel.com, rth@twiddle.net, Dave Hansen , "Yu, Yu-cheng" , "Wang, Yong Y" , "virt-intel-list@redhat.com" , "Kasten, Robert A" , "Lai, Paul C" , "Xiao, Guangrong" , "ruwang@redhat.com" , "Shankar, Ravi V" , "Yu, Fenghua" On 05/04/2016 12:11 AM, Eduardo Habkost wrote: > (CCing KVM mailing list) > > On Tue, May 03, 2016 at 02:38:44PM +0800, Xiao Guangrong wrote: >> On 04/29/2016 01:34 AM, Eduardo Habkost wrote: >>> On Wed, Apr 27, 2016 at 04:13:06PM +0800, Xiao Guangrong wrote: > [...] >>>> 1. As XSAVES is disabled in upstream linux kernel by commit e88221c50 >>>> (x86/fpu: Disable XSAVES* support for now), QEMU will complain about >>>> "warning: host doesn't support requested feature: CPUID.0DH:EAX.xsaves [bit 3]" >>> >>> I have been looking at the GET_SUPPORTED_CPUID code and I am not >>> sure if commit e88221c50 is supposed to be affecting >>> GET_SUPPORTED_CPUID, or not. It looks like it shouldn't, so I >>> don't know why QEMU is reporting xsaves as unsupported. >>> >>> For reference, GET_SUPPORTED_CPUID code for function == 0xd && >>> idx == 1 will run: >>> >>> unsigned f_xsaves = kvm_x86_ops->xsaves_supported() ? F(XSAVES) : 0; >>> const u32 kvm_cpuid_D_1_eax_x86_features = >>> F(XSAVEOPT) | F(XSAVEC) | F(XGETBV1) | f_xsaves; >>> /* [...] */ >>> do_cpuid_1_ent(&entry[i], function, idx); >>> entry[i].eax &= kvm_cpuid_D_1_eax_x86_features; >>> >>> do_cpuid_1_ent() just executes the CPUID instruction. >>> >>> kvm_x86_ops->xsaves_supported is: >>> >>> static bool vmx_xsaves_supported(void) >>> { >>> return vmcs_config.cpu_based_2nd_exec_ctrl & >>> SECONDARY_EXEC_XSAVES; >>> } >>> >>> Is GET_SUPPORTED_CPUID returning XSAVES as unsupported in the >>> system where you are running tests? >> >> No, it returns that XSAVES is supported. > > You mean it returns it as unsupported, right? Sorry for the typo. GET_SUPPORTED_CPUID returns XSAVES is not supported if the feature is cleared by host. > >> >> Actually F(SAVES) bit is cleared later, in __do_cpuid_ent(): >> 536 entry[i].eax &= kvm_cpuid_D_1_eax_x86_features; >> 537 cpuid_mask(&entry[i].eax, CPUID_D_1_EAX); >> >> The bits unsupported by boot_cpu_data.x86_capability[CPUID_D_1_EAX] will be cleared. > > Oh, I didn't notice the cpuid_mask() call. You're right. > >> >> During boot, the capability is adjusted by cpu_caps_cleared, in arch/x86/kernel/cpu/common.c: >> 971 /* Clear/Set all flags overriden by options, after probe */ >> 972 for (i = 0; i < NCAPINTS; i++) { >> 973 c->x86_capability[i] &= ~cpu_caps_cleared[i]; >> 974 c->x86_capability[i] |= cpu_caps_set[i]; >> 975 } >> >> Actually, setup_clear_cpu_cap() exactly acts on cpu_caps_cleared(): >> 112 #define setup_clear_cpu_cap(bit) do { \ >> 113 clear_cpu_cap(&boot_cpu_data, bit); \ >> 114 set_bit(bit, (unsigned long *)cpu_caps_cleared); \ >> 115 } while (0) >> >> This is the reason why setup_clear_cpu_cap(X86_FEATURE_XSAVES) introduced by commit e88221c50 >> caused XSAVES unreported by KVM. > > So, is this the right behavior, or KVM can support exposing > XSAVES to guests even if the cpu_cap bit is cleared? I don't know > if exposing XSAVES to KVM guests depend on reworking kernel xsave > code or not. > I think current behavior is right. KVM uses kernel's APIs to save/restore FPU context that may cause XSAVE not properly switched if XSAVES is used in VM but host does not see it.