On Thu, Aug 30, 2018 at 01:16:46PM +1000, Alexey Kardashevskiy wrote:
> The userspace can request an arbitrary supported page size for a DMA
> window and this works fine as long as the mapped memory is backed with
> the pages of the same or bigger size; if this is not the case,
> mm_iommu_ua_to_hpa{_rm}() fail and tables do not populated with
> dangerously incorrect TCEs.
>
> However since it is quite easy to misconfigure the KVM and we do not do
> reverts to all changes made to TCE tables if an error happens in a middle,
> we better do the acceptable page size validation before we even touch
> the tables.
>
> This enhances kvmppc_tce_validate() to check the hardware IOMMU page sizes
> against the preregistered memory page sizes.
>
> Since the new check uses real/virtual mode helpers, this renames
> kvmppc_tce_validate() to kvmppc_rm_tce_validate() to handle the real mode
> case and mirrors it for the virtual mode under the old name. The real
> mode handler is not used for the virtual mode as:
> 1. it uses _lockless() list traversing primitives instead of RCU;
> 2. realmode's mm_iommu_ua_to_hpa_rm() uses vmalloc_to_phys() which
> virtual mode does not have to use and since on POWER9+radix only virtual
> mode handlers actually work, we do not want to slow down that path even
> a bit.
>
> This removes EXPORT_SYMBOL_GPL(kvmppc_tce_validate) as the validators
> are static now.
>
> >From now on the attempts on mapping IOMMU pages bigger than allowed will
> result in KVM exit.
>
> Signed-off-by: Alexey Kardashevskiy <aik@xxxxxxxxx>
Reviewed-by: David Gibson <david@xxxxxxxxxxxxxxxxxxxxx>
> ---
> arch/powerpc/include/asm/kvm_ppc.h | 2 --
> arch/powerpc/kvm/book3s_64_vio.c | 42 +++++++++++++++++++++++++++++++++++++
> arch/powerpc/kvm/book3s_64_vio_hv.c | 30 +++++++++++++++++++-------
> 3 files changed, 65 insertions(+), 9 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h
> index e991821..2f5d431 100644
> --- a/arch/powerpc/include/asm/kvm_ppc.h
> +++ b/arch/powerpc/include/asm/kvm_ppc.h
> @@ -194,8 +194,6 @@ extern struct kvmppc_spapr_tce_table *kvmppc_find_table(
> (iommu_tce_check_ioba((stt)->page_shift, (stt)->offset, \
> (stt)->size, (ioba), (npages)) ? \
> H_PARAMETER : H_SUCCESS)
> -extern long kvmppc_tce_validate(struct kvmppc_spapr_tce_table *tt,
> - unsigned long tce);
> extern long kvmppc_gpa_to_ua(struct kvm *kvm, unsigned long gpa,
> unsigned long *ua, unsigned long **prmap);
> extern void kvmppc_tce_put(struct kvmppc_spapr_tce_table *tt,
> diff --git a/arch/powerpc/kvm/book3s_64_vio.c b/arch/powerpc/kvm/book3s_64_vio.c
> index 3e8ac98..5cd2a66 100644
> --- a/arch/powerpc/kvm/book3s_64_vio.c
> +++ b/arch/powerpc/kvm/book3s_64_vio.c
> @@ -363,6 +363,41 @@ long kvm_vm_ioctl_create_spapr_tce(struct kvm *kvm,
> return ret;
> }
>
> +static long kvmppc_tce_validate(struct kvmppc_spapr_tce_table *stt,
> + unsigned long tce)
> +{
> + unsigned long gpa = tce & ~(TCE_PCI_READ | TCE_PCI_WRITE);
> + enum dma_data_direction dir = iommu_tce_direction(tce);
> + struct kvmppc_spapr_tce_iommu_table *stit;
> + unsigned long ua = 0;
> +
> + /* Allow userspace to poison TCE table */
> + if (dir == DMA_NONE)
> + return H_SUCCESS;
> +
> + if (iommu_tce_check_gpa(stt->page_shift, gpa))
> + return H_TOO_HARD;
> +
> + if (kvmppc_gpa_to_ua(stt->kvm, tce & ~(TCE_PCI_READ | TCE_PCI_WRITE),
> + &ua, NULL))
> + return H_TOO_HARD;
> +
> + list_for_each_entry_rcu(stit, &stt->iommu_tables, next) {
> + unsigned long hpa = 0;
> + struct mm_iommu_table_group_mem_t *mem;
> + long shift = stit->tbl->it_page_shift;
> +
> + mem = mm_iommu_lookup(stt->kvm->mm, ua, 1ULL << shift);
> + if (!mem)
> + return H_TOO_HARD;
> +
> + if (mm_iommu_ua_to_hpa(mem, ua, shift, &hpa))
> + return H_TOO_HARD;
> + }
> +
> + return H_SUCCESS;
> +}
> +
> static void kvmppc_clear_tce(struct iommu_table *tbl, unsigned long entry)
> {
> unsigned long hpa = 0;
> @@ -602,6 +637,13 @@ long kvmppc_h_put_tce_indirect(struct kvm_vcpu *vcpu,
> }
>
> for (i = 0; i < npages; ++i) {
> + /*
> + * This get_user() may produce a different result than few
> + * lines in the validation loop above but we translate it
> + * again little later anyway and if that fails, we simply stop
> + * and return error as it is likely the userspace shooting
> + * itself in a foot.
> + */
> if (get_user(tce, tces + i)) {
> ret = H_TOO_HARD;
> goto unlock_exit;
> diff --git a/arch/powerpc/kvm/book3s_64_vio_hv.c b/arch/powerpc/kvm/book3s_64_vio_hv.c
> index 9584d9b..e79ffbb 100644
> --- a/arch/powerpc/kvm/book3s_64_vio_hv.c
> +++ b/arch/powerpc/kvm/book3s_64_vio_hv.c
> @@ -94,14 +94,14 @@ EXPORT_SYMBOL_GPL(kvmppc_find_table);
> * 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).
> - *
> - * 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 long tce)
> +static long kvmppc_rm_tce_validate(struct kvmppc_spapr_tce_table *stt,
> + unsigned long tce)
> {
> unsigned long gpa = tce & ~(TCE_PCI_READ | TCE_PCI_WRITE);
> enum dma_data_direction dir = iommu_tce_direction(tce);
> + struct kvmppc_spapr_tce_iommu_table *stit;
> + unsigned long ua = 0;
>
> /* Allow userspace to poison TCE table */
> if (dir == DMA_NONE)
> @@ -110,9 +110,25 @@ long kvmppc_tce_validate(struct kvmppc_spapr_tce_table *stt, unsigned long tce)
> if (iommu_tce_check_gpa(stt->page_shift, gpa))
> return H_PARAMETER;
>
> + if (kvmppc_gpa_to_ua(stt->kvm, tce & ~(TCE_PCI_READ | TCE_PCI_WRITE),
> + &ua, NULL))
> + return H_TOO_HARD;
> +
> + list_for_each_entry_lockless(stit, &stt->iommu_tables, next) {
> + unsigned long hpa = 0;
> + struct mm_iommu_table_group_mem_t *mem;
> + long shift = stit->tbl->it_page_shift;
> +
> + mem = mm_iommu_lookup_rm(stt->kvm->mm, ua, 1ULL << shift);
> + if (!mem)
> + return H_TOO_HARD;
> +
> + if (mm_iommu_ua_to_hpa_rm(mem, ua, shift, &hpa))
> + return H_TOO_HARD;
> + }
> +
> return H_SUCCESS;
> }
> -EXPORT_SYMBOL_GPL(kvmppc_tce_validate);
>
> /* Note on the use of page_address() in real mode,
> *
> @@ -345,7 +361,7 @@ long kvmppc_rm_h_put_tce(struct kvm_vcpu *vcpu, unsigned long liobn,
> if (ret != H_SUCCESS)
> return ret;
>
> - ret = kvmppc_tce_validate(stt, tce);
> + ret = kvmppc_rm_tce_validate(stt, tce);
> if (ret != H_SUCCESS)
> return ret;
>
> @@ -498,7 +514,7 @@ long kvmppc_rm_h_put_tce_indirect(struct kvm_vcpu *vcpu,
> for (i = 0; i < npages; ++i) {
> unsigned long tce = be64_to_cpu(((u64 *)tces)[i]);
>
> - ret = kvmppc_tce_validate(stt, tce);
> + ret = kvmppc_rm_tce_validate(stt, tce);
> if (ret != H_SUCCESS)
> goto unlock_exit;
> }
--
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
Attachment:
signature.asc
Description: PGP signature
