genpt: Make GENERIC_PT invisible

[PATCH] genpt: Make GENERIC_PT invisible

Posted by Geert Uytterhoeven 2 months, 4 weeks ago

There is no point in asking the user about the Generic Radix Page
Table API:
  - All IOMMU drivers that use this API already select GENERIC_PT when
    needed,
  - Most users probably do not know what to answer anyway.

Fixes: 7c5b184db7145fd4 ("genpt: Generic Page Table base API")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/iommu/generic_pt/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/generic_pt/Kconfig b/drivers/iommu/generic_pt/Kconfig
index c88971675662feb5..ce4fb478691457e1 100644
--- a/drivers/iommu/generic_pt/Kconfig
+++ b/drivers/iommu/generic_pt/Kconfig
@@ -1,7 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0-only
 
 menuconfig GENERIC_PT
-	bool "Generic Radix Page Table"
+	bool "Generic Radix Page Table" if COMPILE_TEST
 	help
 	  Generic library for building radix tree page tables.
 
-- 
2.43.0

Re: [PATCH] genpt: Make GENERIC_PT invisible

Posted by Jason Gunthorpe 2 months, 2 weeks ago

On Wed, Nov 12, 2025 at 03:08:05PM +0100, Geert Uytterhoeven wrote:
> There is no point in asking the user about the Generic Radix Page
> Table API:
>   - All IOMMU drivers that use this API already select GENERIC_PT when
>     needed,
>   - Most users probably do not know what to answer anyway.
> 
> Fixes: 7c5b184db7145fd4 ("genpt: Generic Page Table base API")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  drivers/iommu/generic_pt/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>

Jason

Re: [PATCH] genpt: Make GENERIC_PT invisible

Posted by Jason Gunthorpe 2 months, 2 weeks ago

On Thu, Nov 20, 2025 at 12:49:33PM -0400, Jason Gunthorpe wrote:
> On Wed, Nov 12, 2025 at 03:08:05PM +0100, Geert Uytterhoeven wrote:
> > There is no point in asking the user about the Generic Radix Page
> > Table API:
> >   - All IOMMU drivers that use this API already select GENERIC_PT when
> >     needed,
> >   - Most users probably do not know what to answer anyway.
> > 
> > Fixes: 7c5b184db7145fd4 ("genpt: Generic Page Table base API")
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > ---
> >  drivers/iommu/generic_pt/Kconfig | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>

Actually, it doesn't work :\

$ tools/testing/kunit/kunit.py run --build_dir build_kunit_x86_64 --arch x86_64 --kunitconfig ./drivers/iommu/generic_pt/.kunitconfig
[13:01:26] Configuring KUnit Kernel ...
[13:01:26] Building KUnit Kernel ...
Populating config with:
$ make ARCH=x86_64 O=build_kunit_x86_64 olddefconfig
Building with:
$ make all compile_commands.json scripts_gdb ARCH=x86_64 O=build_kunit_x86_64 --jobs=20
ERROR:root:Not all Kconfig options selected in kunitconfig were in the generated .config.
This is probably due to unsatisfied dependencies.
Missing: CONFIG_IOMMUFD_TEST=y, CONFIG_DEBUG_GENERIC_PT=y, CONFIG_IOMMU_PT_VTDSS=y, CONFIG_IOMMU_PT=y, CONFIG_IOMMU_PT_AMDV1=y, CONFIG_IOMMU_PT_X86_64=y, CONFIG_GENERIC_PT=y, CONFIG_IOMMU_PT_KUNIT_TEST=y

Can you add this hunk and send a v2?

--- a/drivers/iommu/generic_pt/.kunitconfig
+++ b/drivers/iommu/generic_pt/.kunitconfig
@@ -1,4 +1,5 @@
 CONFIG_KUNIT=y
+CONFIG_COMPILE_TEST=y
 CONFIG_GENERIC_PT=y
 CONFIG_DEBUG_GENERIC_PT=y
 CONFIG_IOMMU_PT=y

Thanks,
Jason

Re: [PATCH] genpt: Make GENERIC_PT invisible

Posted by Geert Uytterhoeven 2 months, 2 weeks ago

Hi Jason,

CC kunit

On Thu, 20 Nov 2025 at 18:07, Jason Gunthorpe <jgg@ziepe.ca> wrote:
> On Thu, Nov 20, 2025 at 12:49:33PM -0400, Jason Gunthorpe wrote:
> > On Wed, Nov 12, 2025 at 03:08:05PM +0100, Geert Uytterhoeven wrote:
> > > There is no point in asking the user about the Generic Radix Page
> > > Table API:
> > >   - All IOMMU drivers that use this API already select GENERIC_PT when
> > >     needed,
> > >   - Most users probably do not know what to answer anyway.
> > >
> > > Fixes: 7c5b184db7145fd4 ("genpt: Generic Page Table base API")
> > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > > ---
> > >  drivers/iommu/generic_pt/Kconfig | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
>
> Actually, it doesn't work :\
>
> $ tools/testing/kunit/kunit.py run --build_dir build_kunit_x86_64 --arch x86_64 --kunitconfig ./drivers/iommu/generic_pt/.kunitconfig
> [13:01:26] Configuring KUnit Kernel ...
> [13:01:26] Building KUnit Kernel ...
> Populating config with:
> $ make ARCH=x86_64 O=build_kunit_x86_64 olddefconfig
> Building with:
> $ make all compile_commands.json scripts_gdb ARCH=x86_64 O=build_kunit_x86_64 --jobs=20
> ERROR:root:Not all Kconfig options selected in kunitconfig were in the generated .config.
> This is probably due to unsatisfied dependencies.
> Missing: CONFIG_IOMMUFD_TEST=y, CONFIG_DEBUG_GENERIC_PT=y, CONFIG_IOMMU_PT_VTDSS=y, CONFIG_IOMMU_PT=y, CONFIG_IOMMU_PT_AMDV1=y, CONFIG_IOMMU_PT_X86_64=y, CONFIG_GENERIC_PT=y, CONFIG_IOMMU_PT_KUNIT_TEST=y
>
> Can you add this hunk and send a v2?
>
> --- a/drivers/iommu/generic_pt/.kunitconfig
> +++ b/drivers/iommu/generic_pt/.kunitconfig
> @@ -1,4 +1,5 @@
>  CONFIG_KUNIT=y
> +CONFIG_COMPILE_TEST=y
>  CONFIG_GENERIC_PT=y
>  CONFIG_DEBUG_GENERIC_PT=y
>  CONFIG_IOMMU_PT=y

Do you really want to enable CONFIG_COMPILE_TEST in a .kunitconfig?

Hm, that .kunitconfig already enables IOMMUFD_TEST, which is
documented to be dangerous (why?), and already enabled by allyesconfig
(except on GENERIC_ATOMIC64 architectures).
IOMMUFD_TEST cannot select GENERIC_PT, as that would lead to
a recursive dependency (and I am not a huge fan of test code auto-enabling
extra attack surfaces^W^W functionality).

Or perhaps:

-       bool "Generic Radix Page Table"
+       bool "Generic Radix Page Table" if COMPILE_TEST || KUNIT

?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] genpt: Make GENERIC_PT invisible

Posted by Jason Gunthorpe 2 months, 2 weeks ago

On Thu, Nov 20, 2025 at 07:31:10PM +0100, Geert Uytterhoeven wrote:
> Hi Jason,
> 
> CC kunit
> 
> On Thu, 20 Nov 2025 at 18:07, Jason Gunthorpe <jgg@ziepe.ca> wrote:
> > On Thu, Nov 20, 2025 at 12:49:33PM -0400, Jason Gunthorpe wrote:
> > > On Wed, Nov 12, 2025 at 03:08:05PM +0100, Geert Uytterhoeven wrote:
> > > > There is no point in asking the user about the Generic Radix Page
> > > > Table API:
> > > >   - All IOMMU drivers that use this API already select GENERIC_PT when
> > > >     needed,
> > > >   - Most users probably do not know what to answer anyway.
> > > >
> > > > Fixes: 7c5b184db7145fd4 ("genpt: Generic Page Table base API")
> > > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > > > ---
> > > >  drivers/iommu/generic_pt/Kconfig | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
> >
> > Actually, it doesn't work :\
> >
> > $ tools/testing/kunit/kunit.py run --build_dir build_kunit_x86_64 --arch x86_64 --kunitconfig ./drivers/iommu/generic_pt/.kunitconfig
> > [13:01:26] Configuring KUnit Kernel ...
> > [13:01:26] Building KUnit Kernel ...
> > Populating config with:
> > $ make ARCH=x86_64 O=build_kunit_x86_64 olddefconfig
> > Building with:
> > $ make all compile_commands.json scripts_gdb ARCH=x86_64 O=build_kunit_x86_64 --jobs=20
> > ERROR:root:Not all Kconfig options selected in kunitconfig were in the generated .config.
> > This is probably due to unsatisfied dependencies.
> > Missing: CONFIG_IOMMUFD_TEST=y, CONFIG_DEBUG_GENERIC_PT=y, CONFIG_IOMMU_PT_VTDSS=y, CONFIG_IOMMU_PT=y, CONFIG_IOMMU_PT_AMDV1=y, CONFIG_IOMMU_PT_X86_64=y, CONFIG_GENERIC_PT=y, CONFIG_IOMMU_PT_KUNIT_TEST=y
> >
> > Can you add this hunk and send a v2?
> >
> > --- a/drivers/iommu/generic_pt/.kunitconfig
> > +++ b/drivers/iommu/generic_pt/.kunitconfig
> > @@ -1,4 +1,5 @@
> >  CONFIG_KUNIT=y
> > +CONFIG_COMPILE_TEST=y
> >  CONFIG_GENERIC_PT=y
> >  CONFIG_DEBUG_GENERIC_PT=y
> >  CONFIG_IOMMU_PT=y
> 
> Do you really want to enable CONFIG_COMPILE_TEST in a .kunitconfig?

IDK, why not?

> Hm, that .kunitconfig already enables IOMMUFD_TEST, which is
> documented to be dangerous (why?)

It builds in a kernel module with a uapi that is kind of unsafe.

Though, hmm, maybe that is some weird a leftover I don't recall that
this kunit needed IOMMUFD_TEST stanza at all..

>  and already enabled by allyesconfig (except on GENERIC_ATOMIC64
> architectures).  

I guess allyesconfig would do that.

> IOMMUFD_TEST cannot select GENERIC_PT, as that would lead to a
> recursive dependency (and I am not a huge fan of test code
> auto-enabling extra attack surfaces^W^W functionality).

Yes

> Or perhaps:
> 
> -       bool "Generic Radix Page Table"
> +       bool "Generic Radix Page Table" if COMPILE_TEST || KUNIT
> 
> ?

It would work, that does seem like a better choice if someone wants to
make the kunit run in a normal disto kernel.

Thanks,
Jason

Re: [PATCH] genpt: Make GENERIC_PT invisible

Posted by Jason Gunthorpe 2 months, 4 weeks ago

On Wed, Nov 12, 2025 at 03:08:05PM +0100, Geert Uytterhoeven wrote:
> There is no point in asking the user about the Generic Radix Page
> Table API:
>   - All IOMMU drivers that use this API already select GENERIC_PT when
>     needed,
>   - Most users probably do not know what to answer anyway.
> 
> Fixes: 7c5b184db7145fd4 ("genpt: Generic Page Table base API")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  drivers/iommu/generic_pt/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

I can't check this right now but I do recall trying to do this from
the start and it was not working out, it ended up not being
automatically turned on?

Did you test something like menuconfig and the IOMMU drivers are still
presented starting from an allnoconfig?

Thanks,
Jason

Re: [PATCH] genpt: Make GENERIC_PT invisible

Posted by Geert Uytterhoeven 2 months, 3 weeks ago

Hi Jason,

On Wed, 12 Nov 2025 at 23:34, Jason Gunthorpe <jgg@ziepe.ca> wrote:
> On Wed, Nov 12, 2025 at 03:08:05PM +0100, Geert Uytterhoeven wrote:
> > There is no point in asking the user about the Generic Radix Page
> > Table API:
> >   - All IOMMU drivers that use this API already select GENERIC_PT when
> >     needed,
> >   - Most users probably do not know what to answer anyway.
> >
> > Fixes: 7c5b184db7145fd4 ("genpt: Generic Page Table base API")
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > ---
> >  drivers/iommu/generic_pt/Kconfig | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
>
> I can't check this right now but I do recall trying to do this from
> the start and it was not working out, it ended up not being
> automatically turned on?
>
> Did you test something like menuconfig and the IOMMU drivers are still
> presented starting from an allnoconfig?

Yes, that still works.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds