* Re: iommu/io-pgtable: depend on !GENERIC_ATOMIC64 when using COMPILE_TEST with LPAE [not found] <20170712171654.35DA922D7E@pdx-korg-gitolite-1.ci.codeaurora.org> @ 2017-09-25 7:16 ` Geert Uytterhoeven 2017-09-25 15:21 ` Will Deacon 0 siblings, 1 reply; 4+ messages in thread From: Geert Uytterhoeven @ 2017-09-25 7:16 UTC (permalink / raw) To: Will Deacon; +Cc: Linux Kernel Mailing List, Joerg Roedel, iommu Hi Will, On Wed, Jul 12, 2017 at 7:16 PM, Linux Kernel Mailing List <linux-kernel@vger.kernel.org> wrote: > Web: https://git.kernel.org/torvalds/c/c1004803b40596c1aabbbc78a6b1b33e4dfd96c6 > Commit: c1004803b40596c1aabbbc78a6b1b33e4dfd96c6 > Parent: 58188afeb727e0f73706f1460707bd3ba6ccc221 > Refname: refs/heads/master > Author: Will Deacon <will.deacon@arm.com> > AuthorDate: Fri Jun 23 11:45:57 2017 +0100 > Committer: Will Deacon <will.deacon@arm.com> > CommitDate: Fri Jun 23 17:58:02 2017 +0100 > > iommu/io-pgtable: depend on !GENERIC_ATOMIC64 when using COMPILE_TEST with LPAE > > The LPAE/ARMv8 page table format relies on the ability to read and write > 64-bit page table entries in an atomic fashion. With the move to a lockless > implementation, we also need support for cmpxchg64 to resolve races when > installing table entries concurrently. > > Unfortunately, not all architectures support cmpxchg64, so the code can > fail to compiler when building for these architectures using COMPILE_TEST. > Rather than disable COMPILE_TEST altogether, instead check that > GENERIC_ATOMIC64 is not selected, which is a reasonable indication that > the architecture has support for 64-bit cmpxchg. > > Reported-by: kbuild test robot <fengguang.wu@intel.com> > Signed-off-by: Will Deacon <will.deacon@arm.com> > --- > drivers/iommu/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig > index 6ee3a25ae731..c88cfa7522b2 100644 > --- a/drivers/iommu/Kconfig > +++ b/drivers/iommu/Kconfig > @@ -23,7 +23,7 @@ config IOMMU_IO_PGTABLE > config IOMMU_IO_PGTABLE_LPAE > bool "ARMv7/v8 Long Descriptor Format" > select IOMMU_IO_PGTABLE > - depends on HAS_DMA && (ARM || ARM64 || COMPILE_TEST) > + depends on HAS_DMA && (ARM || ARM64 || (COMPILE_TEST && !GENERIC_ATOMIC64)) > help > Enable support for the ARM long descriptor pagetable format. > This allocator supports 4K/2M/1G, 16K/32M and 64K/512M page I can't find where this patch was submitted and discussed, so I'm replying to this email. On which architectures did it fail to compile? cmpxchg64() is defined by include/asm-generic/cmpxchg.h, so I fail to see what's the relation with GENERIC_ATOMIC64, which is related to lib/atomic64.c instead. E.g. on m68k, which uses GENERIC_ATOMIC64, it compile-tested fine before. Perhaps there's another (SMP vs UP?) dependency, as include/asm-generic/cmpxchg.h cannot be used on SMP? Should it be COMPILE_TEST && (!GENERIC_ATOMIC64 || !SMP)? Thanks! 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: iommu/io-pgtable: depend on !GENERIC_ATOMIC64 when using COMPILE_TEST with LPAE 2017-09-25 7:16 ` iommu/io-pgtable: depend on !GENERIC_ATOMIC64 when using COMPILE_TEST with LPAE Geert Uytterhoeven @ 2017-09-25 15:21 ` Will Deacon 2017-09-25 15:37 ` Geert Uytterhoeven 0 siblings, 1 reply; 4+ messages in thread From: Will Deacon @ 2017-09-25 15:21 UTC (permalink / raw) To: Geert Uytterhoeven; +Cc: Linux Kernel Mailing List, Joerg Roedel, iommu Hi Geert, On Mon, Sep 25, 2017 at 09:16:22AM +0200, Geert Uytterhoeven wrote: > On Wed, Jul 12, 2017 at 7:16 PM, Linux Kernel Mailing List > <linux-kernel@vger.kernel.org> wrote: > > Web: https://git.kernel.org/torvalds/c/c1004803b40596c1aabbbc78a6b1b33e4dfd96c6 > > Commit: c1004803b40596c1aabbbc78a6b1b33e4dfd96c6 > > Parent: 58188afeb727e0f73706f1460707bd3ba6ccc221 > > Refname: refs/heads/master > > Author: Will Deacon <will.deacon@arm.com> > > AuthorDate: Fri Jun 23 11:45:57 2017 +0100 > > Committer: Will Deacon <will.deacon@arm.com> > > CommitDate: Fri Jun 23 17:58:02 2017 +0100 > > > > iommu/io-pgtable: depend on !GENERIC_ATOMIC64 when using COMPILE_TEST with LPAE > > > > The LPAE/ARMv8 page table format relies on the ability to read and write > > 64-bit page table entries in an atomic fashion. With the move to a lockless > > implementation, we also need support for cmpxchg64 to resolve races when > > installing table entries concurrently. > > > > Unfortunately, not all architectures support cmpxchg64, so the code can > > fail to compiler when building for these architectures using COMPILE_TEST. > > Rather than disable COMPILE_TEST altogether, instead check that > > GENERIC_ATOMIC64 is not selected, which is a reasonable indication that > > the architecture has support for 64-bit cmpxchg. > > > > Reported-by: kbuild test robot <fengguang.wu@intel.com> > > Signed-off-by: Will Deacon <will.deacon@arm.com> > > --- > > drivers/iommu/Kconfig | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig > > index 6ee3a25ae731..c88cfa7522b2 100644 > > --- a/drivers/iommu/Kconfig > > +++ b/drivers/iommu/Kconfig > > @@ -23,7 +23,7 @@ config IOMMU_IO_PGTABLE > > config IOMMU_IO_PGTABLE_LPAE > > bool "ARMv7/v8 Long Descriptor Format" > > select IOMMU_IO_PGTABLE > > - depends on HAS_DMA && (ARM || ARM64 || COMPILE_TEST) > > + depends on HAS_DMA && (ARM || ARM64 || (COMPILE_TEST && !GENERIC_ATOMIC64)) > > help > > Enable support for the ARM long descriptor pagetable format. > > This allocator supports 4K/2M/1G, 16K/32M and 64K/512M page > > I can't find where this patch was submitted and discussed, so I'm replying > to this email. On which architectures did it fail to compile? It was in response to a report from the kbuild test robot on m32r, but looking back I now see that it didn't go to the lists for some reason. I've included the report at the end of this email so you can have a look. > cmpxchg64() is defined by include/asm-generic/cmpxchg.h, so I fail to > see what's the relation with GENERIC_ATOMIC64, which is related to > lib/atomic64.c instead. Yeah, it's a bit of a hack, but we're basically relying on architectures that don't select GENERIC_ATOMIC64 providing their own cmpxchg64 implementation, which seems to be the case. > E.g. on m68k, which uses GENERIC_ATOMIC64, it compile-tested fine before. FWIW, the lock-based atomics wouldn't be sufficient at runtime, but I appreciate that we're only talking about COMPILE_TEST here. > Perhaps there's another (SMP vs UP?) dependency, as > include/asm-generic/cmpxchg.h cannot be used on SMP? > Should it be COMPILE_TEST && (!GENERIC_ATOMIC64 || !SMP)? I don't see how that helps. Are you seeing build failures on a non-SMP arch? Will --->8 >From fengguang.wu@intel.com Fri Jun 23 07:06:26 2017 Date: Fri, 23 Jun 2017 14:05:19 +0800 From: kbuild test robot <fengguang.wu@intel.com> To: Robin Murphy <robin.murphy@arm.com> CC: kbuild-all@01.org, Will Deacon <will.deacon@arm.com> Subject: [arm-perf:for-joerg/arm-smmu/updates 10/13] include/linux/atomic.h:475:29: error: implicit declaration of function 'cmpxchg64' User-Agent: Mutt/1.5.23 (2014-03-12) MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset=utf-8 Status: RO X-Status: A tree: https://git.kernel.org/pub/scm/linux/kernel/git/will/linux.git for-joerg/arm-smmu/updates head: 5723c883174667dab5c42cbefa8d4d0e9acc16bc commit: a22753392d403aba096a3b10b2bc6ed721a2eb8f [10/13] iommu/io-pgtable-arm: Support lockless operation config: m32r-allyesconfig (attached as .config) compiler: m32r-linux-gcc (GCC) 6.2.0 reproduce: wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross git checkout a22753392d403aba096a3b10b2bc6ed721a2eb8f # save the attached .config to linux build tree make.cross ARCH=m32r All error/warnings (new ones prefixed by >>): In file included from drivers/iommu/io-pgtable-arm.c:23:0: drivers/iommu/io-pgtable-arm.c: In function 'arm_lpae_install_table': >> include/linux/atomic.h:475:29: error: implicit declaration of function 'cmpxchg64' [-Werror=implicit-function-declaration] #define cmpxchg64_relaxed cmpxchg64 ^ >> drivers/iommu/io-pgtable-arm.c:337:8: note: in expansion of macro 'cmpxchg64_relaxed' old = cmpxchg64_relaxed(ptep, curr, new); ^~~~~~~~~~~~~~~~~ cc1: some warnings being treated as errors -- In file included from drivers//iommu/io-pgtable-arm.c:23:0: drivers//iommu/io-pgtable-arm.c: In function 'arm_lpae_install_table': >> include/linux/atomic.h:475:29: error: implicit declaration of function 'cmpxchg64' [-Werror=implicit-function-declaration] #define cmpxchg64_relaxed cmpxchg64 ^ drivers//iommu/io-pgtable-arm.c:337:8: note: in expansion of macro 'cmpxchg64_relaxed' old = cmpxchg64_relaxed(ptep, curr, new); ^~~~~~~~~~~~~~~~~ cc1: some warnings being treated as errors vim +/cmpxchg64 +475 include/linux/atomic.h 654672d4 Will Deacon 2015-08-06 469 __atomic_op_fence(cmpxchg, __VA_ARGS__) 654672d4 Will Deacon 2015-08-06 470 #endif 654672d4 Will Deacon 2015-08-06 471 #endif /* cmpxchg_relaxed */ 654672d4 Will Deacon 2015-08-06 472 654672d4 Will Deacon 2015-08-06 473 /* cmpxchg64_relaxed */ 654672d4 Will Deacon 2015-08-06 474 #ifndef cmpxchg64_relaxed 654672d4 Will Deacon 2015-08-06 @475 #define cmpxchg64_relaxed cmpxchg64 654672d4 Will Deacon 2015-08-06 476 #define cmpxchg64_acquire cmpxchg64 654672d4 Will Deacon 2015-08-06 477 #define cmpxchg64_release cmpxchg64 654672d4 Will Deacon 2015-08-06 478 :::::: The code at line 475 was first introduced by commit :::::: 654672d4ba1a6001c365833be895f9477c4d5eab locking/atomics: Add _{acquire|release|relaxed}() variants of some atomic operations :::::: TO: Will Deacon <will.deacon@arm.com> :::::: CC: Ingo Molnar <mingo@kernel.org> --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: iommu/io-pgtable: depend on !GENERIC_ATOMIC64 when using COMPILE_TEST with LPAE 2017-09-25 15:21 ` Will Deacon @ 2017-09-25 15:37 ` Geert Uytterhoeven 2017-09-25 15:53 ` Will Deacon 0 siblings, 1 reply; 4+ messages in thread From: Geert Uytterhoeven @ 2017-09-25 15:37 UTC (permalink / raw) To: Will Deacon; +Cc: Linux Kernel Mailing List, Joerg Roedel, iommu Hi Will, On Mon, Sep 25, 2017 at 5:21 PM, Will Deacon <will.deacon@arm.com> wrote: > On Mon, Sep 25, 2017 at 09:16:22AM +0200, Geert Uytterhoeven wrote: >> On Wed, Jul 12, 2017 at 7:16 PM, Linux Kernel Mailing List >> <linux-kernel@vger.kernel.org> wrote: >> > Web: https://git.kernel.org/torvalds/c/c1004803b40596c1aabbbc78a6b1b33e4dfd96c6 >> > Commit: c1004803b40596c1aabbbc78a6b1b33e4dfd96c6 >> > Parent: 58188afeb727e0f73706f1460707bd3ba6ccc221 >> > Refname: refs/heads/master >> > Author: Will Deacon <will.deacon@arm.com> >> > AuthorDate: Fri Jun 23 11:45:57 2017 +0100 >> > Committer: Will Deacon <will.deacon@arm.com> >> > CommitDate: Fri Jun 23 17:58:02 2017 +0100 >> > >> > iommu/io-pgtable: depend on !GENERIC_ATOMIC64 when using COMPILE_TEST with LPAE >> > >> > The LPAE/ARMv8 page table format relies on the ability to read and write >> > 64-bit page table entries in an atomic fashion. With the move to a lockless >> > implementation, we also need support for cmpxchg64 to resolve races when >> > installing table entries concurrently. >> > >> > Unfortunately, not all architectures support cmpxchg64, so the code can >> > fail to compiler when building for these architectures using COMPILE_TEST. >> > Rather than disable COMPILE_TEST altogether, instead check that >> > GENERIC_ATOMIC64 is not selected, which is a reasonable indication that >> > the architecture has support for 64-bit cmpxchg. >> > >> > Reported-by: kbuild test robot <fengguang.wu@intel.com> >> > Signed-off-by: Will Deacon <will.deacon@arm.com> >> > --- >> > drivers/iommu/Kconfig | 2 +- >> > 1 file changed, 1 insertion(+), 1 deletion(-) >> > >> > diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig >> > index 6ee3a25ae731..c88cfa7522b2 100644 >> > --- a/drivers/iommu/Kconfig >> > +++ b/drivers/iommu/Kconfig >> > @@ -23,7 +23,7 @@ config IOMMU_IO_PGTABLE >> > config IOMMU_IO_PGTABLE_LPAE >> > bool "ARMv7/v8 Long Descriptor Format" >> > select IOMMU_IO_PGTABLE >> > - depends on HAS_DMA && (ARM || ARM64 || COMPILE_TEST) >> > + depends on HAS_DMA && (ARM || ARM64 || (COMPILE_TEST && !GENERIC_ATOMIC64)) >> > help >> > Enable support for the ARM long descriptor pagetable format. >> > This allocator supports 4K/2M/1G, 16K/32M and 64K/512M page >> >> I can't find where this patch was submitted and discussed, so I'm replying >> to this email. On which architectures did it fail to compile? > > It was in response to a report from the kbuild test robot on m32r, but > looking back I now see that it didn't go to the lists for some reason. I've > included the report at the end of this email so you can have a look. Thanks! >> cmpxchg64() is defined by include/asm-generic/cmpxchg.h, so I fail to >> see what's the relation with GENERIC_ATOMIC64, which is related to >> lib/atomic64.c instead. > > Yeah, it's a bit of a hack, but we're basically relying on architectures > that don't select GENERIC_ATOMIC64 providing their own cmpxchg64 > implementation, which seems to be the case. > >> E.g. on m68k, which uses GENERIC_ATOMIC64, it compile-tested fine before. > > FWIW, the lock-based atomics wouldn't be sufficient at runtime, but I > appreciate that we're only talking about COMPILE_TEST here. Correct. >> Perhaps there's another (SMP vs UP?) dependency, as >> include/asm-generic/cmpxchg.h cannot be used on SMP? >> Should it be COMPILE_TEST && (!GENERIC_ATOMIC64 || !SMP)? > > I don't see how that helps. Are you seeing build failures on a non-SMP > arch? I'm not seeing build failures. I see reduced build coverage on m68k due to this change. So m32r fails to provide a definition for cmpxchg64(), which has nothing to do with GENERIC_ATOMIC64. Probably m32r just needs the same treatment as in commit feb20ec2bb859d1d ("m68k: Add missing cmpxchg64() if CONFIG_RMW_INSNS=y") Worse, people are now adding more depends on !GENERIC_ATOMIC64 for various IOMMU drivers, to fix up "selects IOMMU_IO_PGTABLE_LPAE which has unmet direct dependencies" warnings :-( 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: iommu/io-pgtable: depend on !GENERIC_ATOMIC64 when using COMPILE_TEST with LPAE 2017-09-25 15:37 ` Geert Uytterhoeven @ 2017-09-25 15:53 ` Will Deacon 0 siblings, 0 replies; 4+ messages in thread From: Will Deacon @ 2017-09-25 15:53 UTC (permalink / raw) To: Geert Uytterhoeven; +Cc: Linux Kernel Mailing List, Joerg Roedel, iommu, peterz [+PeterZ because he enjoys things like this] On Mon, Sep 25, 2017 at 05:37:46PM +0200, Geert Uytterhoeven wrote: > On Mon, Sep 25, 2017 at 5:21 PM, Will Deacon <will.deacon@arm.com> wrote: > > On Mon, Sep 25, 2017 at 09:16:22AM +0200, Geert Uytterhoeven wrote: > >> On Wed, Jul 12, 2017 at 7:16 PM, Linux Kernel Mailing List > >> <linux-kernel@vger.kernel.org> wrote: > >> > Web: https://git.kernel.org/torvalds/c/c1004803b40596c1aabbbc78a6b1b33e4dfd96c6 > >> > Commit: c1004803b40596c1aabbbc78a6b1b33e4dfd96c6 > >> > Parent: 58188afeb727e0f73706f1460707bd3ba6ccc221 > >> > Refname: refs/heads/master > >> > Author: Will Deacon <will.deacon@arm.com> > >> > AuthorDate: Fri Jun 23 11:45:57 2017 +0100 > >> > Committer: Will Deacon <will.deacon@arm.com> > >> > CommitDate: Fri Jun 23 17:58:02 2017 +0100 > >> > > >> > iommu/io-pgtable: depend on !GENERIC_ATOMIC64 when using COMPILE_TEST with LPAE > >> > > >> > The LPAE/ARMv8 page table format relies on the ability to read and write > >> > 64-bit page table entries in an atomic fashion. With the move to a lockless > >> > implementation, we also need support for cmpxchg64 to resolve races when > >> > installing table entries concurrently. > >> > > >> > Unfortunately, not all architectures support cmpxchg64, so the code can > >> > fail to compiler when building for these architectures using COMPILE_TEST. > >> > Rather than disable COMPILE_TEST altogether, instead check that > >> > GENERIC_ATOMIC64 is not selected, which is a reasonable indication that > >> > the architecture has support for 64-bit cmpxchg. > >> > > >> > Reported-by: kbuild test robot <fengguang.wu@intel.com> > >> > Signed-off-by: Will Deacon <will.deacon@arm.com> > >> > --- > >> > drivers/iommu/Kconfig | 2 +- > >> > 1 file changed, 1 insertion(+), 1 deletion(-) > >> > > >> > diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig > >> > index 6ee3a25ae731..c88cfa7522b2 100644 > >> > --- a/drivers/iommu/Kconfig > >> > +++ b/drivers/iommu/Kconfig > >> > @@ -23,7 +23,7 @@ config IOMMU_IO_PGTABLE > >> > config IOMMU_IO_PGTABLE_LPAE > >> > bool "ARMv7/v8 Long Descriptor Format" > >> > select IOMMU_IO_PGTABLE > >> > - depends on HAS_DMA && (ARM || ARM64 || COMPILE_TEST) > >> > + depends on HAS_DMA && (ARM || ARM64 || (COMPILE_TEST && !GENERIC_ATOMIC64)) > >> > help > >> > Enable support for the ARM long descriptor pagetable format. > >> > This allocator supports 4K/2M/1G, 16K/32M and 64K/512M page > >> > >> I can't find where this patch was submitted and discussed, so I'm replying > >> to this email. On which architectures did it fail to compile? > > > > It was in response to a report from the kbuild test robot on m32r, but > > looking back I now see that it didn't go to the lists for some reason. I've > > included the report at the end of this email so you can have a look. > > Thanks! > > >> cmpxchg64() is defined by include/asm-generic/cmpxchg.h, so I fail to > >> see what's the relation with GENERIC_ATOMIC64, which is related to > >> lib/atomic64.c instead. > > > > Yeah, it's a bit of a hack, but we're basically relying on architectures > > that don't select GENERIC_ATOMIC64 providing their own cmpxchg64 > > implementation, which seems to be the case. > > > >> E.g. on m68k, which uses GENERIC_ATOMIC64, it compile-tested fine before. > > > > FWIW, the lock-based atomics wouldn't be sufficient at runtime, but I > > appreciate that we're only talking about COMPILE_TEST here. > > Correct. > > >> Perhaps there's another (SMP vs UP?) dependency, as > >> include/asm-generic/cmpxchg.h cannot be used on SMP? > >> Should it be COMPILE_TEST && (!GENERIC_ATOMIC64 || !SMP)? > > > > I don't see how that helps. Are you seeing build failures on a non-SMP > > arch? > > I'm not seeing build failures. > I see reduced build coverage on m68k due to this change. > > So m32r fails to provide a definition for cmpxchg64(), which has nothing to > do with GENERIC_ATOMIC64. > Probably m32r just needs the same treatment as in commit feb20ec2bb859d1d > ("m68k: Add missing cmpxchg64() if CONFIG_RMW_INSNS=y") I have no idea, tbh. m32r claims to be SMP in some cases but it's orphaned so I don't know who we could ask about this. If we ended up with a lock-based 64-bit cmpxchg but a native 8/16/32-bit cmpxchg then we're asking for trouble because they don't interwork. > Worse, people are now adding more depends on !GENERIC_ATOMIC64 for > various IOMMU drivers, to fix up "selects IOMMU_IO_PGTABLE_LPAE which > has unmet direct dependencies" warnings :-( Well I'd certainly be willing to revisit the dependency if all architectures provided cmpxchg64, but that's not something that I'm comfortable implementing without the ability to test it since it could actually cause runtime breakage just for the sake of getting a driver building that will never be used on those architectures. I didn't want to drop the || COMPILE_TEST completely because we get useful coverage from it on x86. Will ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-09-25 15:53 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20170712171654.35DA922D7E@pdx-korg-gitolite-1.ci.codeaurora.org>
2017-09-25 7:16 ` iommu/io-pgtable: depend on !GENERIC_ATOMIC64 when using COMPILE_TEST with LPAE Geert Uytterhoeven
2017-09-25 15:21 ` Will Deacon
2017-09-25 15:37 ` Geert Uytterhoeven
2017-09-25 15:53 ` Will Deacon
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
Powered by JetHome