diff mbox series

iommu: PGTABLE_LPAE is also for RISCV

Message ID	20230330060105.29460-1-rdunlap@infradead.org
State	New
Headers	Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; From: Randy Dunlap <rdunlap@infradead.org> To: linux-kernel@vger.kernel.org Cc: Randy Dunlap <rdunlap@infradead.org>, Joerg Roedel <joro@8bytes.org>, Will Deacon <will@kernel.org>, Robin Murphy <robin.murphy@arm.com>, iommu@lists.linux.dev, Conor Dooley <conor@kernel.org>, linux-riscv@lists.infradead.org Subject: [PATCH] iommu: PGTABLE_LPAE is also for RISCV Date: Wed, 29 Mar 2023 23:01:05 -0700 Message-Id: <20230330060105.29460-1-rdunlap@infradead.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Precedence: bulk
Series	iommu: PGTABLE_LPAE is also for RISCV \| iommu: PGTABLE_LPAE is also for RISCV

Commit Message

Randy Dunlap March 30, 2023, 6:01 a.m. UTC

  On riscv64, linux-next-20233030 (and for several days earlier),
there is a kconfig warning:

WARNING: unmet direct dependencies detected for IOMMU_IO_PGTABLE_LPAE
  Depends on [n]: IOMMU_SUPPORT [=y] && (ARM || ARM64 || COMPILE_TEST [=n]) && !GENERIC_ATOMIC64 [=n]
  Selected by [y]:
  - IPMMU_VMSA [=y] && IOMMU_SUPPORT [=y] && (ARCH_RENESAS [=y] || COMPILE_TEST [=n]) && !GENERIC_ATOMIC64 [=n]

and build errors:

riscv64-linux-ld: drivers/iommu/io-pgtable-arm.o: in function `.L140':
io-pgtable-arm.c:(.init.text+0x1e8): undefined reference to `alloc_io_pgtable_ops'
riscv64-linux-ld: drivers/iommu/io-pgtable-arm.o: in function `.L168':
io-pgtable-arm.c:(.init.text+0xab0): undefined reference to `free_io_pgtable_ops'
riscv64-linux-ld: drivers/iommu/ipmmu-vmsa.o: in function `.L140':
ipmmu-vmsa.c:(.text+0xbc4): undefined reference to `free_io_pgtable_ops'
riscv64-linux-ld: drivers/iommu/ipmmu-vmsa.o: in function `.L0 ':
ipmmu-vmsa.c:(.text+0x145e): undefined reference to `alloc_io_pgtable_ops'

Add RISCV as an allowed ARCH dependency to fix these problems.

Fixes: d286a58bc8f4 ("iommu: Tidy up io-pgtable dependencies")
Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
Cc: Joerg Roedel <joro@8bytes.org>
Cc: Will Deacon <will@kernel.org>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: iommu@lists.linux.dev
Cc: Conor Dooley <conor@kernel.org>
Cc: linux-riscv@lists.infradead.org
---
 drivers/iommu/Kconfig |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Conor Dooley March 30, 2023, 6:25 a.m. UTC | #1

Hey Randy,

On Wed, Mar 29, 2023 at 11:01:05PM -0700, Randy Dunlap wrote:
> On riscv64, linux-next-20233030 (and for several days earlier),
> there is a kconfig warning:
> 
> WARNING: unmet direct dependencies detected for IOMMU_IO_PGTABLE_LPAE
>   Depends on [n]: IOMMU_SUPPORT [=y] && (ARM || ARM64 || COMPILE_TEST [=n]) && !GENERIC_ATOMIC64 [=n]
>   Selected by [y]:
>   - IPMMU_VMSA [=y] && IOMMU_SUPPORT [=y] && (ARCH_RENESAS [=y] || COMPILE_TEST [=n]) && !GENERIC_ATOMIC64 [=n]
> 
> and build errors:
> 
> riscv64-linux-ld: drivers/iommu/io-pgtable-arm.o: in function `.L140':
> io-pgtable-arm.c:(.init.text+0x1e8): undefined reference to `alloc_io_pgtable_ops'
> riscv64-linux-ld: drivers/iommu/io-pgtable-arm.o: in function `.L168':
> io-pgtable-arm.c:(.init.text+0xab0): undefined reference to `free_io_pgtable_ops'
> riscv64-linux-ld: drivers/iommu/ipmmu-vmsa.o: in function `.L140':
> ipmmu-vmsa.c:(.text+0xbc4): undefined reference to `free_io_pgtable_ops'
> riscv64-linux-ld: drivers/iommu/ipmmu-vmsa.o: in function `.L0 ':
> ipmmu-vmsa.c:(.text+0x145e): undefined reference to `alloc_io_pgtable_ops'
> 
> Add RISCV as an allowed ARCH dependency to fix these problems.
> 
> Fixes: d286a58bc8f4 ("iommu: Tidy up io-pgtable dependencies")
> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> Cc: Joerg Roedel <joro@8bytes.org>
> Cc: Will Deacon <will@kernel.org>
> Cc: Robin Murphy <robin.murphy@arm.com>
> Cc: iommu@lists.linux.dev
> Cc: Conor Dooley <conor@kernel.org>
> Cc: linux-riscv@lists.infradead.org
> ---
>  drivers/iommu/Kconfig |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff -- a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> --- a/drivers/iommu/Kconfig
> +++ b/drivers/iommu/Kconfig
> @@ -32,7 +32,7 @@ config IOMMU_IO_PGTABLE
>  config IOMMU_IO_PGTABLE_LPAE
>  	bool "ARMv7/v8 Long Descriptor Format"

I'm probably missing something here, but why would we want to enable
"ARMv7/v8 Long Descriptor Format" on RISC-V?
Would it not be better to make the Renesas depend on, rather than
select the option? It does seem highly arch specific, and I feel like
Geert previously mentioned that the RZ/Five (their RISC-V offering)
didn't use it.

Cheers,
Conor.

>  	select IOMMU_IO_PGTABLE
> -	depends on ARM || ARM64 || COMPILE_TEST
> +	depends on ARM || ARM64 || RISCV || COMPILE_TEST
>  	depends on !GENERIC_ATOMIC64	# for cmpxchg64()
>  	help
>  	  Enable support for the ARM long descriptor pagetable format.
>

Geert Uytterhoeven March 30, 2023, 7:31 a.m. UTC | #2

Hi Conor,

On Thu, Mar 30, 2023 at 8:25 AM Conor Dooley <conor.dooley@microchip.com> wrote:
> On Wed, Mar 29, 2023 at 11:01:05PM -0700, Randy Dunlap wrote:
> > On riscv64, linux-next-20233030 (and for several days earlier),
> > there is a kconfig warning:
> >
> > WARNING: unmet direct dependencies detected for IOMMU_IO_PGTABLE_LPAE
> >   Depends on [n]: IOMMU_SUPPORT [=y] && (ARM || ARM64 || COMPILE_TEST [=n]) && !GENERIC_ATOMIC64 [=n]
> >   Selected by [y]:
> >   - IPMMU_VMSA [=y] && IOMMU_SUPPORT [=y] && (ARCH_RENESAS [=y] || COMPILE_TEST [=n]) && !GENERIC_ATOMIC64 [=n]
> >
> > and build errors:
> >
> > riscv64-linux-ld: drivers/iommu/io-pgtable-arm.o: in function `.L140':
> > io-pgtable-arm.c:(.init.text+0x1e8): undefined reference to `alloc_io_pgtable_ops'
> > riscv64-linux-ld: drivers/iommu/io-pgtable-arm.o: in function `.L168':
> > io-pgtable-arm.c:(.init.text+0xab0): undefined reference to `free_io_pgtable_ops'
> > riscv64-linux-ld: drivers/iommu/ipmmu-vmsa.o: in function `.L140':
> > ipmmu-vmsa.c:(.text+0xbc4): undefined reference to `free_io_pgtable_ops'
> > riscv64-linux-ld: drivers/iommu/ipmmu-vmsa.o: in function `.L0 ':
> > ipmmu-vmsa.c:(.text+0x145e): undefined reference to `alloc_io_pgtable_ops'
> >
> > Add RISCV as an allowed ARCH dependency to fix these problems.
> >
> > Fixes: d286a58bc8f4 ("iommu: Tidy up io-pgtable dependencies")
> > Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> > Cc: Joerg Roedel <joro@8bytes.org>
> > Cc: Will Deacon <will@kernel.org>
> > Cc: Robin Murphy <robin.murphy@arm.com>
> > Cc: iommu@lists.linux.dev
> > Cc: Conor Dooley <conor@kernel.org>
> > Cc: linux-riscv@lists.infradead.org
> > ---
> >  drivers/iommu/Kconfig |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff -- a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> > --- a/drivers/iommu/Kconfig
> > +++ b/drivers/iommu/Kconfig
> > @@ -32,7 +32,7 @@ config IOMMU_IO_PGTABLE
> >  config IOMMU_IO_PGTABLE_LPAE
> >       bool "ARMv7/v8 Long Descriptor Format"
>
> I'm probably missing something here, but why would we want to enable
> "ARMv7/v8 Long Descriptor Format" on RISC-V?

Indeed, we should not enable it, unless compile-testing.

> Would it not be better to make the Renesas depend on, rather than
> select the option? It does seem highly arch specific, and I feel like
> Geert previously mentioned that the RZ/Five (their RISC-V offering)
> didn't use it.

I think the IPMMU_VMSA dependency should gain

        depends on ARM || ARM64 || COMPILE_TEST

> >       select IOMMU_IO_PGTABLE
> > -     depends on ARM || ARM64 || COMPILE_TEST
> > +     depends on ARM || ARM64 || RISCV || COMPILE_TEST
> >       depends on !GENERIC_ATOMIC64    # for cmpxchg64()
> >       help
> >         Enable support for the ARM long descriptor pagetable format.

Gr{oetje,eeting}s,

                        Geert

Robin Murphy March 30, 2023, 10:43 a.m. UTC | #3

On 2023-03-30 08:31, Geert Uytterhoeven wrote:
> Hi Conor,
> 
> On Thu, Mar 30, 2023 at 8:25 AM Conor Dooley <conor.dooley@microchip.com> wrote:
>> On Wed, Mar 29, 2023 at 11:01:05PM -0700, Randy Dunlap wrote:
>>> On riscv64, linux-next-20233030 (and for several days earlier),
>>> there is a kconfig warning:
>>>
>>> WARNING: unmet direct dependencies detected for IOMMU_IO_PGTABLE_LPAE
>>>    Depends on [n]: IOMMU_SUPPORT [=y] && (ARM || ARM64 || COMPILE_TEST [=n]) && !GENERIC_ATOMIC64 [=n]
>>>    Selected by [y]:
>>>    - IPMMU_VMSA [=y] && IOMMU_SUPPORT [=y] && (ARCH_RENESAS [=y] || COMPILE_TEST [=n]) && !GENERIC_ATOMIC64 [=n]
>>>
>>> and build errors:
>>>
>>> riscv64-linux-ld: drivers/iommu/io-pgtable-arm.o: in function `.L140':
>>> io-pgtable-arm.c:(.init.text+0x1e8): undefined reference to `alloc_io_pgtable_ops'
>>> riscv64-linux-ld: drivers/iommu/io-pgtable-arm.o: in function `.L168':
>>> io-pgtable-arm.c:(.init.text+0xab0): undefined reference to `free_io_pgtable_ops'
>>> riscv64-linux-ld: drivers/iommu/ipmmu-vmsa.o: in function `.L140':
>>> ipmmu-vmsa.c:(.text+0xbc4): undefined reference to `free_io_pgtable_ops'
>>> riscv64-linux-ld: drivers/iommu/ipmmu-vmsa.o: in function `.L0 ':
>>> ipmmu-vmsa.c:(.text+0x145e): undefined reference to `alloc_io_pgtable_ops'
>>>
>>> Add RISCV as an allowed ARCH dependency to fix these problems.
>>>
>>> Fixes: d286a58bc8f4 ("iommu: Tidy up io-pgtable dependencies")
>>> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
>>> Cc: Joerg Roedel <joro@8bytes.org>
>>> Cc: Will Deacon <will@kernel.org>
>>> Cc: Robin Murphy <robin.murphy@arm.com>
>>> Cc: iommu@lists.linux.dev
>>> Cc: Conor Dooley <conor@kernel.org>
>>> Cc: linux-riscv@lists.infradead.org
>>> ---
>>>   drivers/iommu/Kconfig |    2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff -- a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
>>> --- a/drivers/iommu/Kconfig
>>> +++ b/drivers/iommu/Kconfig
>>> @@ -32,7 +32,7 @@ config IOMMU_IO_PGTABLE
>>>   config IOMMU_IO_PGTABLE_LPAE
>>>        bool "ARMv7/v8 Long Descriptor Format"
>>
>> I'm probably missing something here, but why would we want to enable
>> "ARMv7/v8 Long Descriptor Format" on RISC-V?
> 
> Indeed, we should not enable it, unless compile-testing.
> 
>> Would it not be better to make the Renesas depend on, rather than
>> select the option? It does seem highly arch specific, and I feel like
>> Geert previously mentioned that the RZ/Five (their RISC-V offering)
>> didn't use it.
> 
> I think the IPMMU_VMSA dependency should gain
> 
>          depends on ARM || ARM64 || COMPILE_TEST

Indeed, I thought we'd settled on something like that in the previous 
thread(s) on ARCH_RENESAS vs. RV32, but I guess nobody got round to 
actually respininng the patch (possibly since my refactoring here ended 
up papering over the immediate RV32 issue).

Cheers,
Robin.

>>>        select IOMMU_IO_PGTABLE
>>> -     depends on ARM || ARM64 || COMPILE_TEST
>>> +     depends on ARM || ARM64 || RISCV || COMPILE_TEST
>>>        depends on !GENERIC_ATOMIC64    # for cmpxchg64()
>>>        help
>>>          Enable support for the ARM long descriptor pagetable format.
> 
> Gr{oetje,eeting}s,
> 
>                          Geert
>

Randy Dunlap March 30, 2023, 3:48 p.m. UTC | #4

Hi all,

On 3/30/23 00:31, Geert Uytterhoeven wrote:
> Hi Conor,
> 
> On Thu, Mar 30, 2023 at 8:25 AM Conor Dooley <conor.dooley@microchip.com> wrote:
>> On Wed, Mar 29, 2023 at 11:01:05PM -0700, Randy Dunlap wrote:
>>> On riscv64, linux-next-20233030 (and for several days earlier),
>>> there is a kconfig warning:
>>>
>>> WARNING: unmet direct dependencies detected for IOMMU_IO_PGTABLE_LPAE
>>>   Depends on [n]: IOMMU_SUPPORT [=y] && (ARM || ARM64 || COMPILE_TEST [=n]) && !GENERIC_ATOMIC64 [=n]
>>>   Selected by [y]:
>>>   - IPMMU_VMSA [=y] && IOMMU_SUPPORT [=y] && (ARCH_RENESAS [=y] || COMPILE_TEST [=n]) && !GENERIC_ATOMIC64 [=n]
>>>
>>> and build errors:
>>>
>>> riscv64-linux-ld: drivers/iommu/io-pgtable-arm.o: in function `.L140':
>>> io-pgtable-arm.c:(.init.text+0x1e8): undefined reference to `alloc_io_pgtable_ops'
>>> riscv64-linux-ld: drivers/iommu/io-pgtable-arm.o: in function `.L168':
>>> io-pgtable-arm.c:(.init.text+0xab0): undefined reference to `free_io_pgtable_ops'
>>> riscv64-linux-ld: drivers/iommu/ipmmu-vmsa.o: in function `.L140':
>>> ipmmu-vmsa.c:(.text+0xbc4): undefined reference to `free_io_pgtable_ops'
>>> riscv64-linux-ld: drivers/iommu/ipmmu-vmsa.o: in function `.L0 ':
>>> ipmmu-vmsa.c:(.text+0x145e): undefined reference to `alloc_io_pgtable_ops'
>>>
>>> Add RISCV as an allowed ARCH dependency to fix these problems.
>>>
>>> Fixes: d286a58bc8f4 ("iommu: Tidy up io-pgtable dependencies")
>>> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
>>> Cc: Joerg Roedel <joro@8bytes.org>
>>> Cc: Will Deacon <will@kernel.org>
>>> Cc: Robin Murphy <robin.murphy@arm.com>
>>> Cc: iommu@lists.linux.dev
>>> Cc: Conor Dooley <conor@kernel.org>
>>> Cc: linux-riscv@lists.infradead.org
>>> ---
>>>  drivers/iommu/Kconfig |    2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff -- a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
>>> --- a/drivers/iommu/Kconfig
>>> +++ b/drivers/iommu/Kconfig
>>> @@ -32,7 +32,7 @@ config IOMMU_IO_PGTABLE
>>>  config IOMMU_IO_PGTABLE_LPAE
>>>       bool "ARMv7/v8 Long Descriptor Format"
>>
>> I'm probably missing something here, but why would we want to enable
>> "ARMv7/v8 Long Descriptor Format" on RISC-V?
> 
> Indeed, we should not enable it, unless compile-testing.
> 
>> Would it not be better to make the Renesas depend on, rather than
>> select the option? It does seem highly arch specific, and I feel like
>> Geert previously mentioned that the RZ/Five (their RISC-V offering)
>> didn't use it.
> 
> I think the IPMMU_VMSA dependency should gain
> 
>         depends on ARM || ARM64 || COMPILE_TEST

so like this?
Or did you mean to drop the ARCH_RENESAS part also?


 config IPMMU_VMSA
        bool "Renesas VMSA-compatible IPMMU"
-       depends on ARCH_RENESAS || COMPILE_TEST
+       depends on ARCH_RENESAS || ARM || ARM64 || COMPILE_TEST

The failing config file has ARCH_RENESAS=y.  After the change above, I still get:

WARNING: unmet direct dependencies detected for IOMMU_IO_PGTABLE_LPAE
  Depends on [n]: IOMMU_SUPPORT [=y] && (ARM || ARM64 || COMPILE_TEST [=n]) && !GENERIC_ATOMIC64 [=n]
  Selected by [y]:
  - IPMMU_VMSA [=y] && IOMMU_SUPPORT [=y] && (ARCH_RENESAS [=y] || ARM || ARM64 || COMPILE_TEST [=n]) && !GENERIC_ATOMIC64 [=n]


I am attaching the config file.

>>>       select IOMMU_IO_PGTABLE
>>> -     depends on ARM || ARM64 || COMPILE_TEST
>>> +     depends on ARM || ARM64 || RISCV || COMPILE_TEST
>>>       depends on !GENERIC_ATOMIC64    # for cmpxchg64()
>>>       help
>>>         Enable support for the ARM long descriptor pagetable format.
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
>

Geert Uytterhoeven March 30, 2023, 4:11 p.m. UTC | #5

Hi Randy,

On Thu, Mar 30, 2023 at 5:48 PM Randy Dunlap <rdunlap@infradead.org> wrote:
> On 3/30/23 00:31, Geert Uytterhoeven wrote:
> > On Thu, Mar 30, 2023 at 8:25 AM Conor Dooley <conor.dooley@microchip.com> wrote:
> >> On Wed, Mar 29, 2023 at 11:01:05PM -0700, Randy Dunlap wrote:
> >>> On riscv64, linux-next-20233030 (and for several days earlier),
> >>> there is a kconfig warning:
> >>>
> >>> WARNING: unmet direct dependencies detected for IOMMU_IO_PGTABLE_LPAE
> >>>   Depends on [n]: IOMMU_SUPPORT [=y] && (ARM || ARM64 || COMPILE_TEST [=n]) && !GENERIC_ATOMIC64 [=n]
> >>>   Selected by [y]:
> >>>   - IPMMU_VMSA [=y] && IOMMU_SUPPORT [=y] && (ARCH_RENESAS [=y] || COMPILE_TEST [=n]) && !GENERIC_ATOMIC64 [=n]
> >>>
> >>> and build errors:
> >>>
> >>> riscv64-linux-ld: drivers/iommu/io-pgtable-arm.o: in function `.L140':
> >>> io-pgtable-arm.c:(.init.text+0x1e8): undefined reference to `alloc_io_pgtable_ops'
> >>> riscv64-linux-ld: drivers/iommu/io-pgtable-arm.o: in function `.L168':
> >>> io-pgtable-arm.c:(.init.text+0xab0): undefined reference to `free_io_pgtable_ops'
> >>> riscv64-linux-ld: drivers/iommu/ipmmu-vmsa.o: in function `.L140':
> >>> ipmmu-vmsa.c:(.text+0xbc4): undefined reference to `free_io_pgtable_ops'
> >>> riscv64-linux-ld: drivers/iommu/ipmmu-vmsa.o: in function `.L0 ':
> >>> ipmmu-vmsa.c:(.text+0x145e): undefined reference to `alloc_io_pgtable_ops'
> >>>
> >>> Add RISCV as an allowed ARCH dependency to fix these problems.
> >>>
> >>> Fixes: d286a58bc8f4 ("iommu: Tidy up io-pgtable dependencies")
> >>> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> >>> Cc: Joerg Roedel <joro@8bytes.org>
> >>> Cc: Will Deacon <will@kernel.org>
> >>> Cc: Robin Murphy <robin.murphy@arm.com>
> >>> Cc: iommu@lists.linux.dev
> >>> Cc: Conor Dooley <conor@kernel.org>
> >>> Cc: linux-riscv@lists.infradead.org
> >>> ---
> >>>  drivers/iommu/Kconfig |    2 +-
> >>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff -- a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> >>> --- a/drivers/iommu/Kconfig
> >>> +++ b/drivers/iommu/Kconfig
> >>> @@ -32,7 +32,7 @@ config IOMMU_IO_PGTABLE
> >>>  config IOMMU_IO_PGTABLE_LPAE
> >>>       bool "ARMv7/v8 Long Descriptor Format"
> >>
> >> I'm probably missing something here, but why would we want to enable
> >> "ARMv7/v8 Long Descriptor Format" on RISC-V?
> >
> > Indeed, we should not enable it, unless compile-testing.
> >
> >> Would it not be better to make the Renesas depend on, rather than
> >> select the option? It does seem highly arch specific, and I feel like
> >> Geert previously mentioned that the RZ/Five (their RISC-V offering)
> >> didn't use it.
> >
> > I think the IPMMU_VMSA dependency should gain
> >
> >         depends on ARM || ARM64 || COMPILE_TEST
>
> so like this?
> Or did you mean to drop the ARCH_RENESAS part also?
>
>
>  config IPMMU_VMSA
>         bool "Renesas VMSA-compatible IPMMU"
> -       depends on ARCH_RENESAS || COMPILE_TEST
> +       depends on ARCH_RENESAS || ARM || ARM64 || COMPILE_TEST

No, you want "depends on (ARCH_RENESAS && (ARM || ARM64)) || COMPILE_TEST",
which is a bit hard to read.

Hence I really meant adding that line, i.e.:

     config IPMMU_VMSA
           bool "Renesas VMSA-compatible IPMMU"
           depends on ARCH_RENESAS || COMPILE_TEST
    +      depends on ARM || ARM64 || COMPILE_TEST

>
> The failing config file has ARCH_RENESAS=y.  After the change above, I still get:
>
> WARNING: unmet direct dependencies detected for IOMMU_IO_PGTABLE_LPAE
>   Depends on [n]: IOMMU_SUPPORT [=y] && (ARM || ARM64 || COMPILE_TEST [=n]) && !GENERIC_ATOMIC64 [=n]
>   Selected by [y]:
>   - IPMMU_VMSA [=y] && IOMMU_SUPPORT [=y] && (ARCH_RENESAS [=y] || ARM || ARM64 || COMPILE_TEST [=n]) && !GENERIC_ATOMIC64 [=n]

Randy Dunlap March 30, 2023, 4:33 p.m. UTC | #6

Hi--

On 3/30/23 09:11, Geert Uytterhoeven wrote:
> Hi Randy,
> 
> On Thu, Mar 30, 2023 at 5:48 PM Randy Dunlap <rdunlap@infradead.org> wrote:
>> On 3/30/23 00:31, Geert Uytterhoeven wrote:
>>> On Thu, Mar 30, 2023 at 8:25 AM Conor Dooley <conor.dooley@microchip.com> wrote:
>>>> On Wed, Mar 29, 2023 at 11:01:05PM -0700, Randy Dunlap wrote:
>>>>> On riscv64, linux-next-20233030 (and for several days earlier),
>>>>> there is a kconfig warning:
>>>>>
>>>>> WARNING: unmet direct dependencies detected for IOMMU_IO_PGTABLE_LPAE
>>>>>   Depends on [n]: IOMMU_SUPPORT [=y] && (ARM || ARM64 || COMPILE_TEST [=n]) && !GENERIC_ATOMIC64 [=n]
>>>>>   Selected by [y]:
>>>>>   - IPMMU_VMSA [=y] && IOMMU_SUPPORT [=y] && (ARCH_RENESAS [=y] || COMPILE_TEST [=n]) && !GENERIC_ATOMIC64 [=n]
>>>>>
>>>>> and build errors:
>>>>>
>>>>> riscv64-linux-ld: drivers/iommu/io-pgtable-arm.o: in function `.L140':
>>>>> io-pgtable-arm.c:(.init.text+0x1e8): undefined reference to `alloc_io_pgtable_ops'
>>>>> riscv64-linux-ld: drivers/iommu/io-pgtable-arm.o: in function `.L168':
>>>>> io-pgtable-arm.c:(.init.text+0xab0): undefined reference to `free_io_pgtable_ops'
>>>>> riscv64-linux-ld: drivers/iommu/ipmmu-vmsa.o: in function `.L140':
>>>>> ipmmu-vmsa.c:(.text+0xbc4): undefined reference to `free_io_pgtable_ops'
>>>>> riscv64-linux-ld: drivers/iommu/ipmmu-vmsa.o: in function `.L0 ':
>>>>> ipmmu-vmsa.c:(.text+0x145e): undefined reference to `alloc_io_pgtable_ops'
>>>>>
>>>>> Add RISCV as an allowed ARCH dependency to fix these problems.
>>>>>
>>>>> Fixes: d286a58bc8f4 ("iommu: Tidy up io-pgtable dependencies")
>>>>> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
>>>>> Cc: Joerg Roedel <joro@8bytes.org>
>>>>> Cc: Will Deacon <will@kernel.org>
>>>>> Cc: Robin Murphy <robin.murphy@arm.com>
>>>>> Cc: iommu@lists.linux.dev
>>>>> Cc: Conor Dooley <conor@kernel.org>
>>>>> Cc: linux-riscv@lists.infradead.org
>>>>> ---
>>>>>  drivers/iommu/Kconfig |    2 +-
>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff -- a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
>>>>> --- a/drivers/iommu/Kconfig
>>>>> +++ b/drivers/iommu/Kconfig
>>>>> @@ -32,7 +32,7 @@ config IOMMU_IO_PGTABLE
>>>>>  config IOMMU_IO_PGTABLE_LPAE
>>>>>       bool "ARMv7/v8 Long Descriptor Format"
>>>>
>>>> I'm probably missing something here, but why would we want to enable
>>>> "ARMv7/v8 Long Descriptor Format" on RISC-V?
>>>
>>> Indeed, we should not enable it, unless compile-testing.
>>>
>>>> Would it not be better to make the Renesas depend on, rather than
>>>> select the option? It does seem highly arch specific, and I feel like
>>>> Geert previously mentioned that the RZ/Five (their RISC-V offering)
>>>> didn't use it.
>>>
>>> I think the IPMMU_VMSA dependency should gain
>>>
>>>         depends on ARM || ARM64 || COMPILE_TEST
>>
>> so like this?
>> Or did you mean to drop the ARCH_RENESAS part also?
>>
>>
>>  config IPMMU_VMSA
>>         bool "Renesas VMSA-compatible IPMMU"
>> -       depends on ARCH_RENESAS || COMPILE_TEST
>> +       depends on ARCH_RENESAS || ARM || ARM64 || COMPILE_TEST
> 
> No, you want "depends on (ARCH_RENESAS && (ARM || ARM64)) || COMPILE_TEST",
> which is a bit hard to read.
> 
> Hence I really meant adding that line, i.e.:
> 
>      config IPMMU_VMSA
>            bool "Renesas VMSA-compatible IPMMU"
>            depends on ARCH_RENESAS || COMPILE_TEST
>     +      depends on ARM || ARM64 || COMPILE_TEST
> 

OK, that fixes the kconfig warning and the build errors.
I can read the first method easier that the second one, but I'll
go with your suggestion. Hopefully no one will come along and
"fix it up" but instead muck it up.

Thanks.

Robin Murphy March 30, 2023, 4:34 p.m. UTC | #7

On 2023-03-30 07:01, Randy Dunlap wrote:
> On riscv64, linux-next-20233030 (and for several days earlier),
> there is a kconfig warning:
> 
> WARNING: unmet direct dependencies detected for IOMMU_IO_PGTABLE_LPAE
>    Depends on [n]: IOMMU_SUPPORT [=y] && (ARM || ARM64 || COMPILE_TEST [=n]) && !GENERIC_ATOMIC64 [=n]
>    Selected by [y]:
>    - IPMMU_VMSA [=y] && IOMMU_SUPPORT [=y] && (ARCH_RENESAS [=y] || COMPILE_TEST [=n]) && !GENERIC_ATOMIC64 [=n]
> 
> and build errors:
> 
> riscv64-linux-ld: drivers/iommu/io-pgtable-arm.o: in function `.L140':
> io-pgtable-arm.c:(.init.text+0x1e8): undefined reference to `alloc_io_pgtable_ops'
> riscv64-linux-ld: drivers/iommu/io-pgtable-arm.o: in function `.L168':
> io-pgtable-arm.c:(.init.text+0xab0): undefined reference to `free_io_pgtable_ops'
> riscv64-linux-ld: drivers/iommu/ipmmu-vmsa.o: in function `.L140':
> ipmmu-vmsa.c:(.text+0xbc4): undefined reference to `free_io_pgtable_ops'
> riscv64-linux-ld: drivers/iommu/ipmmu-vmsa.o: in function `.L0 ':
> ipmmu-vmsa.c:(.text+0x145e): undefined reference to `alloc_io_pgtable_ops'
> 
> Add RISCV as an allowed ARCH dependency to fix these problems.
> 
> Fixes: d286a58bc8f4 ("iommu: Tidy up io-pgtable dependencies")

BTW that doesn't look like the right fix target - this presumably dates 
back as far as when ARCH_RENESAS was added to RISCV, such that it was 
possible to start selecting IPMMU_VMSA without COMPILE_TEST?

Thanks,
Robin.

> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> Cc: Joerg Roedel <joro@8bytes.org>
> Cc: Will Deacon <will@kernel.org>
> Cc: Robin Murphy <robin.murphy@arm.com>
> Cc: iommu@lists.linux.dev
> Cc: Conor Dooley <conor@kernel.org>
> Cc: linux-riscv@lists.infradead.org
> ---
>   drivers/iommu/Kconfig |    2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff -- a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> --- a/drivers/iommu/Kconfig
> +++ b/drivers/iommu/Kconfig
> @@ -32,7 +32,7 @@ config IOMMU_IO_PGTABLE
>   config IOMMU_IO_PGTABLE_LPAE
>   	bool "ARMv7/v8 Long Descriptor Format"
>   	select IOMMU_IO_PGTABLE
> -	depends on ARM || ARM64 || COMPILE_TEST
> +	depends on ARM || ARM64 || RISCV || COMPILE_TEST
>   	depends on !GENERIC_ATOMIC64	# for cmpxchg64()
>   	help
>   	  Enable support for the ARM long descriptor pagetable format.

Randy Dunlap March 30, 2023, 4:49 p.m. UTC | #8

On 3/30/23 09:34, Robin Murphy wrote:
> On 2023-03-30 07:01, Randy Dunlap wrote:
>> On riscv64, linux-next-20233030 (and for several days earlier),
>> there is a kconfig warning:
>>
>> WARNING: unmet direct dependencies detected for IOMMU_IO_PGTABLE_LPAE
>>    Depends on [n]: IOMMU_SUPPORT [=y] && (ARM || ARM64 || COMPILE_TEST [=n]) && !GENERIC_ATOMIC64 [=n]
>>    Selected by [y]:
>>    - IPMMU_VMSA [=y] && IOMMU_SUPPORT [=y] && (ARCH_RENESAS [=y] || COMPILE_TEST [=n]) && !GENERIC_ATOMIC64 [=n]
>>
>> and build errors:
>>
>> riscv64-linux-ld: drivers/iommu/io-pgtable-arm.o: in function `.L140':
>> io-pgtable-arm.c:(.init.text+0x1e8): undefined reference to `alloc_io_pgtable_ops'
>> riscv64-linux-ld: drivers/iommu/io-pgtable-arm.o: in function `.L168':
>> io-pgtable-arm.c:(.init.text+0xab0): undefined reference to `free_io_pgtable_ops'
>> riscv64-linux-ld: drivers/iommu/ipmmu-vmsa.o: in function `.L140':
>> ipmmu-vmsa.c:(.text+0xbc4): undefined reference to `free_io_pgtable_ops'
>> riscv64-linux-ld: drivers/iommu/ipmmu-vmsa.o: in function `.L0 ':
>> ipmmu-vmsa.c:(.text+0x145e): undefined reference to `alloc_io_pgtable_ops'
>>
>> Add RISCV as an allowed ARCH dependency to fix these problems.
>>
>> Fixes: d286a58bc8f4 ("iommu: Tidy up io-pgtable dependencies")
> 
> BTW that doesn't look like the right fix target - this presumably dates back as far as when ARCH_RENESAS was added to RISCV, such that it was possible to start selecting IPMMU_VMSA without COMPILE_TEST?

OK, I can go with that. Thanks for your help.

Alexandre Ghiti May 16, 2023, 7:02 a.m. UTC | #9

Hi Randy,


On 3/30/23 18:49, Randy Dunlap wrote:
>
> On 3/30/23 09:34, Robin Murphy wrote:
>> On 2023-03-30 07:01, Randy Dunlap wrote:
>>> On riscv64, linux-next-20233030 (and for several days earlier),
>>> there is a kconfig warning:
>>>
>>> WARNING: unmet direct dependencies detected for IOMMU_IO_PGTABLE_LPAE
>>>     Depends on [n]: IOMMU_SUPPORT [=y] && (ARM || ARM64 || COMPILE_TEST [=n]) && !GENERIC_ATOMIC64 [=n]
>>>     Selected by [y]:
>>>     - IPMMU_VMSA [=y] && IOMMU_SUPPORT [=y] && (ARCH_RENESAS [=y] || COMPILE_TEST [=n]) && !GENERIC_ATOMIC64 [=n]
>>>
>>> and build errors:
>>>
>>> riscv64-linux-ld: drivers/iommu/io-pgtable-arm.o: in function `.L140':
>>> io-pgtable-arm.c:(.init.text+0x1e8): undefined reference to `alloc_io_pgtable_ops'
>>> riscv64-linux-ld: drivers/iommu/io-pgtable-arm.o: in function `.L168':
>>> io-pgtable-arm.c:(.init.text+0xab0): undefined reference to `free_io_pgtable_ops'
>>> riscv64-linux-ld: drivers/iommu/ipmmu-vmsa.o: in function `.L140':
>>> ipmmu-vmsa.c:(.text+0xbc4): undefined reference to `free_io_pgtable_ops'
>>> riscv64-linux-ld: drivers/iommu/ipmmu-vmsa.o: in function `.L0 ':
>>> ipmmu-vmsa.c:(.text+0x145e): undefined reference to `alloc_io_pgtable_ops'
>>>
>>> Add RISCV as an allowed ARCH dependency to fix these problems.
>>>
>>> Fixes: d286a58bc8f4 ("iommu: Tidy up io-pgtable dependencies")
>> BTW that doesn't look like the right fix target - this presumably dates back as far as when ARCH_RENESAS was added to RISCV, such that it was possible to start selecting IPMMU_VMSA without COMPILE_TEST?
> OK, I can go with that. Thanks for your help.
>

Any updates on this fix?


Thanks,


Alex

Conor Dooley May 16, 2023, 7:12 a.m. UTC | #10

On Tue, May 16, 2023 at 09:02:50AM +0200, Alexandre Ghiti wrote:
> Hi Randy,
> 
> 
> On 3/30/23 18:49, Randy Dunlap wrote:
> > 
> > On 3/30/23 09:34, Robin Murphy wrote:
> > > On 2023-03-30 07:01, Randy Dunlap wrote:
> > > > On riscv64, linux-next-20233030 (and for several days earlier),
> > > > there is a kconfig warning:
> > > > 
> > > > WARNING: unmet direct dependencies detected for IOMMU_IO_PGTABLE_LPAE
> > > >     Depends on [n]: IOMMU_SUPPORT [=y] && (ARM || ARM64 || COMPILE_TEST [=n]) && !GENERIC_ATOMIC64 [=n]
> > > >     Selected by [y]:
> > > >     - IPMMU_VMSA [=y] && IOMMU_SUPPORT [=y] && (ARCH_RENESAS [=y] || COMPILE_TEST [=n]) && !GENERIC_ATOMIC64 [=n]
> > > > 
> > > > and build errors:
> > > > 
> > > > riscv64-linux-ld: drivers/iommu/io-pgtable-arm.o: in function `.L140':
> > > > io-pgtable-arm.c:(.init.text+0x1e8): undefined reference to `alloc_io_pgtable_ops'
> > > > riscv64-linux-ld: drivers/iommu/io-pgtable-arm.o: in function `.L168':
> > > > io-pgtable-arm.c:(.init.text+0xab0): undefined reference to `free_io_pgtable_ops'
> > > > riscv64-linux-ld: drivers/iommu/ipmmu-vmsa.o: in function `.L140':
> > > > ipmmu-vmsa.c:(.text+0xbc4): undefined reference to `free_io_pgtable_ops'
> > > > riscv64-linux-ld: drivers/iommu/ipmmu-vmsa.o: in function `.L0 ':
> > > > ipmmu-vmsa.c:(.text+0x145e): undefined reference to `alloc_io_pgtable_ops'
> > > > 
> > > > Add RISCV as an allowed ARCH dependency to fix these problems.
> > > > 
> > > > Fixes: d286a58bc8f4 ("iommu: Tidy up io-pgtable dependencies")
> > > BTW that doesn't look like the right fix target - this presumably dates back as far as when ARCH_RENESAS was added to RISCV, such that it was possible to start selecting IPMMU_VMSA without COMPILE_TEST?
> > OK, I can go with that. Thanks for your help.
> > 
> 
> Any updates on this fix?

Superseeded by
https://lore.kernel.org/linux-iommu/20230330165817.21920-1-rdunlap@infradead.org/

I guess that never went anywhere?

diff mbox series

Patch

diff -- a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -32,7 +32,7 @@  config IOMMU_IO_PGTABLE
 config IOMMU_IO_PGTABLE_LPAE
 	bool "ARMv7/v8 Long Descriptor Format"
 	select IOMMU_IO_PGTABLE
-	depends on ARM || ARM64 || COMPILE_TEST
+	depends on ARM || ARM64 || RISCV || COMPILE_TEST
 	depends on !GENERIC_ATOMIC64	# for cmpxchg64()
 	help
 	  Enable support for the ARM long descriptor pagetable format.