Re: GET_SUPPORTED_CPUID and irqchip (was Re: kvm_pv_unhalt and kernel_irqchip=off)

["Radim Krčmář" <rkrcmar@redhat.com>]

2016-08-12 15:37-0300, Eduardo Habkost:
> Now I have another question: are features that require the
> in-kernel irqchip supposed to be present in GET_SUPPORTED_CPUID?

I don't think so.  Simply removing X2APIC and PV_UNHALT would disable
them on old userspaces, though, which would probably cause more bugs.

(The blunder doesn't seem to be bad enough for a new capability or
 interface and a deprecation protocol on these features.)

> We have examples of both cases in KVM:
> 
> * TSC_DEADLINE_TIMER is _not_ present in GET_SUPPORTED_CPUID,
>   and is reported through KVM_CAP_TSC_DEADLINE_TIMER.
> * X2APIC is present in GET_SUPPORTED_CPUID, but the bit
>   makes sense only if the in-kernel irqchip is used.
> * KVM_PV_UNHALT is present in GET_SUPPORTED_CPUID, but
>   requires the in-kernel irqchip to work.

Well ... no excuses for that.

> Should userspace expect more cases like x2apic and kvm_pv_unhalt
> in the future?

At least one userspace (QEMU) doesn't filter unknown features from
GET_SUPPORTED_CPUID, so KVM cannot plan to pass conditionally buggy
features.
KVM would need to define a new interface to handle these issues, so I
think that userspace can ignore unknown KVM bugs.

I would like to return -EINVAL from KVM_SET_CPUID2 if userspace
requested a new CPUID feature that cannot work in given situation.

Another way would be to disable buggy features in KVM_SET_CPUID2, which
would require userspace to call KVM_GET_CPUID2 afterwards to learn what
the guest is actually using.

I have patches that implement the latter for X2APIC and PV_UNHALT, but
I'm not sure if it's better than leaving the bug unfixed, because QEMU
doesn't use KVM_GET_CPUID2 and migration to older KVM would change
CPUID, which is a very subtle bug.
What do you think?