From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.6 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_PASS,USER_AGENT_MUTT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2705CC10F13 for ; Mon, 15 Apr 2019 03:32:35 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 89E652084E for ; Mon, 15 Apr 2019 03:32:34 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=gibson.dropbear.id.au header.i=@gibson.dropbear.id.au header.b="h53EnMCu" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726283AbfDODca (ORCPT ); Sun, 14 Apr 2019 23:32:30 -0400 Received: from ozlabs.org ([203.11.71.1]:51161 "EHLO ozlabs.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725972AbfDODc3 (ORCPT ); Sun, 14 Apr 2019 23:32:29 -0400 Received: by ozlabs.org (Postfix, from userid 1007) id 44jDZy5yQ8z9s00; Mon, 15 Apr 2019 13:32:26 +1000 (AEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=gibson.dropbear.id.au; s=201602; t=1555299146; bh=THNzNU3bmUresf9OJQ6K/4bn3zwgCcWsCN8DAKaftGU=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=h53EnMCuxypZDjCkx7oki7RtWAsQfBAhHdTKpBGXdM6cr/3teKiVNmWJknZNO/vfZ srFOqrhevPm0btkdbHOoj3Lo6Apwfaeieeoigt0qVn3qHtjcWTA9xM5+ue21G4T1XR GM0K85M7lForBKiNbcNQMdIvqI6T3CMTTt1LG160= Date: Mon, 15 Apr 2019 13:26:28 +1000 From: David Gibson To: =?iso-8859-1?Q?C=E9dric?= Le Goater Cc: kvm-ppc@vger.kernel.org, Paul Mackerras , kvm@vger.kernel.org Subject: Re: [RFC PATCH v4.1 16/17] KVM: PPC: Book3S HV: XIVE: introduce a xive_devices array under the VM Message-ID: <20190415032628.GB32705@umbus.fritz.box> References: <20190320083751.27001-1-clg@kaod.org> <20190409141347.3029-1-clg@kaod.org> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="7ZAtKRhVyVSsbBD2" Content-Disposition: inline In-Reply-To: <20190409141347.3029-1-clg@kaod.org> User-Agent: Mutt/1.11.3 (2019-02-01) Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org --7ZAtKRhVyVSsbBD2 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Apr 09, 2019 at 04:13:46PM +0200, C=E9dric Le Goater wrote: > On P9 sPAPR guests, the interrupt mode (XICS legacy or XIVE native) is > determine at CAS time and the chosen mode is activated after a machine > reset. To be able to switch from one mode to another, subsequent > patches will introduce the capability to destroy the KVM device > without destroying the VM. >=20 > This is not considered as a safe operation as the vCPUs are still > running and could be referencing the KVM device through their > presenters. >=20 > To protect the system from any breakage, the kvmppc_xive objects > representing both KVM devices are now stored in an array under the > VM. Allocation is performed on first usage and memory is freed only > when the VM exits. >=20 > Signed-off-by: C=E9dric Le Goater Reviewed-by: David Gibson Although... > --- > arch/powerpc/include/asm/kvm_host.h | 1 + > arch/powerpc/kvm/book3s_xive.h | 1 + > arch/powerpc/kvm/book3s_xive.c | 23 +++++++++++++++++++++-- > arch/powerpc/kvm/book3s_xive_native.c | 9 +++++++-- > arch/powerpc/kvm/powerpc.c | 6 ++++++ > 5 files changed, 36 insertions(+), 4 deletions(-) >=20 > diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/a= sm/kvm_host.h > index 9cc6abdce1b9..ed059c95e56a 100644 > --- a/arch/powerpc/include/asm/kvm_host.h > +++ b/arch/powerpc/include/asm/kvm_host.h > @@ -314,6 +314,7 @@ struct kvm_arch { > #ifdef CONFIG_KVM_XICS > struct kvmppc_xics *xics; > struct kvmppc_xive *xive; > + struct kvmppc_xive *xive_devices[2]; =2E.. as noted on the previous revision, I feel this use of a bool-indexed array instead of separate fields makes things more confusing than necessary, for a negligible reduction in code size. > struct kvmppc_passthru_irqmap *pimap; > #endif > struct kvmppc_ops *kvm_ops; > diff --git a/arch/powerpc/kvm/book3s_xive.h b/arch/powerpc/kvm/book3s_xiv= e.h > index e011622dc038..426146332984 100644 > --- a/arch/powerpc/kvm/book3s_xive.h > +++ b/arch/powerpc/kvm/book3s_xive.h > @@ -283,6 +283,7 @@ void kvmppc_xive_free_sources(struct kvmppc_xive_src_= block *sb); > int kvmppc_xive_select_target(struct kvm *kvm, u32 *server, u8 prio); > int kvmppc_xive_attach_escalation(struct kvm_vcpu *vcpu, u8 prio, > bool single_escalation); > +struct kvmppc_xive *kvmppc_xive_get_device(struct kvm *kvm, u32 type); > =20 > #endif /* CONFIG_KVM_XICS */ > #endif /* _KVM_PPC_BOOK3S_XICS_H */ > diff --git a/arch/powerpc/kvm/book3s_xive.c b/arch/powerpc/kvm/book3s_xiv= e.c > index 480a3fc6b9fd..4d4e1730de84 100644 > --- a/arch/powerpc/kvm/book3s_xive.c > +++ b/arch/powerpc/kvm/book3s_xive.c > @@ -1846,11 +1846,30 @@ static void kvmppc_xive_free(struct kvm_device *d= ev) > if (xive->vp_base !=3D XIVE_INVALID_VP) > xive_native_free_vp_block(xive->vp_base); > =20 > + /* > + * A reference of the kvmppc_xive pointer is now kept under > + * the xive_devices[] array of the machine for reuse. It is > + * freed when the VM is destroyed. > + */ > =20 > - kfree(xive); > kfree(dev); > } > =20 > +struct kvmppc_xive *kvmppc_xive_get_device(struct kvm *kvm, u32 type) > +{ > + struct kvmppc_xive *xive; > + bool xive_native_index =3D type =3D=3D KVM_DEV_TYPE_XIVE; > + > + xive =3D kvm->arch.xive_devices[xive_native_index]; > + > + if (!xive) { > + xive =3D kzalloc(sizeof(*xive), GFP_KERNEL); > + kvm->arch.xive_devices[xive_native_index] =3D xive; > + } > + > + return xive; > +} > + > static int kvmppc_xive_create(struct kvm_device *dev, u32 type) > { > struct kvmppc_xive *xive; > @@ -1859,7 +1878,7 @@ static int kvmppc_xive_create(struct kvm_device *de= v, u32 type) > =20 > pr_devel("Creating xive for partition\n"); > =20 > - xive =3D kzalloc(sizeof(*xive), GFP_KERNEL); > + xive =3D kvmppc_xive_get_device(kvm, type); > if (!xive) > return -ENOMEM; > =20 > diff --git a/arch/powerpc/kvm/book3s_xive_native.c b/arch/powerpc/kvm/boo= k3s_xive_native.c > index 62648f833adf..092db0efe628 100644 > --- a/arch/powerpc/kvm/book3s_xive_native.c > +++ b/arch/powerpc/kvm/book3s_xive_native.c > @@ -987,7 +987,12 @@ static void kvmppc_xive_native_free(struct kvm_devic= e *dev) > if (xive->vp_base !=3D XIVE_INVALID_VP) > xive_native_free_vp_block(xive->vp_base); > =20 > - kfree(xive); > + /* > + * A reference of the kvmppc_xive pointer is now kept under > + * the xive_devices[] array of the machine for reuse. It is > + * freed when the VM is destroyed. > + */ > + > kfree(dev); > } > =20 > @@ -1002,7 +1007,7 @@ static int kvmppc_xive_native_create(struct kvm_dev= ice *dev, u32 type) > if (kvm->arch.xive) > return -EEXIST; > =20 > - xive =3D kzalloc(sizeof(*xive), GFP_KERNEL); > + xive =3D kvmppc_xive_get_device(kvm, type); > if (!xive) > return -ENOMEM; > =20 > diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c > index f54926c78320..d0914316ddc7 100644 > --- a/arch/powerpc/kvm/powerpc.c > +++ b/arch/powerpc/kvm/powerpc.c > @@ -501,6 +501,12 @@ void kvm_arch_destroy_vm(struct kvm *kvm) > =20 > mutex_unlock(&kvm->lock); > =20 > + for (i =3D 0; i < ARRAY_SIZE(kvm->arch.xive_devices); i++) { > + struct kvmppc_xive *xive =3D kvm->arch.xive_devices[i]; > + if (xive) > + kfree(xive); > + } > + > /* drop the module reference */ > module_put(kvm->arch.kvm_ops->owner); > } --=20 David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson --7ZAtKRhVyVSsbBD2 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEdfRlhq5hpmzETofcbDjKyiDZs5IFAlyz+eMACgkQbDjKyiDZ s5J6sA//YUYe6/uoOJAcjLuaOCOJQnR/T2D02ieor/ojtuSzH7aNOOxIyzIuKMeS MDebuHqOLzllFPwMxetfi1C2rxp6mv8QJ7TtXf1JEoq9Rbdf2u1zMvJ6yl7Kazt2 PPNuTF5dpHHcfy15i0o9fW/h8DzXQB7Y6I4+HEw1fKtMrEtrsH2OZkTvGQq5u7Vc AKXNz1HfLCOOKVmhIrvuI+jNkslCtAV7vaEDocfROi3O/OjLZ2H0bxlWldbbuL4D g9DPoCLP5BeRzrAvSUvVxrUTxEuDBpObDvoPyD6VqglZVXEkJwtxcGgLUq2OeLOf 63Ov3qozn20ROBtWMrWWaiPcZn8oUYgeztg54NZu5Aczd3bGEHDYgMoXJ8AoOXCW tgPqH7GAkvrIjd+d5hDKW/uS8T2AqmHs3c50CNx83WiSOS2IMN+OROjyGgt/VrP9 w7mtHcxmHP7YtrISgAW4HJFERLjl4oUxgYLUHgVSzMGoJBZSiw3YNBGBTZGwrK4z wZRAeBqtEhZMf35jWirSnEMNloxC+OPRGdPyVX3M+LQqoPenfxKtFrExYVjBcH+8 WlovFEaabJ0ym+98PCqpE2svmxGTrqz8M8cCtoi3hoIG5yzI/VELvuF9Zkgwhz6w HsTDhwcy3vbbbo3dJCVqyGP43BxSsSqp1B02og6bH8xo8Y69HqU= =EbiQ -----END PGP SIGNATURE----- --7ZAtKRhVyVSsbBD2-- From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Gibson Date: Mon, 15 Apr 2019 03:26:28 +0000 Subject: Re: [RFC PATCH v4.1 16/17] KVM: PPC: Book3S HV: XIVE: introduce a xive_devices array under the VM Message-Id: <20190415032628.GB32705@umbus.fritz.box> MIME-Version: 1 Content-Type: multipart/mixed; boundary="7ZAtKRhVyVSsbBD2" List-Id: References: <20190320083751.27001-1-clg@kaod.org> <20190409141347.3029-1-clg@kaod.org> In-Reply-To: <20190409141347.3029-1-clg@kaod.org> To: =?iso-8859-1?Q?C=E9dric?= Le Goater Cc: kvm-ppc@vger.kernel.org, Paul Mackerras , kvm@vger.kernel.org --7ZAtKRhVyVSsbBD2 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Apr 09, 2019 at 04:13:46PM +0200, C=E9dric Le Goater wrote: > On P9 sPAPR guests, the interrupt mode (XICS legacy or XIVE native) is > determine at CAS time and the chosen mode is activated after a machine > reset. To be able to switch from one mode to another, subsequent > patches will introduce the capability to destroy the KVM device > without destroying the VM. >=20 > This is not considered as a safe operation as the vCPUs are still > running and could be referencing the KVM device through their > presenters. >=20 > To protect the system from any breakage, the kvmppc_xive objects > representing both KVM devices are now stored in an array under the > VM. Allocation is performed on first usage and memory is freed only > when the VM exits. >=20 > Signed-off-by: C=E9dric Le Goater Reviewed-by: David Gibson Although... > --- > arch/powerpc/include/asm/kvm_host.h | 1 + > arch/powerpc/kvm/book3s_xive.h | 1 + > arch/powerpc/kvm/book3s_xive.c | 23 +++++++++++++++++++++-- > arch/powerpc/kvm/book3s_xive_native.c | 9 +++++++-- > arch/powerpc/kvm/powerpc.c | 6 ++++++ > 5 files changed, 36 insertions(+), 4 deletions(-) >=20 > diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/a= sm/kvm_host.h > index 9cc6abdce1b9..ed059c95e56a 100644 > --- a/arch/powerpc/include/asm/kvm_host.h > +++ b/arch/powerpc/include/asm/kvm_host.h > @@ -314,6 +314,7 @@ struct kvm_arch { > #ifdef CONFIG_KVM_XICS > struct kvmppc_xics *xics; > struct kvmppc_xive *xive; > + struct kvmppc_xive *xive_devices[2]; =2E.. as noted on the previous revision, I feel this use of a bool-indexed array instead of separate fields makes things more confusing than necessary, for a negligible reduction in code size. > struct kvmppc_passthru_irqmap *pimap; > #endif > struct kvmppc_ops *kvm_ops; > diff --git a/arch/powerpc/kvm/book3s_xive.h b/arch/powerpc/kvm/book3s_xiv= e.h > index e011622dc038..426146332984 100644 > --- a/arch/powerpc/kvm/book3s_xive.h > +++ b/arch/powerpc/kvm/book3s_xive.h > @@ -283,6 +283,7 @@ void kvmppc_xive_free_sources(struct kvmppc_xive_src_= block *sb); > int kvmppc_xive_select_target(struct kvm *kvm, u32 *server, u8 prio); > int kvmppc_xive_attach_escalation(struct kvm_vcpu *vcpu, u8 prio, > bool single_escalation); > +struct kvmppc_xive *kvmppc_xive_get_device(struct kvm *kvm, u32 type); > =20 > #endif /* CONFIG_KVM_XICS */ > #endif /* _KVM_PPC_BOOK3S_XICS_H */ > diff --git a/arch/powerpc/kvm/book3s_xive.c b/arch/powerpc/kvm/book3s_xiv= e.c > index 480a3fc6b9fd..4d4e1730de84 100644 > --- a/arch/powerpc/kvm/book3s_xive.c > +++ b/arch/powerpc/kvm/book3s_xive.c > @@ -1846,11 +1846,30 @@ static void kvmppc_xive_free(struct kvm_device *d= ev) > if (xive->vp_base !=3D XIVE_INVALID_VP) > xive_native_free_vp_block(xive->vp_base); > =20 > + /* > + * A reference of the kvmppc_xive pointer is now kept under > + * the xive_devices[] array of the machine for reuse. It is > + * freed when the VM is destroyed. > + */ > =20 > - kfree(xive); > kfree(dev); > } > =20 > +struct kvmppc_xive *kvmppc_xive_get_device(struct kvm *kvm, u32 type) > +{ > + struct kvmppc_xive *xive; > + bool xive_native_index =3D type =3D=3D KVM_DEV_TYPE_XIVE; > + > + xive =3D kvm->arch.xive_devices[xive_native_index]; > + > + if (!xive) { > + xive =3D kzalloc(sizeof(*xive), GFP_KERNEL); > + kvm->arch.xive_devices[xive_native_index] =3D xive; > + } > + > + return xive; > +} > + > static int kvmppc_xive_create(struct kvm_device *dev, u32 type) > { > struct kvmppc_xive *xive; > @@ -1859,7 +1878,7 @@ static int kvmppc_xive_create(struct kvm_device *de= v, u32 type) > =20 > pr_devel("Creating xive for partition\n"); > =20 > - xive =3D kzalloc(sizeof(*xive), GFP_KERNEL); > + xive =3D kvmppc_xive_get_device(kvm, type); > if (!xive) > return -ENOMEM; > =20 > diff --git a/arch/powerpc/kvm/book3s_xive_native.c b/arch/powerpc/kvm/boo= k3s_xive_native.c > index 62648f833adf..092db0efe628 100644 > --- a/arch/powerpc/kvm/book3s_xive_native.c > +++ b/arch/powerpc/kvm/book3s_xive_native.c > @@ -987,7 +987,12 @@ static void kvmppc_xive_native_free(struct kvm_devic= e *dev) > if (xive->vp_base !=3D XIVE_INVALID_VP) > xive_native_free_vp_block(xive->vp_base); > =20 > - kfree(xive); > + /* > + * A reference of the kvmppc_xive pointer is now kept under > + * the xive_devices[] array of the machine for reuse. It is > + * freed when the VM is destroyed. > + */ > + > kfree(dev); > } > =20 > @@ -1002,7 +1007,7 @@ static int kvmppc_xive_native_create(struct kvm_dev= ice *dev, u32 type) > if (kvm->arch.xive) > return -EEXIST; > =20 > - xive =3D kzalloc(sizeof(*xive), GFP_KERNEL); > + xive =3D kvmppc_xive_get_device(kvm, type); > if (!xive) > return -ENOMEM; > =20 > diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c > index f54926c78320..d0914316ddc7 100644 > --- a/arch/powerpc/kvm/powerpc.c > +++ b/arch/powerpc/kvm/powerpc.c > @@ -501,6 +501,12 @@ void kvm_arch_destroy_vm(struct kvm *kvm) > =20 > mutex_unlock(&kvm->lock); > =20 > + for (i =3D 0; i < ARRAY_SIZE(kvm->arch.xive_devices); i++) { > + struct kvmppc_xive *xive =3D kvm->arch.xive_devices[i]; > + if (xive) > + kfree(xive); > + } > + > /* drop the module reference */ > module_put(kvm->arch.kvm_ops->owner); > } --=20 David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson --7ZAtKRhVyVSsbBD2 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEdfRlhq5hpmzETofcbDjKyiDZs5IFAlyz+eMACgkQbDjKyiDZ s5J6sA//YUYe6/uoOJAcjLuaOCOJQnR/T2D02ieor/ojtuSzH7aNOOxIyzIuKMeS MDebuHqOLzllFPwMxetfi1C2rxp6mv8QJ7TtXf1JEoq9Rbdf2u1zMvJ6yl7Kazt2 PPNuTF5dpHHcfy15i0o9fW/h8DzXQB7Y6I4+HEw1fKtMrEtrsH2OZkTvGQq5u7Vc AKXNz1HfLCOOKVmhIrvuI+jNkslCtAV7vaEDocfROi3O/OjLZ2H0bxlWldbbuL4D g9DPoCLP5BeRzrAvSUvVxrUTxEuDBpObDvoPyD6VqglZVXEkJwtxcGgLUq2OeLOf 63Ov3qozn20ROBtWMrWWaiPcZn8oUYgeztg54NZu5Aczd3bGEHDYgMoXJ8AoOXCW tgPqH7GAkvrIjd+d5hDKW/uS8T2AqmHs3c50CNx83WiSOS2IMN+OROjyGgt/VrP9 w7mtHcxmHP7YtrISgAW4HJFERLjl4oUxgYLUHgVSzMGoJBZSiw3YNBGBTZGwrK4z wZRAeBqtEhZMf35jWirSnEMNloxC+OPRGdPyVX3M+LQqoPenfxKtFrExYVjBcH+8 WlovFEaabJ0ym+98PCqpE2svmxGTrqz8M8cCtoi3hoIG5yzI/VELvuF9Zkgwhz6w HsTDhwcy3vbbbo3dJCVqyGP43BxSsSqp1B02og6bH8xo8Y69HqU= =EbiQ -----END PGP SIGNATURE----- --7ZAtKRhVyVSsbBD2--