From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50749) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aalZw-0006Qk-QF for qemu-devel@nongnu.org; Tue, 01 Mar 2016 09:47:13 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aalZr-0001o6-Q2 for qemu-devel@nongnu.org; Tue, 01 Mar 2016 09:47:12 -0500 Received: from mga14.intel.com ([192.55.52.115]:45385) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aalZr-0001nt-Jt for qemu-devel@nongnu.org; Tue, 01 Mar 2016 09:47:07 -0500 References: <1456413312-24063-1-git-send-email-tianyu.lan@intel.com> <20160226195400.GF3313@thinpad.lan.raisama.net> <56D3B344.2020100@intel.com> <20160301140038.GA3632@thinpad.lan.raisama.net> From: "Lan, Tianyu" Message-ID: <56D5AB66.6010308@intel.com> Date: Tue, 1 Mar 2016 22:47:02 +0800 MIME-Version: 1.0 In-Reply-To: <20160301140038.GA3632@thinpad.lan.raisama.net> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH] Qemu/KVM: Remove x2apic feature from CPU model when kernel_irqchip is off List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Habkost Cc: kvm@vger.kernel.org, qemu-devel@nongnu.org, jan.kiszka@web.de, pbonzini@redhat.com, afaerber@suse.de, rth@twiddle.net On 3/1/2016 10:00 PM, Eduardo Habkost wrote: > On Mon, Feb 29, 2016 at 10:56:04AM +0800, Lan Tianyu wrote: >> On 2016年02月27日 03:54, Eduardo Habkost wrote: >>> On Thu, Feb 25, 2016 at 11:15:12PM +0800, Lan Tianyu wrote: >>>> x2apic feature is in the kvm_default_props and automatically added to all >>>> CPU models when KVM is enabled. But userspace devices don't support x2apic >>>> which can't be enabled without the in-kernel irqchip. It will trigger >>>> warning of "host doesn't support requested feature: CPUID.01H:ECX.x2apic >>>> [bit 21]" when kernel_irqchip is off. This patch is to fix it via removing >>>> x2apic feature when kernel_irqchip is off. >>>> >>>> Signed-off-by: Lan Tianyu >>>> --- >>>> target-i386/cpu.c | 4 ++++ >>>> 1 file changed, 4 insertions(+) >>>> >>>> diff --git a/target-i386/cpu.c b/target-i386/cpu.c >>>> index c78f824..298fb62 100644 >>>> --- a/target-i386/cpu.c >>>> +++ b/target-i386/cpu.c >>>> @@ -2125,6 +2125,10 @@ static void x86_cpu_load_def(X86CPU *cpu, X86CPUDefinition *def, Error **errp) >>>> >>>> /* Special cases not set in the X86CPUDefinition structs: */ >>>> if (kvm_enabled()) { >>>> + if (!kvm_irqchip_in_kernel()) { >>>> + x86_cpu_change_kvm_default("x2apic", "off"); >>> >>> This should be NULL instead of "off". >> >> I tried "NULL" before. But some cpus modules(E,G SandyBridge, >> IvyBridge, haswell) already have x2apic feature in their default >> features of struct X86CPUDefinition, passing "NULL" is not to add x2apic >> feature to the cpu module and will not help to remove x2apic feature for >> these cpu modules. So I changed "NULL" to "off". > > In this case, I suggest we remove x2apic from these CPU models to > avoid confusion, as the presence of the flag in the table makes > no difference at all (this can be done in a separate patch). > > If we do that, NULL and "off" would have the same results, but > NULL is clearer, IMO. NULL simply disables the KVM-specific hack > (so we don't touch the property anymore), but "off" adds a new > TCG-specific hack. I will submit that as a follow-up patch. Yes, that sounds reasonable. Thanks for your review. > > Reviewed-by: Eduardo Habkost > >> >>> Otherwise, the warning will >>> be disabled if using "-cpu ...,+x2apic". >>> >> >> kvm_arch_get_supported_cpuid() always returns no x2apic support when >> kernel_irqchip is off and so it still triggers warning with "-cpu >> ...,+x2apic". >> >> #qemu-system-x86_64 -cpu qemu64,+x2apic -machine kernel-irqchip=off >> warning: TCG doesn't support requested feature: CPUID.01H:ECX.x2apic >> [bit 21] > > You are right. The +x2apic flag is applied after > x86_cpu_load_def() runs. My mistake. >