2022-09-27 13:55:38

by Geert Uytterhoeven

[permalink] [raw]
Subject: [PATCH] iommu/io-pgtable: Make IOMMU_IO_PGTABLE_DART invisible

There is no point in asking the user about both "Apple DART Formats" and
"Apple DART IOMMU Support", as the former is useless without the latter,
and the latter auto-selects the former.

Fixes: 745ef1092bcfcf3b ("iommu/io-pgtable: Move Apple DART support to its own file")
Signed-off-by: Geert Uytterhoeven <[email protected]>
---
Should IOMMU_IO_PGTABLE_LPAE and IOMMU_IO_PGTABLE_ARMV7S be made
invisible, too?
Are there users that do not select them?
---
drivers/iommu/Kconfig | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index dc5f7a156ff5ec73..f1affca6022e0a54 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -68,16 +68,13 @@ config IOMMU_IO_PGTABLE_ARMV7S_SELFTEST
If unsure, say N here.

config IOMMU_IO_PGTABLE_DART
- bool "Apple DART Formats"
+ bool
select IOMMU_IO_PGTABLE
- depends on ARM64 || (COMPILE_TEST && !GENERIC_ATOMIC64)
help
Enable support for the Apple DART pagetable formats. These include
the t8020 and t6000/t8110 DART formats used in Apple M1/M2 family
SoCs.

- If unsure, say N here.
-
endmenu

config IOMMU_DEBUGFS
--
2.25.1


2022-09-27 14:27:28

by Robin Murphy

[permalink] [raw]
Subject: Re: [PATCH] iommu/io-pgtable: Make IOMMU_IO_PGTABLE_DART invisible

On 2022-09-27 14:36, Geert Uytterhoeven wrote:
> There is no point in asking the user about both "Apple DART Formats" and
> "Apple DART IOMMU Support", as the former is useless without the latter,
> and the latter auto-selects the former.
>
> Fixes: 745ef1092bcfcf3b ("iommu/io-pgtable: Move Apple DART support to its own file")
> Signed-off-by: Geert Uytterhoeven <[email protected]>
> ---
> Should IOMMU_IO_PGTABLE_LPAE and IOMMU_IO_PGTABLE_ARMV7S be made
> invisible, too?
> Are there users that do not select them?

The aim was for formats to be independently selectable for COMPILE_TEST
coverage. The Arm formats are manually selectable for the sake of their
runtime self-tests, which are self-contained, but since DART format
doesn't do anything by itself I'd agree there's no need to prompt when
!COMPILE_TEST here.

Thanks,
Robin.

> ---
> drivers/iommu/Kconfig | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> index dc5f7a156ff5ec73..f1affca6022e0a54 100644
> --- a/drivers/iommu/Kconfig
> +++ b/drivers/iommu/Kconfig
> @@ -68,16 +68,13 @@ config IOMMU_IO_PGTABLE_ARMV7S_SELFTEST
> If unsure, say N here.
>
> config IOMMU_IO_PGTABLE_DART
> - bool "Apple DART Formats"
> + bool
> select IOMMU_IO_PGTABLE
> - depends on ARM64 || (COMPILE_TEST && !GENERIC_ATOMIC64)
> help
> Enable support for the Apple DART pagetable formats. These include
> the t8020 and t6000/t8110 DART formats used in Apple M1/M2 family
> SoCs.
>
> - If unsure, say N here.
> -
> endmenu
>
> config IOMMU_DEBUGFS

2022-09-27 15:02:04

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH] iommu/io-pgtable: Make IOMMU_IO_PGTABLE_DART invisible

Hi Robin,

On Tue, Sep 27, 2022 at 4:15 PM Robin Murphy <[email protected]> wrote:
> On 2022-09-27 14:36, Geert Uytterhoeven wrote:
> > There is no point in asking the user about both "Apple DART Formats" and
> > "Apple DART IOMMU Support", as the former is useless without the latter,
> > and the latter auto-selects the former.
> >
> > Fixes: 745ef1092bcfcf3b ("iommu/io-pgtable: Move Apple DART support to its own file")
> > Signed-off-by: Geert Uytterhoeven <[email protected]>
> > ---
> > Should IOMMU_IO_PGTABLE_LPAE and IOMMU_IO_PGTABLE_ARMV7S be made
> > invisible, too?
> > Are there users that do not select them?
>
> The aim was for formats to be independently selectable for COMPILE_TEST
> coverage. The Arm formats are manually selectable for the sake of their
> runtime self-tests, which are self-contained, but since DART format
> doesn't do anything by itself I'd agree there's no need to prompt when
> !COMPILE_TEST here.

IOMMU_IO_PGTABLE_LPAE and IOMMU_IO_PGTABLE_ARMV7S are
selected by other symbols that can be enabled when compile-testing, so
the tests can still be enabled in those cases, too

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

2022-09-27 15:49:11

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH] iommu/io-pgtable: Make IOMMU_IO_PGTABLE_DART invisible

Hi Robin,

On Tue, Sep 27, 2022 at 5:09 PM Robin Murphy <[email protected]> wrote:
> On 2022-09-27 15:48, Geert Uytterhoeven wrote:
> > On Tue, Sep 27, 2022 at 4:15 PM Robin Murphy <[email protected]> wrote:
> >> On 2022-09-27 14:36, Geert Uytterhoeven wrote:
> >>> There is no point in asking the user about both "Apple DART Formats" and
> >>> "Apple DART IOMMU Support", as the former is useless without the latter,
> >>> and the latter auto-selects the former.
> >>>
> >>> Fixes: 745ef1092bcfcf3b ("iommu/io-pgtable: Move Apple DART support to its own file")
> >>> Signed-off-by: Geert Uytterhoeven <[email protected]>
> >>> ---
> >>> Should IOMMU_IO_PGTABLE_LPAE and IOMMU_IO_PGTABLE_ARMV7S be made
> >>> invisible, too?
> >>> Are there users that do not select them?
> >>
> >> The aim was for formats to be independently selectable for COMPILE_TEST
> >> coverage. The Arm formats are manually selectable for the sake of their
> >> runtime self-tests, which are self-contained, but since DART format
> >> doesn't do anything by itself I'd agree there's no need to prompt when
> >> !COMPILE_TEST here.
> >
> > IOMMU_IO_PGTABLE_LPAE and IOMMU_IO_PGTABLE_ARMV7S are
> > selected by other symbols that can be enabled when compile-testing, so
> > the tests can still be enabled in those cases, too
>
> Sure, but when you want to compile-test a thing, what would you rather
> do: enable the thing, or go hunting to find some other thing that
> happens to select the thing you actually want, then potentially have to
> figure out *that* thing's dependencies, and so on?

Agreed.

> Coverage isn't solely about whether it's technically possible to ever
> reach somewhere at all, it's just as much about how easily and/or often
> you can get there in practice. I don't see who benefits from making
> COMPILE_TEST harder to use :/

So perhaps the visibility of IOMMU_IO_PGTABLE_LPAE and
IOMMU_IO_PGTABLE_ARMV7S should depend on COMPILE_TEST?
Normal users would still get it through select when needed.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

2022-09-27 15:53:33

by Robin Murphy

[permalink] [raw]
Subject: Re: [PATCH] iommu/io-pgtable: Make IOMMU_IO_PGTABLE_DART invisible

On 2022-09-27 15:48, Geert Uytterhoeven wrote:
> Hi Robin,
>
> On Tue, Sep 27, 2022 at 4:15 PM Robin Murphy <[email protected]> wrote:
>> On 2022-09-27 14:36, Geert Uytterhoeven wrote:
>>> There is no point in asking the user about both "Apple DART Formats" and
>>> "Apple DART IOMMU Support", as the former is useless without the latter,
>>> and the latter auto-selects the former.
>>>
>>> Fixes: 745ef1092bcfcf3b ("iommu/io-pgtable: Move Apple DART support to its own file")
>>> Signed-off-by: Geert Uytterhoeven <[email protected]>
>>> ---
>>> Should IOMMU_IO_PGTABLE_LPAE and IOMMU_IO_PGTABLE_ARMV7S be made
>>> invisible, too?
>>> Are there users that do not select them?
>>
>> The aim was for formats to be independently selectable for COMPILE_TEST
>> coverage. The Arm formats are manually selectable for the sake of their
>> runtime self-tests, which are self-contained, but since DART format
>> doesn't do anything by itself I'd agree there's no need to prompt when
>> !COMPILE_TEST here.
>
> IOMMU_IO_PGTABLE_LPAE and IOMMU_IO_PGTABLE_ARMV7S are
> selected by other symbols that can be enabled when compile-testing, so
> the tests can still be enabled in those cases, too

Sure, but when you want to compile-test a thing, what would you rather
do: enable the thing, or go hunting to find some other thing that
happens to select the thing you actually want, then potentially have to
figure out *that* thing's dependencies, and so on?

Coverage isn't solely about whether it's technically possible to ever
reach somewhere at all, it's just as much about how easily and/or often
you can get there in practice. I don't see who benefits from making
COMPILE_TEST harder to use :/

Thanks,
Robin.

2022-09-27 16:01:45

by Robin Murphy

[permalink] [raw]
Subject: Re: [PATCH] iommu/io-pgtable: Make IOMMU_IO_PGTABLE_DART invisible

On 2022-09-27 16:29, Geert Uytterhoeven wrote:
> Hi Robin,
>
> On Tue, Sep 27, 2022 at 5:09 PM Robin Murphy <[email protected]> wrote:
>> On 2022-09-27 15:48, Geert Uytterhoeven wrote:
>>> On Tue, Sep 27, 2022 at 4:15 PM Robin Murphy <[email protected]> wrote:
>>>> On 2022-09-27 14:36, Geert Uytterhoeven wrote:
>>>>> There is no point in asking the user about both "Apple DART Formats" and
>>>>> "Apple DART IOMMU Support", as the former is useless without the latter,
>>>>> and the latter auto-selects the former.
>>>>>
>>>>> Fixes: 745ef1092bcfcf3b ("iommu/io-pgtable: Move Apple DART support to its own file")
>>>>> Signed-off-by: Geert Uytterhoeven <[email protected]>
>>>>> ---
>>>>> Should IOMMU_IO_PGTABLE_LPAE and IOMMU_IO_PGTABLE_ARMV7S be made
>>>>> invisible, too?
>>>>> Are there users that do not select them?
>>>>
>>>> The aim was for formats to be independently selectable for COMPILE_TEST
>>>> coverage. The Arm formats are manually selectable for the sake of their
>>>> runtime self-tests, which are self-contained, but since DART format
>>>> doesn't do anything by itself I'd agree there's no need to prompt when
>>>> !COMPILE_TEST here.
>>>
>>> IOMMU_IO_PGTABLE_LPAE and IOMMU_IO_PGTABLE_ARMV7S are
>>> selected by other symbols that can be enabled when compile-testing, so
>>> the tests can still be enabled in those cases, too
>>
>> Sure, but when you want to compile-test a thing, what would you rather
>> do: enable the thing, or go hunting to find some other thing that
>> happens to select the thing you actually want, then potentially have to
>> figure out *that* thing's dependencies, and so on?
>
> Agreed.
>
>> Coverage isn't solely about whether it's technically possible to ever
>> reach somewhere at all, it's just as much about how easily and/or often
>> you can get there in practice. I don't see who benefits from making
>> COMPILE_TEST harder to use :/
>
> So perhaps the visibility of IOMMU_IO_PGTABLE_LPAE and
> IOMMU_IO_PGTABLE_ARMV7S should depend on COMPILE_TEST?
> Normal users would still get it through select when needed.

As I say those still offer functionality beyond compile-testing, but now
you've got me suspecting that it's already suboptimal that one has to
enable the format to make the self-test option appear... Perhaps what we
want is a separate master option to enable io-pgtable self-tests in
general, then rejig the rest around that.

Of course the self-tests would be even more useful if the harness was at
the level of the core io-pgtable API so it could cover new formats
automatically as long as they provide the configuration parameters, but
that's a separate matter for someone with sufficient free time and
enthusiasm :)

Cheers,
Robin.

2022-09-27 16:04:02

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH] iommu/io-pgtable: Make IOMMU_IO_PGTABLE_DART invisible

Hi Robin,

On Tue, Sep 27, 2022 at 5:50 PM Robin Murphy <[email protected]> wrote:
> On 2022-09-27 16:29, Geert Uytterhoeven wrote:
> > On Tue, Sep 27, 2022 at 5:09 PM Robin Murphy <[email protected]> wrote:
> >> On 2022-09-27 15:48, Geert Uytterhoeven wrote:
> >>> On Tue, Sep 27, 2022 at 4:15 PM Robin Murphy <[email protected]> wrote:
> >>>> On 2022-09-27 14:36, Geert Uytterhoeven wrote:
> >>>>> There is no point in asking the user about both "Apple DART Formats" and
> >>>>> "Apple DART IOMMU Support", as the former is useless without the latter,
> >>>>> and the latter auto-selects the former.
> >>>>>
> >>>>> Fixes: 745ef1092bcfcf3b ("iommu/io-pgtable: Move Apple DART support to its own file")
> >>>>> Signed-off-by: Geert Uytterhoeven <[email protected]>
> >>>>> ---
> >>>>> Should IOMMU_IO_PGTABLE_LPAE and IOMMU_IO_PGTABLE_ARMV7S be made
> >>>>> invisible, too?
> >>>>> Are there users that do not select them?
> >>>>
> >>>> The aim was for formats to be independently selectable for COMPILE_TEST
> >>>> coverage. The Arm formats are manually selectable for the sake of their
> >>>> runtime self-tests, which are self-contained, but since DART format
> >>>> doesn't do anything by itself I'd agree there's no need to prompt when
> >>>> !COMPILE_TEST here.
> >>>
> >>> IOMMU_IO_PGTABLE_LPAE and IOMMU_IO_PGTABLE_ARMV7S are
> >>> selected by other symbols that can be enabled when compile-testing, so
> >>> the tests can still be enabled in those cases, too
> >>
> >> Sure, but when you want to compile-test a thing, what would you rather
> >> do: enable the thing, or go hunting to find some other thing that
> >> happens to select the thing you actually want, then potentially have to
> >> figure out *that* thing's dependencies, and so on?
> >
> > Agreed.
> >
> >> Coverage isn't solely about whether it's technically possible to ever
> >> reach somewhere at all, it's just as much about how easily and/or often
> >> you can get there in practice. I don't see who benefits from making
> >> COMPILE_TEST harder to use :/
> >
> > So perhaps the visibility of IOMMU_IO_PGTABLE_LPAE and
> > IOMMU_IO_PGTABLE_ARMV7S should depend on COMPILE_TEST?
> > Normal users would still get it through select when needed.
>
> As I say those still offer functionality beyond compile-testing, but now
> you've got me suspecting that it's already suboptimal that one has to
> enable the format to make the self-test option appear... Perhaps what we

IMHO that's the only sensible thing to do: you want to have the option
to enable (preferably modular) tests for the functionality you have
enabled in your product, so you can run the tests when needed.

> want is a separate master option to enable io-pgtable self-tests in
> general, then rejig the rest around that.

IMHO you do not want a master test option that suddenly enables
lots of functionality you do not need in your product.

> Of course the self-tests would be even more useful if the harness was at
> the level of the core io-pgtable API so it could cover new formats
> automatically as long as they provide the configuration parameters, but
> that's a separate matter for someone with sufficient free time and
> enthusiasm :)

OK, I will shut up ;-)

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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