From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ozlabs.org (ozlabs.org [IPv6:2401:3900:2:1::2]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id C2E661A0F25 for ; Tue, 8 Dec 2015 20:05:55 +1100 (AEDT) Date: Tue, 8 Dec 2015 16:27:40 +1100 From: David Gibson To: Alexey Kardashevskiy Cc: linuxppc-dev@lists.ozlabs.org, Paul Mackerras , Alexander Graf , kvm-ppc@vger.kernel.org, kvm@vger.kernel.org Subject: Re: [PATCH kernel 7/9] KVM: PPC: Move reusable bits of H_PUT_TCE handler to helpers Message-ID: <20151208052740.GS20139@voom.fritz.box> References: <1442314179-9706-1-git-send-email-aik@ozlabs.ru> <1442314179-9706-8-git-send-email-aik@ozlabs.ru> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="wnBGVoaGQwxWUIo6" In-Reply-To: <1442314179-9706-8-git-send-email-aik@ozlabs.ru> List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , --wnBGVoaGQwxWUIo6 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Sep 15, 2015 at 08:49:37PM +1000, Alexey Kardashevskiy wrote: > Upcoming multi-tce support (H_PUT_TCE_INDIRECT/H_STUFF_TCE hypercalls) > will validate TCE (not to have unexpected bits) and IO address > (to be within the DMA window boundaries). >=20 > This introduces helpers to validate TCE and IO address. >=20 > Signed-off-by: Alexey Kardashevskiy > --- > arch/powerpc/include/asm/kvm_ppc.h | 4 ++ > arch/powerpc/kvm/book3s_64_vio_hv.c | 89 +++++++++++++++++++++++++++++++= +----- > 2 files changed, 83 insertions(+), 10 deletions(-) >=20 > diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/as= m/kvm_ppc.h > index c6ef05b..fcde896 100644 > --- a/arch/powerpc/include/asm/kvm_ppc.h > +++ b/arch/powerpc/include/asm/kvm_ppc.h > @@ -166,6 +166,10 @@ extern int kvmppc_pseries_do_hcall(struct kvm_vcpu *= vcpu); > =20 > extern long kvm_vm_ioctl_create_spapr_tce(struct kvm *kvm, > struct kvm_create_spapr_tce *args); > +extern long kvmppc_ioba_validate(struct kvmppc_spapr_tce_table *stt, > + unsigned long ioba, unsigned long npages); > +extern long kvmppc_tce_validate(struct kvmppc_spapr_tce_table *tt, > + unsigned long tce); > extern long kvmppc_h_put_tce(struct kvm_vcpu *vcpu, unsigned long liobn, > unsigned long ioba, unsigned long tce); > extern long kvmppc_h_get_tce(struct kvm_vcpu *vcpu, unsigned long liobn, > diff --git a/arch/powerpc/kvm/book3s_64_vio_hv.c b/arch/powerpc/kvm/book3= s_64_vio_hv.c > index 6cf1ab3..f0fd84c 100644 > --- a/arch/powerpc/kvm/book3s_64_vio_hv.c > +++ b/arch/powerpc/kvm/book3s_64_vio_hv.c > @@ -36,6 +36,7 @@ > #include > #include > #include > +#include > =20 > #define TCES_PER_PAGE (PAGE_SIZE / sizeof(u64)) > =20 > @@ -64,7 +65,7 @@ static struct kvmppc_spapr_tce_table *kvmppc_find_table= (struct kvm_vcpu *vcpu, > * WARNING: This will be called in real-mode on HV KVM and virtual > * mode on PR KVM > */ > -static long kvmppc_ioba_validate(struct kvmppc_spapr_tce_table *stt, > +long kvmppc_ioba_validate(struct kvmppc_spapr_tce_table *stt, > unsigned long ioba, unsigned long npages) > { > unsigned long mask =3D (1ULL << IOMMU_PAGE_SHIFT_4K) - 1; > @@ -76,6 +77,79 @@ static long kvmppc_ioba_validate(struct kvmppc_spapr_t= ce_table *stt, > =20 > return H_SUCCESS; > } > +EXPORT_SYMBOL_GPL(kvmppc_ioba_validate); Why does it need to be exported - the new users will still be in the KVM module, won't they? > + > +/* > + * Validates TCE address. > + * At the moment flags and page mask are validated. > + * As the host kernel does not access those addresses (just puts them > + * to the table and user space is supposed to process them), we can skip > + * checking other things (such as TCE is a guest RAM address or the page > + * was actually allocated). Hmm. These comments apply given that the only current user of this is the kvm acceleration of userspace TCE tables, but the name suggests it would validate any TCE, including in kernel ones for which this would be unsafe. > + * WARNING: This will be called in real-mode on HV KVM and virtual > + * mode on PR KVM > + */ > +long kvmppc_tce_validate(struct kvmppc_spapr_tce_table *stt, unsigned lo= ng tce) > +{ > + unsigned long mask =3D ((1ULL << IOMMU_PAGE_SHIFT_4K) - 1) & > + ~(TCE_PCI_WRITE | TCE_PCI_READ); > + > + if (tce & mask) > + return H_PARAMETER; > + > + return H_SUCCESS; > +} > +EXPORT_SYMBOL_GPL(kvmppc_tce_validate); > + > +/* Note on the use of page_address() in real mode, > + * > + * It is safe to use page_address() in real mode on ppc64 because > + * page_address() is always defined as lowmem_page_address() > + * which returns __va(PFN_PHYS(page_to_pfn(page))) which is arithmetial > + * operation and does not access page struct. > + * > + * Theoretically page_address() could be defined different > + * but either WANT_PAGE_VIRTUAL or HASHED_PAGE_VIRTUAL > + * should be enabled. > + * WANT_PAGE_VIRTUAL is never enabled on ppc32/ppc64, > + * HASHED_PAGE_VIRTUAL could be enabled for ppc32 only and only > + * if CONFIG_HIGHMEM is defined. As CONFIG_SPARSEMEM_VMEMMAP > + * is not expected to be enabled on ppc32, page_address() > + * is safe for ppc32 as well. > + * > + * WARNING: This will be called in real-mode on HV KVM and virtual > + * mode on PR KVM > + */ > +static u64 *kvmppc_page_address(struct page *page) > +{ > +#if defined(HASHED_PAGE_VIRTUAL) || defined(WANT_PAGE_VIRTUAL) > +#error TODO: fix to avoid page_address() here > +#endif > + return (u64 *) page_address(page); > +} > + > +/* > + * Handles TCE requests for emulated devices. > + * Puts guest TCE values to the table and expects user space to convert = them. > + * Called in both real and virtual modes. > + * Cannot fail so kvmppc_tce_validate must be called before it. > + * > + * WARNING: This will be called in real-mode on HV KVM and virtual > + * mode on PR KVM > + */ > +void kvmppc_tce_put(struct kvmppc_spapr_tce_table *stt, > + unsigned long idx, unsigned long tce) > +{ > + struct page *page; > + u64 *tbl; > + > + page =3D stt->pages[idx / TCES_PER_PAGE]; > + tbl =3D kvmppc_page_address(page); > + > + tbl[idx % TCES_PER_PAGE] =3D tce; > +} > +EXPORT_SYMBOL_GPL(kvmppc_tce_put); > =20 > /* WARNING: This will be called in real-mode on HV KVM and virtual > * mode on PR KVM > @@ -85,9 +159,6 @@ long kvmppc_h_put_tce(struct kvm_vcpu *vcpu, unsigned = long liobn, > { > struct kvmppc_spapr_tce_table *stt =3D kvmppc_find_table(vcpu, liobn); > long ret =3D H_TOO_HARD; > - unsigned long idx; > - struct page *page; > - u64 *tbl; > =20 > /* udbg_printf("H_PUT_TCE(): liobn=3D0x%lx ioba=3D0x%lx, tce=3D0x%lx\n"= , */ > /* liobn, ioba, tce); */ > @@ -99,13 +170,11 @@ long kvmppc_h_put_tce(struct kvm_vcpu *vcpu, unsigne= d long liobn, > if (ret) > return ret; > =20 > - idx =3D ioba >> IOMMU_PAGE_SHIFT_4K; > - page =3D stt->pages[idx / TCES_PER_PAGE]; > - tbl =3D (u64 *)page_address(page); > + ret =3D kvmppc_tce_validate(stt, tce); > + if (ret) > + return ret; > =20 > - /* FIXME: Need to validate the TCE itself */ > - /* udbg_printf("tce @ %p\n", &tbl[idx % TCES_PER_PAGE]); */ > - tbl[idx % TCES_PER_PAGE] =3D tce; > + kvmppc_tce_put(stt, ioba >> IOMMU_PAGE_SHIFT_4K, tce); > =20 > return ret; > } --=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 --wnBGVoaGQwxWUIo6 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJWZmpMAAoJEGw4ysog2bOSAIoP/Ru/D9Ab03cwFL0RQ7Go57dF MEy8BNw8j6srZz/i/elMX4a8Eg3sQObTmrZuQ0oz0Kiu2GTxkOL1F7CuV9Ur/IAD Fdu3VkLZO6OlK0A8xDXuszeXfD7d3odVOjyNzPnkyUnwxWmSPDIgpWgApTYJ489f 50bL1TpCBNfKCORUERz1wVXwVQiDK5o3Gu7DRXCS76gDrbATNzPlBKGjiCQmMDj4 YsjOfAI2tbNZEOUS24mzfKPobRUcZN3AKbHefz1of1bICGoUjd/YVXClsVMSPXxd szTmlWVIk0F111ae7ZQCtb+w8IpCC+yVYb1G87zywqmrQB3+cGU8WVM1LwKx8OTY udy4Nr4uWJpWM12MdaN6fiYzeq4q3ZteQE6Ov878jzIzDvyE0ThlHm8z/X+EzUAi 19PfEUPS94j/9UJpxmTMam/l4TVHmGxbhRofwekOtktEgpzk2NguzxzmEzUphNOP O+rJnO266UP/yvWtgKcW11/kSBQheHE2U0OjL151dQG6Ttg1DbMItAkUgayh0XR9 be61kJjYTgYqY2MONRWx3ox7OuAJgAe6ctVGmpBnyeZEzJhnuReYtEcs2AeLiTKf ayiQ6UMZa+22NbbfNVFSVjZDlbfXVTQdhaLhagb26UkNvj8CB/vOgQphIHqZ6HWb RsJVBFvjpAleb2paRkSG =xMis -----END PGP SIGNATURE----- --wnBGVoaGQwxWUIo6--