From mboxrd@z Thu Jan 1 00:00:00 1970 From: Radim =?utf-8?B?S3LEjW3DocWZ?= Subject: Re: GET_SUPPORTED_CPUID and irqchip (was Re: kvm_pv_unhalt and kernel_irqchip=off) Date: Sat, 13 Aug 2016 14:43:03 +0200 Message-ID: <20160813124303.GA20960@potion> References: <20160810182704.GE5627@thinpad.lan.raisama.net> <20160810190412.GA8001@potion> <20160812183742.GM5627@thinpad.lan.raisama.net> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Marcelo Tosatti , Paolo Bonzini , peterx@redhat.com, Andrew Jones , kvm@vger.kernel.org To: Eduardo Habkost Return-path: Received: from mx1.redhat.com ([209.132.183.28]:60768 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752273AbcHMMna (ORCPT ); Sat, 13 Aug 2016 08:43:30 -0400 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 6E4473B3C0 for ; Sat, 13 Aug 2016 12:43:07 +0000 (UTC) Content-Disposition: inline In-Reply-To: <20160812183742.GM5627@thinpad.lan.raisama.net> Sender: kvm-owner@vger.kernel.org List-ID: 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?