| 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: <ee935a6d-a94c-313e-f0ed-e14cc6dac055@arm.com>
Date: Fri, 19 Mar 2021 16:13:17 +0000
From: Robin Murphy <robin.murphy@....com>
To: John Garry <john.garry@...wei.com>, joro@...tes.org,
will@...nel.org, jejb@...ux.ibm.com, martin.petersen@...cle.com,
hch@....de, m.szyprowski@...sung.com
Cc: iommu@...ts.linux-foundation.org, linux-kernel@...r.kernel.org,
linux-scsi@...r.kernel.org, linuxarm@...wei.com
Subject: Re: [PATCH 1/6] iommu: Move IOVA power-of-2 roundup into allocator
On 2021-03-19 13:25, John Garry wrote:
> Move the IOVA size power-of-2 rcache roundup into the IOVA allocator.
>
> This is to eventually make it possible to be able to configure the upper
> limit of the IOVA rcache range.
>
> Signed-off-by: John Garry <john.garry@...wei.com>
> ---
> drivers/iommu/dma-iommu.c | 8 ------
> drivers/iommu/iova.c | 51 ++++++++++++++++++++++++++-------------
> 2 files changed, 34 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
> index af765c813cc8..15b7270a5c2a 100644
> --- a/drivers/iommu/dma-iommu.c
> +++ b/drivers/iommu/dma-iommu.c
> @@ -429,14 +429,6 @@ static dma_addr_t iommu_dma_alloc_iova(struct iommu_domain *domain,
>
> shift = iova_shift(iovad);
> iova_len = size >> shift;
> - /*
> - * Freeing non-power-of-two-sized allocations back into the IOVA caches
> - * will come back to bite us badly, so we have to waste a bit of space
> - * rounding up anything cacheable to make sure that can't happen. The
> - * order of the unadjusted size will still match upon freeing.
> - */
> - if (iova_len < (1 << (IOVA_RANGE_CACHE_MAX_SIZE - 1)))
> - iova_len = roundup_pow_of_two(iova_len);
>
> dma_limit = min_not_zero(dma_limit, dev->bus_dma_limit);
>
> diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c
> index e6e2fa85271c..e62e9e30b30c 100644
> --- a/drivers/iommu/iova.c
> +++ b/drivers/iommu/iova.c
> @@ -179,7 +179,7 @@ iova_insert_rbtree(struct rb_root *root, struct iova *iova,
>
> static int __alloc_and_insert_iova_range(struct iova_domain *iovad,
> unsigned long size, unsigned long limit_pfn,
> - struct iova *new, bool size_aligned)
> + struct iova *new, bool size_aligned, bool fast)
> {
> struct rb_node *curr, *prev;
> struct iova *curr_iova;
> @@ -188,6 +188,15 @@ static int __alloc_and_insert_iova_range(struct iova_domain *iovad,
> unsigned long align_mask = ~0UL;
> unsigned long high_pfn = limit_pfn, low_pfn = iovad->start_pfn;
>
> + /*
> + * Freeing non-power-of-two-sized allocations back into the IOVA caches
> + * will come back to bite us badly, so we have to waste a bit of space
> + * rounding up anything cacheable to make sure that can't happen. The
> + * order of the unadjusted size will still match upon freeing.
> + */
> + if (fast && size < (1 << (IOVA_RANGE_CACHE_MAX_SIZE - 1)))
> + size = roundup_pow_of_two(size);
If this transformation is only relevant to alloc_iova_fast(), and we
have to add a special parameter here to tell whether we were called from
alloc_iova_fast(), doesn't it seem more sensible to just do it in
alloc_iova_fast() rather than here?
But then the API itself has no strict requirement that a pfn passed to
free_iova_fast() wasn't originally allocated with alloc_iova(), so
arguably hiding the adjustment away makes it less clear that the
responsibility is really on any caller of free_iova_fast() to make sure
they don't get things wrong.
Robin.
> +
> if (size_aligned)
> align_mask <<= fls_long(size - 1);
>
> @@ -288,21 +297,10 @@ void iova_cache_put(void)
> }
> EXPORT_SYMBOL_GPL(iova_cache_put);
>
> -/**
> - * alloc_iova - allocates an iova
> - * @iovad: - iova domain in question
> - * @size: - size of page frames to allocate
> - * @limit_pfn: - max limit address
> - * @size_aligned: - set if size_aligned address range is required
> - * This function allocates an iova in the range iovad->start_pfn to limit_pfn,
> - * searching top-down from limit_pfn to iovad->start_pfn. If the size_aligned
> - * flag is set then the allocated address iova->pfn_lo will be naturally
> - * aligned on roundup_power_of_two(size).
> - */
> -struct iova *
> -alloc_iova(struct iova_domain *iovad, unsigned long size,
> +static struct iova *
> +__alloc_iova(struct iova_domain *iovad, unsigned long size,
> unsigned long limit_pfn,
> - bool size_aligned)
> + bool size_aligned, bool fast)
> {
> struct iova *new_iova;
> int ret;
> @@ -312,7 +310,7 @@ alloc_iova(struct iova_domain *iovad, unsigned long size,
> return NULL;
>
> ret = __alloc_and_insert_iova_range(iovad, size, limit_pfn + 1,
> - new_iova, size_aligned);
> + new_iova, size_aligned, fast);
>
> if (ret) {
> free_iova_mem(new_iova);
> @@ -321,6 +319,25 @@ alloc_iova(struct iova_domain *iovad, unsigned long size,
>
> return new_iova;
> }
> +
> +/**
> + * alloc_iova - allocates an iova
> + * @iovad: - iova domain in question
> + * @size: - size of page frames to allocate
> + * @limit_pfn: - max limit address
> + * @size_aligned: - set if size_aligned address range is required
> + * This function allocates an iova in the range iovad->start_pfn to limit_pfn,
> + * searching top-down from limit_pfn to iovad->start_pfn. If the size_aligned
> + * flag is set then the allocated address iova->pfn_lo will be naturally
> + * aligned on roundup_power_of_two(size).
> + */
> +struct iova *
> +alloc_iova(struct iova_domain *iovad, unsigned long size,
> + unsigned long limit_pfn,
> + bool size_aligned)
> +{
> + return __alloc_iova(iovad, size, limit_pfn, size_aligned, false);
> +}
> EXPORT_SYMBOL_GPL(alloc_iova);
>
> static struct iova *
> @@ -433,7 +450,7 @@ alloc_iova_fast(struct iova_domain *iovad, unsigned long size,
> return iova_pfn;
>
> retry:
> - new_iova = alloc_iova(iovad, size, limit_pfn, true);
> + new_iova = __alloc_iova(iovad, size, limit_pfn, true, true);
> if (!new_iova) {
> unsigned int cpu;
>
>
Powered by blists - more mailing lists