lkml.org 
[lkml]   [2020]   [Jul]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v4 5/7] powerpc/iommu: Move iommu_table cleaning routine to iommu_table_clean
From
Date


On 16/07/2020 17:16, Leonardo Bras wrote:
> Move the part of iommu_table_free() that does struct iommu_table cleaning
> into iommu_table_clean, so we can invoke it separately.
>
> This new function is useful for cleaning struct iommu_table before
> initializing it again with a new DMA window, without having it freed and
> allocated again.
>
> Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
> ---
> arch/powerpc/kernel/iommu.c | 30 ++++++++++++++++++------------
> 1 file changed, 18 insertions(+), 12 deletions(-)
>
> diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
> index 9704f3f76e63..c3242253a4e7 100644
> --- a/arch/powerpc/kernel/iommu.c
> +++ b/arch/powerpc/kernel/iommu.c
> @@ -735,21 +735,10 @@ struct iommu_table *iommu_init_table(struct iommu_table *tbl, int nid,
> return tbl;
> }
>
> -static void iommu_table_free(struct kref *kref)
> +static void iommu_table_clean(struct iommu_table *tbl)


iommu_table_free() + iommu_init_table() + set_iommu_table_base() should
work too, why new helper?

There is also iommu_table_clear() which does a different thing so you
need a better name.

Second, iommu_table_free() would print a warning if any IOMMU page is in
use and it would be ok as we would only see this when hot-unplugging a
PE because we always kept the default window.

Btw you must be seeing these warnings now every time you create DDW with
these patches as at least the first page is reserved, do not you?

Since we are replacing a table for a device which is still in the
system, we should not try messing with its DMA if it already has
mappings so the warning should become an error preventing DDW. It is
rather hard to trigger in practice but I could hack a driver to ask for
32bit DMA mask first, map few pages and then ask for 64bit DMA mask, it
is not illegal, I think. So this needs a new helper - "bool
iommu_table_in_use(tbl)" - to use in enable_ddw(). Or I am overthinking
this?... Thanks,



> {
> unsigned long bitmap_sz;
> unsigned int order;
> - struct iommu_table *tbl;
> -
> - tbl = container_of(kref, struct iommu_table, it_kref);
> -
> - if (tbl->it_ops->free)
> - tbl->it_ops->free(tbl);
> -
> - if (!tbl->it_map) {
> - kfree(tbl);
> - return;
> - }
>
> iommu_table_release_pages(tbl);
>
> @@ -763,6 +752,23 @@ static void iommu_table_free(struct kref *kref)
> /* free bitmap */
> order = get_order(bitmap_sz);
> free_pages((unsigned long) tbl->it_map, order);
> +}
> +
> +static void iommu_table_free(struct kref *kref)
> +{
> + struct iommu_table *tbl;
> +
> + tbl = container_of(kref, struct iommu_table, it_kref);
> +
> + if (tbl->it_ops->free)
> + tbl->it_ops->free(tbl);
> +
> + if (!tbl->it_map) {
> + kfree(tbl);
> + return;
> + }
> +
> + iommu_table_clean(tbl);
>
> /* free table */
> kfree(tbl);
>

--
Alexey

\
 
 \ /
  Last update: 2020-07-21 07:00    [W:0.134 / U:0.256 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site