Re: [Qemu-devel] [PATCH] Qemu/KVM: Remove x2apic feature from CPU model when kernel_irqchip is off

[Lan Tianyu <tianyu.lan@intel.com>]

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