From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paul Mackerras Subject: Re: [PATCH v2 03/16] KVM: PPC: Book3S HV: XIVE: introduce a new capability KVM_CAP_PPC_IRQ_XIVE Date: Mon, 25 Feb 2019 15:35:51 +1100 Message-ID: <20190225043551.GA20501@blackberry> References: <20190222112840.25000-1-clg@kaod.org> <20190222112840.25000-4-clg@kaod.org> Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: 8bit Cc: kvm@vger.kernel.org, kvm-ppc@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, David Gibson To: =?iso-8859-1?Q?C=E9dric?= Le Goater Return-path: Content-Disposition: inline In-Reply-To: <20190222112840.25000-4-clg@kaod.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: linuxppc-dev-bounces+glppe-linuxppc-embedded-2=m.gmane.org@lists.ozlabs.org Sender: "Linuxppc-dev" List-Id: kvm.vger.kernel.org On Fri, Feb 22, 2019 at 12:28:27PM +0100, Cédric Le Goater wrote: > The user interface exposes a new capability to let QEMU connect the > vCPU to the XIVE KVM device if required. The capability is only > advertised on a PowerNV Hypervisor as support for nested guests > (pseries KVM Hypervisor) is not yet available. If a bisection happened to land on this commit, we would have KVM saying it had the ability to support guests using XIVE natively, but it wouldn't actually work since we don't have all the code that is in the following patches. Thus, in order to avoid breaking bisection, you should either add the capability now but have it always return false until the rest of the code is in place, or else defer the addition of the capability until the end of the patch series. > diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c > index 8c69af10f91d..a38a643a24dd 100644 > --- a/arch/powerpc/kvm/powerpc.c > +++ b/arch/powerpc/kvm/powerpc.c > @@ -570,6 +570,12 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext) > case KVM_CAP_PPC_GET_CPU_CHAR: > r = 1; > break; > +#ifdef CONFIG_KVM_XIVE > + case KVM_CAP_PPC_IRQ_XIVE: > + /* only for PowerNV */ > + r = !!cpu_has_feature(CPU_FTR_HVMODE); Shouldn't this be r = xive_enabled() && !!cpu_has_feature(CPU_FTR_HVMODE) (or alternatively r = xics_on_xive(), though that would be confusing to the reader)? As it stands this would report true on POWER8, unless I'm missing something. Paul. From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paul Mackerras Date: Mon, 25 Feb 2019 04:35:51 +0000 Subject: Re: [PATCH v2 03/16] KVM: PPC: Book3S HV: XIVE: introduce a new capability KVM_CAP_PPC_IRQ_XIVE Message-Id: <20190225043551.GA20501@blackberry> List-Id: References: <20190222112840.25000-1-clg@kaod.org> <20190222112840.25000-4-clg@kaod.org> In-Reply-To: <20190222112840.25000-4-clg@kaod.org> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable To: =?iso-8859-1?Q?C=E9dric?= Le Goater Cc: kvm@vger.kernel.org, kvm-ppc@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, David Gibson On Fri, Feb 22, 2019 at 12:28:27PM +0100, C=E9dric Le Goater wrote: > The user interface exposes a new capability to let QEMU connect the > vCPU to the XIVE KVM device if required. The capability is only > advertised on a PowerNV Hypervisor as support for nested guests > (pseries KVM Hypervisor) is not yet available. If a bisection happened to land on this commit, we would have KVM saying it had the ability to support guests using XIVE natively, but it wouldn't actually work since we don't have all the code that is in the following patches. Thus, in order to avoid breaking bisection, you should either add the capability now but have it always return false until the rest of the code is in place, or else defer the addition of the capability until the end of the patch series. > diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c > index 8c69af10f91d..a38a643a24dd 100644 > --- a/arch/powerpc/kvm/powerpc.c > +++ b/arch/powerpc/kvm/powerpc.c > @@ -570,6 +570,12 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, lo= ng ext) > case KVM_CAP_PPC_GET_CPU_CHAR: > r =3D 1; > break; > +#ifdef CONFIG_KVM_XIVE > + case KVM_CAP_PPC_IRQ_XIVE: > + /* only for PowerNV */ > + r =3D !!cpu_has_feature(CPU_FTR_HVMODE); Shouldn't this be r =3D xive_enabled() && !!cpu_has_feature(CPU_FTR_HVMODE) (or alternatively r =3D xics_on_xive(), though that would be confusing to the reader)? As it stands this would report true on POWER8, unless I'm missing something. Paul.