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

[Eduardo Habkost]

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 <address@hidden>
> >> ---
> >>  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 <address@hidden>

> 
> > 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