Re: [PATCH] KVM: lapic: stop advertising DIRECTED_EOI when in-kernel IOAPIC is in use

[Nikita Leshenko]

The patch looks correct, however Iâm confused about why you consider
this to be a bug in the guest rather than a bug in KVM.

The spec for x2APIC states:
"The support for Directed EOI capability can be detected by means of
bit 24 in the Local APIC Version Registerâ (Intelâs x2APIC spec, 2.5.1
Directed EOI)
It seems to me that Windows did the right thing by testing for the
presence of directed EOI feature rather than implying it exists by
testing a version number. KVM did the wrong thing by advertising a
feature it doesnât support.

Therefore I think that you should change the comment to something like
âKVMâs in-kernel IOAPIC doesnât support Directed EOI register, so donât
advertise this capability in the LAPIC Version Register.â instead of
talking about buggy guests, as it may confuse future readers of this
code.

Thanks,
Nikita
> On 9 Feb 2018, at 15:01, Vitaly Kuznetsov <vkuznets@xxxxxxxxxx> wrote:
> 
> Devices which use level-triggered interrupts under Windows 2016 with
> Hyper-V role enabled don't work: Windows disables EOI broadcast in SPIV
> unconditionally. Our in-kernel IOAPIC implementation emulates an old IOAPIC
> version which has no EOI register so EOI never happens.
> 
> The issue was discovered and discussed a while ago:
> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.spinics.net_lists_kvm_msg148098.html&d=DwIBAg&c=RoP1YumCXCgaWHvlZYR8PZh8Bv7qIrMUB65eapI_JnE&r=JD7W0KpKqI3xo5AglC-aIVDRz_ysy5CrQRnZ9Jb7je0&m=GWIw1X7PvyWESZaIau591RwjCXYZTi6THVNSOEcdaxU&s=5QUI6ED5i6frC8BzcF_e7hp6Kd_OqAxkg0z73R-UIDI&e=
> 
> While this is a guest OS bug (it should check that IOAPIC has the required
> capabilities before disabling EOI broadcast) we can workaround it in KVM:
> advertising DIRECTED_EOI with in-kernel IOAPIC makes little sense anyway.
> 
> Signed-off-by: Vitaly Kuznetsov <vkuznets@xxxxxxxxxx>
> ---
> - Radim's suggestion was to disable DIRECTED_EOI unconditionally but I'm not
>  that radical :-) In theory, we may have multiple IOAPICs in userspace in
>  future and DIRECTED_EOI can be leveraged.
> ---
> arch/x86/kvm/lapic.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index 924ac8ce9d50..5339287fee63 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -321,8 +321,16 @@ void kvm_apic_set_version(struct kvm_vcpu *vcpu)
> 	if (!lapic_in_kernel(vcpu))
> 		return;
> 
> +	/*
> +	 * KVM emulates 82093AA datasheet (with in-kernel IOAPIC implementation)
> +	 * which doesn't have EOI register; Some buggy OSes (e.g. Windows with
> +	 * Hyper-V role) disable EOI broadcast in lapic not checking for IOAPIC
> +	 * version first and level-triggered interrupts never get EOIed in
> +	 * IOAPIC.
> +	 */
> 	feat = kvm_find_cpuid_entry(apic->vcpu, 0x1, 0);
> -	if (feat && (feat->ecx & (1 << (X86_FEATURE_X2APIC & 31))))
> +	if (feat && (feat->ecx & (1 << (X86_FEATURE_X2APIC & 31))) &&
> +	    !ioapic_in_kernel(vcpu->kvm))
> 		v |= APIC_LVR_DIRECTED_EOI;
> 	kvm_lapic_set_reg(apic, APIC_LVR, v);
> }
> -- 
> 2.14.3
>