* [Qemu-devel] [PATCH] Qemu/KVM: Remove x2apic feature from CPU model when kernel_irqchip is off @ 2016-02-25 15:15 Lan Tianyu 2016-02-26 9:56 ` Paolo Bonzini 2016-02-26 19:54 ` Eduardo Habkost 0 siblings, 2 replies; 6+ messages in thread From: Lan Tianyu @ 2016-02-25 15:15 UTC (permalink / raw) To: afaerber, pbonzini, rth, ehabkost, jan.kiszka; +Cc: Lan Tianyu, qemu-devel, kvm 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 <tianyu.lan@intel.com> --- 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"); + } + x86_cpu_apply_props(cpu, kvm_default_props); } -- 1.9.3 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] Qemu/KVM: Remove x2apic feature from CPU model when kernel_irqchip is off 2016-02-25 15:15 [Qemu-devel] [PATCH] Qemu/KVM: Remove x2apic feature from CPU model when kernel_irqchip is off Lan Tianyu @ 2016-02-26 9:56 ` Paolo Bonzini 2016-02-26 19:54 ` Eduardo Habkost 1 sibling, 0 replies; 6+ messages in thread From: Paolo Bonzini @ 2016-02-26 9:56 UTC (permalink / raw) To: Lan Tianyu, afaerber, rth, ehabkost, jan.kiszka; +Cc: qemu-devel, kvm On 25/02/2016 16:15, 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 <tianyu.lan@intel.com> > --- > 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"); > + } > + > x86_cpu_apply_props(cpu, kvm_default_props); > } > > Acked-by: Paolo Bonzini <pbonzini@redhat.com> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] Qemu/KVM: Remove x2apic feature from CPU model when kernel_irqchip is off 2016-02-25 15:15 [Qemu-devel] [PATCH] Qemu/KVM: Remove x2apic feature from CPU model when kernel_irqchip is off Lan Tianyu 2016-02-26 9:56 ` Paolo Bonzini @ 2016-02-26 19:54 ` Eduardo Habkost 2016-02-29 2:56 ` Lan Tianyu 1 sibling, 1 reply; 6+ messages in thread From: Eduardo Habkost @ 2016-02-26 19:54 UTC (permalink / raw) To: Lan Tianyu; +Cc: kvm, qemu-devel, jan.kiszka, pbonzini, afaerber, rth 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 <tianyu.lan@intel.com> > --- > 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". Otherwise, the warning will be disabled if using "-cpu ...,+x2apic". > + } > + This function runs multiple times (once for each VCPU being created). Should be harmless, though. Eventually we could move the whole kvm_default_props logic outside cpu.c, to a KVM-x86 accel class or to common PC initialization code. > x86_cpu_apply_props(cpu, kvm_default_props); > } > > -- > 1.9.3 > -- Eduardo ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] Qemu/KVM: Remove x2apic feature from CPU model when kernel_irqchip is off 2016-02-26 19:54 ` Eduardo Habkost @ 2016-02-29 2:56 ` Lan Tianyu 2016-03-01 14:00 ` Eduardo Habkost 0 siblings, 1 reply; 6+ messages in thread From: Lan Tianyu @ 2016-02-29 2:56 UTC (permalink / raw) To: Eduardo Habkost; +Cc: kvm, qemu-devel, jan.kiszka, pbonzini, afaerber, rth 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 <tianyu.lan@intel.com> >> --- >> 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". > 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] >> + } >> + > > This function runs multiple times (once for each VCPU being > created). Should be harmless, though. Eventually we could move > the whole kvm_default_props logic outside cpu.c, to a KVM-x86 > accel class or to common PC initialization code. Good suggestion and we can try that later. > > >> x86_cpu_apply_props(cpu, kvm_default_props); >> } >> >> -- >> 1.9.3 >> > -- Best regards Tianyu Lan ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] Qemu/KVM: Remove x2apic feature from CPU model when kernel_irqchip is off 2016-02-29 2:56 ` Lan Tianyu @ 2016-03-01 14:00 ` Eduardo Habkost 2016-03-01 14:47 ` Lan, Tianyu 0 siblings, 1 reply; 6+ messages in thread From: Eduardo Habkost @ 2016-03-01 14:00 UTC (permalink / raw) To: Lan Tianyu; +Cc: kvm, qemu-devel, jan.kiszka, pbonzini, afaerber, rth 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 <tianyu.lan@intel.com> > >> --- > >> 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. Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> > > > 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. -- Eduardo ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] Qemu/KVM: Remove x2apic feature from CPU model when kernel_irqchip is off 2016-03-01 14:00 ` Eduardo Habkost @ 2016-03-01 14:47 ` Lan, Tianyu 0 siblings, 0 replies; 6+ messages in thread From: Lan, Tianyu @ 2016-03-01 14:47 UTC (permalink / raw) To: Eduardo Habkost; +Cc: kvm, qemu-devel, jan.kiszka, pbonzini, afaerber, rth 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 <tianyu.lan@intel.com> >>>> --- >>>> 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 <ehabkost@redhat.com> > >> >>> 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. > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-03-01 14:47 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-02-25 15:15 [Qemu-devel] [PATCH] Qemu/KVM: Remove x2apic feature from CPU model when kernel_irqchip is off Lan Tianyu 2016-02-26 9:56 ` Paolo Bonzini 2016-02-26 19:54 ` Eduardo Habkost 2016-02-29 2:56 ` Lan Tianyu 2016-03-01 14:00 ` Eduardo Habkost 2016-03-01 14:47 ` Lan, Tianyu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).