mbox series

[v3,0/3] Remove split on unmap behavior

Message ID 0-v3-b3a5b5937f56+7bb-arm_no_split_jgg@nvidia.com (mailing list archive)
Headers show
Series Remove split on unmap behavior | expand

Message

Jason Gunthorpe Nov. 5, 2024, 6:14 p.m. UTC 
This is the result of the discussion on removing split. We agreed that
split is not required, and no application should ask for anything that
would not unmap a full large IOPTE.

Instead of split the two ARM drivers will now WARN_ON and return 0. This
is in contrast to what several other drivers do of removing the whole
IOPTE and returning 0.

The kdoc is updated to describe this.

v3:
 - Remove selftest code too
 - Fix copy/paste error in iommu_unmap kdoc
v2: https://patch.msgid.link/r/0-v2-fd55d00a60b2+c69-arm_no_split_jgg@nvidia.com
 - Use WARN_ON instead of duplicating AMD behavior
 - Add arm-v7s patch
 - Write a kdoc for iommu_unmap()
v1: https://patch.msgid.link/r/0-v1-8c5f369ec2e5+75-arm_no_split_jgg@nvidia.com

Jason Gunthorpe (3):
  iommu/io-pgtable-arm: Remove split on unmap behavior
  iommu/io-pgtable-arm-v7s: Remove split on unmap behavior
  iommu: Add a kdoc to iommu_unmap()

 drivers/iommu/io-pgtable-arm-v7s.c | 146 +----------------------------
 drivers/iommu/io-pgtable-arm.c     |  81 +---------------
 drivers/iommu/iommu.c              |  14 +++
 3 files changed, 21 insertions(+), 220 deletions(-)


base-commit: 8e929cb546ee42c9a61d24fae60605e9e3192354

Comments

Will Deacon Nov. 6, 2024, 3:53 p.m. UTC | #1 
On Tue, 05 Nov 2024 14:14:23 -0400, Jason Gunthorpe wrote:
> This is the result of the discussion on removing split. We agreed that
> split is not required, and no application should ask for anything that
> would not unmap a full large IOPTE.
> 
> Instead of split the two ARM drivers will now WARN_ON and return 0. This
> is in contrast to what several other drivers do of removing the whole
> IOPTE and returning 0.
> 
> [...]

Applied to iommu (arm/smmu), thanks!

[1/3] iommu/io-pgtable-arm: Remove split on unmap behavior
      https://git.kernel.org/iommu/c/33729a5fc0ca
[2/3] iommu/io-pgtable-arm-v7s: Remove split on unmap behavior
      https://git.kernel.org/iommu/c/fd50651636fb
[3/3] iommu: Add a kdoc to iommu_unmap()
      https://git.kernel.org/iommu/c/6ac7dffe7cca

Cheers,
Jason Gunthorpe Nov. 6, 2024, 4:15 p.m. UTC | #2 
On Wed, Nov 06, 2024 at 03:53:23PM +0000, Will Deacon wrote:
> On Tue, 05 Nov 2024 14:14:23 -0400, Jason Gunthorpe wrote:
> > This is the result of the discussion on removing split. We agreed that
> > split is not required, and no application should ask for anything that
> > would not unmap a full large IOPTE.
> > 
> > Instead of split the two ARM drivers will now WARN_ON and return 0. This
> > is in contrast to what several other drivers do of removing the whole
> > IOPTE and returning 0.
> > 
> > [...]
> 
> Applied to iommu (arm/smmu), thanks!
> 
> [1/3] iommu/io-pgtable-arm: Remove split on unmap behavior
>       https://git.kernel.org/iommu/c/33729a5fc0ca
> [2/3] iommu/io-pgtable-arm-v7s: Remove split on unmap behavior
>       https://git.kernel.org/iommu/c/fd50651636fb
> [3/3] iommu: Add a kdoc to iommu_unmap()
>       https://git.kernel.org/iommu/c/6ac7dffe7cca

Thanks, can you add this hunk to fix the W=1 warning?

--- a/drivers/iommu/io-pgtable-arm-v7s.c
+++ b/drivers/iommu/io-pgtable-arm-v7s.c
@@ -820,7 +820,7 @@ static int __init arm_v7s_do_selftests(void)
                .pgsize_bitmap = SZ_4K | SZ_64K | SZ_1M | SZ_16M,
        };
        unsigned int iova, size;
-       unsigned int i, loopnr = 0;
+       unsigned int i;
        size_t mapped;
 
        selftest_running = true;
@@ -868,7 +868,6 @@ static int __init arm_v7s_do_selftests(void)
                        return __FAIL(ops);
 
                iova += SZ_16M;
-               loopnr++;
        }
 
        /* Full unmap */


Jason
Will Deacon Nov. 6, 2024, 4:22 p.m. UTC | #3 
On Wed, Nov 06, 2024 at 12:15:20PM -0400, Jason Gunthorpe wrote:
> On Wed, Nov 06, 2024 at 03:53:23PM +0000, Will Deacon wrote:
> > On Tue, 05 Nov 2024 14:14:23 -0400, Jason Gunthorpe wrote:
> > > This is the result of the discussion on removing split. We agreed that
> > > split is not required, and no application should ask for anything that
> > > would not unmap a full large IOPTE.
> > > 
> > > Instead of split the two ARM drivers will now WARN_ON and return 0. This
> > > is in contrast to what several other drivers do of removing the whole
> > > IOPTE and returning 0.
> > > 
> > > [...]
> > 
> > Applied to iommu (arm/smmu), thanks!
> > 
> > [1/3] iommu/io-pgtable-arm: Remove split on unmap behavior
> >       https://git.kernel.org/iommu/c/33729a5fc0ca
> > [2/3] iommu/io-pgtable-arm-v7s: Remove split on unmap behavior
> >       https://git.kernel.org/iommu/c/fd50651636fb
> > [3/3] iommu: Add a kdoc to iommu_unmap()
> >       https://git.kernel.org/iommu/c/6ac7dffe7cca
> 
> Thanks, can you add this hunk to fix the W=1 warning?
> 
> --- a/drivers/iommu/io-pgtable-arm-v7s.c
> +++ b/drivers/iommu/io-pgtable-arm-v7s.c
> @@ -820,7 +820,7 @@ static int __init arm_v7s_do_selftests(void)
>                 .pgsize_bitmap = SZ_4K | SZ_64K | SZ_1M | SZ_16M,
>         };
>         unsigned int iova, size;
> -       unsigned int i, loopnr = 0;
> +       unsigned int i;
>         size_t mapped;
>  
>         selftest_running = true;
> @@ -868,7 +868,6 @@ static int __init arm_v7s_do_selftests(void)
>                         return __FAIL(ops);
>  
>                 iova += SZ_16M;
> -               loopnr++;
>         }
>  
>         /* Full unmap */

I think I'd already folded that in, but please check!

Will