* [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).