| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20160406171207.750f1224@t450s.home>
Date: Wed, 6 Apr 2016 17:12:07 -0600
From: Alex Williamson <alex.williamson@...hat.com>
To: Eric Auger <eric.auger@...aro.org>
Cc: eric.auger@...com, robin.murphy@....com, will.deacon@....com,
joro@...tes.org, tglx@...utronix.de, jason@...edaemon.net,
marc.zyngier@....com, christoffer.dall@...aro.org,
linux-arm-kernel@...ts.infradead.org, kvmarm@...ts.cs.columbia.edu,
kvm@...r.kernel.org, suravee.suthikulpanit@....com,
patches@...aro.org, linux-kernel@...r.kernel.org,
Manish.Jaggi@...iumnetworks.com, Bharat.Bhushan@...escale.com,
pranav.sawargaonkar@...il.com, p.fedin@...sung.com,
iommu@...ts.linux-foundation.org, Jean-Philippe.Brucker@....com,
julien.grall@....com
Subject: Re: [PATCH v6 6/7] dma-reserved-iommu:
iommu_get/put_single_reserved
On Mon, 4 Apr 2016 08:07:01 +0000
Eric Auger <eric.auger@...aro.org> wrote:
> This patch introduces iommu_get/put_single_reserved.
>
> iommu_get_single_reserved allows to allocate a new reserved iova page
> and map it onto the physical page that contains a given physical address.
> Page size is the IOMMU page one. It is the responsability of the
> system integrator to make sure the in use IOMMU page size corresponds
> to the granularity of the MSI frame.
>
> It returns the iova that is mapped onto the provided physical address.
> Hence the physical address passed in argument does not need to be aligned.
>
> In case a mapping already exists between both pages, the IOVA mapped
> to the PA is directly returned.
>
> Each time an iova is successfully returned a binding ref count is
> incremented.
>
> iommu_put_single_reserved decrements the ref count and when this latter
> is null, the mapping is destroyed and the iova is released.
>
> Signed-off-by: Eric Auger <eric.auger@...aro.org>
> Signed-off-by: Ankit Jindal <ajindal@....com>
> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@...aro.org>
> Signed-off-by: Bharat Bhushan <Bharat.Bhushan@...escale.com>
>
> ---
>
> v5 -> v6:
> - revisit locking with spin_lock instead of mutex
> - do not kref_get on 1st get
> - add size parameter to the get function following Marc's request
> - use the iova domain shift instead of using the smallest supported page size
>
> v3 -> v4:
> - formerly in iommu: iommu_get/put_single_reserved &
> iommu/arm-smmu: implement iommu_get/put_single_reserved
> - Attempted to address Marc's doubts about missing size/alignment
> at VFIO level (user-space knows the IOMMU page size and the number
> of IOVA pages to provision)
>
> v2 -> v3:
> - remove static implementation of iommu_get_single_reserved &
> iommu_put_single_reserved when CONFIG_IOMMU_API is not set
>
> v1 -> v2:
> - previously a VFIO API, named vfio_alloc_map/unmap_free_reserved_iova
> ---
> drivers/iommu/dma-reserved-iommu.c | 146 +++++++++++++++++++++++++++++++++++++
> include/linux/dma-reserved-iommu.h | 28 +++++++
> 2 files changed, 174 insertions(+)
>
> diff --git a/drivers/iommu/dma-reserved-iommu.c b/drivers/iommu/dma-reserved-iommu.c
> index f592118..3c759d9 100644
> --- a/drivers/iommu/dma-reserved-iommu.c
> +++ b/drivers/iommu/dma-reserved-iommu.c
> @@ -136,3 +136,149 @@ void iommu_free_reserved_iova_domain(struct iommu_domain *domain)
> spin_unlock_irqrestore(&domain->reserved_lock, flags);
> }
> EXPORT_SYMBOL_GPL(iommu_free_reserved_iova_domain);
> +
> +static void delete_reserved_binding(struct iommu_domain *domain,
> + struct iommu_reserved_binding *b)
> +{
> + struct iova_domain *iovad =
> + (struct iova_domain *)domain->reserved_iova_cookie;
> + unsigned long order = iova_shift(iovad);
> +
> + iommu_unmap(domain, b->iova, b->size);
> + free_iova(iovad, b->iova >> order);
> + kfree(b);
> +}
> +
> +int iommu_get_reserved_iova(struct iommu_domain *domain,
> + phys_addr_t addr, size_t size, int prot,
> + dma_addr_t *iova)
> +{
> + struct iova_domain *iovad =
> + (struct iova_domain *)domain->reserved_iova_cookie;
> + unsigned long order = iova_shift(iovad);
> + unsigned long base_pfn, end_pfn, nb_iommu_pages;
> + size_t iommu_page_size = 1 << order, binding_size;
> + phys_addr_t aligned_base, offset;
> + struct iommu_reserved_binding *b, *newb;
> + unsigned long flags;
> + struct iova *p_iova;
> + bool unmap = false;
> + int ret;
> +
> + base_pfn = addr >> order;
> + end_pfn = (addr + size - 1) >> order;
> + nb_iommu_pages = end_pfn - base_pfn + 1;
> + aligned_base = base_pfn << order;
> + offset = addr - aligned_base;
> + binding_size = nb_iommu_pages * iommu_page_size;
> +
> + if (!iovad)
> + return -EINVAL;
> +
> + spin_lock_irqsave(&domain->reserved_lock, flags);
> +
> + b = find_reserved_binding(domain, aligned_base, binding_size);
> + if (b) {
> + *iova = b->iova + offset;
> + kref_get(&b->kref);
> + ret = 0;
> + goto unlock;
> + }
> +
> + spin_unlock_irqrestore(&domain->reserved_lock, flags);
> +
> + /*
> + * no reserved IOVA was found for this PA, start allocating and
> + * registering one while the spin-lock is not held. iommu_map/unmap
> + * are not supposed to be atomic
> + */
> +
> + p_iova = alloc_iova(iovad, nb_iommu_pages, iovad->dma_32bit_pfn, true);
> + if (!p_iova)
> + return -ENOMEM;
Here we're using iovad, which was the reserved_iova_cookie outside of
the locking, which makes the locking ineffective. Isn't this lock also
protecting our iova domain, I'm confused.
> +
> + *iova = iova_dma_addr(iovad, p_iova);
> +
> + newb = kzalloc(sizeof(*b), GFP_KERNEL);
> + if (!newb) {
> + free_iova(iovad, p_iova->pfn_lo);
> + return -ENOMEM;
> + }
> +
> + ret = iommu_map(domain, *iova, aligned_base, binding_size, prot);
> + if (ret) {
> + kfree(newb);
> + free_iova(iovad, p_iova->pfn_lo);
> + return ret;
> + }
> +
> + spin_lock_irqsave(&domain->reserved_lock, flags);
> +
> + /* re-check the PA was not mapped in our back when lock was not held */
> + b = find_reserved_binding(domain, aligned_base, binding_size);
> + if (b) {
> + *iova = b->iova + offset;
> + kref_get(&b->kref);
> + ret = 0;
> + unmap = true;
> + goto unlock;
> + }
> +
> + kref_init(&newb->kref);
> + newb->domain = domain;
> + newb->addr = aligned_base;
> + newb->iova = *iova;
> + newb->size = binding_size;
> +
> + link_reserved_binding(domain, newb);
> +
> + *iova += offset;
> + goto unlock;
??
> +
> +unlock:
> + spin_unlock_irqrestore(&domain->reserved_lock, flags);
> + if (unmap)
> + delete_reserved_binding(domain, newb);
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(iommu_get_reserved_iova);
> +
> +void iommu_put_reserved_iova(struct iommu_domain *domain, dma_addr_t iova)
> +{
> + struct iova_domain *iovad =
> + (struct iova_domain *)domain->reserved_iova_cookie;
> + unsigned long order;
> + phys_addr_t aligned_addr;
> + dma_addr_t aligned_iova, page_size, mask, offset;
> + struct iommu_reserved_binding *b;
> + unsigned long flags;
> + bool unmap = false;
> +
> + order = iova_shift(iovad);
> + page_size = (uint64_t)1 << order;
> + mask = page_size - 1;
> +
> + aligned_iova = iova & ~mask;
> + offset = iova - aligned_iova;
> +
> + aligned_addr = iommu_iova_to_phys(domain, aligned_iova);
> +
> + spin_lock_irqsave(&domain->reserved_lock, flags);
> + b = find_reserved_binding(domain, aligned_addr, page_size);
> + if (!b)
> + goto unlock;
> +
> + if (atomic_sub_and_test(1, &b->kref.refcount)) {
> + unlink_reserved_binding(domain, b);
> + unmap = true;
> + }
> +
> +unlock:
> + spin_unlock_irqrestore(&domain->reserved_lock, flags);
> + if (unmap)
> + delete_reserved_binding(domain, b);
> +}
> +EXPORT_SYMBOL_GPL(iommu_put_reserved_iova);
> +
> +
> +
> diff --git a/include/linux/dma-reserved-iommu.h b/include/linux/dma-reserved-iommu.h
> index 5bf863b..dedea56 100644
> --- a/include/linux/dma-reserved-iommu.h
> +++ b/include/linux/dma-reserved-iommu.h
> @@ -40,6 +40,34 @@ int iommu_alloc_reserved_iova_domain(struct iommu_domain *domain,
> */
> void iommu_free_reserved_iova_domain(struct iommu_domain *domain);
>
> +/**
> + * iommu_get_reserved_iova: allocate a contiguous set of iova pages and
> + * map them to the physical range defined by @addr and @size.
> + *
> + * @domain: iommu domain handle
> + * @addr: physical address to bind
> + * @size: size of the binding
> + * @prot: mapping protection attribute
> + * @iova: returned iova
> + *
> + * Mapped physical pfns are within [@addr >> order, (@addr + size -1) >> order]
> + * where order corresponds to the iova domain order.
> + * This mapping is reference counted as a whole and cannot by split.
> + */
> +int iommu_get_reserved_iova(struct iommu_domain *domain,
> + phys_addr_t addr, size_t size, int prot,
> + dma_addr_t *iova);
> +
> +/**
> + * iommu_put_reserved_iova: decrement a ref count of the reserved mapping
> + *
> + * @domain: iommu domain handle
> + * @iova: reserved iova whose binding ref count is decremented
> + *
> + * if the binding ref count is null, destroy the reserved mapping
> + */
> +void iommu_put_reserved_iova(struct iommu_domain *domain, dma_addr_t iova);
> +
> #endif /* CONFIG_IOMMU_DMA_RESERVED */
> #endif /* __KERNEL__ */
> #endif /* __DMA_RESERVED_IOMMU_H */
Powered by blists - more mailing lists