* [PATCH] kbuild: move bin2c back to scripts/ from scripts/basic/
From: Masahiro Yamada @ 2018-06-25 16:40 UTC (permalink / raw)
To: linux-kbuild
Cc: Masahiro Yamada, linux-s390, H. Peter Anvin, Kentaro Takeda,
Michael Ellerman, Heiko Carstens, x86, Benjamin Herrenschmidt,
linux-kernel, Thomas Gleixner, Michal Marek, Paul Mackerras,
Tetsuo Handa, Serge E. Hallyn, James Morris, Ingo Molnar,
linuxppc-dev, linux-security-module, Martin Schwidefsky
Commit 8370edea81e3 ("bin2c: move bin2c in scripts/basic") moved bin2c
to the scripts/basic/ directory, incorrectly stating "Kexec wants to
use bin2c and it wants to use it really early in the build process.
See arch/x86/purgatory/ code in later patches."
Commit bdab125c9301 ("Revert "kexec/purgatory: Add clean-up for
purgatory directory"") and commit d6605b6bbee8 ("x86/build: Remove
unnecessary preparation for purgatory") removed the redundant
purgatory build magic entirely.
That means that the move of bin2c was unnecessary in the first place.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
arch/powerpc/purgatory/Makefile | 3 +--
arch/s390/purgatory/Makefile | 3 +--
arch/x86/purgatory/Makefile | 3 +--
kernel/Makefile | 2 +-
scripts/.gitignore | 1 +
scripts/Makefile | 1 +
scripts/basic/.gitignore | 1 -
scripts/basic/Makefile | 1 -
scripts/{basic => }/bin2c.c | 0
security/tomoyo/Makefile | 2 +-
10 files changed, 7 insertions(+), 10 deletions(-)
rename scripts/{basic => }/bin2c.c (100%)
diff --git a/arch/powerpc/purgatory/Makefile b/arch/powerpc/purgatory/Makefile
index 30e05de..4314ba5 100644
--- a/arch/powerpc/purgatory/Makefile
+++ b/arch/powerpc/purgatory/Makefile
@@ -6,9 +6,8 @@ LDFLAGS_purgatory.ro := -e purgatory_start -r --no-undefined
$(obj)/purgatory.ro: $(obj)/trampoline.o FORCE
$(call if_changed,ld)
-CMD_BIN2C = $(objtree)/scripts/basic/bin2c
quiet_cmd_bin2c = BIN2C $@
- cmd_bin2c = $(CMD_BIN2C) kexec_purgatory < $< > $@
+ cmd_bin2c = $(objtree)/scripts/bin2c kexec_purgatory < $< > $@
$(obj)/kexec-purgatory.c: $(obj)/purgatory.ro FORCE
$(call if_changed,bin2c)
diff --git a/arch/s390/purgatory/Makefile b/arch/s390/purgatory/Makefile
index 1ace023..445c460 100644
--- a/arch/s390/purgatory/Makefile
+++ b/arch/s390/purgatory/Makefile
@@ -27,9 +27,8 @@ KBUILD_CFLAGS += $(call cc-option,-fno-PIE)
$(obj)/purgatory.ro: $(PURGATORY_OBJS) FORCE
$(call if_changed,ld)
-CMD_BIN2C = $(objtree)/scripts/basic/bin2c
quiet_cmd_bin2c = BIN2C $@
- cmd_bin2c = $(CMD_BIN2C) kexec_purgatory < $< > $@
+ cmd_bin2c = $(objtree)/scripts/bin2c kexec_purgatory < $< > $@
$(obj)/kexec-purgatory.c: $(obj)/purgatory.ro FORCE
$(call if_changed,bin2c)
diff --git a/arch/x86/purgatory/Makefile b/arch/x86/purgatory/Makefile
index 2e9ee02..d6ac098 100644
--- a/arch/x86/purgatory/Makefile
+++ b/arch/x86/purgatory/Makefile
@@ -28,9 +28,8 @@ $(obj)/purgatory.ro: $(PURGATORY_OBJS) FORCE
targets += kexec-purgatory.c
-CMD_BIN2C = $(objtree)/scripts/basic/bin2c
quiet_cmd_bin2c = BIN2C $@
- cmd_bin2c = $(CMD_BIN2C) kexec_purgatory < $< > $@
+ cmd_bin2c = $(objtree)/scripts/bin2c kexec_purgatory < $< > $@
$(obj)/kexec-purgatory.c: $(obj)/purgatory.ro FORCE
$(call if_changed,bin2c)
diff --git a/kernel/Makefile b/kernel/Makefile
index 04bc07c..7a63d56 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -123,7 +123,7 @@ targets += config_data.gz
$(obj)/config_data.gz: $(KCONFIG_CONFIG) FORCE
$(call if_changed,gzip)
- filechk_ikconfiggz = (echo "static const char kernel_config_data[] __used = MAGIC_START"; cat $< | scripts/basic/bin2c; echo "MAGIC_END;")
+ filechk_ikconfiggz = (echo "static const char kernel_config_data[] __used = MAGIC_START"; cat $< | scripts/bin2c; echo "MAGIC_END;")
targets += config_data.h
$(obj)/config_data.h: $(obj)/config_data.gz FORCE
$(call filechk,ikconfiggz)
diff --git a/scripts/.gitignore b/scripts/.gitignore
index 0442c06..12d302d 100644
--- a/scripts/.gitignore
+++ b/scripts/.gitignore
@@ -1,6 +1,7 @@
#
# Generated files
#
+bin2c
conmakehash
kallsyms
pnmtologo
diff --git a/scripts/Makefile b/scripts/Makefile
index 25ab143..59c21ec 100644
--- a/scripts/Makefile
+++ b/scripts/Makefile
@@ -10,6 +10,7 @@
HOST_EXTRACFLAGS += -I$(srctree)/tools/include
+hostprogs-$(CONFIG_BUILD_BIN2C) += bin2c
hostprogs-$(CONFIG_KALLSYMS) += kallsyms
hostprogs-$(CONFIG_LOGO) += pnmtologo
hostprogs-$(CONFIG_VT) += conmakehash
diff --git a/scripts/basic/.gitignore b/scripts/basic/.gitignore
index 9528ec9..a776371 100644
--- a/scripts/basic/.gitignore
+++ b/scripts/basic/.gitignore
@@ -1,2 +1 @@
fixdep
-bin2c
diff --git a/scripts/basic/Makefile b/scripts/basic/Makefile
index 0372b33..af49b44 100644
--- a/scripts/basic/Makefile
+++ b/scripts/basic/Makefile
@@ -9,7 +9,6 @@
# fixdep: Used to generate dependency information during build process
hostprogs-y := fixdep
-hostprogs-$(CONFIG_BUILD_BIN2C) += bin2c
always := $(hostprogs-y)
# fixdep is needed to compile other host programs
diff --git a/scripts/basic/bin2c.c b/scripts/bin2c.c
similarity index 100%
rename from scripts/basic/bin2c.c
rename to scripts/bin2c.c
diff --git a/security/tomoyo/Makefile b/security/tomoyo/Makefile
index b7c6a7f..cca5a30 100644
--- a/security/tomoyo/Makefile
+++ b/security/tomoyo/Makefile
@@ -4,7 +4,7 @@ obj-y = audit.o common.o condition.o domain.o environ.o file.o gc.o group.o load
targets += builtin-policy.h
define do_policy
echo "static char tomoyo_builtin_$(1)[] __initdata ="; \
-$(objtree)/scripts/basic/bin2c <$(firstword $(wildcard $(obj)/policy/$(1).conf $(srctree)/$(src)/policy/$(1).conf.default) /dev/null); \
+$(objtree)/scripts/bin2c <$(firstword $(wildcard $(obj)/policy/$(1).conf $(srctree)/$(src)/policy/$(1).conf.default) /dev/null); \
echo ";"
endef
quiet_cmd_policy = POLICY $@
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v2 0/6] powerpc/pkeys: fixes to pkeys
From: Ram Pai @ 2018-06-25 17:06 UTC (permalink / raw)
To: Michael Ellerman
Cc: Florian Weimer, linuxppc-dev, dave.hansen, aneesh.kumar,
bsingharora, hbabu, mhocko, bauerman, Ulrich.Weigand, luto,
msuchanek
In-Reply-To: <87in69eeat.fsf@concordia.ellerman.id.au>
On Sun, Jun 24, 2018 at 01:02:50AM +1000, Michael Ellerman wrote:
> Ram Pai <linuxram@us.ibm.com> writes:
> > On Thu, Jun 21, 2018 at 08:28:47PM +1000, Michael Ellerman wrote:
> >> Florian Weimer <fweimer@redhat.com> writes:
> >> > On 06/19/2018 02:40 PM, Michael Ellerman wrote:
> >> >>> I tested the whole series with the new selftests, with the printamr.c
> >> >>> program I posted earlier, and the glibc test for pkey_alloc &c. The
> >> >>> latter required some test fixes, but now passes as well. As far as I
> >> >>> can tell, everything looks good now.
> >> >>>
> >> >>> Tested-By: Florian Weimer<fweimer@redhat.com>
> >> >> Thanks. I'll add that to each patch I guess, if you're happy with that?
> >> >
> >> > Sure, but I only tested the whole series as a whole.
> >>
> >> Yeah OK. We don't have a good way to express that, other than using a
> >> merge which I'd prefer to avoid.
> >>
> >> So I've tagged them all with your Tested-by. If any of them turn out to
> >> have bugs you can blame me :)
> >
> > I just tested the patches incrementally using the pkey selftests.
> >
> > So I feel confident these patches are not bugs. I will take the blame
> > if the blame lands on Mpe :)
>
> Did you run core-pkey and ptrace-pkey?
>
> The pkey selftests that are in tools/testing/selftests/powerpc/ptrace ?
No. Ran the tools/testing/selftests/vm/protection_keys.
>
> Because those are failing for me:
>
> test: core_pkey
> tags: git_version:c899d94
> [FAIL] Test FAILED on line 245
> [Core Read (Running)] AMR: 3fcfffffffffffff IAMR: 1105555555555555 UAMOR: 33cfffffffffffff
> failure: core_pkey
>
> test: ptrace_pkey
> tags: git_version:c899d94
> [FAIL] Test FAILED on line 214
> [Ptrace Read (Running)] AMR: 3fcfffffffffffff IAMR: 1105555555555555 UAMOR: 33cfffffffffffff
> [User Write (Running)] AMR: 3fffffffffffffff pkey1: 3 pkey2: 4 pkey3: 5
> failure: ptrace_pkey
>
>
> Some of which is presumably test case bugs.
The test case is assuming execute-disable is disabled by default, i.e
all keys by default are execute-enabled. The new behavior by default is
execute-disable.
The test case need to be made aware of that.
> but there's at least one
> kernel bug with the UAMOR handling.
hmm.. yes. The UAMOR of the key is getting reset when the key is freed.
An artifact of the old behavior. The new behavior should never touch the
UAMOR register after initialization.
will send fixes to the above two anomolies.
RP
^ permalink raw reply
* Re: [PATCH] selftests/powerpc: Fix strncpy usage
From: Breno Leitao @ 2018-06-25 21:21 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: linuxppc-dev, Anshuman Khandual
In-Reply-To: <20180623011009.GX16221@gate.crashing.org>
hi Segher,
On 06/22/2018 10:10 PM, Segher Boessenkool wrote:
>> - strncpy(prog, argv[0], strlen(argv[0]));
>> + if (strlen(argv[0]) >= LEN_MAX){
>> + fprintf(stderr, "Very big executable name: %s\n", argv[0]);
>> + return 1;
>> + }
>> +
>> + strncpy(prog, argv[0], sizeof(prog) - 1);
>
> The strlen reads all of argv[0], which can be very big in theory. It won't
> matter in this test file -- program arguments cannot be super long, for one
> thing -- but it's not a good idea in general (that is one of the problems
> of strlcpy, btw).
>
> Best of course is to avoid string length restrictions completely, if you can.
Right, I was thinking about this problem and there is no motivation to have a
statically allocated and limited region.
I will send a v2 where 'prog' and avoid this restriction completely.
Thanks
^ permalink raw reply
* [PATCH v2] selftests/powerpc: Fix strncpy usage
From: Breno Leitao @ 2018-06-25 21:30 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Breno Leitao, Segher Boessenkool, Anshuman Khandual
In-Reply-To: <1529535071-14555-1-git-send-email-leitao@debian.org>
There is a buffer overflow in dscr_inherit_test.c test. In main(), strncpy()'s
third argument is the length of the source, not the size of the destination
buffer, which makes strncpy() behaves like strcpy(), causing a buffer overflow
if argv[0] is bigger than LEN_MAX (100).
This patch allocates 'prog' according to the argv[0] length, avoiding LEN_MAX
restriction.
CC: Segher Boessenkool <segher@kernel.crashing.org>
CC: Anshuman Khandual <khandual@linux.vnet.ibm.com>
Signed-off-by: Breno Leitao <leitao@debian.org>
---
tools/testing/selftests/powerpc/dscr/dscr_inherit_exec_test.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/powerpc/dscr/dscr_inherit_exec_test.c b/tools/testing/selftests/powerpc/dscr/dscr_inherit_exec_test.c
index 08a8b95e3bc1..ecac4900c7dd 100644
--- a/tools/testing/selftests/powerpc/dscr/dscr_inherit_exec_test.c
+++ b/tools/testing/selftests/powerpc/dscr/dscr_inherit_exec_test.c
@@ -19,7 +19,7 @@
*/
#include "dscr.h"
-static char prog[LEN_MAX];
+static char *prog;
static void do_exec(unsigned long parent_dscr)
{
@@ -104,6 +104,13 @@ int main(int argc, char *argv[])
exit(1);
}
- strncpy(prog, argv[0], strlen(argv[0]));
+ prog = malloc(strlen(argv[0]) + 1);
+ if (prog == NULL) {
+ fprintf(stderr, "Unable to allocate enough memory\n");
+ exit(1);
+ }
+
+ strcpy(prog, argv[0]);
+
return test_harness(dscr_inherit_exec, "dscr_inherit_exec_test");
}
--
2.16.3
^ permalink raw reply related
* [PATCH 1/2] powerpc/pkeys: preallocate execute_only key only if the key is available.
From: Ram Pai @ 2018-06-26 2:16 UTC (permalink / raw)
To: mpe
Cc: linuxppc-dev, hbabu, mhocko, bauerman, linuxram, Ulrich.Weigand,
fweimer, msuchanek
Key 2 is preallocated and reserved for execute-only key. In rare
cases if key-2 is unavailable, mprotect(PROT_EXEC) will behave
incorrectly. NOTE: mprotect(PROT_EXEC) uses execute-only key.
Ensure key 2 is available for preallocation before reserving it for
execute_only purpose. Problem noticed by Michael Ellermen.
Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
arch/powerpc/mm/pkeys.c | 14 +++++++++-----
1 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/mm/pkeys.c b/arch/powerpc/mm/pkeys.c
index cec990c..0b03914 100644
--- a/arch/powerpc/mm/pkeys.c
+++ b/arch/powerpc/mm/pkeys.c
@@ -19,6 +19,7 @@
u64 pkey_amr_mask; /* Bits in AMR not to be touched */
u64 pkey_iamr_mask; /* Bits in AMR not to be touched */
u64 pkey_uamor_mask; /* Bits in UMOR not to be touched */
+int execute_only_key = 2;
#define AMR_BITS_PER_PKEY 2
#define AMR_RD_BIT 0x1UL
@@ -26,7 +27,6 @@
#define IAMR_EX_BIT 0x1UL
#define PKEY_REG_BITS (sizeof(u64)*8)
#define pkeyshift(pkey) (PKEY_REG_BITS - ((pkey+1) * AMR_BITS_PER_PKEY))
-#define EXECUTE_ONLY_KEY 2
static void scan_pkey_feature(void)
{
@@ -122,8 +122,12 @@ int pkey_initialize(void)
#else
os_reserved = 0;
#endif
+
+ if ((pkeys_total - os_reserved) <= execute_only_key)
+ execute_only_key = -1;
+
/* Bits are in LE format. */
- reserved_allocation_mask = (0x1 << 1) | (0x1 << EXECUTE_ONLY_KEY);
+ reserved_allocation_mask = (0x1 << 1) | (0x1 << execute_only_key);
initial_allocation_mask = reserved_allocation_mask | (0x1 << PKEY_0);
/* register mask is in BE format */
@@ -132,11 +136,11 @@ int pkey_initialize(void)
pkey_iamr_mask = ~0x0ul;
pkey_iamr_mask &= ~(0x3ul << pkeyshift(PKEY_0));
- pkey_iamr_mask &= ~(0x3ul << pkeyshift(EXECUTE_ONLY_KEY));
+ pkey_iamr_mask &= ~(0x3ul << pkeyshift(execute_only_key));
pkey_uamor_mask = ~0x0ul;
pkey_uamor_mask &= ~(0x3ul << pkeyshift(PKEY_0));
- pkey_uamor_mask &= ~(0x3ul << pkeyshift(EXECUTE_ONLY_KEY));
+ pkey_uamor_mask &= ~(0x3ul << pkeyshift(execute_only_key));
for (i = (pkeys_total - os_reserved); i < pkeys_total; i++)
pkey_uamor_mask &= ~(0x3ul << pkeyshift(i));
@@ -151,7 +155,7 @@ void pkey_mm_init(struct mm_struct *mm)
if (static_branch_likely(&pkey_disabled))
return;
mm_pkey_allocation_map(mm) = initial_allocation_mask;
- mm->context.execute_only_pkey = EXECUTE_ONLY_KEY;
+ mm->context.execute_only_pkey = execute_only_key;
}
static inline u64 read_amr(void)
--
1.7.1
^ permalink raw reply related
* [PATCH 2/2] powerpc/pkeys: key allocation/deallocation must not change pkey registers
From: Ram Pai @ 2018-06-26 2:16 UTC (permalink / raw)
To: mpe
Cc: linuxppc-dev, hbabu, mhocko, bauerman, linuxram, Ulrich.Weigand,
fweimer, msuchanek
In-Reply-To: <1529979376-7292-1-git-send-email-linuxram@us.ibm.com>
Key allocation and deallocation has the side effect of programming the
UAMOR/AMR/IAMR registers. This is wrong, since its the responsibility of
the application and not that of the kernel, to modify the permission on
the key.
Do not modify the pkey registers at key allocation/deallocation.
This patch also fixes a bug where a sys_pkey_free() resets the UAMOR
bits of the key, thus making its permissions unmodifiable from user
space. Latter if the same key gets reallocated from a different thread
this thread will no longer be able to change the permissions on the key.
Problem noticed/reported by Michael Ellermen while running
selftests/core-pkeys
Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
arch/powerpc/include/asm/pkeys.h | 11 -----------
arch/powerpc/mm/pkeys.c | 27 ---------------------------
2 files changed, 0 insertions(+), 38 deletions(-)
diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
index c824528..92a9962 100644
--- a/arch/powerpc/include/asm/pkeys.h
+++ b/arch/powerpc/include/asm/pkeys.h
@@ -101,8 +101,6 @@ static inline bool mm_pkey_is_allocated(struct mm_struct *mm, int pkey)
return __mm_pkey_is_allocated(mm, pkey);
}
-extern void __arch_activate_pkey(int pkey);
-extern void __arch_deactivate_pkey(int pkey);
/*
* Returns a positive, 5-bit key on success, or -1 on failure.
* Relies on the mmap_sem to protect against concurrency in mm_pkey_alloc() and
@@ -131,11 +129,6 @@ static inline int mm_pkey_alloc(struct mm_struct *mm)
ret = ffz((u32)mm_pkey_allocation_map(mm));
__mm_pkey_allocated(mm, ret);
- /*
- * Enable the key in the hardware
- */
- if (ret > 0)
- __arch_activate_pkey(ret);
return ret;
}
@@ -147,10 +140,6 @@ static inline int mm_pkey_free(struct mm_struct *mm, int pkey)
if (!mm_pkey_is_allocated(mm, pkey))
return -EINVAL;
- /*
- * Disable the key in the hardware
- */
- __arch_deactivate_pkey(pkey);
__mm_pkey_free(mm, pkey);
return 0;
diff --git a/arch/powerpc/mm/pkeys.c b/arch/powerpc/mm/pkeys.c
index 0b03914..27ac7f0 100644
--- a/arch/powerpc/mm/pkeys.c
+++ b/arch/powerpc/mm/pkeys.c
@@ -224,33 +224,6 @@ static inline void init_iamr(int pkey, u8 init_bits)
write_iamr(old_iamr | new_iamr_bits);
}
-static void pkey_status_change(int pkey, bool enable)
-{
- u64 old_uamor;
-
- /* Reset the AMR and IAMR bits for this key */
- init_amr(pkey, 0x0);
- init_iamr(pkey, 0x0);
-
- /* Enable/disable key */
- old_uamor = read_uamor();
- if (enable)
- old_uamor |= (0x3ul << pkeyshift(pkey));
- else
- old_uamor &= ~(0x3ul << pkeyshift(pkey));
- write_uamor(old_uamor);
-}
-
-void __arch_activate_pkey(int pkey)
-{
- pkey_status_change(pkey, true);
-}
-
-void __arch_deactivate_pkey(int pkey)
-{
- pkey_status_change(pkey, false);
-}
-
/*
* Set the access rights in AMR IAMR and UAMOR registers for @pkey to that
* specified in @init_val.
--
1.7.1
^ permalink raw reply related
* [PATCH 1/2] powerpc/core-pkeys: execute-permission on keys are disabled by default
From: Ram Pai @ 2018-06-26 2:16 UTC (permalink / raw)
To: mpe
Cc: linuxppc-dev, hbabu, mhocko, bauerman, linuxram, Ulrich.Weigand,
fweimer, msuchanek
In-Reply-To: <1529979376-7292-1-git-send-email-linuxram@us.ibm.com>
Only when the key is allocated, its permission are enabled.
Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
tools/testing/selftests/powerpc/ptrace/core-pkey.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/tools/testing/selftests/powerpc/ptrace/core-pkey.c b/tools/testing/selftests/powerpc/ptrace/core-pkey.c
index 36bc312..b353d86 100644
--- a/tools/testing/selftests/powerpc/ptrace/core-pkey.c
+++ b/tools/testing/selftests/powerpc/ptrace/core-pkey.c
@@ -140,6 +140,10 @@ static int child(struct shared_info *info)
if (disable_execute)
info->iamr |= 1ul << pkeyshift(pkey1);
+ else
+ info->iamr &= ~(1ul << pkeyshift(pkey1));
+ info->iamr &= ~(1ul << pkeyshift(pkey2) | 1ul << pkeyshift(pkey3));
+
info->uamor |= 3ul << pkeyshift(pkey1) | 3ul << pkeyshift(pkey2);
--
1.7.1
^ permalink raw reply related
* [PATCH 2/2] powerpc/ptrace-pkeys: execute-permission on keys are disabled by default
From: Ram Pai @ 2018-06-26 2:16 UTC (permalink / raw)
To: mpe
Cc: linuxppc-dev, hbabu, mhocko, bauerman, linuxram, Ulrich.Weigand,
fweimer, msuchanek
In-Reply-To: <1529979376-7292-1-git-send-email-linuxram@us.ibm.com>
The test case assumes execute-permissions of unallocated keys are
enabled by default.
Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
.../testing/selftests/powerpc/ptrace/ptrace-pkey.c | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/tools/testing/selftests/powerpc/ptrace/ptrace-pkey.c b/tools/testing/selftests/powerpc/ptrace/ptrace-pkey.c
index 5cf631f..559c6cb 100644
--- a/tools/testing/selftests/powerpc/ptrace/ptrace-pkey.c
+++ b/tools/testing/selftests/powerpc/ptrace/ptrace-pkey.c
@@ -104,6 +104,11 @@ static int child(struct shared_info *info)
if (disable_execute)
info->expected_iamr |= 1ul << pkeyshift(pkey1);
+ else
+ info->expected_iamr &= ~(1ul << pkeyshift(pkey1));
+ info->expected_iamr &= ~(1ul << pkeyshift(pkey2) | 1ul << pkeyshift(pkey3));
+
+
info->expected_uamor |= 3ul << pkeyshift(pkey1) |
3ul << pkeyshift(pkey2);
--
1.7.1
^ permalink raw reply related
* Re: [PATCH 00/10] cxl: Remove abandonned capi support for the Mellanox CX4
From: Andrew Donnellan @ 2018-06-26 2:31 UTC (permalink / raw)
To: Frederic Barrat, alastair, vaibhav, clombard, felix, linuxppc-dev; +Cc: huyn
In-Reply-To: <20180625162406.10813-1-fbarrat@linux.ibm.com>
On 26/06/18 02:23, Frederic Barrat wrote:
> An attempt was made to add capi support for the Mellanox CX4 card, so
> that it could operate in "traditional" PCI mode or capi mode. The
> project ended up being canceled and a different approach was taken for
> CX5. Mellanox never upstreamed any capi support in their CX4 driver.
>
> CX4 was not following exactly the CAIA 1.0 model, so some CX4-specific
> code was added. That code is now dead and hasn't been tested for a
> while, so it's probably broken anyway. Let's remove it. It has been
> agreed with engineers at Mellanox, of course.
>
> No user API is affected. Some (unused) symbols exported by cxl are removed.
>
> Most of the patch set consists of reverts of older patches, with
> sometimes very minor tweaking. The last patch aggregates a few changes
> where reverting was not possible easily.
>
> Tests run to prevent regressions:
> - memcpy tests on POWER8 and POWER9
> - Mellanox CX5, which uses a different code path, based on cxllib
> - cxlflash tests on POWER8
9 of these commits are missing Signed-off-bys.
git revert -s is a thing :)
--
Andrew Donnellan OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com IBM Australia Limited
^ permalink raw reply
* Re: [PATCH 00/10] cxl: Remove abandonned capi support for the Mellanox CX4
From: Andrew Donnellan @ 2018-06-26 3:26 UTC (permalink / raw)
To: Frederic Barrat, alastair, vaibhav, clombard, felix, linuxppc-dev; +Cc: huyn
In-Reply-To: <21307b77-9c3e-574b-e4af-e61ecc54a803@au1.ibm.com>
On 26/06/18 12:31, Andrew Donnellan wrote:
> 9 of these commits are missing Signed-off-bys.
>
> git revert -s is a thing :)
After mentally preparing myself for the flashbacks from when we had to
implement some of these hacks in the first place, I've read through the
rest of the series and it all looks good!
Consider this entire series, with the signoffs fixed:
Acked-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
--
Andrew Donnellan OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com IBM Australia Limited
^ permalink raw reply
* Re: [PATCHv2 2/2] drivers/base: reorder consumer and its children behind suppliers
From: Pingfan Liu @ 2018-06-26 3:29 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: linux-kernel, Grygorii Strashko, Christoph Hellwig, Bjorn Helgaas,
Dave Young, linux-pci, linuxppc-dev
In-Reply-To: <20180625104505.GA3058@kroah.com>
On Mon, Jun 25, 2018 at 6:45 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Mon, Jun 25, 2018 at 03:47:39PM +0800, Pingfan Liu wrote:
> > commit 52cdbdd49853 ("driver core: correct device's shutdown order")
> > introduces supplier<-consumer order in devices_kset. The commit tries
> > to cleverly maintain both parent<-child and supplier<-consumer order by
> > reordering a device when probing. This method makes things simple and
> > clean, but unfortunately, breaks parent<-child order in some case,
> > which is described in next patch in this series.
>
> There is no "next patch in this series" :(
>
Oh, re-arrange the patches, and forget the comment in log
> > Here this patch tries to resolve supplier<-consumer by only reordering a
> > device when it has suppliers, and takes care of the following scenario:
> > [consumer, children] [ ... potential ... ] supplier
> > ^ ^
> > After moving the consumer and its children after the supplier, the
> > potentail section may contain consumers whose supplier is inside
> > children, and this poses the requirement to dry out all consumpers in
> > the section recursively.
> >
> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Cc: Grygorii Strashko <grygorii.strashko@ti.com>
> > Cc: Christoph Hellwig <hch@infradead.org>
> > Cc: Bjorn Helgaas <helgaas@kernel.org>
> > Cc: Dave Young <dyoung@redhat.com>
> > Cc: linux-pci@vger.kernel.org
> > Cc: linuxppc-dev@lists.ozlabs.org
> > Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
> > ---
> > note: there is lock issue in this patch, should be fixed in next version
>
> Please send patches that you know are correct, why would I want to
> review this if you know it is not correct?
>
> And if the original commit is causing problems for you, why not just
> revert that instead of adding this much-increased complexity?
>
Revert the original commit, then it will expose the error order
"consumer <- supplier" again.
This patch tries to resolve the error and fix the following scenario:
step0: before the consumer device's probing, (note child_a is a
supplier of consumer_a, etc)
[ consumer-X, child_a, ...., child_z] [.... consumer_a, ...,
consumer_z, ....] supplier-X
^^^
affected range during moving^^^
step1: When probing, moving consumer-X after supplier-X
[ child_a, ...., child_z] [.... consumer_a, ..., consumer_z,
....] supplier-X, consumer-X
But it breaks "parent <-child" seq now, and should be fixed like:
step2:
[.... consumer_a, ..., consumer_z, ....] supplier-X [
consumer-X, child_a, ...., child_z] <---
descendants_reorder_after_pos() does it.
Again, the seq "consumer_a <- child_a" breaks the "supplier<-consumer"
order, should be fixed like:
step3:
[.... consumer_z, .....] supplier-X [ consumer-X, child_a,
consumer_a ...., child_z] <--- __device_reorder_consumer() does it.
^^ affected range^^
The moving of consumer_a brings us to face the same scenario of step1,
hence we need an external recursion.
Each round of step3, __device_reorder_consumer() resolves its "local
affected range", which is a fraction of the "whole affected range".
Hence finally, we have all potential consumers in affected range resolved.
(Maybe I can split patch at step2 and step3 to ease the review for the
next version)
Since __device_reorder_consumer() has already hold devices_kset's spin
lock, and need to get srcu lock on devices->links.consumers.
This needs a breakage of spin lock, and will incur much effort. If the
above algorithm is fine, I can do it.
>
>
> >
> > ---
> > drivers/base/core.c | 132 ++++++++++++++++++++++++++++++++++++++++++++++++++--
> > 1 file changed, 129 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/base/core.c b/drivers/base/core.c
> > index 66f06ff..db30e86 100644
> > --- a/drivers/base/core.c
> > +++ b/drivers/base/core.c
> > @@ -123,12 +123,138 @@ static int device_is_dependent(struct device *dev, void *target)
> > return ret;
> > }
> >
> > -/* a temporary place holder to mark out the root cause of the bug.
> > - * The proposal algorithm will come in next patch
> > +struct pos_info {
> > + struct device *pos;
> > + struct device *tail;
> > +};
> > +
> > +/* caller takes the devices_kset->list_lock */
> > +static int descendants_reorder_after_pos(struct device *dev,
> > + void *data)
>
> Why are you wrapping lines that do not need to be wrapped?
>
OK, will fix.
> What does this function do?
>
As the name implies, reordering dev and its children after a position.
When moving a consumer after a supplier, we break down the order
of "parent <-child" order of consumer and its children in devices_kset.
Hence we should move the children too.
The param "data" contains the position info, and its name is not
illuminated :(,
since the func proto is required by device_for_each_child(), may be better to
name it as postion_info
> > +{
> > + struct device *pos;
> > + struct pos_info *p = data;
> > +
> > + pos = p->pos;
> > + pr_debug("devices_kset: Moving %s after %s\n",
> > + dev_name(dev), dev_name(pos));
>
> You have a device, use it for debugging, i.e. dev_dbg().
>
But here we have two devices.
> > + device_for_each_child(dev, p, descendants_reorder_after_pos);
>
> Recursive?
>
Yes, in order to move all children of the consumer.
> > + /* children at the tail */
> > + list_move(&dev->kobj.entry, &pos->kobj.entry);
> > + /* record the right boundary of the section */
> > + if (p->tail == NULL)
> > + p->tail = dev;
> > + return 0;
> > +}
>
> I really do not understand what the above code is supposed to be doing :(
>
The moved consumer's children may be suppliers of devices,
[.... consumer_a, ..., consumer_z, ....] supplier-X [
consumer-X, child_a, ............, child_z]
^^^ potential consumers ^^^^^^
^^potential suppliers^^
Now, consumer_a and its supplier child_a violate the order
"supplier<-consumer".
To pick out such violation, we need to check the potential suppliers
against potential
consumers. And p->tail helps to record the new moved position of child_z.
> > +
> > +/* iterate over an open section */
> > +#define list_opensect_for_each_reverse(cur, left, right) \
> > + for (cur = right->prev; cur == left; cur = cur->prev)
> > +
> > +static bool is_consumer(struct device *query, struct device *supplier)
> > +{
> > + struct device_link *link;
> > + /* todo, lock protection */
>
> Always run checkpatch.pl on patches so you do not get grumpy maintainers
> telling you to run checkpatch.pl :(
>
Yes, I had run it, and only got a warning:
WARNING: Avoid crashing the kernel - try using WARN_ON & recovery code
rather than BUG() or BUG_ON()
#167: FILE: drivers/base/core.c:245:
+ BUG_ON(!ret);
total: 0 errors, 1 warnings, 141 lines checked
> > + list_for_each_entry(link, &supplier->links.consumers, s_node)
> > + if (link->consumer == query)
> > + return true;
> > + return false;
> > +}
> > +
> > +/* recursively move the potential consumers in open section (left, right)
> > + * after the barrier
>
> What barrier?
>
A position that moved devices can not cross before.
> I'm stopping here as I have no idea what is going on, and this needs a
> lot more work at the basic level of "it handles locking correctly"...
>
> If you are working on this for power9, I'm guessing you work for IBM?
No. I just hit this bug.
> If so, please run this through your internal patch review process before
> sending it out again...
>
I will try my best to find some guys to review. But is the assumption
of step0 and
the following algorithm worth to try?
Thanks and regards,
Pingfan
^ permalink raw reply
* Re: [PATCH kernel] powerpc/powernv/ioda2: Reduce upper limit for DMA window size
From: Alexey Kardashevskiy @ 2018-06-26 4:56 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <20180601080616.29279-1-aik@ozlabs.ru>
On Fri, 1 Jun 2018 18:06:16 +1000
Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> We use PHB in mode1 which uses bit 59 to select a correct DMA window.
> However there is mode2 which uses bits 59:55 and allows up to 32 DMA
> windows per a PE.
>
> Even though documentation does not clearly specify that, it seems that
> the actual hardware does not support bits 59:55 even in mode1, in other
> words we can create a window as big as 1<<58 but DMA simply won't work.
>
> This reduces the upper limit from 59 to 55 bits to let the userspace know
> about the hardware limits.
>
> Fixes: 7aafac11e3 "powerpc/powernv/ioda2: Gracefully fail if too many TCE levels requested"
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Ping?
> ---
> arch/powerpc/platforms/powernv/pci-ioda.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
> index 92ca662..50e21d7 100644
> --- a/arch/powerpc/platforms/powernv/pci-ioda.c
> +++ b/arch/powerpc/platforms/powernv/pci-ioda.c
> @@ -2839,7 +2839,7 @@ static long pnv_pci_ioda2_table_alloc_pages(int nid, __u64 bus_offset,
> level_shift = entries_shift + 3;
> level_shift = max_t(unsigned, level_shift, PAGE_SHIFT);
>
> - if ((level_shift - 3) * levels + page_shift >= 60)
> + if ((level_shift - 3) * levels + page_shift >= 55)
> return -EINVAL;
>
> /* Allocate TCE table */
> --
> 2.11.0
>
--
Alexey
^ permalink raw reply
* Re: [PATCH v2] selftests/powerpc: Fix strncpy usage
From: Michael Ellerman @ 2018-06-26 5:24 UTC (permalink / raw)
To: Breno Leitao, linuxppc-dev; +Cc: Breno Leitao, Anshuman Khandual
In-Reply-To: <1529962249-24774-1-git-send-email-leitao@debian.org>
Breno Leitao <leitao@debian.org> writes:
> There is a buffer overflow in dscr_inherit_test.c test. In main(), strncpy()'s
> third argument is the length of the source, not the size of the destination
> buffer, which makes strncpy() behaves like strcpy(), causing a buffer overflow
> if argv[0] is bigger than LEN_MAX (100).
>
> This patch allocates 'prog' according to the argv[0] length, avoiding LEN_MAX
> restriction.
>
> CC: Segher Boessenkool <segher@kernel.crashing.org>
> CC: Anshuman Khandual <khandual@linux.vnet.ibm.com>
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
> tools/testing/selftests/powerpc/dscr/dscr_inherit_exec_test.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/tools/testing/selftests/powerpc/dscr/dscr_inherit_exec_test.c b/tools/testing/selftests/powerpc/dscr/dscr_inherit_exec_test.c
> index 08a8b95e3bc1..ecac4900c7dd 100644
> --- a/tools/testing/selftests/powerpc/dscr/dscr_inherit_exec_test.c
> +++ b/tools/testing/selftests/powerpc/dscr/dscr_inherit_exec_test.c
> @@ -19,7 +19,7 @@
> */
> #include "dscr.h"
>
> -static char prog[LEN_MAX];
> +static char *prog;
>
> static void do_exec(unsigned long parent_dscr)
> {
> @@ -104,6 +104,13 @@ int main(int argc, char *argv[])
> exit(1);
> }
>
> - strncpy(prog, argv[0], strlen(argv[0]));
> + prog = malloc(strlen(argv[0]) + 1);
> + if (prog == NULL) {
> + fprintf(stderr, "Unable to allocate enough memory\n");
> + exit(1);
> + }
> +
> + strcpy(prog, argv[0]);
Why do we need to copy it at all?
Can't we just save a pointer it? ie, prog = argv[0];
What am I missing?
cheers
^ permalink raw reply
* [PATCH kernel v2 0/2] KVM: PPC: Check if IOMMU page is contained in the pinned physical page
From: Alexey Kardashevskiy @ 2018-06-26 5:59 UTC (permalink / raw)
To: linuxppc-dev
Cc: Alexey Kardashevskiy, David Gibson, kvm-ppc, Alex Williamson,
Paul Mackerras
This is to improve page boundaries checking and should probably
be cc:stable. I came accross this while debugging nvlink2 passthrough
but the lack of checking might be exploited by the existing userspace.
Changes:
v2:
* 2/2: explicitly check for compound pages before calling compound_order()
Please comment. Thanks.
Alexey Kardashevskiy (2):
vfio/spapr: Use IOMMU pageshift rather than pagesize
KVM: PPC: Check if IOMMU page is contained in the pinned physical page
arch/powerpc/include/asm/mmu_context.h | 4 ++--
arch/powerpc/kvm/book3s_64_vio.c | 2 +-
arch/powerpc/kvm/book3s_64_vio_hv.c | 6 ++++--
arch/powerpc/mm/mmu_context_iommu.c | 20 +++++++++++++++++---
drivers/vfio/vfio_iommu_spapr_tce.c | 10 +++++-----
5 files changed, 29 insertions(+), 13 deletions(-)
--
2.11.0
^ permalink raw reply
* [PATCH kernel v2 1/2] vfio/spapr: Use IOMMU pageshift rather than pagesize
From: Alexey Kardashevskiy @ 2018-06-26 5:59 UTC (permalink / raw)
To: linuxppc-dev
Cc: Alexey Kardashevskiy, David Gibson, kvm-ppc, Alex Williamson,
Paul Mackerras
In-Reply-To: <20180626055926.27703-1-aik@ozlabs.ru>
The size is always equal to 1 page so let's use this. Later on this will
be used for other checks which use page shifts to check the granularity
of access.
This should cause no behavioral change.
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
drivers/vfio/vfio_iommu_spapr_tce.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c b/drivers/vfio/vfio_iommu_spapr_tce.c
index 759a5bd..2da5f05 100644
--- a/drivers/vfio/vfio_iommu_spapr_tce.c
+++ b/drivers/vfio/vfio_iommu_spapr_tce.c
@@ -457,13 +457,13 @@ static void tce_iommu_unuse_page(struct tce_container *container,
}
static int tce_iommu_prereg_ua_to_hpa(struct tce_container *container,
- unsigned long tce, unsigned long size,
+ unsigned long tce, unsigned long shift,
unsigned long *phpa, struct mm_iommu_table_group_mem_t **pmem)
{
long ret = 0;
struct mm_iommu_table_group_mem_t *mem;
- mem = mm_iommu_lookup(container->mm, tce, size);
+ mem = mm_iommu_lookup(container->mm, tce, 1ULL << shift);
if (!mem)
return -EINVAL;
@@ -487,7 +487,7 @@ static void tce_iommu_unuse_page_v2(struct tce_container *container,
if (!pua)
return;
- ret = tce_iommu_prereg_ua_to_hpa(container, *pua, IOMMU_PAGE_SIZE(tbl),
+ ret = tce_iommu_prereg_ua_to_hpa(container, *pua, tbl->it_page_shift,
&hpa, &mem);
if (ret)
pr_debug("%s: tce %lx at #%lx was not cached, ret=%d\n",
@@ -611,7 +611,7 @@ static long tce_iommu_build_v2(struct tce_container *container,
entry + i);
ret = tce_iommu_prereg_ua_to_hpa(container,
- tce, IOMMU_PAGE_SIZE(tbl), &hpa, &mem);
+ tce, tbl->it_page_shift, &hpa, &mem);
if (ret)
break;
--
2.11.0
^ permalink raw reply related
* [PATCH kernel v2 2/2] KVM: PPC: Check if IOMMU page is contained in the pinned physical page
From: Alexey Kardashevskiy @ 2018-06-26 5:59 UTC (permalink / raw)
To: linuxppc-dev
Cc: Alexey Kardashevskiy, David Gibson, kvm-ppc, Alex Williamson,
Paul Mackerras
In-Reply-To: <20180626055926.27703-1-aik@ozlabs.ru>
We already have a check in drivers/vfio/vfio_iommu_spapr_tce.c that
an IOMMU page is contained in the physical page so the PCI hardware won't
get access to unassigned host memory.
However we do not have this check in KVM fastpath (H_PUT_TCE accelerated
code) so the user space can pin memory backed with 64k pages and create
a hardware TCE table with a bigger page size. We were lucky so far and
did not hit this yet as the very first time the mapping happens
we do not have tbl::it_userspace allocated yet and fall back to
the userspace which in turn calls VFIO IOMMU driver and that fails
because of the check in vfio_iommu_spapr_tce.c which is really
sustainable solution.
This stores the smallest preregistered page size in the preregistered
region descriptor and changes the mm_iommu_xxx API to check this against
the IOMMU page size.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
Changes:
v2:
* explicitly check for compound pages before calling compound_order()
---
The bug is: run QEMU _without_ hugepages (no -mempath) and tell it to
advertise 16MB pages to the guest; a typical pseries guest will use 16MB
for IOMMU pages without checking the mmu pagesize and this will fail
at https://git.qemu.org/?p=qemu.git;a=blob;f=hw/vfio/common.c;h=fb396cf00ac40eb35967a04c9cc798ca896eed57;hb=refs/heads/master#l256
With the change, mapping will fail in KVM and the guest will print:
mlx5_core 0000:00:00.0: ibm,create-pe-dma-window(2027) 0 8000000 20000000 18 1f returned 0 (liobn = 0x80000001 starting addr = 8000000 0)
mlx5_core 0000:00:00.0: created tce table LIOBN 0x80000001 for /pci@800000020000000/ethernet@0
mlx5_core 0000:00:00.0: failed to map direct window for /pci@800000020000000/ethernet@0: -1
---
arch/powerpc/include/asm/mmu_context.h | 4 ++--
arch/powerpc/kvm/book3s_64_vio.c | 2 +-
arch/powerpc/kvm/book3s_64_vio_hv.c | 6 ++++--
arch/powerpc/mm/mmu_context_iommu.c | 20 +++++++++++++++++---
drivers/vfio/vfio_iommu_spapr_tce.c | 2 +-
5 files changed, 25 insertions(+), 9 deletions(-)
diff --git a/arch/powerpc/include/asm/mmu_context.h b/arch/powerpc/include/asm/mmu_context.h
index 896efa5..79d570c 100644
--- a/arch/powerpc/include/asm/mmu_context.h
+++ b/arch/powerpc/include/asm/mmu_context.h
@@ -35,9 +35,9 @@ extern struct mm_iommu_table_group_mem_t *mm_iommu_lookup_rm(
extern struct mm_iommu_table_group_mem_t *mm_iommu_find(struct mm_struct *mm,
unsigned long ua, unsigned long entries);
extern long mm_iommu_ua_to_hpa(struct mm_iommu_table_group_mem_t *mem,
- unsigned long ua, unsigned long *hpa);
+ unsigned long ua, unsigned int pageshift, unsigned long *hpa);
extern long mm_iommu_ua_to_hpa_rm(struct mm_iommu_table_group_mem_t *mem,
- unsigned long ua, unsigned long *hpa);
+ unsigned long ua, unsigned int pageshift, unsigned long *hpa);
extern long mm_iommu_mapped_inc(struct mm_iommu_table_group_mem_t *mem);
extern void mm_iommu_mapped_dec(struct mm_iommu_table_group_mem_t *mem);
#endif
diff --git a/arch/powerpc/kvm/book3s_64_vio.c b/arch/powerpc/kvm/book3s_64_vio.c
index d066e37..8c456fa 100644
--- a/arch/powerpc/kvm/book3s_64_vio.c
+++ b/arch/powerpc/kvm/book3s_64_vio.c
@@ -449,7 +449,7 @@ long kvmppc_tce_iommu_do_map(struct kvm *kvm, struct iommu_table *tbl,
/* This only handles v2 IOMMU type, v1 is handled via ioctl() */
return H_TOO_HARD;
- if (WARN_ON_ONCE(mm_iommu_ua_to_hpa(mem, ua, &hpa)))
+ if (WARN_ON_ONCE(mm_iommu_ua_to_hpa(mem, ua, tbl->it_page_shift, &hpa)))
return H_HARDWARE;
if (mm_iommu_mapped_inc(mem))
diff --git a/arch/powerpc/kvm/book3s_64_vio_hv.c b/arch/powerpc/kvm/book3s_64_vio_hv.c
index 925fc31..5b298f5 100644
--- a/arch/powerpc/kvm/book3s_64_vio_hv.c
+++ b/arch/powerpc/kvm/book3s_64_vio_hv.c
@@ -279,7 +279,8 @@ static long kvmppc_rm_tce_iommu_do_map(struct kvm *kvm, struct iommu_table *tbl,
if (!mem)
return H_TOO_HARD;
- if (WARN_ON_ONCE_RM(mm_iommu_ua_to_hpa_rm(mem, ua, &hpa)))
+ if (WARN_ON_ONCE_RM(mm_iommu_ua_to_hpa_rm(mem, ua, tbl->it_page_shift,
+ &hpa)))
return H_HARDWARE;
pua = (void *) vmalloc_to_phys(pua);
@@ -469,7 +470,8 @@ long kvmppc_rm_h_put_tce_indirect(struct kvm_vcpu *vcpu,
mem = mm_iommu_lookup_rm(vcpu->kvm->mm, ua, IOMMU_PAGE_SIZE_4K);
if (mem)
- prereg = mm_iommu_ua_to_hpa_rm(mem, ua, &tces) == 0;
+ prereg = mm_iommu_ua_to_hpa_rm(mem, ua,
+ IOMMU_PAGE_SHIFT_4K, &tces) == 0;
}
if (!prereg) {
diff --git a/arch/powerpc/mm/mmu_context_iommu.c b/arch/powerpc/mm/mmu_context_iommu.c
index abb4364..dc0e8cd 100644
--- a/arch/powerpc/mm/mmu_context_iommu.c
+++ b/arch/powerpc/mm/mmu_context_iommu.c
@@ -27,6 +27,7 @@ struct mm_iommu_table_group_mem_t {
struct rcu_head rcu;
unsigned long used;
atomic64_t mapped;
+ unsigned int pageshift;
u64 ua; /* userspace address */
u64 entries; /* number of entries in hpas[] */
u64 *hpas; /* vmalloc'ed */
@@ -124,7 +125,7 @@ long mm_iommu_get(struct mm_struct *mm, unsigned long ua, unsigned long entries,
struct mm_iommu_table_group_mem_t **pmem)
{
struct mm_iommu_table_group_mem_t *mem;
- long i, j, ret = 0, locked_entries = 0;
+ long i, j, ret = 0, locked_entries = 0, pageshift;
struct page *page = NULL;
mutex_lock(&mem_list_mutex);
@@ -166,6 +167,8 @@ long mm_iommu_get(struct mm_struct *mm, unsigned long ua, unsigned long entries,
goto unlock_exit;
}
+ mem->pageshift = 30; /* start from 1G pages - the biggest we have */
+
for (i = 0; i < entries; ++i) {
if (1 != get_user_pages_fast(ua + (i << PAGE_SHIFT),
1/* pages */, 1/* iswrite */, &page)) {
@@ -199,6 +202,11 @@ long mm_iommu_get(struct mm_struct *mm, unsigned long ua, unsigned long entries,
}
}
populate:
+ pageshift = PAGE_SHIFT;
+ if (PageCompound(page))
+ pageshift += compound_order(compound_head(page));
+ mem->pageshift = min_t(unsigned int, mem->pageshift, pageshift);
+
mem->hpas[i] = page_to_pfn(page) << PAGE_SHIFT;
}
@@ -349,7 +357,7 @@ struct mm_iommu_table_group_mem_t *mm_iommu_find(struct mm_struct *mm,
EXPORT_SYMBOL_GPL(mm_iommu_find);
long mm_iommu_ua_to_hpa(struct mm_iommu_table_group_mem_t *mem,
- unsigned long ua, unsigned long *hpa)
+ unsigned long ua, unsigned int pageshift, unsigned long *hpa)
{
const long entry = (ua - mem->ua) >> PAGE_SHIFT;
u64 *va = &mem->hpas[entry];
@@ -357,6 +365,9 @@ long mm_iommu_ua_to_hpa(struct mm_iommu_table_group_mem_t *mem,
if (entry >= mem->entries)
return -EFAULT;
+ if (pageshift > mem->pageshift)
+ return -EFAULT;
+
*hpa = *va | (ua & ~PAGE_MASK);
return 0;
@@ -364,7 +375,7 @@ long mm_iommu_ua_to_hpa(struct mm_iommu_table_group_mem_t *mem,
EXPORT_SYMBOL_GPL(mm_iommu_ua_to_hpa);
long mm_iommu_ua_to_hpa_rm(struct mm_iommu_table_group_mem_t *mem,
- unsigned long ua, unsigned long *hpa)
+ unsigned long ua, unsigned int pageshift, unsigned long *hpa)
{
const long entry = (ua - mem->ua) >> PAGE_SHIFT;
void *va = &mem->hpas[entry];
@@ -373,6 +384,9 @@ long mm_iommu_ua_to_hpa_rm(struct mm_iommu_table_group_mem_t *mem,
if (entry >= mem->entries)
return -EFAULT;
+ if (pageshift > mem->pageshift)
+ return -EFAULT;
+
pa = (void *) vmalloc_to_phys(va);
if (!pa)
return -EFAULT;
diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c b/drivers/vfio/vfio_iommu_spapr_tce.c
index 2da5f05..7cd63b0 100644
--- a/drivers/vfio/vfio_iommu_spapr_tce.c
+++ b/drivers/vfio/vfio_iommu_spapr_tce.c
@@ -467,7 +467,7 @@ static int tce_iommu_prereg_ua_to_hpa(struct tce_container *container,
if (!mem)
return -EINVAL;
- ret = mm_iommu_ua_to_hpa(mem, tce, phpa);
+ ret = mm_iommu_ua_to_hpa(mem, tce, shift, phpa);
if (ret)
return -EINVAL;
--
2.11.0
^ permalink raw reply related
* Re: [PATCH] powerpc/mm: remove warning about ‘type’ being set
From: Mathieu Malaterre @ 2018-06-26 6:25 UTC (permalink / raw)
To: a.p.zijlstra
Cc: Christophe LEROY, Michael Ellerman, Kate Stewart,
Greg Kroah-Hartman, LKML, Paul Mackerras, Philippe Ombredanne,
Thomas Gleixner, linuxppc-dev, Andrew Morton
In-Reply-To: <c182ca65-f41e-4a1c-95ee-6cb979a6e76b@c-s.fr>
On Sat, Jun 23, 2018 at 7:12 PM christophe leroy
<christophe.leroy@c-s.fr> wrote:
>
>
>
> Le 22/06/2018 =C3=A0 21:27, Mathieu Malaterre a =C3=A9crit :
> > =E2=80=98type=E2=80=99 is only used when CONFIG_DEBUG_HIGHMEM is set. S=
o add a possibly
> > unused tag to variable. Remove warning treated as error with W=3D1:
> >
> > arch/powerpc/mm/highmem.c:59:6: error: variable =E2=80=98type=E2=80=
=99 set but not used [-Werror=3Dunused-but-set-variable]
>
> Is type neeeded at all when CONFIG_DEBUG_HIGHMEM is not set ?
>
> The call type =3D kmap_atomic_idx(); seems useless when
> CONFIG_DEBUG_HIGHMEM isn't set. Couldn't we just most type definition
> and setting inside the CONFIG_DEBUG_HIGHMEM {} below ?
>
> Alternatively, maybe you could replace the #ifdef CONFIG_DEBUG_HIGHMEM
> by an if (IS_ENABLED(CONFIG_DEBUG_HIGHMEM)) ?
I am not familiar with this code. But starring at other arch
implementations (eg. `arch/x86/mm/highmem_32.c`), it feels like
powerpc is skipping `pte_clear(&init_mm, vaddr, kmap_pte-idx);` unless
`CONFIG_DEBUG_HIGHMEM=3Dy`. Could someone please confirm this is the
correct behavior ?
Or else I can rewrite the code a bit like `arch/arm/mm/highmem.c`
which skips `set_fixmap_pte(idx, __pte(0));` unless
`CONFIG_DEBUG_HIGHMEM=3Dy`.
> Christophe
>
> >
> > Signed-off-by: Mathieu Malaterre <malat@debian.org>
> > ---
> > arch/powerpc/mm/highmem.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/powerpc/mm/highmem.c b/arch/powerpc/mm/highmem.c
> > index 668e87d03f9e..82a0e37557a5 100644
> > --- a/arch/powerpc/mm/highmem.c
> > +++ b/arch/powerpc/mm/highmem.c
> > @@ -56,7 +56,7 @@ EXPORT_SYMBOL(kmap_atomic_prot);
> > void __kunmap_atomic(void *kvaddr)
> > {
> > unsigned long vaddr =3D (unsigned long) kvaddr & PAGE_MASK;
> > - int type;
> > + int type __maybe_unused;
> >
> > if (vaddr < __fix_to_virt(FIX_KMAP_END)) {
> > pagefault_enable();
> >
>
> ---
> L'absence de virus dans ce courrier =C3=A9lectronique a =C3=A9t=C3=A9 v=
=C3=A9rifi=C3=A9e par le logiciel antivirus Avast.
> https://www.avast.com/antivirus
>
^ permalink raw reply
* Re: [PATCH] powerpc/xmon: avoid warnings about variables that might be clobbered by ‘longjmp’
From: Mathieu Malaterre @ 2018-06-26 6:27 UTC (permalink / raw)
To: Segher Boessenkool
Cc: Christophe LEROY, Michael Ellerman, Yisheng Xie, Vaibhav Jain,
Nicholas Piggin, LKML, Paul Mackerras, Breno Leitao, linuxppc-dev
In-Reply-To: <20180623194716.GY16221@gate.crashing.org>
On Sat, Jun 23, 2018 at 9:47 PM Segher Boessenkool
<segher@kernel.crashing.org> wrote:
>
> On Sat, Jun 23, 2018 at 06:59:27PM +0200, christophe leroy wrote:
> >
> >
> > Le 22/06/2018 =C3=A0 21:27, Mathieu Malaterre a =C3=A9crit :
> > >Move initialization of variables after data definitions. This silence
> > >warnings treated as error with W=3D1:
> > >
> > > arch/powerpc/xmon/xmon.c:3389:14: error: variable =E2=80=98name=E2=
=80=99 might be
> > > clobbered by =E2=80=98longjmp=E2=80=99 or =E2=80=98vfork=E2=80=99 [=
-Werror=3Dclobbered]
> > > arch/powerpc/xmon/xmon.c:3100:22: error: variable =E2=80=98tsk=E2=
=80=99 might be
> > > clobbered by =E2=80=98longjmp=E2=80=99 or =E2=80=98vfork=E2=80=99 [=
-Werror=3Dclobbered]
> >
> > Is that an invalid warning ?
>
> No, both are correct warnings. GCC can not see which functions it only
> has a declaration of can call longjmp.
I assumed those were false positive warnings, given how easy it was to
defeat them. Let give it another try.
> > Otherwise, I'd expect one to fix the warning, not just cheat on GCC.
>
> Yes, the patch seems to change the code in such a way that some versions
> of GCC will no longer warn. Which does not make to code any more correct=
.
>
> Either restructure the code, or make the var non-automatic, or make it
> volatile.
>
>
> Segher
^ permalink raw reply
* [next-20180601][nvme][ppc] Kernel Oops is triggered when creating lvm snapshots on nvme disks
From: Abdul Haleem @ 2018-06-26 9:00 UTC (permalink / raw)
To: linuxppc-dev
Cc: linux-fsdevel, linux-next, linux-kernel, linux-scsi,
Stephen Rothwell, sachinp, sim, manvanth, mpe, Brian King
Greeting's
Kernel Oops is seen on 4.17.0-rc7-next-20180601 kernel on a bare-metal
machine when running lvm snapshot tests on nvme disks.
Machine Type: Power 8 bare-metal
kernel : 4.17.0-rc7-next-20180601
test:
$ pvcreate -y /dev/nvme0n1
$ vgcreate avocado_vg /dev/nvme0n1
$ lvcreate --size 1.4T --name avocado_lv avocado_vg -y
$ mkfs.ext2 /dev/avocado_vg/avocado_lv
$ lvcreate --size 1G --snapshot --name avocado_sn /dev/avocado_vg/avocado_lv -y
$ lvconvert --merge /dev/avocado_vg/avocado_sn
the last command results in Oops:
Unable to handle kernel paging request for data at address 0x000000d0
Faulting instruction address: 0xc0000000002dced4
Oops: Kernel access of bad area, sig: 11 [#1]
LE SMP NR_CPUS=2048 NUMA PowerNV
Dumping ftrace buffer:
(ftrace buffer empty)
Modules linked in: dm_snapshot dm_bufio nvme bnx2x iptable_mangle
ipt_MASQUERADE iptable_nat nf_nat_ipv4 nf_nat nf_conntrack_ipv4
nf_defrag_ipv4 xt_conntrack nf_conntrack ipt_REJECT nf_reject_ipv4
xt_tcpudp tun bridge stp llc iptable_filter dm_mirror dm_region_hash
dm_log dm_service_time vmx_crypto powernv_rng rng_core dm_multipath
kvm_hv binfmt_misc kvm nfsd ip_tables x_tables autofs4 xfs lpfc
crc_t10dif crct10dif_generic mdio nvme_fc libcrc32c nvme_fabrics
nvme_core crct10dif_common [last unloaded: nvme]
CPU: 70 PID: 157763 Comm: lvconvert Not tainted 4.17.0-rc7-next-20180601-autotest-autotest #1
NIP: c0000000002dced4 LR: c000000000244d14 CTR: c000000000244cf0
REGS: c000001f81d6b5a0 TRAP: 0300 Not tainted (4.17.0-rc7-next-20180601-autotest-autotest)
MSR: 900000010280b033 <SF,HV,VEC,VSX,EE,FP,ME,IR,DR,RI,LE,TM[E]> CR: 22442444 XER: 20000000
CFAR: c000000000008934 DAR: 00000000000000d0 DSISR: 40000000 SOFTE: 0
GPR00: c000000000244d14 c000001f81d6b820 c00000000109c400 c000003c9d080180
GPR04: 0000000000000001 c000001fad510000 c000001fad510000 0000000000000001
GPR08: 0000000000000000 f000000000000000 f000000000000008 0000000000000000
GPR12: c000000000244cf0 c000001ffffc4f80 00007fffa0e31090 00007fffd9d9b470
GPR16: 0000000000000000 000000000000005c 00007fffa0e3a5b0 00007fffa0e62040
GPR20: 0000010014ad7d50 0000010014ad7d20 00007fffa0e64210 0000000000000001
GPR24: 0000000000000000 c00000000081bae0 c000001ed2461b00 d00000000f859d08
GPR28: c000003c9d080180 c000000000244d14 0000000000000001 0000000000000000
NIP [c0000000002dced4] kmem_cache_free+0x1a4/0x2b0
LR [c000000000244d14] mempool_free_slab+0x24/0x40
Call Trace:
[c000001f81d6b820] [c0000000002dcfbc] kmem_cache_free+0x28c/0x2b0 (unreliable)
[c000001f81d6b8b0] [c000000000244d14] mempool_free_slab+0x24/0x40
[c000001f81d6b8d0] [c000000000244e10] mempool_exit+0x50/0x90
[c000001f81d6b900] [c00000000081d730] dm_io_client_destroy+0x20/0x50
[c000001f81d6b930] [c00000000081f1dc] dm_kcopyd_client_destroy+0x9c/0x140
[c000001f81d6b9a0] [d00000000f851da4] dm_exception_table_exit.isra.14+0x204/0xaa0 [dm_snapshot]
[c000001f81d6ba40] [c0000000008162d0] dm_table_destroy+0xa0/0x190
[c000001f81d6bad0] [c00000000081bc24] dev_suspend+0x144/0x330
[c000001f81d6bb10] [c00000000081c870] ctl_ioctl+0x350/0x4e0
[c000001f81d6bd00] [c00000000081ca18] dm_ctl_ioctl+0x18/0x30
[c000001f81d6bd20] [c000000000329b38] do_vfs_ioctl+0xc8/0x8b0
[c000001f81d6bdc0] [c00000000032a37c] ksys_ioctl+0x5c/0xe0
[c000001f81d6be10] [c00000000032a420] sys_ioctl+0x20/0x80
[c000001f81d6be30] [c00000000000b9e0] system_call+0x58/0x6c
Instruction dump:
39295e50 7bca8502 794a3664 e9290000 7d495214 7d495378 e94a0008 794807e1
40c2010c ebe90018 7fbcf840 419e00f8 <e93f00d0> 7fbc4800 419efe9c e8bc0058
---[ end trace d60580773711c361 ]---
essage from syslogd@localhost at Jun 4 08:34:20 ...
kernel:Dumping ftrace buffer:
[cache_from_obj: Wrong slab cache. ksm_rmap_item but object is from kmalloc-128
[WARNING: CPU: 0 PID: 157807 at mm/slab.h:381 kmem_cache_free+0x1d0/0x2b0
[Modules linked in: dm_snapshot dm_bufio nvme bnx2x iptable_mangle
ipt_MASQUERADE iptable_nat nf_nat_ipv4 nf_nat nf_conntrack_ipv4
nf_defrag_ipv4 xt_conntrack nf_conntrack ipt_REJECT nf_reject_ipv4
xt_tcpudp tun bridge stp llc iptable_filter dm_mirror dm_region_hash
dm_log dm_service_time vmx_crypto powernv_rng rng_core dm_multipath
kvm_hv binfmt_misc kvm nfsd ip_tables x_tables autofs4 xfs lpfc
crc_t10dif crct10dif_generic mdio nvme_fc libcrc32c nvme_fabrics
nvme_core crct10dif_common [last unloaded: nvme]
[CPU: 0 PID: 157807 Comm: vgremove Tainted: G D 4.17.0-rc7-next-20180601-autotest-autotest #1
[NIP: c0000000002dcf00 LR: c0000000002dcefc CTR: 0000000000000000
[REGS: c000001ee130f500 TRAP: 0700 Tainted: G D (4.17.0-rc7-next-20180601-autotest-autotest)
[MSR: 900000010282b033 <SF,HV,VEC,VSX,EE,FP,ME,IR,DR,RI,LE,TM[E]> CR: 22442422 XER: 20000000
[CFAR: c0000000001668f4 SOFTE: 0
[GPR00: c0000000002dcefc c000001ee130f780 c00000000109c400 000000000000004e
[GPR04: c000001ff5c0cdd0 c000001ff5c23a70 0000000000000001 ffffffffffffffff
[GPR08: 0000000000000000 c000001ff5c13880 0000001ff4e90000 9000000102803003
[GPR12: 0000000000002200 c000000001260000 00007fff7f2e1090 00007fffecda3550
[GPR16: 0000000000000000 000000000000005c 00007fff7f2ea5b0 00007fff7f312040
[GPR20: 0000010022679490 0000010022679460 00007fff7f314210 0000000000000003
[GPR24: 0000000000000000 c00000000081be10 c000001fad301500 d00000000f859d08
[GPR28: c000003c9d080180 c000000000244d14 c000001f63723908 c000001ff401f480
[NIP [c0000000002dcf00] kmem_cache_free+0x1d0/0x2b0
[LR [c0000000002dcefc] kmem_cache_free+0x1cc/0x2b0
[Call Trace:
[[c000001ee130f780] [c0000000002dcefc] kmem_cache_free+0x1cc/0x2b0 (unreliable)
[[c000001ee130f810] [c000000000244d14] mempool_free_slab+0x24/0x40
[[c000001ee130f830] [c000000000244e10] mempool_exit+0x50/0x90
[[c000001ee130f860] [c00000000081d730] dm_io_client_destroy+0x20/0x50
[[c000001ee130f890] [c00000000081f1dc] dm_kcopyd_client_destroy+0x9c/0x140
[[c000001ee130f900] [d00000000f851da4] dm_exception_table_exit.isra.14+0x204/0xaa0 [dm_snapshot]
[[c000001ee130f9a0] [c0000000008162d0] dm_table_destroy+0xa0/0x190
[[c000001ee130fa30] [c0000000008118e8] __dm_destroy+0x198/0x230
[[c000001ee130fac0] [c00000000081bf64] dev_remove+0x154/0x1d0
[[c000001ee130fb10] [c00000000081c870] ctl_ioctl+0x350/0x4e0
[[c000001ee130fd00] [c00000000081ca18] dm_ctl_ioctl+0x18/0x30
[[c000001ee130fd20] [c000000000329b38] do_vfs_ioctl+0xc8/0x8b0
[[c000001ee130fdc0] [c00000000032a37c] ksys_ioctl+0x5c/0xe0
[[c000001ee130fe10] [c00000000032a420] sys_ioctl+0x20/0x80
[[c000001ee130fe30] [c00000000000b9e0] system_call+0x58/0x6c
[419e00f8 e93f00d0 7fbc4800 419efe9c e8bc0058 e8df0058 3c62ffb5 3c82ff99
[3863f0a8 38846d18 4be899bd 60000000 <0fe00000> 7f9fe378 4bfffe70 60420000
[---[ end trace d60580773711c362 ]---
[cache_from_obj: Wrong slab cache. ksm_rmap_item but object is from kmalloc-128
--
Regard's
Abdul Haleem
IBM Linux Technology Centre
^ permalink raw reply
* Re: [PATCH 2/3] drivers/base: reorder consumer and its children behind suppliers
From: Dan Carpenter @ 2018-06-26 7:44 UTC (permalink / raw)
To: kbuild, Pingfan Liu
Cc: kbuild-all, linux-kernel, Pingfan Liu, Greg Kroah-Hartman,
Grygorii Strashko, Christoph Hellwig, Bjorn Helgaas, Dave Young,
linux-pci, linuxppc-dev
In-Reply-To: <1529904187-18673-3-git-send-email-kernelfans@gmail.com>
[ There is a bug with kbuild where it's not showing the Smatch warnings
but I can probably guess... - dan ]
Hi Pingfan,
Thank you for the patch! Perhaps something to improve:
url: https://github.com/0day-ci/linux/commits/Pingfan-Liu/drivers-base-bugfix-for-supplier-consumer-ordering-in-device_kset/20180625-132702
# https://github.com/0day-ci/linux/commit/1b2a1e63898baf80e8e830991284e1534bc54766
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 1b2a1e63898baf80e8e830991284e1534bc54766
vim +/ret +245 drivers/base/core.c
1b2a1e63 Pingfan Liu 2018-06-25 216
1b2a1e63 Pingfan Liu 2018-06-25 217 /* When reodering, take care of the range of (old_pos(dev), new_pos(dev)),
1b2a1e63 Pingfan Liu 2018-06-25 218 * there may be requirement to recursively move item.
1b2a1e63 Pingfan Liu 2018-06-25 219 */
1b2a1e63 Pingfan Liu 2018-06-25 220 int device_reorder_consumer(struct device *dev)
1b2a1e63 Pingfan Liu 2018-06-25 221 {
1b2a1e63 Pingfan Liu 2018-06-25 222 struct list_head *iter, *left, *right;
1b2a1e63 Pingfan Liu 2018-06-25 223 struct device *cur_dev;
1b2a1e63 Pingfan Liu 2018-06-25 224 struct pos_info info;
1b2a1e63 Pingfan Liu 2018-06-25 225 int ret, idx;
1b2a1e63 Pingfan Liu 2018-06-25 226
1b2a1e63 Pingfan Liu 2018-06-25 227 idx = device_links_read_lock();
1b2a1e63 Pingfan Liu 2018-06-25 228 if (list_empty(&dev->links.suppliers)) {
1b2a1e63 Pingfan Liu 2018-06-25 229 device_links_read_unlock(idx);
1b2a1e63 Pingfan Liu 2018-06-25 230 return 0;
1b2a1e63 Pingfan Liu 2018-06-25 231 }
1b2a1e63 Pingfan Liu 2018-06-25 232 spin_lock(&devices_kset->list_lock);
1b2a1e63 Pingfan Liu 2018-06-25 233 list_for_each_prev(iter, &devices_kset->list) {
1b2a1e63 Pingfan Liu 2018-06-25 234 cur_dev = list_entry(iter, struct device, kobj.entry);
1b2a1e63 Pingfan Liu 2018-06-25 235 ret = find_last_supplier(dev, cur_dev);
1b2a1e63 Pingfan Liu 2018-06-25 236 switch (ret) {
1b2a1e63 Pingfan Liu 2018-06-25 237 case -1:
1b2a1e63 Pingfan Liu 2018-06-25 238 goto unlock;
1b2a1e63 Pingfan Liu 2018-06-25 239 case 1:
1b2a1e63 Pingfan Liu 2018-06-25 240 break;
1b2a1e63 Pingfan Liu 2018-06-25 241 case 0:
1b2a1e63 Pingfan Liu 2018-06-25 242 continue;
The break breaks from the switch and the continue continues the loop so
they're equivalent. Perhaps you intended to break from the loop?
1b2a1e63 Pingfan Liu 2018-06-25 243 }
1b2a1e63 Pingfan Liu 2018-06-25 244 }
1b2a1e63 Pingfan Liu 2018-06-25 @245 BUG_ON(!ret);
If the list is empty then "ret" can be unitialized. We test a different
list "dev->links.suppliers" to see if that's empty. I wrote a bunch of
code to make Smatch try to understand about empty lists, but I don't
think it works...
1b2a1e63 Pingfan Liu 2018-06-25 246
1b2a1e63 Pingfan Liu 2018-06-25 247 /* record the affected open section */
1b2a1e63 Pingfan Liu 2018-06-25 248 left = dev->kobj.entry.prev;
1b2a1e63 Pingfan Liu 2018-06-25 249 right = iter;
1b2a1e63 Pingfan Liu 2018-06-25 250 info.pos = list_entry(iter, struct device, kobj.entry);
1b2a1e63 Pingfan Liu 2018-06-25 251 info.tail = NULL;
1b2a1e63 Pingfan Liu 2018-06-25 252 /* dry out the consumers in (left,right) */
1b2a1e63 Pingfan Liu 2018-06-25 253 __device_reorder_consumer(dev, left, right, &info);
1b2a1e63 Pingfan Liu 2018-06-25 254
1b2a1e63 Pingfan Liu 2018-06-25 255 unlock:
1b2a1e63 Pingfan Liu 2018-06-25 256 spin_unlock(&devices_kset->list_lock);
1b2a1e63 Pingfan Liu 2018-06-25 257 device_links_read_unlock(idx);
1b2a1e63 Pingfan Liu 2018-06-25 258 return 0;
1b2a1e63 Pingfan Liu 2018-06-25 259 }
1b2a1e63 Pingfan Liu 2018-06-25 260
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
^ permalink raw reply
* [PATCH 0/9] Fix references for some missing documentation files
From: Mauro Carvalho Chehab @ 2018-06-26 9:49 UTC (permalink / raw)
To: Linux Doc Mailing List
Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab, linux-kernel,
Jonathan Corbet, Jacek Anaszewski, devicetree, Ingo Molnar,
linux-kernel, Andrew Morton, linux-leds, intel-wired-lan,
Mark Rutland, linux-gpio, David S. Miller, James Morris,
Jeff Kirsher, Changbin Du, Masami Hiramatsu, netdev,
Steven Rostedt, linux-input, linuxppc-dev, linux-scsi, kvm,
virtualization, Andy Whitcroft, Joe Perches
Having nothing to do while waiting for my plane to arrive while
returning back from Japan, I ended by writing a small series of
patches meant to reduce the number of bad Documentation/*
links that are detected by:
./scripts/documentation-file-ref-check
I ended by rebasing this patch series against linux-next, because
of those two patches:
3b0c3ebe2a42 Documentation: e100: Fix docs build error
805f16a5f12f Documentation: e1000: Fix docs build error
They basically fix documentation builds with upstream Kernel. Both
got merged on -rc2.
The first two patches in this series makes the script to ignore some
false positives.
Patches 3 to 6 corrects the location of some documentation files.
Patches 7 and 8 were actually two patches meant to fix the build
error. I ended by rebasing them over linux-next, as they fix some
troubles with the ReST syntax with causes warnings.
Patch 9 converts Documentation/trace/histogram.txt to ReST
syntax. It also had to be rebased against linux-next, due to some minor
conflicts with:
064f35a95224 ("tracing: Fix some errors in histogram documentation")
After this series, the script still produces 16 warnings:
Documentation/devicetree/bindings/input/mtk-pmic-keys.txt: Documentation/devicetree/bindings/input/keys.txt
Documentation/devicetree/bindings/input/mtk-pmic-keys.txt: Documentation/devicetree/bindings/input/keys.txt
Documentation/devicetree/bindings/regulator/rohm,bd71837-regulator.txt: Documentation/devicetree/bindings/mfd/rohm,bd71837-pmic.txt
Documentation/devicetree/dynamic-resolution-notes.txt: Documentation/devicetree/dt-object-internal.txt
Documentation/scsi/scsi_mid_low_api.txt: Documentation/Configure.help
Documentation/translations/zh_CN/HOWTO: Documentation/DocBook/
Documentation/translations/zh_CN/basic_profiling.txt: Documentation/basic_profiling
Documentation/translations/zh_CN/basic_profiling.txt: Documentation/basic_profiling
MAINTAINERS: Documentation/fpga/
MAINTAINERS: Documentation/devicetree/bindings/rng/samsung,exynos5250-trng.txt
arch/powerpc/Kconfig: Documentation/vm/protection-keys.rst
drivers/isdn/mISDN/dsp_core.c: Documentation/isdn/mISDN.cert
drivers/scsi/Kconfig: file:Documentation/scsi/tmscsim.txt
drivers/vhost/vhost.c: Documentation/virtual/lguest/lguest.c
include/linux/fs_context.h: Documentation/filesystems/mounting.txt
include/linux/lsm_hooks.h: Documentation/filesystems/mounting.txt
IMHO, the above should be fixed by the corresponding maintainers.
The ones that scarry me most are the DT binding documentation, as
the binding documentation for some stuff are likely broken.
Btw, two of the above are new on linux-next (include/linux/fs_context.h
and include/linux/lsm_hooks.h) . That makes me wander that we should
likely add some logic (or run the detect script) at checkpatch.pl or make
it to call ./scripts/documentation-file-ref-check.
Mauro Carvalho Chehab (9):
scripts/documentation-file-ref-check: remove some false positives
scripts/documentation-file-ref-check: ignore sched-pelt false positive
docs: zh_CN: fix location of oops-tracing.txt
devicectree: bindings: fix location of leds common file
MAINTAINERS: fix location of ina2xx.txt device tree file
gpio.h: fix location of gpio legacy documentation
networking: e100.rst: Get rid of Sphinx warnings
networking: e1000.rst: Get rid of Sphinx warnings
docs: histogram.txt: convert it to ReST file format
.../devicetree/bindings/leds/common.txt | 2 +-
Documentation/networking/e100.rst | 27 +-
Documentation/networking/e1000.rst | 187 ++-
Documentation/trace/events.rst | 2 +-
.../trace/{histogram.txt => histogram.rst} | 1242 +++++++++--------
Documentation/trace/index.rst | 1 +
.../translations/zh_CN/oops-tracing.txt | 4 +-
MAINTAINERS | 2 +-
include/linux/gpio.h | 2 +-
kernel/trace/Kconfig | 2 +-
scripts/documentation-file-ref-check | 6 +
11 files changed, 767 insertions(+), 710 deletions(-)
rename Documentation/trace/{histogram.txt => histogram.rst} (73%)
--
2.17.1
^ permalink raw reply
* Re: [PATCHv2 2/2] drivers/base: reorder consumer and its children behind suppliers
From: Greg Kroah-Hartman @ 2018-06-26 11:54 UTC (permalink / raw)
To: Pingfan Liu
Cc: linux-kernel, Grygorii Strashko, Christoph Hellwig, Bjorn Helgaas,
Dave Young, linux-pci, linuxppc-dev
In-Reply-To: <CAFgQCTt6MTwXbbcQCcJroM3wGdx+5c7A+eVy+dXJscu1JtfA-w@mail.gmail.com>
On Tue, Jun 26, 2018 at 11:29:48AM +0800, Pingfan Liu wrote:
> On Mon, Jun 25, 2018 at 6:45 PM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Mon, Jun 25, 2018 at 03:47:39PM +0800, Pingfan Liu wrote:
> > > commit 52cdbdd49853 ("driver core: correct device's shutdown order")
> > > introduces supplier<-consumer order in devices_kset. The commit tries
> > > to cleverly maintain both parent<-child and supplier<-consumer order by
> > > reordering a device when probing. This method makes things simple and
> > > clean, but unfortunately, breaks parent<-child order in some case,
> > > which is described in next patch in this series.
> >
> > There is no "next patch in this series" :(
> >
> Oh, re-arrange the patches, and forget the comment in log
>
> > > Here this patch tries to resolve supplier<-consumer by only reordering a
> > > device when it has suppliers, and takes care of the following scenario:
> > > [consumer, children] [ ... potential ... ] supplier
> > > ^ ^
> > > After moving the consumer and its children after the supplier, the
> > > potentail section may contain consumers whose supplier is inside
> > > children, and this poses the requirement to dry out all consumpers in
> > > the section recursively.
> > >
> > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > Cc: Grygorii Strashko <grygorii.strashko@ti.com>
> > > Cc: Christoph Hellwig <hch@infradead.org>
> > > Cc: Bjorn Helgaas <helgaas@kernel.org>
> > > Cc: Dave Young <dyoung@redhat.com>
> > > Cc: linux-pci@vger.kernel.org
> > > Cc: linuxppc-dev@lists.ozlabs.org
> > > Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
> > > ---
> > > note: there is lock issue in this patch, should be fixed in next version
> >
> > Please send patches that you know are correct, why would I want to
> > review this if you know it is not correct?
> >
> > And if the original commit is causing problems for you, why not just
> > revert that instead of adding this much-increased complexity?
> >
> Revert the original commit, then it will expose the error order
> "consumer <- supplier" again.
> This patch tries to resolve the error and fix the following scenario:
> step0: before the consumer device's probing, (note child_a is a
> supplier of consumer_a, etc)
> [ consumer-X, child_a, ...., child_z] [.... consumer_a, ...,
> consumer_z, ....] supplier-X
> ^^^
> affected range during moving^^^
> step1: When probing, moving consumer-X after supplier-X
> [ child_a, ...., child_z] [.... consumer_a, ..., consumer_z,
> ....] supplier-X, consumer-X
> But it breaks "parent <-child" seq now, and should be fixed like:
> step2:
> [.... consumer_a, ..., consumer_z, ....] supplier-X [
> consumer-X, child_a, ...., child_z] <---
> descendants_reorder_after_pos() does it.
> Again, the seq "consumer_a <- child_a" breaks the "supplier<-consumer"
> order, should be fixed like:
> step3:
> [.... consumer_z, .....] supplier-X [ consumer-X, child_a,
> consumer_a ...., child_z] <--- __device_reorder_consumer() does it.
> ^^ affected range^^
> The moving of consumer_a brings us to face the same scenario of step1,
> hence we need an external recursion.
Something really got messed up here, and this all does not make any
sense :(
Can you try again?
Also, please cc: Rafael on all of this, as he wrote all of this
consumer/supplier logic and I am not that familiar with it at all.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v2] selftests/powerpc: Fix strncpy usage
From: Breno Leitao @ 2018-06-26 13:13 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev; +Cc: Anshuman Khandual
In-Reply-To: <87lgb286iq.fsf@concordia.ellerman.id.au>
On 06/26/2018 02:24 AM, Michael Ellerman wrote:
> Breno Leitao <leitao@debian.org> writes:
>
>> There is a buffer overflow in dscr_inherit_test.c test. In main(), strncpy()'s
>> third argument is the length of the source, not the size of the destination
>> buffer, which makes strncpy() behaves like strcpy(), causing a buffer overflow
>> if argv[0] is bigger than LEN_MAX (100).
>>
>> This patch allocates 'prog' according to the argv[0] length, avoiding LEN_MAX
>> restriction.
>>
>> CC: Segher Boessenkool <segher@kernel.crashing.org>
>> CC: Anshuman Khandual <khandual@linux.vnet.ibm.com>
>> Signed-off-by: Breno Leitao <leitao@debian.org>
>> ---
>> tools/testing/selftests/powerpc/dscr/dscr_inherit_exec_test.c | 11 +++++++++--
>> 1 file changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/testing/selftests/powerpc/dscr/dscr_inherit_exec_test.c b/tools/testing/selftests/powerpc/dscr/dscr_inherit_exec_test.c
>> index 08a8b95e3bc1..ecac4900c7dd 100644
>> --- a/tools/testing/selftests/powerpc/dscr/dscr_inherit_exec_test.c
>> +++ b/tools/testing/selftests/powerpc/dscr/dscr_inherit_exec_test.c
>> @@ -19,7 +19,7 @@
>> */
>> #include "dscr.h"
>>
>> -static char prog[LEN_MAX];
>> +static char *prog;
>>
>> static void do_exec(unsigned long parent_dscr)
>> {
>> @@ -104,6 +104,13 @@ int main(int argc, char *argv[])
>> exit(1);
>> }
>>
>> - strncpy(prog, argv[0], strlen(argv[0]));
>> + prog = malloc(strlen(argv[0]) + 1);
>> + if (prog == NULL) {
>> + fprintf(stderr, "Unable to allocate enough memory\n");
>> + exit(1);
>> + }
>> +
>> + strcpy(prog, argv[0]);
>
> Why do we need to copy it at all?
We do not. Pointing proj to argv[0], as you proposed, should be the best
solution for this problem.
Thanks!
^ permalink raw reply
* [PATCH v3 1/2] selftests/powerpc: Fix strncpy usage
From: Breno Leitao @ 2018-06-26 13:20 UTC (permalink / raw)
To: linuxppc-dev
Cc: Breno Leitao, Michael Ellerman, Segher Boessenkool,
Anshuman Khandual
In-Reply-To: <1529535071-14555-1-git-send-email-leitao@debian.org>
There is a buffer overflow in dscr_inherit_test.c test. In main(), strncpy()'s
third argument is the length of the source, not the size of the destination
buffer, which makes strncpy() behaves like strcpy(), causing a buffer overflow
if argv[0] is bigger than LEN_MAX (100).
This patch maps 'prog' to the argv[0] memory region, removing the static
allocation and the LEN_MAX size restriction.
CC: Michael Ellerman <mpe@ellerman.id.au>
CC: Segher Boessenkool <segher@kernel.crashing.org>
CC: Anshuman Khandual <khandual@linux.vnet.ibm.com>
Signed-off-by: Breno Leitao <leitao@debian.org>
---
tools/testing/selftests/powerpc/dscr/dscr_inherit_exec_test.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/powerpc/dscr/dscr_inherit_exec_test.c b/tools/testing/selftests/powerpc/dscr/dscr_inherit_exec_test.c
index 08a8b95e3bc1..55c55f39b6a6 100644
--- a/tools/testing/selftests/powerpc/dscr/dscr_inherit_exec_test.c
+++ b/tools/testing/selftests/powerpc/dscr/dscr_inherit_exec_test.c
@@ -19,7 +19,7 @@
*/
#include "dscr.h"
-static char prog[LEN_MAX];
+static char *prog;
static void do_exec(unsigned long parent_dscr)
{
@@ -104,6 +104,6 @@ int main(int argc, char *argv[])
exit(1);
}
- strncpy(prog, argv[0], strlen(argv[0]));
+ prog = argv[0];
return test_harness(dscr_inherit_exec, "dscr_inherit_exec_test");
}
--
2.16.3
^ permalink raw reply related
* [PATCH v3 2/2] selftests/powerpc: Fix typos
From: Breno Leitao @ 2018-06-26 13:20 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Breno Leitao, Gustavo Romero
In-Reply-To: <1530019213-2347-1-git-send-email-leitao@debian.org>
Fix two typos in the file header. Replacing the word 'priviledged'
by 'privileged' and 'exuecuted' by 'executed'.
Signed-off-by: Breno Leitao <leitao@debian.org>
Signed-off-by: Gustavo Romero <gromero@linux.vnet.ibm.com>
---
tools/testing/selftests/powerpc/dscr/dscr_inherit_exec_test.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/powerpc/dscr/dscr_inherit_exec_test.c b/tools/testing/selftests/powerpc/dscr/dscr_inherit_exec_test.c
index 55c55f39b6a6..c8c240accc0c 100644
--- a/tools/testing/selftests/powerpc/dscr/dscr_inherit_exec_test.c
+++ b/tools/testing/selftests/powerpc/dscr/dscr_inherit_exec_test.c
@@ -5,8 +5,8 @@
* verifies that the child is using the changed DSCR using mfspr.
*
* When using the privilege state SPR, the instructions such as
- * mfspr or mtspr are priviledged and the kernel emulates them
- * for us. Instructions using problem state SPR can be exuecuted
+ * mfspr or mtspr are privileged and the kernel emulates them
+ * for us. Instructions using problem state SPR can be executed
* directly without any emulation if the HW supports them. Else
* they also get emulated by the kernel.
*
--
2.16.3
^ permalink raw reply related
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