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? -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
