From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Gibson Subject: Re: [PATCH v3 28/33] KVM: PPC: Book3S HV: Sanitise hv_regs on nested guest entry Date: Wed, 3 Oct 2018 16:07:36 +1000 Message-ID: <20181003060736.GU1886@umbus.fritz.box> References: <1538479892-14835-1-git-send-email-paulus@ozlabs.org> <1538479892-14835-29-git-send-email-paulus@ozlabs.org> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="GAoked8QSizNecZ5" Cc: linuxppc-dev@ozlabs.org, kvm-ppc@vger.kernel.org, kvm@vger.kernel.org To: Paul Mackerras Return-path: Content-Disposition: inline In-Reply-To: <1538479892-14835-29-git-send-email-paulus@ozlabs.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 --GAoked8QSizNecZ5 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Oct 02, 2018 at 09:31:27PM +1000, Paul Mackerras wrote: > From: Suraj Jitindar Singh >=20 > restore_hv_regs() is used to copy the hv_regs L1 wants to set to run the > nested (L2) guest into the vcpu structure. We need to sanitise these > values to ensure we don't let the L1 guest hypervisor do things we don't > want it to. >=20 > We don't let data address watchpoints or completed instruction address > breakpoints be set to match in hypervisor state. >=20 > We also don't let L1 enable features in the hypervisor facility status > and control register (HFSCR) for L2 which we have disabled for L1. That > is L2 will get the subset of features which the L0 hypervisor has > enabled for L1 and the features L1 wants to enable for L2. This could > mean we give L1 a hypervisor facility unavailable interrupt for a > facility it thinks it has enabled, however it shouldn't have enabled a > facility it itself doesn't have for the L2 guest. >=20 > We sanitise the registers when copying in the L2 hv_regs. We don't need > to sanitise when copying back the L1 hv_regs since these shouldn't be > able to contain invalid values as they're just what was copied out. >=20 > Signed-off-by: Suraj Jitindar Singh > Signed-off-by: Paul Mackerras Reviewed-by: David Gibson > --- > arch/powerpc/include/asm/reg.h | 1 + > arch/powerpc/kvm/book3s_hv_nested.c | 17 +++++++++++++++++ > 2 files changed, 18 insertions(+) >=20 > diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/re= g.h > index 9c42abf..47489f6 100644 > --- a/arch/powerpc/include/asm/reg.h > +++ b/arch/powerpc/include/asm/reg.h > @@ -415,6 +415,7 @@ > #define HFSCR_DSCR __MASK(FSCR_DSCR_LG) > #define HFSCR_VECVSX __MASK(FSCR_VECVSX_LG) > #define HFSCR_FP __MASK(FSCR_FP_LG) > +#define HFSCR_INTR_CAUSE (ASM_CONST(0xFF) << 56) /* interrupt cause */ > #define SPRN_TAR 0x32f /* Target Address Register */ > #define SPRN_LPCR 0x13E /* LPAR Control Register */ > #define LPCR_VPM0 ASM_CONST(0x8000000000000000) > diff --git a/arch/powerpc/kvm/book3s_hv_nested.c b/arch/powerpc/kvm/book3= s_hv_nested.c > index 7656cb3..7b1088a 100644 > --- a/arch/powerpc/kvm/book3s_hv_nested.c > +++ b/arch/powerpc/kvm/book3s_hv_nested.c > @@ -86,6 +86,22 @@ static void save_hv_return_state(struct kvm_vcpu *vcpu= , int trap, > } > } > =20 > +static void sanitise_hv_regs(struct kvm_vcpu *vcpu, struct hv_guest_stat= e *hr) > +{ > + /* > + * Don't let L1 enable features for L2 which we've disabled for L1, > + * but preserve the interrupt cause field. > + */ > + hr->hfscr &=3D (HFSCR_INTR_CAUSE | vcpu->arch.hfscr); > + > + /* Don't let data address watchpoint match in hypervisor state */ > + hr->dawrx0 &=3D ~DAWRX_HYP; > + > + /* Don't let completed instruction address breakpt match in HV state */ > + if ((hr->ciabr & CIABR_PRIV) =3D=3D CIABR_PRIV_HYPER) > + hr->ciabr &=3D ~CIABR_PRIV; > +} > + > static void restore_hv_regs(struct kvm_vcpu *vcpu, struct hv_guest_state= *hr) > { > struct kvmppc_vcore *vc =3D vcpu->arch.vcore; > @@ -198,6 +214,7 @@ long kvmhv_enter_nested_guest(struct kvm_vcpu *vcpu) > mask =3D LPCR_DPFD | LPCR_ILE | LPCR_TC | LPCR_AIL | LPCR_LD | > LPCR_LPES | LPCR_MER; > lpcr =3D (vc->lpcr & ~mask) | (l2_hv.lpcr & mask); > + sanitise_hv_regs(vcpu, &l2_hv); > restore_hv_regs(vcpu, &l2_hv); > =20 > vcpu->arch.ret =3D RESUME_GUEST; --=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 --GAoked8QSizNecZ5 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEdfRlhq5hpmzETofcbDjKyiDZs5IFAlu0XKMACgkQbDjKyiDZ s5LWFA//TsPiGp8ItmLNehs/iUTtvZE9Es09k61dQr7NkGqnIZLS7gnkD73oQgnf S0fMkUH78wmFVrdJWkPG521DKHVhyMUJnGcrWyZ8RlKKjRuQqxAzg/NOPoxiaAwh OW61oG62gKIbLmaJba0bkYB4WcHxTo+TgS6XTSR2/ZuqmqN68AoPw/CNBVXNbSmn udw6FwbcwqP6rhtVzk+eW90bgFo1/qD8FXHKJN9ObCDLo2j/s34gUqeYYbxYhbkm uCNBrQbh7Qjv1JDEmhsvoDUH3nY52AeFeNT7jq8vl0FgwPL6kmXcvtGlUAztnNnT 4s9atAxSQ4bYQmn+MkJceCx6g7d62ginikn8A9PjfnuT4YQ6A+5KZdcdu6VI6xEy HHITUFa3x9jXXdRSby6A6ljYfUP9aYZg4MwiV9T/IdlHZhUPYDF2azkXQtBQu3A4 eTGeHZrfWsqJKibLEzS7K2pvb+/l1IdMuNmZ7YafYFZahth4RhLQn0IvFCinkErD E8nu7q6YgWi6niUClmNANIZZXAR6sPPvD7BK7NjCSd6KVywx+zocZ5k+UIw4Zo+J mJKmJ/W5e52QKrhZLJ4xlhECUa16MydbX38Y2DmpwUuZuIzNVrdrZ/M5iHJm1sjG UVXf2J/SYfGzKr+iSb+L8aSI3vwwFdLvkL4pggY+axdJoar1wpo= =NBQA -----END PGP SIGNATURE----- --GAoked8QSizNecZ5--