* Re: [PATCH 1/2] audit: add support for the openat2 syscall
From: Christian Brauner @ 2021-03-18 10:52 UTC (permalink / raw)
To: Richard Guy Briggs
Cc: linux-s390, linux-ia64, Paul Moore, linux-parisc, x86, LKML,
Eric Paris, linux-fsdevel, Aleksa Sarai, Linux-Audit Mailing List,
Alexander Viro, linux-alpha, sparclinux, Eric Paris, Steve Grubb,
linuxppc-dev
In-Reply-To: <20210318104843.uiga6tmmhn5wfhbs@wittgenstein>
On Thu, Mar 18, 2021 at 11:48:45AM +0100, Christian Brauner wrote:
> [+Cc Aleksa, the author of openat2()]
>
> and a comment below. :)
>
> On Wed, Mar 17, 2021 at 09:47:17PM -0400, Richard Guy Briggs wrote:
> > The openat2(2) syscall was added in kernel v5.6 with commit fddb5d430ad9
> > ("open: introduce openat2(2) syscall")
> >
> > Add the openat2(2) syscall to the audit syscall classifier.
> >
> > See the github issue
> > https://github.com/linux-audit/audit-kernel/issues/67
> >
> > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > ---
> > arch/alpha/kernel/audit.c | 2 ++
> > arch/ia64/kernel/audit.c | 2 ++
> > arch/parisc/kernel/audit.c | 2 ++
> > arch/parisc/kernel/compat_audit.c | 2 ++
> > arch/powerpc/kernel/audit.c | 2 ++
> > arch/powerpc/kernel/compat_audit.c | 2 ++
> > arch/s390/kernel/audit.c | 2 ++
> > arch/s390/kernel/compat_audit.c | 2 ++
> > arch/sparc/kernel/audit.c | 2 ++
> > arch/sparc/kernel/compat_audit.c | 2 ++
> > arch/x86/ia32/audit.c | 2 ++
> > arch/x86/kernel/audit_64.c | 2 ++
> > kernel/auditsc.c | 3 +++
> > lib/audit.c | 4 ++++
> > lib/compat_audit.c | 4 ++++
> > 15 files changed, 35 insertions(+)
> >
> > diff --git a/arch/alpha/kernel/audit.c b/arch/alpha/kernel/audit.c
> > index 96a9d18ff4c4..06a911b685d1 100644
> > --- a/arch/alpha/kernel/audit.c
> > +++ b/arch/alpha/kernel/audit.c
> > @@ -42,6 +42,8 @@ int audit_classify_syscall(int abi, unsigned syscall)
> > return 3;
> > case __NR_execve:
> > return 5;
> > + case __NR_openat2:
> > + return 6;
> > default:
> > return 0;
> > }
> > diff --git a/arch/ia64/kernel/audit.c b/arch/ia64/kernel/audit.c
> > index 5192ca899fe6..5eaa888c8fd3 100644
> > --- a/arch/ia64/kernel/audit.c
> > +++ b/arch/ia64/kernel/audit.c
> > @@ -43,6 +43,8 @@ int audit_classify_syscall(int abi, unsigned syscall)
> > return 3;
> > case __NR_execve:
> > return 5;
> > + case __NR_openat2:
> > + return 6;
> > default:
> > return 0;
> > }
> > diff --git a/arch/parisc/kernel/audit.c b/arch/parisc/kernel/audit.c
> > index 9eb47b2225d2..fc721a7727ba 100644
> > --- a/arch/parisc/kernel/audit.c
> > +++ b/arch/parisc/kernel/audit.c
> > @@ -52,6 +52,8 @@ int audit_classify_syscall(int abi, unsigned syscall)
> > return 3;
> > case __NR_execve:
> > return 5;
> > + case __NR_openat2:
> > + return 6;
> > default:
> > return 0;
> > }
> > diff --git a/arch/parisc/kernel/compat_audit.c b/arch/parisc/kernel/compat_audit.c
> > index 20c39c9d86a9..fc6d35918c44 100644
> > --- a/arch/parisc/kernel/compat_audit.c
> > +++ b/arch/parisc/kernel/compat_audit.c
> > @@ -35,6 +35,8 @@ int parisc32_classify_syscall(unsigned syscall)
> > return 3;
> > case __NR_execve:
> > return 5;
> > + case __NR_openat2:
> > + return 6;
> > default:
> > return 1;
> > }
> > diff --git a/arch/powerpc/kernel/audit.c b/arch/powerpc/kernel/audit.c
> > index a2dddd7f3d09..8f32700b0baa 100644
> > --- a/arch/powerpc/kernel/audit.c
> > +++ b/arch/powerpc/kernel/audit.c
> > @@ -54,6 +54,8 @@ int audit_classify_syscall(int abi, unsigned syscall)
> > return 4;
> > case __NR_execve:
> > return 5;
> > + case __NR_openat2:
> > + return 6;
> > default:
> > return 0;
> > }
> > diff --git a/arch/powerpc/kernel/compat_audit.c b/arch/powerpc/kernel/compat_audit.c
> > index 55c6ccda0a85..ebe45534b1c9 100644
> > --- a/arch/powerpc/kernel/compat_audit.c
> > +++ b/arch/powerpc/kernel/compat_audit.c
> > @@ -38,6 +38,8 @@ int ppc32_classify_syscall(unsigned syscall)
> > return 4;
> > case __NR_execve:
> > return 5;
> > + case __NR_openat2:
> > + return 6;
> > default:
> > return 1;
> > }
> > diff --git a/arch/s390/kernel/audit.c b/arch/s390/kernel/audit.c
> > index d395c6c9944c..d964cb94cfaf 100644
> > --- a/arch/s390/kernel/audit.c
> > +++ b/arch/s390/kernel/audit.c
> > @@ -54,6 +54,8 @@ int audit_classify_syscall(int abi, unsigned syscall)
> > return 4;
> > case __NR_execve:
> > return 5;
> > + case __NR_openat2:
> > + return 6;
> > default:
> > return 0;
> > }
> > diff --git a/arch/s390/kernel/compat_audit.c b/arch/s390/kernel/compat_audit.c
> > index 444fb1f66944..f7b32933ce0e 100644
> > --- a/arch/s390/kernel/compat_audit.c
> > +++ b/arch/s390/kernel/compat_audit.c
> > @@ -39,6 +39,8 @@ int s390_classify_syscall(unsigned syscall)
> > return 4;
> > case __NR_execve:
> > return 5;
> > + case __NR_openat2:
> > + return 6;
> > default:
> > return 1;
> > }
> > diff --git a/arch/sparc/kernel/audit.c b/arch/sparc/kernel/audit.c
> > index a6e91bf34d48..b6dcca9c6520 100644
> > --- a/arch/sparc/kernel/audit.c
> > +++ b/arch/sparc/kernel/audit.c
> > @@ -55,6 +55,8 @@ int audit_classify_syscall(int abi, unsigned int syscall)
> > return 4;
> > case __NR_execve:
> > return 5;
> > + case __NR_openat2:
> > + return 6;
> > default:
> > return 0;
> > }
> > diff --git a/arch/sparc/kernel/compat_audit.c b/arch/sparc/kernel/compat_audit.c
> > index 10eeb4f15b20..d2652a1083ad 100644
> > --- a/arch/sparc/kernel/compat_audit.c
> > +++ b/arch/sparc/kernel/compat_audit.c
> > @@ -39,6 +39,8 @@ int sparc32_classify_syscall(unsigned int syscall)
> > return 4;
> > case __NR_execve:
> > return 5;
> > + case __NR_openat2:
> > + return 6;
> > default:
> > return 1;
> > }
> > diff --git a/arch/x86/ia32/audit.c b/arch/x86/ia32/audit.c
> > index 6efe6cb3768a..57a02ade5503 100644
> > --- a/arch/x86/ia32/audit.c
> > +++ b/arch/x86/ia32/audit.c
> > @@ -39,6 +39,8 @@ int ia32_classify_syscall(unsigned syscall)
> > case __NR_execve:
> > case __NR_execveat:
> > return 5;
> > + case __NR_openat2:
> > + return 6;
> > default:
> > return 1;
> > }
> > diff --git a/arch/x86/kernel/audit_64.c b/arch/x86/kernel/audit_64.c
> > index 83d9cad4e68b..39de1e021258 100644
> > --- a/arch/x86/kernel/audit_64.c
> > +++ b/arch/x86/kernel/audit_64.c
> > @@ -53,6 +53,8 @@ int audit_classify_syscall(int abi, unsigned syscall)
> > case __NR_execve:
> > case __NR_execveat:
> > return 5;
> > + case __NR_openat2:
> > + return 6;
> > default:
> > return 0;
> > }
> > diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> > index 8bb9ac84d2fb..f5616e70d129 100644
> > --- a/kernel/auditsc.c
> > +++ b/kernel/auditsc.c
> > @@ -76,6 +76,7 @@
> > #include <linux/fsnotify_backend.h>
> > #include <uapi/linux/limits.h>
> > #include <uapi/linux/netfilter/nf_tables.h>
> > +#include <uapi/linux/openat2.h>
> >
> > #include "audit.h"
> >
> > @@ -195,6 +196,8 @@ static int audit_match_perm(struct audit_context *ctx, int mask)
> > return ((mask & AUDIT_PERM_WRITE) && ctx->argv[0] == SYS_BIND);
> > case 5: /* execve */
> > return mask & AUDIT_PERM_EXEC;
> > + case 6: /* openat2 */
> > + return mask & ACC_MODE((u32)((struct open_how *)ctx->argv[2])->flags);
>
> That looks a bit dodgy. Maybe sm like the below would be a bit better?
>
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 47fb48f42c93..531e882a5096 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -159,6 +159,7 @@ static const struct audit_nfcfgop_tab audit_nfcfgs[] = {
>
> static int audit_match_perm(struct audit_context *ctx, int mask)
> {
> + struct open_how *openat2;
> unsigned n;
> if (unlikely(!ctx))
> return 0;
> @@ -195,6 +196,12 @@ static int audit_match_perm(struct audit_context *ctx, int mask)
> return ((mask & AUDIT_PERM_WRITE) && ctx->argv[0] == SYS_BIND);
> case 5: /* execve */
> return mask & AUDIT_PERM_EXEC;
> + case 6: /* openat2 */
> + openat2 = ctx->argv[2];
> + if (upper_32_bits(openat2->flags))
> + pr_warn("Some sensible warning about unknown flags");
> +
> + return mask & ACC_MODE(lower_32_bits(openat2->flags));
> default:
> return 0;
> }
>
> (Ideally we'd probably notice at build-time that we've got flags
> exceeding 32bits. Could probably easily been done by exposing an all
> flags macro somewhere and then we can place a BUILD_BUG_ON() or sm into
> such places.)
And one more comment, why return a hard-coded integer from all of these
architectures instead of introducing an enum in a central place with
proper names idk:
enum audit_match_perm_t {
.
.
.
AUDIT_MATCH_PERM_EXECVE = 5,
AUDIT_MATCH_PERM_OPENAT2 = 6,
.
.
.
}
Then you can drop these hard-coded comments too and it's way less
brittle overall.
Christian
^ permalink raw reply
* Re: [PATCH 1/2] audit: add support for the openat2 syscall
From: Richard Guy Briggs @ 2021-03-18 12:03 UTC (permalink / raw)
To: Christian Brauner
Cc: linux-s390, linux-ia64, Paul Moore, linux-parisc, x86, LKML,
Eric Paris, linux-fsdevel, Aleksa Sarai, Linux-Audit Mailing List,
Alexander Viro, linux-alpha, sparclinux, Eric Paris, Steve Grubb,
linuxppc-dev
In-Reply-To: <20210318105230.4ggpg5r3clloa6br@wittgenstein>
[-- Attachment #1: Type: text/plain, Size: 1819 bytes --]
On 2021-03-18 11:52, Christian Brauner wrote:
> On Thu, Mar 18, 2021 at 11:48:45AM +0100, Christian Brauner wrote:
> > On Wed, Mar 17, 2021 at 09:47:17PM -0400, Richard Guy Briggs wrote:
> > > The openat2(2) syscall was added in kernel v5.6 with commit fddb5d430ad9
> > > ("open: introduce openat2(2) syscall")
> > > Add the openat2(2) syscall to the audit syscall classifier.
> > > See the github issue
> > > https://github.com/linux-audit/audit-kernel/issues/67
> > > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
...
> And one more comment, why return a hard-coded integer from all of these
> architectures instead of introducing an enum in a central place with
> proper names idk:
Oh, believe me, I tried hard to do that because I really don't like
hard-coded magic values, but for expediency I continued the same
approach until I could sort out the header file mess. There was an
extra preparatory patch (attached) in this patchset with a different
audit syscall perms patch (also attached). By including "#include
<linux/audit.h>" in each of the compat source files there were warnings
of redefinitions of every __NR_* syscall number. The easiest way to get
rid of it would have been to pull the new AUDITSC_* definitions into a
new <linux/auditsc.h> file and include that from <linux/audit.h> and
each of the arch/*/*/*audit.c (and lib/*audit.c) files.
> enum audit_match_perm_t {
> .
> .
> .
> AUDIT_MATCH_PERM_EXECVE = 5,
> AUDIT_MATCH_PERM_OPENAT2 = 6,
> .
> .
> .
> }
>
> Then you can drop these hard-coded comments too and it's way less
> brittle overall.
Totally agree.
> Christian
- RGB
--
Richard Guy Briggs <rgb@redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635
[-- Attachment #2: 0001-audit-replace-magic-audit-syscall-class-numbers-with.patch --]
[-- Type: text/plain, Size: 12190 bytes --]
From 599ae48091296a3ad3eb4259e7af39cdf0f743c7 Mon Sep 17 00:00:00 2001
Message-Id: <599ae48091296a3ad3eb4259e7af39cdf0f743c7.1616067847.git.rgb@redhat.com>
In-Reply-To: <cover.1616067847.git.rgb@redhat.com>
References: <cover.1616067847.git.rgb@redhat.com>
From: Richard Guy Briggs <rgb@redhat.com>
Date: Fri, 22 Jan 2021 16:27:42 -0500
Subject: [PATCH 1/3] audit: replace magic audit syscall class numbers with
macros
Replace the magic numbers used to indicate audit syscall classes with macros.
Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
---
arch/alpha/kernel/audit.c | 8 ++++----
arch/ia64/kernel/audit.c | 8 ++++----
arch/parisc/kernel/audit.c | 8 ++++----
arch/parisc/kernel/compat_audit.c | 9 +++++----
arch/powerpc/kernel/audit.c | 10 +++++-----
arch/powerpc/kernel/compat_audit.c | 11 ++++++-----
arch/s390/kernel/audit.c | 10 +++++-----
arch/s390/kernel/compat_audit.c | 11 ++++++-----
arch/sparc/kernel/audit.c | 10 +++++-----
arch/sparc/kernel/compat_audit.c | 11 ++++++-----
arch/x86/ia32/audit.c | 11 ++++++-----
arch/x86/kernel/audit_64.c | 8 ++++----
include/linux/audit.h | 7 +++++++
kernel/auditsc.c | 12 ++++++------
lib/audit.c | 10 +++++-----
lib/compat_audit.c | 11 ++++++-----
16 files changed, 84 insertions(+), 71 deletions(-)
diff --git a/arch/alpha/kernel/audit.c b/arch/alpha/kernel/audit.c
index 96a9d18ff4c4..81cbd804e375 100644
--- a/arch/alpha/kernel/audit.c
+++ b/arch/alpha/kernel/audit.c
@@ -37,13 +37,13 @@ int audit_classify_syscall(int abi, unsigned syscall)
{
switch(syscall) {
case __NR_open:
- return 2;
+ return AUDITSC_OPEN;
case __NR_openat:
- return 3;
+ return AUDITSC_OPENAT;
case __NR_execve:
- return 5;
+ return AUDITSC_EXECVE;
default:
- return 0;
+ return AUDITSC_NATIVE;
}
}
diff --git a/arch/ia64/kernel/audit.c b/arch/ia64/kernel/audit.c
index 5192ca899fe6..dba6a74c9ab3 100644
--- a/arch/ia64/kernel/audit.c
+++ b/arch/ia64/kernel/audit.c
@@ -38,13 +38,13 @@ int audit_classify_syscall(int abi, unsigned syscall)
{
switch(syscall) {
case __NR_open:
- return 2;
+ return AUDITSC_OPEN;
case __NR_openat:
- return 3;
+ return AUDITSC_OPENAT;
case __NR_execve:
- return 5;
+ return AUDITSC_EXECVE;
default:
- return 0;
+ return AUDITSC_NATIVE;
}
}
diff --git a/arch/parisc/kernel/audit.c b/arch/parisc/kernel/audit.c
index 9eb47b2225d2..14244e83db75 100644
--- a/arch/parisc/kernel/audit.c
+++ b/arch/parisc/kernel/audit.c
@@ -47,13 +47,13 @@ int audit_classify_syscall(int abi, unsigned syscall)
#endif
switch (syscall) {
case __NR_open:
- return 2;
+ return AUDITSC_OPEN;
case __NR_openat:
- return 3;
+ return AUDITSC_OPENAT;
case __NR_execve:
- return 5;
+ return AUDITSC_EXECVE;
default:
- return 0;
+ return AUDITSC_NATIVE;
}
}
diff --git a/arch/parisc/kernel/compat_audit.c b/arch/parisc/kernel/compat_audit.c
index 20c39c9d86a9..68102807aba7 100644
--- a/arch/parisc/kernel/compat_audit.c
+++ b/arch/parisc/kernel/compat_audit.c
@@ -1,4 +1,5 @@
// SPDX-License-Identifier: GPL-2.0
+#include <linux/audit.h>
#include <asm/unistd.h>
unsigned int parisc32_dir_class[] = {
@@ -30,12 +31,12 @@ int parisc32_classify_syscall(unsigned syscall)
{
switch (syscall) {
case __NR_open:
- return 2;
+ return AUDITSC_OPEN;
case __NR_openat:
- return 3;
+ return AUDITSC_OPENAT;
case __NR_execve:
- return 5;
+ return AUDITSC_EXECVE;
default:
- return 1;
+ return AUDITSC_COMPAT;
}
}
diff --git a/arch/powerpc/kernel/audit.c b/arch/powerpc/kernel/audit.c
index a2dddd7f3d09..6eb18ef77dff 100644
--- a/arch/powerpc/kernel/audit.c
+++ b/arch/powerpc/kernel/audit.c
@@ -47,15 +47,15 @@ int audit_classify_syscall(int abi, unsigned syscall)
#endif
switch(syscall) {
case __NR_open:
- return 2;
+ return AUDITSC_OPEN;
case __NR_openat:
- return 3;
+ return AUDITSC_OPENAT;
case __NR_socketcall:
- return 4;
+ return AUDITSC_SOCKETCALL;
case __NR_execve:
- return 5;
+ return AUDITSC_EXECVE;
default:
- return 0;
+ return AUDITSC_NATIVE;
}
}
diff --git a/arch/powerpc/kernel/compat_audit.c b/arch/powerpc/kernel/compat_audit.c
index 55c6ccda0a85..b3fd2d43bfff 100644
--- a/arch/powerpc/kernel/compat_audit.c
+++ b/arch/powerpc/kernel/compat_audit.c
@@ -1,5 +1,6 @@
// SPDX-License-Identifier: GPL-2.0
#undef __powerpc64__
+#include <linux/audit.h>
#include <asm/unistd.h>
unsigned ppc32_dir_class[] = {
@@ -31,14 +32,14 @@ int ppc32_classify_syscall(unsigned syscall)
{
switch(syscall) {
case __NR_open:
- return 2;
+ return AUDITSC_OPEN;
case __NR_openat:
- return 3;
+ return AUDITSC_OPENAT;
case __NR_socketcall:
- return 4;
+ return AUDITSC_SOCKETCALL;
case __NR_execve:
- return 5;
+ return AUDITSC_EXECVE;
default:
- return 1;
+ return AUDITSC_COMPAT;
}
}
diff --git a/arch/s390/kernel/audit.c b/arch/s390/kernel/audit.c
index d395c6c9944c..7e331e1831d4 100644
--- a/arch/s390/kernel/audit.c
+++ b/arch/s390/kernel/audit.c
@@ -47,15 +47,15 @@ int audit_classify_syscall(int abi, unsigned syscall)
#endif
switch(syscall) {
case __NR_open:
- return 2;
+ return AUDITSC_OPEN;
case __NR_openat:
- return 3;
+ return AUDITSC_OPENAT;
case __NR_socketcall:
- return 4;
+ return AUDITSC_SOCKETCALL;
case __NR_execve:
- return 5;
+ return AUDITSC_EXECVE;
default:
- return 0;
+ return AUDITSC_NATIVE;
}
}
diff --git a/arch/s390/kernel/compat_audit.c b/arch/s390/kernel/compat_audit.c
index 444fb1f66944..704d04cfd9dd 100644
--- a/arch/s390/kernel/compat_audit.c
+++ b/arch/s390/kernel/compat_audit.c
@@ -1,5 +1,6 @@
// SPDX-License-Identifier: GPL-2.0
#undef __s390x__
+#include <linux/audit.h>
#include <asm/unistd.h>
#include "audit.h"
@@ -32,14 +33,14 @@ int s390_classify_syscall(unsigned syscall)
{
switch(syscall) {
case __NR_open:
- return 2;
+ return AUDITSC_OPEN;
case __NR_openat:
- return 3;
+ return AUDITSC_OPENAT;
case __NR_socketcall:
- return 4;
+ return AUDITSC_SOCKETCALL;
case __NR_execve:
- return 5;
+ return AUDITSC_EXECVE;
default:
- return 1;
+ return AUDITSC_COMPAT;
}
}
diff --git a/arch/sparc/kernel/audit.c b/arch/sparc/kernel/audit.c
index a6e91bf34d48..50fab35bdaba 100644
--- a/arch/sparc/kernel/audit.c
+++ b/arch/sparc/kernel/audit.c
@@ -48,15 +48,15 @@ int audit_classify_syscall(int abi, unsigned int syscall)
#endif
switch(syscall) {
case __NR_open:
- return 2;
+ return AUDITSC_OPEN;
case __NR_openat:
- return 3;
+ return AUDITSC_OPENAT;
case __NR_socketcall:
- return 4;
+ return AUDITSC_SOCKETCALL;
case __NR_execve:
- return 5;
+ return AUDITSC_EXECVE;
default:
- return 0;
+ return AUDITSC_NATIVE;
}
}
diff --git a/arch/sparc/kernel/compat_audit.c b/arch/sparc/kernel/compat_audit.c
index 10eeb4f15b20..4c2f9a4ee845 100644
--- a/arch/sparc/kernel/compat_audit.c
+++ b/arch/sparc/kernel/compat_audit.c
@@ -1,5 +1,6 @@
// SPDX-License-Identifier: GPL-2.0
#define __32bit_syscall_numbers__
+#include <linux/audit.h>
#include <asm/unistd.h>
#include "kernel.h"
@@ -32,14 +33,14 @@ int sparc32_classify_syscall(unsigned int syscall)
{
switch(syscall) {
case __NR_open:
- return 2;
+ return AUDITSC_OPEN;
case __NR_openat:
- return 3;
+ return AUDITSC_OPENAT;
case __NR_socketcall:
- return 4;
+ return AUDITSC_SOCKETCALL;
case __NR_execve:
- return 5;
+ return AUDITSC_EXECVE;
default:
- return 1;
+ return AUDITSC_COMPAT;
}
}
diff --git a/arch/x86/ia32/audit.c b/arch/x86/ia32/audit.c
index 6efe6cb3768a..0798a6b66314 100644
--- a/arch/x86/ia32/audit.c
+++ b/arch/x86/ia32/audit.c
@@ -1,4 +1,5 @@
// SPDX-License-Identifier: GPL-2.0
+#include <linux/audit.h>
#include <asm/unistd_32.h>
#include <asm/audit.h>
@@ -31,15 +32,15 @@ int ia32_classify_syscall(unsigned syscall)
{
switch (syscall) {
case __NR_open:
- return 2;
+ return AUDITSC_OPEN;
case __NR_openat:
- return 3;
+ return AUDITSC_OPENAT;
case __NR_socketcall:
- return 4;
+ return AUDITSC_SOCKETCALL;
case __NR_execve:
case __NR_execveat:
- return 5;
+ return AUDITSC_EXECVE;
default:
- return 1;
+ return AUDITSC_COMPAT;
}
}
diff --git a/arch/x86/kernel/audit_64.c b/arch/x86/kernel/audit_64.c
index 83d9cad4e68b..2a6cc9c9c881 100644
--- a/arch/x86/kernel/audit_64.c
+++ b/arch/x86/kernel/audit_64.c
@@ -47,14 +47,14 @@ int audit_classify_syscall(int abi, unsigned syscall)
#endif
switch(syscall) {
case __NR_open:
- return 2;
+ return AUDITSC_OPEN;
case __NR_openat:
- return 3;
+ return AUDITSC_OPENAT;
case __NR_execve:
case __NR_execveat:
- return 5;
+ return AUDITSC_EXECVE;
default:
- return 0;
+ return AUDITSC_NATIVE;
}
}
diff --git a/include/linux/audit.h b/include/linux/audit.h
index 82b7c1116a85..bcf0150b1528 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -120,6 +120,13 @@ enum audit_nfcfgop {
extern int is_audit_feature_set(int which);
+#define AUDITSC_NATIVE 0
+#define AUDITSC_COMPAT 1
+#define AUDITSC_OPEN 2
+#define AUDITSC_OPENAT 3
+#define AUDITSC_SOCKETCALL 4
+#define AUDITSC_EXECVE 5
+
extern int __init audit_register_class(int class, unsigned *list);
extern int audit_classify_syscall(int abi, unsigned syscall);
extern int audit_classify_arch(int arch);
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 71ead2969eeb..dddea985f23e 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -165,7 +165,7 @@ static int audit_match_perm(struct audit_context *ctx, int mask)
n = ctx->major;
switch (audit_classify_syscall(ctx->arch, n)) {
- case 0: /* native */
+ case AUDITSC_NATIVE:
if ((mask & AUDIT_PERM_WRITE) &&
audit_match_class(AUDIT_CLASS_WRITE, n))
return 1;
@@ -176,7 +176,7 @@ static int audit_match_perm(struct audit_context *ctx, int mask)
audit_match_class(AUDIT_CLASS_CHATTR, n))
return 1;
return 0;
- case 1: /* 32bit on biarch */
+ case AUDITSC_COMPAT: /* 32bit on biarch */
if ((mask & AUDIT_PERM_WRITE) &&
audit_match_class(AUDIT_CLASS_WRITE_32, n))
return 1;
@@ -187,13 +187,13 @@ static int audit_match_perm(struct audit_context *ctx, int mask)
audit_match_class(AUDIT_CLASS_CHATTR_32, n))
return 1;
return 0;
- case 2: /* open */
+ case AUDITSC_OPEN:
return mask & ACC_MODE(ctx->argv[1]);
- case 3: /* openat */
+ case AUDITSC_OPENAT:
return mask & ACC_MODE(ctx->argv[2]);
- case 4: /* socketcall */
+ case AUDITSC_SOCKETCALL:
return ((mask & AUDIT_PERM_WRITE) && ctx->argv[0] == SYS_BIND);
- case 5: /* execve */
+ case AUDITSC_EXECVE:
return mask & AUDIT_PERM_EXEC;
default:
return 0;
diff --git a/lib/audit.c b/lib/audit.c
index 5004bff928a7..3ec1a94d8d64 100644
--- a/lib/audit.c
+++ b/lib/audit.c
@@ -45,23 +45,23 @@ int audit_classify_syscall(int abi, unsigned syscall)
switch(syscall) {
#ifdef __NR_open
case __NR_open:
- return 2;
+ return AUDITSC_OPEN;
#endif
#ifdef __NR_openat
case __NR_openat:
- return 3;
+ return AUDITSC_OPENAT;
#endif
#ifdef __NR_socketcall
case __NR_socketcall:
- return 4;
+ return AUDITSC_SOCKETCALL;
#endif
#ifdef __NR_execveat
case __NR_execveat:
#endif
case __NR_execve:
- return 5;
+ return AUDITSC_EXECVE;
default:
- return 0;
+ return AUDITSC_NATIVE;
}
}
diff --git a/lib/compat_audit.c b/lib/compat_audit.c
index 77eabad69b4a..528dafa2c2bb 100644
--- a/lib/compat_audit.c
+++ b/lib/compat_audit.c
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: GPL-2.0
#include <linux/init.h>
#include <linux/types.h>
+#include <linux/audit.h>
#include <asm/unistd32.h>
unsigned compat_dir_class[] = {
@@ -33,19 +34,19 @@ int audit_classify_compat_syscall(int abi, unsigned syscall)
switch (syscall) {
#ifdef __NR_open
case __NR_open:
- return 2;
+ return AUDITSC_OPEN;
#endif
#ifdef __NR_openat
case __NR_openat:
- return 3;
+ return AUDITSC_OPENAT;
#endif
#ifdef __NR_socketcall
case __NR_socketcall:
- return 4;
+ return AUDITSC_SOCKETCALL;
#endif
case __NR_execve:
- return 5;
+ return AUDITSC_EXECVE;
default:
- return 1;
+ return AUDITSC_COMPAT;
}
}
--
2.27.0
[-- Attachment #3: 0002-audit-add-support-for-the-openat2-syscall.patch --]
[-- Type: text/plain, Size: 8018 bytes --]
From cfd217b99e6e2646e0740b2ddead4c56ba394509 Mon Sep 17 00:00:00 2001
Message-Id: <cfd217b99e6e2646e0740b2ddead4c56ba394509.1616067847.git.rgb@redhat.com>
In-Reply-To: <cover.1616067847.git.rgb@redhat.com>
References: <cover.1616067847.git.rgb@redhat.com>
From: Richard Guy Briggs <rgb@redhat.com>
Date: Fri, 22 Jan 2021 13:48:17 -0500
Subject: [PATCH 2/3] audit: add support for the openat2 syscall
Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
---
arch/alpha/kernel/audit.c | 2 ++
arch/ia64/kernel/audit.c | 2 ++
arch/parisc/kernel/audit.c | 2 ++
arch/parisc/kernel/compat_audit.c | 2 ++
arch/powerpc/kernel/audit.c | 2 ++
arch/powerpc/kernel/compat_audit.c | 2 ++
arch/s390/kernel/audit.c | 2 ++
arch/s390/kernel/compat_audit.c | 2 ++
arch/sparc/kernel/audit.c | 2 ++
arch/sparc/kernel/compat_audit.c | 2 ++
arch/x86/ia32/audit.c | 2 ++
arch/x86/kernel/audit_64.c | 2 ++
include/linux/audit.h | 1 +
kernel/auditsc.c | 3 +++
lib/audit.c | 4 ++++
lib/compat_audit.c | 4 ++++
16 files changed, 36 insertions(+)
diff --git a/arch/alpha/kernel/audit.c b/arch/alpha/kernel/audit.c
index 81cbd804e375..3ab04709784a 100644
--- a/arch/alpha/kernel/audit.c
+++ b/arch/alpha/kernel/audit.c
@@ -42,6 +42,8 @@ int audit_classify_syscall(int abi, unsigned syscall)
return AUDITSC_OPENAT;
case __NR_execve:
return AUDITSC_EXECVE;
+ case __NR_openat2:
+ return AUDITSC_OPENAT2;
default:
return AUDITSC_NATIVE;
}
diff --git a/arch/ia64/kernel/audit.c b/arch/ia64/kernel/audit.c
index dba6a74c9ab3..ec61f20ca61f 100644
--- a/arch/ia64/kernel/audit.c
+++ b/arch/ia64/kernel/audit.c
@@ -43,6 +43,8 @@ int audit_classify_syscall(int abi, unsigned syscall)
return AUDITSC_OPENAT;
case __NR_execve:
return AUDITSC_EXECVE;
+ case __NR_openat2:
+ return AUDITSC_OPENAT2;
default:
return AUDITSC_NATIVE;
}
diff --git a/arch/parisc/kernel/audit.c b/arch/parisc/kernel/audit.c
index 14244e83db75..f420b5552140 100644
--- a/arch/parisc/kernel/audit.c
+++ b/arch/parisc/kernel/audit.c
@@ -52,6 +52,8 @@ int audit_classify_syscall(int abi, unsigned syscall)
return AUDITSC_OPENAT;
case __NR_execve:
return AUDITSC_EXECVE;
+ case __NR_openat2:
+ return AUDITSC_OPENAT2;
default:
return AUDITSC_NATIVE;
}
diff --git a/arch/parisc/kernel/compat_audit.c b/arch/parisc/kernel/compat_audit.c
index 68102807aba7..139b7f736b67 100644
--- a/arch/parisc/kernel/compat_audit.c
+++ b/arch/parisc/kernel/compat_audit.c
@@ -36,6 +36,8 @@ int parisc32_classify_syscall(unsigned syscall)
return AUDITSC_OPENAT;
case __NR_execve:
return AUDITSC_EXECVE;
+ case __NR_openat2:
+ return AUDITSC_OPENAT2;
default:
return AUDITSC_COMPAT;
}
diff --git a/arch/powerpc/kernel/audit.c b/arch/powerpc/kernel/audit.c
index 6eb18ef77dff..1bcfca5fdf67 100644
--- a/arch/powerpc/kernel/audit.c
+++ b/arch/powerpc/kernel/audit.c
@@ -54,6 +54,8 @@ int audit_classify_syscall(int abi, unsigned syscall)
return AUDITSC_SOCKETCALL;
case __NR_execve:
return AUDITSC_EXECVE;
+ case __NR_openat2:
+ return AUDITSC_OPENAT2;
default:
return AUDITSC_NATIVE;
}
diff --git a/arch/powerpc/kernel/compat_audit.c b/arch/powerpc/kernel/compat_audit.c
index b3fd2d43bfff..a702374377d7 100644
--- a/arch/powerpc/kernel/compat_audit.c
+++ b/arch/powerpc/kernel/compat_audit.c
@@ -39,6 +39,8 @@ int ppc32_classify_syscall(unsigned syscall)
return AUDITSC_SOCKETCALL;
case __NR_execve:
return AUDITSC_EXECVE;
+ case __NR_openat2:
+ return AUDITSC_OPENAT2;
default:
return AUDITSC_COMPAT;
}
diff --git a/arch/s390/kernel/audit.c b/arch/s390/kernel/audit.c
index 7e331e1831d4..02051a596b87 100644
--- a/arch/s390/kernel/audit.c
+++ b/arch/s390/kernel/audit.c
@@ -54,6 +54,8 @@ int audit_classify_syscall(int abi, unsigned syscall)
return AUDITSC_SOCKETCALL;
case __NR_execve:
return AUDITSC_EXECVE;
+ case __NR_openat2:
+ return AUDITSC_OPENAT2;
default:
return AUDITSC_NATIVE;
}
diff --git a/arch/s390/kernel/compat_audit.c b/arch/s390/kernel/compat_audit.c
index 704d04cfd9dd..a6d9c82f86e4 100644
--- a/arch/s390/kernel/compat_audit.c
+++ b/arch/s390/kernel/compat_audit.c
@@ -40,6 +40,8 @@ int s390_classify_syscall(unsigned syscall)
return AUDITSC_SOCKETCALL;
case __NR_execve:
return AUDITSC_EXECVE;
+ case __NR_openat2:
+ return AUDITSC_OPENAT2;
default:
return AUDITSC_COMPAT;
}
diff --git a/arch/sparc/kernel/audit.c b/arch/sparc/kernel/audit.c
index 50fab35bdaba..b092274eca79 100644
--- a/arch/sparc/kernel/audit.c
+++ b/arch/sparc/kernel/audit.c
@@ -55,6 +55,8 @@ int audit_classify_syscall(int abi, unsigned int syscall)
return AUDITSC_SOCKETCALL;
case __NR_execve:
return AUDITSC_EXECVE;
+ case __NR_openat2:
+ return AUDITSC_OPENAT2;
default:
return AUDITSC_NATIVE;
}
diff --git a/arch/sparc/kernel/compat_audit.c b/arch/sparc/kernel/compat_audit.c
index 4c2f9a4ee845..047e87efd759 100644
--- a/arch/sparc/kernel/compat_audit.c
+++ b/arch/sparc/kernel/compat_audit.c
@@ -40,6 +40,8 @@ int sparc32_classify_syscall(unsigned int syscall)
return AUDITSC_SOCKETCALL;
case __NR_execve:
return AUDITSC_EXECVE;
+ case __NR_openat2:
+ return AUDITSC_OPENAT2;
default:
return AUDITSC_COMPAT;
}
diff --git a/arch/x86/ia32/audit.c b/arch/x86/ia32/audit.c
index 0798a6b66314..595e5da358ba 100644
--- a/arch/x86/ia32/audit.c
+++ b/arch/x86/ia32/audit.c
@@ -40,6 +40,8 @@ int ia32_classify_syscall(unsigned syscall)
case __NR_execve:
case __NR_execveat:
return AUDITSC_EXECVE;
+ case __NR_openat2:
+ return AUDITSC_OPENAT2;
default:
return AUDITSC_COMPAT;
}
diff --git a/arch/x86/kernel/audit_64.c b/arch/x86/kernel/audit_64.c
index 2a6cc9c9c881..44c3601cfdc4 100644
--- a/arch/x86/kernel/audit_64.c
+++ b/arch/x86/kernel/audit_64.c
@@ -53,6 +53,8 @@ int audit_classify_syscall(int abi, unsigned syscall)
case __NR_execve:
case __NR_execveat:
return AUDITSC_EXECVE;
+ case __NR_openat2:
+ return AUDITSC_OPENAT2;
default:
return AUDITSC_NATIVE;
}
diff --git a/include/linux/audit.h b/include/linux/audit.h
index bcf0150b1528..2eb48c2b3bd4 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -126,6 +126,7 @@ extern int is_audit_feature_set(int which);
#define AUDITSC_OPENAT 3
#define AUDITSC_SOCKETCALL 4
#define AUDITSC_EXECVE 5
+#define AUDITSC_OPENAT2 6
extern int __init audit_register_class(int class, unsigned *list);
extern int audit_classify_syscall(int abi, unsigned syscall);
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index dddea985f23e..f1519f672b20 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -76,6 +76,7 @@
#include <linux/fsnotify_backend.h>
#include <uapi/linux/limits.h>
#include <uapi/linux/netfilter/nf_tables.h>
+#include <uapi/linux/openat2.h>
#include "audit.h"
@@ -195,6 +196,8 @@ static int audit_match_perm(struct audit_context *ctx, int mask)
return ((mask & AUDIT_PERM_WRITE) && ctx->argv[0] == SYS_BIND);
case AUDITSC_EXECVE:
return mask & AUDIT_PERM_EXEC;
+ case AUDITSC_OPENAT2:
+ return mask & ACC_MODE((u32)((struct open_how *)ctx->argv[2])->flags);
default:
return 0;
}
diff --git a/lib/audit.c b/lib/audit.c
index 3ec1a94d8d64..738bda22dd39 100644
--- a/lib/audit.c
+++ b/lib/audit.c
@@ -60,6 +60,10 @@ int audit_classify_syscall(int abi, unsigned syscall)
#endif
case __NR_execve:
return AUDITSC_EXECVE;
+#ifdef __NR_openat2
+ case __NR_openat2:
+ return AUDITSC_OPENAT2;
+#endif
default:
return AUDITSC_NATIVE;
}
diff --git a/lib/compat_audit.c b/lib/compat_audit.c
index 528dafa2c2bb..b2e4f8bcaf1d 100644
--- a/lib/compat_audit.c
+++ b/lib/compat_audit.c
@@ -46,6 +46,10 @@ int audit_classify_compat_syscall(int abi, unsigned syscall)
#endif
case __NR_execve:
return AUDITSC_EXECVE;
+#ifdef __NR_openat2
+ case __NR_openat2:
+ return AUDITSC_OPENAT2;
+#endif
default:
return AUDITSC_COMPAT;
}
--
2.27.0
^ permalink raw reply related
* Re: [PATCH 1/2] audit: add support for the openat2 syscall
From: Richard Guy Briggs @ 2021-03-18 12:08 UTC (permalink / raw)
To: Christian Brauner
Cc: linux-s390, linux-ia64, Paul Moore, linux-parisc, x86, LKML,
Eric Paris, linux-fsdevel, Aleksa Sarai, Linux-Audit Mailing List,
Alexander Viro, linux-alpha, sparclinux, Eric Paris, Steve Grubb,
linuxppc-dev
In-Reply-To: <20210318104843.uiga6tmmhn5wfhbs@wittgenstein>
On 2021-03-18 11:48, Christian Brauner wrote:
> [+Cc Aleksa, the author of openat2()]
Ah! Thanks for pulling in Aleksa. I thought I caught everyone...
> and a comment below. :)
Same...
> On Wed, Mar 17, 2021 at 09:47:17PM -0400, Richard Guy Briggs wrote:
> > The openat2(2) syscall was added in kernel v5.6 with commit fddb5d430ad9
> > ("open: introduce openat2(2) syscall")
> >
> > Add the openat2(2) syscall to the audit syscall classifier.
> >
> > See the github issue
> > https://github.com/linux-audit/audit-kernel/issues/67
> >
> > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > ---
> > arch/alpha/kernel/audit.c | 2 ++
> > arch/ia64/kernel/audit.c | 2 ++
> > arch/parisc/kernel/audit.c | 2 ++
> > arch/parisc/kernel/compat_audit.c | 2 ++
> > arch/powerpc/kernel/audit.c | 2 ++
> > arch/powerpc/kernel/compat_audit.c | 2 ++
> > arch/s390/kernel/audit.c | 2 ++
> > arch/s390/kernel/compat_audit.c | 2 ++
> > arch/sparc/kernel/audit.c | 2 ++
> > arch/sparc/kernel/compat_audit.c | 2 ++
> > arch/x86/ia32/audit.c | 2 ++
> > arch/x86/kernel/audit_64.c | 2 ++
> > kernel/auditsc.c | 3 +++
> > lib/audit.c | 4 ++++
> > lib/compat_audit.c | 4 ++++
> > 15 files changed, 35 insertions(+)
> >
> > diff --git a/arch/alpha/kernel/audit.c b/arch/alpha/kernel/audit.c
> > index 96a9d18ff4c4..06a911b685d1 100644
> > --- a/arch/alpha/kernel/audit.c
> > +++ b/arch/alpha/kernel/audit.c
> > @@ -42,6 +42,8 @@ int audit_classify_syscall(int abi, unsigned syscall)
> > return 3;
> > case __NR_execve:
> > return 5;
> > + case __NR_openat2:
> > + return 6;
> > default:
> > return 0;
> > }
> > diff --git a/arch/ia64/kernel/audit.c b/arch/ia64/kernel/audit.c
> > index 5192ca899fe6..5eaa888c8fd3 100644
> > --- a/arch/ia64/kernel/audit.c
> > +++ b/arch/ia64/kernel/audit.c
> > @@ -43,6 +43,8 @@ int audit_classify_syscall(int abi, unsigned syscall)
> > return 3;
> > case __NR_execve:
> > return 5;
> > + case __NR_openat2:
> > + return 6;
> > default:
> > return 0;
> > }
> > diff --git a/arch/parisc/kernel/audit.c b/arch/parisc/kernel/audit.c
> > index 9eb47b2225d2..fc721a7727ba 100644
> > --- a/arch/parisc/kernel/audit.c
> > +++ b/arch/parisc/kernel/audit.c
> > @@ -52,6 +52,8 @@ int audit_classify_syscall(int abi, unsigned syscall)
> > return 3;
> > case __NR_execve:
> > return 5;
> > + case __NR_openat2:
> > + return 6;
> > default:
> > return 0;
> > }
> > diff --git a/arch/parisc/kernel/compat_audit.c b/arch/parisc/kernel/compat_audit.c
> > index 20c39c9d86a9..fc6d35918c44 100644
> > --- a/arch/parisc/kernel/compat_audit.c
> > +++ b/arch/parisc/kernel/compat_audit.c
> > @@ -35,6 +35,8 @@ int parisc32_classify_syscall(unsigned syscall)
> > return 3;
> > case __NR_execve:
> > return 5;
> > + case __NR_openat2:
> > + return 6;
> > default:
> > return 1;
> > }
> > diff --git a/arch/powerpc/kernel/audit.c b/arch/powerpc/kernel/audit.c
> > index a2dddd7f3d09..8f32700b0baa 100644
> > --- a/arch/powerpc/kernel/audit.c
> > +++ b/arch/powerpc/kernel/audit.c
> > @@ -54,6 +54,8 @@ int audit_classify_syscall(int abi, unsigned syscall)
> > return 4;
> > case __NR_execve:
> > return 5;
> > + case __NR_openat2:
> > + return 6;
> > default:
> > return 0;
> > }
> > diff --git a/arch/powerpc/kernel/compat_audit.c b/arch/powerpc/kernel/compat_audit.c
> > index 55c6ccda0a85..ebe45534b1c9 100644
> > --- a/arch/powerpc/kernel/compat_audit.c
> > +++ b/arch/powerpc/kernel/compat_audit.c
> > @@ -38,6 +38,8 @@ int ppc32_classify_syscall(unsigned syscall)
> > return 4;
> > case __NR_execve:
> > return 5;
> > + case __NR_openat2:
> > + return 6;
> > default:
> > return 1;
> > }
> > diff --git a/arch/s390/kernel/audit.c b/arch/s390/kernel/audit.c
> > index d395c6c9944c..d964cb94cfaf 100644
> > --- a/arch/s390/kernel/audit.c
> > +++ b/arch/s390/kernel/audit.c
> > @@ -54,6 +54,8 @@ int audit_classify_syscall(int abi, unsigned syscall)
> > return 4;
> > case __NR_execve:
> > return 5;
> > + case __NR_openat2:
> > + return 6;
> > default:
> > return 0;
> > }
> > diff --git a/arch/s390/kernel/compat_audit.c b/arch/s390/kernel/compat_audit.c
> > index 444fb1f66944..f7b32933ce0e 100644
> > --- a/arch/s390/kernel/compat_audit.c
> > +++ b/arch/s390/kernel/compat_audit.c
> > @@ -39,6 +39,8 @@ int s390_classify_syscall(unsigned syscall)
> > return 4;
> > case __NR_execve:
> > return 5;
> > + case __NR_openat2:
> > + return 6;
> > default:
> > return 1;
> > }
> > diff --git a/arch/sparc/kernel/audit.c b/arch/sparc/kernel/audit.c
> > index a6e91bf34d48..b6dcca9c6520 100644
> > --- a/arch/sparc/kernel/audit.c
> > +++ b/arch/sparc/kernel/audit.c
> > @@ -55,6 +55,8 @@ int audit_classify_syscall(int abi, unsigned int syscall)
> > return 4;
> > case __NR_execve:
> > return 5;
> > + case __NR_openat2:
> > + return 6;
> > default:
> > return 0;
> > }
> > diff --git a/arch/sparc/kernel/compat_audit.c b/arch/sparc/kernel/compat_audit.c
> > index 10eeb4f15b20..d2652a1083ad 100644
> > --- a/arch/sparc/kernel/compat_audit.c
> > +++ b/arch/sparc/kernel/compat_audit.c
> > @@ -39,6 +39,8 @@ int sparc32_classify_syscall(unsigned int syscall)
> > return 4;
> > case __NR_execve:
> > return 5;
> > + case __NR_openat2:
> > + return 6;
> > default:
> > return 1;
> > }
> > diff --git a/arch/x86/ia32/audit.c b/arch/x86/ia32/audit.c
> > index 6efe6cb3768a..57a02ade5503 100644
> > --- a/arch/x86/ia32/audit.c
> > +++ b/arch/x86/ia32/audit.c
> > @@ -39,6 +39,8 @@ int ia32_classify_syscall(unsigned syscall)
> > case __NR_execve:
> > case __NR_execveat:
> > return 5;
> > + case __NR_openat2:
> > + return 6;
> > default:
> > return 1;
> > }
> > diff --git a/arch/x86/kernel/audit_64.c b/arch/x86/kernel/audit_64.c
> > index 83d9cad4e68b..39de1e021258 100644
> > --- a/arch/x86/kernel/audit_64.c
> > +++ b/arch/x86/kernel/audit_64.c
> > @@ -53,6 +53,8 @@ int audit_classify_syscall(int abi, unsigned syscall)
> > case __NR_execve:
> > case __NR_execveat:
> > return 5;
> > + case __NR_openat2:
> > + return 6;
> > default:
> > return 0;
> > }
> > diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> > index 8bb9ac84d2fb..f5616e70d129 100644
> > --- a/kernel/auditsc.c
> > +++ b/kernel/auditsc.c
> > @@ -76,6 +76,7 @@
> > #include <linux/fsnotify_backend.h>
> > #include <uapi/linux/limits.h>
> > #include <uapi/linux/netfilter/nf_tables.h>
> > +#include <uapi/linux/openat2.h>
> >
> > #include "audit.h"
> >
> > @@ -195,6 +196,8 @@ static int audit_match_perm(struct audit_context *ctx, int mask)
> > return ((mask & AUDIT_PERM_WRITE) && ctx->argv[0] == SYS_BIND);
> > case 5: /* execve */
> > return mask & AUDIT_PERM_EXEC;
> > + case 6: /* openat2 */
> > + return mask & ACC_MODE((u32)((struct open_how *)ctx->argv[2])->flags);
>
> That looks a bit dodgy. Maybe sm like the below would be a bit better?
Ah, ok, fair enough, since original flags use a u32 and this was picked
as u64 for alignment. It was just occurring to me last night that I
might have the dubious honour of being the first usage of 0%llo format
specifier in the kernel... ;-)
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 47fb48f42c93..531e882a5096 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -159,6 +159,7 @@ static const struct audit_nfcfgop_tab audit_nfcfgs[] = {
>
> static int audit_match_perm(struct audit_context *ctx, int mask)
> {
> + struct open_how *openat2;
> unsigned n;
> if (unlikely(!ctx))
> return 0;
> @@ -195,6 +196,12 @@ static int audit_match_perm(struct audit_context *ctx, int mask)
> return ((mask & AUDIT_PERM_WRITE) && ctx->argv[0] == SYS_BIND);
> case 5: /* execve */
> return mask & AUDIT_PERM_EXEC;
> + case 6: /* openat2 */
> + openat2 = ctx->argv[2];
> + if (upper_32_bits(openat2->flags))
> + pr_warn("Some sensible warning about unknown flags");
> +
> + return mask & ACC_MODE(lower_32_bits(openat2->flags));
> default:
> return 0;
> }
>
> (Ideally we'd probably notice at build-time that we've got flags
> exceeding 32bits. Could probably easily been done by exposing an all
> flags macro somewhere and then we can place a BUILD_BUG_ON() or sm into
> such places.)
>
> Christian
- RGB
--
Richard Guy Briggs <rgb@redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635
^ permalink raw reply
* [PATCH 08/10] crypto: vmx: Source headers are not good kernel-doc candidates
From: Lee Jones @ 2021-03-18 12:44 UTC (permalink / raw)
To: lee.jones
Cc: Herbert Xu, Nayna Jain, linux-kernel, Henrique Cerri,
Paulo Flabiano Smorigo, linux-crypto, Breno Leitão,
Paul Mackerras, linuxppc-dev, David S. Miller
In-Reply-To: <20210318124422.3200180-1-lee.jones@linaro.org>
Fixes the following W=1 kernel build warning(s):
drivers/crypto/vmx/vmx.c:23: warning: expecting prototype for Routines supporting VMX instructions on the Power 8(). Prototype was for p8_init() instead
Cc: "Breno Leitão" <leitao@debian.org>
Cc: Nayna Jain <nayna@linux.ibm.com>
Cc: Paulo Flabiano Smorigo <pfsmorigo@gmail.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Henrique Cerri <mhcerri@br.ibm.com>
Cc: linux-crypto@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
drivers/crypto/vmx/vmx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/crypto/vmx/vmx.c b/drivers/crypto/vmx/vmx.c
index a40d08e75fc0b..7eb713cc87c8c 100644
--- a/drivers/crypto/vmx/vmx.c
+++ b/drivers/crypto/vmx/vmx.c
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: GPL-2.0-only
-/**
+/*
* Routines supporting VMX instructions on the Power 8
*
* Copyright (C) 2015 International Business Machines Inc.
--
2.27.0
^ permalink raw reply related
* [PATCH v3 00/10] Rid W=1 warnings in Crypto
From: Lee Jones @ 2021-03-18 12:44 UTC (permalink / raw)
To: lee.jones
Cc: Alexandre Belloni, Aymen Sghaier, Kent Yoder, Ayush Sawal,
Joakim Bech, Nicolas Ferre, Paul Mackerras, Andreas Westin,
Breno Leitão, Atul Gupta, Niklas Hernaeus, M R Gowda,
Herbert Xu, Horia Geantă, Rohit Maheshwari, Nayna Jain,
Manoj Malviya, Ludovic Desroches, Jonas Linde, Rob Rice, Zaibo Xu,
Harsh Jain, Declan Murphy, Tudor Ambarus, Vinay Kumar Yadav,
Shujuan Chen, Henrique Cerri, Daniele Alessandrelli,
linux-arm-kernel, Jonathan Cameron, linux-kernel, Berne Hebark,
linux-crypto, Jitendra Lulla, Paulo Flabiano Smorigo,
linuxppc-dev, David S. Miller
This is set 1 of 2 sets required to fully clean Crypto.
v2: No functional changes since v1.
v3: Description change and additional struct header fix
Lee Jones (10):
crypto: hisilicon: sec_drv: Supply missing description for
'sec_queue_empty()'s 'queue' param
crypto: bcm: Fix a whole host of kernel-doc misdemeanours
crypto: chelsio: chcr_core: Fix some kernel-doc issues
crypto: ux500: hash: hash_core: Fix worthy kernel-doc headers and
remove others
crypto: keembay: ocs-hcu: Fix incorrectly named functions/structs
crypto: atmel-ecc: Struct headers need to start with keyword 'struct'
crypto: caam: caampkc: Provide the name of the function and provide
missing descriptions
crypto: vmx: Source headers are not good kernel-doc candidates
crypto: nx: nx-aes-cbc: Repair some kernel-doc problems
crypto: cavium: nitrox_isr: Demote non-compliant kernel-doc headers
drivers/crypto/atmel-ecc.c | 2 +-
drivers/crypto/bcm/cipher.c | 7 ++--
drivers/crypto/bcm/spu.c | 16 ++++-----
drivers/crypto/bcm/spu2.c | 43 +++++++++++++----------
drivers/crypto/bcm/util.c | 4 +--
drivers/crypto/caam/caamalg_qi2.c | 3 ++
drivers/crypto/caam/caampkc.c | 3 +-
drivers/crypto/cavium/nitrox/nitrox_isr.c | 4 +--
drivers/crypto/chelsio/chcr_algo.c | 8 ++---
drivers/crypto/chelsio/chcr_core.c | 2 +-
drivers/crypto/hisilicon/sec/sec_drv.c | 1 +
drivers/crypto/keembay/ocs-hcu.c | 8 ++---
drivers/crypto/nx/nx-aes-cbc.c | 2 +-
drivers/crypto/nx/nx.c | 5 +--
drivers/crypto/nx/nx_debugfs.c | 2 +-
drivers/crypto/ux500/cryp/cryp.c | 5 +--
drivers/crypto/ux500/cryp/cryp_core.c | 5 +--
drivers/crypto/ux500/cryp/cryp_irq.c | 2 +-
drivers/crypto/ux500/hash/hash_core.c | 15 +++-----
drivers/crypto/vmx/vmx.c | 2 +-
20 files changed, 73 insertions(+), 66 deletions(-)
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: Andreas Westin <andreas.westin@stericsson.com>
Cc: Atul Gupta <atul.gupta@chelsio.com>
Cc: Aymen Sghaier <aymen.sghaier@nxp.com>
Cc: Ayush Sawal <ayush.sawal@chelsio.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Berne Hebark <berne.herbark@stericsson.com>
Cc: "Breno Leitão" <leitao@debian.org>
Cc: Daniele Alessandrelli <daniele.alessandrelli@intel.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Declan Murphy <declan.murphy@intel.com>
Cc: Harsh Jain <harsh@chelsio.com>
Cc: Henrique Cerri <mhcerri@br.ibm.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: "Horia Geantă" <horia.geanta@nxp.com>
Cc: Jitendra Lulla <jlulla@chelsio.com>
Cc: Joakim Bech <joakim.xx.bech@stericsson.com>
Cc: Jonas Linde <jonas.linde@stericsson.com>
Cc: Jonathan Cameron <jonathan.cameron@huawei.com>
Cc: Kent Yoder <yoder1@us.ibm.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-crypto@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: Ludovic Desroches <ludovic.desroches@microchip.com>
Cc: Manoj Malviya <manojmalviya@chelsio.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: M R Gowda <yeshaswi@chelsio.com>
Cc: Nayna Jain <nayna@linux.ibm.com>
Cc: Nicolas Ferre <nicolas.ferre@microchip.com>
Cc: Niklas Hernaeus <niklas.hernaeus@stericsson.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Paulo Flabiano Smorigo <pfsmorigo@gmail.com>
Cc: Rob Rice <rob.rice@broadcom.com>
Cc: Rohit Maheshwari <rohitm@chelsio.com>
Cc: Shujuan Chen <shujuan.chen@stericsson.com>
Cc: Tudor Ambarus <tudor.ambarus@microchip.com>
Cc: Vinay Kumar Yadav <vinay.yadav@chelsio.com>
Cc: Zaibo Xu <xuzaibo@huawei.com>
--
2.27.0
^ permalink raw reply
* [PATCH 09/10] crypto: nx: nx-aes-cbc: Repair some kernel-doc problems
From: Lee Jones @ 2021-03-18 12:44 UTC (permalink / raw)
To: lee.jones
Cc: Herbert Xu, Kent Yoder, Nayna Jain, linux-kernel,
Paulo Flabiano Smorigo, linux-crypto, Breno Leitão,
Paul Mackerras, linuxppc-dev, David S. Miller
In-Reply-To: <20210318124422.3200180-1-lee.jones@linaro.org>
Fixes the following W=1 kernel build warning(s):
drivers/crypto/nx/nx-aes-cbc.c:24: warning: Function parameter or member 'tfm' not described in 'cbc_aes_nx_set_key'
drivers/crypto/nx/nx-aes-cbc.c:24: warning: Function parameter or member 'in_key' not described in 'cbc_aes_nx_set_key'
drivers/crypto/nx/nx-aes-cbc.c:24: warning: Function parameter or member 'key_len' not described in 'cbc_aes_nx_set_key'
drivers/crypto/nx/nx-aes-cbc.c:24: warning: expecting prototype for Nest Accelerators driver(). Prototype was for cbc_aes_nx_set_key() instead
drivers/crypto/nx/nx_debugfs.c:34: warning: Function parameter or member 'drv' not described in 'nx_debugfs_init'
drivers/crypto/nx/nx_debugfs.c:34: warning: expecting prototype for Nest Accelerators driver(). Prototype was for nx_debugfs_init() instead
drivers/crypto/nx/nx.c:31: warning: Incorrect use of kernel-doc format: * nx_hcall_sync - make an H_COP_OP hcall for the passed in op structure
drivers/crypto/nx/nx.c:43: warning: Function parameter or member 'nx_ctx' not described in 'nx_hcall_sync'
drivers/crypto/nx/nx.c:43: warning: Function parameter or member 'op' not described in 'nx_hcall_sync'
drivers/crypto/nx/nx.c:43: warning: Function parameter or member 'may_sleep' not described in 'nx_hcall_sync'
drivers/crypto/nx/nx.c:43: warning: expecting prototype for Nest Accelerators driver(). Prototype was for nx_hcall_sync() instead
drivers/crypto/nx/nx.c:209: warning: Function parameter or member 'nbytes' not described in 'trim_sg_list'
Cc: "Breno Leitão" <leitao@debian.org>
Cc: Nayna Jain <nayna@linux.ibm.com>
Cc: Paulo Flabiano Smorigo <pfsmorigo@gmail.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Kent Yoder <yoder1@us.ibm.com>
Cc: linux-crypto@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
drivers/crypto/nx/nx-aes-cbc.c | 2 +-
drivers/crypto/nx/nx.c | 5 +++--
drivers/crypto/nx/nx_debugfs.c | 2 +-
3 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/crypto/nx/nx-aes-cbc.c b/drivers/crypto/nx/nx-aes-cbc.c
index 92e921eceed75..d6314ea9ae896 100644
--- a/drivers/crypto/nx/nx-aes-cbc.c
+++ b/drivers/crypto/nx/nx-aes-cbc.c
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: GPL-2.0-only
-/**
+/*
* AES CBC routines supporting the Power 7+ Nest Accelerators driver
*
* Copyright (C) 2011-2012 International Business Machines Inc.
diff --git a/drivers/crypto/nx/nx.c b/drivers/crypto/nx/nx.c
index 1d0e8a1ba1605..010e87d9da36b 100644
--- a/drivers/crypto/nx/nx.c
+++ b/drivers/crypto/nx/nx.c
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: GPL-2.0-only
-/**
+/*
* Routines supporting the Power 7+ Nest Accelerators driver
*
* Copyright (C) 2011-2012 International Business Machines Inc.
@@ -200,7 +200,8 @@ struct nx_sg *nx_walk_and_build(struct nx_sg *nx_dst,
* @sg: sg list head
* @end: sg lisg end
* @delta: is the amount we need to crop in order to bound the list.
- *
+ * @nbytes: length of data in the scatterlists or data length - whichever
+ * is greater.
*/
static long int trim_sg_list(struct nx_sg *sg,
struct nx_sg *end,
diff --git a/drivers/crypto/nx/nx_debugfs.c b/drivers/crypto/nx/nx_debugfs.c
index 1975bcbee9974..ee7cd88bb10a7 100644
--- a/drivers/crypto/nx/nx_debugfs.c
+++ b/drivers/crypto/nx/nx_debugfs.c
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: GPL-2.0-only
-/**
+/*
* debugfs routines supporting the Power 7+ Nest Accelerators driver
*
* Copyright (C) 2011-2012 International Business Machines Inc.
--
2.27.0
^ permalink raw reply related
* Re: Advice needed on SMP regression after cpu_core_mask change
From: Srikar Dronamraju @ 2021-03-18 13:42 UTC (permalink / raw)
To: Daniel Henrique Barboza; +Cc: aneesh.kumar, linuxppc-dev, Cedric Le Goater
In-Reply-To: <daa5d05f-dbd0-05ad-7395-5d5a3d364fc6@gmail.com>
* Daniel Henrique Barboza <danielhb413@gmail.com> [2021-03-17 10:00:34]:
> Hello,
>
> Patch 4bce545903fa ("powerpc/topology: Update topology_core_cpumask") introduced
> a regression in both upstream and RHEL downstream kernels [1]. The assumption made
> in the commit:
>
> "Further analysis shows that cpu_core_mask and cpu_cpu_mask for any CPU would be
> equal on Power"
>
> Doesn't seem to be true. After this commit, QEMU is now unable to set single NUMA
> node SMP topologies such as:
>
> -smp 8,maxcpus=8,cores=2,threads=2,sockets=2
What does it mean for a NUMA to have more than one sockets?
If they are all part of the same node, there are at local distance to each
other. cache is per core. So what resources are shared by the Sockets that
are part of the same NUMA. And how does Userspace/ application make use of
the same.
Please don't mistake this as attempt to downplay your report but a honest
attempt to better understand the situation.
For example, if the socket denotes the hemisphere logic in P10, then can we
see if the coregroup feature can be used. "Coregroup" is suppose to mean a
set of cores within a NUMA that have some characteristics and there can be
multiple coregroups within a NUMA. We add that mostly to mimic hemisphere in
P10. However the number of coregroups in a NUMA is not exported to userspace
at this time.
However if each Socket is associated with a memory and node distance, then
should they be NUMA?
Can you provide me with the unique ibm,chip-ids in your 2 NUMA, 4 node case?
Does this cause an performance issues with the guest/application?
Till your report, I was under the impression that NUMAs == Sockets.
>
> lscpu will give the following output in this case:
>
> # lscpu
> Architecture: ppc64le
> Byte Order: Little Endian
> CPU(s): 8
> On-line CPU(s) list: 0-7
> Thread(s) per core: 2
> Core(s) per socket: 4
> Socket(s): 1
> NUMA node(s): 1
> Model: 2.2 (pvr 004e 1202)
> Model name: POWER9 (architected), altivec supported
> Hypervisor vendor: KVM
> Virtualization type: para
> L1d cache: 32K
> L1i cache: 32K
> NUMA node0 CPU(s): 0-7
>
>
> This is happening because the macro cpu_cpu_mask(cpu) expands to
> cpumask_of_node(cpu_to_node(cpu)), which in turn expands to node_to_cpumask_map[node].
> node_to_cpumask_map is a NUMA array that maps CPUs to NUMA nodes (Aneesh is on CC to
> correct me if I'm wrong). We're now associating sockets to NUMA nodes directly.
>
> If I add a second NUMA node then I can get the intended smp topology:
>
> -smp 8,maxcpus=8,cores=2,threads=2,sockets=2
> -numa node,memdev=mem0,cpus=0-3,nodeid=0 \
> -numa node,memdev=mem1,cpus=4-7,nodeid=1 \
>
> # lscpu
> Architecture: ppc64le
> Byte Order: Little Endian
> CPU(s): 8
> On-line CPU(s) list: 0-7
> Thread(s) per core: 2
> Core(s) per socket: 2
> Socket(s): 2
> NUMA node(s): 2
> Model: 2.2 (pvr 004e 1202)
> Model name: POWER9 (architected), altivec supported
> Hypervisor vendor: KVM
> Virtualization type: para
> L1d cache: 32K
> L1i cache: 32K
> NUMA node0 CPU(s): 0-3
> NUMA node1 CPU(s): 4-7
>
>
> However, if I try a single socket with multiple NUMA nodes topology, which is the case
> of Power10, e.g.:
>
>
> -smp 8,maxcpus=8,cores=4,threads=2,sockets=1
> -numa node,memdev=mem0,cpus=0-3,nodeid=0 \
> -numa node,memdev=mem1,cpus=4-7,nodeid=1 \
>
>
> This is the result:
>
> # lscpu
> Architecture: ppc64le
> Byte Order: Little Endian
> CPU(s): 8
> On-line CPU(s) list: 0-7
> Thread(s) per core: 2
> Core(s) per socket: 2
> Socket(s): 2
> NUMA node(s): 2
> Model: 2.2 (pvr 004e 1202)
> Model name: POWER9 (architected), altivec supported
> Hypervisor vendor: KVM
> Virtualization type: para
> L1d cache: 32K
> L1i cache: 32K
> NUMA node0 CPU(s): 0-3
> NUMA node1 CPU(s): 4-7
>
>
> This confirms my suspicions that, at this moment, we're making sockets == NUMA nodes.
>
>
> Cedric, the reason I'm CCing you is because this is related to ibm,chip-id. The commit
> after the one that caused the regression, 4ca234a9cbd7c3a65 ("powerpc/smp: Stop updating
> cpu_core_mask"), is erasing the code that calculated cpu_core_mask. cpu_core_mask, despite
> its shortcomings that caused its removal, was giving a precise SMP topology. And it was
> using physical_package_id/'ibm,chip-id' for that.
>
> Checking in QEMU I can say that the ibm,chip-id calculation is the only place in the code
> that cares about cores per socket information. The kernel is now ignoring that, starting
> on 4bce545903fa, and now QEMU is unable to provide this info to the guest.
>
> If we're not going to use ibm,chip-id any longer, which seems sensible given that PAPR does
> not declare it, we need another way of letting the guest know how much cores per socket
> we want.
>
>
>
> [1] https://bugzilla.redhat.com/1934421
>
>
>
> Thanks,
>
>
> DHB
--
Thanks and Regards
Srikar Dronamraju
^ permalink raw reply
* Re: Advice needed on SMP regression after cpu_core_mask change
From: Daniel Henrique Barboza @ 2021-03-18 15:36 UTC (permalink / raw)
To: Srikar Dronamraju; +Cc: aneesh.kumar, linuxppc-dev, Cedric Le Goater
In-Reply-To: <20210318134236.GC2339179@linux.vnet.ibm.com>
On 3/18/21 10:42 AM, Srikar Dronamraju wrote:
> * Daniel Henrique Barboza <danielhb413@gmail.com> [2021-03-17 10:00:34]:
>
>> Hello,
>>
>> Patch 4bce545903fa ("powerpc/topology: Update topology_core_cpumask") introduced
>> a regression in both upstream and RHEL downstream kernels [1]. The assumption made
>> in the commit:
>>
>> "Further analysis shows that cpu_core_mask and cpu_cpu_mask for any CPU would be
>> equal on Power"
>>
>> Doesn't seem to be true. After this commit, QEMU is now unable to set single NUMA
>> node SMP topologies such as:
>>
>> -smp 8,maxcpus=8,cores=2,threads=2,sockets=2
>
> What does it mean for a NUMA to have more than one sockets?
> If they are all part of the same node, there are at local distance to each
> other. cache is per core. So what resources are shared by the Sockets that
> are part of the same NUMA. And how does Userspace/ application make use of
> the same.
Honestly, I sympathize with the idea that multiple sockets in the same NUMA
node being "weird". QEMU is accepting this kind of topology since forever
because we didn't pay attention to these other details.
I don't see any problems adding more constraints that makes sense in the
virtual layer, as long as the constraints make sense and are documented.
Putting multiple sockets in a single NUMA node seems like a fair restriction.
>
> Please don't mistake this as attempt to downplay your report but a honest
> attempt to better understand the situation.
It's cool. Ask away.
>
> For example, if the socket denotes the hemisphere logic in P10, then can we
> see if the coregroup feature can be used. "Coregroup" is suppose to mean a
> set of cores within a NUMA that have some characteristics and there can be
> multiple coregroups within a NUMA. We add that mostly to mimic hemisphere in
> P10. However the number of coregroups in a NUMA is not exported to userspace
> at this time.
I see. I thought that the presence of the hemispheres inside the chip would
justify more than one NUMA node inside the chip, meaning that a chip/socket
would have more than one NUMA nodes inside of it.
If that's not the case then I guess socket == NUMA node is still valid in
P10 as well. The last 'lscpu' example I gave here, claiming that this would
be a Power10 scenario, doesn't represent P10 after all.
>
> However if each Socket is associated with a memory and node distance, then
> should they be NUMA?
>
> Can you provide me with the unique ibm,chip-ids in your 2 NUMA, 4 node case?
> Does this cause an performance issues with the guest/application?
I can fetch some values, but we're trying to move out of it since it's not on the
pseries spec (PAPR). Perhaps with these restrictions we can live without
ibm,chip-id in QEMU.
>
> Till your report, I was under the impression that NUMAs == Sockets.
After reading and discussing about it, I think the sensible thing to do is to
put this same constraint in QEMU.
In theory it would be nice to let the virtual machine to have whatever topology it
wants, multiple sockets in the same NUMA domain and so on, but in the end we're
emulating Power hardware. If Power hardware - and the powerpc kernel - operates
under these assumptions, then I don't see much point into allowing users to
set unrealistic virtual CPU topologies that will be misrepresented in the
kernel.
I'll try this restriction in QEMU and see how upstream kernel behaves, with
and without ibm,chip-id being advertised in the DT.
Thanks,
DHB
>
>>
>> lscpu will give the following output in this case:
>>
>> # lscpu
>> Architecture: ppc64le
>> Byte Order: Little Endian
>> CPU(s): 8
>> On-line CPU(s) list: 0-7
>> Thread(s) per core: 2
>> Core(s) per socket: 4
>> Socket(s): 1
>> NUMA node(s): 1
>> Model: 2.2 (pvr 004e 1202)
>> Model name: POWER9 (architected), altivec supported
>> Hypervisor vendor: KVM
>> Virtualization type: para
>> L1d cache: 32K
>> L1i cache: 32K
>> NUMA node0 CPU(s): 0-7
>>
>>
>> This is happening because the macro cpu_cpu_mask(cpu) expands to
>> cpumask_of_node(cpu_to_node(cpu)), which in turn expands to node_to_cpumask_map[node].
>> node_to_cpumask_map is a NUMA array that maps CPUs to NUMA nodes (Aneesh is on CC to
>> correct me if I'm wrong). We're now associating sockets to NUMA nodes directly.
>>
>> If I add a second NUMA node then I can get the intended smp topology:
>>
>> -smp 8,maxcpus=8,cores=2,threads=2,sockets=2
>> -numa node,memdev=mem0,cpus=0-3,nodeid=0 \
>> -numa node,memdev=mem1,cpus=4-7,nodeid=1 \
>>
>> # lscpu
>> Architecture: ppc64le
>> Byte Order: Little Endian
>> CPU(s): 8
>> On-line CPU(s) list: 0-7
>> Thread(s) per core: 2
>> Core(s) per socket: 2
>> Socket(s): 2
>> NUMA node(s): 2
>> Model: 2.2 (pvr 004e 1202)
>> Model name: POWER9 (architected), altivec supported
>> Hypervisor vendor: KVM
>> Virtualization type: para
>> L1d cache: 32K
>> L1i cache: 32K
>> NUMA node0 CPU(s): 0-3
>> NUMA node1 CPU(s): 4-7
>>
>>
>> However, if I try a single socket with multiple NUMA nodes topology, which is the case
>> of Power10, e.g.:
>>
>>
>> -smp 8,maxcpus=8,cores=4,threads=2,sockets=1
>> -numa node,memdev=mem0,cpus=0-3,nodeid=0 \
>> -numa node,memdev=mem1,cpus=4-7,nodeid=1 \
>>
>>
>> This is the result:
>>
>> # lscpu
>> Architecture: ppc64le
>> Byte Order: Little Endian
>> CPU(s): 8
>> On-line CPU(s) list: 0-7
>> Thread(s) per core: 2
>> Core(s) per socket: 2
>> Socket(s): 2
>> NUMA node(s): 2
>> Model: 2.2 (pvr 004e 1202)
>> Model name: POWER9 (architected), altivec supported
>> Hypervisor vendor: KVM
>> Virtualization type: para
>> L1d cache: 32K
>> L1i cache: 32K
>> NUMA node0 CPU(s): 0-3
>> NUMA node1 CPU(s): 4-7
>>
>>
>> This confirms my suspicions that, at this moment, we're making sockets == NUMA nodes.
>>
>>
>> Cedric, the reason I'm CCing you is because this is related to ibm,chip-id. The commit
>> after the one that caused the regression, 4ca234a9cbd7c3a65 ("powerpc/smp: Stop updating
>> cpu_core_mask"), is erasing the code that calculated cpu_core_mask. cpu_core_mask, despite
>> its shortcomings that caused its removal, was giving a precise SMP topology. And it was
>> using physical_package_id/'ibm,chip-id' for that.
>>
>> Checking in QEMU I can say that the ibm,chip-id calculation is the only place in the code
>> that cares about cores per socket information. The kernel is now ignoring that, starting
>> on 4bce545903fa, and now QEMU is unable to provide this info to the guest.
>>
>> If we're not going to use ibm,chip-id any longer, which seems sensible given that PAPR does
>> not declare it, we need another way of letting the guest know how much cores per socket
>> we want.
>>
>>
>>
>> [1] https://bugzilla.redhat.com/1934421
>>
>>
>>
>> Thanks,
>>
>>
>> DHB
>
^ permalink raw reply
* swiotlb cleanups v3
From: Christoph Hellwig @ 2021-03-18 16:14 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: iommu, xen-devel, Claire Chang, linuxppc-dev, Dongli Zhang
Hi Konrad,
this series contains a bunch of swiotlb cleanups, mostly to reduce the
amount of internals exposed to code outside of swiotlb.c, which should
helper to prepare for supporting multiple different bounce buffer pools.
Changes since v2:
- fix a bisetion hazard that did not allocate the alloc_size array
- dropped all patches already merged
Changes since v1:
- rebased to v5.12-rc1
- a few more cleanups
- merge and forward port the patch from Claire to move all the global
variables into a struct to prepare for multiple instances
^ permalink raw reply
* [PATCH 1/3] swiotlb: move global variables into a new io_tlb_mem structure
From: Christoph Hellwig @ 2021-03-18 16:14 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: iommu, xen-devel, Claire Chang, linuxppc-dev, Dongli Zhang
In-Reply-To: <20210318161424.489045-1-hch@lst.de>
From: Claire Chang <tientzu@chromium.org>
Added a new struct, io_tlb_mem, as the IO TLB memory pool descriptor and
moved relevant global variables into that struct.
This will be useful later to allow for restricted DMA pool.
Signed-off-by: Claire Chang <tientzu@chromium.org>
[hch: rebased]
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/xen/swiotlb-xen.c | 2 +-
include/linux/swiotlb.h | 43 ++++-
kernel/dma/swiotlb.c | 354 ++++++++++++++++++--------------------
3 files changed, 206 insertions(+), 193 deletions(-)
diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
index 4ecfce2c6f7263..5329ad54a5f34e 100644
--- a/drivers/xen/swiotlb-xen.c
+++ b/drivers/xen/swiotlb-xen.c
@@ -548,7 +548,7 @@ xen_swiotlb_sync_sg_for_device(struct device *dev, struct scatterlist *sgl,
static int
xen_swiotlb_dma_supported(struct device *hwdev, u64 mask)
{
- return xen_phys_to_dma(hwdev, io_tlb_end - 1) <= mask;
+ return xen_phys_to_dma(hwdev, io_tlb_default_mem.end - 1) <= mask;
}
const struct dma_map_ops xen_swiotlb_dma_ops = {
diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index 0696bdc8072e97..5ec5378b17c333 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -6,6 +6,7 @@
#include <linux/init.h>
#include <linux/types.h>
#include <linux/limits.h>
+#include <linux/spinlock.h>
struct device;
struct page;
@@ -61,11 +62,49 @@ dma_addr_t swiotlb_map(struct device *dev, phys_addr_t phys,
#ifdef CONFIG_SWIOTLB
extern enum swiotlb_force swiotlb_force;
-extern phys_addr_t io_tlb_start, io_tlb_end;
+
+/**
+ * struct io_tlb_mem - IO TLB Memory Pool Descriptor
+ *
+ * @start: The start address of the swiotlb memory pool. Used to do a quick
+ * range check to see if the memory was in fact allocated by this
+ * API.
+ * @end: The end address of the swiotlb memory pool. Used to do a quick
+ * range check to see if the memory was in fact allocated by this
+ * API.
+ * @nslabs: The number of IO TLB blocks (in groups of 64) between @start and
+ * @end. This is command line adjustable via setup_io_tlb_npages.
+ * @used: The number of used IO TLB block.
+ * @list: The free list describing the number of free entries available
+ * from each index.
+ * @index: The index to start searching in the next round.
+ * @orig_addr: The original address corresponding to a mapped entry.
+ * @alloc_size: Size of the allocated buffer.
+ * @lock: The lock to protect the above data structures in the map and
+ * unmap calls.
+ * @debugfs: The dentry to debugfs.
+ * @late_alloc: %true if allocated using the page allocator
+ */
+struct io_tlb_mem {
+ phys_addr_t start;
+ phys_addr_t end;
+ unsigned long nslabs;
+ unsigned long used;
+ unsigned int *list;
+ unsigned int index;
+ phys_addr_t *orig_addr;
+ size_t *alloc_size;
+ spinlock_t lock;
+ struct dentry *debugfs;
+ bool late_alloc;
+};
+extern struct io_tlb_mem io_tlb_default_mem;
static inline bool is_swiotlb_buffer(phys_addr_t paddr)
{
- return paddr >= io_tlb_start && paddr < io_tlb_end;
+ struct io_tlb_mem *mem = &io_tlb_default_mem;
+
+ return paddr >= mem->start && paddr < mem->end;
}
void __init swiotlb_exit(void);
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index 35e24f0ff8b207..d9c097f0f78cec 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -59,32 +59,11 @@
*/
#define IO_TLB_MIN_SLABS ((1<<20) >> IO_TLB_SHIFT)
-enum swiotlb_force swiotlb_force;
-
-/*
- * Used to do a quick range check in swiotlb_tbl_unmap_single and
- * swiotlb_tbl_sync_single_*, to see if the memory was in fact allocated by this
- * API.
- */
-phys_addr_t io_tlb_start, io_tlb_end;
-
-/*
- * The number of IO TLB blocks (in groups of 64) between io_tlb_start and
- * io_tlb_end. This is command line adjustable via setup_io_tlb_npages.
- */
-static unsigned long io_tlb_nslabs;
+#define INVALID_PHYS_ADDR (~(phys_addr_t)0)
-/*
- * The number of used IO TLB block
- */
-static unsigned long io_tlb_used;
+enum swiotlb_force swiotlb_force;
-/*
- * This is a free list describing the number of free entries available from
- * each index
- */
-static unsigned int *io_tlb_list;
-static unsigned int io_tlb_index;
+struct io_tlb_mem io_tlb_default_mem;
/*
* Max segment that we can provide which (if pages are contingous) will
@@ -92,32 +71,15 @@ static unsigned int io_tlb_index;
*/
static unsigned int max_segment;
-/*
- * We need to save away the original address corresponding to a mapped entry
- * for the sync operations.
- */
-#define INVALID_PHYS_ADDR (~(phys_addr_t)0)
-static phys_addr_t *io_tlb_orig_addr;
-
-/*
- * The mapped buffer's size should be validated during a sync operation.
- */
-static size_t *io_tlb_alloc_size;
-
-/*
- * Protect the above data structures in the map and unmap calls
- */
-static DEFINE_SPINLOCK(io_tlb_lock);
-
-static int late_alloc;
-
static int __init
setup_io_tlb_npages(char *str)
{
+ struct io_tlb_mem *mem = &io_tlb_default_mem;
+
if (isdigit(*str)) {
- io_tlb_nslabs = simple_strtoul(str, &str, 0);
+ mem->nslabs = simple_strtoul(str, &str, 0);
/* avoid tail segment of size < IO_TLB_SEGSIZE */
- io_tlb_nslabs = ALIGN(io_tlb_nslabs, IO_TLB_SEGSIZE);
+ mem->nslabs = ALIGN(mem->nslabs, IO_TLB_SEGSIZE);
}
if (*str == ',')
++str;
@@ -125,7 +87,7 @@ setup_io_tlb_npages(char *str)
swiotlb_force = SWIOTLB_FORCE;
} else if (!strcmp(str, "noforce")) {
swiotlb_force = SWIOTLB_NO_FORCE;
- io_tlb_nslabs = 1;
+ mem->nslabs = 1;
}
return 0;
@@ -136,7 +98,7 @@ static bool no_iotlb_memory;
unsigned long swiotlb_nr_tbl(void)
{
- return unlikely(no_iotlb_memory) ? 0 : io_tlb_nslabs;
+ return unlikely(no_iotlb_memory) ? 0 : io_tlb_default_mem.nslabs;
}
EXPORT_SYMBOL_GPL(swiotlb_nr_tbl);
@@ -158,13 +120,14 @@ unsigned long swiotlb_size_or_default(void)
{
unsigned long size;
- size = io_tlb_nslabs << IO_TLB_SHIFT;
+ size = io_tlb_default_mem.nslabs << IO_TLB_SHIFT;
return size ? size : (IO_TLB_DEFAULT_SIZE);
}
void __init swiotlb_adjust_size(unsigned long new_size)
{
+ struct io_tlb_mem *mem = &io_tlb_default_mem;
unsigned long size;
/*
@@ -172,10 +135,10 @@ void __init swiotlb_adjust_size(unsigned long new_size)
* architectures such as those supporting memory encryption to
* adjust/expand SWIOTLB size for their use.
*/
- if (!io_tlb_nslabs) {
+ if (!mem->nslabs) {
size = ALIGN(new_size, IO_TLB_SIZE);
- io_tlb_nslabs = size >> IO_TLB_SHIFT;
- io_tlb_nslabs = ALIGN(io_tlb_nslabs, IO_TLB_SEGSIZE);
+ mem->nslabs = size >> IO_TLB_SHIFT;
+ mem->nslabs = ALIGN(mem->nslabs, IO_TLB_SEGSIZE);
pr_info("SWIOTLB bounce buffer size adjusted to %luMB", size >> 20);
}
@@ -183,14 +146,15 @@ void __init swiotlb_adjust_size(unsigned long new_size)
void swiotlb_print_info(void)
{
- unsigned long bytes = io_tlb_nslabs << IO_TLB_SHIFT;
+ struct io_tlb_mem *mem = &io_tlb_default_mem;
+ unsigned long bytes = mem->nslabs << IO_TLB_SHIFT;
if (no_iotlb_memory) {
pr_warn("No low mem\n");
return;
}
- pr_info("mapped [mem %pa-%pa] (%luMB)\n", &io_tlb_start, &io_tlb_end,
+ pr_info("mapped [mem %pa-%pa] (%luMB)\n", &mem->start, &mem->end,
bytes >> 20);
}
@@ -212,68 +176,71 @@ static inline unsigned long nr_slots(u64 val)
*/
void __init swiotlb_update_mem_attributes(void)
{
+ struct io_tlb_mem *mem = &io_tlb_default_mem;
void *vaddr;
unsigned long bytes;
- if (no_iotlb_memory || late_alloc)
+ if (no_iotlb_memory || mem->late_alloc)
return;
- vaddr = phys_to_virt(io_tlb_start);
- bytes = PAGE_ALIGN(io_tlb_nslabs << IO_TLB_SHIFT);
+ vaddr = phys_to_virt(mem->start);
+ bytes = PAGE_ALIGN(mem->nslabs << IO_TLB_SHIFT);
set_memory_decrypted((unsigned long)vaddr, bytes >> PAGE_SHIFT);
memset(vaddr, 0, bytes);
}
int __init swiotlb_init_with_tbl(char *tlb, unsigned long nslabs, int verbose)
{
+ struct io_tlb_mem *mem = &io_tlb_default_mem;
unsigned long i, bytes;
size_t alloc_size;
/* protect against double initialization */
- if (WARN_ON_ONCE(io_tlb_start))
+ if (WARN_ON_ONCE(mem->start))
return -ENOMEM;
bytes = nslabs << IO_TLB_SHIFT;
- io_tlb_nslabs = nslabs;
- io_tlb_start = __pa(tlb);
- io_tlb_end = io_tlb_start + bytes;
+ mem->nslabs = nslabs;
+ mem->start = __pa(tlb);
+ mem->end = mem->start + bytes;
+ mem->index = 0;
+ spin_lock_init(&mem->lock);
/*
* Allocate and initialize the free list array. This array is used
* to find contiguous free memory regions of size up to IO_TLB_SEGSIZE
- * between io_tlb_start and io_tlb_end.
+ * between mem->start and mem->end.
*/
- alloc_size = PAGE_ALIGN(io_tlb_nslabs * sizeof(int));
- io_tlb_list = memblock_alloc(alloc_size, PAGE_SIZE);
- if (!io_tlb_list)
+ alloc_size = PAGE_ALIGN(mem->nslabs * sizeof(int));
+ mem->list = memblock_alloc(alloc_size, PAGE_SIZE);
+ if (!mem->list)
panic("%s: Failed to allocate %zu bytes align=0x%lx\n",
__func__, alloc_size, PAGE_SIZE);
- alloc_size = PAGE_ALIGN(io_tlb_nslabs * sizeof(phys_addr_t));
- io_tlb_orig_addr = memblock_alloc(alloc_size, PAGE_SIZE);
- if (!io_tlb_orig_addr)
+ alloc_size = PAGE_ALIGN(mem->nslabs * sizeof(phys_addr_t));
+ mem->orig_addr = memblock_alloc(alloc_size, PAGE_SIZE);
+ if (!mem->orig_addr)
panic("%s: Failed to allocate %zu bytes align=0x%lx\n",
__func__, alloc_size, PAGE_SIZE);
- alloc_size = PAGE_ALIGN(io_tlb_nslabs * sizeof(size_t));
- io_tlb_alloc_size = memblock_alloc(alloc_size, PAGE_SIZE);
- if (!io_tlb_alloc_size)
+ alloc_size = PAGE_ALIGN(mem->nslabs * sizeof(size_t));
+ mem->alloc_size = memblock_alloc(alloc_size, PAGE_SIZE);
+ if (mem->alloc_size)
panic("%s: Failed to allocate %zu bytes align=0x%lx\n",
__func__, alloc_size, PAGE_SIZE);
- for (i = 0; i < io_tlb_nslabs; i++) {
- io_tlb_list[i] = IO_TLB_SEGSIZE - io_tlb_offset(i);
- io_tlb_orig_addr[i] = INVALID_PHYS_ADDR;
- io_tlb_alloc_size[i] = 0;
+ for (i = 0; i < mem->nslabs; i++) {
+ mem->list[i] = IO_TLB_SEGSIZE - io_tlb_offset(i);
+ mem->orig_addr[i] = INVALID_PHYS_ADDR;
+ mem->alloc_size[i] = 0;
}
- io_tlb_index = 0;
no_iotlb_memory = false;
if (verbose)
swiotlb_print_info();
- swiotlb_set_max_segment(io_tlb_nslabs << IO_TLB_SHIFT);
+ swiotlb_set_max_segment(mem->nslabs << IO_TLB_SHIFT);
return 0;
}
@@ -284,26 +251,27 @@ int __init swiotlb_init_with_tbl(char *tlb, unsigned long nslabs, int verbose)
void __init
swiotlb_init(int verbose)
{
+ struct io_tlb_mem *mem = &io_tlb_default_mem;
size_t default_size = IO_TLB_DEFAULT_SIZE;
unsigned char *vstart;
unsigned long bytes;
- if (!io_tlb_nslabs) {
- io_tlb_nslabs = (default_size >> IO_TLB_SHIFT);
- io_tlb_nslabs = ALIGN(io_tlb_nslabs, IO_TLB_SEGSIZE);
+ if (!mem->nslabs) {
+ mem->nslabs = (default_size >> IO_TLB_SHIFT);
+ mem->nslabs = ALIGN(mem->nslabs, IO_TLB_SEGSIZE);
}
- bytes = io_tlb_nslabs << IO_TLB_SHIFT;
+ bytes = mem->nslabs << IO_TLB_SHIFT;
/* Get IO TLB memory from the low pages */
vstart = memblock_alloc_low(PAGE_ALIGN(bytes), PAGE_SIZE);
- if (vstart && !swiotlb_init_with_tbl(vstart, io_tlb_nslabs, verbose))
+ if (vstart && !swiotlb_init_with_tbl(vstart, mem->nslabs, verbose))
return;
- if (io_tlb_start) {
- memblock_free_early(io_tlb_start,
- PAGE_ALIGN(io_tlb_nslabs << IO_TLB_SHIFT));
- io_tlb_start = 0;
+ if (mem->start) {
+ memblock_free_early(mem->start,
+ PAGE_ALIGN(mem->nslabs << IO_TLB_SHIFT));
+ mem->start = 0;
}
pr_warn("Cannot allocate buffer");
no_iotlb_memory = true;
@@ -317,22 +285,23 @@ swiotlb_init(int verbose)
int
swiotlb_late_init_with_default_size(size_t default_size)
{
- unsigned long bytes, req_nslabs = io_tlb_nslabs;
+ struct io_tlb_mem *mem = &io_tlb_default_mem;
+ unsigned long bytes, req_nslabs = mem->nslabs;
unsigned char *vstart = NULL;
unsigned int order;
int rc = 0;
- if (!io_tlb_nslabs) {
- io_tlb_nslabs = (default_size >> IO_TLB_SHIFT);
- io_tlb_nslabs = ALIGN(io_tlb_nslabs, IO_TLB_SEGSIZE);
+ if (!mem->nslabs) {
+ mem->nslabs = (default_size >> IO_TLB_SHIFT);
+ mem->nslabs = ALIGN(mem->nslabs, IO_TLB_SEGSIZE);
}
/*
* Get IO TLB memory from the low pages
*/
- order = get_order(io_tlb_nslabs << IO_TLB_SHIFT);
- io_tlb_nslabs = SLABS_PER_PAGE << order;
- bytes = io_tlb_nslabs << IO_TLB_SHIFT;
+ order = get_order(mem->nslabs << IO_TLB_SHIFT);
+ mem->nslabs = SLABS_PER_PAGE << order;
+ bytes = mem->nslabs << IO_TLB_SHIFT;
while ((SLABS_PER_PAGE << order) > IO_TLB_MIN_SLABS) {
vstart = (void *)__get_free_pages(GFP_DMA | __GFP_NOWARN,
@@ -343,15 +312,15 @@ swiotlb_late_init_with_default_size(size_t default_size)
}
if (!vstart) {
- io_tlb_nslabs = req_nslabs;
+ mem->nslabs = req_nslabs;
return -ENOMEM;
}
if (order != get_order(bytes)) {
pr_warn("only able to allocate %ld MB\n",
(PAGE_SIZE << order) >> 20);
- io_tlb_nslabs = SLABS_PER_PAGE << order;
+ mem->nslabs = SLABS_PER_PAGE << order;
}
- rc = swiotlb_late_init_with_tbl(vstart, io_tlb_nslabs);
+ rc = swiotlb_late_init_with_tbl(vstart, mem->nslabs);
if (rc)
free_pages((unsigned long)vstart, order);
@@ -360,26 +329,32 @@ swiotlb_late_init_with_default_size(size_t default_size)
static void swiotlb_cleanup(void)
{
- io_tlb_end = 0;
- io_tlb_start = 0;
- io_tlb_nslabs = 0;
+ struct io_tlb_mem *mem = &io_tlb_default_mem;
+
+ mem->end = 0;
+ mem->start = 0;
+ mem->nslabs = 0;
max_segment = 0;
}
int
swiotlb_late_init_with_tbl(char *tlb, unsigned long nslabs)
{
+ struct io_tlb_mem *mem = &io_tlb_default_mem;
unsigned long i, bytes;
/* protect against double initialization */
- if (WARN_ON_ONCE(io_tlb_start))
+ if (WARN_ON_ONCE(mem->start))
return -ENOMEM;
bytes = nslabs << IO_TLB_SHIFT;
- io_tlb_nslabs = nslabs;
- io_tlb_start = virt_to_phys(tlb);
- io_tlb_end = io_tlb_start + bytes;
+ mem->nslabs = nslabs;
+ mem->start = virt_to_phys(tlb);
+ mem->end = mem->start + bytes;
+ mem->index = 0;
+ mem->late_alloc = 1;
+ spin_lock_init(&mem->lock);
set_memory_decrypted((unsigned long)tlb, bytes >> PAGE_SHIFT);
memset(tlb, 0, bytes);
@@ -387,52 +362,45 @@ swiotlb_late_init_with_tbl(char *tlb, unsigned long nslabs)
/*
* Allocate and initialize the free list array. This array is used
* to find contiguous free memory regions of size up to IO_TLB_SEGSIZE
- * between io_tlb_start and io_tlb_end.
+ * between mem->start and mem->end.
*/
- io_tlb_list = (unsigned int *)__get_free_pages(GFP_KERNEL,
- get_order(io_tlb_nslabs * sizeof(int)));
- if (!io_tlb_list)
+ mem->list = (unsigned int *)__get_free_pages(GFP_KERNEL,
+ get_order(mem->nslabs * sizeof(int)));
+ if (!mem->list)
goto cleanup3;
- io_tlb_orig_addr = (phys_addr_t *)
+ mem->orig_addr = (phys_addr_t *)
__get_free_pages(GFP_KERNEL,
- get_order(io_tlb_nslabs *
+ get_order(mem->nslabs *
sizeof(phys_addr_t)));
- if (!io_tlb_orig_addr)
+ if (!mem->orig_addr)
goto cleanup4;
- io_tlb_alloc_size = (size_t *)
+ mem->alloc_size = (size_t *)
__get_free_pages(GFP_KERNEL,
- get_order(io_tlb_nslabs *
+ get_order(mem->nslabs *
sizeof(size_t)));
- if (!io_tlb_alloc_size)
+ if (!mem->alloc_size)
goto cleanup5;
-
- for (i = 0; i < io_tlb_nslabs; i++) {
- io_tlb_list[i] = IO_TLB_SEGSIZE - io_tlb_offset(i);
- io_tlb_orig_addr[i] = INVALID_PHYS_ADDR;
- io_tlb_alloc_size[i] = 0;
+ for (i = 0; i < mem->nslabs; i++) {
+ mem->list[i] = IO_TLB_SEGSIZE - io_tlb_offset(i);
+ mem->orig_addr[i] = INVALID_PHYS_ADDR;
+ mem->alloc_size[i] = 0;
}
- io_tlb_index = 0;
no_iotlb_memory = false;
swiotlb_print_info();
-
- late_alloc = 1;
-
- swiotlb_set_max_segment(io_tlb_nslabs << IO_TLB_SHIFT);
-
+ swiotlb_set_max_segment(mem->nslabs << IO_TLB_SHIFT);
return 0;
cleanup5:
- free_pages((unsigned long)io_tlb_orig_addr, get_order(io_tlb_nslabs *
- sizeof(phys_addr_t)));
-
+ free_pages((unsigned long)mem->orig_addr,
+ get_order(mem->nslabs * sizeof(phys_addr_t)));
cleanup4:
- free_pages((unsigned long)io_tlb_list, get_order(io_tlb_nslabs *
- sizeof(int)));
- io_tlb_list = NULL;
+ free_pages((unsigned long)mem->list,
+ get_order(mem->nslabs * sizeof(int)));
+ mem->list = NULL;
cleanup3:
swiotlb_cleanup();
return -ENOMEM;
@@ -440,27 +408,29 @@ swiotlb_late_init_with_tbl(char *tlb, unsigned long nslabs)
void __init swiotlb_exit(void)
{
- if (!io_tlb_orig_addr)
+ struct io_tlb_mem *mem = &io_tlb_default_mem;
+
+ if (!mem->orig_addr)
return;
- if (late_alloc) {
- free_pages((unsigned long)io_tlb_alloc_size,
- get_order(io_tlb_nslabs * sizeof(size_t)));
- free_pages((unsigned long)io_tlb_orig_addr,
- get_order(io_tlb_nslabs * sizeof(phys_addr_t)));
- free_pages((unsigned long)io_tlb_list, get_order(io_tlb_nslabs *
- sizeof(int)));
- free_pages((unsigned long)phys_to_virt(io_tlb_start),
- get_order(io_tlb_nslabs << IO_TLB_SHIFT));
+ if (mem->late_alloc) {
+ free_pages((unsigned long)mem->alloc_size,
+ get_order(mem->nslabs * sizeof(size_t)));
+ free_pages((unsigned long)mem->orig_addr,
+ get_order(mem->nslabs * sizeof(phys_addr_t)));
+ free_pages((unsigned long)mem->list,
+ get_order(mem->nslabs * sizeof(int)));
+ free_pages((unsigned long)phys_to_virt(mem->start),
+ get_order(mem->nslabs << IO_TLB_SHIFT));
} else {
- memblock_free_late(__pa(io_tlb_orig_addr),
- PAGE_ALIGN(io_tlb_nslabs * sizeof(phys_addr_t)));
- memblock_free_late(__pa(io_tlb_alloc_size),
- PAGE_ALIGN(io_tlb_nslabs * sizeof(size_t)));
- memblock_free_late(__pa(io_tlb_list),
- PAGE_ALIGN(io_tlb_nslabs * sizeof(int)));
- memblock_free_late(io_tlb_start,
- PAGE_ALIGN(io_tlb_nslabs << IO_TLB_SHIFT));
+ memblock_free_late(__pa(mem->alloc_size),
+ PAGE_ALIGN(mem->nslabs * sizeof(size_t)));
+ memblock_free_late(__pa(mem->orig_addr),
+ PAGE_ALIGN(mem->nslabs * sizeof(phys_addr_t)));
+ memblock_free_late(__pa(mem->list),
+ PAGE_ALIGN(mem->nslabs * sizeof(int)));
+ memblock_free_late(mem->start,
+ PAGE_ALIGN(mem->nslabs << IO_TLB_SHIFT));
}
swiotlb_cleanup();
}
@@ -471,9 +441,10 @@ void __init swiotlb_exit(void)
static void swiotlb_bounce(struct device *dev, phys_addr_t tlb_addr, size_t size,
enum dma_data_direction dir)
{
- int index = (tlb_addr - io_tlb_start) >> IO_TLB_SHIFT;
- size_t alloc_size = io_tlb_alloc_size[index];
- phys_addr_t orig_addr = io_tlb_orig_addr[index];
+ struct io_tlb_mem *mem = &io_tlb_default_mem;
+ int index = (tlb_addr - mem->start) >> IO_TLB_SHIFT;
+ phys_addr_t orig_addr = mem->orig_addr[index];
+ size_t alloc_size = mem->alloc_size[index];
unsigned long pfn = PFN_DOWN(orig_addr);
unsigned char *vaddr = phys_to_virt(tlb_addr);
@@ -538,9 +509,9 @@ static inline unsigned long get_max_slots(unsigned long boundary_mask)
return nr_slots(boundary_mask + 1);
}
-static unsigned int wrap_index(unsigned int index)
+static unsigned int wrap_index(struct io_tlb_mem *mem, unsigned int index)
{
- if (index >= io_tlb_nslabs)
+ if (index >= mem->nslabs)
return 0;
return index;
}
@@ -552,9 +523,10 @@ static unsigned int wrap_index(unsigned int index)
static int find_slots(struct device *dev, phys_addr_t orig_addr,
size_t alloc_size)
{
+ struct io_tlb_mem *mem = &io_tlb_default_mem;
unsigned long boundary_mask = dma_get_seg_boundary(dev);
dma_addr_t tbl_dma_addr =
- phys_to_dma_unencrypted(dev, io_tlb_start) & boundary_mask;
+ phys_to_dma_unencrypted(dev, mem->start) & boundary_mask;
unsigned long max_slots = get_max_slots(boundary_mask);
unsigned int iotlb_align_mask =
dma_get_min_align_mask(dev) & ~(IO_TLB_SIZE - 1);
@@ -573,15 +545,15 @@ static int find_slots(struct device *dev, phys_addr_t orig_addr,
if (alloc_size >= PAGE_SIZE)
stride = max(stride, stride << (PAGE_SHIFT - IO_TLB_SHIFT));
- spin_lock_irqsave(&io_tlb_lock, flags);
- if (unlikely(nslots > io_tlb_nslabs - io_tlb_used))
+ spin_lock_irqsave(&mem->lock, flags);
+ if (unlikely(nslots > mem->nslabs - mem->used))
goto not_found;
- index = wrap = wrap_index(ALIGN(io_tlb_index, stride));
+ index = wrap = wrap_index(mem, ALIGN(mem->index, stride));
do {
if ((slot_addr(tbl_dma_addr, index) & iotlb_align_mask) !=
(orig_addr & iotlb_align_mask)) {
- index = wrap_index(index + 1);
+ index = wrap_index(mem, index + 1);
continue;
}
@@ -593,34 +565,34 @@ static int find_slots(struct device *dev, phys_addr_t orig_addr,
if (!iommu_is_span_boundary(index, nslots,
nr_slots(tbl_dma_addr),
max_slots)) {
- if (io_tlb_list[index] >= nslots)
+ if (mem->list[index] >= nslots)
goto found;
}
- index = wrap_index(index + stride);
+ index = wrap_index(mem, index + stride);
} while (index != wrap);
not_found:
- spin_unlock_irqrestore(&io_tlb_lock, flags);
+ spin_unlock_irqrestore(&mem->lock, flags);
return -1;
found:
for (i = index; i < index + nslots; i++)
- io_tlb_list[i] = 0;
+ mem->list[i] = 0;
for (i = index - 1;
io_tlb_offset(i) != IO_TLB_SEGSIZE - 1 &&
- io_tlb_list[i]; i--)
- io_tlb_list[i] = ++count;
+ mem->list[i]; i--)
+ mem->list[i] = ++count;
/*
* Update the indices to avoid searching in the next round.
*/
- if (index + nslots < io_tlb_nslabs)
- io_tlb_index = index + nslots;
+ if (index + nslots < mem->nslabs)
+ mem->index = index + nslots;
else
- io_tlb_index = 0;
- io_tlb_used += nslots;
+ mem->index = 0;
+ mem->used += nslots;
- spin_unlock_irqrestore(&io_tlb_lock, flags);
+ spin_unlock_irqrestore(&mem->lock, flags);
return index;
}
@@ -628,6 +600,7 @@ phys_addr_t swiotlb_tbl_map_single(struct device *dev, phys_addr_t orig_addr,
size_t mapping_size, size_t alloc_size,
enum dma_data_direction dir, unsigned long attrs)
{
+ struct io_tlb_mem *mem = &io_tlb_default_mem;
unsigned int offset = swiotlb_align_offset(dev, orig_addr);
unsigned int index, i;
phys_addr_t tlb_addr;
@@ -649,7 +622,7 @@ phys_addr_t swiotlb_tbl_map_single(struct device *dev, phys_addr_t orig_addr,
if (!(attrs & DMA_ATTR_NO_WARN))
dev_warn_ratelimited(dev,
"swiotlb buffer is full (sz: %zd bytes), total %lu (slots), used %lu (slots)\n",
- alloc_size, io_tlb_nslabs, io_tlb_used);
+ alloc_size, mem->nslabs, mem->used);
return (phys_addr_t)DMA_MAPPING_ERROR;
}
@@ -659,10 +632,10 @@ phys_addr_t swiotlb_tbl_map_single(struct device *dev, phys_addr_t orig_addr,
* needed.
*/
for (i = 0; i < nr_slots(alloc_size + offset); i++) {
- io_tlb_orig_addr[index + i] = slot_addr(orig_addr, i);
- io_tlb_alloc_size[index+i] = alloc_size - (i << IO_TLB_SHIFT);
+ mem->orig_addr[index + i] = slot_addr(orig_addr, i);
+ mem->alloc_size[index + i] = alloc_size - (i << IO_TLB_SHIFT);
}
- tlb_addr = slot_addr(io_tlb_start, index) + offset;
+ tlb_addr = slot_addr(mem->start, index) + offset;
if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC) &&
(dir == DMA_TO_DEVICE || dir == DMA_BIDIRECTIONAL))
swiotlb_bounce(dev, tlb_addr, mapping_size, DMA_TO_DEVICE);
@@ -676,10 +649,11 @@ void swiotlb_tbl_unmap_single(struct device *hwdev, phys_addr_t tlb_addr,
size_t mapping_size, enum dma_data_direction dir,
unsigned long attrs)
{
+ struct io_tlb_mem *mem = &io_tlb_default_mem;
unsigned long flags;
unsigned int offset = swiotlb_align_offset(hwdev, tlb_addr);
- int index = (tlb_addr - offset - io_tlb_start) >> IO_TLB_SHIFT;
- int nslots = nr_slots(io_tlb_alloc_size[index] + offset);
+ int index = (tlb_addr - offset - mem->start) >> IO_TLB_SHIFT;
+ int nslots = nr_slots(mem->alloc_size[index] + offset);
int count, i;
/*
@@ -695,9 +669,9 @@ void swiotlb_tbl_unmap_single(struct device *hwdev, phys_addr_t tlb_addr,
* While returning the entries to the free list, we merge the entries
* with slots below and above the pool being returned.
*/
- spin_lock_irqsave(&io_tlb_lock, flags);
+ spin_lock_irqsave(&mem->lock, flags);
if (index + nslots < ALIGN(index + 1, IO_TLB_SEGSIZE))
- count = io_tlb_list[index + nslots];
+ count = mem->list[index + nslots];
else
count = 0;
@@ -706,9 +680,9 @@ void swiotlb_tbl_unmap_single(struct device *hwdev, phys_addr_t tlb_addr,
* superceeding slots
*/
for (i = index + nslots - 1; i >= index; i--) {
- io_tlb_list[i] = ++count;
- io_tlb_orig_addr[i] = INVALID_PHYS_ADDR;
- io_tlb_alloc_size[i] = 0;
+ mem->list[i] = ++count;
+ mem->orig_addr[i] = INVALID_PHYS_ADDR;
+ mem->alloc_size[i] = 0;
}
/*
@@ -716,11 +690,11 @@ void swiotlb_tbl_unmap_single(struct device *hwdev, phys_addr_t tlb_addr,
* available (non zero)
*/
for (i = index - 1;
- io_tlb_offset(i) != IO_TLB_SEGSIZE - 1 && io_tlb_list[i];
+ io_tlb_offset(i) != IO_TLB_SEGSIZE - 1 && mem->list[i];
i--)
- io_tlb_list[i] = ++count;
- io_tlb_used -= nslots;
- spin_unlock_irqrestore(&io_tlb_lock, flags);
+ mem->list[i] = ++count;
+ mem->used -= nslots;
+ spin_unlock_irqrestore(&mem->lock, flags);
}
void swiotlb_sync_single_for_device(struct device *dev, phys_addr_t tlb_addr,
@@ -783,21 +757,21 @@ size_t swiotlb_max_mapping_size(struct device *dev)
bool is_swiotlb_active(void)
{
/*
- * When SWIOTLB is initialized, even if io_tlb_start points to physical
- * address zero, io_tlb_end surely doesn't.
+ * When SWIOTLB is initialized, even if mem->start points to physical
+ * address zero, mem->end surely doesn't.
*/
- return io_tlb_end != 0;
+ return io_tlb_default_mem.end != 0;
}
#ifdef CONFIG_DEBUG_FS
static int __init swiotlb_create_debugfs(void)
{
- struct dentry *root;
+ struct io_tlb_mem *mem = &io_tlb_default_mem;
- root = debugfs_create_dir("swiotlb", NULL);
- debugfs_create_ulong("io_tlb_nslabs", 0400, root, &io_tlb_nslabs);
- debugfs_create_ulong("io_tlb_used", 0400, root, &io_tlb_used);
+ mem->debugfs = debugfs_create_dir("swiotlb", NULL);
+ debugfs_create_ulong("io_tlb_nslabs", 0400, mem->debugfs, &mem->nslabs);
+ debugfs_create_ulong("io_tlb_used", 0400, mem->debugfs, &mem->used);
return 0;
}
--
2.30.1
^ permalink raw reply related
* [PATCH 2/3] swiotlb: dynamically allocate io_tlb_default_mem
From: Christoph Hellwig @ 2021-03-18 16:14 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: iommu, xen-devel, Claire Chang, linuxppc-dev, Dongli Zhang
In-Reply-To: <20210318161424.489045-1-hch@lst.de>
Instead of allocating ->list and ->orig_addr separately just do one
dynamic allocation for the actual io_tlb_mem structure. This simplifies
a lot of the initialization code, and also allows to just check
io_tlb_default_mem to see if swiotlb is in use.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/xen/swiotlb-xen.c | 22 +--
include/linux/swiotlb.h | 18 ++-
kernel/dma/swiotlb.c | 306 ++++++++++++--------------------------
3 files changed, 117 insertions(+), 229 deletions(-)
diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
index 5329ad54a5f34e..4c89afc0df6289 100644
--- a/drivers/xen/swiotlb-xen.c
+++ b/drivers/xen/swiotlb-xen.c
@@ -158,17 +158,14 @@ static const char *xen_swiotlb_error(enum xen_swiotlb_err err)
int __ref xen_swiotlb_init(void)
{
enum xen_swiotlb_err m_ret = XEN_SWIOTLB_UNKNOWN;
- unsigned long nslabs, bytes, order;
- unsigned int repeat = 3;
+ unsigned long bytes = swiotlb_size_or_default();
+ unsigned long nslabs = bytes >> IO_TLB_SHIFT;
+ unsigned int order, repeat = 3;
int rc = -ENOMEM;
char *start;
- nslabs = swiotlb_nr_tbl();
- if (!nslabs)
- nslabs = DEFAULT_NSLABS;
retry:
m_ret = XEN_SWIOTLB_ENOMEM;
- bytes = nslabs << IO_TLB_SHIFT;
order = get_order(bytes);
/*
@@ -221,19 +218,16 @@ int __ref xen_swiotlb_init(void)
#ifdef CONFIG_X86
void __init xen_swiotlb_init_early(void)
{
- unsigned long nslabs, bytes;
+ unsigned long bytes = swiotlb_size_or_default();
+ unsigned long nslabs = bytes >> IO_TLB_SHIFT;
unsigned int repeat = 3;
char *start;
int rc;
- nslabs = swiotlb_nr_tbl();
- if (!nslabs)
- nslabs = DEFAULT_NSLABS;
retry:
/*
* Get IO TLB memory from any location.
*/
- bytes = nslabs << IO_TLB_SHIFT;
start = memblock_alloc(PAGE_ALIGN(bytes), PAGE_SIZE);
if (!start)
panic("%s: Failed to allocate %lu bytes align=0x%lx\n",
@@ -248,8 +242,8 @@ void __init xen_swiotlb_init_early(void)
if (repeat--) {
/* Min is 2MB */
nslabs = max(1024UL, (nslabs >> 1));
- pr_info("Lowering to %luMB\n",
- (nslabs << IO_TLB_SHIFT) >> 20);
+ bytes = nslabs << IO_TLB_SHIFT;
+ pr_info("Lowering to %luMB\n", bytes >> 20);
goto retry;
}
panic("%s (rc:%d)", xen_swiotlb_error(XEN_SWIOTLB_EFIXUP), rc);
@@ -548,7 +542,7 @@ xen_swiotlb_sync_sg_for_device(struct device *dev, struct scatterlist *sgl,
static int
xen_swiotlb_dma_supported(struct device *hwdev, u64 mask)
{
- return xen_phys_to_dma(hwdev, io_tlb_default_mem.end - 1) <= mask;
+ return xen_phys_to_dma(hwdev, io_tlb_default_mem->end - 1) <= mask;
}
const struct dma_map_ops xen_swiotlb_dma_ops = {
diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index 5ec5378b17c333..63f7a63f61d098 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -90,28 +90,30 @@ struct io_tlb_mem {
phys_addr_t end;
unsigned long nslabs;
unsigned long used;
- unsigned int *list;
unsigned int index;
- phys_addr_t *orig_addr;
- size_t *alloc_size;
spinlock_t lock;
struct dentry *debugfs;
bool late_alloc;
+ struct io_tlb_slot {
+ phys_addr_t orig_addr;
+ size_t alloc_size;
+ unsigned int list;
+ } slots[];
};
-extern struct io_tlb_mem io_tlb_default_mem;
+extern struct io_tlb_mem *io_tlb_default_mem;
static inline bool is_swiotlb_buffer(phys_addr_t paddr)
{
- struct io_tlb_mem *mem = &io_tlb_default_mem;
+ struct io_tlb_mem *mem = io_tlb_default_mem;
- return paddr >= mem->start && paddr < mem->end;
+ return mem && paddr >= mem->start && paddr < mem->end;
}
void __init swiotlb_exit(void);
unsigned int swiotlb_max_segment(void);
size_t swiotlb_max_mapping_size(struct device *dev);
bool is_swiotlb_active(void);
-void __init swiotlb_adjust_size(unsigned long new_size);
+void __init swiotlb_adjust_size(unsigned long size);
#else
#define swiotlb_force SWIOTLB_NO_FORCE
static inline bool is_swiotlb_buffer(phys_addr_t paddr)
@@ -135,7 +137,7 @@ static inline bool is_swiotlb_active(void)
return false;
}
-static inline void swiotlb_adjust_size(unsigned long new_size)
+static inline void swiotlb_adjust_size(unsigned long size)
{
}
#endif /* CONFIG_SWIOTLB */
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index d9c097f0f78cec..13de669a9b4681 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -63,7 +63,7 @@
enum swiotlb_force swiotlb_force;
-struct io_tlb_mem io_tlb_default_mem;
+struct io_tlb_mem *io_tlb_default_mem;
/*
* Max segment that we can provide which (if pages are contingous) will
@@ -71,15 +71,15 @@ struct io_tlb_mem io_tlb_default_mem;
*/
static unsigned int max_segment;
+static unsigned long default_nslabs = IO_TLB_DEFAULT_SIZE >> IO_TLB_SHIFT;
+
static int __init
setup_io_tlb_npages(char *str)
{
- struct io_tlb_mem *mem = &io_tlb_default_mem;
-
if (isdigit(*str)) {
- mem->nslabs = simple_strtoul(str, &str, 0);
/* avoid tail segment of size < IO_TLB_SEGSIZE */
- mem->nslabs = ALIGN(mem->nslabs, IO_TLB_SEGSIZE);
+ default_nslabs =
+ ALIGN(simple_strtoul(str, &str, 0), IO_TLB_SEGSIZE);
}
if (*str == ',')
++str;
@@ -87,24 +87,22 @@ setup_io_tlb_npages(char *str)
swiotlb_force = SWIOTLB_FORCE;
} else if (!strcmp(str, "noforce")) {
swiotlb_force = SWIOTLB_NO_FORCE;
- mem->nslabs = 1;
+ default_nslabs = 1;
}
return 0;
}
early_param("swiotlb", setup_io_tlb_npages);
-static bool no_iotlb_memory;
-
unsigned long swiotlb_nr_tbl(void)
{
- return unlikely(no_iotlb_memory) ? 0 : io_tlb_default_mem.nslabs;
+ return io_tlb_default_mem ? io_tlb_default_mem->nslabs : 0;
}
EXPORT_SYMBOL_GPL(swiotlb_nr_tbl);
unsigned int swiotlb_max_segment(void)
{
- return unlikely(no_iotlb_memory) ? 0 : max_segment;
+ return io_tlb_default_mem ? max_segment : 0;
}
EXPORT_SYMBOL_GPL(swiotlb_max_segment);
@@ -118,44 +116,32 @@ void swiotlb_set_max_segment(unsigned int val)
unsigned long swiotlb_size_or_default(void)
{
- unsigned long size;
-
- size = io_tlb_default_mem.nslabs << IO_TLB_SHIFT;
-
- return size ? size : (IO_TLB_DEFAULT_SIZE);
+ return default_nslabs << IO_TLB_SHIFT;
}
-void __init swiotlb_adjust_size(unsigned long new_size)
+void __init swiotlb_adjust_size(unsigned long size)
{
- struct io_tlb_mem *mem = &io_tlb_default_mem;
- unsigned long size;
-
/*
* If swiotlb parameter has not been specified, give a chance to
* architectures such as those supporting memory encryption to
* adjust/expand SWIOTLB size for their use.
*/
- if (!mem->nslabs) {
- size = ALIGN(new_size, IO_TLB_SIZE);
- mem->nslabs = size >> IO_TLB_SHIFT;
- mem->nslabs = ALIGN(mem->nslabs, IO_TLB_SEGSIZE);
-
- pr_info("SWIOTLB bounce buffer size adjusted to %luMB", size >> 20);
- }
+ size = ALIGN(size, IO_TLB_SIZE);
+ default_nslabs = ALIGN(size >> IO_TLB_SHIFT, IO_TLB_SEGSIZE);
+ pr_info("SWIOTLB bounce buffer size adjusted to %luMB", size >> 20);
}
void swiotlb_print_info(void)
{
- struct io_tlb_mem *mem = &io_tlb_default_mem;
- unsigned long bytes = mem->nslabs << IO_TLB_SHIFT;
+ struct io_tlb_mem *mem = io_tlb_default_mem;
- if (no_iotlb_memory) {
+ if (!mem) {
pr_warn("No low mem\n");
return;
}
pr_info("mapped [mem %pa-%pa] (%luMB)\n", &mem->start, &mem->end,
- bytes >> 20);
+ (mem->nslabs << IO_TLB_SHIFT) >> 20);
}
static inline unsigned long io_tlb_offset(unsigned long val)
@@ -176,13 +162,12 @@ static inline unsigned long nr_slots(u64 val)
*/
void __init swiotlb_update_mem_attributes(void)
{
- struct io_tlb_mem *mem = &io_tlb_default_mem;
+ struct io_tlb_mem *mem = io_tlb_default_mem;
void *vaddr;
unsigned long bytes;
- if (no_iotlb_memory || mem->late_alloc)
+ if (!mem || mem->late_alloc)
return;
-
vaddr = phys_to_virt(mem->start);
bytes = PAGE_ALIGN(mem->nslabs << IO_TLB_SHIFT);
set_memory_decrypted((unsigned long)vaddr, bytes >> PAGE_SHIFT);
@@ -191,55 +176,33 @@ void __init swiotlb_update_mem_attributes(void)
int __init swiotlb_init_with_tbl(char *tlb, unsigned long nslabs, int verbose)
{
- struct io_tlb_mem *mem = &io_tlb_default_mem;
- unsigned long i, bytes;
+ unsigned long bytes = nslabs << IO_TLB_SHIFT, i;
+ struct io_tlb_mem *mem;
size_t alloc_size;
/* protect against double initialization */
- if (WARN_ON_ONCE(mem->start))
+ if (WARN_ON_ONCE(io_tlb_default_mem))
return -ENOMEM;
- bytes = nslabs << IO_TLB_SHIFT;
-
+ alloc_size = PAGE_ALIGN(struct_size(mem, slots, nslabs));
+ mem = memblock_alloc(alloc_size, PAGE_SIZE);
+ if (!mem)
+ panic("%s: Failed to allocate %zu bytes align=0x%lx\n",
+ __func__, alloc_size, PAGE_SIZE);
mem->nslabs = nslabs;
mem->start = __pa(tlb);
mem->end = mem->start + bytes;
mem->index = 0;
spin_lock_init(&mem->lock);
-
- /*
- * Allocate and initialize the free list array. This array is used
- * to find contiguous free memory regions of size up to IO_TLB_SEGSIZE
- * between mem->start and mem->end.
- */
- alloc_size = PAGE_ALIGN(mem->nslabs * sizeof(int));
- mem->list = memblock_alloc(alloc_size, PAGE_SIZE);
- if (!mem->list)
- panic("%s: Failed to allocate %zu bytes align=0x%lx\n",
- __func__, alloc_size, PAGE_SIZE);
-
- alloc_size = PAGE_ALIGN(mem->nslabs * sizeof(phys_addr_t));
- mem->orig_addr = memblock_alloc(alloc_size, PAGE_SIZE);
- if (!mem->orig_addr)
- panic("%s: Failed to allocate %zu bytes align=0x%lx\n",
- __func__, alloc_size, PAGE_SIZE);
-
- alloc_size = PAGE_ALIGN(mem->nslabs * sizeof(size_t));
- mem->alloc_size = memblock_alloc(alloc_size, PAGE_SIZE);
- if (mem->alloc_size)
- panic("%s: Failed to allocate %zu bytes align=0x%lx\n",
- __func__, alloc_size, PAGE_SIZE);
-
for (i = 0; i < mem->nslabs; i++) {
- mem->list[i] = IO_TLB_SEGSIZE - io_tlb_offset(i);
- mem->orig_addr[i] = INVALID_PHYS_ADDR;
- mem->alloc_size[i] = 0;
+ mem->slots[i].list = IO_TLB_SEGSIZE - io_tlb_offset(i);
+ mem->slots[i].orig_addr = INVALID_PHYS_ADDR;
+ mem->slots[i].alloc_size = 0;
}
- no_iotlb_memory = false;
+ io_tlb_default_mem = mem;
if (verbose)
swiotlb_print_info();
-
swiotlb_set_max_segment(mem->nslabs << IO_TLB_SHIFT);
return 0;
}
@@ -251,30 +214,21 @@ int __init swiotlb_init_with_tbl(char *tlb, unsigned long nslabs, int verbose)
void __init
swiotlb_init(int verbose)
{
- struct io_tlb_mem *mem = &io_tlb_default_mem;
- size_t default_size = IO_TLB_DEFAULT_SIZE;
- unsigned char *vstart;
- unsigned long bytes;
-
- if (!mem->nslabs) {
- mem->nslabs = (default_size >> IO_TLB_SHIFT);
- mem->nslabs = ALIGN(mem->nslabs, IO_TLB_SEGSIZE);
- }
-
- bytes = mem->nslabs << IO_TLB_SHIFT;
+ size_t bytes = PAGE_ALIGN(default_nslabs << IO_TLB_SHIFT);
+ void *tlb;
/* Get IO TLB memory from the low pages */
- vstart = memblock_alloc_low(PAGE_ALIGN(bytes), PAGE_SIZE);
- if (vstart && !swiotlb_init_with_tbl(vstart, mem->nslabs, verbose))
- return;
-
- if (mem->start) {
- memblock_free_early(mem->start,
- PAGE_ALIGN(mem->nslabs << IO_TLB_SHIFT));
- mem->start = 0;
- }
+ tlb = memblock_alloc_low(bytes, PAGE_SIZE);
+ if (!tlb)
+ goto fail;
+ if (swiotlb_init_with_tbl(tlb, default_nslabs, verbose))
+ goto fail_free_mem;
+ return;
+
+fail_free_mem:
+ memblock_free_early(__pa(tlb), bytes);
+fail:
pr_warn("Cannot allocate buffer");
- no_iotlb_memory = true;
}
/*
@@ -285,23 +239,19 @@ swiotlb_init(int verbose)
int
swiotlb_late_init_with_default_size(size_t default_size)
{
- struct io_tlb_mem *mem = &io_tlb_default_mem;
- unsigned long bytes, req_nslabs = mem->nslabs;
+ unsigned long nslabs =
+ ALIGN(default_size >> IO_TLB_SHIFT, IO_TLB_SEGSIZE);
+ unsigned long bytes;
unsigned char *vstart = NULL;
unsigned int order;
int rc = 0;
- if (!mem->nslabs) {
- mem->nslabs = (default_size >> IO_TLB_SHIFT);
- mem->nslabs = ALIGN(mem->nslabs, IO_TLB_SEGSIZE);
- }
-
/*
* Get IO TLB memory from the low pages
*/
- order = get_order(mem->nslabs << IO_TLB_SHIFT);
- mem->nslabs = SLABS_PER_PAGE << order;
- bytes = mem->nslabs << IO_TLB_SHIFT;
+ order = get_order(nslabs << IO_TLB_SHIFT);
+ nslabs = SLABS_PER_PAGE << order;
+ bytes = nslabs << IO_TLB_SHIFT;
while ((SLABS_PER_PAGE << order) > IO_TLB_MIN_SLABS) {
vstart = (void *)__get_free_pages(GFP_DMA | __GFP_NOWARN,
@@ -311,43 +261,35 @@ swiotlb_late_init_with_default_size(size_t default_size)
order--;
}
- if (!vstart) {
- mem->nslabs = req_nslabs;
+ if (!vstart)
return -ENOMEM;
- }
+
if (order != get_order(bytes)) {
pr_warn("only able to allocate %ld MB\n",
(PAGE_SIZE << order) >> 20);
- mem->nslabs = SLABS_PER_PAGE << order;
+ nslabs = SLABS_PER_PAGE << order;
}
- rc = swiotlb_late_init_with_tbl(vstart, mem->nslabs);
+ rc = swiotlb_late_init_with_tbl(vstart, nslabs);
if (rc)
free_pages((unsigned long)vstart, order);
return rc;
}
-static void swiotlb_cleanup(void)
-{
- struct io_tlb_mem *mem = &io_tlb_default_mem;
-
- mem->end = 0;
- mem->start = 0;
- mem->nslabs = 0;
- max_segment = 0;
-}
-
int
swiotlb_late_init_with_tbl(char *tlb, unsigned long nslabs)
{
- struct io_tlb_mem *mem = &io_tlb_default_mem;
- unsigned long i, bytes;
+ unsigned long bytes = nslabs << IO_TLB_SHIFT, i;
+ struct io_tlb_mem *mem;
/* protect against double initialization */
- if (WARN_ON_ONCE(mem->start))
+ if (WARN_ON_ONCE(io_tlb_default_mem))
return -ENOMEM;
- bytes = nslabs << IO_TLB_SHIFT;
+ mem = (void *)__get_free_pages(GFP_KERNEL,
+ get_order(struct_size(mem, slots, nslabs)));
+ if (!mem)
+ return -ENOMEM;
mem->nslabs = nslabs;
mem->start = virt_to_phys(tlb);
@@ -355,84 +297,35 @@ swiotlb_late_init_with_tbl(char *tlb, unsigned long nslabs)
mem->index = 0;
mem->late_alloc = 1;
spin_lock_init(&mem->lock);
+ for (i = 0; i < mem->nslabs; i++) {
+ mem->slots[i].list = IO_TLB_SEGSIZE - io_tlb_offset(i);
+ mem->slots[i].orig_addr = INVALID_PHYS_ADDR;
+ mem->slots[i].alloc_size = 0;
+ }
set_memory_decrypted((unsigned long)tlb, bytes >> PAGE_SHIFT);
memset(tlb, 0, bytes);
- /*
- * Allocate and initialize the free list array. This array is used
- * to find contiguous free memory regions of size up to IO_TLB_SEGSIZE
- * between mem->start and mem->end.
- */
- mem->list = (unsigned int *)__get_free_pages(GFP_KERNEL,
- get_order(mem->nslabs * sizeof(int)));
- if (!mem->list)
- goto cleanup3;
-
- mem->orig_addr = (phys_addr_t *)
- __get_free_pages(GFP_KERNEL,
- get_order(mem->nslabs *
- sizeof(phys_addr_t)));
- if (!mem->orig_addr)
- goto cleanup4;
-
- mem->alloc_size = (size_t *)
- __get_free_pages(GFP_KERNEL,
- get_order(mem->nslabs *
- sizeof(size_t)));
- if (!mem->alloc_size)
- goto cleanup5;
-
- for (i = 0; i < mem->nslabs; i++) {
- mem->list[i] = IO_TLB_SEGSIZE - io_tlb_offset(i);
- mem->orig_addr[i] = INVALID_PHYS_ADDR;
- mem->alloc_size[i] = 0;
- }
- no_iotlb_memory = false;
-
+ io_tlb_default_mem = mem;
swiotlb_print_info();
swiotlb_set_max_segment(mem->nslabs << IO_TLB_SHIFT);
return 0;
-
-cleanup5:
- free_pages((unsigned long)mem->orig_addr,
- get_order(mem->nslabs * sizeof(phys_addr_t)));
-cleanup4:
- free_pages((unsigned long)mem->list,
- get_order(mem->nslabs * sizeof(int)));
- mem->list = NULL;
-cleanup3:
- swiotlb_cleanup();
- return -ENOMEM;
}
void __init swiotlb_exit(void)
{
- struct io_tlb_mem *mem = &io_tlb_default_mem;
+ struct io_tlb_mem *mem = io_tlb_default_mem;
+ size_t size;
- if (!mem->orig_addr)
+ if (!mem)
return;
- if (mem->late_alloc) {
- free_pages((unsigned long)mem->alloc_size,
- get_order(mem->nslabs * sizeof(size_t)));
- free_pages((unsigned long)mem->orig_addr,
- get_order(mem->nslabs * sizeof(phys_addr_t)));
- free_pages((unsigned long)mem->list,
- get_order(mem->nslabs * sizeof(int)));
- free_pages((unsigned long)phys_to_virt(mem->start),
- get_order(mem->nslabs << IO_TLB_SHIFT));
- } else {
- memblock_free_late(__pa(mem->alloc_size),
- PAGE_ALIGN(mem->nslabs * sizeof(size_t)));
- memblock_free_late(__pa(mem->orig_addr),
- PAGE_ALIGN(mem->nslabs * sizeof(phys_addr_t)));
- memblock_free_late(__pa(mem->list),
- PAGE_ALIGN(mem->nslabs * sizeof(int)));
- memblock_free_late(mem->start,
- PAGE_ALIGN(mem->nslabs << IO_TLB_SHIFT));
- }
- swiotlb_cleanup();
+ size = struct_size(mem, slots, mem->nslabs);
+ if (mem->late_alloc)
+ free_pages((unsigned long)mem, get_order(size));
+ else
+ memblock_free_late(__pa(mem), PAGE_ALIGN(size));
+ io_tlb_default_mem = NULL;
}
/*
@@ -441,10 +334,10 @@ void __init swiotlb_exit(void)
static void swiotlb_bounce(struct device *dev, phys_addr_t tlb_addr, size_t size,
enum dma_data_direction dir)
{
- struct io_tlb_mem *mem = &io_tlb_default_mem;
+ struct io_tlb_mem *mem = io_tlb_default_mem;
int index = (tlb_addr - mem->start) >> IO_TLB_SHIFT;
- phys_addr_t orig_addr = mem->orig_addr[index];
- size_t alloc_size = mem->alloc_size[index];
+ phys_addr_t orig_addr = mem->slots[index].orig_addr;
+ size_t alloc_size = mem->slots[index].alloc_size;
unsigned long pfn = PFN_DOWN(orig_addr);
unsigned char *vaddr = phys_to_virt(tlb_addr);
@@ -523,7 +416,7 @@ static unsigned int wrap_index(struct io_tlb_mem *mem, unsigned int index)
static int find_slots(struct device *dev, phys_addr_t orig_addr,
size_t alloc_size)
{
- struct io_tlb_mem *mem = &io_tlb_default_mem;
+ struct io_tlb_mem *mem = io_tlb_default_mem;
unsigned long boundary_mask = dma_get_seg_boundary(dev);
dma_addr_t tbl_dma_addr =
phys_to_dma_unencrypted(dev, mem->start) & boundary_mask;
@@ -565,7 +458,7 @@ static int find_slots(struct device *dev, phys_addr_t orig_addr,
if (!iommu_is_span_boundary(index, nslots,
nr_slots(tbl_dma_addr),
max_slots)) {
- if (mem->list[index] >= nslots)
+ if (mem->slots[index].list >= nslots)
goto found;
}
index = wrap_index(mem, index + stride);
@@ -577,11 +470,11 @@ static int find_slots(struct device *dev, phys_addr_t orig_addr,
found:
for (i = index; i < index + nslots; i++)
- mem->list[i] = 0;
+ mem->slots[i].list = 0;
for (i = index - 1;
io_tlb_offset(i) != IO_TLB_SEGSIZE - 1 &&
- mem->list[i]; i--)
- mem->list[i] = ++count;
+ mem->slots[i].list; i--)
+ mem->slots[i].list = ++count;
/*
* Update the indices to avoid searching in the next round.
@@ -600,12 +493,12 @@ phys_addr_t swiotlb_tbl_map_single(struct device *dev, phys_addr_t orig_addr,
size_t mapping_size, size_t alloc_size,
enum dma_data_direction dir, unsigned long attrs)
{
- struct io_tlb_mem *mem = &io_tlb_default_mem;
+ struct io_tlb_mem *mem = io_tlb_default_mem;
unsigned int offset = swiotlb_align_offset(dev, orig_addr);
unsigned int index, i;
phys_addr_t tlb_addr;
- if (no_iotlb_memory)
+ if (!mem)
panic("Can not allocate SWIOTLB buffer earlier and can't now provide you with the DMA bounce buffer");
if (mem_encrypt_active())
@@ -632,8 +525,9 @@ phys_addr_t swiotlb_tbl_map_single(struct device *dev, phys_addr_t orig_addr,
* needed.
*/
for (i = 0; i < nr_slots(alloc_size + offset); i++) {
- mem->orig_addr[index + i] = slot_addr(orig_addr, i);
- mem->alloc_size[index + i] = alloc_size - (i << IO_TLB_SHIFT);
+ mem->slots[index + i].orig_addr = slot_addr(orig_addr, i);
+ mem->slots[index + i].alloc_size =
+ alloc_size - (i << IO_TLB_SHIFT);
}
tlb_addr = slot_addr(mem->start, index) + offset;
if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC) &&
@@ -649,11 +543,11 @@ void swiotlb_tbl_unmap_single(struct device *hwdev, phys_addr_t tlb_addr,
size_t mapping_size, enum dma_data_direction dir,
unsigned long attrs)
{
- struct io_tlb_mem *mem = &io_tlb_default_mem;
+ struct io_tlb_mem *mem = io_tlb_default_mem;
unsigned long flags;
unsigned int offset = swiotlb_align_offset(hwdev, tlb_addr);
int index = (tlb_addr - offset - mem->start) >> IO_TLB_SHIFT;
- int nslots = nr_slots(mem->alloc_size[index] + offset);
+ int nslots = nr_slots(mem->slots[index].alloc_size + offset);
int count, i;
/*
@@ -671,7 +565,7 @@ void swiotlb_tbl_unmap_single(struct device *hwdev, phys_addr_t tlb_addr,
*/
spin_lock_irqsave(&mem->lock, flags);
if (index + nslots < ALIGN(index + 1, IO_TLB_SEGSIZE))
- count = mem->list[index + nslots];
+ count = mem->slots[index + nslots].list;
else
count = 0;
@@ -680,9 +574,9 @@ void swiotlb_tbl_unmap_single(struct device *hwdev, phys_addr_t tlb_addr,
* superceeding slots
*/
for (i = index + nslots - 1; i >= index; i--) {
- mem->list[i] = ++count;
- mem->orig_addr[i] = INVALID_PHYS_ADDR;
- mem->alloc_size[i] = 0;
+ mem->slots[i].list = ++count;
+ mem->slots[i].orig_addr = INVALID_PHYS_ADDR;
+ mem->slots[i].alloc_size = 0;
}
/*
@@ -690,9 +584,9 @@ void swiotlb_tbl_unmap_single(struct device *hwdev, phys_addr_t tlb_addr,
* available (non zero)
*/
for (i = index - 1;
- io_tlb_offset(i) != IO_TLB_SEGSIZE - 1 && mem->list[i];
+ io_tlb_offset(i) != IO_TLB_SEGSIZE - 1 && mem->slots[i].list;
i--)
- mem->list[i] = ++count;
+ mem->slots[i].list = ++count;
mem->used -= nslots;
spin_unlock_irqrestore(&mem->lock, flags);
}
@@ -756,19 +650,17 @@ size_t swiotlb_max_mapping_size(struct device *dev)
bool is_swiotlb_active(void)
{
- /*
- * When SWIOTLB is initialized, even if mem->start points to physical
- * address zero, mem->end surely doesn't.
- */
- return io_tlb_default_mem.end != 0;
+ return io_tlb_default_mem != NULL;
}
#ifdef CONFIG_DEBUG_FS
static int __init swiotlb_create_debugfs(void)
{
- struct io_tlb_mem *mem = &io_tlb_default_mem;
+ struct io_tlb_mem *mem = io_tlb_default_mem;
+ if (!mem)
+ return 0;
mem->debugfs = debugfs_create_dir("swiotlb", NULL);
debugfs_create_ulong("io_tlb_nslabs", 0400, mem->debugfs, &mem->nslabs);
debugfs_create_ulong("io_tlb_used", 0400, mem->debugfs, &mem->used);
--
2.30.1
^ permalink raw reply related
* [PATCH 3/3] swiotlb: remove swiotlb_nr_tbl
From: Christoph Hellwig @ 2021-03-18 16:14 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: iommu, xen-devel, Claire Chang, linuxppc-dev, Dongli Zhang
In-Reply-To: <20210318161424.489045-1-hch@lst.de>
All callers just use it to check if swiotlb is active at all, for which
they can just use is_swiotlb_active. In the longer run drivers need
to stop using is_swiotlb_active as well, but let's do the simple step
first.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/gpu/drm/i915/gem/i915_gem_internal.c | 2 +-
drivers/gpu/drm/nouveau/nouveau_ttm.c | 2 +-
drivers/pci/xen-pcifront.c | 2 +-
include/linux/swiotlb.h | 1 -
kernel/dma/swiotlb.c | 7 +------
5 files changed, 4 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_internal.c b/drivers/gpu/drm/i915/gem/i915_gem_internal.c
index ad22f42541bda6..a9d65fc8aa0eab 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_internal.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_internal.c
@@ -42,7 +42,7 @@ static int i915_gem_object_get_pages_internal(struct drm_i915_gem_object *obj)
max_order = MAX_ORDER;
#ifdef CONFIG_SWIOTLB
- if (swiotlb_nr_tbl()) {
+ if (is_swiotlb_active()) {
unsigned int max_segment;
max_segment = swiotlb_max_segment();
diff --git a/drivers/gpu/drm/nouveau/nouveau_ttm.c b/drivers/gpu/drm/nouveau/nouveau_ttm.c
index a37bc3d7b38b3b..9662522aa0664a 100644
--- a/drivers/gpu/drm/nouveau/nouveau_ttm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_ttm.c
@@ -321,7 +321,7 @@ nouveau_ttm_init(struct nouveau_drm *drm)
}
#if IS_ENABLED(CONFIG_SWIOTLB) && IS_ENABLED(CONFIG_X86)
- need_swiotlb = !!swiotlb_nr_tbl();
+ need_swiotlb = is_swiotlb_active();
#endif
ret = ttm_bo_device_init(&drm->ttm.bdev, &nouveau_bo_driver,
diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c
index 2d75026482197d..b7a8f3a1921f83 100644
--- a/drivers/pci/xen-pcifront.c
+++ b/drivers/pci/xen-pcifront.c
@@ -693,7 +693,7 @@ static int pcifront_connect_and_init_dma(struct pcifront_device *pdev)
spin_unlock(&pcifront_dev_lock);
- if (!err && !swiotlb_nr_tbl()) {
+ if (!err && !is_swiotlb_active()) {
err = pci_xen_swiotlb_init_late();
if (err)
dev_err(&pdev->xdev->dev, "Could not setup SWIOTLB!\n");
diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index 63f7a63f61d098..216854a5e5134b 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -37,7 +37,6 @@ enum swiotlb_force {
extern void swiotlb_init(int verbose);
int swiotlb_init_with_tbl(char *tlb, unsigned long nslabs, int verbose);
-extern unsigned long swiotlb_nr_tbl(void);
unsigned long swiotlb_size_or_default(void);
extern int swiotlb_late_init_with_tbl(char *tlb, unsigned long nslabs);
extern int swiotlb_late_init_with_default_size(size_t default_size);
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index 13de669a9b4681..539c76beb52e07 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -94,12 +94,6 @@ setup_io_tlb_npages(char *str)
}
early_param("swiotlb", setup_io_tlb_npages);
-unsigned long swiotlb_nr_tbl(void)
-{
- return io_tlb_default_mem ? io_tlb_default_mem->nslabs : 0;
-}
-EXPORT_SYMBOL_GPL(swiotlb_nr_tbl);
-
unsigned int swiotlb_max_segment(void)
{
return io_tlb_default_mem ? max_segment : 0;
@@ -652,6 +646,7 @@ bool is_swiotlb_active(void)
{
return io_tlb_default_mem != NULL;
}
+EXPORT_SYMBOL_GPL(is_swiotlb_active);
#ifdef CONFIG_DEBUG_FS
--
2.30.1
^ permalink raw reply related
* Re: [PATCH 08/10] MIPS: disable CONFIG_IDE in malta*_defconfig
From: Thomas Bogendoerfer @ 2021-03-18 14:19 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Jens Axboe, linux-doc, Russell King, linux-kernel, linux-ide,
linux-m68k, Ivan Kokshaysky, linux-alpha, Geert Uytterhoeven,
Matt Turner, linux-mips, linuxppc-dev, David S. Miller,
linux-arm-kernel, Richard Henderson
In-Reply-To: <20210318045706.200458-9-hch@lst.de>
On Thu, Mar 18, 2021 at 05:57:04AM +0100, Christoph Hellwig wrote:
> arch/mips/configs/malta_kvm_guest_defconfig | 3 ---
that file is gone in mips-next.
I could take all MIPS patches into mips-next, if you want...
Thomas.
--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]
^ permalink raw reply
* Re: [PATCH 01/10] alpha: use libata instead of the legacy ide driver
From: Måns Rullgård @ 2021-03-18 17:09 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Jens Axboe, Thomas Bogendoerfer, linux-doc, linux-kernel,
Russell King, linux-mips, linux-ide, linux-m68k, Ivan Kokshaysky,
Al Viro, linux-alpha, Geert Uytterhoeven, Matt Turner,
linuxppc-dev, David S. Miller, linux-arm-kernel,
Richard Henderson
In-Reply-To: <20210318060724.GA29117@lst.de>
Christoph Hellwig <hch@lst.de> writes:
> On Thu, Mar 18, 2021 at 05:54:55AM +0000, Al Viro wrote:
>> On Thu, Mar 18, 2021 at 05:56:57AM +0100, Christoph Hellwig wrote:
>> > Switch the alpha defconfig from the legacy ide driver to libata.
>>
>> Umm... I don't have an IDE alpha box in a usable shape (fans on
>> CPU module shat themselves), and it would take a while to resurrect
>> it, but I remember the joy it used to cause in some versions.
>>
>> Do you have reports of libata variants of drivers actually tested on
>> those?
>
> No, I haven't. The whole point is that we're not going to keep 40000
> lines of code around despite notice for users that don't exist or
> care. If there is a regression we'll fix it, but we're not going to
> make life miserable just because we can.
The pata_ali driver works fine on my UP1500 machine, unless something
broke recently. I'll build the latest kernel and report back.
--
Måns Rullgård
^ permalink raw reply
* [PATCH] powerpc/embedded6xx: Remove CONFIG_MV64X60
From: Christophe Leroy @ 2021-03-18 17:25 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Wolfram Sang
Cc: linuxppc-dev, linux-kernel, linux-i2c
In-Reply-To: <9c2952bcfaec3b1789909eaa36bbce2afbfab7ab.1616085654.git.christophe.leroy@csgroup.eu>
Commit 92c8c16f3457 ("powerpc/embedded6xx: Remove C2K board support")
moved the last selector of CONFIG_MV64X60.
As it is not a user selectable config, it can be removed.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/platforms/embedded6xx/Kconfig | 5 -----
drivers/i2c/busses/Kconfig | 2 +-
2 files changed, 1 insertion(+), 6 deletions(-)
diff --git a/arch/powerpc/platforms/embedded6xx/Kconfig b/arch/powerpc/platforms/embedded6xx/Kconfig
index c1920961f410..4c6d703a4284 100644
--- a/arch/powerpc/platforms/embedded6xx/Kconfig
+++ b/arch/powerpc/platforms/embedded6xx/Kconfig
@@ -71,11 +71,6 @@ config MPC10X_BRIDGE
bool
select PPC_INDIRECT_PCI
-config MV64X60
- bool
- select PPC_INDIRECT_PCI
- select CHECK_CACHE_COHERENCY
-
config GAMECUBE_COMMON
bool
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 05ebf7546e3f..20edcda1c6f4 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -776,7 +776,7 @@ config I2C_MT7621
config I2C_MV64XXX
tristate "Marvell mv64xxx I2C Controller"
- depends on MV64X60 || PLAT_ORION || ARCH_SUNXI || ARCH_MVEBU || COMPILE_TEST
+ depends on PLAT_ORION || ARCH_SUNXI || ARCH_MVEBU || COMPILE_TEST
help
If you say yes to this option, support will be included for the
built-in I2C interface on the Marvell 64xxx line of host bridges.
--
2.25.0
^ permalink raw reply related
* [PATCH] watchdog: Remove MV64x60 watchdog driver
From: Christophe Leroy @ 2021-03-18 17:25 UTC (permalink / raw)
To: Wim Van Sebroeck, Guenter Roeck, Sebastian Hesselbarth
Cc: netdev, linuxppc-dev, linux-kernel, linux-watchdog
Commit 92c8c16f3457 ("powerpc/embedded6xx: Remove C2K board support")
removed the last selector of CONFIG_MV64X60.
Therefore CONFIG_MV64X60_WDT cannot be selected anymore and
can be removed.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
drivers/watchdog/Kconfig | 4 -
drivers/watchdog/Makefile | 1 -
drivers/watchdog/mv64x60_wdt.c | 324 ---------------------------------
include/linux/mv643xx.h | 8 -
4 files changed, 337 deletions(-)
delete mode 100644 drivers/watchdog/mv64x60_wdt.c
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 1fe0042a48d2..178296bda151 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -1831,10 +1831,6 @@ config 8xxx_WDT
For BookE processors (MPC85xx) use the BOOKE_WDT driver instead.
-config MV64X60_WDT
- tristate "MV64X60 (Marvell Discovery) Watchdog Timer"
- depends on MV64X60 || COMPILE_TEST
-
config PIKA_WDT
tristate "PIKA FPGA Watchdog"
depends on WARP || (PPC64 && COMPILE_TEST)
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index f3a6540e725e..752c6513f731 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -175,7 +175,6 @@ obj-$(CONFIG_PIC32_DMT) += pic32-dmt.o
# POWERPC Architecture
obj-$(CONFIG_GEF_WDT) += gef_wdt.o
obj-$(CONFIG_8xxx_WDT) += mpc8xxx_wdt.o
-obj-$(CONFIG_MV64X60_WDT) += mv64x60_wdt.o
obj-$(CONFIG_PIKA_WDT) += pika_wdt.o
obj-$(CONFIG_BOOKE_WDT) += booke_wdt.o
obj-$(CONFIG_MEN_A21_WDT) += mena21_wdt.o
diff --git a/drivers/watchdog/mv64x60_wdt.c b/drivers/watchdog/mv64x60_wdt.c
deleted file mode 100644
index 894aa63488d3..000000000000
--- a/drivers/watchdog/mv64x60_wdt.c
+++ /dev/null
@@ -1,324 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/*
- * mv64x60_wdt.c - MV64X60 (Marvell Discovery) watchdog userspace interface
- *
- * Author: James Chapman <jchapman@katalix.com>
- *
- * Platform-specific setup code should configure the dog to generate
- * interrupt or reset as required. This code only enables/disables
- * and services the watchdog.
- *
- * Derived from mpc8xx_wdt.c, with the following copyright.
- *
- * 2002 (c) Florian Schirmer <jolt@tuxbox.org>
- */
-
-#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
-
-#include <linux/fs.h>
-#include <linux/init.h>
-#include <linux/kernel.h>
-#include <linux/miscdevice.h>
-#include <linux/module.h>
-#include <linux/watchdog.h>
-#include <linux/platform_device.h>
-#include <linux/mv643xx.h>
-#include <linux/uaccess.h>
-#include <linux/io.h>
-
-#define MV64x60_WDT_WDC_OFFSET 0
-
-/*
- * The watchdog configuration register contains a pair of 2-bit fields,
- * 1. a reload field, bits 27-26, which triggers a reload of
- * the countdown register, and
- * 2. an enable field, bits 25-24, which toggles between
- * enabling and disabling the watchdog timer.
- * Bit 31 is a read-only field which indicates whether the
- * watchdog timer is currently enabled.
- *
- * The low 24 bits contain the timer reload value.
- */
-#define MV64x60_WDC_ENABLE_SHIFT 24
-#define MV64x60_WDC_SERVICE_SHIFT 26
-#define MV64x60_WDC_ENABLED_SHIFT 31
-
-#define MV64x60_WDC_ENABLED_TRUE 1
-#define MV64x60_WDC_ENABLED_FALSE 0
-
-/* Flags bits */
-#define MV64x60_WDOG_FLAG_OPENED 0
-
-static unsigned long wdt_flags;
-static int wdt_status;
-static void __iomem *mv64x60_wdt_regs;
-static int mv64x60_wdt_timeout;
-static int mv64x60_wdt_count;
-static unsigned int bus_clk;
-static char expect_close;
-static DEFINE_SPINLOCK(mv64x60_wdt_spinlock);
-
-static bool nowayout = WATCHDOG_NOWAYOUT;
-module_param(nowayout, bool, 0);
-MODULE_PARM_DESC(nowayout,
- "Watchdog cannot be stopped once started (default="
- __MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
-
-static int mv64x60_wdt_toggle_wdc(int enabled_predicate, int field_shift)
-{
- u32 data;
- u32 enabled;
- int ret = 0;
-
- spin_lock(&mv64x60_wdt_spinlock);
- data = readl(mv64x60_wdt_regs + MV64x60_WDT_WDC_OFFSET);
- enabled = (data >> MV64x60_WDC_ENABLED_SHIFT) & 1;
-
- /* only toggle the requested field if enabled state matches predicate */
- if ((enabled ^ enabled_predicate) == 0) {
- /* We write a 1, then a 2 -- to the appropriate field */
- data = (1 << field_shift) | mv64x60_wdt_count;
- writel(data, mv64x60_wdt_regs + MV64x60_WDT_WDC_OFFSET);
-
- data = (2 << field_shift) | mv64x60_wdt_count;
- writel(data, mv64x60_wdt_regs + MV64x60_WDT_WDC_OFFSET);
- ret = 1;
- }
- spin_unlock(&mv64x60_wdt_spinlock);
-
- return ret;
-}
-
-static void mv64x60_wdt_service(void)
-{
- mv64x60_wdt_toggle_wdc(MV64x60_WDC_ENABLED_TRUE,
- MV64x60_WDC_SERVICE_SHIFT);
-}
-
-static void mv64x60_wdt_handler_enable(void)
-{
- if (mv64x60_wdt_toggle_wdc(MV64x60_WDC_ENABLED_FALSE,
- MV64x60_WDC_ENABLE_SHIFT)) {
- mv64x60_wdt_service();
- pr_notice("watchdog activated\n");
- }
-}
-
-static void mv64x60_wdt_handler_disable(void)
-{
- if (mv64x60_wdt_toggle_wdc(MV64x60_WDC_ENABLED_TRUE,
- MV64x60_WDC_ENABLE_SHIFT))
- pr_notice("watchdog deactivated\n");
-}
-
-static void mv64x60_wdt_set_timeout(unsigned int timeout)
-{
- /* maximum bus cycle count is 0xFFFFFFFF */
- if (timeout > 0xFFFFFFFF / bus_clk)
- timeout = 0xFFFFFFFF / bus_clk;
-
- mv64x60_wdt_count = timeout * bus_clk >> 8;
- mv64x60_wdt_timeout = timeout;
-}
-
-static int mv64x60_wdt_open(struct inode *inode, struct file *file)
-{
- if (test_and_set_bit(MV64x60_WDOG_FLAG_OPENED, &wdt_flags))
- return -EBUSY;
-
- if (nowayout)
- __module_get(THIS_MODULE);
-
- mv64x60_wdt_handler_enable();
-
- return stream_open(inode, file);
-}
-
-static int mv64x60_wdt_release(struct inode *inode, struct file *file)
-{
- if (expect_close == 42)
- mv64x60_wdt_handler_disable();
- else {
- pr_crit("unexpected close, not stopping timer!\n");
- mv64x60_wdt_service();
- }
- expect_close = 0;
-
- clear_bit(MV64x60_WDOG_FLAG_OPENED, &wdt_flags);
-
- return 0;
-}
-
-static ssize_t mv64x60_wdt_write(struct file *file, const char __user *data,
- size_t len, loff_t *ppos)
-{
- if (len) {
- if (!nowayout) {
- size_t i;
-
- expect_close = 0;
-
- for (i = 0; i != len; i++) {
- char c;
- if (get_user(c, data + i))
- return -EFAULT;
- if (c == 'V')
- expect_close = 42;
- }
- }
- mv64x60_wdt_service();
- }
-
- return len;
-}
-
-static long mv64x60_wdt_ioctl(struct file *file,
- unsigned int cmd, unsigned long arg)
-{
- int timeout;
- int options;
- void __user *argp = (void __user *)arg;
- static const struct watchdog_info info = {
- .options = WDIOF_SETTIMEOUT |
- WDIOF_MAGICCLOSE |
- WDIOF_KEEPALIVEPING,
- .firmware_version = 0,
- .identity = "MV64x60 watchdog",
- };
-
- switch (cmd) {
- case WDIOC_GETSUPPORT:
- if (copy_to_user(argp, &info, sizeof(info)))
- return -EFAULT;
- break;
-
- case WDIOC_GETSTATUS:
- case WDIOC_GETBOOTSTATUS:
- if (put_user(wdt_status, (int __user *)argp))
- return -EFAULT;
- wdt_status &= ~WDIOF_KEEPALIVEPING;
- break;
-
- case WDIOC_GETTEMP:
- return -EOPNOTSUPP;
-
- case WDIOC_SETOPTIONS:
- if (get_user(options, (int __user *)argp))
- return -EFAULT;
-
- if (options & WDIOS_DISABLECARD)
- mv64x60_wdt_handler_disable();
-
- if (options & WDIOS_ENABLECARD)
- mv64x60_wdt_handler_enable();
- break;
-
- case WDIOC_KEEPALIVE:
- mv64x60_wdt_service();
- wdt_status |= WDIOF_KEEPALIVEPING;
- break;
-
- case WDIOC_SETTIMEOUT:
- if (get_user(timeout, (int __user *)argp))
- return -EFAULT;
- mv64x60_wdt_set_timeout(timeout);
- fallthrough;
-
- case WDIOC_GETTIMEOUT:
- if (put_user(mv64x60_wdt_timeout, (int __user *)argp))
- return -EFAULT;
- break;
-
- default:
- return -ENOTTY;
- }
-
- return 0;
-}
-
-static const struct file_operations mv64x60_wdt_fops = {
- .owner = THIS_MODULE,
- .llseek = no_llseek,
- .write = mv64x60_wdt_write,
- .unlocked_ioctl = mv64x60_wdt_ioctl,
- .compat_ioctl = compat_ptr_ioctl,
- .open = mv64x60_wdt_open,
- .release = mv64x60_wdt_release,
-};
-
-static struct miscdevice mv64x60_wdt_miscdev = {
- .minor = WATCHDOG_MINOR,
- .name = "watchdog",
- .fops = &mv64x60_wdt_fops,
-};
-
-static int mv64x60_wdt_probe(struct platform_device *dev)
-{
- struct mv64x60_wdt_pdata *pdata = dev_get_platdata(&dev->dev);
- struct resource *r;
- int timeout = 10;
-
- bus_clk = 133; /* in MHz */
- if (pdata) {
- timeout = pdata->timeout;
- bus_clk = pdata->bus_clk;
- }
-
- /* Since bus_clk is truncated MHz, actual frequency could be
- * up to 1MHz higher. Round up, since it's better to time out
- * too late than too soon.
- */
- bus_clk++;
- bus_clk *= 1000000; /* convert to Hz */
-
- r = platform_get_resource(dev, IORESOURCE_MEM, 0);
- if (!r)
- return -ENODEV;
-
- mv64x60_wdt_regs = devm_ioremap(&dev->dev, r->start, resource_size(r));
- if (mv64x60_wdt_regs == NULL)
- return -ENOMEM;
-
- mv64x60_wdt_set_timeout(timeout);
-
- mv64x60_wdt_handler_disable(); /* in case timer was already running */
-
- return misc_register(&mv64x60_wdt_miscdev);
-}
-
-static int mv64x60_wdt_remove(struct platform_device *dev)
-{
- misc_deregister(&mv64x60_wdt_miscdev);
-
- mv64x60_wdt_handler_disable();
-
- return 0;
-}
-
-static struct platform_driver mv64x60_wdt_driver = {
- .probe = mv64x60_wdt_probe,
- .remove = mv64x60_wdt_remove,
- .driver = {
- .name = MV64x60_WDT_NAME,
- },
-};
-
-static int __init mv64x60_wdt_init(void)
-{
- pr_info("MV64x60 watchdog driver\n");
-
- return platform_driver_register(&mv64x60_wdt_driver);
-}
-
-static void __exit mv64x60_wdt_exit(void)
-{
- platform_driver_unregister(&mv64x60_wdt_driver);
-}
-
-module_init(mv64x60_wdt_init);
-module_exit(mv64x60_wdt_exit);
-
-MODULE_AUTHOR("James Chapman <jchapman@katalix.com>");
-MODULE_DESCRIPTION("MV64x60 watchdog driver");
-MODULE_LICENSE("GPL");
-MODULE_ALIAS("platform:" MV64x60_WDT_NAME);
diff --git a/include/linux/mv643xx.h b/include/linux/mv643xx.h
index 47e5679b48e1..000b126acfb6 100644
--- a/include/linux/mv643xx.h
+++ b/include/linux/mv643xx.h
@@ -918,12 +918,4 @@
extern void mv64340_irq_init(unsigned int base);
-/* Watchdog Platform Device, Driver Data */
-#define MV64x60_WDT_NAME "mv64x60_wdt"
-
-struct mv64x60_wdt_pdata {
- int timeout; /* watchdog expiry in seconds, default 10 */
- int bus_clk; /* bus clock in MHz, default 133 */
-};
-
#endif /* __ASM_MV643XX_H */
--
2.25.0
^ permalink raw reply related
* [PATCH] net: marvell: Remove reference to CONFIG_MV64X60
From: Christophe Leroy @ 2021-03-18 17:25 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski, Sebastian Hesselbarth
Cc: netdev, linuxppc-dev, linux-kernel
In-Reply-To: <9c2952bcfaec3b1789909eaa36bbce2afbfab7ab.1616085654.git.christophe.leroy@csgroup.eu>
Commit 92c8c16f3457 ("powerpc/embedded6xx: Remove C2K board support")
removed last selector of CONFIG_MV64X60.
As it is not a user selectable config item, all references to it
are stale. Remove them.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
drivers/net/ethernet/marvell/Kconfig | 4 ++--
drivers/net/ethernet/marvell/mv643xx_eth.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/marvell/Kconfig b/drivers/net/ethernet/marvell/Kconfig
index 7fe15a3286f4..fe0989c0fc25 100644
--- a/drivers/net/ethernet/marvell/Kconfig
+++ b/drivers/net/ethernet/marvell/Kconfig
@@ -6,7 +6,7 @@
config NET_VENDOR_MARVELL
bool "Marvell devices"
default y
- depends on PCI || CPU_PXA168 || MV64X60 || PPC32 || PLAT_ORION || INET || COMPILE_TEST
+ depends on PCI || CPU_PXA168 || PPC32 || PLAT_ORION || INET || COMPILE_TEST
help
If you have a network (Ethernet) card belonging to this class, say Y.
@@ -19,7 +19,7 @@ if NET_VENDOR_MARVELL
config MV643XX_ETH
tristate "Marvell Discovery (643XX) and Orion ethernet support"
- depends on MV64X60 || PPC32 || PLAT_ORION || COMPILE_TEST
+ depends on PPC32 || PLAT_ORION || COMPILE_TEST
depends on INET
select PHYLIB
select MVMDIO
diff --git a/drivers/net/ethernet/marvell/mv643xx_eth.c b/drivers/net/ethernet/marvell/mv643xx_eth.c
index 90e6111ce534..3bfb659b5c99 100644
--- a/drivers/net/ethernet/marvell/mv643xx_eth.c
+++ b/drivers/net/ethernet/marvell/mv643xx_eth.c
@@ -2684,7 +2684,7 @@ static const struct of_device_id mv643xx_eth_shared_ids[] = {
MODULE_DEVICE_TABLE(of, mv643xx_eth_shared_ids);
#endif
-#if defined(CONFIG_OF_IRQ) && !defined(CONFIG_MV64X60)
+#ifdef CONFIG_OF_IRQ
#define mv643xx_eth_property(_np, _name, _v) \
do { \
u32 tmp; \
--
2.25.0
^ permalink raw reply related
* [PATCH 1/1] powerpc/kernel/iommu: Align size for IOMMU_PAGE_SIZE() to save TCEs
From: Leonardo Bras @ 2021-03-18 17:44 UTC (permalink / raw)
To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
Leonardo Bras, Alexey Kardashevskiy, Nicolin Chen,
Niklas Schnelle
Cc: linuxppc-dev, linux-kernel
Currently both iommu_alloc_coherent() and iommu_free_coherent() align the
desired allocation size to PAGE_SIZE, and gets system pages and IOMMU
mappings (TCEs) for that value.
When IOMMU_PAGE_SIZE < PAGE_SIZE, this behavior may cause unnecessary
TCEs to be created for mapping the whole system page.
Example:
- PAGE_SIZE = 64k, IOMMU_PAGE_SIZE() = 4k
- iommu_alloc_coherent() is called for 128 bytes
- 1 system page (64k) is allocated
- 16 IOMMU pages (16 x 4k) are allocated (16 TCEs used)
It would be enough to use a single TCE for this, so 15 TCEs are
wasted in the process.
Update iommu_*_coherent() to make sure the size alignment happens only
for IOMMU_PAGE_SIZE() before calling iommu_alloc() and iommu_free().
Also, on iommu_range_alloc(), replace ALIGN(n, 1 << tbl->it_page_shift)
with IOMMU_PAGE_ALIGN(n, tbl), which is easier to read and does the
same.
Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
arch/powerpc/kernel/iommu.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
index 5b69a6a72a0e..3329ef045805 100644
--- a/arch/powerpc/kernel/iommu.c
+++ b/arch/powerpc/kernel/iommu.c
@@ -851,6 +851,7 @@ void *iommu_alloc_coherent(struct device *dev, struct iommu_table *tbl,
unsigned int order;
unsigned int nio_pages, io_order;
struct page *page;
+ size_t size_io = size;
size = PAGE_ALIGN(size);
order = get_order(size);
@@ -877,8 +878,9 @@ void *iommu_alloc_coherent(struct device *dev, struct iommu_table *tbl,
memset(ret, 0, size);
/* Set up tces to cover the allocated range */
- nio_pages = size >> tbl->it_page_shift;
- io_order = get_iommu_order(size, tbl);
+ size_io = IOMMU_PAGE_ALIGN(size_io, tbl);
+ nio_pages = size_io >> tbl->it_page_shift;
+ io_order = get_iommu_order(size_io, tbl);
mapping = iommu_alloc(dev, tbl, ret, nio_pages, DMA_BIDIRECTIONAL,
mask >> tbl->it_page_shift, io_order, 0);
if (mapping == DMA_MAPPING_ERROR) {
@@ -893,10 +895,9 @@ void iommu_free_coherent(struct iommu_table *tbl, size_t size,
void *vaddr, dma_addr_t dma_handle)
{
if (tbl) {
- unsigned int nio_pages;
+ size_t size_io = IOMMU_PAGE_ALIGN(size, tbl);
+ unsigned int nio_pages = size_io >> tbl->it_page_shift;
- size = PAGE_ALIGN(size);
- nio_pages = size >> tbl->it_page_shift;
iommu_free(tbl, dma_handle, nio_pages);
size = PAGE_ALIGN(size);
free_pages((unsigned long)vaddr, get_order(size));
--
2.29.2
^ permalink raw reply related
* [PATCH 1/1] powerpc/kernel/iommu: Use largepool as a last resort when !largealloc
From: Leonardo Bras @ 2021-03-18 17:44 UTC (permalink / raw)
To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
Leonardo Bras, Alexey Kardashevskiy, Niklas Schnelle,
Nicolin Chen
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20210318174414.684630-1-leobras.c@gmail.com>
As of today, doing iommu_range_alloc() only for !largealloc (npages <= 15)
will only be able to use 3/4 of the available pages, given pages on
largepool not being available for !largealloc.
This could mean some drivers not being able to fully use all the available
pages for the DMA window.
Add pages on largepool as a last resort for !largealloc, making all pages
of the DMA window available.
Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
arch/powerpc/kernel/iommu.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
index 3329ef045805..ae6ad8dca605 100644
--- a/arch/powerpc/kernel/iommu.c
+++ b/arch/powerpc/kernel/iommu.c
@@ -255,6 +255,15 @@ static unsigned long iommu_range_alloc(struct device *dev,
pass++;
goto again;
+ } else if (pass == tbl->nr_pools + 1) {
+ /* Last resort: try largepool */
+ spin_unlock(&pool->lock);
+ pool = &tbl->large_pool;
+ spin_lock(&pool->lock);
+ pool->hint = pool->start;
+ pass++;
+ goto again;
+
} else {
/* Give up */
spin_unlock_irqrestore(&(pool->lock), flags);
--
2.29.2
^ permalink raw reply related
* Re: [PATCH 01/10] alpha: use libata instead of the legacy ide driver
From: Måns Rullgård @ 2021-03-18 18:03 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Jens Axboe, Thomas Bogendoerfer, linux-doc, linux-kernel,
Russell King, linux-mips, linux-ide, linux-m68k, Ivan Kokshaysky,
Al Viro, linux-alpha, Geert Uytterhoeven, Matt Turner,
linuxppc-dev, David S. Miller, linux-arm-kernel,
Richard Henderson
In-Reply-To: <yw1x1rccie35.fsf@mansr.com>
Måns Rullgård <mans@mansr.com> writes:
> Christoph Hellwig <hch@lst.de> writes:
>
>> On Thu, Mar 18, 2021 at 05:54:55AM +0000, Al Viro wrote:
>>> On Thu, Mar 18, 2021 at 05:56:57AM +0100, Christoph Hellwig wrote:
>>> > Switch the alpha defconfig from the legacy ide driver to libata.
>>>
>>> Umm... I don't have an IDE alpha box in a usable shape (fans on
>>> CPU module shat themselves), and it would take a while to resurrect
>>> it, but I remember the joy it used to cause in some versions.
>>>
>>> Do you have reports of libata variants of drivers actually tested on
>>> those?
>>
>> No, I haven't. The whole point is that we're not going to keep 40000
>> lines of code around despite notice for users that don't exist or
>> care. If there is a regression we'll fix it, but we're not going to
>> make life miserable just because we can.
>
> The pata_ali driver works fine on my UP1500 machine, unless something
> broke recently. I'll build the latest kernel and report back.
5.11.7 seems fine too.
--
Måns Rullgård
^ permalink raw reply
* Re: [PATCH] watchdog: Remove MV64x60 watchdog driver
From: Guenter Roeck @ 2021-03-18 18:32 UTC (permalink / raw)
To: Christophe Leroy, Wim Van Sebroeck, Sebastian Hesselbarth
Cc: netdev, linuxppc-dev, linux-kernel, linux-watchdog
In-Reply-To: <9c2952bcfaec3b1789909eaa36bbce2afbfab7ab.1616085654.git.christophe.leroy@csgroup.eu>
On 3/18/21 10:25 AM, Christophe Leroy wrote:
> Commit 92c8c16f3457 ("powerpc/embedded6xx: Remove C2K board support")
> removed the last selector of CONFIG_MV64X60.
>
> Therefore CONFIG_MV64X60_WDT cannot be selected anymore and
> can be removed.
>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> ---
> drivers/watchdog/Kconfig | 4 -
> drivers/watchdog/Makefile | 1 -
> drivers/watchdog/mv64x60_wdt.c | 324 ---------------------------------
> include/linux/mv643xx.h | 8 -
> 4 files changed, 337 deletions(-)
> delete mode 100644 drivers/watchdog/mv64x60_wdt.c
>
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 1fe0042a48d2..178296bda151 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -1831,10 +1831,6 @@ config 8xxx_WDT
>
> For BookE processors (MPC85xx) use the BOOKE_WDT driver instead.
>
> -config MV64X60_WDT
> - tristate "MV64X60 (Marvell Discovery) Watchdog Timer"
> - depends on MV64X60 || COMPILE_TEST
> -
> config PIKA_WDT
> tristate "PIKA FPGA Watchdog"
> depends on WARP || (PPC64 && COMPILE_TEST)
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index f3a6540e725e..752c6513f731 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -175,7 +175,6 @@ obj-$(CONFIG_PIC32_DMT) += pic32-dmt.o
> # POWERPC Architecture
> obj-$(CONFIG_GEF_WDT) += gef_wdt.o
> obj-$(CONFIG_8xxx_WDT) += mpc8xxx_wdt.o
> -obj-$(CONFIG_MV64X60_WDT) += mv64x60_wdt.o
> obj-$(CONFIG_PIKA_WDT) += pika_wdt.o
> obj-$(CONFIG_BOOKE_WDT) += booke_wdt.o
> obj-$(CONFIG_MEN_A21_WDT) += mena21_wdt.o
> diff --git a/drivers/watchdog/mv64x60_wdt.c b/drivers/watchdog/mv64x60_wdt.c
> deleted file mode 100644
> index 894aa63488d3..000000000000
> --- a/drivers/watchdog/mv64x60_wdt.c
> +++ /dev/null
> @@ -1,324 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0
> -/*
> - * mv64x60_wdt.c - MV64X60 (Marvell Discovery) watchdog userspace interface
> - *
> - * Author: James Chapman <jchapman@katalix.com>
> - *
> - * Platform-specific setup code should configure the dog to generate
> - * interrupt or reset as required. This code only enables/disables
> - * and services the watchdog.
> - *
> - * Derived from mpc8xx_wdt.c, with the following copyright.
> - *
> - * 2002 (c) Florian Schirmer <jolt@tuxbox.org>
> - */
> -
> -#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> -
> -#include <linux/fs.h>
> -#include <linux/init.h>
> -#include <linux/kernel.h>
> -#include <linux/miscdevice.h>
> -#include <linux/module.h>
> -#include <linux/watchdog.h>
> -#include <linux/platform_device.h>
> -#include <linux/mv643xx.h>
> -#include <linux/uaccess.h>
> -#include <linux/io.h>
> -
> -#define MV64x60_WDT_WDC_OFFSET 0
> -
> -/*
> - * The watchdog configuration register contains a pair of 2-bit fields,
> - * 1. a reload field, bits 27-26, which triggers a reload of
> - * the countdown register, and
> - * 2. an enable field, bits 25-24, which toggles between
> - * enabling and disabling the watchdog timer.
> - * Bit 31 is a read-only field which indicates whether the
> - * watchdog timer is currently enabled.
> - *
> - * The low 24 bits contain the timer reload value.
> - */
> -#define MV64x60_WDC_ENABLE_SHIFT 24
> -#define MV64x60_WDC_SERVICE_SHIFT 26
> -#define MV64x60_WDC_ENABLED_SHIFT 31
> -
> -#define MV64x60_WDC_ENABLED_TRUE 1
> -#define MV64x60_WDC_ENABLED_FALSE 0
> -
> -/* Flags bits */
> -#define MV64x60_WDOG_FLAG_OPENED 0
> -
> -static unsigned long wdt_flags;
> -static int wdt_status;
> -static void __iomem *mv64x60_wdt_regs;
> -static int mv64x60_wdt_timeout;
> -static int mv64x60_wdt_count;
> -static unsigned int bus_clk;
> -static char expect_close;
> -static DEFINE_SPINLOCK(mv64x60_wdt_spinlock);
> -
> -static bool nowayout = WATCHDOG_NOWAYOUT;
> -module_param(nowayout, bool, 0);
> -MODULE_PARM_DESC(nowayout,
> - "Watchdog cannot be stopped once started (default="
> - __MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
> -
> -static int mv64x60_wdt_toggle_wdc(int enabled_predicate, int field_shift)
> -{
> - u32 data;
> - u32 enabled;
> - int ret = 0;
> -
> - spin_lock(&mv64x60_wdt_spinlock);
> - data = readl(mv64x60_wdt_regs + MV64x60_WDT_WDC_OFFSET);
> - enabled = (data >> MV64x60_WDC_ENABLED_SHIFT) & 1;
> -
> - /* only toggle the requested field if enabled state matches predicate */
> - if ((enabled ^ enabled_predicate) == 0) {
> - /* We write a 1, then a 2 -- to the appropriate field */
> - data = (1 << field_shift) | mv64x60_wdt_count;
> - writel(data, mv64x60_wdt_regs + MV64x60_WDT_WDC_OFFSET);
> -
> - data = (2 << field_shift) | mv64x60_wdt_count;
> - writel(data, mv64x60_wdt_regs + MV64x60_WDT_WDC_OFFSET);
> - ret = 1;
> - }
> - spin_unlock(&mv64x60_wdt_spinlock);
> -
> - return ret;
> -}
> -
> -static void mv64x60_wdt_service(void)
> -{
> - mv64x60_wdt_toggle_wdc(MV64x60_WDC_ENABLED_TRUE,
> - MV64x60_WDC_SERVICE_SHIFT);
> -}
> -
> -static void mv64x60_wdt_handler_enable(void)
> -{
> - if (mv64x60_wdt_toggle_wdc(MV64x60_WDC_ENABLED_FALSE,
> - MV64x60_WDC_ENABLE_SHIFT)) {
> - mv64x60_wdt_service();
> - pr_notice("watchdog activated\n");
> - }
> -}
> -
> -static void mv64x60_wdt_handler_disable(void)
> -{
> - if (mv64x60_wdt_toggle_wdc(MV64x60_WDC_ENABLED_TRUE,
> - MV64x60_WDC_ENABLE_SHIFT))
> - pr_notice("watchdog deactivated\n");
> -}
> -
> -static void mv64x60_wdt_set_timeout(unsigned int timeout)
> -{
> - /* maximum bus cycle count is 0xFFFFFFFF */
> - if (timeout > 0xFFFFFFFF / bus_clk)
> - timeout = 0xFFFFFFFF / bus_clk;
> -
> - mv64x60_wdt_count = timeout * bus_clk >> 8;
> - mv64x60_wdt_timeout = timeout;
> -}
> -
> -static int mv64x60_wdt_open(struct inode *inode, struct file *file)
> -{
> - if (test_and_set_bit(MV64x60_WDOG_FLAG_OPENED, &wdt_flags))
> - return -EBUSY;
> -
> - if (nowayout)
> - __module_get(THIS_MODULE);
> -
> - mv64x60_wdt_handler_enable();
> -
> - return stream_open(inode, file);
> -}
> -
> -static int mv64x60_wdt_release(struct inode *inode, struct file *file)
> -{
> - if (expect_close == 42)
> - mv64x60_wdt_handler_disable();
> - else {
> - pr_crit("unexpected close, not stopping timer!\n");
> - mv64x60_wdt_service();
> - }
> - expect_close = 0;
> -
> - clear_bit(MV64x60_WDOG_FLAG_OPENED, &wdt_flags);
> -
> - return 0;
> -}
> -
> -static ssize_t mv64x60_wdt_write(struct file *file, const char __user *data,
> - size_t len, loff_t *ppos)
> -{
> - if (len) {
> - if (!nowayout) {
> - size_t i;
> -
> - expect_close = 0;
> -
> - for (i = 0; i != len; i++) {
> - char c;
> - if (get_user(c, data + i))
> - return -EFAULT;
> - if (c == 'V')
> - expect_close = 42;
> - }
> - }
> - mv64x60_wdt_service();
> - }
> -
> - return len;
> -}
> -
> -static long mv64x60_wdt_ioctl(struct file *file,
> - unsigned int cmd, unsigned long arg)
> -{
> - int timeout;
> - int options;
> - void __user *argp = (void __user *)arg;
> - static const struct watchdog_info info = {
> - .options = WDIOF_SETTIMEOUT |
> - WDIOF_MAGICCLOSE |
> - WDIOF_KEEPALIVEPING,
> - .firmware_version = 0,
> - .identity = "MV64x60 watchdog",
> - };
> -
> - switch (cmd) {
> - case WDIOC_GETSUPPORT:
> - if (copy_to_user(argp, &info, sizeof(info)))
> - return -EFAULT;
> - break;
> -
> - case WDIOC_GETSTATUS:
> - case WDIOC_GETBOOTSTATUS:
> - if (put_user(wdt_status, (int __user *)argp))
> - return -EFAULT;
> - wdt_status &= ~WDIOF_KEEPALIVEPING;
> - break;
> -
> - case WDIOC_GETTEMP:
> - return -EOPNOTSUPP;
> -
> - case WDIOC_SETOPTIONS:
> - if (get_user(options, (int __user *)argp))
> - return -EFAULT;
> -
> - if (options & WDIOS_DISABLECARD)
> - mv64x60_wdt_handler_disable();
> -
> - if (options & WDIOS_ENABLECARD)
> - mv64x60_wdt_handler_enable();
> - break;
> -
> - case WDIOC_KEEPALIVE:
> - mv64x60_wdt_service();
> - wdt_status |= WDIOF_KEEPALIVEPING;
> - break;
> -
> - case WDIOC_SETTIMEOUT:
> - if (get_user(timeout, (int __user *)argp))
> - return -EFAULT;
> - mv64x60_wdt_set_timeout(timeout);
> - fallthrough;
> -
> - case WDIOC_GETTIMEOUT:
> - if (put_user(mv64x60_wdt_timeout, (int __user *)argp))
> - return -EFAULT;
> - break;
> -
> - default:
> - return -ENOTTY;
> - }
> -
> - return 0;
> -}
> -
> -static const struct file_operations mv64x60_wdt_fops = {
> - .owner = THIS_MODULE,
> - .llseek = no_llseek,
> - .write = mv64x60_wdt_write,
> - .unlocked_ioctl = mv64x60_wdt_ioctl,
> - .compat_ioctl = compat_ptr_ioctl,
> - .open = mv64x60_wdt_open,
> - .release = mv64x60_wdt_release,
> -};
> -
> -static struct miscdevice mv64x60_wdt_miscdev = {
> - .minor = WATCHDOG_MINOR,
> - .name = "watchdog",
> - .fops = &mv64x60_wdt_fops,
> -};
> -
> -static int mv64x60_wdt_probe(struct platform_device *dev)
> -{
> - struct mv64x60_wdt_pdata *pdata = dev_get_platdata(&dev->dev);
> - struct resource *r;
> - int timeout = 10;
> -
> - bus_clk = 133; /* in MHz */
> - if (pdata) {
> - timeout = pdata->timeout;
> - bus_clk = pdata->bus_clk;
> - }
> -
> - /* Since bus_clk is truncated MHz, actual frequency could be
> - * up to 1MHz higher. Round up, since it's better to time out
> - * too late than too soon.
> - */
> - bus_clk++;
> - bus_clk *= 1000000; /* convert to Hz */
> -
> - r = platform_get_resource(dev, IORESOURCE_MEM, 0);
> - if (!r)
> - return -ENODEV;
> -
> - mv64x60_wdt_regs = devm_ioremap(&dev->dev, r->start, resource_size(r));
> - if (mv64x60_wdt_regs == NULL)
> - return -ENOMEM;
> -
> - mv64x60_wdt_set_timeout(timeout);
> -
> - mv64x60_wdt_handler_disable(); /* in case timer was already running */
> -
> - return misc_register(&mv64x60_wdt_miscdev);
> -}
> -
> -static int mv64x60_wdt_remove(struct platform_device *dev)
> -{
> - misc_deregister(&mv64x60_wdt_miscdev);
> -
> - mv64x60_wdt_handler_disable();
> -
> - return 0;
> -}
> -
> -static struct platform_driver mv64x60_wdt_driver = {
> - .probe = mv64x60_wdt_probe,
> - .remove = mv64x60_wdt_remove,
> - .driver = {
> - .name = MV64x60_WDT_NAME,
> - },
> -};
> -
> -static int __init mv64x60_wdt_init(void)
> -{
> - pr_info("MV64x60 watchdog driver\n");
> -
> - return platform_driver_register(&mv64x60_wdt_driver);
> -}
> -
> -static void __exit mv64x60_wdt_exit(void)
> -{
> - platform_driver_unregister(&mv64x60_wdt_driver);
> -}
> -
> -module_init(mv64x60_wdt_init);
> -module_exit(mv64x60_wdt_exit);
> -
> -MODULE_AUTHOR("James Chapman <jchapman@katalix.com>");
> -MODULE_DESCRIPTION("MV64x60 watchdog driver");
> -MODULE_LICENSE("GPL");
> -MODULE_ALIAS("platform:" MV64x60_WDT_NAME);
> diff --git a/include/linux/mv643xx.h b/include/linux/mv643xx.h
> index 47e5679b48e1..000b126acfb6 100644
> --- a/include/linux/mv643xx.h
> +++ b/include/linux/mv643xx.h
> @@ -918,12 +918,4 @@
>
> extern void mv64340_irq_init(unsigned int base);
>
> -/* Watchdog Platform Device, Driver Data */
> -#define MV64x60_WDT_NAME "mv64x60_wdt"
> -
> -struct mv64x60_wdt_pdata {
> - int timeout; /* watchdog expiry in seconds, default 10 */
> - int bus_clk; /* bus clock in MHz, default 133 */
> -};
> -
> #endif /* __ASM_MV643XX_H */
>
^ permalink raw reply
* Re: [PATCH] powerpc/embedded6xx: Remove CONFIG_MV64X60
From: Wolfram Sang @ 2021-03-18 19:22 UTC (permalink / raw)
To: Christophe Leroy; +Cc: linux-kernel, Paul Mackerras, linux-i2c, linuxppc-dev
In-Reply-To: <19e57d16692dcd1ca67ba880d7273a57fab416aa.1616085654.git.christophe.leroy@csgroup.eu>
[-- Attachment #1: Type: text/plain, Size: 375 bytes --]
On Thu, Mar 18, 2021 at 05:25:07PM +0000, Christophe Leroy wrote:
> Commit 92c8c16f3457 ("powerpc/embedded6xx: Remove C2K board support")
> moved the last selector of CONFIG_MV64X60.
>
> As it is not a user selectable config, it can be removed.
>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Acked-by: Wolfram Sang <wsa@kernel.org> # for I2C
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 3/3] powerpc/qspinlock: Use generic smp_cond_load_relaxed
From: Davidlohr Bueso @ 2021-03-18 20:02 UTC (permalink / raw)
To: Nicholas Piggin
Cc: will, Davidlohr Bueso, peterz, linux-kernel, mingo, paulus,
longman, linuxppc-dev
In-Reply-To: <1615870473.h7h4jetmjb.astroid@bobo.none>
On Tue, 16 Mar 2021, Nicholas Piggin wrote:
>One request, could you add a comment in place that references
>smp_cond_load_relaxed() so this commit can be found again if
>someone looks at it? Something like this
>
>/*
> * smp_cond_load_relaxed was found to have performance problems if
> * implemented with spin_begin()/spin_end().
> */
Sure, let me see where I can fit that in and send out a v2.
Similarly, but unrelated to this patch, is there any chance we could
remove the whole spin_until_cond() machinery and make it specific to
powerpc? This was introduced in 2017 and doesn't really have any users
outside of powerpc, except for these:
drivers/firmware/arm_scmi/driver.c: spin_until_cond(scmi_xfer_done_no_timeout(cinfo, xfer, stop));
drivers/firmware/arm_scmi/shmem.c: spin_until_cond(ioread32(&shmem->channel_status) &
drivers/net/ethernet/xilinx/ll_temac_main.c: spin_until_cond(hard_acs_rdy_or_timeout(lp, timeout));
... which afaict only the xilinx one can actually build on powerpc.
Regardless, these could be converted to smp_cond_load_relaxed(), being
the more standard way to do optimized busy-waiting, caring more about
the family of barriers than ad-hoc SMT priorities. Of course, I have
no way of testing any of these changes.
>I wonder if it should have a Fixes: tag to the original commit as
>well.
I'm not sure either. I've actually been informed recently of other
workloads that benefit from the revert on large Power9 boxes. So I'll
go ahead and add it.
>
>Otherwise,
>
>Acked-by: Nicholas Piggin <npiggin@gmail.com>
Thanks,
Davidlohr
^ permalink raw reply
* [PATCH v2] powerpc/qspinlock: Use generic smp_cond_load_relaxed
From: Davidlohr Bueso @ 2021-03-18 20:47 UTC (permalink / raw)
To: npiggin
Cc: Davidlohr Bueso, will, dbueso, peterz, linux-kernel, mingo,
paulus, longman, linuxppc-dev
In-Reply-To: <1615870473.h7h4jetmjb.astroid@bobo.none>
49a7d46a06c3 (powerpc: Implement smp_cond_load_relaxed()) added
busy-waiting pausing with a preferred SMT priority pattern, lowering
the priority (reducing decode cycles) during the whole loop slowpath.
However, data shows that while this pattern works well with simple
spinlocks, queued spinlocks benefit more being kept in medium priority,
with a cpu_relax() instead, being a low+medium combo on powerpc.
Data is from three benchmarks on a Power9: 9008-22L 64 CPUs with
2 sockets and 8 threads per core.
1. locktorture.
This is data for the lowest and most artificial/pathological level,
with increasing thread counts pounding on the lock. Metrics are total
ops/minute. Despite some small hits in the 4-8 range, scenarios are
either neutral or favorable to this patch.
+=========+==========+==========+=======+
| # tasks | vanilla | dirty | %diff |
+=========+==========+==========+=======+
| 2 | 46718565 | 48751350 | 4.35 |
+---------+----------+----------+-------+
| 4 | 51740198 | 50369082 | -2.65 |
+---------+----------+----------+-------+
| 8 | 63756510 | 62568821 | -1.86 |
+---------+----------+----------+-------+
| 16 | 67824531 | 70966546 | 4.63 |
+---------+----------+----------+-------+
| 32 | 53843519 | 61155508 | 13.58 |
+---------+----------+----------+-------+
| 64 | 53005778 | 53104412 | 0.18 |
+---------+----------+----------+-------+
| 128 | 53331980 | 54606910 | 2.39 |
+=========+==========+==========+=======+
2. sockperf (tcp throughput)
Here a client will do one-way throughput tests to a localhost server, with
increasing message sizes, dealing with the sk_lock. This patch shows to put
the performance of the qspinlock back to par with that of the simple lock:
simple-spinlock vanilla dirty
Hmean 14 73.50 ( 0.00%) 54.44 * -25.93%* 73.45 * -0.07%*
Hmean 100 654.47 ( 0.00%) 385.61 * -41.08%* 771.43 * 17.87%*
Hmean 300 2719.39 ( 0.00%) 2181.67 * -19.77%* 2666.50 * -1.94%*
Hmean 500 4400.59 ( 0.00%) 3390.77 * -22.95%* 4322.14 * -1.78%*
Hmean 850 6726.21 ( 0.00%) 5264.03 * -21.74%* 6863.12 * 2.04%*
3. dbench (tmpfs)
Configured to run with up to ncpusx8 clients, it shows both latency and
throughput metrics. For the latency, with the exception of the 64 case,
there is really nothing to go by:
vanilla dirty
Amean latency-1 1.67 ( 0.00%) 1.67 * 0.09%*
Amean latency-2 2.15 ( 0.00%) 2.08 * 3.36%*
Amean latency-4 2.50 ( 0.00%) 2.56 * -2.27%*
Amean latency-8 2.49 ( 0.00%) 2.48 * 0.31%*
Amean latency-16 2.69 ( 0.00%) 2.72 * -1.37%*
Amean latency-32 2.96 ( 0.00%) 3.04 * -2.60%*
Amean latency-64 7.78 ( 0.00%) 8.17 * -5.07%*
Amean latency-512 186.91 ( 0.00%) 186.41 * 0.27%*
For the dbench4 Throughput (misleading but traditional) there's a small
but rather constant improvement:
vanilla dirty
Hmean 1 849.13 ( 0.00%) 851.51 * 0.28%*
Hmean 2 1664.03 ( 0.00%) 1663.94 * -0.01%*
Hmean 4 3073.70 ( 0.00%) 3104.29 * 1.00%*
Hmean 8 5624.02 ( 0.00%) 5694.16 * 1.25%*
Hmean 16 9169.49 ( 0.00%) 9324.43 * 1.69%*
Hmean 32 11969.37 ( 0.00%) 12127.09 * 1.32%*
Hmean 64 15021.12 ( 0.00%) 15243.14 * 1.48%*
Hmean 512 14891.27 ( 0.00%) 15162.11 * 1.82%*
Measuring the dbench4 Per-VFS Operation latency, shows some very minor
differences within the noise level, around the 0-1% ranges.
Fixes: 49a7d46a06c3 (powerpc: Implement smp_cond_load_relaxed())
Acked-by: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: Davidlohr Bueso <dbueso@suse.de>
---
Changes from v1:
Added small description and labeling smp_cond_load_relaxed requested by Nick.
Added Nick's ack.
arch/powerpc/include/asm/barrier.h | 16 ----------------
arch/powerpc/include/asm/qspinlock.h | 7 +++++++
2 files changed, 7 insertions(+), 16 deletions(-)
diff --git a/arch/powerpc/include/asm/barrier.h b/arch/powerpc/include/asm/barrier.h
index aecfde829d5d..7ae29cfb06c0 100644
--- a/arch/powerpc/include/asm/barrier.h
+++ b/arch/powerpc/include/asm/barrier.h
@@ -80,22 +80,6 @@ do { \
___p1; \
})
-#ifdef CONFIG_PPC64
-#define smp_cond_load_relaxed(ptr, cond_expr) ({ \
- typeof(ptr) __PTR = (ptr); \
- __unqual_scalar_typeof(*ptr) VAL; \
- VAL = READ_ONCE(*__PTR); \
- if (unlikely(!(cond_expr))) { \
- spin_begin(); \
- do { \
- VAL = READ_ONCE(*__PTR); \
- } while (!(cond_expr)); \
- spin_end(); \
- } \
- (typeof(*ptr))VAL; \
-})
-#endif
-
#ifdef CONFIG_PPC_BOOK3S_64
#define NOSPEC_BARRIER_SLOT nop
#elif defined(CONFIG_PPC_FSL_BOOK3E)
diff --git a/arch/powerpc/include/asm/qspinlock.h b/arch/powerpc/include/asm/qspinlock.h
index b052b0624816..9da649e1a488 100644
--- a/arch/powerpc/include/asm/qspinlock.h
+++ b/arch/powerpc/include/asm/qspinlock.h
@@ -72,6 +72,13 @@ static inline void pv_spinlocks_init(void)
#endif
+/*
+ * Queued spinlocks rely heavily on smp_cond_load_relaxed to busy-wait,
+ * which was found that have performance problems if implemented with
+ * the preferred spin_begin()/spin_end() SMT priority pattern. Use the
+ * generic version instead.
+ */
+
#include <asm-generic/qspinlock.h>
#endif /* _ASM_POWERPC_QSPINLOCK_H */
--
2.26.2
^ permalink raw reply related
* Re: [PATCH] net: marvell: Remove reference to CONFIG_MV64X60
From: patchwork-bot+netdevbpf @ 2021-03-18 21:30 UTC (permalink / raw)
To: Christophe Leroy
Cc: netdev, linux-kernel, kuba, linuxppc-dev, davem,
sebastian.hesselbarth
In-Reply-To: <7fc233cfbda60b87894c3f4a3b0d1e63fdb24b37.1616085654.git.christophe.leroy@csgroup.eu>
Hello:
This patch was applied to netdev/net.git (refs/heads/master):
On Thu, 18 Mar 2021 17:25:08 +0000 (UTC) you wrote:
> Commit 92c8c16f3457 ("powerpc/embedded6xx: Remove C2K board support")
> removed last selector of CONFIG_MV64X60.
>
> As it is not a user selectable config item, all references to it
> are stale. Remove them.
>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
>
> [...]
Here is the summary with links:
- net: marvell: Remove reference to CONFIG_MV64X60
https://git.kernel.org/netdev/net/c/600cc3c9c62d
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox