Re: [PATCH V3 2/2] tools/perf/tests: Fix object code reading to skip address that falls out of text section
> On 15-Sep-2023, at 10:56 AM, Adrian Hunter wrote: > > On 15/09/23 08:24, Athira Rajeev wrote: >> The testcase "Object code reading" fails in somecases >> for "fs_something" sub test as below: >> >>Reading object code for memory address: 0xc00807f0142c >>File is: /lib/modules/6.5.0-rc3+/kernel/fs/xfs/xfs.ko >>On file address is: 0x1114cc >>Objdump command is: objdump -z -d --start-address=0x11142c >> --stop-address=0x1114ac /lib/modules/6.5.0-rc3+/kernel/fs/xfs/xfs.ko >>objdump read too few bytes: 128 >>test child finished with -1 >> >> This can alo be reproduced when running perf record with >> workload that exercises fs_something() code. In the test >> setup, this is exercising xfs code since root is xfs. >> >># perf record ./a.out >># perf report -v |grep "xfs.ko" >> 0.76% a.out /lib/modules/6.5.0-rc3+/kernel/fs/xfs/xfs.ko >> 0xc00807de5efc B [k] xlog_cil_commit >> 0.74% a.out /lib/modules/6.5.0-rc3+/kernel/fs/xfs/xfs.ko >> 0xc00807d5ae18 B [k] xfs_btree_key_offset >> 0.74% a.out /lib/modules/6.5.0-rc3+/kernel/fs/xfs/xfs.ko >> 0xc00807e11fd4 B [k] 0x00112074 >> >> Here addr "0xc00807e11fd4" is not resolved. since this is a >> kernel module, its offset is from the DSO. Xfs module is loaded >> at 0xc00807d0 >> >> # cat /proc/modules | grep xfs >>xfs 2228224 3 - Live 0xc00807d0 >> >> And size is 0x22. So its loaded between 0xc00807d0 >> and 0xc00807f2. From objdump, text section is: >>text 0010f7bc 00a0 2**4 >> >> Hence perf captured ip maps to 0x112074 which is: >> ( ip - start of module ) + a0 >> >> This offset 0x112074 falls out .text section which is up to 0x10f7bc >> In this case for module, the address 0xc00807e11fd4 is pointing >> to stub instructions. This address range represents the module stubs >> which is allocated on module load and hence is not part of DSO offset. >> >> To address this issue in "object code reading", skip the sample if >> address falls out of text section and is within the module end. >> Use the "text_end" member of "struct dso" to do this check. >> >> To address this issue in "perf report", exploring an option of >> having stubs range as part of the /proc/kallsyms, so that perf >> report can resolve addresses in stubs range >> >> However this patch uses text_end to skip the stub range for >> Object code reading testcase. >> >> Reported-by: Disha Goel >> Signed-off-by: Athira Rajeev >> Tested-by: Disha Goel >> Reviewed-by: Adrian Hunter >> --- >> Changelog: >> v2 -> v3: >> Used strtailcmp in comparison for module check and added Reviewed-by >> from Adrian, Tested-by from Disha. >> >> v1 -> v2: >> Updated comment to add description on which arch has stub and >> reason for skipping as suggested by Adrian >> >> tools/perf/tests/code-reading.c | 10 ++ >> 1 file changed, 10 insertions(+) >> >> diff --git a/tools/perf/tests/code-reading.c >> b/tools/perf/tests/code-reading.c >> index ed3815163d1b..45334d26058e 100644 >> --- a/tools/perf/tests/code-reading.c >> +++ b/tools/perf/tests/code-reading.c >> @@ -269,6 +269,16 @@ static int read_object_code(u64 addr, size_t len, u8 >> cpumode, >> if (addr + len > map__end(al.map)) >> len = map__end(al.map) - addr; >> >> + /* >> + * Some architectures (ex: powerpc) have stubs (trampolines) in kernel >> + * modules to manage long jumps. Check if the ip offset falls in stubs >> + * sections for kernel modules. And skip module address after text end >> + */ >> + if (!strtailcmp(dso->long_name, ".ko") && al.addr > dso->text_end) { >> + pr_debug("skipping the module address %#"PRIx64" after text end\n", >> al.addr); >> + goto out; > > Double indent My bad, addressed in V4 Athira > >> + } >> + >> /* Read the object code using perf */ >> ret_len = dso__data_read_offset(dso, maps__machine(thread__maps(thread)), >> al.addr, buf1, len);
[PATCH V4 1/2] tools/perf: Add text_end to "struct dso" to save .text section size
Update "struct dso" to include new member "text_end". This new field will represent the offset for end of text section for a dso. For elf, this value is derived as: sh_size (Size of section in byes) + sh_offset (Section file offst) of the elf header for text. For bfd, this value is derived as: 1. For PE file, section->size + ( section->vma - dso->text_offset) 2. Other cases: section->filepos (file position) + section->size (size of section) To resolve the address from a sample, perf looks at the DSO maps. In case of address from a kernel module, there were some address found to be not resolved. This was observed while running perf test for "Object code reading". Though the ip falls beteen the start address of the loaded module (perf map->start ) and end address ( perf map->end), it was unresolved. Example: Reading object code for memory address: 0xc00807f0142c File is: /lib/modules/6.5.0-rc3+/kernel/fs/xfs/xfs.ko On file address is: 0x1114cc Objdump command is: objdump -z -d --start-address=0x11142c --stop-address=0x1114ac /lib/modules/6.5.0-rc3+/kernel/fs/xfs/xfs.ko objdump read too few bytes: 128 test child finished with -1 Here, module is loaded at: # cat /proc/modules | grep xfs xfs 2228224 3 - Live 0xc00807d0 >From objdump for xfs module, text section is: text 0010f7bc 00a0 2**4 Here the offset for 0xc00807f0142c ie 0x112074 falls out .text section which is up to 0x10f7bc. In this case for module, the address 0xc00807e11fd4 is pointing to stub instructions. This address range represents the module stubs which is allocated on module load and hence is not part of DSO offset. To identify such address, which falls out of text section and within module end, added the new field "text_end" to "struct dso". Reported-by: Disha Goel Signed-off-by: Athira Rajeev Reviewed-by: Adrian Hunter --- Changelog: v2 -> v3: Added Reviewed-by from Adrian v1 -> v2: Added text_end for bfd also by updating dso__load_bfd_symbols as suggested by Adrian. tools/perf/util/dso.h| 1 + tools/perf/util/symbol-elf.c | 4 +++- tools/perf/util/symbol.c | 2 ++ 3 files changed, 6 insertions(+), 1 deletion(-) diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h index b41c9782c754..70fe0fe69bef 100644 --- a/tools/perf/util/dso.h +++ b/tools/perf/util/dso.h @@ -181,6 +181,7 @@ struct dso { u8 rel; struct build_id bid; u64 text_offset; + u64 text_end; const char *short_name; const char *long_name; u16 long_name_len; diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c index 95e99c332d7e..9e7eeaf616b8 100644 --- a/tools/perf/util/symbol-elf.c +++ b/tools/perf/util/symbol-elf.c @@ -1514,8 +1514,10 @@ dso__load_sym_internal(struct dso *dso, struct map *map, struct symsrc *syms_ss, } if (elf_section_by_name(runtime_ss->elf, _ss->ehdr, , - ".text", NULL)) + ".text", NULL)) { dso->text_offset = tshdr.sh_addr - tshdr.sh_offset; + dso->text_end = tshdr.sh_offset + tshdr.sh_size; + } if (runtime_ss->opdsec) opddata = elf_rawdata(runtime_ss->opdsec, NULL); diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c index 3f36675b7c8f..f25e4e62cf25 100644 --- a/tools/perf/util/symbol.c +++ b/tools/perf/util/symbol.c @@ -1733,8 +1733,10 @@ int dso__load_bfd_symbols(struct dso *dso, const char *debugfile) /* PE symbols can only have 4 bytes, so use .text high bits */ dso->text_offset = section->vma - (u32)section->vma; dso->text_offset += (u32)bfd_asymbol_value(symbols[i]); + dso->text_end = (section->vma - dso->text_offset) + section->size; } else { dso->text_offset = section->vma - section->filepos; + dso->text_end = section->filepos + section->size; } } -- 2.31.1
[PATCH V4 2/2] tools/perf/tests: Fix object code reading to skip address that falls out of text section
The testcase "Object code reading" fails in somecases for "fs_something" sub test as below: Reading object code for memory address: 0xc00807f0142c File is: /lib/modules/6.5.0-rc3+/kernel/fs/xfs/xfs.ko On file address is: 0x1114cc Objdump command is: objdump -z -d --start-address=0x11142c --stop-address=0x1114ac /lib/modules/6.5.0-rc3+/kernel/fs/xfs/xfs.ko objdump read too few bytes: 128 test child finished with -1 This can alo be reproduced when running perf record with workload that exercises fs_something() code. In the test setup, this is exercising xfs code since root is xfs. # perf record ./a.out # perf report -v |grep "xfs.ko" 0.76% a.out /lib/modules/6.5.0-rc3+/kernel/fs/xfs/xfs.ko 0xc00807de5efc B [k] xlog_cil_commit 0.74% a.out /lib/modules/6.5.0-rc3+/kernel/fs/xfs/xfs.ko 0xc00807d5ae18 B [k] xfs_btree_key_offset 0.74% a.out /lib/modules/6.5.0-rc3+/kernel/fs/xfs/xfs.ko 0xc00807e11fd4 B [k] 0x00112074 Here addr "0xc00807e11fd4" is not resolved. since this is a kernel module, its offset is from the DSO. Xfs module is loaded at 0xc00807d0 # cat /proc/modules | grep xfs xfs 2228224 3 - Live 0xc00807d0 And size is 0x22. So its loaded between 0xc00807d0 and 0xc00807f2. From objdump, text section is: text 0010f7bc 00a0 2**4 Hence perf captured ip maps to 0x112074 which is: ( ip - start of module ) + a0 This offset 0x112074 falls out .text section which is up to 0x10f7bc In this case for module, the address 0xc00807e11fd4 is pointing to stub instructions. This address range represents the module stubs which is allocated on module load and hence is not part of DSO offset. To address this issue in "object code reading", skip the sample if address falls out of text section and is within the module end. Use the "text_end" member of "struct dso" to do this check. To address this issue in "perf report", exploring an option of having stubs range as part of the /proc/kallsyms, so that perf report can resolve addresses in stubs range However this patch uses text_end to skip the stub range for Object code reading testcase. Reported-by: Disha Goel Signed-off-by: Athira Rajeev Tested-by: Disha Goel Reviewed-by: Adrian Hunter --- Changelog: v3 -> v4: Fixed indent in V3 v2 -> v3: Used strtailcmp in comparison for module check and added Reviewed-by from Adrian, Tested-by from Disha. v1 -> v2: Updated comment to add description on which arch has stub and reason for skipping as suggested by Adrian tools/perf/tests/code-reading.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/tools/perf/tests/code-reading.c b/tools/perf/tests/code-reading.c index ed3815163d1b..9e6e6c985840 100644 --- a/tools/perf/tests/code-reading.c +++ b/tools/perf/tests/code-reading.c @@ -269,6 +269,16 @@ static int read_object_code(u64 addr, size_t len, u8 cpumode, if (addr + len > map__end(al.map)) len = map__end(al.map) - addr; + /* +* Some architectures (ex: powerpc) have stubs (trampolines) in kernel +* modules to manage long jumps. Check if the ip offset falls in stubs +* sections for kernel modules. And skip module address after text end +*/ + if (!strtailcmp(dso->long_name, ".ko") && al.addr > dso->text_end) { + pr_debug("skipping the module address %#"PRIx64" after text end\n", al.addr); + goto out; + } + /* Read the object code using perf */ ret_len = dso__data_read_offset(dso, maps__machine(thread__maps(thread)), al.addr, buf1, len); -- 2.31.1
Re: [PATCH] hwmon: (ibmpowernv) refactor deprecated strncpy
On 9/14/23 22:24, Kees Cook wrote: On Thu, Sep 14, 2023 at 11:21:06PM +, Justin Stitt wrote: `strncpy` is deprecated for use on NUL-terminated destination strings [1]. We should prefer more robust and less ambiguous string interfaces. A suitable replacement is `strscpy` [2] due to the fact that it guarantees NUL-termination on the destination buffer without unnecessarily NUL-padding since `buf` is already zero-initialized. Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1] Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2] Link: https://github.com/KSPP/linux/issues/90 Cc: linux-harden...@vger.kernel.org Signed-off-by: Justin Stitt --- drivers/hwmon/ibmpowernv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/hwmon/ibmpowernv.c b/drivers/hwmon/ibmpowernv.c index 594254d6a72d..57d829dbcda6 100644 --- a/drivers/hwmon/ibmpowernv.c +++ b/drivers/hwmon/ibmpowernv.c @@ -234,7 +234,7 @@ static int get_sensor_index_attr(const char *name, u32 *index, char *attr) if (copy_len >= sizeof(buf)) return -EINVAL; - strncpy(buf, hash_pos + 1, copy_len); + strscpy(buf, hash_pos + 1, copy_len); This is another case of precise byte copying -- this just needs to be memcpy. Otherwise this truncates the trailing character. Imagine a name input of "fan#2-data". "buf" wants to get "2". copy_len is 1, and strscpy would eat it. :) It is really sad that the submitters of such "cleanup" patches can't be bothered to check what they are doing. They can't even be bothered to write a coccinelle script that would avoid pitfalls like this one, and they expect others to do their homework for them. And then people wonder why there is maintainer burnout. I am so tired of that. Guenter
Re: [PATCH V3 2/2] tools/perf/tests: Fix object code reading to skip address that falls out of text section
On 15/09/23 08:24, Athira Rajeev wrote: > The testcase "Object code reading" fails in somecases > for "fs_something" sub test as below: > > Reading object code for memory address: 0xc00807f0142c > File is: /lib/modules/6.5.0-rc3+/kernel/fs/xfs/xfs.ko > On file address is: 0x1114cc > Objdump command is: objdump -z -d --start-address=0x11142c > --stop-address=0x1114ac /lib/modules/6.5.0-rc3+/kernel/fs/xfs/xfs.ko > objdump read too few bytes: 128 > test child finished with -1 > > This can alo be reproduced when running perf record with > workload that exercises fs_something() code. In the test > setup, this is exercising xfs code since root is xfs. > > # perf record ./a.out > # perf report -v |grep "xfs.ko" > 0.76% a.out /lib/modules/6.5.0-rc3+/kernel/fs/xfs/xfs.ko > 0xc00807de5efc B [k] xlog_cil_commit > 0.74% a.out /lib/modules/6.5.0-rc3+/kernel/fs/xfs/xfs.ko > 0xc00807d5ae18 B [k] xfs_btree_key_offset > 0.74% a.out /lib/modules/6.5.0-rc3+/kernel/fs/xfs/xfs.ko > 0xc00807e11fd4 B [k] 0x00112074 > > Here addr "0xc00807e11fd4" is not resolved. since this is a > kernel module, its offset is from the DSO. Xfs module is loaded > at 0xc00807d0 > ># cat /proc/modules | grep xfs > xfs 2228224 3 - Live 0xc00807d0 > > And size is 0x22. So its loaded between 0xc00807d0 > and 0xc00807f2. From objdump, text section is: > text 0010f7bc 00a0 2**4 > > Hence perf captured ip maps to 0x112074 which is: > ( ip - start of module ) + a0 > > This offset 0x112074 falls out .text section which is up to 0x10f7bc > In this case for module, the address 0xc00807e11fd4 is pointing > to stub instructions. This address range represents the module stubs > which is allocated on module load and hence is not part of DSO offset. > > To address this issue in "object code reading", skip the sample if > address falls out of text section and is within the module end. > Use the "text_end" member of "struct dso" to do this check. > > To address this issue in "perf report", exploring an option of > having stubs range as part of the /proc/kallsyms, so that perf > report can resolve addresses in stubs range > > However this patch uses text_end to skip the stub range for > Object code reading testcase. > > Reported-by: Disha Goel > Signed-off-by: Athira Rajeev > Tested-by: Disha Goel > Reviewed-by: Adrian Hunter > --- > Changelog: > v2 -> v3: > Used strtailcmp in comparison for module check and added Reviewed-by > from Adrian, Tested-by from Disha. > > v1 -> v2: > Updated comment to add description on which arch has stub and > reason for skipping as suggested by Adrian > > tools/perf/tests/code-reading.c | 10 ++ > 1 file changed, 10 insertions(+) > > diff --git a/tools/perf/tests/code-reading.c b/tools/perf/tests/code-reading.c > index ed3815163d1b..45334d26058e 100644 > --- a/tools/perf/tests/code-reading.c > +++ b/tools/perf/tests/code-reading.c > @@ -269,6 +269,16 @@ static int read_object_code(u64 addr, size_t len, u8 > cpumode, > if (addr + len > map__end(al.map)) > len = map__end(al.map) - addr; > > + /* > + * Some architectures (ex: powerpc) have stubs (trampolines) in kernel > + * modules to manage long jumps. Check if the ip offset falls in stubs > + * sections for kernel modules. And skip module address after text end > + */ > + if (!strtailcmp(dso->long_name, ".ko") && al.addr > dso->text_end) { > + pr_debug("skipping the module address %#"PRIx64" after > text end\n", al.addr); > + goto out; Double indent > + } > + > /* Read the object code using perf */ > ret_len = dso__data_read_offset(dso, > maps__machine(thread__maps(thread)), > al.addr, buf1, len);
Re: [PATCH] hwmon: (ibmpowernv) refactor deprecated strncpy
On Thu, Sep 14, 2023 at 11:21:06PM +, Justin Stitt wrote: > `strncpy` is deprecated for use on NUL-terminated destination strings [1]. > > We should prefer more robust and less ambiguous string interfaces. > > A suitable replacement is `strscpy` [2] due to the fact that it > guarantees NUL-termination on the destination buffer without > unnecessarily NUL-padding since `buf` is already zero-initialized. > > Link: > https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings > [1] > Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html > [2] > Link: https://github.com/KSPP/linux/issues/90 > Cc: linux-harden...@vger.kernel.org > Signed-off-by: Justin Stitt > --- > drivers/hwmon/ibmpowernv.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/hwmon/ibmpowernv.c b/drivers/hwmon/ibmpowernv.c > index 594254d6a72d..57d829dbcda6 100644 > --- a/drivers/hwmon/ibmpowernv.c > +++ b/drivers/hwmon/ibmpowernv.c > @@ -234,7 +234,7 @@ static int get_sensor_index_attr(const char *name, u32 > *index, char *attr) > if (copy_len >= sizeof(buf)) > return -EINVAL; > > - strncpy(buf, hash_pos + 1, copy_len); > + strscpy(buf, hash_pos + 1, copy_len); This is another case of precise byte copying -- this just needs to be memcpy. Otherwise this truncates the trailing character. Imagine a name input of "fan#2-data". "buf" wants to get "2". copy_len is 1, and strscpy would eat it. :) -Kees > > err = kstrtou32(buf, 10, index); > if (err) > > --- > base-commit: 3669558bdf354cd352be955ef2764cde6a9bf5ec > change-id: 20230914-strncpy-drivers-hwmon-ibmpowernv-c-80a03f16d93a > > Best regards, > -- > Justin Stitt > -- Kees Cook
[PATCH V3 2/2] tools/perf/tests: Fix object code reading to skip address that falls out of text section
The testcase "Object code reading" fails in somecases for "fs_something" sub test as below: Reading object code for memory address: 0xc00807f0142c File is: /lib/modules/6.5.0-rc3+/kernel/fs/xfs/xfs.ko On file address is: 0x1114cc Objdump command is: objdump -z -d --start-address=0x11142c --stop-address=0x1114ac /lib/modules/6.5.0-rc3+/kernel/fs/xfs/xfs.ko objdump read too few bytes: 128 test child finished with -1 This can alo be reproduced when running perf record with workload that exercises fs_something() code. In the test setup, this is exercising xfs code since root is xfs. # perf record ./a.out # perf report -v |grep "xfs.ko" 0.76% a.out /lib/modules/6.5.0-rc3+/kernel/fs/xfs/xfs.ko 0xc00807de5efc B [k] xlog_cil_commit 0.74% a.out /lib/modules/6.5.0-rc3+/kernel/fs/xfs/xfs.ko 0xc00807d5ae18 B [k] xfs_btree_key_offset 0.74% a.out /lib/modules/6.5.0-rc3+/kernel/fs/xfs/xfs.ko 0xc00807e11fd4 B [k] 0x00112074 Here addr "0xc00807e11fd4" is not resolved. since this is a kernel module, its offset is from the DSO. Xfs module is loaded at 0xc00807d0 # cat /proc/modules | grep xfs xfs 2228224 3 - Live 0xc00807d0 And size is 0x22. So its loaded between 0xc00807d0 and 0xc00807f2. From objdump, text section is: text 0010f7bc 00a0 2**4 Hence perf captured ip maps to 0x112074 which is: ( ip - start of module ) + a0 This offset 0x112074 falls out .text section which is up to 0x10f7bc In this case for module, the address 0xc00807e11fd4 is pointing to stub instructions. This address range represents the module stubs which is allocated on module load and hence is not part of DSO offset. To address this issue in "object code reading", skip the sample if address falls out of text section and is within the module end. Use the "text_end" member of "struct dso" to do this check. To address this issue in "perf report", exploring an option of having stubs range as part of the /proc/kallsyms, so that perf report can resolve addresses in stubs range However this patch uses text_end to skip the stub range for Object code reading testcase. Reported-by: Disha Goel Signed-off-by: Athira Rajeev Tested-by: Disha Goel Reviewed-by: Adrian Hunter --- Changelog: v2 -> v3: Used strtailcmp in comparison for module check and added Reviewed-by from Adrian, Tested-by from Disha. v1 -> v2: Updated comment to add description on which arch has stub and reason for skipping as suggested by Adrian tools/perf/tests/code-reading.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/tools/perf/tests/code-reading.c b/tools/perf/tests/code-reading.c index ed3815163d1b..45334d26058e 100644 --- a/tools/perf/tests/code-reading.c +++ b/tools/perf/tests/code-reading.c @@ -269,6 +269,16 @@ static int read_object_code(u64 addr, size_t len, u8 cpumode, if (addr + len > map__end(al.map)) len = map__end(al.map) - addr; + /* +* Some architectures (ex: powerpc) have stubs (trampolines) in kernel +* modules to manage long jumps. Check if the ip offset falls in stubs +* sections for kernel modules. And skip module address after text end +*/ + if (!strtailcmp(dso->long_name, ".ko") && al.addr > dso->text_end) { + pr_debug("skipping the module address %#"PRIx64" after text end\n", al.addr); + goto out; + } + /* Read the object code using perf */ ret_len = dso__data_read_offset(dso, maps__machine(thread__maps(thread)), al.addr, buf1, len); -- 2.31.1
[PATCH V3 1/2] tools/perf: Add text_end to "struct dso" to save .text section size
Update "struct dso" to include new member "text_end". This new field will represent the offset for end of text section for a dso. For elf, this value is derived as: sh_size (Size of section in byes) + sh_offset (Section file offst) of the elf header for text. For bfd, this value is derived as: 1. For PE file, section->size + ( section->vma - dso->text_offset) 2. Other cases: section->filepos (file position) + section->size (size of section) To resolve the address from a sample, perf looks at the DSO maps. In case of address from a kernel module, there were some address found to be not resolved. This was observed while running perf test for "Object code reading". Though the ip falls beteen the start address of the loaded module (perf map->start ) and end address ( perf map->end), it was unresolved. Example: Reading object code for memory address: 0xc00807f0142c File is: /lib/modules/6.5.0-rc3+/kernel/fs/xfs/xfs.ko On file address is: 0x1114cc Objdump command is: objdump -z -d --start-address=0x11142c --stop-address=0x1114ac /lib/modules/6.5.0-rc3+/kernel/fs/xfs/xfs.ko objdump read too few bytes: 128 test child finished with -1 Here, module is loaded at: # cat /proc/modules | grep xfs xfs 2228224 3 - Live 0xc00807d0 >From objdump for xfs module, text section is: text 0010f7bc 00a0 2**4 Here the offset for 0xc00807f0142c ie 0x112074 falls out .text section which is up to 0x10f7bc. In this case for module, the address 0xc00807e11fd4 is pointing to stub instructions. This address range represents the module stubs which is allocated on module load and hence is not part of DSO offset. To identify such address, which falls out of text section and within module end, added the new field "text_end" to "struct dso". Reported-by: Disha Goel Signed-off-by: Athira Rajeev Reviewed-by: Adrian Hunter --- Changelog: v2 -> v3: Added Reviewed-by from Adrian v1 -> v2: Added text_end for bfd also by updating dso__load_bfd_symbols as suggested by Adrian. tools/perf/util/dso.h| 1 + tools/perf/util/symbol-elf.c | 4 +++- tools/perf/util/symbol.c | 2 ++ 3 files changed, 6 insertions(+), 1 deletion(-) diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h index b41c9782c754..70fe0fe69bef 100644 --- a/tools/perf/util/dso.h +++ b/tools/perf/util/dso.h @@ -181,6 +181,7 @@ struct dso { u8 rel; struct build_id bid; u64 text_offset; + u64 text_end; const char *short_name; const char *long_name; u16 long_name_len; diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c index 95e99c332d7e..9e7eeaf616b8 100644 --- a/tools/perf/util/symbol-elf.c +++ b/tools/perf/util/symbol-elf.c @@ -1514,8 +1514,10 @@ dso__load_sym_internal(struct dso *dso, struct map *map, struct symsrc *syms_ss, } if (elf_section_by_name(runtime_ss->elf, _ss->ehdr, , - ".text", NULL)) + ".text", NULL)) { dso->text_offset = tshdr.sh_addr - tshdr.sh_offset; + dso->text_end = tshdr.sh_offset + tshdr.sh_size; + } if (runtime_ss->opdsec) opddata = elf_rawdata(runtime_ss->opdsec, NULL); diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c index 3f36675b7c8f..f25e4e62cf25 100644 --- a/tools/perf/util/symbol.c +++ b/tools/perf/util/symbol.c @@ -1733,8 +1733,10 @@ int dso__load_bfd_symbols(struct dso *dso, const char *debugfile) /* PE symbols can only have 4 bytes, so use .text high bits */ dso->text_offset = section->vma - (u32)section->vma; dso->text_offset += (u32)bfd_asymbol_value(symbols[i]); + dso->text_end = (section->vma - dso->text_offset) + section->size; } else { dso->text_offset = section->vma - section->filepos; + dso->text_end = section->filepos + section->size; } } -- 2.31.1
Re: [V2 2/2] tools/perf/tests: Fix object code reading to skip address that falls out of text section
> On 14-Sep-2023, at 11:54 PM, Adrian Hunter wrote: > > On 7/09/23 19:45, Athira Rajeev wrote: >> The testcase "Object code reading" fails in somecases >> for "fs_something" sub test as below: >> >>Reading object code for memory address: 0xc00807f0142c >>File is: /lib/modules/6.5.0-rc3+/kernel/fs/xfs/xfs.ko >>On file address is: 0x1114cc >>Objdump command is: objdump -z -d --start-address=0x11142c >> --stop-address=0x1114ac /lib/modules/6.5.0-rc3+/kernel/fs/xfs/xfs.ko >>objdump read too few bytes: 128 >>test child finished with -1 >> >> This can alo be reproduced when running perf record with >> workload that exercises fs_something() code. In the test >> setup, this is exercising xfs code since root is xfs. >> >># perf record ./a.out >># perf report -v |grep "xfs.ko" >> 0.76% a.out /lib/modules/6.5.0-rc3+/kernel/fs/xfs/xfs.ko >> 0xc00807de5efc B [k] xlog_cil_commit >> 0.74% a.out /lib/modules/6.5.0-rc3+/kernel/fs/xfs/xfs.ko >> 0xc00807d5ae18 B [k] xfs_btree_key_offset >> 0.74% a.out /lib/modules/6.5.0-rc3+/kernel/fs/xfs/xfs.ko >> 0xc00807e11fd4 B [k] 0x00112074 >> >> Here addr "0xc00807e11fd4" is not resolved. since this is a >> kernel module, its offset is from the DSO. Xfs module is loaded >> at 0xc00807d0 >> >> # cat /proc/modules | grep xfs >>xfs 2228224 3 - Live 0xc00807d0 >> >> And size is 0x22. So its loaded between 0xc00807d0 >> and 0xc00807f2. From objdump, text section is: >>text 0010f7bc 00a0 2**4 >> >> Hence perf captured ip maps to 0x112074 which is: >> ( ip - start of module ) + a0 >> >> This offset 0x112074 falls out .text section which is up to 0x10f7bc >> In this case for module, the address 0xc00807e11fd4 is pointing >> to stub instructions. This address range represents the module stubs >> which is allocated on module load and hence is not part of DSO offset. >> >> To address this issue in "object code reading", skip the sample if >> address falls out of text section and is within the module end. >> Use the "text_end" member of "struct dso" to do this check. >> >> To address this issue in "perf report", exploring an option of >> having stubs range as part of the /proc/kallsyms, so that perf >> report can resolve addresses in stubs range >> >> However this patch uses text_end to skip the stub range for >> Object code reading testcase. >> >> Reported-by: Disha Goel >> Signed-off-by: Athira Rajeev >> --- >> Changelog: >> v1 -> v2: >> Updated comment to add description on which arch has stub and >> reason for skipping as suggested by Adrian >> >> tools/perf/tests/code-reading.c | 12 >> 1 file changed, 12 insertions(+) >> >> diff --git a/tools/perf/tests/code-reading.c >> b/tools/perf/tests/code-reading.c >> index ed3815163d1b..3cf6c2d42416 100644 >> --- a/tools/perf/tests/code-reading.c >> +++ b/tools/perf/tests/code-reading.c >> @@ -269,6 +269,18 @@ static int read_object_code(u64 addr, size_t len, u8 >> cpumode, >> if (addr + len > map__end(al.map)) >> len = map__end(al.map) - addr; >> >> + /* >> + * Some architectures (ex: powerpc) have stubs (trampolines) in kernel >> + * modules to manage long jumps. Check if the ip offset falls in stubs >> + * sections for kernel modules. And skip module address after text end >> + */ >> + if (strstr(dso->long_name, ".ko")) { > > Sorry for slow reply > > !strtailcmp() is slightly better here > >> + if (al.addr > dso->text_end) { > > We normally avoid nesting if-statements e.g. > > if (!strtailcmp(dso->long_name, ".ko") && al.addr > dso->text_end) > > Make those changes and you can add: > > Reviewed-by: Adrian Hunter Sure, will post a V3 with this change Athira > > >> + pr_debug("skipping the module address %#"PRIx64" after text end\n", >> al.addr); >> + goto out; >> + } >> + } >> + >> /* Read the object code using perf */ >> ret_len = dso__data_read_offset(dso, maps__machine(thread__maps(thread)), >> al.addr, buf1, len);
Re: [V2 1/2] tools/perf: Add text_end to "struct dso" to save .text section size
> On 14-Sep-2023, at 11:49 PM, Adrian Hunter wrote: > > On 7/09/23 19:45, Athira Rajeev wrote: >> Update "struct dso" to include new member "text_end". >> This new field will represent the offset for end of text >> section for a dso. For elf, this value is derived as: >> sh_size (Size of section in byes) + sh_offset (Section file >> offst) of the elf header for text. >> >> For bfd, this value is derived as: >> 1. For PE file, >> section->size + ( section->vma - dso->text_offset) >> 2. Other cases: >> section->filepos (file position) + section->size (size of >> section) >> >> To resolve the address from a sample, perf looks at the >> DSO maps. In case of address from a kernel module, there >> were some address found to be not resolved. This was >> observed while running perf test for "Object code reading". >> Though the ip falls beteen the start address of the loaded >> module (perf map->start ) and end address ( perf map->end), >> it was unresolved. >> >> Example: >> >>Reading object code for memory address: 0xc00807f0142c >>File is: /lib/modules/6.5.0-rc3+/kernel/fs/xfs/xfs.ko >>On file address is: 0x1114cc >>Objdump command is: objdump -z -d --start-address=0x11142c >> --stop-address=0x1114ac /lib/modules/6.5.0-rc3+/kernel/fs/xfs/xfs.ko >>objdump read too few bytes: 128 >>test child finished with -1 >> >> Here, module is loaded at: >># cat /proc/modules | grep xfs >>xfs 2228224 3 - Live 0xc00807d0 >> >> From objdump for xfs module, text section is: >>text 0010f7bc 00a0 2**4 >> >> Here the offset for 0xc00807f0142c ie 0x112074 falls out >> .text section which is up to 0x10f7bc. >> >> In this case for module, the address 0xc00807e11fd4 is pointing >> to stub instructions. This address range represents the module stubs >> which is allocated on module load and hence is not part of DSO offset. >> >> To identify such address, which falls out of text >> section and within module end, added the new field "text_end" to >> "struct dso". >> >> Reported-by: Disha Goel >> Signed-off-by: Athira Rajeev > > Reviewed-by: Adrian Hunter Hi Adrian Thanks for the review > >> --- >> Changelog: >> v1 -> v2: >> Added text_end for bfd also by updating dso__load_bfd_symbols >> as suggested by Adrian. >> >> tools/perf/util/dso.h| 1 + >> tools/perf/util/symbol-elf.c | 4 +++- >> tools/perf/util/symbol.c | 2 ++ >> 3 files changed, 6 insertions(+), 1 deletion(-) >> >> diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h >> index b41c9782c754..70fe0fe69bef 100644 >> --- a/tools/perf/util/dso.h >> +++ b/tools/perf/util/dso.h >> @@ -181,6 +181,7 @@ struct dso { >> u8 rel; >> struct build_id bid; >> u64 text_offset; >> + u64 text_end; >> const char *short_name; >> const char *long_name; >> u16 long_name_len; >> diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c >> index 95e99c332d7e..9e7eeaf616b8 100644 >> --- a/tools/perf/util/symbol-elf.c >> +++ b/tools/perf/util/symbol-elf.c >> @@ -1514,8 +1514,10 @@ dso__load_sym_internal(struct dso *dso, struct map >> *map, struct symsrc *syms_ss, >> } >> >> if (elf_section_by_name(runtime_ss->elf, _ss->ehdr, , >> - ".text", NULL)) >> + ".text", NULL)) { >> dso->text_offset = tshdr.sh_addr - tshdr.sh_offset; >> + dso->text_end = tshdr.sh_offset + tshdr.sh_size; >> + } >> >> if (runtime_ss->opdsec) >> opddata = elf_rawdata(runtime_ss->opdsec, NULL); >> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c >> index 3f36675b7c8f..f25e4e62cf25 100644 >> --- a/tools/perf/util/symbol.c >> +++ b/tools/perf/util/symbol.c >> @@ -1733,8 +1733,10 @@ int dso__load_bfd_symbols(struct dso *dso, const char >> *debugfile) >> /* PE symbols can only have 4 bytes, so use .text high bits */ >> dso->text_offset = section->vma - (u32)section->vma; >> dso->text_offset += (u32)bfd_asymbol_value(symbols[i]); >> + dso->text_end = (section->vma - dso->text_offset) + section->size; >> } else { >> dso->text_offset = section->vma - section->filepos; >> + dso->text_end = section->filepos + section->size; >> } >> }
Re: [PATCH] powerpc/82xx: Select FSL_SOC
Le 15/09/2023 à 02:43, Michael Ellerman a écrit : > Christophe Leroy writes: >> It used to be impossible to select CONFIG_CPM2 without selecting >> CONFIG_FSL_SOC at the same time because CONFIG_CPM2 was dependent >> on CONFIG_8260 and CONFIG_8260 was selecting CONFIG_FSL_SOC. >> >> But after commit eb5aa2137275 ("powerpc/82xx: Remove CONFIG_8260 >> and CONFIG_8272") CONFIG_CPM2 depends on CONFIG_MPC82xx instead > ^ > CONFIG_PPC_82xx > > All the references to CONFIG_MPC82xx should be CONFIG_PPC_82xx right? > I can update when applying. Ah right, I mixed things up. This is CONFIG_PPC_82xx, CONFIG_PPC_8xx, CONFIG_PPC_83xx and CONFIG_PPC_MPC512x > > cheers > > >> but CONFIG_MPC82xx doesn't directly selects CONFIG_FSL_SOC. >> >> Fix it by forcing CONFIG_MPC82xx to select CONFIG_FSL_SOC just >> like already done by MPC8xx, MPC512x, MPC83xx, PPC_86xx. >> >> Reported-by: Randy Dunlap >> Fixes: eb5aa2137275 ("powerpc/82xx: Remove CONFIG_8260 and CONFIG_8272") >> Signed-off-by: Christophe Leroy >> --- >> arch/powerpc/platforms/82xx/Kconfig | 3 +-- >> 1 file changed, 1 insertion(+), 2 deletions(-) >> >> diff --git a/arch/powerpc/platforms/82xx/Kconfig >> b/arch/powerpc/platforms/82xx/Kconfig >> index d9f1a2a83158..1824536cf6f2 100644 >> --- a/arch/powerpc/platforms/82xx/Kconfig >> +++ b/arch/powerpc/platforms/82xx/Kconfig >> @@ -2,6 +2,7 @@ >> menuconfig PPC_82xx >> bool "82xx-based boards (PQ II)" >> depends on PPC_BOOK3S_32 >> +select FSL_SOC >> >> if PPC_82xx >> >> @@ -9,7 +10,6 @@ config EP8248E >> bool "Embedded Planet EP8248E (a.k.a. CWH-PPC-8248N-VE)" >> select CPM2 >> select PPC_INDIRECT_PCI if PCI >> -select FSL_SOC >> select PHYLIB if NETDEVICES >> select MDIO_BITBANG if PHYLIB >> help >> @@ -22,7 +22,6 @@ config MGCOGE >> bool "Keymile MGCOGE" >> select CPM2 >> select PPC_INDIRECT_PCI if PCI >> -select FSL_SOC >> help >>This enables support for the Keymile MGCOGE board. >> >> -- >> 2.41.0
[PATCH v2] powerpc/dexcr: Move HASHCHK trap handler
Syzkaller reported a sleep in atomic context bug relating to the HASHCHK handler logic BUG: sleeping function called from invalid context at arch/powerpc/kernel/traps.c:1518 in_atomic(): 0, irqs_disabled(): 1, non_block: 0, pid: 25040, name: syz-executor preempt_count: 0, expected: 0 RCU nest depth: 0, expected: 0 no locks held by syz-executor/25040. irq event stamp: 34 hardirqs last enabled at (33): [] prep_irq_for_enabled_exit arch/powerpc/kernel/interrupt.c:56 [inline] hardirqs last enabled at (33): [] interrupt_exit_user_prepare_main+0x148/0x600 arch/powerpc/kernel/interrupt.c:230 hardirqs last disabled at (34): [] interrupt_enter_prepare+0x144/0x4f0 arch/powerpc/include/asm/interrupt.h:176 softirqs last enabled at (0): [] copy_process+0x16e4/0x4750 kernel/fork.c:2436 softirqs last disabled at (0): [<>] 0x0 CPU: 15 PID: 25040 Comm: syz-executor Not tainted 6.5.0-rc5-1-g3ccdff6bb06d #3 Hardware name: IBM,9105-22A POWER10 (raw) 0x800200 0xf06 of:IBM,FW1040.00 (NL1040_021) hv:phyp pSeries Call Trace: [c000a8247ce0] [c032b0e4] __might_resched+0x3b4/0x400 kernel/sched/core.c:10189 [c000a8247d80] [c08c7dc8] __might_fault+0xa8/0x170 mm/memory.c:5853 [c000a8247dc0] [c004160c] do_program_check+0x32c/0xb20 arch/powerpc/kernel/traps.c:1518 [c000a8247e50] [c0009b2c] program_check_common_virt+0x3bc/0x3c0 To determine if a trap was caused by a HASHCHK instruction, we inspect the user instruction that triggered the trap. However this may sleep if the page needs to be faulted in (get_user_instr() reaches __get_user(), which calls might_fault() and triggers the bug message). Move the HASHCHK handler logic to after we allow IRQs, which is fine because we are only interested in HASHCHK if it's a user space trap. Fixes: 5bcba4e6c13f ("powerpc/dexcr: Handle hashchk exception") Signed-off-by: Benjamin Gray --- v1: https://lore.kernel.org/all/20230825014910.488822-1-bg...@linux.ibm.com/ v1 -> v2: Changed commit description to mention Syzkaller and bug output --- arch/powerpc/kernel/traps.c | 56 - 1 file changed, 36 insertions(+), 20 deletions(-) diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c index eeff136b83d9..64ff37721fd0 100644 --- a/arch/powerpc/kernel/traps.c +++ b/arch/powerpc/kernel/traps.c @@ -1512,23 +1512,11 @@ static void do_program_check(struct pt_regs *regs) return; } - if (cpu_has_feature(CPU_FTR_DEXCR_NPHIE) && user_mode(regs)) { - ppc_inst_t insn; - - if (get_user_instr(insn, (void __user *)regs->nip)) { - _exception(SIGSEGV, regs, SEGV_MAPERR, regs->nip); - return; - } - - if (ppc_inst_primary_opcode(insn) == 31 && - get_xop(ppc_inst_val(insn)) == OP_31_XOP_HASHCHK) { - _exception(SIGILL, regs, ILL_ILLOPN, regs->nip); - return; - } + /* User mode considers other cases after enabling IRQs */ + if (!user_mode(regs)) { + _exception(SIGTRAP, regs, TRAP_BRKPT, regs->nip); + return; } - - _exception(SIGTRAP, regs, TRAP_BRKPT, regs->nip); - return; } #ifdef CONFIG_PPC_TRANSACTIONAL_MEM if (reason & REASON_TM) { @@ -1561,16 +1549,44 @@ static void do_program_check(struct pt_regs *regs) /* * If we took the program check in the kernel skip down to sending a -* SIGILL. The subsequent cases all relate to emulating instructions -* which we should only do for userspace. We also do not want to enable -* interrupts for kernel faults because that might lead to further -* faults, and loose the context of the original exception. +* SIGILL. The subsequent cases all relate to user space, such as +* emulating instructions which we should only do for user space. We +* also do not want to enable interrupts for kernel faults because that +* might lead to further faults, and loose the context of the original +* exception. */ if (!user_mode(regs)) goto sigill; interrupt_cond_local_irq_enable(regs); + /* +* (reason & REASON_TRAP) is mostly handled before enabling IRQs, +* except get_user_instr() can sleep so we cannot reliably inspect the +* current instruction in that context. Now that we know we are +* handling a user space trap and can sleep, we can check if the trap +* was a hashchk failure. +*/ + if (reason & REASON_TRAP) { + if (cpu_has_feature(CPU_FTR_DEXCR_NPHIE)) { +
[PATCH v2 1/1] powerpc: fix a memory leak
When one of the methods xive_native_alloc_irq_on_chip, irq_create_mapping or irq_get_handler_data fails, the function will directly return without disposing vinst->name and vinst. Fix it. Fixes: c20e1e299d93 ("powerpc/vas: Alloc and setup IRQ and trigger port address") Signed-off-by: Yuanjun Gong --- arch/powerpc/platforms/powernv/vas.c | 12 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/arch/powerpc/platforms/powernv/vas.c b/arch/powerpc/platforms/powernv/vas.c index b65256a63e87..40cb7a03d180 100644 --- a/arch/powerpc/platforms/powernv/vas.c +++ b/arch/powerpc/platforms/powernv/vas.c @@ -102,6 +102,7 @@ static int init_vas_instance(struct platform_device *pdev) res = >resource[3]; if (res->end > 62) { pr_err("Bad 'paste_win_id_shift' in DT, %llx\n", res->end); + rc = -ENODEV goto free_vinst; } @@ -111,21 +112,24 @@ static int init_vas_instance(struct platform_device *pdev) if (!hwirq) { pr_err("Inst%d: Unable to allocate global irq for chip %d\n", vinst->vas_id, chipid); - return -ENOENT; + rc = -ENOENT; + goto free_vinst; } vinst->virq = irq_create_mapping(NULL, hwirq); if (!vinst->virq) { pr_err("Inst%d: Unable to map global irq %d\n", vinst->vas_id, hwirq); - return -EINVAL; + rc = -EINVAL; + goto free_vinst; } xd = irq_get_handler_data(vinst->virq); if (!xd) { pr_err("Inst%d: Invalid virq %d\n", vinst->vas_id, vinst->virq); - return -EINVAL; + rc = -EINVAL; + goto free_vinst; } vinst->irq_port = xd->trig_page; @@ -168,7 +172,7 @@ static int init_vas_instance(struct platform_device *pdev) free_vinst: kfree(vinst->name); kfree(vinst); - return -ENODEV; + return rc; } -- 2.37.2
Re: [PATCH] powerpc: add `cur_cpu_spec` symbol to vmcoreinfo
Sachin Sant writes: >> On 14-Sep-2023, at 6:52 PM, Michael Ellerman wrote: >> >> Sachin Sant writes: On 11-Sep-2023, at 2:44 PM, Aditya Gupta wrote: Presently, while reading a vmcore, makedumpfile uses `cur_cpu_spec.mmu_features` to decide whether the crashed system had RADIX MMU or not. Currently, makedumpfile fails to get the `cur_cpu_spec` symbol (unless a vmlinux is passed with the `-x` flag to makedumpfile), and hence assigns offsets and shifts (such as pgd_offset_l4) incorrecly considering MMU to be hash MMU. Add `cur_cpu_spec` symbol and offset of `mmu_features` in the `cpu_spec` struct, to VMCOREINFO, so that the symbol address and offset is accessible to makedumpfile, without needing the vmlinux file Signed-off-by: Aditya Gupta --- >>> >>> Thanks for the patch. With this patch applied (along with makedumpfile >>> changes) >>> I am able to capture vmcore against a kernel which contains commit >>> 8dc9a0ad0c3e >> >> I can't find that commit? Was just wondering if it should be referenced >> in the commit message. >> > > My bad, I copied that commit id from the email when I first reported this > issue > against linux-next. > > The commit should be > 368a0590d954: (powerpc/book3s64/vmemmap: switch radix to use a > different vmemmap handling function) OK thanks. Aditya, can you please rephrase the commit message to mention how that commit broke the previous behaviour. Also I don't know what pgd_offset_l4 is? cheers
Re: [PATCH 1/1] powerpc: fix a memory leak
Tyrel Datwyler writes: > On 9/14/23 02:46, Yuanjun Gong wrote: >> When one of the methods xive_native_alloc_irq_on_chip, irq_create_mapping >> or irq_get_handler_data fails, the function will directly return without >> disposing vinst->name and vinst. Fix it. >> >> Fixes: c20e1e299d93 ("powerpc/vas: Alloc and setup IRQ and trigger port >> address") >> Signed-off-by: Yuanjun Gong >> --- >> arch/powerpc/platforms/powernv/vas.c | 14 +- >> 1 file changed, 9 insertions(+), 5 deletions(-) >> >> diff --git a/arch/powerpc/platforms/powernv/vas.c >> b/arch/powerpc/platforms/powernv/vas.c >> index b65256a63e87..780740b478f0 100644 >> --- a/arch/powerpc/platforms/powernv/vas.c >> +++ b/arch/powerpc/platforms/powernv/vas.c >> @@ -54,7 +54,7 @@ static int init_vas_instance(struct platform_device *pdev) >> struct xive_irq_data *xd; >> uint32_t chipid, hwirq; >> struct resource *res; >> -int rc, cpu, vasid; >> +int rc, cpu, vasid, ret; > > You can you reuse rc for the return value in the error path instead of > introducing a new ret variable. Yep, please send a v2. cheers
Re: [PATCH] powerpc/82xx: Select FSL_SOC
Christophe Leroy writes: > It used to be impossible to select CONFIG_CPM2 without selecting > CONFIG_FSL_SOC at the same time because CONFIG_CPM2 was dependent > on CONFIG_8260 and CONFIG_8260 was selecting CONFIG_FSL_SOC. > > But after commit eb5aa2137275 ("powerpc/82xx: Remove CONFIG_8260 > and CONFIG_8272") CONFIG_CPM2 depends on CONFIG_MPC82xx instead ^ CONFIG_PPC_82xx All the references to CONFIG_MPC82xx should be CONFIG_PPC_82xx right? I can update when applying. cheers > but CONFIG_MPC82xx doesn't directly selects CONFIG_FSL_SOC. > > Fix it by forcing CONFIG_MPC82xx to select CONFIG_FSL_SOC just > like already done by MPC8xx, MPC512x, MPC83xx, PPC_86xx. > > Reported-by: Randy Dunlap > Fixes: eb5aa2137275 ("powerpc/82xx: Remove CONFIG_8260 and CONFIG_8272") > Signed-off-by: Christophe Leroy > --- > arch/powerpc/platforms/82xx/Kconfig | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/arch/powerpc/platforms/82xx/Kconfig > b/arch/powerpc/platforms/82xx/Kconfig > index d9f1a2a83158..1824536cf6f2 100644 > --- a/arch/powerpc/platforms/82xx/Kconfig > +++ b/arch/powerpc/platforms/82xx/Kconfig > @@ -2,6 +2,7 @@ > menuconfig PPC_82xx > bool "82xx-based boards (PQ II)" > depends on PPC_BOOK3S_32 > + select FSL_SOC > > if PPC_82xx > > @@ -9,7 +10,6 @@ config EP8248E > bool "Embedded Planet EP8248E (a.k.a. CWH-PPC-8248N-VE)" > select CPM2 > select PPC_INDIRECT_PCI if PCI > - select FSL_SOC > select PHYLIB if NETDEVICES > select MDIO_BITBANG if PHYLIB > help > @@ -22,7 +22,6 @@ config MGCOGE > bool "Keymile MGCOGE" > select CPM2 > select PPC_INDIRECT_PCI if PCI > - select FSL_SOC > help > This enables support for the Keymile MGCOGE board. > > -- > 2.41.0
Re: [RFC PATCH v11 12/29] KVM: Add KVM_CREATE_GUEST_MEMFD ioctl() for guest-specific backing memory
On Thu, Sep 14, 2023, Ackerley Tng wrote: > Sean Christopherson writes: > > > On Mon, Aug 28, 2023, Ackerley Tng wrote: > >> Sean Christopherson writes: > >> >> If we track struct kvm with the inode, then I think (a), (b) and (c) can > >> >> be independent of the refcounting method. What do you think? > >> > > >> > No go. Because again, the inode (physical memory) is coupled to the > >> > virtual machine > >> > as a thing, not to a "struct kvm". Or more concretely, the inode is > >> > coupled to an > >> > ASID or an HKID, and there can be multiple "struct kvm" objects > >> > associated with a > >> > single ASID. And at some point in the future, I suspect we'll have > >> > multiple KVM > >> > objects per HKID too. > >> > > >> > The current SEV use case is for the migration helper, where two KVM > >> > objects share > >> > a single ASID (the "real" VM and the helper). I suspect TDX will end up > >> > with > >> > similar behavior where helper "VMs" can use the HKID of the "real" VM. > >> > For KVM, > >> > that means multiple struct kvm objects being associated with a single > >> > HKID. > >> > > >> > To prevent use-after-free, KVM "just" needs to ensure the helper > >> > instances can't > >> > outlive the real instance, i.e. can't use the HKID/ASID after the owning > >> > virtual > >> > machine has been destroyed. > >> > > >> > To put it differently, "struct kvm" is a KVM software construct that > >> > _usually_, > >> > but not always, is associated 1:1 with a virtual machine. > >> > > >> > And FWIW, stashing the pointer without holding a reference would not be > >> > a complete > >> > solution, because it couldn't guard against KVM reusing a pointer. E.g. > >> > if a > >> > struct kvm was unbound and then freed, KVM could reuse the same memory > >> > for a new > >> > struct kvm, with a different ASID/HKID, and get a false negative on the > >> > rebinding > >> > check. > >> > >> I agree that inode (physical memory) is coupled to the virtual machine > >> as a more generic concept. > >> > >> I was hoping that in the absence of CC hardware providing a HKID/ASID, > >> the struct kvm pointer could act as a representation of the "virtual > >> machine". You're definitely right that KVM could reuse a pointer and so > >> that idea doesn't stand. > >> > >> I thought about generating UUIDs to represent "virtual machines" in the > >> absence of CC hardware, and this UUID could be transferred during > >> intra-host migration, but this still doesn't take host userspace out of > >> the TCB. A malicious host VMM could just use the migration ioctl to copy > >> the UUID to a malicious dumper VM, which would then pass checks with a > >> gmem file linked to the malicious dumper VM. This is fine for HKID/ASIDs > >> because the memory is encrypted; with UUIDs there's no memory > >> encryption. > > > > I don't understand what problem you're trying to solve. I don't see a need > > to > > provide a single concrete representation/definition of a "virtual machine". > > E.g. > > there's no need for a formal definition to securely perform intrahost > > migration, > > KVM just needs to ensure that the migration doesn't compromise guest > > security, > > functionality, etc. > > > > That gets a lot more complex if the target KVM instance (module, not > > "struct kvm") > > is a different KVM, e.g. when migrating to a different host. Then there > > needs to > > be a way to attest that the target is trusted and whatnot, but that still > > doesn't > > require there to be a formal definition of a "virtual machine". > > > >> Circling back to the original topic, was associating the file with > >> struct kvm at gmem file creation time meant to constrain the use of the > >> gmem file to one struct kvm, or one virtual machine, or something else? > > > > It's meant to keep things as simple as possible (relatively speaking). A > > 1:1 > > association between a KVM instance and a gmem instance means we don't have > > to > > worry about the edge cases and oddities I pointed out earlier in this > > thread. > > > > I looked through this thread again and re-read the edge cases and > oddities that was pointed out earlier (last paragraph at [1]) and I > think I understand better, and I have just one last clarification. > > It was previously mentioned that binding on creation time simplifies the > lifecycle of memory: > > "(a) prevent a different VM from *ever* binding to the gmem instance" [1] > > Does this actually mean > > "prevent a different struct kvm from *ever* binding to this gmem file" > > ? Yes. > If so, then binding on creation > > + Makes the gmem *file* (and just not the bindings xarray) the binding > between struct kvm and the file. Yep. > + Simplifies the KVM-userspace contract to "this gmem file can only be > used with this struct kvm" Yep. > Binding on creation doesn't offer any way to block the contents of the > inode from being used with another "virtual machine" though, since we >
[PATCH] hwmon: (ibmpowernv) refactor deprecated strncpy
`strncpy` is deprecated for use on NUL-terminated destination strings [1]. We should prefer more robust and less ambiguous string interfaces. A suitable replacement is `strscpy` [2] due to the fact that it guarantees NUL-termination on the destination buffer without unnecessarily NUL-padding since `buf` is already zero-initialized. Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1] Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2] Link: https://github.com/KSPP/linux/issues/90 Cc: linux-harden...@vger.kernel.org Signed-off-by: Justin Stitt --- drivers/hwmon/ibmpowernv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/hwmon/ibmpowernv.c b/drivers/hwmon/ibmpowernv.c index 594254d6a72d..57d829dbcda6 100644 --- a/drivers/hwmon/ibmpowernv.c +++ b/drivers/hwmon/ibmpowernv.c @@ -234,7 +234,7 @@ static int get_sensor_index_attr(const char *name, u32 *index, char *attr) if (copy_len >= sizeof(buf)) return -EINVAL; - strncpy(buf, hash_pos + 1, copy_len); + strscpy(buf, hash_pos + 1, copy_len); err = kstrtou32(buf, 10, index); if (err) --- base-commit: 3669558bdf354cd352be955ef2764cde6a9bf5ec change-id: 20230914-strncpy-drivers-hwmon-ibmpowernv-c-80a03f16d93a Best regards, -- Justin Stitt
Re: [RFC PATCH v11 12/29] KVM: Add KVM_CREATE_GUEST_MEMFD ioctl() for guest-specific backing memory
Sean Christopherson writes: > On Mon, Aug 28, 2023, Ackerley Tng wrote: >> Sean Christopherson writes: >> >> If we track struct kvm with the inode, then I think (a), (b) and (c) can >> >> be independent of the refcounting method. What do you think? >> > >> > No go. Because again, the inode (physical memory) is coupled to the >> > virtual machine >> > as a thing, not to a "struct kvm". Or more concretely, the inode is >> > coupled to an >> > ASID or an HKID, and there can be multiple "struct kvm" objects associated >> > with a >> > single ASID. And at some point in the future, I suspect we'll have >> > multiple KVM >> > objects per HKID too. >> > >> > The current SEV use case is for the migration helper, where two KVM >> > objects share >> > a single ASID (the "real" VM and the helper). I suspect TDX will end up >> > with >> > similar behavior where helper "VMs" can use the HKID of the "real" VM. >> > For KVM, >> > that means multiple struct kvm objects being associated with a single HKID. >> > >> > To prevent use-after-free, KVM "just" needs to ensure the helper instances >> > can't >> > outlive the real instance, i.e. can't use the HKID/ASID after the owning >> > virtual >> > machine has been destroyed. >> > >> > To put it differently, "struct kvm" is a KVM software construct that >> > _usually_, >> > but not always, is associated 1:1 with a virtual machine. >> > >> > And FWIW, stashing the pointer without holding a reference would not be a >> > complete >> > solution, because it couldn't guard against KVM reusing a pointer. E.g. >> > if a >> > struct kvm was unbound and then freed, KVM could reuse the same memory for >> > a new >> > struct kvm, with a different ASID/HKID, and get a false negative on the >> > rebinding >> > check. >> >> I agree that inode (physical memory) is coupled to the virtual machine >> as a more generic concept. >> >> I was hoping that in the absence of CC hardware providing a HKID/ASID, >> the struct kvm pointer could act as a representation of the "virtual >> machine". You're definitely right that KVM could reuse a pointer and so >> that idea doesn't stand. >> >> I thought about generating UUIDs to represent "virtual machines" in the >> absence of CC hardware, and this UUID could be transferred during >> intra-host migration, but this still doesn't take host userspace out of >> the TCB. A malicious host VMM could just use the migration ioctl to copy >> the UUID to a malicious dumper VM, which would then pass checks with a >> gmem file linked to the malicious dumper VM. This is fine for HKID/ASIDs >> because the memory is encrypted; with UUIDs there's no memory >> encryption. > > I don't understand what problem you're trying to solve. I don't see a need to > provide a single concrete representation/definition of a "virtual machine". > E.g. > there's no need for a formal definition to securely perform intrahost > migration, > KVM just needs to ensure that the migration doesn't compromise guest security, > functionality, etc. > > That gets a lot more complex if the target KVM instance (module, not "struct > kvm") > is a different KVM, e.g. when migrating to a different host. Then there > needs to > be a way to attest that the target is trusted and whatnot, but that still > doesn't > require there to be a formal definition of a "virtual machine". > >> Circling back to the original topic, was associating the file with >> struct kvm at gmem file creation time meant to constrain the use of the >> gmem file to one struct kvm, or one virtual machine, or something else? > > It's meant to keep things as simple as possible (relatively speaking). A 1:1 > association between a KVM instance and a gmem instance means we don't have to > worry about the edge cases and oddities I pointed out earlier in this thread. > I looked through this thread again and re-read the edge cases and oddities that was pointed out earlier (last paragraph at [1]) and I think I understand better, and I have just one last clarification. It was previously mentioned that binding on creation time simplifies the lifecycle of memory: "(a) prevent a different VM from *ever* binding to the gmem instance" [1] Does this actually mean "prevent a different struct kvm from *ever* binding to this gmem file" ? If so, then binding on creation + Makes the gmem *file* (and just not the bindings xarray) the binding between struct kvm and the file. + Simplifies the KVM-userspace contract to "this gmem file can only be used with this struct kvm" Binding on creation doesn't offer any way to block the contents of the inode from being used with another "virtual machine" though, since we can have more than one gmem file pointing to the same inode, and the other gmem file is associated with another struct kvm. (And a strut kvm isn't associated 1:1 with a virtual machine [2]) The point about an inode needing to be coupled to a virtual machine as a thing [2] led me to try to find a single concrete
Re: [PATCH v2] arch: Reserve map_shadow_stack() syscall number for all architectures
On Thu, 2023-09-14 at 18:58 +, Sohil Mehta wrote: > commit c35559f94ebc ("x86/shstk: Introduce map_shadow_stack syscall") > recently added support for map_shadow_stack() but it is limited to > x86 > only for now. There is a possibility that other architectures > (namely, > arm64 and RISC-V), that are implementing equivalent support for > shadow > stacks, might need to add support for it. > > Independent of that, reserving arch-specific syscall numbers in the > syscall tables of all architectures is good practice and would help > avoid future conflicts. map_shadow_stack() is marked as a conditional > syscall in sys_ni.c. Adding it to the syscall tables of other > architectures is harmless and would return ENOSYS when exercised. > > Note, map_shadow_stack() was assigned #453 during the merge process > since #452 was taken by fchmodat2(). > > For Powerpc, map it to sys_ni_syscall() as is the norm for Powerpc > syscall tables. > > For Alpha, map_shadow_stack() takes up #563 as Alpha still diverges > from > the common syscall numbering system in the other architectures. Reviewed-by: Rick Edgecombe
Re: [PATCH tty v1 00/74] serial: wrappers for uart port lock
On Thu, 14 Sep 2023, John Ogness wrote: > Patches 2-74 switch all uart port locking call sites to use the new > wrappers. These patches were automatically generated using coccinelle. Hmm, no need to do this for drivers/tty/serial/zs.c? Maciej
[PATCH v2] arch: Reserve map_shadow_stack() syscall number for all architectures
commit c35559f94ebc ("x86/shstk: Introduce map_shadow_stack syscall") recently added support for map_shadow_stack() but it is limited to x86 only for now. There is a possibility that other architectures (namely, arm64 and RISC-V), that are implementing equivalent support for shadow stacks, might need to add support for it. Independent of that, reserving arch-specific syscall numbers in the syscall tables of all architectures is good practice and would help avoid future conflicts. map_shadow_stack() is marked as a conditional syscall in sys_ni.c. Adding it to the syscall tables of other architectures is harmless and would return ENOSYS when exercised. Note, map_shadow_stack() was assigned #453 during the merge process since #452 was taken by fchmodat2(). For Powerpc, map it to sys_ni_syscall() as is the norm for Powerpc syscall tables. For Alpha, map_shadow_stack() takes up #563 as Alpha still diverges from the common syscall numbering system in the other architectures. Link: https://lore.kernel.org/lkml/20230515212255.ga562...@debug.ba.rivosinc.com/ Link: https://lore.kernel.org/lkml/b402b80b-a7c6-4ef0-b977-c0f5f582b...@sirena.org.uk/ Signed-off-by: Sohil Mehta --- v2: - Skip syscall table changes to tools/. They will be handled separetely by the perf folks. - Map Powerpc to sys_ni_syscall (Rick Edgecombe) --- arch/alpha/kernel/syscalls/syscall.tbl | 1 + arch/arm/tools/syscall.tbl | 1 + arch/arm64/include/asm/unistd.h | 2 +- arch/arm64/include/asm/unistd32.h | 2 ++ arch/ia64/kernel/syscalls/syscall.tbl | 1 + arch/m68k/kernel/syscalls/syscall.tbl | 1 + arch/microblaze/kernel/syscalls/syscall.tbl | 1 + arch/mips/kernel/syscalls/syscall_n32.tbl | 1 + arch/mips/kernel/syscalls/syscall_n64.tbl | 1 + arch/mips/kernel/syscalls/syscall_o32.tbl | 1 + arch/parisc/kernel/syscalls/syscall.tbl | 1 + arch/powerpc/kernel/syscalls/syscall.tbl| 1 + arch/s390/kernel/syscalls/syscall.tbl | 1 + arch/sh/kernel/syscalls/syscall.tbl | 1 + arch/sparc/kernel/syscalls/syscall.tbl | 1 + arch/x86/entry/syscalls/syscall_32.tbl | 1 + arch/xtensa/kernel/syscalls/syscall.tbl | 1 + include/uapi/asm-generic/unistd.h | 5 - 18 files changed, 22 insertions(+), 2 deletions(-) diff --git a/arch/alpha/kernel/syscalls/syscall.tbl b/arch/alpha/kernel/syscalls/syscall.tbl index ad37569d0507..6e8479c96e65 100644 --- a/arch/alpha/kernel/syscalls/syscall.tbl +++ b/arch/alpha/kernel/syscalls/syscall.tbl @@ -492,3 +492,4 @@ 560common set_mempolicy_home_node sys_ni_syscall 561common cachestat sys_cachestat 562common fchmodat2 sys_fchmodat2 +563common map_shadow_stacksys_map_shadow_stack diff --git a/arch/arm/tools/syscall.tbl b/arch/arm/tools/syscall.tbl index c572d6c3dee0..6d494dfbf5e4 100644 --- a/arch/arm/tools/syscall.tbl +++ b/arch/arm/tools/syscall.tbl @@ -466,3 +466,4 @@ 450common set_mempolicy_home_node sys_set_mempolicy_home_node 451common cachestat sys_cachestat 452common fchmodat2 sys_fchmodat2 +453common map_shadow_stacksys_map_shadow_stack diff --git a/arch/arm64/include/asm/unistd.h b/arch/arm64/include/asm/unistd.h index bd77253b62e0..6a28fb91b85d 100644 --- a/arch/arm64/include/asm/unistd.h +++ b/arch/arm64/include/asm/unistd.h @@ -39,7 +39,7 @@ #define __ARM_NR_compat_set_tls(__ARM_NR_COMPAT_BASE + 5) #define __ARM_NR_COMPAT_END(__ARM_NR_COMPAT_BASE + 0x800) -#define __NR_compat_syscalls 453 +#define __NR_compat_syscalls 454 #endif #define __ARCH_WANT_SYS_CLONE diff --git a/arch/arm64/include/asm/unistd32.h b/arch/arm64/include/asm/unistd32.h index 78b68311ec81..a201d842ec82 100644 --- a/arch/arm64/include/asm/unistd32.h +++ b/arch/arm64/include/asm/unistd32.h @@ -911,6 +911,8 @@ __SYSCALL(__NR_set_mempolicy_home_node, sys_set_mempolicy_home_node) __SYSCALL(__NR_cachestat, sys_cachestat) #define __NR_fchmodat2 452 __SYSCALL(__NR_fchmodat2, sys_fchmodat2) +#define __NR_map_shadow_stack 453 +__SYSCALL(__NR_map_shadow_stack, sys_map_shadow_stack) /* * Please add new compat syscalls above this comment and update diff --git a/arch/ia64/kernel/syscalls/syscall.tbl b/arch/ia64/kernel/syscalls/syscall.tbl index 83d8609aec03..be02ce9d376f 100644 --- a/arch/ia64/kernel/syscalls/syscall.tbl +++ b/arch/ia64/kernel/syscalls/syscall.tbl @@ -373,3 +373,4 @@ 450common set_mempolicy_home_node sys_set_mempolicy_home_node 451common cachestat sys_cachestat 452common fchmodat2 sys_fchmodat2 +453common map_shadow_stacksys_map_shadow_stack diff --git a/arch/m68k/kernel/syscalls/syscall.tbl b/arch/m68k/kernel/syscalls/syscall.tbl index 259ceb125367..bee2d2f7f82c 100644 ---
[PATCH tty v1 00/74] serial: wrappers for uart port lock
When a serial port is used for kernel console output, then all modifications to the UART registers which are done from other contexts, e.g. getty, termios, are interference points for the kernel console. So far this has been ignored and the printk output is based on the principle of hope. The rework of the console infrastructure which aims to support threaded and atomic consoles, requires to mark sections which modify the UART registers as unsafe. This allows the atomic write function to make informed decisions and eventually to restore operational state. It also allows to prevent the regular UART code from modifying UART registers while printk output is in progress. All modifications of UART registers are guarded by the UART port lock, which provides an obvious synchronization point with the console infrastructure. Provide and use wrapper functions for spin_[un]lock*(port->lock) invocations so that the console mechanics can be applied later on at a single place and does not require to copy the same logic all over the drivers. Patch 1 adds the wrapper functions. Patches 2-74 switch all uart port locking call sites to use the new wrappers. These patches were automatically generated using coccinelle. The 2 used coccinelle scripts are included below and executed as follows: $ spatch --sp-file uartlock-1.cocci $FILE $ spatch --sp-file uartlock-2.cocci --recursive-includes $FILE This series brings no functional change. Patches 2-74 contain identical commit message bodies. Feel free to fold them into a single commit if that seems more reasonable. Thomas Gleixner (74): serial: core: Provide port lock wrappers serial: core: Use lock wrappers serial: 21285: Use port lock wrappers serial: 8250_aspeed_vuart: Use port lock wrappers serial: 8250_bcm7271: Use port lock wrappers serial: 8250: Use port lock wrappers serial: 8250_dma: Use port lock wrappers serial: 8250_dw: Use port lock wrappers serial: 8250_exar: Use port lock wrappers serial: 8250_fsl: Use port lock wrappers serial: 8250_mtk: Use port lock wrappers serial: 8250_omap: Use port lock wrappers serial: 8250_pci1: Use port lock wrappers serial: altera_jtaguart: Use port lock wrappers serial: altera_uart: Use port lock wrappers serial: amba-pl010: Use port lock wrappers serial: amba-pl011: Use port lock wrappers serial: apb: Use port lock wrappers serial: ar933x: Use port lock wrappers serial: arc_uart: Use port lock wrappers serial: atmel: Use port lock wrappers serial: bcm63xx-uart: Use port lock wrappers serial: cpm_uart: Use port lock wrappers serial: digicolor: Use port lock wrappers serial: dz: Use port lock wrappers serial: linflexuart: Use port lock wrappers serial: fsl_lpuart: Use port lock wrappers serial: icom: Use port lock wrappers serial: imx: Use port lock wrappers serial: ip22zilog: Use port lock wrappers serial: jsm: Use port lock wrappers serial: liteuart: Use port lock wrappers serial: lpc32xx_hs: Use port lock wrappers serial: ma35d1: Use port lock wrappers serial: mcf: Use port lock wrappers serial: men_z135_uart: Use port lock wrappers serial: meson: Use port lock wrappers serial: milbeaut_usio: Use port lock wrappers serial: mpc52xx: Use port lock wrappers serial: mps2-uart: Use port lock wrappers serial: msm: Use port lock wrappers serial: mvebu-uart: Use port lock wrappers serial: omap: Use port lock wrappers serial: owl: Use port lock wrappers serial: pch: Use port lock wrappers serial: pic32: Use port lock wrappers serial: pmac_zilog: Use port lock wrappers serial: pxa: Use port lock wrappers serial: qcom-geni: Use port lock wrappers serial: rda: Use port lock wrappers serial: rp2: Use port lock wrappers serial: sa1100: Use port lock wrappers serial: samsung_tty: Use port lock wrappers serial: sb1250-duart: Use port lock wrappers serial: sc16is7xx: Use port lock wrappers serial: tegra: Use port lock wrappers serial: core: Use port lock wrappers serial: mctrl_gpio: Use port lock wrappers serial: txx9: Use port lock wrappers serial: sh-sci: Use port lock wrappers serial: sifive: Use port lock wrappers serial: sprd: Use port lock wrappers serial: st-asc: Use port lock wrappers serial: stm32: Use port lock wrappers serial: sunhv: Use port lock wrappers serial: sunplus-uart: Use port lock wrappers serial: sunsab: Use port lock wrappers serial: sunsu: Use port lock wrappers serial: sunzilog: Use port lock wrappers serial: timbuart: Use port lock wrappers serial: uartlite: Use port lock wrappers serial: ucc_uart: Use port lock wrappers serial: vt8500: Use port lock wrappers serial: xilinx_uartps: Use port lock wrappers drivers/tty/serial/21285.c | 8 +- drivers/tty/serial/8250/8250_aspeed_vuart.c | 6 +- drivers/tty/serial/8250/8250_bcm7271.c | 28 +++--- drivers/tty/serial/8250/8250_core.c | 12 +-- drivers/tty/serial/8250/8250_dma.c
Re: [PATCH v3 1/3] powerpc/config: Cleanup pmac32_defconfig
Hi Michael On 9/14/2023 9:10 PM, Michael Ellerman wrote: Yuan Tan writes: Use 'make savedefconfig' to cleanup pmac32_defconfig, based on Linux 7.6-rc1 Thanks but I don't like doing these updates in a single commit like this, it's easy to accidentally lose a symbol. Yeah I have the same concerns too. I prefer an explanation for what's changing for each symbol. See 1ce7fda142af ("powerpc/configs/64s: Drop IPV6 which is default y") and the commits leading up to it, to see what I mean. But I suspect you probably don't want to go to all that effort, which is fine :) I am not familiar with other options, so I'd better not do that. :) By the way, just to be cautious, since the defconfig can only be updated by 'savedefconfig'[1], how can we write an explanation for a single change in an option? I mean, when I change one option, the value of the other undetermined option will be set just like in patch 1. [1]: https://lore.kernel.org/all/54da2376-dc65-0a96-55df-7a5acfbb9...@csgroup.eu/ So I won't take patch 1, but patch 2 and 3 look fine. No need to resend, I'll deal with any merge fixup needed. cheers Thank you :)
Re: [PATCH 2/2] arch: Reserve map_shadow_stack() syscall number for all architectures
"Edgecombe, Rick P" writes: > On Mon, 2023-09-11 at 18:02 +, Sohil Mehta wrote: >> diff --git a/arch/powerpc/kernel/syscalls/syscall.tbl >> b/arch/powerpc/kernel/syscalls/syscall.tbl >> index 20e50586e8a2..2767b8a42636 100644 >> --- a/arch/powerpc/kernel/syscalls/syscall.tbl >> +++ b/arch/powerpc/kernel/syscalls/syscall.tbl >> @@ -539,3 +539,4 @@ >> 450nospu set_mempolicy_home_node sys_set_mempolicy_hom >> e_node >> 451common cachestat sys_cachestat >> 452common fchmodat2 sys_fchmodat2 >> +453common map_shadow_stacksys_map_shadow_stack > > I noticed in powerpc, the not implemented syscalls are manually mapped > to sys_ni_syscall. It also has some special extra sys_ni_syscall() > implementation bits to handle both ARCH_HAS_SYSCALL_WRAPPER and > !ARCH_HAS_SYSCALL_WRAPPER. So wondering if it might need special > treatment. Did you see those parts? I don't think it needs any special treatment. It's processed by the same script as other arches (scripts/syscalltbl.sh). So if there's no compat or native entry it will default to sys_ni_syscall. I think it's just habit/historical that we always spell out sys_ni_syscall. cheers
Re: [V2 2/2] tools/perf/tests: Fix object code reading to skip address that falls out of text section
On 07/09/23 10:15 pm, Athira Rajeev wrote: The testcase "Object code reading" fails in somecases for "fs_something" sub test as below: Reading object code for memory address: 0xc00807f0142c File is: /lib/modules/6.5.0-rc3+/kernel/fs/xfs/xfs.ko On file address is: 0x1114cc Objdump command is: objdump -z -d --start-address=0x11142c --stop-address=0x1114ac /lib/modules/6.5.0-rc3+/kernel/fs/xfs/xfs.ko objdump read too few bytes: 128 test child finished with -1 This can alo be reproduced when running perf record with workload that exercises fs_something() code. In the test setup, this is exercising xfs code since root is xfs. # perf record ./a.out # perf report -v |grep "xfs.ko" 0.76% a.out /lib/modules/6.5.0-rc3+/kernel/fs/xfs/xfs.ko 0xc00807de5efc B [k] xlog_cil_commit 0.74% a.out /lib/modules/6.5.0-rc3+/kernel/fs/xfs/xfs.ko 0xc00807d5ae18 B [k] xfs_btree_key_offset 0.74% a.out /lib/modules/6.5.0-rc3+/kernel/fs/xfs/xfs.ko 0xc00807e11fd4 B [k] 0x00112074 Here addr "0xc00807e11fd4" is not resolved. since this is a kernel module, its offset is from the DSO. Xfs module is loaded at 0xc00807d0 # cat /proc/modules | grep xfs xfs 2228224 3 - Live 0xc00807d0 And size is 0x22. So its loaded between 0xc00807d0 and 0xc00807f2. From objdump, text section is: text 0010f7bc 00a0 2**4 Hence perf captured ip maps to 0x112074 which is: ( ip - start of module ) + a0 This offset 0x112074 falls out .text section which is up to 0x10f7bc In this case for module, the address 0xc00807e11fd4 is pointing to stub instructions. This address range represents the module stubs which is allocated on module load and hence is not part of DSO offset. To address this issue in "object code reading", skip the sample if address falls out of text section and is within the module end. Use the "text_end" member of "struct dso" to do this check. To address this issue in "perf report", exploring an option of having stubs range as part of the /proc/kallsyms, so that perf report can resolve addresses in stubs range However this patch uses text_end to skip the stub range for Object code reading testcase. Reported-by: Disha Goel Signed-off-by: Athira Rajeev --- Changelog: v1 -> v2: Updated comment to add description on which arch has stub and reason for skipping as suggested by Adrian With this patch applied perf Object code reading test works correctly. 26: Object code reading : Ok Tested-by: Disha Goel tools/perf/tests/code-reading.c | 12 1 file changed, 12 insertions(+) diff --git a/tools/perf/tests/code-reading.c b/tools/perf/tests/code-reading.c index ed3815163d1b..3cf6c2d42416 100644 --- a/tools/perf/tests/code-reading.c +++ b/tools/perf/tests/code-reading.c @@ -269,6 +269,18 @@ static int read_object_code(u64 addr, size_t len, u8 cpumode, if (addr + len > map__end(al.map)) len = map__end(al.map) - addr; + /* +* Some architectures (ex: powerpc) have stubs (trampolines) in kernel +* modules to manage long jumps. Check if the ip offset falls in stubs +* sections for kernel modules. And skip module address after text end +*/ + if (strstr(dso->long_name, ".ko")) { + if (al.addr > dso->text_end) { + pr_debug("skipping the module address %#"PRIx64" after text end\n", al.addr); + goto out; + } + } + /* Read the object code using perf */ ret_len = dso__data_read_offset(dso, maps__machine(thread__maps(thread)), al.addr, buf1, len);
Re: [PATCH] powerpc: Export kvm_guest static key, for bcachefs six locks
On Thu, Sep 14, 2023 at 12:26:53PM +1000, Michael Ellerman wrote: > Kent Overstreet writes: > > bcachefs's six locks need kvm_guest, via > > ower_on_cpu() -> vcpu_is_preempted() -> is_kvm_guest() > > > > Signed-off-by: Kent Overstreet > > Cc: linuxppc-dev@lists.ozlabs.org > > --- > > arch/powerpc/kernel/firmware.c | 2 ++ > > 1 file changed, 2 insertions(+) > > Acked-by: Michael Ellerman (powerpc) > > I'm happy for you to take this via your tree. Thanks!
Re: [PATCH] powerpc/82xx: Select FSL_SOC
On 9/14/23 08:23, Christophe Leroy wrote: > It used to be impossible to select CONFIG_CPM2 without selecting > CONFIG_FSL_SOC at the same time because CONFIG_CPM2 was dependent > on CONFIG_8260 and CONFIG_8260 was selecting CONFIG_FSL_SOC. > > But after commit eb5aa2137275 ("powerpc/82xx: Remove CONFIG_8260 > and CONFIG_8272") CONFIG_CPM2 depends on CONFIG_MPC82xx instead > but CONFIG_MPC82xx doesn't directly selects CONFIG_FSL_SOC. > > Fix it by forcing CONFIG_MPC82xx to select CONFIG_FSL_SOC just > like already done by MPC8xx, MPC512x, MPC83xx, PPC_86xx. > > Reported-by: Randy Dunlap > Fixes: eb5aa2137275 ("powerpc/82xx: Remove CONFIG_8260 and CONFIG_8272") > Signed-off-by: Christophe Leroy Acked-by: Randy Dunlap Tested-by: Randy Dunlap Thanks for tracking this down. > --- > arch/powerpc/platforms/82xx/Kconfig | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/arch/powerpc/platforms/82xx/Kconfig > b/arch/powerpc/platforms/82xx/Kconfig > index d9f1a2a83158..1824536cf6f2 100644 > --- a/arch/powerpc/platforms/82xx/Kconfig > +++ b/arch/powerpc/platforms/82xx/Kconfig > @@ -2,6 +2,7 @@ > menuconfig PPC_82xx > bool "82xx-based boards (PQ II)" > depends on PPC_BOOK3S_32 > + select FSL_SOC > > if PPC_82xx > > @@ -9,7 +10,6 @@ config EP8248E > bool "Embedded Planet EP8248E (a.k.a. CWH-PPC-8248N-VE)" > select CPM2 > select PPC_INDIRECT_PCI if PCI > - select FSL_SOC > select PHYLIB if NETDEVICES > select MDIO_BITBANG if PHYLIB > help > @@ -22,7 +22,6 @@ config MGCOGE > bool "Keymile MGCOGE" > select CPM2 > select PPC_INDIRECT_PCI if PCI > - select FSL_SOC > help > This enables support for the Keymile MGCOGE board. > -- ~Randy
Re: [RFC PATCH v11 12/29] KVM: Add KVM_CREATE_GUEST_MEMFD ioctl() for guest-specific backing memory
On Mon, Aug 28, 2023, Elliot Berman wrote: > I had a 3rd question that's related to how to wire the gmem up to a virtual > machine: > > I learned of a usecase to implement copy-on-write for gmem. The premise > would be to have a "golden copy" of the memory that multiple virtual > machines can map in as RO. If a virtual machine tries to write to those > pages, they get copied to a virtual machine-specific page that isn't shared > with other VMs. How do we track those pages? The answer is going to be gunyah specific, because gmem itself isn't designed to provide a virtualization layer ("virtual" in the virtual memory sense, not in the virtual machine sense). Like any other CoW implementation, the RO page would need to be copied to a different physical page, and whatever layer translates gfns to physical pages would need to be updated. E.g. in gmem terms, allocate a new gmem page/instance and update the gfn=>gmem[offset] translation in KVM/gunyah. For VMA-based memory, that translation happens in the primary MMU, and is largely transparent to KVM (or any other secondary MMU). E.g. the primary MMU works with the backing store (if necessary) to allocate a new page and do the copy, notifies secondary MMUs, zaps the old PTE(s), and then installs the new PTE(s). KVM/gunyah just needs to react to the mmu_notifier event, e.g. zap secondary MMU PTEs, and then KVM/gunyah naturally gets the new, writable page/PTE when following the host virtual address, e.g. via gup(). The downside of eliminating the middle-man (primary MMU) from gmem is that the "owner" (KVM or gunyah) is now responsible for these types of operations. For some things, e.g. page migration, it's actually easier in some ways, but for CoW it's quite a bit more work for KVM/gunyah because KVM/gunyah now needs to do things that were previously handled by the primary MMU. In KVM, assuming no additional support in KVM, doing CoW would mean modifying memslots to redirect the gfn from the RO page to the writable page. For a variety of reasons, that would be _extremely_ expensive in KVM, but still possible. If there were a strong use case for supporting CoW with KVM+gmem, then I suspect that we'd probably implement new KVM uAPI of some form to provide reasonable performance. But I highly doubt we'll ever do that, because one of core tenets of KVM+gmem is to isolate guest memory from the rest of the world, and especially from host userspace, and that just doesn't mesh well with CoW'd memory being shared across multiple VMs.
[PATCH tty v1 47/74] serial: pmac_zilog: Use port lock wrappers
From: Thomas Gleixner When a serial port is used for kernel console output, then all modifications to the UART registers which are done from other contexts, e.g. getty, termios, are interference points for the kernel console. So far this has been ignored and the printk output is based on the principle of hope. The rework of the console infrastructure which aims to support threaded and atomic consoles, requires to mark sections which modify the UART registers as unsafe. This allows the atomic write function to make informed decisions and eventually to restore operational state. It also allows to prevent the regular UART code from modifying UART registers while printk output is in progress. All modifications of UART registers are guarded by the UART port lock, which provides an obvious synchronization point with the console infrastructure. To avoid adding this functionality to all UART drivers, wrap the spin_[un]lock*() invocations for uart_port::lock into helper functions which just contain the spin_[un]lock*() invocations for now. In a subsequent step these helpers will gain the console synchronization mechanisms. Converted with coccinelle. No functional change. Signed-off-by: Thomas Gleixner --- drivers/tty/serial/pmac_zilog.c | 52 - 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/drivers/tty/serial/pmac_zilog.c b/drivers/tty/serial/pmac_zilog.c index 13668ffdb1e7..c8bf08c19c64 100644 --- a/drivers/tty/serial/pmac_zilog.c +++ b/drivers/tty/serial/pmac_zilog.c @@ -246,9 +246,9 @@ static bool pmz_receive_chars(struct uart_pmac_port *uap) #endif /* USE_CTRL_O_SYSRQ */ if (uap->port.sysrq) { int swallow; - spin_unlock(>port.lock); + uart_port_unlock(>port); swallow = uart_handle_sysrq_char(>port, ch); - spin_lock(>port.lock); + uart_port_lock(>port); if (swallow) goto next_char; } @@ -435,7 +435,7 @@ static irqreturn_t pmz_interrupt(int irq, void *dev_id) uap_a = pmz_get_port_A(uap); uap_b = uap_a->mate; - spin_lock(_a->port.lock); + uart_port_lock(_a->port); r3 = read_zsreg(uap_a, R3); /* Channel A */ @@ -456,14 +456,14 @@ static irqreturn_t pmz_interrupt(int irq, void *dev_id) rc = IRQ_HANDLED; } skip_a: - spin_unlock(_a->port.lock); + uart_port_unlock(_a->port); if (push) tty_flip_buffer_push(>port.state->port); if (!uap_b) goto out; - spin_lock(_b->port.lock); + uart_port_lock(_b->port); push = false; if (r3 & (CHBEXT | CHBTxIP | CHBRxIP)) { if (!ZS_IS_OPEN(uap_b)) { @@ -481,7 +481,7 @@ static irqreturn_t pmz_interrupt(int irq, void *dev_id) rc = IRQ_HANDLED; } skip_b: - spin_unlock(_b->port.lock); + uart_port_unlock(_b->port); if (push) tty_flip_buffer_push(>port.state->port); @@ -497,9 +497,9 @@ static inline u8 pmz_peek_status(struct uart_pmac_port *uap) unsigned long flags; u8 status; - spin_lock_irqsave(>port.lock, flags); + uart_port_lock_irqsave(>port, ); status = read_zsreg(uap, R0); - spin_unlock_irqrestore(>port.lock, flags); + uart_port_unlock_irqrestore(>port, flags); return status; } @@ -685,7 +685,7 @@ static void pmz_break_ctl(struct uart_port *port, int break_state) else clear_bits |= SND_BRK; - spin_lock_irqsave(>lock, flags); + uart_port_lock_irqsave(port, ); new_reg = (uap->curregs[R5] | set_bits) & ~clear_bits; if (new_reg != uap->curregs[R5]) { @@ -693,7 +693,7 @@ static void pmz_break_ctl(struct uart_port *port, int break_state) write_zsreg(uap, R5, uap->curregs[R5]); } - spin_unlock_irqrestore(>lock, flags); + uart_port_unlock_irqrestore(port, flags); } #ifdef CONFIG_PPC_PMAC @@ -865,18 +865,18 @@ static void pmz_irda_reset(struct uart_pmac_port *uap) { unsigned long flags; - spin_lock_irqsave(>port.lock, flags); + uart_port_lock_irqsave(>port, ); uap->curregs[R5] |= DTR; write_zsreg(uap, R5, uap->curregs[R5]); zssync(uap); - spin_unlock_irqrestore(>port.lock, flags); + uart_port_unlock_irqrestore(>port, flags); msleep(110); - spin_lock_irqsave(>port.lock, flags); + uart_port_lock_irqsave(>port, ); uap->curregs[R5] &= ~DTR; write_zsreg(uap, R5, uap->curregs[R5]); zssync(uap); - spin_unlock_irqrestore(>port.lock, flags); + uart_port_unlock_irqrestore(>port, flags); msleep(10); } @@ -896,9 +896,9 @@ static int pmz_startup(struct uart_port *port) * initialize the
[PATCH tty v1 72/74] serial: ucc_uart: Use port lock wrappers
From: Thomas Gleixner When a serial port is used for kernel console output, then all modifications to the UART registers which are done from other contexts, e.g. getty, termios, are interference points for the kernel console. So far this has been ignored and the printk output is based on the principle of hope. The rework of the console infrastructure which aims to support threaded and atomic consoles, requires to mark sections which modify the UART registers as unsafe. This allows the atomic write function to make informed decisions and eventually to restore operational state. It also allows to prevent the regular UART code from modifying UART registers while printk output is in progress. All modifications of UART registers are guarded by the UART port lock, which provides an obvious synchronization point with the console infrastructure. To avoid adding this functionality to all UART drivers, wrap the spin_[un]lock*() invocations for uart_port::lock into helper functions which just contain the spin_[un]lock*() invocations for now. In a subsequent step these helpers will gain the console synchronization mechanisms. Converted with coccinelle. No functional change. Signed-off-by: Thomas Gleixner --- drivers/tty/serial/ucc_uart.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/tty/serial/ucc_uart.c b/drivers/tty/serial/ucc_uart.c index b06661b80f41..ed7a6bb5596a 100644 --- a/drivers/tty/serial/ucc_uart.c +++ b/drivers/tty/serial/ucc_uart.c @@ -931,7 +931,7 @@ static void qe_uart_set_termios(struct uart_port *port, baud = uart_get_baud_rate(port, termios, old, 0, port->uartclk / 16); /* Do we really need a spinlock here? */ - spin_lock_irqsave(>lock, flags); + uart_port_lock_irqsave(port, ); /* Update the per-port timeout. */ uart_update_timeout(port, termios->c_cflag, baud); @@ -949,7 +949,7 @@ static void qe_uart_set_termios(struct uart_port *port, qe_setbrg(qe_port->us_info.tx_clock, baud, 16); } - spin_unlock_irqrestore(>lock, flags); + uart_port_unlock_irqrestore(port, flags); } /* -- 2.39.2
Re: [V2 2/2] tools/perf/tests: Fix object code reading to skip address that falls out of text section
On 7/09/23 19:45, Athira Rajeev wrote: > The testcase "Object code reading" fails in somecases > for "fs_something" sub test as below: > > Reading object code for memory address: 0xc00807f0142c > File is: /lib/modules/6.5.0-rc3+/kernel/fs/xfs/xfs.ko > On file address is: 0x1114cc > Objdump command is: objdump -z -d --start-address=0x11142c > --stop-address=0x1114ac /lib/modules/6.5.0-rc3+/kernel/fs/xfs/xfs.ko > objdump read too few bytes: 128 > test child finished with -1 > > This can alo be reproduced when running perf record with > workload that exercises fs_something() code. In the test > setup, this is exercising xfs code since root is xfs. > > # perf record ./a.out > # perf report -v |grep "xfs.ko" > 0.76% a.out /lib/modules/6.5.0-rc3+/kernel/fs/xfs/xfs.ko > 0xc00807de5efc B [k] xlog_cil_commit > 0.74% a.out /lib/modules/6.5.0-rc3+/kernel/fs/xfs/xfs.ko > 0xc00807d5ae18 B [k] xfs_btree_key_offset > 0.74% a.out /lib/modules/6.5.0-rc3+/kernel/fs/xfs/xfs.ko > 0xc00807e11fd4 B [k] 0x00112074 > > Here addr "0xc00807e11fd4" is not resolved. since this is a > kernel module, its offset is from the DSO. Xfs module is loaded > at 0xc00807d0 > ># cat /proc/modules | grep xfs > xfs 2228224 3 - Live 0xc00807d0 > > And size is 0x22. So its loaded between 0xc00807d0 > and 0xc00807f2. From objdump, text section is: > text 0010f7bc 00a0 2**4 > > Hence perf captured ip maps to 0x112074 which is: > ( ip - start of module ) + a0 > > This offset 0x112074 falls out .text section which is up to 0x10f7bc > In this case for module, the address 0xc00807e11fd4 is pointing > to stub instructions. This address range represents the module stubs > which is allocated on module load and hence is not part of DSO offset. > > To address this issue in "object code reading", skip the sample if > address falls out of text section and is within the module end. > Use the "text_end" member of "struct dso" to do this check. > > To address this issue in "perf report", exploring an option of > having stubs range as part of the /proc/kallsyms, so that perf > report can resolve addresses in stubs range > > However this patch uses text_end to skip the stub range for > Object code reading testcase. > > Reported-by: Disha Goel > Signed-off-by: Athira Rajeev > --- > Changelog: > v1 -> v2: > Updated comment to add description on which arch has stub and > reason for skipping as suggested by Adrian > > tools/perf/tests/code-reading.c | 12 > 1 file changed, 12 insertions(+) > > diff --git a/tools/perf/tests/code-reading.c b/tools/perf/tests/code-reading.c > index ed3815163d1b..3cf6c2d42416 100644 > --- a/tools/perf/tests/code-reading.c > +++ b/tools/perf/tests/code-reading.c > @@ -269,6 +269,18 @@ static int read_object_code(u64 addr, size_t len, u8 > cpumode, > if (addr + len > map__end(al.map)) > len = map__end(al.map) - addr; > > + /* > + * Some architectures (ex: powerpc) have stubs (trampolines) in kernel > + * modules to manage long jumps. Check if the ip offset falls in stubs > + * sections for kernel modules. And skip module address after text end > + */ > + if (strstr(dso->long_name, ".ko")) { Sorry for slow reply !strtailcmp() is slightly better here > + if (al.addr > dso->text_end) { We normally avoid nesting if-statements e.g. if (!strtailcmp(dso->long_name, ".ko") && al.addr > dso->text_end) Make those changes and you can add: Reviewed-by: Adrian Hunter > + pr_debug("skipping the module address %#"PRIx64" after > text end\n", al.addr); > + goto out; > + } > + } > + > /* Read the object code using perf */ > ret_len = dso__data_read_offset(dso, > maps__machine(thread__maps(thread)), > al.addr, buf1, len);
Re: [V2 1/2] tools/perf: Add text_end to "struct dso" to save .text section size
On 7/09/23 19:45, Athira Rajeev wrote: > Update "struct dso" to include new member "text_end". > This new field will represent the offset for end of text > section for a dso. For elf, this value is derived as: > sh_size (Size of section in byes) + sh_offset (Section file > offst) of the elf header for text. > > For bfd, this value is derived as: > 1. For PE file, > section->size + ( section->vma - dso->text_offset) > 2. Other cases: > section->filepos (file position) + section->size (size of > section) > > To resolve the address from a sample, perf looks at the > DSO maps. In case of address from a kernel module, there > were some address found to be not resolved. This was > observed while running perf test for "Object code reading". > Though the ip falls beteen the start address of the loaded > module (perf map->start ) and end address ( perf map->end), > it was unresolved. > > Example: > > Reading object code for memory address: 0xc00807f0142c > File is: /lib/modules/6.5.0-rc3+/kernel/fs/xfs/xfs.ko > On file address is: 0x1114cc > Objdump command is: objdump -z -d --start-address=0x11142c > --stop-address=0x1114ac /lib/modules/6.5.0-rc3+/kernel/fs/xfs/xfs.ko > objdump read too few bytes: 128 > test child finished with -1 > > Here, module is loaded at: > # cat /proc/modules | grep xfs > xfs 2228224 3 - Live 0xc00807d0 > > From objdump for xfs module, text section is: > text 0010f7bc 00a0 2**4 > > Here the offset for 0xc00807f0142c ie 0x112074 falls out > .text section which is up to 0x10f7bc. > > In this case for module, the address 0xc00807e11fd4 is pointing > to stub instructions. This address range represents the module stubs > which is allocated on module load and hence is not part of DSO offset. > > To identify such address, which falls out of text > section and within module end, added the new field "text_end" to > "struct dso". > > Reported-by: Disha Goel > Signed-off-by: Athira Rajeev Reviewed-by: Adrian Hunter > --- > Changelog: > v1 -> v2: > Added text_end for bfd also by updating dso__load_bfd_symbols > as suggested by Adrian. > > tools/perf/util/dso.h| 1 + > tools/perf/util/symbol-elf.c | 4 +++- > tools/perf/util/symbol.c | 2 ++ > 3 files changed, 6 insertions(+), 1 deletion(-) > > diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h > index b41c9782c754..70fe0fe69bef 100644 > --- a/tools/perf/util/dso.h > +++ b/tools/perf/util/dso.h > @@ -181,6 +181,7 @@ struct dso { > u8 rel; > struct build_id bid; > u64 text_offset; > + u64 text_end; > const char *short_name; > const char *long_name; > u16 long_name_len; > diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c > index 95e99c332d7e..9e7eeaf616b8 100644 > --- a/tools/perf/util/symbol-elf.c > +++ b/tools/perf/util/symbol-elf.c > @@ -1514,8 +1514,10 @@ dso__load_sym_internal(struct dso *dso, struct map > *map, struct symsrc *syms_ss, > } > > if (elf_section_by_name(runtime_ss->elf, _ss->ehdr, , > - ".text", NULL)) > + ".text", NULL)) { > dso->text_offset = tshdr.sh_addr - tshdr.sh_offset; > + dso->text_end = tshdr.sh_offset + tshdr.sh_size; > + } > > if (runtime_ss->opdsec) > opddata = elf_rawdata(runtime_ss->opdsec, NULL); > diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c > index 3f36675b7c8f..f25e4e62cf25 100644 > --- a/tools/perf/util/symbol.c > +++ b/tools/perf/util/symbol.c > @@ -1733,8 +1733,10 @@ int dso__load_bfd_symbols(struct dso *dso, const char > *debugfile) > /* PE symbols can only have 4 bytes, so use .text high > bits */ > dso->text_offset = section->vma - (u32)section->vma; > dso->text_offset += (u32)bfd_asymbol_value(symbols[i]); > + dso->text_end = (section->vma - dso->text_offset) + > section->size; > } else { > dso->text_offset = section->vma - section->filepos; > + dso->text_end = section->filepos + section->size; > } > } >
Re: [RFC PATCH v11 12/29] KVM: Add KVM_CREATE_GUEST_MEMFD ioctl() for guest-specific backing memory
On Mon, Aug 28, 2023, Ackerley Tng wrote: > Sean Christopherson writes: > >> If we track struct kvm with the inode, then I think (a), (b) and (c) can > >> be independent of the refcounting method. What do you think? > > > > No go. Because again, the inode (physical memory) is coupled to the > > virtual machine > > as a thing, not to a "struct kvm". Or more concretely, the inode is > > coupled to an > > ASID or an HKID, and there can be multiple "struct kvm" objects associated > > with a > > single ASID. And at some point in the future, I suspect we'll have > > multiple KVM > > objects per HKID too. > > > > The current SEV use case is for the migration helper, where two KVM objects > > share > > a single ASID (the "real" VM and the helper). I suspect TDX will end up > > with > > similar behavior where helper "VMs" can use the HKID of the "real" VM. For > > KVM, > > that means multiple struct kvm objects being associated with a single HKID. > > > > To prevent use-after-free, KVM "just" needs to ensure the helper instances > > can't > > outlive the real instance, i.e. can't use the HKID/ASID after the owning > > virtual > > machine has been destroyed. > > > > To put it differently, "struct kvm" is a KVM software construct that > > _usually_, > > but not always, is associated 1:1 with a virtual machine. > > > > And FWIW, stashing the pointer without holding a reference would not be a > > complete > > solution, because it couldn't guard against KVM reusing a pointer. E.g. if > > a > > struct kvm was unbound and then freed, KVM could reuse the same memory for > > a new > > struct kvm, with a different ASID/HKID, and get a false negative on the > > rebinding > > check. > > I agree that inode (physical memory) is coupled to the virtual machine > as a more generic concept. > > I was hoping that in the absence of CC hardware providing a HKID/ASID, > the struct kvm pointer could act as a representation of the "virtual > machine". You're definitely right that KVM could reuse a pointer and so > that idea doesn't stand. > > I thought about generating UUIDs to represent "virtual machines" in the > absence of CC hardware, and this UUID could be transferred during > intra-host migration, but this still doesn't take host userspace out of > the TCB. A malicious host VMM could just use the migration ioctl to copy > the UUID to a malicious dumper VM, which would then pass checks with a > gmem file linked to the malicious dumper VM. This is fine for HKID/ASIDs > because the memory is encrypted; with UUIDs there's no memory > encryption. I don't understand what problem you're trying to solve. I don't see a need to provide a single concrete representation/definition of a "virtual machine". E.g. there's no need for a formal definition to securely perform intrahost migration, KVM just needs to ensure that the migration doesn't compromise guest security, functionality, etc. That gets a lot more complex if the target KVM instance (module, not "struct kvm") is a different KVM, e.g. when migrating to a different host. Then there needs to be a way to attest that the target is trusted and whatnot, but that still doesn't require there to be a formal definition of a "virtual machine". > Circling back to the original topic, was associating the file with > struct kvm at gmem file creation time meant to constrain the use of the > gmem file to one struct kvm, or one virtual machine, or something else? It's meant to keep things as simple as possible (relatively speaking). A 1:1 association between a KVM instance and a gmem instance means we don't have to worry about the edge cases and oddities I pointed out earlier in this thread. > Follow up questions: > > 1. Since the physical memory's representation is the inode and should be >coupled to the virtual machine (as a concept, not struct kvm), should >the binding/coupling be with the file, or the inode? Both. The @kvm instance is bound to a file, because the file is that @kvm's view of the underlying memory, e.g. effectively provides the translation of guest addresses to host memory. The @kvm instance is indirectly bound to the inode because the file is bound to the inode. > 2. Should struct kvm still be bound to the file/inode at gmem file >creation time, since Yes. >+ struct kvm isn't a good representation of a "virtual machine" I don't see how this is relevant, because as above, I don't see why we need a canonical represenation of a virtual machine. >+ we currently don't have anything that really represents a "virtual > machine" without hardware support HKIDs and ASIDs don't provide a "virtual machine" representation either. E.g. if a TDX guest is live migrated to a different host, it will likely have a different HKID, and definitely have a different encryption key, but it's still the same virtual machine. > I'd also like to bring up another userspace use case that Google has: > re-use of gmem files for rebooting
Re: [V2 2/2] tools/perf/tests: Fix object code reading to skip address that falls out of text section
> On 14-Sep-2023, at 5:47 PM, Disha Goel wrote: > > On 07/09/23 10:15 pm, Athira Rajeev wrote: >> The testcase "Object code reading" fails in somecases >> for "fs_something" sub test as below: >> >> Reading object code for memory address: 0xc00807f0142c >> File is: /lib/modules/6.5.0-rc3+/kernel/fs/xfs/xfs.ko >> On file address is: 0x1114cc >> Objdump command is: objdump -z -d --start-address=0x11142c >> --stop-address=0x1114ac /lib/modules/6.5.0-rc3+/kernel/fs/xfs/xfs.ko >> objdump read too few bytes: 128 >> test child finished with -1 >> >> This can alo be reproduced when running perf record with >> workload that exercises fs_something() code. In the test >> setup, this is exercising xfs code since root is xfs. >> >> # perf record ./a.out >> # perf report -v |grep "xfs.ko" >> 0.76% a.out /lib/modules/6.5.0-rc3+/kernel/fs/xfs/xfs.ko 0xc00807de5efc >> B [k] xlog_cil_commit >> 0.74% a.out /lib/modules/6.5.0-rc3+/kernel/fs/xfs/xfs.ko 0xc00807d5ae18 >> B [k] xfs_btree_key_offset >> 0.74% a.out /lib/modules/6.5.0-rc3+/kernel/fs/xfs/xfs.ko 0xc00807e11fd4 >> B [k] 0x00112074 >> >> Here addr "0xc00807e11fd4" is not resolved. since this is a >> kernel module, its offset is from the DSO. Xfs module is loaded >> at 0xc00807d0 >> >> # cat /proc/modules | grep xfs >> xfs 2228224 3 - Live 0xc00807d0 >> >> And size is 0x22. So its loaded between ߦ0xc00807d0 >> and 0xc00807f2. From objdump, text section is: >> text 0010f7bc 00a0 2**4 >> >> Hence perf captured ip maps to 0x112074 which is: >> ( ip - start of module ) + a0 >> >> This offset 0x112074 falls out .text section which is up to 0x10f7bc >> In this case for module, the address 0xc00807e11fd4 is pointing >> to stub instructions. This address range represents the module stubs >> which is allocated on module load and hence is not part of DSO offset. >> >> To address this issue in "object code reading", skip the sample if >> address falls out of text section and is within the module end. >> Use the "text_end" member of "struct dso" to do this check. >> >> To address this issue in "perf report", exploring an option of >> having stubs range as part of the /proc/kallsyms, so that perf >> report can resolve addresses in stubs range >> >> However this patch uses text_end to skip the stub range for >> Object code reading testcase. >> >> Reported-by: Disha Goel >> Signed-off-by: Athira Rajeev >> --- >> Changelog: >> v1 -> v2: >> Updated comment to add description on which arch has stub and >> reason for skipping as suggested by Adrian Thanks for testing Disha. Hi Adrian, Can you please review and share feedback on this version. Thanks Athira > With this patch applied perf Object code reading test works correctly. > > 26: Object code reading : Ok > > Tested-by: Disha Goel > >> tools/perf/tests/code-reading.c | 12 >> 1 file changed, 12 insertions(+) >> >> diff --git a/tools/perf/tests/code-reading.c >> b/tools/perf/tests/code-reading.c >> index ed3815163d1b..3cf6c2d42416 100644 >> --- a/tools/perf/tests/code-reading.c >> +++ b/tools/perf/tests/code-reading.c >> @@ -269,6 +269,18 @@ static int read_object_code(u64 addr, size_t len, u8 >> cpumode, >> if (addr + len > map__end(al.map)) >> len = map__end(al.map) - addr; >> >> + /* >> + * Some architectures (ex: powerpc) have stubs (trampolines) in kernel >> + * modules to manage long jumps. Check if the ip offset falls in stubs >> + * sections for kernel modules. And skip module address after text end >> + */ >> + if (strstr(dso->long_name, ".ko")) { >> + if (al.addr > dso->text_end) { >> + pr_debug("skipping the module address %#"PRIx64" after text end\n", >> al.addr); >> + goto out; >> + } >> + } >> + >> /* Read the object code using perf */ >> ret_len = dso__data_read_offset(dso, maps__machine(thread__maps(thread)), >> al.addr, buf1, len); >>
Re: [PATCH V3] tools/perf: Add includes for detected configs in Makefile.perf
> On 13-Sep-2023, at 1:06 AM, Arnaldo Carvalho de Melo wrote: > > Em Tue, Sep 12, 2023 at 07:00:00AM -0700, Ian Rogers escreveu: >> On Mon, Sep 11, 2023 at 11:38 PM Athira Rajeev >> wrote: >>> >>> Makefile.perf uses "CONFIG_*" checks in the code. Example the config >>> for libtraceevent is used to set PYTHON_EXT_SRCS >>> >>>ifeq ($(CONFIG_LIBTRACEEVENT),y) >>> PYTHON_EXT_SRCS := $(shell grep -v ^\# util/python-ext-sources) >>>else >>> PYTHON_EXT_SRCS := $(shell grep -v '^\#\|util/trace-event.c' >>> util/python-ext-sources) >>>endif >>> >>> But this is not picking the value for CONFIG_LIBTRACEEVENT that is >>> set using the settings in Makefile.config. Include the file >>> ".config-detected" so that make will use the system detected >>> configuration in the CONFIG checks. This will fix isues that >>> could arise when other "CONFIG_*" checks are added to Makefile.perf >>> in future as well. >>> >>> Signed-off-by: Athira Rajeev >> >> Reviewed-by: Ian Rogers > > Thanks, applied. > > - Arnaldo > Thanks Ian for the review and thanks Arnaldo for picking this fix Athira > >> Thanks, >> Ian >> >>> --- >>> Changelog: >>> v2 -> v3: >>> Added -include since in some cases make clean or make >>> will fail when config is not included and if config-detected >>> file is not present. >>> >>> v1 -> v2: >>> Added $(OUTPUT) prefix to config-detected as pointed >>> out by Ian >>> >>> tools/perf/Makefile.perf | 3 +++ >>> 1 file changed, 3 insertions(+) >>> >>> diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf >>> index 37af6df7b978..f6fdc2d5a92f 100644 >>> --- a/tools/perf/Makefile.perf >>> +++ b/tools/perf/Makefile.perf >>> @@ -351,6 +351,9 @@ export PYTHON_EXTBUILD_LIB PYTHON_EXTBUILD_TMP >>> >>> python-clean := $(call QUIET_CLEAN, python) $(RM) -r $(PYTHON_EXTBUILD) >>> $(OUTPUT)python/perf*.so >>> >>> +# Use the detected configuration >>> +-include $(OUTPUT).config-detected >>> + >>> ifeq ($(CONFIG_LIBTRACEEVENT),y) >>> PYTHON_EXT_SRCS := $(shell grep -v ^\# util/python-ext-sources) >>> else >>> -- >>> 2.31.1 >>> > > -- > > - Arnaldo
[PATCH 2/2] tools/perf: Add perf binary dependent rule for shellcheck log in Makefile.perf
Add rule in new Makefile "tests/Makefile.tests" for running shellcheck on shell test scripts. This automates below shellcheck into the build. $ for F in $(find tests/shell/ -perm -o=x -name '*.sh'); do shellcheck -S warning $F; done CONFIG_SHELLCHECK check is added to avoid build breakage in the absence of shellcheck binary. Update Makefile.perf to contain new rule for "SHELLCHECK_TEST" which is for making shellcheck test as a dependency on perf binary. Added "tests/Makefile.tests" to run shellcheck on shellscripts in tests/shell. The make rule "SHLLCHECK_RUN" ensures that, every time during make, shellcheck will be run only on modified files during subsequent invocations. By this, if any newly added shell scripts or fixes in existing scripts breaks coding/formatting style, it will get captured during the perf build. Signed-off-by: Athira Rajeev --- tools/perf/Makefile.perf| 12 +++- tools/perf/tests/Makefile.tests | 24 2 files changed, 35 insertions(+), 1 deletion(-) create mode 100644 tools/perf/tests/Makefile.tests diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf index f6fdc2d5a92f..c27f54771e90 100644 --- a/tools/perf/Makefile.perf +++ b/tools/perf/Makefile.perf @@ -667,7 +667,16 @@ $(PERF_IN): prepare FORCE $(PMU_EVENTS_IN): FORCE prepare $(Q)$(MAKE) -f $(srctree)/tools/build/Makefile.build dir=pmu-events obj=pmu-events -$(OUTPUT)perf: $(PERFLIBS) $(PERF_IN) $(PMU_EVENTS_IN) +# Runs shellcheck on perf test shell scripts +ifeq ($(CONFIG_SHELLCHECK),y) +SHELLCHECK_TEST: FORCE prepare + $(Q)$(MAKE) -f $(srctree)/tools/perf/tests/Makefile.tests +else +SHELLCHECK_TEST: + @: +endif + +$(OUTPUT)perf: $(PERFLIBS) $(PERF_IN) $(PMU_EVENTS_IN) SHELLCHECK_TEST $(QUIET_LINK)$(CC) $(CFLAGS) $(LDFLAGS) \ $(PERF_IN) $(PMU_EVENTS_IN) $(LIBS) -o $@ @@ -1129,6 +1138,7 @@ bpf-skel-clean: $(call QUIET_CLEAN, bpf-skel) $(RM) -r $(SKEL_TMP_OUT) $(SKELETONS) clean:: $(LIBAPI)-clean $(LIBBPF)-clean $(LIBSUBCMD)-clean $(LIBSYMBOL)-clean $(LIBPERF)-clean fixdep-clean python-clean bpf-skel-clean tests-coresight-targets-clean + $(Q)$(MAKE) -f $(srctree)/tools/perf/tests/Makefile.tests clean $(call QUIET_CLEAN, core-objs) $(RM) $(LIBPERF_A) $(OUTPUT)perf-archive $(OUTPUT)perf-iostat $(LANG_BINDINGS) $(Q)find $(or $(OUTPUT),.) -name '*.o' -delete -o -name '\.*.cmd' -delete -o -name '\.*.d' -delete $(Q)$(RM) $(OUTPUT).config-detected diff --git a/tools/perf/tests/Makefile.tests b/tools/perf/tests/Makefile.tests new file mode 100644 index ..e74575559e83 --- /dev/null +++ b/tools/perf/tests/Makefile.tests @@ -0,0 +1,24 @@ +# SPDX-License-Identifier: GPL-2.0 +# Athira Rajeev , 2023 +-include $(OUTPUT).config-detected + +log_file = $(OUTPUT)shellcheck_test.log +PROGS = $(subst ./,,$(shell find tests/shell -perm -o=x -type f -name '*.sh')) +DEPS = $(addprefix output/,$(addsuffix .dep,$(basename $(PROGS +DIRS = $(shell echo $(dir $(DEPS)) | xargs -n1 | sort -u | xargs) + +.PHONY: all +all: SHELLCHECK_RUN + @: + +SHELLCHECK_RUN: $(DEPS) $(DIRS) + +output/%.dep: %.sh | $(DIRS) + $(call rule_mkdir) + $(Q)$(call frecho-cmd,test)@touch $@ + $(Q)$(call frecho-cmd,test)@shellcheck -S warning $(subst output/,./,$(patsubst %.dep, %.sh, $@)) 1> ${log_file} && ([[ ! -s ${log_file} ]]) +$(DIRS): + @mkdir -p $@ + +clean: + @rm -rf $(log_file) output -- 2.31.1
[PATCH 1/2] tools/perf: Add new CONFIG_SHELLCHECK for detecting shellcheck binary
shellcheck tool can detect coding/formatting issues on shell scripts. In perf directory "tests/shell", there are lot of shell test scripts and this tool can detect coding/formatting issues on these scripts. Example to use shellcheck for severity level for errors and warnings, below command is used: # for F in $(find tests/shell/ -perm -o=x -name '*.sh'); do shellcheck -S warning $F; done # echo $? 0 This testing needs to be automated into the build so that it can avoid regressions and also run the check for newly added during build test itself. Add a new feature check to detect presence of shellcheck. Add CONFIG_SHELLCHECK feature check in the build to avoid not having shellcheck breaking the build. Signed-off-by: Athira Rajeev --- tools/build/Makefile.feature | 6 -- tools/build/feature/Makefile | 8 +++- tools/perf/Makefile.config | 10 ++ 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature index 934e2777a2db..23f56b95babf 100644 --- a/tools/build/Makefile.feature +++ b/tools/build/Makefile.feature @@ -72,7 +72,8 @@ FEATURE_TESTS_BASIC := \ libzstd\ disassembler-four-args \ disassembler-init-styled \ -file-handle +file-handle\ +shellcheck # FEATURE_TESTS_BASIC + FEATURE_TESTS_EXTRA is the complete list # of all feature tests @@ -138,7 +139,8 @@ FEATURE_DISPLAY ?= \ get_cpuid \ bpf \ libaio\ - libzstd + libzstd \ + shellcheck # # Declare group members of a feature to display the logical OR of the detection diff --git a/tools/build/feature/Makefile b/tools/build/feature/Makefile index 3184f387990a..44ba6d0c98d0 100644 --- a/tools/build/feature/Makefile +++ b/tools/build/feature/Makefile @@ -76,7 +76,8 @@ FILES= \ test-libzstd.bin \ test-clang-bpf-co-re.bin \ test-file-handle.bin \ - test-libpfm4.bin + test-libpfm4.bin \ + test-shellcheck.bin FILES := $(addprefix $(OUTPUT),$(FILES)) @@ -92,6 +93,8 @@ __BUILD = $(CC) $(CFLAGS) -MD -Wall -Werror -o $@ $(patsubst %.bin,%.c,$(@F)) $( __BUILDXX = $(CXX) $(CXXFLAGS) -MD -Wall -Werror -o $@ $(patsubst %.bin,%.cpp,$(@F)) $(LDFLAGS) BUILDXX = $(__BUILDXX) > $(@:.bin=.make.output) 2>&1 + BUILD_BINARY = sh -c $1 > $(@:.bin=.make.output) 2>&1 + ### $(OUTPUT)test-all.bin: @@ -207,6 +210,9 @@ $(OUTPUT)test-libslang-include-subdir.bin: $(OUTPUT)test-libtraceevent.bin: $(BUILD) -ltraceevent +$(OUTPUT)test-shellcheck.bin: + $(BUILD_BINARY) "shellcheck --version" + $(OUTPUT)test-libtracefs.bin: $(BUILD) $(shell $(PKG_CONFIG) --cflags libtraceevent 2>/dev/null) -ltracefs diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config index d66b52407e19..e71fe95ad865 100644 --- a/tools/perf/Makefile.config +++ b/tools/perf/Makefile.config @@ -779,6 +779,16 @@ ifndef NO_SLANG endif endif +ifneq ($(NO_SHELLCHECK),1) + $(call feature_check,shellcheck) + ifneq ($(feature-shellcheck), 1) +msg := $(warning No shellcheck found. please install ShellCheck); + else +$(call detected,CONFIG_SHELLCHECK) +NO_SHELLCHECK := 0 + endif +endif + ifdef GTK2 FLAGS_GTK2=$(CFLAGS) $(LDFLAGS) $(EXTLIBS) $(shell $(PKG_CONFIG) --libs --cflags gtk+-2.0 2>/dev/null) $(call feature_check,gtk2) -- 2.31.1
[PATCH 3/3] skiboot: Update IMC PMU node names for power10
The nest IMC (In Memory Collection) Performance Monitoring Unit(PMU) node names are saved as "struct nest_pmus_struct" in the "hw/imc.c" IMC code. Not all the IMC PMUs listed in the device tree may be available. Nest IMC PMU names along with their bit values is represented in imc availability vector. This struct is used to remove the unavailable nodes by checking this vector. For power10, the imc_chip_avl_vector ie, imc availability vector ( which is a part of the IMC control block structure ), has change in mapping of units and bit positions. Hence rename the existing nest_pmus array to nest_pmus_p9 and add entry for power10 as nest_pmus_p10. Also the avl_vector has another change in bit positions 11:34. These bit positions tells the availability of Xlink/Alink/CAPI. There are total 8 links and three bit field combination says which link is available. Patch implements all these change to handle nest_pmus_p10. Signed-off-by: Athira Rajeev --- Changelog: v5 -> v6: - Addressed review comment from Reza by using PPC_BIT instead of PPC_BITMASK v4 -> v5: - Addressed review comment from Reza and renamed dt_find_by_name_substr to dt_find_by_name_before_addr v3 -> v4: - Addressed review comment from Mahesh and added his Reviewed-by for patch 1. v2 -> v3: - After review comments from Mahesh, fixed the code to consider string upto "@" for both input node name as well as child node name. V2 version was comparing input node name and child node name upto string length of child name. But this will return wrong node if input name is larger than child name. Because it will match as substring for child name. https://lists.ozlabs.org/pipermail/skiboot/2023-January/018596.html v1 -> v2: - Addressed review comment from Dan to update the utility funtion to search and compare upto "@". Renamed it as dt_find_by_name_substr. hw/imc.c | 196 --- 1 file changed, 186 insertions(+), 10 deletions(-) diff --git a/hw/imc.c b/hw/imc.c index 73f25dae8..9f59348ad 100644 --- a/hw/imc.c +++ b/hw/imc.c @@ -49,7 +49,7 @@ static unsigned int *htm_scom_index; * imc_chip_avl_vector(in struct imc_chip_cb, look at include/imc.h). * nest_pmus[] is an array containing all the possible nest IMC PMU node names. */ -static char const *nest_pmus[] = { +static const char *nest_pmus_p9[] = { "powerbus0", "mcs0", "mcs1", @@ -104,6 +104,67 @@ static char const *nest_pmus[] = { /* reserved bits : 51 - 63 */ }; +static const char *nest_pmus_p10[] = { + "pb", + "mcs0", + "mcs1", + "mcs2", + "mcs3", + "mcs4", + "mcs5", + "mcs6", + "mcs7", + "pec0", + "pec1", + "NA", + "NA", + "NA", + "NA", + "NA", + "NA", + "NA", + "NA", + "NA", + "NA", + "NA", + "NA", + "NA", + "NA", + "NA", + "NA", + "NA", + "NA", + "NA", + "NA", + "NA", + "NA", + "NA", + "NA", + "phb0", + "phb1", + "phb2", + "phb3", + "phb4", + "phb5", + "ocmb0", + "ocmb1", + "ocmb2", + "ocmb3", + "ocmb4", + "ocmb5", + "ocmb6", + "ocmb7", + "ocmb8", + "ocmb9", + "ocmb10", + "ocmb11", + "ocmb12", + "ocmb13", + "ocmb14", + "ocmb15", + "nx", +}; + /* * Due to Nest HW/OCC restriction, microcode will not support individual unit * events for these nest units mcs0, mcs1 ... mcs7 in the accumulation mode. @@ -371,7 +432,7 @@ static void disable_unavailable_units(struct dt_node *dev) uint64_t avl_vec; struct imc_chip_cb *cb; struct dt_node *target; - int i; + int i, j; bool disable_all_nests = false; struct proc_chip *chip; @@ -409,14 +470,129 @@ static void disable_unavailable_units(struct dt_node *dev) avl_vec = (0xffULL) << 56; } - for (i = 0; i < ARRAY_SIZE(nest_pmus); i++) { - if (!(PPC_BITMASK(i, i) & avl_vec)) { - /* Check if the device node exists */ - target = dt_find_by_name_before_addr(dev, nest_pmus[i]); - if (!target) - continue; - /* Remove the device node */ - dt_free(target); + if (proc_gen == proc_gen_p9) { + for (i = 0; i < ARRAY_SIZE(nest_pmus_p9); i++) { + if (!(PPC_BIT(i) & avl_vec)) { + /* Check if the device node exists */ + target = dt_find_by_name_before_addr(dev, nest_pmus_p9[i]); + if (!target) + continue; + /* Remove the device node */ +
[PATCH 2/3] skiboot: Update IMC code to use dt_find_by_name_before_addr for checking dt nodes
The nest IMC (In Memory Collection) Performance Monitoring Unit(PMU) node names are saved in nest_pmus[] array in the "hw/imc.c" IMC code. Not all the IMC PMUs listed in the device tree may be available. Nest IMC PMU names along with their bit values is represented in imc availability vector. The nest_pmus[] array is used to remove the unavailable nodes by checking this vector. To check node availability, code was using "dt_find_by_substr". But since the node names have format like: "name@offset", dt_find_by_name doesn't return the expected result. Fix this by using dt_find_by_name_before_addr. Also, update the char array to use correct node names. Signed-off-by: Athira Rajeev --- Changelog: v4 -> v5: - Addressed review comment from Reza and renamed dt_find_by_name_substr to dt_find_by_name_before_addr v3 -> v4: - Addressed review comment from Mahesh and added his Reviewed-by for patch 1. v2 -> v3: - After review comments from Mahesh, fixed the code to consider string upto "@" for both input node name as well as child node name. V2 version was comparing input node name and child node name upto string length of child name. But this will return wrong node if input name is larger than child name. Because it will match as substring for child name. https://lists.ozlabs.org/pipermail/skiboot/2023-January/018596.html v1 -> v2: - Addressed review comment from Dan to update the utility funtion to search and compare upto "@". Renamed it as dt_find_by_name_substr. hw/imc.c | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/hw/imc.c b/hw/imc.c index 97e0809f0..73f25dae8 100644 --- a/hw/imc.c +++ b/hw/imc.c @@ -67,14 +67,14 @@ static char const *nest_pmus[] = { "mba5", "mba6", "mba7", - "cen0", - "cen1", - "cen2", - "cen3", - "cen4", - "cen5", - "cen6", - "cen7", + "centaur0", + "centaur1", + "centaur2", + "centaur3", + "centaur4", + "centaur5", + "centaur6", + "centaur7", "xlink0", "xlink1", "xlink2", @@ -412,7 +412,7 @@ static void disable_unavailable_units(struct dt_node *dev) for (i = 0; i < ARRAY_SIZE(nest_pmus); i++) { if (!(PPC_BITMASK(i, i) & avl_vec)) { /* Check if the device node exists */ - target = dt_find_by_name(dev, nest_pmus[i]); + target = dt_find_by_name_before_addr(dev, nest_pmus[i]); if (!target) continue; /* Remove the device node */ -- 2.31.1
[PATCH 1/3] core/device: Add function to return child node using name at substring "@"
Add a function dt_find_by_name_before_addr() that returns the child node if it matches till first occurrence at "@" of a given name, otherwise NULL. This is helpful for cases with node name like: "name@addr". In scenarios where nodes are added with "name@addr" format and if the value of "addr" is not known, that node can't be matched with node name or addr. Hence matching with substring as node name will return the expected result. Patch adds dt_find_by_name_before_addr() function and testcase for the same in core/test/run-device.c Signed-off-by: Athira Rajeev Reviewed-by: Mahesh Salgaonkar --- Changelog: v5 -> v6: - Addressed review comment from Reza. Instead of using new variable for "node", use the node "name" as-is since the utility is to check the name before addr. Updated the test/run-device.c accordingly v4 -> v5: - Addressed review comment from Reza and renamed dt_find_by_name_substr to dt_find_by_name_before_addr v3 -> v4: - Addressed review comment from Mahesh and added his Reviewed-by. v2 -> v3: - After review comments from Mahesh, fixed the code to consider string upto "@" for both input node name as well as child node name. V2 version was comparing input node name and child node name upto string length of child name. But this will return wrong node if input name is larger than child name. Because it will match as substring for child name. https://lists.ozlabs.org/pipermail/skiboot/2023-January/018596.html v1 -> v2: - Addressed review comment from Dan to update the utility funtion to search and compare upto "@". Renamed it as dt_find_by_name_substr. core/device.c | 25 + core/test/run-device.c | 14 ++ include/device.h | 3 +++ 3 files changed, 42 insertions(+) diff --git a/core/device.c b/core/device.c index 2de37c741..c22b6b3c3 100644 --- a/core/device.c +++ b/core/device.c @@ -395,6 +395,31 @@ struct dt_node *dt_find_by_name(struct dt_node *root, const char *name) } +struct dt_node *dt_find_by_name_before_addr(struct dt_node *root, const char *name) +{ + struct dt_node *child, *match; + char *child_node = NULL; + + list_for_each(>children, child, list) { + child_node = strdup(child->name); + if (!child_node) + goto err; + child_node = strtok(child_node, "@"); + if (!strcmp(child_node, name)) { + free(child_node); + return child; + } + + match = dt_find_by_name_before_addr(child, name); + if (match) + return match; + } + + free(child_node); +err: + return NULL; +} + struct dt_node *dt_new_check(struct dt_node *parent, const char *name) { struct dt_node *node = dt_find_by_name(parent, name); diff --git a/core/test/run-device.c b/core/test/run-device.c index 4a12382bb..fb7a7d2c0 100644 --- a/core/test/run-device.c +++ b/core/test/run-device.c @@ -466,6 +466,20 @@ int main(void) new_prop_ph = dt_prop_get_u32(ut2, "something"); assert(!(new_prop_ph == ev1_ph)); dt_free(subtree); + + /* Test dt_find_by_name_before_addr */ + root = dt_new_root(""); + addr1 = dt_new_addr(root, "node", 0x1); + addr2 = dt_new_addr(root, "node0_1", 0x2); + assert(dt_find_by_name(root, "node@1") == addr1); + assert(dt_find_by_name(root, "node0_1@2") == addr2); + assert(dt_find_by_name_before_addr(root, "node") == addr1); + assert(dt_find_by_name_before_addr(root, "node0_") == NULL); + assert(dt_find_by_name_before_addr(root, "node0_1") == addr2); + assert(dt_find_by_name_before_addr(root, "node0") == NULL); + assert(dt_find_by_name_before_addr(root, "node0_") == NULL); + dt_free(root); + return 0; } diff --git a/include/device.h b/include/device.h index 93fb90ff4..f2402cc4d 100644 --- a/include/device.h +++ b/include/device.h @@ -184,6 +184,9 @@ struct dt_node *dt_find_by_path(struct dt_node *root, const char *path); /* Find a child node by name */ struct dt_node *dt_find_by_name(struct dt_node *root, const char *name); +/* Find a child node by name and substring */ +struct dt_node *dt_find_by_name_before_addr(struct dt_node *root, const char *name); + /* Find a node by phandle */ struct dt_node *dt_find_by_phandle(struct dt_node *root, u32 phandle); -- 2.31.1
Re: [PATCH 1/1] powerpc: fix a memory leak
On 9/14/23 02:46, Yuanjun Gong wrote: > When one of the methods xive_native_alloc_irq_on_chip, irq_create_mapping > or irq_get_handler_data fails, the function will directly return without > disposing vinst->name and vinst. Fix it. > > Fixes: c20e1e299d93 ("powerpc/vas: Alloc and setup IRQ and trigger port > address") > Signed-off-by: Yuanjun Gong > --- > arch/powerpc/platforms/powernv/vas.c | 14 +- > 1 file changed, 9 insertions(+), 5 deletions(-) > > diff --git a/arch/powerpc/platforms/powernv/vas.c > b/arch/powerpc/platforms/powernv/vas.c > index b65256a63e87..780740b478f0 100644 > --- a/arch/powerpc/platforms/powernv/vas.c > +++ b/arch/powerpc/platforms/powernv/vas.c > @@ -54,7 +54,7 @@ static int init_vas_instance(struct platform_device *pdev) > struct xive_irq_data *xd; > uint32_t chipid, hwirq; > struct resource *res; > - int rc, cpu, vasid; > + int rc, cpu, vasid, ret; You can you reuse rc for the return value in the error path instead of introducing a new ret variable. -Tyrel > > rc = of_property_read_u32(dn, "ibm,vas-id", ); > if (rc) { > @@ -102,6 +102,7 @@ static int init_vas_instance(struct platform_device *pdev) > res = >resource[3]; > if (res->end > 62) { > pr_err("Bad 'paste_win_id_shift' in DT, %llx\n", res->end); > + ret = -ENODEV > goto free_vinst; > } > > @@ -111,21 +112,24 @@ static int init_vas_instance(struct platform_device > *pdev) > if (!hwirq) { > pr_err("Inst%d: Unable to allocate global irq for chip %d\n", > vinst->vas_id, chipid); > - return -ENOENT; > + ret = -ENOENT; > + goto free_vinst; > } > > vinst->virq = irq_create_mapping(NULL, hwirq); > if (!vinst->virq) { > pr_err("Inst%d: Unable to map global irq %d\n", > vinst->vas_id, hwirq); > - return -EINVAL; > + ret = -EINVAL; > + goto free_vinst; > } > > xd = irq_get_handler_data(vinst->virq); > if (!xd) { > pr_err("Inst%d: Invalid virq %d\n", > vinst->vas_id, vinst->virq); > - return -EINVAL; > + ret = -EINVAL; > + goto free_vinst; > } > > vinst->irq_port = xd->trig_page; > @@ -168,7 +172,7 @@ static int init_vas_instance(struct platform_device *pdev) > free_vinst: > kfree(vinst->name); > kfree(vinst); > - return -ENODEV; > + return ret; > > } >
[PATCH] powerpc/82xx: Select FSL_SOC
It used to be impossible to select CONFIG_CPM2 without selecting CONFIG_FSL_SOC at the same time because CONFIG_CPM2 was dependent on CONFIG_8260 and CONFIG_8260 was selecting CONFIG_FSL_SOC. But after commit eb5aa2137275 ("powerpc/82xx: Remove CONFIG_8260 and CONFIG_8272") CONFIG_CPM2 depends on CONFIG_MPC82xx instead but CONFIG_MPC82xx doesn't directly selects CONFIG_FSL_SOC. Fix it by forcing CONFIG_MPC82xx to select CONFIG_FSL_SOC just like already done by MPC8xx, MPC512x, MPC83xx, PPC_86xx. Reported-by: Randy Dunlap Fixes: eb5aa2137275 ("powerpc/82xx: Remove CONFIG_8260 and CONFIG_8272") Signed-off-by: Christophe Leroy --- arch/powerpc/platforms/82xx/Kconfig | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/arch/powerpc/platforms/82xx/Kconfig b/arch/powerpc/platforms/82xx/Kconfig index d9f1a2a83158..1824536cf6f2 100644 --- a/arch/powerpc/platforms/82xx/Kconfig +++ b/arch/powerpc/platforms/82xx/Kconfig @@ -2,6 +2,7 @@ menuconfig PPC_82xx bool "82xx-based boards (PQ II)" depends on PPC_BOOK3S_32 + select FSL_SOC if PPC_82xx @@ -9,7 +10,6 @@ config EP8248E bool "Embedded Planet EP8248E (a.k.a. CWH-PPC-8248N-VE)" select CPM2 select PPC_INDIRECT_PCI if PCI - select FSL_SOC select PHYLIB if NETDEVICES select MDIO_BITBANG if PHYLIB help @@ -22,7 +22,6 @@ config MGCOGE bool "Keymile MGCOGE" select CPM2 select PPC_INDIRECT_PCI if PCI - select FSL_SOC help This enables support for the Keymile MGCOGE board. -- 2.41.0
Re: [PATCH] powerpc: add `cur_cpu_spec` symbol to vmcoreinfo
> On 14-Sep-2023, at 6:52 PM, Michael Ellerman wrote: > > Sachin Sant writes: >>> On 11-Sep-2023, at 2:44 PM, Aditya Gupta wrote: >>> >>> Presently, while reading a vmcore, makedumpfile uses >>> `cur_cpu_spec.mmu_features` to decide whether the crashed system had >>> RADIX MMU or not. >>> >>> Currently, makedumpfile fails to get the `cur_cpu_spec` symbol (unless >>> a vmlinux is passed with the `-x` flag to makedumpfile), and hence >>> assigns offsets and shifts (such as pgd_offset_l4) incorrecly considering >>> MMU to be hash MMU. >>> >>> Add `cur_cpu_spec` symbol and offset of `mmu_features` in the >>> `cpu_spec` struct, to VMCOREINFO, so that the symbol address and offset >>> is accessible to makedumpfile, without needing the vmlinux file >>> >>> Signed-off-by: Aditya Gupta >>> --- >> >> Thanks for the patch. With this patch applied (along with makedumpfile >> changes) >> I am able to capture vmcore against a kernel which contains commit >> 8dc9a0ad0c3e > > I can't find that commit? Was just wondering if it should be referenced > in the commit message. > My bad, I copied that commit id from the email when I first reported this issue against linux-next. The commit should be 368a0590d954: (powerpc/book3s64/vmemmap: switch radix to use a different vmemmap handling function)
Re: [PATCH v2] ASoC: imx-rpmsg: Set ignore_pmdown_time for dai_link
On Wed, 13 Sep 2023 18:26:56 +0800, Chancel Liu wrote: > i.MX rpmsg sound cards work on codec slave mode. MCLK will be disabled > by CPU DAI driver in hw_free(). Some codec requires MCLK present at > power up/down sequence. So need to set ignore_pmdown_time to power down > codec immediately before MCLK is turned off. > > Take WM8962 as an example, if MCLK is disabled before DAPM power down > playback stream, FIFO error will arise in WM8962 which will have bad > impact on playback next. > > [...] Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next Thanks! [1/1] ASoC: imx-rpmsg: Set ignore_pmdown_time for dai_link commit: fac58baf8fcfcd7481e8f6d60206ce2a47c1476c All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark
Re: [RFC PATCH v12 02/33] KVM: Use gfn instead of hva for mmu_notifier_retry
On Thu, Sep 14, 2023, Binbin Wu wrote: > > On 9/14/2023 9:55 AM, Sean Christopherson wrote: > > +void kvm_mmu_invalidate_end(struct kvm *kvm) > > { > > /* > > * This sequence increase will notify the kvm page fault that > > @@ -833,6 +848,13 @@ void kvm_mmu_invalidate_end(struct kvm *kvm, unsigned > > long start, > > * in conjunction with the smp_rmb in mmu_invalidate_retry(). > > */ > > kvm->mmu_invalidate_in_progress--; > > + > > + /* > > +* Assert that at least one range must be added between start() and > > +* end(). Not adding a range isn't fatal, but it is a KVM bug. > > +*/ > > + WARN_ON_ONCE(kvm->mmu_invalidate_in_progress && > > +kvm->mmu_invalidate_range_start == INVALID_GPA); > Should the check happen before the decrease of > kvm->mmu_invalidate_in_progress? > Otherwise, KVM calls kvm_mmu_invalidate_begin(), then kvm_mmu_invalidate_end() > the check will not take effect. Indeed. I'm pretty sure I added this code, not sure what I was thinking. There's no reason to check mmu_invalidate_in_progress, it's not like KVM allows mmu_invalidate_in_progress to go negative. The comment is also a bit funky. I'll post a fixup patch to make it look like this (assuming I'm not forgetting a subtle reason for guarding the check with the in-progress flag): /* * Assert that at least one range was added between start() and end(). * Not adding a range isn't fatal, but it is a KVM bug. */ WARN_ON_ONCE(kvm->mmu_invalidate_range_start == INVALID_GPA); Regarding kvm->mmu_invalidate_in_progress, this would be a good opportunity to move the BUG_ON() into the common end(), e.g. as is, an end() without a start() from something other than the generic mmu_notifier would go unnoticed. And I _think_ we can replace the BUG_ON() with a KVM_BUG_ON() without putting the kernel at risk. E.g. diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index dd948276e5d6..54480655bcce 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -870,6 +870,7 @@ void kvm_mmu_invalidate_end(struct kvm *kvm) * in conjunction with the smp_rmb in mmu_invalidate_retry(). */ kvm->mmu_invalidate_in_progress--; + KVM_BUG_ON(kvm->mmu_invalidate_in_progress < 0, kvm); /* * Assert that at least one range was added between start() and end(). @@ -905,8 +906,6 @@ static void kvm_mmu_notifier_invalidate_range_end(struct mmu_notifier *mn, */ if (wake) rcuwait_wake_up(>mn_memslots_update_rcuwait); - - BUG_ON(kvm->mmu_invalidate_in_progress < 0); } static int kvm_mmu_notifier_clear_flush_young(struct mmu_notifier *mn,
Re: [PATCH] powerpc/dexcr: Move HASHCHK trap handler
Benjamin Gray writes: > To determine if a trap was caused by a HASHCHK instruction, we inspect > the user instruction that triggered the trap. However this may sleep > if the page needs to be faulted in. It would be good to include the output from the might_sleep() check that failed. And I think this was found by syzkaller? If so we should say so. cheers > Move the HASHCHK handler logic to after we allow IRQs, which is fine > because we are only interested in HASHCHK if it's a user space trap. > > Fixes: 5bcba4e6c13f ("powerpc/dexcr: Handle hashchk exception") > Signed-off-by: Benjamin Gray > --- > arch/powerpc/kernel/traps.c | 56 - > 1 file changed, 36 insertions(+), 20 deletions(-) > > diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c > index f5ce282dc4b8..32b5e841ea97 100644 > --- a/arch/powerpc/kernel/traps.c > +++ b/arch/powerpc/kernel/traps.c > @@ -1512,23 +1512,11 @@ static void do_program_check(struct pt_regs *regs) > return; > } > > - if (cpu_has_feature(CPU_FTR_DEXCR_NPHIE) && user_mode(regs)) { > - ppc_inst_t insn; > - > - if (get_user_instr(insn, (void __user *)regs->nip)) { > - _exception(SIGSEGV, regs, SEGV_MAPERR, > regs->nip); > - return; > - } > - > - if (ppc_inst_primary_opcode(insn) == 31 && > - get_xop(ppc_inst_val(insn)) == OP_31_XOP_HASHCHK) { > - _exception(SIGILL, regs, ILL_ILLOPN, regs->nip); > - return; > - } > + /* User mode considers other cases after enabling IRQs */ > + if (!user_mode(regs)) { > + _exception(SIGTRAP, regs, TRAP_BRKPT, regs->nip); > + return; > } > - > - _exception(SIGTRAP, regs, TRAP_BRKPT, regs->nip); > - return; > } > #ifdef CONFIG_PPC_TRANSACTIONAL_MEM > if (reason & REASON_TM) { > @@ -1561,16 +1549,44 @@ static void do_program_check(struct pt_regs *regs) > > /* >* If we took the program check in the kernel skip down to sending a > - * SIGILL. The subsequent cases all relate to emulating instructions > - * which we should only do for userspace. We also do not want to enable > - * interrupts for kernel faults because that might lead to further > - * faults, and loose the context of the original exception. > + * SIGILL. The subsequent cases all relate to user space, such as > + * emulating instructions which we should only do for user space. We > + * also do not want to enable interrupts for kernel faults because that > + * might lead to further faults, and loose the context of the original > + * exception. >*/ > if (!user_mode(regs)) > goto sigill; > > interrupt_cond_local_irq_enable(regs); > > + /* > + * (reason & REASON_TRAP) is mostly handled before enabling IRQs, > + * except get_user_instr() can sleep so we cannot reliably inspect the > + * current instruction in that context. Now that we know we are > + * handling a user space trap and can sleep, we can check if the trap > + * was a hashchk failure. > + */ > + if (reason & REASON_TRAP) { > + if (cpu_has_feature(CPU_FTR_DEXCR_NPHIE)) { > + ppc_inst_t insn; > + > + if (get_user_instr(insn, (void __user *)regs->nip)) { > + _exception(SIGSEGV, regs, SEGV_MAPERR, > regs->nip); > + return; > + } > + > + if (ppc_inst_primary_opcode(insn) == 31 && > + get_xop(ppc_inst_val(insn)) == OP_31_XOP_HASHCHK) { > + _exception(SIGILL, regs, ILL_ILLOPN, regs->nip); > + return; > + } > + } > + > + _exception(SIGTRAP, regs, TRAP_BRKPT, regs->nip); > + return; > + } > + > /* (reason & REASON_ILLEGAL) would be the obvious thing here, >* but there seems to be a hardware bug on the 405GP (RevD) >* that means ESR is sometimes set incorrectly - either to > -- > 2.41.0
Re: [PATCH] powerpc: add `cur_cpu_spec` symbol to vmcoreinfo
Sachin Sant writes: >> On 11-Sep-2023, at 2:44 PM, Aditya Gupta wrote: >> >> Presently, while reading a vmcore, makedumpfile uses >> `cur_cpu_spec.mmu_features` to decide whether the crashed system had >> RADIX MMU or not. >> >> Currently, makedumpfile fails to get the `cur_cpu_spec` symbol (unless >> a vmlinux is passed with the `-x` flag to makedumpfile), and hence >> assigns offsets and shifts (such as pgd_offset_l4) incorrecly considering >> MMU to be hash MMU. >> >> Add `cur_cpu_spec` symbol and offset of `mmu_features` in the >> `cpu_spec` struct, to VMCOREINFO, so that the symbol address and offset >> is accessible to makedumpfile, without needing the vmlinux file >> >> Signed-off-by: Aditya Gupta >> --- > > Thanks for the patch. With this patch applied (along with makedumpfile > changes) > I am able to capture vmcore against a kernel which contains commit > 8dc9a0ad0c3e I can't find that commit? Was just wondering if it should be referenced in the commit message. cheers
Re: [PATCH] powerpc: add `cur_cpu_spec` symbol to vmcoreinfo
Aditya Gupta writes: > Presently, while reading a vmcore, makedumpfile uses > `cur_cpu_spec.mmu_features` to decide whether the crashed system had > RADIX MMU or not. > > Currently, makedumpfile fails to get the `cur_cpu_spec` symbol (unless > a vmlinux is passed with the `-x` flag to makedumpfile), and hence > assigns offsets and shifts (such as pgd_offset_l4) incorrecly considering > MMU to be hash MMU. > > Add `cur_cpu_spec` symbol and offset of `mmu_features` in the > `cpu_spec` struct, to VMCOREINFO, so that the symbol address and offset > is accessible to makedumpfile, without needing the vmlinux file This looks fine. Seems like cpu_features would be needed or at least pretty useful too? cheers
Re: [PATCH v3 1/3] powerpc/config: Cleanup pmac32_defconfig
Yuan Tan writes: > Use 'make savedefconfig' to cleanup pmac32_defconfig, based on Linux > 7.6-rc1 Thanks but I don't like doing these updates in a single commit like this, it's easy to accidentally lose a symbol. I prefer an explanation for what's changing for each symbol. See 1ce7fda142af ("powerpc/configs/64s: Drop IPV6 which is default y") and the commits leading up to it, to see what I mean. But I suspect you probably don't want to go to all that effort, which is fine :) So I won't take patch 1, but patch 2 and 3 look fine. No need to resend, I'll deal with any merge fixup needed. cheers
Re: KASAN debug kernel fails to boot at early stage when CONFIG_SMP=y is set (kernel 6.5-rc5, PowerMac G4 3,6)
On Thu, 14 Sep 2023 04:54:17 + Christophe Leroy wrote: > Le 12/09/2023 à 19:39, Christophe Leroy a écrit : > > > > > > Le 12/09/2023 à 17:59, Erhard Furtner a écrit : > >> > >> printk: bootconsole [udbg0] enabled > >> Total memory = 2048MB; using 4096kB for hash table > >> mapin_ram:125 > >> mmu_mapin_ram:169 0 3000 140 200 > >> __mmu_mapin_ram:146 0 140 > >> __mmu_mapin_ram:155 140 > >> __mmu_mapin_ram:146 140 3000 > >> __mmu_mapin_ram:155 2000 > >> __mapin_ram_chunk:107 2000 3000 > >> __mapin_ram_chunk:117 > >> mapin_ram:134 > >> kasan_mmu_init:129 > >> kasan_mmu_init:132 0 > >> kasan_mmu_init:137 > >> ioremap() called early from btext_map+0x64/0xdc. Use early_ioremap() > >> instead > >> Linux version 6.6.0-rc1-PMacG4-dirty (root@T1000) (gcc (Gentoo > >> 12.3.1_p20230526 p2) 12.3.1 20230526, GNU ld (Gentoo 2.40 p7) 2.40.0) #5 > >> SMP Tue Sep 12 16:50:47 CEST 2023 > >> kasan_init_region: c000 3000 f800 fe00 > >> kasan_init_region: loop f800 fe00 > >> > >> > >> So I get no "kasan_init_region: setbat" line and don't reach "KASAN init > >> done". > > > > Ah ok, maybe your CPU only has 4 BATs and they are all used, following > > change would tell us. > > > > diff --git a/arch/powerpc/mm/book3s32/mmu.c b/arch/powerpc/mm/book3s32/mmu.c > > index 850783cfa9c7..bd26767edce7 100644 > > --- a/arch/powerpc/mm/book3s32/mmu.c > > +++ b/arch/powerpc/mm/book3s32/mmu.c > > @@ -86,6 +86,7 @@ int __init find_free_bat(void) > > if (!(bat[1].batu & 3)) > > return b; > > } > > + pr_err("NO FREE BAT (%d)\n", n); > > return -1; > >} > > > > > > Or you have 8 BATs in which case it's an alignment problem, you need to > > increase CONFIG_DATA_SHIFT to 23, for that you need CONFIG_ADVANCED and > > CONFIG_DATA_SHIFT_BOOL > > > > But regardless of that there is a problem we need to find out, because > > it should work without BATs. > > > > As the BATs allocation fails, it falls back to : > > > > phys = memblock_phys_alloc_range(k_end - k_start, PAGE_SIZE, 0, > > MEMBLOCK_ALLOC_ANYWHERE); > > if (!phys) > > return -ENOMEM; > > } > > > > ret = kasan_init_shadow_page_tables(k_start, k_end); > > if (ret) > > return ret; > > > > for (k_cur = k_start; k_cur < k_end; k_cur += PAGE_SIZE) { > > pmd_t *pmd = pmd_off_k(k_cur); > > pte_t pte = pfn_pte(PHYS_PFN(phys + k_cur - k_start), > > PAGE_KERNEL); > > > > __set_pte_at(_mm, k_cur, pte_offset_kernel(pmd, k_cur), > > pte, 0); > > } > > flush_tlb_kernel_range(k_start, k_end); > > memset(kasan_mem_to_shadow(start), 0, k_end - k_start); > > > > > > While the __weak function that you confirmed working is: > > > > ret = kasan_init_shadow_page_tables(k_start, k_end); > > if (ret) > > return ret; > > > > block = memblock_alloc(k_end - k_start, PAGE_SIZE); > > if (!block) > > return -ENOMEM; > > > > for (k_cur = k_start & PAGE_MASK; k_cur < k_end; k_cur += PAGE_SIZE) { > > pmd_t *pmd = pmd_off_k(k_cur); > > void *va = block + k_cur - k_start; > > pte_t pte = pfn_pte(PHYS_PFN(__pa(va)), PAGE_KERNEL); > > > > __set_pte_at(_mm, k_cur, pte_offset_kernel(pmd, k_cur), > > pte, 0); > > } > > flush_tlb_kernel_range(k_start, k_end); > > > > > > I'm having hard time to understand what's could be wrong at the first place. > > > > Could you try following change: > > > > diff --git a/arch/powerpc/mm/kasan/book3s_32.c > > b/arch/powerpc/mm/kasan/book3s_32.c > > index 9954b7a3b7ae..e04f21908c6a 100644 > > --- a/arch/powerpc/mm/kasan/book3s_32.c > > +++ b/arch/powerpc/mm/kasan/book3s_32.c > > @@ -38,7 +38,7 @@ int __init kasan_init_region(void *start, size_t size) > > > > if (k_nobat < k_end) { > > phys = memblock_phys_alloc_range(k_end - k_nobat, PAGE_SIZE, 0, > > -MEMBLOCK_ALLOC_ANYWHERE); > > +MEMBLOCK_ALLOC_ACCESSIBLE); > > if (!phys) > > return -ENOMEM; > > } > > > > And also that one: > > > > > > diff --git a/arch/powerpc/mm/kasan/init_32.c > > b/arch/powerpc/mm/kasan/init_32.c > > index a70828a6d935..bc1c075489f4 100644 > > --- a/arch/powerpc/mm/kasan/init_32.c > > +++ b/arch/powerpc/mm/kasan/init_32.c > > @@ -84,6 +84,9 @@ kasan_update_early_region(unsigned long k_start, > > unsigned long k_end, pte_t pte) > >{ > > unsigned long k_cur; > > > > + if (k_start == k_end) > > + return; > > + > > for (k_cur = k_start; k_cur != k_end; k_cur += PAGE_SIZE) { > > pmd_t *pmd = pmd_off_k(k_cur); > > pte_t *ptep = pte_offset_kernel(pmd, k_cur); > > > > > > > > I tested the two vmlinux you sent me offlist, they both start
Re: [PATCH v7 3/3 RESEND] powerpc/pseries: PLPKS SED Opal keystore support
Michal Suchánek writes: > Hello, > > On Thu, Sep 14, 2023 at 02:13:32PM +1000, Michael Ellerman wrote: >> Nathan Chancellor writes: >> > Hi Greg, >> > >> > On Fri, Sep 08, 2023 at 10:30:56AM -0500, gjo...@linux.vnet.ibm.com wrote: >> >> From: Greg Joyce >> >> >> >> Define operations for SED Opal to read/write keys >> >> from POWER LPAR Platform KeyStore(PLPKS). This allows >> >> non-volatile storage of SED Opal keys. >> >> >> >> Signed-off-by: Greg Joyce >> >> Reviewed-by: Jonathan Derrick >> >> Reviewed-by: Hannes Reinecke >> > >> > After this change in -next as commit 9f2c7411ada9 ("powerpc/pseries: >> > PLPKS SED Opal keystore support"), I see the following crash when >> > booting some distribution configurations, such as OpenSUSE's [1] (the >> > rootfs is available at [2] if necessary): >> >> Thanks for testing Nathan. >> >> The code needs to check plpks_is_available() somewhere, before calling >> the plpks routines. > > would this fixup do it? > > I don't really see any other place to plug the check with the current > code structure. I think the plpks_sed code should call plpks_is_available() once at init time and cache the result. Otherwise it's will be doing an extra hcall (in _plpks_get_config()) for every call, which would be wasteful. cheers > diff --git a/arch/powerpc/platforms/pseries/plpks_sed_ops.c > b/arch/powerpc/platforms/pseries/plpks_sed_ops.c > index c1d08075e850..f8038d998eae 100644 > --- a/arch/powerpc/platforms/pseries/plpks_sed_ops.c > +++ b/arch/powerpc/platforms/pseries/plpks_sed_ops.c > @@ -64,6 +64,9 @@ int sed_read_key(char *keyname, char *key, u_int *keylen) > int ret; > u_int len; > > + if (!plpks_is_available()) > + return -ENODEV; > + > plpks_init_var(, keyname); > var.data = (u8 *) > var.datalen = sizeof(data); > @@ -89,6 +92,9 @@ int sed_write_key(char *keyname, char *key, u_int keylen) > struct plpks_sed_object_data data; > struct plpks_var_name vname; > > + if (!plpks_is_available()) > + return -ENODEV; > + > plpks_init_var(, keyname); > > var.datalen = sizeof(struct plpks_sed_object_data); > -- > 2.41.0
Re: [PATCH v7 3/3 RESEND] powerpc/pseries: PLPKS SED Opal keystore support
Hello, On Thu, Sep 14, 2023 at 02:13:32PM +1000, Michael Ellerman wrote: > Nathan Chancellor writes: > > Hi Greg, > > > > On Fri, Sep 08, 2023 at 10:30:56AM -0500, gjo...@linux.vnet.ibm.com wrote: > >> From: Greg Joyce > >> > >> Define operations for SED Opal to read/write keys > >> from POWER LPAR Platform KeyStore(PLPKS). This allows > >> non-volatile storage of SED Opal keys. > >> > >> Signed-off-by: Greg Joyce > >> Reviewed-by: Jonathan Derrick > >> Reviewed-by: Hannes Reinecke > > > > After this change in -next as commit 9f2c7411ada9 ("powerpc/pseries: > > PLPKS SED Opal keystore support"), I see the following crash when > > booting some distribution configurations, such as OpenSUSE's [1] (the > > rootfs is available at [2] if necessary): > > Thanks for testing Nathan. > > The code needs to check plpks_is_available() somewhere, before calling > the plpks routines. would this fixup do it? I don't really see any other place to plug the check with the current code structure. Thanks Michal diff --git a/arch/powerpc/platforms/pseries/plpks_sed_ops.c b/arch/powerpc/platforms/pseries/plpks_sed_ops.c index c1d08075e850..f8038d998eae 100644 --- a/arch/powerpc/platforms/pseries/plpks_sed_ops.c +++ b/arch/powerpc/platforms/pseries/plpks_sed_ops.c @@ -64,6 +64,9 @@ int sed_read_key(char *keyname, char *key, u_int *keylen) int ret; u_int len; + if (!plpks_is_available()) + return -ENODEV; + plpks_init_var(, keyname); var.data = (u8 *) var.datalen = sizeof(data); @@ -89,6 +92,9 @@ int sed_write_key(char *keyname, char *key, u_int keylen) struct plpks_sed_object_data data; struct plpks_var_name vname; + if (!plpks_is_available()) + return -ENODEV; + plpks_init_var(, keyname); var.datalen = sizeof(struct plpks_sed_object_data); -- 2.41.0
Re: [RFC PATCH v3 6/9] media: v4l2: Add audio capture and output support
Hi Shenjiu, Thanks for the update. On Thu, Sep 14, 2023 at 01:54:02PM +0800, Shengjiu Wang wrote: > Audio signal processing has the requirement for memory to > memory similar as Video. > > This patch is to add this support in v4l2 framework, defined > new buffer type V4L2_BUF_TYPE_AUDIO_CAPTURE and > V4L2_BUF_TYPE_AUDIO_OUTPUT, defined new format v4l2_audio_format > for audio case usage. > > Defined V4L2_AUDIO_FMT_LPCM format type for audio. This would be nicer as a separate patch. Also see the related comments below. > > Defined V4L2_CAP_AUDIO_M2M capability type for audio memory > to memory case. > > The created audio device is named "/dev/v4l-audioX". > > Signed-off-by: Shengjiu Wang > --- > .../userspace-api/media/v4l/audio-formats.rst | 15 + > .../userspace-api/media/v4l/buffer.rst| 6 ++ > .../userspace-api/media/v4l/dev-audio.rst | 63 +++ > .../userspace-api/media/v4l/devices.rst | 1 + > .../media/v4l/pixfmt-aud-lpcm.rst | 31 + > .../userspace-api/media/v4l/pixfmt.rst| 1 + > .../media/v4l/vidioc-enum-fmt.rst | 2 + > .../userspace-api/media/v4l/vidioc-g-fmt.rst | 4 ++ > .../media/v4l/vidioc-querycap.rst | 3 + > .../media/videodev2.h.rst.exceptions | 2 + > .../media/common/videobuf2/videobuf2-v4l2.c | 4 ++ > drivers/media/v4l2-core/v4l2-dev.c| 17 + > drivers/media/v4l2-core/v4l2-ioctl.c | 53 > include/media/v4l2-dev.h | 2 + > include/media/v4l2-ioctl.h| 34 ++ > include/uapi/linux/videodev2.h| 25 > 16 files changed, 263 insertions(+) > create mode 100644 Documentation/userspace-api/media/v4l/audio-formats.rst > create mode 100644 Documentation/userspace-api/media/v4l/dev-audio.rst > create mode 100644 Documentation/userspace-api/media/v4l/pixfmt-aud-lpcm.rst > > diff --git a/Documentation/userspace-api/media/v4l/audio-formats.rst > b/Documentation/userspace-api/media/v4l/audio-formats.rst > new file mode 100644 > index ..bc52712d20d3 > --- /dev/null > +++ b/Documentation/userspace-api/media/v4l/audio-formats.rst > @@ -0,0 +1,15 @@ > +.. SPDX-License-Identifier: GFDL-1.1-no-invariants-or-later > + > +.. _audio-formats: > + > +* > +Audio Formats > +* > + > +These formats are used for :ref:`audio` interface only. > + > + > +.. toctree:: > +:maxdepth: 1 > + > +pixfmt-aud-lpcm > diff --git a/Documentation/userspace-api/media/v4l/buffer.rst > b/Documentation/userspace-api/media/v4l/buffer.rst > index 04dec3e570ed..80cf2cb20dfe 100644 > --- a/Documentation/userspace-api/media/v4l/buffer.rst > +++ b/Documentation/userspace-api/media/v4l/buffer.rst > @@ -438,6 +438,12 @@ enum v4l2_buf_type > * - ``V4L2_BUF_TYPE_META_OUTPUT`` >- 14 >- Buffer for metadata output, see :ref:`metadata`. > +* - ``V4L2_BUF_TYPE_AUDIO_CAPTURE`` > + - 15 > + - Buffer for audio capture, see :ref:`audio`. > +* - ``V4L2_BUF_TYPE_AUDIO_OUTPUT`` > + - 16 > + - Buffer for audio output, see :ref:`audio`. > > > .. _buffer-flags: > diff --git a/Documentation/userspace-api/media/v4l/dev-audio.rst > b/Documentation/userspace-api/media/v4l/dev-audio.rst > new file mode 100644 > index ..f9bcf0c7b056 > --- /dev/null > +++ b/Documentation/userspace-api/media/v4l/dev-audio.rst > @@ -0,0 +1,63 @@ > +.. SPDX-License-Identifier: GFDL-1.1-no-invariants-or-later > + > +.. _audiodev: > + > +** > +audio Interface Capital "A"? > +** Too many asterisks (same a few lines above, too). > + > +The audio interface is implemented on audio device nodes. The audio device > +which uses application software for modulation or demodulation. This > +interface is intended for controlling and data streaming of such devices > + > +Audio devices are accessed through character device special files named > +``/dev/v4l-audio`` > + > +Querying Capabilities > += > + > +Device nodes supporting the audio capture and output interface set the > +``V4L2_CAP_AUDIO_M2M`` flag in the ``device_caps`` field of the > +:c:type:`v4l2_capability` structure returned by the :c:func:`VIDIOC_QUERYCAP` > +ioctl. > + > +At least one of the read/write or streaming I/O methods must be supported. > + > + > +Data Format Negotiation > +=== > + > +The audio device uses the :ref:`format` ioctls to select the capture format. > +The audio buffer content format is bound to that selected format. In addition > +to the basic :ref:`format` ioctls, the :c:func:`VIDIOC_ENUM_FMT` ioctl must > be > +supported as well. > + > +To use the :ref:`format` ioctls applications set the ``type`` field of the > +:c:type:`v4l2_format` structure to ``V4L2_BUF_TYPE_AUDIO_CAPTURE`` or to > +``V4L2_BUF_TYPE_AUDIO_OUTPUT``. Both drivers and applications must set the > +remainder of the
[PATCH v3 3/3] powerpc/config: Simplify pmac32_defconfig
Simplify pmac32_defconfig with POWER_RESET dependences. Regenerate pmac32_defconfig with 'make savedefconfig'. Suggested-by: Philippe Mathieu-Daudé Suggested-by: Christophe Leroy Signed-off-by: Yuan Tan --- arch/powerpc/configs/pmac32_defconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/powerpc/configs/pmac32_defconfig b/arch/powerpc/configs/pmac32_defconfig index 11d489c2c3e0..17df965be099 100644 --- a/arch/powerpc/configs/pmac32_defconfig +++ b/arch/powerpc/configs/pmac32_defconfig @@ -138,7 +138,6 @@ CONFIG_DM_SNAPSHOT=m CONFIG_DM_MIRROR=m CONFIG_DM_ZERO=m CONFIG_ADB=y -CONFIG_ADB_CUDA=y CONFIG_ADB_PMU=y CONFIG_ADB_PMU_LED=y CONFIG_ADB_PMU_LED_DISK=y @@ -180,6 +179,7 @@ CONFIG_SERIAL_PMACZILOG=y CONFIG_SERIAL_PMACZILOG_TTYS=y CONFIG_SERIAL_PMACZILOG_CONSOLE=y CONFIG_I2C_CHARDEV=m +CONFIG_POWER_RESET=y CONFIG_APM_POWER=y CONFIG_BATTERY_PMU=y CONFIG_HWMON=m -- 2.34.1
[PATCH v3 2/3] Kconfig: Add dependencies of POWER_RESET for pmac32
pmac32's power off depends on ADB_CUDA to work. Enable it when POWER_RESET is set for convenience. Suggested-by: Zhangjin Wu Signed-off-by: Yuan Tan --- arch/powerpc/platforms/powermac/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/powerpc/platforms/powermac/Kconfig b/arch/powerpc/platforms/powermac/Kconfig index 130707ec9f99..8bdae0caf21e 100644 --- a/arch/powerpc/platforms/powermac/Kconfig +++ b/arch/powerpc/platforms/powermac/Kconfig @@ -2,6 +2,7 @@ config PPC_PMAC bool "Apple PowerMac based machines" depends on PPC_BOOK3S && CPU_BIG_ENDIAN + select ADB_CUDA if POWER_RESET && PPC32 select MPIC select FORCE_PCI select PPC_INDIRECT_PCI if PPC32 -- 2.34.1
[PATCH v3 1/3] powerpc/config: Cleanup pmac32_defconfig
Use 'make savedefconfig' to cleanup pmac32_defconfig, based on Linux 7.6-rc1 Suggested-by: Philippe Mathieu-Daudé Suggested-by: Christophe Leroy Signed-off-by: Yuan Tan --- arch/powerpc/configs/pmac32_defconfig | 44 --- 1 file changed, 19 insertions(+), 25 deletions(-) diff --git a/arch/powerpc/configs/pmac32_defconfig b/arch/powerpc/configs/pmac32_defconfig index a205da9ee5f2..11d489c2c3e0 100644 --- a/arch/powerpc/configs/pmac32_defconfig +++ b/arch/powerpc/configs/pmac32_defconfig @@ -1,4 +1,3 @@ -CONFIG_ALTIVEC=y # CONFIG_LOCALVERSION_AUTO is not set CONFIG_SYSVIPC=y CONFIG_POSIX_MQUEUE=y @@ -8,12 +7,8 @@ CONFIG_IKCONFIG=y CONFIG_IKCONFIG_PROC=y CONFIG_LOG_BUF_SHIFT=14 CONFIG_BLK_DEV_INITRD=y -# CONFIG_COMPAT_BRK is not set CONFIG_PROFILING=y -CONFIG_MODULES=y -CONFIG_MODULE_UNLOAD=y -CONFIG_MODULE_FORCE_UNLOAD=y -CONFIG_PARTITION_ADVANCED=y +CONFIG_ALTIVEC=y # CONFIG_PPC_CHRP is not set CONFIG_CPU_FREQ=y CONFIG_CPU_FREQ_GOV_POWERSAVE=y @@ -21,12 +16,15 @@ CONFIG_CPU_FREQ_GOV_USERSPACE=y CONFIG_CPU_FREQ_PMAC=y CONFIG_GEN_RTC=y CONFIG_HIGHMEM=y -CONFIG_BINFMT_MISC=m CONFIG_HIBERNATION=y CONFIG_PM_DEBUG=y CONFIG_APM_EMULATION=y -CONFIG_PCCARD=m -CONFIG_YENTA=m +CONFIG_MODULES=y +CONFIG_MODULE_UNLOAD=y +CONFIG_MODULE_FORCE_UNLOAD=y +CONFIG_PARTITION_ADVANCED=y +CONFIG_BINFMT_MISC=m +# CONFIG_COMPAT_BRK is not set CONFIG_NET=y CONFIG_PACKET=y CONFIG_UNIX=y @@ -100,6 +98,8 @@ CONFIG_BT_HCIBFUSB=m CONFIG_CFG80211=m CONFIG_MAC80211=m CONFIG_MAC80211_LEDS=y +CONFIG_PCCARD=m +CONFIG_YENTA=m # CONFIG_STANDALONE is not set CONFIG_CONNECTOR=y CONFIG_MAC_FLOPPY=m @@ -179,7 +179,6 @@ CONFIG_SERIAL_8250=m CONFIG_SERIAL_PMACZILOG=y CONFIG_SERIAL_PMACZILOG_TTYS=y CONFIG_SERIAL_PMACZILOG_CONSOLE=y -CONFIG_NVRAM=y CONFIG_I2C_CHARDEV=m CONFIG_APM_POWER=y CONFIG_BATTERY_PMU=y @@ -189,7 +188,6 @@ CONFIG_AGP_UNINORTH=m CONFIG_DRM=m CONFIG_DRM_RADEON=m CONFIG_DRM_LEGACY=y -CONFIG_DRM_R128=m CONFIG_FB=y CONFIG_FB_OF=y CONFIG_FB_CONTROL=y @@ -275,23 +273,19 @@ CONFIG_NFSD_V3_ACL=y CONFIG_NFSD_V4=y CONFIG_NLS_CODEPAGE_437=m CONFIG_NLS_ISO8859_1=m -CONFIG_CRC_T10DIF=y -CONFIG_MAGIC_SYSRQ=y -CONFIG_DEBUG_KERNEL=y -CONFIG_DETECT_HUNG_TASK=y -CONFIG_XMON=y -CONFIG_XMON_DEFAULT=y -CONFIG_BOOTX_TEXT=y -CONFIG_CRYPTO_PCBC=m -CONFIG_CRYPTO_MD4=m -CONFIG_CRYPTO_SHA512=m -CONFIG_CRYPTO_WP512=m -CONFIG_CRYPTO_ANUBIS=m CONFIG_CRYPTO_BLOWFISH=m CONFIG_CRYPTO_CAST5=m CONFIG_CRYPTO_CAST6=m -CONFIG_CRYPTO_KHAZAD=m CONFIG_CRYPTO_SERPENT=m -CONFIG_CRYPTO_TEA=m CONFIG_CRYPTO_TWOFISH=m +CONFIG_CRYPTO_PCBC=m +CONFIG_CRYPTO_MD4=m +CONFIG_CRYPTO_WP512=m CONFIG_CRYPTO_DEFLATE=m +CONFIG_CRC_T10DIF=y +CONFIG_DEBUG_KERNEL=y +CONFIG_MAGIC_SYSRQ=y +CONFIG_DETECT_HUNG_TASK=y +CONFIG_XMON=y +CONFIG_XMON_DEFAULT=y +CONFIG_BOOTX_TEXT=y -- 2.34.1
[PATCH v3 0/3] Kconfig: Add dependencies of POWER_RESET for pmac32
These patches are to add dependencies of POWER_RESET for pmac32. As I have to use "savedefconfig" on the latest branch of different architectures, I am sending separate patches for each architecture in v3. To simplify the enablement of the poweroff support, selecting the required options for CONFIG_POWER_RESET=y may make many people happy especially when they are using a customized config (maybe tinyconfig based) for a target qemu board. Without normal poweroff support from the kernel side, qemu will simply hang[1] there after a 'poweroff' command, which is a very bad experience for the automatical tests. However, CONFIG_POWER_RESET is ineffective if there are no dependencies that enable certain devices in Kconfig. Currently, based on tinyconfig, it is very hard to find the exact poweroff related option[2]. Some architectures' poweroff works well without any dependence, the others' poweroff options are hidden deeply, which make things hard. After multiple verifications, these options have been identified as the minimum dependencies required for poweroff to function normally. Additionally, 'make savedefconfig' simplifies the defconfig automatically. Zhangjin and I invested a significant amount of time in searching for the current options on these devices. We hope that this set of patches will save time for others. If community like it, we will consider adding dependencies for POWER_RESET on other devices. We hope every device's CONFIG_POWER_RESET will have proper dependencies. :) --- [1]: https://lore.kernel.org/lkml/511b2f6009fb830b3f32b4be3dca99596c684fa3.1689759351.git.fal...@tinylab.org/ [2]: https://lore.kernel.org/all/983843582e52e83fba79ad45cea6c79e1f62ec6c.1690489039.git.fal...@tinylab.org/ v1: https://lore.kernel.org/all/20230831201727.3177853-1-tany...@tinylab.org/ v2: https://lore.kernel.org/all/cover.1693535514.git.tany...@tinylab.org/ --- Changes in v2: - Fix the mistake of using spaces instead of tabs in kconfig. Changes in v3: - Enable POWER_RESET and simplify the deconfig. - Select ADB_CUDA in PPC_PMAC32 only. Yuan Tan (3): powerpc/config: Cleanup pmac32_defconfig Kconfig: Add dependencies of POWER_RESET for pmac32 powerpc/config: Simplify pmac32_defconfig arch/powerpc/configs/pmac32_defconfig | 46 +++-- arch/powerpc/platforms/powermac/Kconfig | 1 + 2 files changed, 21 insertions(+), 26 deletions(-) base-commit: 0bb80ecc33a8fb5a682236443c1e740d5c917d1d -- 2.34.1
[PATCH 1/1] powerpc: fix a memory leak
When one of the methods xive_native_alloc_irq_on_chip, irq_create_mapping or irq_get_handler_data fails, the function will directly return without disposing vinst->name and vinst. Fix it. Fixes: c20e1e299d93 ("powerpc/vas: Alloc and setup IRQ and trigger port address") Signed-off-by: Yuanjun Gong --- arch/powerpc/platforms/powernv/vas.c | 14 +- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/arch/powerpc/platforms/powernv/vas.c b/arch/powerpc/platforms/powernv/vas.c index b65256a63e87..780740b478f0 100644 --- a/arch/powerpc/platforms/powernv/vas.c +++ b/arch/powerpc/platforms/powernv/vas.c @@ -54,7 +54,7 @@ static int init_vas_instance(struct platform_device *pdev) struct xive_irq_data *xd; uint32_t chipid, hwirq; struct resource *res; - int rc, cpu, vasid; + int rc, cpu, vasid, ret; rc = of_property_read_u32(dn, "ibm,vas-id", ); if (rc) { @@ -102,6 +102,7 @@ static int init_vas_instance(struct platform_device *pdev) res = >resource[3]; if (res->end > 62) { pr_err("Bad 'paste_win_id_shift' in DT, %llx\n", res->end); + ret = -ENODEV goto free_vinst; } @@ -111,21 +112,24 @@ static int init_vas_instance(struct platform_device *pdev) if (!hwirq) { pr_err("Inst%d: Unable to allocate global irq for chip %d\n", vinst->vas_id, chipid); - return -ENOENT; + ret = -ENOENT; + goto free_vinst; } vinst->virq = irq_create_mapping(NULL, hwirq); if (!vinst->virq) { pr_err("Inst%d: Unable to map global irq %d\n", vinst->vas_id, hwirq); - return -EINVAL; + ret = -EINVAL; + goto free_vinst; } xd = irq_get_handler_data(vinst->virq); if (!xd) { pr_err("Inst%d: Invalid virq %d\n", vinst->vas_id, vinst->virq); - return -EINVAL; + ret = -EINVAL; + goto free_vinst; } vinst->irq_port = xd->trig_page; @@ -168,7 +172,7 @@ static int init_vas_instance(struct platform_device *pdev) free_vinst: kfree(vinst->name); kfree(vinst); - return -ENODEV; + return ret; } -- 2.37.2
Re: [PATCH v4 8/8] bpf ppc32: Access only if addr is kernel address
On 14/09/23 11:48 am, Christophe Leroy wrote: Hi, Hi Christophe, Le 29/09/2021 à 13:18, Hari Bathini a écrit : With KUAP enabled, any kernel code which wants to access userspace needs to be surrounded by disable-enable KUAP. But that is not happening for BPF_PROBE_MEM load instruction. Though PPC32 does not support read protection, considering the fact that PTR_TO_BTF_ID (which uses BPF_PROBE_MEM mode) could either be a valid kernel pointer or NULL but should never be a pointer to userspace address, execute BPF_PROBE_MEM load only if addr is kernel address, otherwise set dst_reg=0 and move on. While looking at the series "bpf: verifier: stop emitting zext for LDX" from Puranjay I got a question on this old commit, see below. This will catch NULL, valid or invalid userspace pointers. Only bad kernel pointer will be handled by BPF exception table. [Alexei suggested for x86] Suggested-by: Alexei Starovoitov Signed-off-by: Hari Bathini --- Changes in v4: * Adjusted the emit code to avoid using temporary reg. arch/powerpc/net/bpf_jit_comp32.c | 34 +++ 1 file changed, 34 insertions(+) diff --git a/arch/powerpc/net/bpf_jit_comp32.c b/arch/powerpc/net/bpf_jit_comp32.c index 6ee13a09c70d..2ac81563c78d 100644 --- a/arch/powerpc/net/bpf_jit_comp32.c +++ b/arch/powerpc/net/bpf_jit_comp32.c @@ -818,6 +818,40 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct codegen_context * case BPF_LDX | BPF_PROBE_MEM | BPF_W: case BPF_LDX | BPF_MEM | BPF_DW: /* dst = *(u64 *)(ul) (src + off) */ case BPF_LDX | BPF_PROBE_MEM | BPF_DW: + /* +* As PTR_TO_BTF_ID that uses BPF_PROBE_MEM mode could either be a valid +* kernel pointer or NULL but not a userspace address, execute BPF_PROBE_MEM +* load only if addr is kernel address (see is_kernel_addr()), otherwise +* set dst_reg=0 and move on. +*/ + if (BPF_MODE(code) == BPF_PROBE_MEM) { + PPC_LI32(_R0, TASK_SIZE - off); + EMIT(PPC_RAW_CMPLW(src_reg, _R0)); + PPC_BCC(COND_GT, (ctx->idx + 5) * 4); + EMIT(PPC_RAW_LI(dst_reg, 0)); + /* +* For BPF_DW case, "li reg_h,0" would be needed when +* !fp->aux->verifier_zext. Emit NOP otherwise. +* +* Note that "li reg_h,0" is emitted for BPF_B/H/W case, +* if necessary. So, jump there insted of emitting an +* additional "li reg_h,0" instruction. +*/ + if (size == BPF_DW && !fp->aux->verifier_zext) + EMIT(PPC_RAW_LI(dst_reg_h, 0)); + else + EMIT(PPC_RAW_NOP()); While do you need a NOP in the else case ? Can't we just emit no instruction in that case ? Yeah but used the same offset for all cases in the conditional branch above. To drop the NOP, the conditional branch offset can be calculated based on the above if condition, I guess.. Thanks, Hari
Re: [PATCH v2 1/3] kconfig: add dependencies of POWER_RESET for mips malta
On 9/4/2023 6:51 PM, Philippe Mathieu-Daudé wrote: On 4/9/23 11:24, Yuan Tan wrote: Hi, On 9/4/2023 3:40 PM, Philippe Mathieu-Daudé wrote: Hi, On 1/9/23 04:42, Yuan Tan wrote: MIPS Malta's power off depends on PCI, PCI_QUIRKS, and POWER_RESET_PIIX4_POWEROFF to work. Enable them when POWER_RESET is set for convenience. Suggested-by: Zhangjin Wu Signed-off-by: Yuan Tan --- arch/mips/Kconfig | 3 +++ 1 file changed, 3 insertions(+) diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig index bc8421859006..13bacbd05125 100644 --- a/arch/mips/Kconfig +++ b/arch/mips/Kconfig @@ -547,6 +547,9 @@ config MIPS_MALTA select MIPS_L1_CACHE_SHIFT_6 select MIPS_MSC select PCI_GT64XXX_PCI0 + select PCI if POWER_RESET + select PCI_QUIRKS if POWER_RESET + select POWER_RESET_PIIX4_POWEROFF if POWER_RESET select SMP_UP if SMP select SWAP_IO_SPACE select SYS_HAS_CPU_MIPS32_R1 Shouldn't we also update the _defconfig files? Sorry, in my last email, I forgot to reply to all. So I am now resending this email. In malta_defconfig, PCI and POWER_RESET_PIIX4_POWEROFF have already been set and PCI_QUIRKS is also selected by FSL_PCI [=n]. So shutdown and reboot with malta_defconfig is working and there is no need to update the malta_defconfig Since the dependency is now enforced by Kconfig, the defconfig can be simplified: --- a/arch/mips/configs/malta_defconfig +++ b/arch/mips/configs/malta_defconfig @@ -306,3 +306,2 @@ CONFIG_SERIAL_8250_CONSOLE=y CONFIG_POWER_RESET=y -CONFIG_POWER_RESET_PIIX4_POWEROFF=y CONFIG_POWER_RESET_SYSCON=y But maybe we don't care, I don't know. After testing, I found that "savedefconfig" will automatically generate the simplified configuration. As I have to use "savedefconfig" on the latest branch of the three architectures, in v3, I will send a separate patch for each architecture. Thanks to your advice.
[RFC PATCH v3 9/9] media: imx-asrc: Add memory to memory driver
Implement the ASRC memory to memory function using the v4l2 framework, user can use this function with v4l2 ioctl interface. User send the output and capture buffer to driver and driver store the converted data to the capture buffer. This feature can be shared by ASRC and EASRC drivers Signed-off-by: Shengjiu Wang --- drivers/media/platform/nxp/Kconfig| 12 + drivers/media/platform/nxp/Makefile |1 + drivers/media/platform/nxp/imx-asrc.c | 1058 + 3 files changed, 1071 insertions(+) create mode 100644 drivers/media/platform/nxp/imx-asrc.c diff --git a/drivers/media/platform/nxp/Kconfig b/drivers/media/platform/nxp/Kconfig index 40e3436669e2..8234644ee341 100644 --- a/drivers/media/platform/nxp/Kconfig +++ b/drivers/media/platform/nxp/Kconfig @@ -67,3 +67,15 @@ config VIDEO_MX2_EMMAPRP source "drivers/media/platform/nxp/dw100/Kconfig" source "drivers/media/platform/nxp/imx-jpeg/Kconfig" + +config VIDEO_IMX_ASRC + tristate "NXP i.MX ASRC M2M support" + depends on V4L_MEM2MEM_DRIVERS + depends on MEDIA_SUPPORT + select VIDEOBUF2_DMA_CONTIG + select V4L2_MEM2MEM_DEV + help + Say Y if you want to add ASRC M2M support for NXP CPUs. + It is a complement for ASRC M2P and ASRC P2M features. + This option is only useful for out-of-tree drivers since + in-tree drivers select it automatically. diff --git a/drivers/media/platform/nxp/Makefile b/drivers/media/platform/nxp/Makefile index 4d90eb713652..1325675e34f5 100644 --- a/drivers/media/platform/nxp/Makefile +++ b/drivers/media/platform/nxp/Makefile @@ -9,3 +9,4 @@ obj-$(CONFIG_VIDEO_IMX8MQ_MIPI_CSI2) += imx8mq-mipi-csi2.o obj-$(CONFIG_VIDEO_IMX_MIPI_CSIS) += imx-mipi-csis.o obj-$(CONFIG_VIDEO_IMX_PXP) += imx-pxp.o obj-$(CONFIG_VIDEO_MX2_EMMAPRP) += mx2_emmaprp.o +obj-$(CONFIG_VIDEO_IMX_ASRC) += imx-asrc.o diff --git a/drivers/media/platform/nxp/imx-asrc.c b/drivers/media/platform/nxp/imx-asrc.c new file mode 100644 index ..21079c7abd27 --- /dev/null +++ b/drivers/media/platform/nxp/imx-asrc.c @@ -0,0 +1,1058 @@ +// SPDX-License-Identifier: GPL-2.0 +// +// Copyright (C) 2014-2016 Freescale Semiconductor, Inc. +// Copyright (C) 2019-2023 NXP +// +// Freescale ASRC Memory to Memory (M2M) driver + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define V4L_CAP OUT +#define V4L_OUT IN + +/* Flags that indicate a format can be used for capture/output */ +#define MEM2MEM_CAPTUREBIT(0) +#define MEM2MEM_OUTPUT BIT(1) + +#define ASRC_xPUT_DMA_CALLBACK(dir) \ + (((dir) == V4L_OUT) ? asrc_input_dma_callback \ + : asrc_output_dma_callback) + +#define DIR_STR(dir) (dir) == V4L_OUT ? "out" : "cap" + +#define ASRC_M2M_BUFFER_SIZE (512 * 1024) +#define ASRC_M2M_PERIOD_SIZE (48 * 1024) +#define ASRC_M2M_SG_NUM (20) + +struct asrc_pair_m2m { + struct fsl_asrc_pair *pair; + struct asrc_m2m *m2m; + struct v4l2_fh fh; + struct v4l2_ctrl_handler ctrl_handler; +}; + +struct asrc_m2m { + struct fsl_asrc *asrc; + struct v4l2_device v4l2_dev; + struct v4l2_m2m_dev *m2m_dev; + struct video_device *dec_vdev; + struct mutex mlock; /* v4l2 ioctls serialization */ + struct platform_device *pdev; +}; + +struct asrc_fmt { + u32 fourcc; + u32 types; +}; + +static struct asrc_fmt formats[] = { + { + .fourcc = V4L2_AUDIO_FMT_LPCM, + .types = MEM2MEM_CAPTURE | MEM2MEM_OUTPUT, + }, +}; + +#define NUM_FORMATS ARRAY_SIZE(formats) + +static inline struct asrc_pair_m2m *asrc_m2m_fh_to_ctx(struct v4l2_fh *fh) +{ + return container_of(fh, struct asrc_pair_m2m, fh); +} + +/** + * asrc_read_last_fifo: read all the remaining data from FIFO + * @pair: Structure pointer of fsl_asrc_pair + * @dma_vaddr: virtual address of capture buffer + * @length: payload length of capture buffer + */ +static void asrc_read_last_fifo(struct fsl_asrc_pair *pair, void *dma_vaddr, u32 *length) +{ + struct fsl_asrc *asrc = pair->asrc; + enum asrc_pair_index index = pair->index; + u32 i, reg, size, t_size = 0, width; + u32 *reg32 = NULL; + u16 *reg16 = NULL; + u8 *reg24 = NULL; + + width = snd_pcm_format_physical_width(pair->sample_format[V4L_CAP]); + if (width == 32) + reg32 = dma_vaddr + *length; + else if (width == 16) + reg16 = dma_vaddr + *length; + else + reg24 = dma_vaddr + *length; +retry: + size = asrc->get_output_fifo_size(pair); + if (size + *length > ASRC_M2M_BUFFER_SIZE) + goto end; + + for (i = 0; i < size * pair->channels; i++) { + regmap_read(asrc->regmap, asrc->get_fifo_addr(OUT, index), ); + if (reg32) { + *(reg32) = reg; + reg32++; +
[RFC PATCH v3 8/9] media: audm2m: add virtual driver for audio memory to memory
Audio memory to memory virtual driver use video memory to memory virtual driver vim2m.c as example. The main difference is device type is VFL_TYPE_AUDIO and device cap type is V4L2_CAP_AUDIO_M2M. The device_run function is a dummy function, which is simply copy the data from input buffer to output buffer. Signed-off-by: Shengjiu Wang --- drivers/media/test-drivers/Kconfig | 9 + drivers/media/test-drivers/Makefile | 1 + drivers/media/test-drivers/audm2m.c | 767 3 files changed, 777 insertions(+) create mode 100644 drivers/media/test-drivers/audm2m.c diff --git a/drivers/media/test-drivers/Kconfig b/drivers/media/test-drivers/Kconfig index 459b433e9fae..be60d73cbf97 100644 --- a/drivers/media/test-drivers/Kconfig +++ b/drivers/media/test-drivers/Kconfig @@ -17,6 +17,15 @@ config VIDEO_VIM2M This is a virtual test device for the memory-to-memory driver framework. +config VIDEO_AUDM2M + tristate "Virtual Memory-to-Memory Driver For Audio" + depends on VIDEO_DEV + select VIDEOBUF2_VMALLOC + select V4L2_MEM2MEM_DEV + help + This is a virtual audio test device for the memory-to-memory driver + framework. + source "drivers/media/test-drivers/vicodec/Kconfig" source "drivers/media/test-drivers/vimc/Kconfig" source "drivers/media/test-drivers/vivid/Kconfig" diff --git a/drivers/media/test-drivers/Makefile b/drivers/media/test-drivers/Makefile index 740714a4584d..b53ed7e6eaf1 100644 --- a/drivers/media/test-drivers/Makefile +++ b/drivers/media/test-drivers/Makefile @@ -10,6 +10,7 @@ obj-$(CONFIG_DVB_VIDTV) += vidtv/ obj-$(CONFIG_VIDEO_VICODEC) += vicodec/ obj-$(CONFIG_VIDEO_VIM2M) += vim2m.o +obj-$(CONFIG_VIDEO_AUDM2M) += audm2m.o obj-$(CONFIG_VIDEO_VIMC) += vimc/ obj-$(CONFIG_VIDEO_VIVID) += vivid/ obj-$(CONFIG_VIDEO_VISL) += visl/ diff --git a/drivers/media/test-drivers/audm2m.c b/drivers/media/test-drivers/audm2m.c new file mode 100644 index ..d54bc99b9275 --- /dev/null +++ b/drivers/media/test-drivers/audm2m.c @@ -0,0 +1,767 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * A virtual v4l2-mem2mem example for audio device. + */ + +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include + +MODULE_DESCRIPTION("Virtual device for audio mem2mem testing"); +MODULE_LICENSE("GPL"); +MODULE_VERSION("0.1"); +MODULE_ALIAS("audio_mem2mem_testdev"); + +static unsigned int debug; +module_param(debug, uint, 0644); +MODULE_PARM_DESC(debug, "debug level"); + +/* Flags that indicate a format can be used for capture/output */ +#define MEM2MEM_CAPTUREBIT(0) +#define MEM2MEM_OUTPUT BIT(1) + +#define MEM2MEM_NAME "audm2m" + +#define dprintk(dev, lvl, fmt, arg...) \ + v4l2_dbg(lvl, debug, &(dev)->v4l2_dev, "%s: " fmt, __func__, ## arg) + +#define SAMPLE_NUM 4096 + +static void audm2m_dev_release(struct device *dev) +{} + +static struct platform_device audm2m_pdev = { + .name = MEM2MEM_NAME, + .dev.release= audm2m_dev_release, +}; + +struct audm2m_fmt { + u32 fourcc; + u32 types; +}; + +static struct audm2m_fmt formats[] = { + { + .fourcc = V4L2_AUDIO_FMT_LPCM, + .types = MEM2MEM_CAPTURE | MEM2MEM_OUTPUT, + } +}; + +#define NUM_FORMATS ARRAY_SIZE(formats) + +/* Per-queue, driver-specific private data */ +struct audm2m_q_data { + unsigned intrate; + snd_pcm_format_tformat; + unsigned intchannels; + unsigned intbuffersize; + struct audm2m_fmt *fmt; +}; + +enum { + V4L2_M2M_SRC = 0, + V4L2_M2M_DST = 1, +}; + +static struct audm2m_fmt *find_format(u32 fourcc) +{ + struct audm2m_fmt *fmt; + unsigned int k; + + for (k = 0; k < NUM_FORMATS; k++) { + fmt = [k]; + if (fmt->fourcc == fourcc) + break; + } + + if (k == NUM_FORMATS) + return NULL; + + return [k]; +} + +struct audm2m_dev { + struct v4l2_device v4l2_dev; + struct video_device vfd; + + atomic_tnum_inst; + struct mutexdev_mutex; + + struct v4l2_m2m_dev *m2m_dev; +}; + +struct audm2m_ctx { + struct v4l2_fh fh; + struct audm2m_dev *dev; + + struct mutexvb_mutex; + + /* Abort requested by m2m */ + int aborting; + + /* Source and destination queue data */ + struct audm2m_q_data q_data[2]; +}; + +static inline struct audm2m_ctx *file2ctx(struct file *file) +{ + return container_of(file->private_data, struct audm2m_ctx, fh); +} + +static struct audm2m_q_data *get_q_data(struct audm2m_ctx *ctx, + enum v4l2_buf_type type) +{ + switch (type) { + case
[RFC PATCH v3 7/9] media: uapi: Add V4L2_CID_USER_IMX_ASRC_RATIO_MOD control
The input clock and output clock may not be the accurate rate as the sample rate, there is some drift, so the convert ratio of i.MX ASRC module need to be changed according to actual clock rate. Add V4L2_CID_USER_IMX_ASRC_RATIO_MOD control for user to adjust the ratio. Signed-off-by: Shengjiu Wang --- Documentation/userspace-api/media/v4l/control.rst | 5 + drivers/media/v4l2-core/v4l2-ctrls-defs.c | 1 + include/uapi/linux/v4l2-controls.h| 1 + 3 files changed, 7 insertions(+) diff --git a/Documentation/userspace-api/media/v4l/control.rst b/Documentation/userspace-api/media/v4l/control.rst index 4463fce694b0..2bc175900a34 100644 --- a/Documentation/userspace-api/media/v4l/control.rst +++ b/Documentation/userspace-api/media/v4l/control.rst @@ -318,6 +318,11 @@ Control IDs depending on particular custom controls should check the driver name and version, see :ref:`querycap`. +.. _v4l2-audio-imx: + +``V4L2_CID_USER_IMX_ASRC_RATIO_MOD`` +sets the rasampler ratio modifier of i.MX asrc module. + Applications can enumerate the available controls with the :ref:`VIDIOC_QUERYCTRL` and :ref:`VIDIOC_QUERYMENU ` ioctls, get and set a diff --git a/drivers/media/v4l2-core/v4l2-ctrls-defs.c b/drivers/media/v4l2-core/v4l2-ctrls-defs.c index 8696eb1cdd61..16f66f66198c 100644 --- a/drivers/media/v4l2-core/v4l2-ctrls-defs.c +++ b/drivers/media/v4l2-core/v4l2-ctrls-defs.c @@ -1242,6 +1242,7 @@ const char *v4l2_ctrl_get_name(u32 id) case V4L2_CID_COLORIMETRY_CLASS:return "Colorimetry Controls"; case V4L2_CID_COLORIMETRY_HDR10_CLL_INFO: return "HDR10 Content Light Info"; case V4L2_CID_COLORIMETRY_HDR10_MASTERING_DISPLAY: return "HDR10 Mastering Display"; + case V4L2_CID_USER_IMX_ASRC_RATIO_MOD: return "ASRC RATIO MOD"; default: return NULL; } diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h index c3604a0a3e30..b1c319906d12 100644 --- a/include/uapi/linux/v4l2-controls.h +++ b/include/uapi/linux/v4l2-controls.h @@ -162,6 +162,7 @@ enum v4l2_colorfx { /* The base for the imx driver controls. * We reserve 16 controls for this driver. */ #define V4L2_CID_USER_IMX_BASE (V4L2_CID_USER_BASE + 0x10b0) +#define V4L2_CID_USER_IMX_ASRC_RATIO_MOD (V4L2_CID_USER_IMX_BASE + 0) /* * The base for the atmel isc driver controls. -- 2.34.1
[RFC PATCH v3 6/9] media: v4l2: Add audio capture and output support
Audio signal processing has the requirement for memory to memory similar as Video. This patch is to add this support in v4l2 framework, defined new buffer type V4L2_BUF_TYPE_AUDIO_CAPTURE and V4L2_BUF_TYPE_AUDIO_OUTPUT, defined new format v4l2_audio_format for audio case usage. Defined V4L2_AUDIO_FMT_LPCM format type for audio. Defined V4L2_CAP_AUDIO_M2M capability type for audio memory to memory case. The created audio device is named "/dev/v4l-audioX". Signed-off-by: Shengjiu Wang --- .../userspace-api/media/v4l/audio-formats.rst | 15 + .../userspace-api/media/v4l/buffer.rst| 6 ++ .../userspace-api/media/v4l/dev-audio.rst | 63 +++ .../userspace-api/media/v4l/devices.rst | 1 + .../media/v4l/pixfmt-aud-lpcm.rst | 31 + .../userspace-api/media/v4l/pixfmt.rst| 1 + .../media/v4l/vidioc-enum-fmt.rst | 2 + .../userspace-api/media/v4l/vidioc-g-fmt.rst | 4 ++ .../media/v4l/vidioc-querycap.rst | 3 + .../media/videodev2.h.rst.exceptions | 2 + .../media/common/videobuf2/videobuf2-v4l2.c | 4 ++ drivers/media/v4l2-core/v4l2-dev.c| 17 + drivers/media/v4l2-core/v4l2-ioctl.c | 53 include/media/v4l2-dev.h | 2 + include/media/v4l2-ioctl.h| 34 ++ include/uapi/linux/videodev2.h| 25 16 files changed, 263 insertions(+) create mode 100644 Documentation/userspace-api/media/v4l/audio-formats.rst create mode 100644 Documentation/userspace-api/media/v4l/dev-audio.rst create mode 100644 Documentation/userspace-api/media/v4l/pixfmt-aud-lpcm.rst diff --git a/Documentation/userspace-api/media/v4l/audio-formats.rst b/Documentation/userspace-api/media/v4l/audio-formats.rst new file mode 100644 index ..bc52712d20d3 --- /dev/null +++ b/Documentation/userspace-api/media/v4l/audio-formats.rst @@ -0,0 +1,15 @@ +.. SPDX-License-Identifier: GFDL-1.1-no-invariants-or-later + +.. _audio-formats: + +* +Audio Formats +* + +These formats are used for :ref:`audio` interface only. + + +.. toctree:: +:maxdepth: 1 + +pixfmt-aud-lpcm diff --git a/Documentation/userspace-api/media/v4l/buffer.rst b/Documentation/userspace-api/media/v4l/buffer.rst index 04dec3e570ed..80cf2cb20dfe 100644 --- a/Documentation/userspace-api/media/v4l/buffer.rst +++ b/Documentation/userspace-api/media/v4l/buffer.rst @@ -438,6 +438,12 @@ enum v4l2_buf_type * - ``V4L2_BUF_TYPE_META_OUTPUT`` - 14 - Buffer for metadata output, see :ref:`metadata`. +* - ``V4L2_BUF_TYPE_AUDIO_CAPTURE`` + - 15 + - Buffer for audio capture, see :ref:`audio`. +* - ``V4L2_BUF_TYPE_AUDIO_OUTPUT`` + - 16 + - Buffer for audio output, see :ref:`audio`. .. _buffer-flags: diff --git a/Documentation/userspace-api/media/v4l/dev-audio.rst b/Documentation/userspace-api/media/v4l/dev-audio.rst new file mode 100644 index ..f9bcf0c7b056 --- /dev/null +++ b/Documentation/userspace-api/media/v4l/dev-audio.rst @@ -0,0 +1,63 @@ +.. SPDX-License-Identifier: GFDL-1.1-no-invariants-or-later + +.. _audiodev: + +** +audio Interface +** + +The audio interface is implemented on audio device nodes. The audio device +which uses application software for modulation or demodulation. This +interface is intended for controlling and data streaming of such devices + +Audio devices are accessed through character device special files named +``/dev/v4l-audio`` + +Querying Capabilities += + +Device nodes supporting the audio capture and output interface set the +``V4L2_CAP_AUDIO_M2M`` flag in the ``device_caps`` field of the +:c:type:`v4l2_capability` structure returned by the :c:func:`VIDIOC_QUERYCAP` +ioctl. + +At least one of the read/write or streaming I/O methods must be supported. + + +Data Format Negotiation +=== + +The audio device uses the :ref:`format` ioctls to select the capture format. +The audio buffer content format is bound to that selected format. In addition +to the basic :ref:`format` ioctls, the :c:func:`VIDIOC_ENUM_FMT` ioctl must be +supported as well. + +To use the :ref:`format` ioctls applications set the ``type`` field of the +:c:type:`v4l2_format` structure to ``V4L2_BUF_TYPE_AUDIO_CAPTURE`` or to +``V4L2_BUF_TYPE_AUDIO_OUTPUT``. Both drivers and applications must set the +remainder of the :c:type:`v4l2_format` structure to 0. + +.. c:type:: v4l2_audio_format + +.. tabularcolumns:: |p{1.4cm}|p{2.4cm}|p{13.5cm}| + +.. flat-table:: struct v4l2_audio_format +:header-rows: 0 +:stub-columns: 0 +:widths: 1 1 2 + +* - __u32 + - ``rate`` + - The sample rate, set by the application. The range is [5512, 768000]. +* - __u32 + - ``format`` + - The sample format, set by the application. format is defined as +SNDRV_PCM_FORMAT_S8,
[RFC PATCH v3 5/9] ASoC: fsl_easrc: register m2m platform device
Register m2m platform device,that user can use M2M feature. Signed-off-by: Shengjiu Wang --- sound/soc/fsl/fsl_easrc.c | 13 + 1 file changed, 13 insertions(+) diff --git a/sound/soc/fsl/fsl_easrc.c b/sound/soc/fsl/fsl_easrc.c index f517b407672d..b719d517f9b4 100644 --- a/sound/soc/fsl/fsl_easrc.c +++ b/sound/soc/fsl/fsl_easrc.c @@ -2084,6 +2084,7 @@ MODULE_DEVICE_TABLE(of, fsl_easrc_dt_ids); static int fsl_easrc_probe(struct platform_device *pdev) { struct fsl_easrc_priv *easrc_priv; + struct fsl_asrc_m2m_pdata m2m_pdata; struct device *dev = >dev; struct fsl_asrc *easrc; struct resource *res; @@ -2202,11 +2203,23 @@ static int fsl_easrc_probe(struct platform_device *pdev) return ret; } + m2m_pdata.asrc = easrc; + easrc->m2m_pdev = platform_device_register_data(>dev, + M2M_DRV_NAME, + PLATFORM_DEVID_AUTO, + _pdata, + sizeof(m2m_pdata)); + return 0; } static void fsl_easrc_remove(struct platform_device *pdev) { + struct fsl_asrc *easrc = dev_get_drvdata(>dev); + + if (easrc->m2m_pdev && !IS_ERR(easrc->m2m_pdev)) + platform_device_unregister(easrc->m2m_pdev); + pm_runtime_disable(>dev); } -- 2.34.1
[RFC PATCH v3 4/9] ASoC: fsl_asrc: register m2m platform device
Register m2m platform device, that user can use M2M feature. Defined platform data structure and platform driver name. Signed-off-by: Shengjiu Wang --- include/sound/fsl_asrc_common.h | 12 sound/soc/fsl/fsl_asrc.c| 12 2 files changed, 24 insertions(+) diff --git a/include/sound/fsl_asrc_common.h b/include/sound/fsl_asrc_common.h index 7f7e725075fe..e978a2f9cd24 100644 --- a/include/sound/fsl_asrc_common.h +++ b/include/sound/fsl_asrc_common.h @@ -69,6 +69,7 @@ struct fsl_asrc_pair { * @dma_params_rx: DMA parameters for receive channel * @dma_params_tx: DMA parameters for transmit channel * @pdev: platform device pointer + * @m2m_pdev: m2m platform device pointer * @regmap: regmap handler * @paddr: physical address to the base address of registers * @mem_clk: clock source to access register @@ -104,6 +105,7 @@ struct fsl_asrc { struct snd_dmaengine_dai_dma_data dma_params_rx; struct snd_dmaengine_dai_dma_data dma_params_tx; struct platform_device *pdev; + struct platform_device *m2m_pdev; struct regmap *regmap; unsigned long paddr; struct clk *mem_clk; @@ -144,6 +146,16 @@ struct fsl_asrc { void *private; }; +/** + * struct fsl_asrc_m2m_pdata - platform data + * @asrc: pointer to struct fsl_asrc + * + */ +struct fsl_asrc_m2m_pdata { + struct fsl_asrc *asrc; +}; + +#define M2M_DRV_NAME "fsl_asrc_m2m" #define DRV_NAME "fsl-asrc-dai" extern struct snd_soc_component_driver fsl_asrc_component; diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c index f9d830e0957f..7b72d6bcf281 100644 --- a/sound/soc/fsl/fsl_asrc.c +++ b/sound/soc/fsl/fsl_asrc.c @@ -1208,6 +1208,7 @@ static int fsl_asrc_runtime_suspend(struct device *dev); static int fsl_asrc_probe(struct platform_device *pdev) { struct device_node *np = pdev->dev.of_node; + struct fsl_asrc_m2m_pdata m2m_pdata; struct fsl_asrc_priv *asrc_priv; struct fsl_asrc *asrc; struct resource *res; @@ -1392,6 +1393,12 @@ static int fsl_asrc_probe(struct platform_device *pdev) goto err_pm_get_sync; } + m2m_pdata.asrc = asrc; + asrc->m2m_pdev = platform_device_register_data(>dev, + M2M_DRV_NAME, + PLATFORM_DEVID_AUTO, + _pdata, + sizeof(m2m_pdata)); return 0; err_pm_get_sync: @@ -1404,6 +1411,11 @@ static int fsl_asrc_probe(struct platform_device *pdev) static void fsl_asrc_remove(struct platform_device *pdev) { + struct fsl_asrc *asrc = dev_get_drvdata(>dev); + + if (asrc->m2m_pdev && !IS_ERR(asrc->m2m_pdev)) + platform_device_unregister(asrc->m2m_pdev); + pm_runtime_disable(>dev); if (!pm_runtime_status_suspended(>dev)) fsl_asrc_runtime_suspend(>dev); -- 2.34.1
[RFC PATCH v3 3/9] ASoC: fsl_asrc: move fsl_asrc_common.h to include/sound
Move fsl_asrc_common.h to include/sound that it can be included from other drivers. Signed-off-by: Shengjiu Wang --- {sound/soc/fsl => include/sound}/fsl_asrc_common.h | 0 sound/soc/fsl/fsl_asrc.h | 2 +- sound/soc/fsl/fsl_asrc_dma.c | 2 +- sound/soc/fsl/fsl_easrc.h | 2 +- 4 files changed, 3 insertions(+), 3 deletions(-) rename {sound/soc/fsl => include/sound}/fsl_asrc_common.h (100%) diff --git a/sound/soc/fsl/fsl_asrc_common.h b/include/sound/fsl_asrc_common.h similarity index 100% rename from sound/soc/fsl/fsl_asrc_common.h rename to include/sound/fsl_asrc_common.h diff --git a/sound/soc/fsl/fsl_asrc.h b/sound/soc/fsl/fsl_asrc.h index 1c492eb237f5..66544624de7b 100644 --- a/sound/soc/fsl/fsl_asrc.h +++ b/sound/soc/fsl/fsl_asrc.h @@ -10,7 +10,7 @@ #ifndef _FSL_ASRC_H #define _FSL_ASRC_H -#include "fsl_asrc_common.h" +#include #define ASRC_M2M_INPUTFIFO_WML 0x4 #define ASRC_M2M_OUTPUTFIFO_WML0x2 diff --git a/sound/soc/fsl/fsl_asrc_dma.c b/sound/soc/fsl/fsl_asrc_dma.c index 05a7d1588d20..b034fee3f1f4 100644 --- a/sound/soc/fsl/fsl_asrc_dma.c +++ b/sound/soc/fsl/fsl_asrc_dma.c @@ -12,7 +12,7 @@ #include #include -#include "fsl_asrc_common.h" +#include #define FSL_ASRC_DMABUF_SIZE (256 * 1024) diff --git a/sound/soc/fsl/fsl_easrc.h b/sound/soc/fsl/fsl_easrc.h index bee887c8b4f2..f571647c508f 100644 --- a/sound/soc/fsl/fsl_easrc.h +++ b/sound/soc/fsl/fsl_easrc.h @@ -9,7 +9,7 @@ #include #include -#include "fsl_asrc_common.h" +#include /* EASRC Register Map */ -- 2.34.1
[RFC PATCH v3 1/9] ASoC: fsl_asrc: define functions for memory to memory usage
ASRC can be used on memory to memory case, define several functions for m2m usage. m2m_start_part_one: first part of the start steps m2m_start_part_two: second part of the start steps m2m_stop_part_one: first part of stop steps m2m_stop_part_two: second part of stop steps, optional m2m_check_format: check format is supported or not m2m_calc_out_len: calculate output length according to input length m2m_get_maxburst: burst size for dma m2m_pair_suspend: suspend function of pair, optional. m2m_pair_resume: resume function of pair get_output_fifo_size: get remaining data size in FIFO Signed-off-by: Shengjiu Wang --- sound/soc/fsl/fsl_asrc.c| 150 sound/soc/fsl/fsl_asrc.h| 2 + sound/soc/fsl/fsl_asrc_common.h | 42 + 3 files changed, 194 insertions(+) diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c index b793263291dc..f9d830e0957f 100644 --- a/sound/soc/fsl/fsl_asrc.c +++ b/sound/soc/fsl/fsl_asrc.c @@ -1063,6 +1063,145 @@ static int fsl_asrc_get_fifo_addr(u8 dir, enum asrc_pair_index index) return REG_ASRDx(dir, index); } +/* Get sample numbers in FIFO */ +static unsigned int fsl_asrc_get_output_fifo_size(struct fsl_asrc_pair *pair) +{ + struct fsl_asrc *asrc = pair->asrc; + enum asrc_pair_index index = pair->index; + u32 val; + + regmap_read(asrc->regmap, REG_ASRFST(index), ); + + val &= ASRFSTi_OUTPUT_FIFO_MASK; + + return val >> ASRFSTi_OUTPUT_FIFO_SHIFT; +} + +static int fsl_asrc_m2m_start_part_one(struct fsl_asrc_pair *pair) +{ + struct fsl_asrc_pair_priv *pair_priv = pair->private; + struct fsl_asrc *asrc = pair->asrc; + struct device *dev = >pdev->dev; + struct asrc_config config; + int ret; + + /* fill config */ + config.pair = pair->index; + config.channel_num = pair->channels; + config.input_sample_rate = pair->rate[IN]; + config.output_sample_rate = pair->rate[OUT]; + config.input_format = pair->sample_format[IN]; + config.output_format = pair->sample_format[OUT]; + config.inclk = INCLK_NONE; + config.outclk = OUTCLK_ASRCK1_CLK; + + pair_priv->config = + ret = fsl_asrc_config_pair(pair, true); + if (ret) { + dev_err(dev, "failed to config pair: %d\n", ret); + return ret; + } + + fsl_asrc_start_pair(pair); + + return 0; +} + +static int fsl_asrc_m2m_start_part_two(struct fsl_asrc_pair *pair) +{ + /* +* Clear DMA request during the stall state of ASRC: +* During STALL state, the remaining in input fifo would never be +* smaller than the input threshold while the output fifo would not +* be bigger than output one. Thus the DMA request would be cleared. +*/ + fsl_asrc_set_watermarks(pair, ASRC_FIFO_THRESHOLD_MIN, + ASRC_FIFO_THRESHOLD_MAX); + + /* Update the real input threshold to raise DMA request */ + fsl_asrc_set_watermarks(pair, ASRC_M2M_INPUTFIFO_WML, + ASRC_M2M_OUTPUTFIFO_WML); + + return 0; +} + +static int fsl_asrc_m2m_stop_part_one(struct fsl_asrc_pair *pair) +{ + fsl_asrc_stop_pair(pair); + + return 0; +} + +static int fsl_asrc_m2m_check_format(u8 dir, u32 format) +{ + u64 support_format = FSL_ASRC_FORMATS; + + if (dir == IN) + support_format |= SNDRV_PCM_FMTBIT_S8; + + if (!(1 << format & support_format)) + return -EINVAL; + + return 0; +} + +static int fsl_asrc_m2m_check_rate(u8 dir, u32 rate) +{ + if (rate < 5512 || rate > 192000) + return -EINVAL; + + return 0; +} + +static int fsl_asrc_m2m_check_channel(u8 dir, u32 channels) +{ + if (channels < 1 || channels > 10) + return -EINVAL; + + return 0; +} + +/* calculate capture data length according to output data length and sample rate */ +static int fsl_asrc_m2m_calc_out_len(struct fsl_asrc_pair *pair, int input_buffer_length) +{ + unsigned int in_width, out_width; + unsigned int channels = pair->channels; + unsigned int in_samples, out_samples; + unsigned int out_length; + + in_width = snd_pcm_format_physical_width(pair->sample_format[IN]) / 8; + out_width = snd_pcm_format_physical_width(pair->sample_format[OUT]) / 8; + + in_samples = input_buffer_length / in_width / channels; + out_samples = pair->rate[OUT] * in_samples / pair->rate[IN]; + out_length = (out_samples - ASRC_OUTPUT_LAST_SAMPLE) * out_width * channels; + + return out_length; +} + +static int fsl_asrc_m2m_get_maxburst(u8 dir, struct fsl_asrc_pair *pair) +{ + struct fsl_asrc *asrc = pair->asrc; + struct fsl_asrc_priv *asrc_priv = asrc->private; + int wml = (dir == IN) ? ASRC_M2M_INPUTFIFO_WML : ASRC_M2M_OUTPUTFIFO_WML; + + if (!asrc_priv->soc->use_edma) + return
[RFC PATCH v3 2/9] ASoC: fsl_easrc: define functions for memory to memory usage
ASRC can be used on memory to memory case, define several functions for m2m usage and export them as function pointer. Signed-off-by: Shengjiu Wang --- sound/soc/fsl/fsl_easrc.c | 226 ++ sound/soc/fsl/fsl_easrc.h | 6 + 2 files changed, 232 insertions(+) diff --git a/sound/soc/fsl/fsl_easrc.c b/sound/soc/fsl/fsl_easrc.c index ba62995c909a..f517b407672d 100644 --- a/sound/soc/fsl/fsl_easrc.c +++ b/sound/soc/fsl/fsl_easrc.c @@ -1861,6 +1861,220 @@ static int fsl_easrc_get_fifo_addr(u8 dir, enum asrc_pair_index index) return REG_EASRC_FIFO(dir, index); } +/* Get sample numbers in FIFO */ +static unsigned int fsl_easrc_get_output_fifo_size(struct fsl_asrc_pair *pair) +{ + struct fsl_asrc *asrc = pair->asrc; + enum asrc_pair_index index = pair->index; + u32 val; + + regmap_read(asrc->regmap, REG_EASRC_SFS(index), ); + val &= EASRC_SFS_NSGO_MASK; + + return val >> EASRC_SFS_NSGO_SHIFT; +} + +static int fsl_easrc_m2m_start_part_one(struct fsl_asrc_pair *pair) +{ + struct fsl_easrc_ctx_priv *ctx_priv = pair->private; + struct fsl_asrc *asrc = pair->asrc; + struct device *dev = >pdev->dev; + int ret; + + ctx_priv->in_params.sample_rate = pair->rate[IN]; + ctx_priv->in_params.sample_format = pair->sample_format[IN]; + ctx_priv->out_params.sample_rate = pair->rate[OUT]; + ctx_priv->out_params.sample_format = pair->sample_format[OUT]; + + ctx_priv->in_params.fifo_wtmk = FSL_EASRC_INPUTFIFO_WML; + ctx_priv->out_params.fifo_wtmk = FSL_EASRC_OUTPUTFIFO_WML; + /* Fill the right half of the re-sampler with zeros */ + ctx_priv->rs_init_mode = 0x2; + /* Zero fill the right half of the prefilter */ + ctx_priv->pf_init_mode = 0x2; + + ret = fsl_easrc_set_ctx_format(pair, + _priv->in_params.sample_format, + _priv->out_params.sample_format); + if (ret) { + dev_err(dev, "failed to set context format: %d\n", ret); + return ret; + } + + ret = fsl_easrc_config_context(asrc, pair->index); + if (ret) { + dev_err(dev, "failed to config context %d\n", ret); + return ret; + } + + ctx_priv->in_params.iterations = 1; + ctx_priv->in_params.group_len = pair->channels; + ctx_priv->in_params.access_len = pair->channels; + ctx_priv->out_params.iterations = 1; + ctx_priv->out_params.group_len = pair->channels; + ctx_priv->out_params.access_len = pair->channels; + + ret = fsl_easrc_set_ctx_organziation(pair); + if (ret) { + dev_err(dev, "failed to set fifo organization\n"); + return ret; + } + + /* The context start flag */ + ctx_priv->first_convert = 1; + return 0; +} + +static int fsl_easrc_m2m_start_part_two(struct fsl_asrc_pair *pair) +{ + struct fsl_easrc_ctx_priv *ctx_priv = pair->private; + /* start context once */ + if (ctx_priv->first_convert) { + fsl_easrc_start_context(pair); + ctx_priv->first_convert = 0; + } + + return 0; +} + +static int fsl_easrc_m2m_stop_part_two(struct fsl_asrc_pair *pair) +{ + struct fsl_easrc_ctx_priv *ctx_priv = pair->private; + /* Stop pair/context */ + if (!ctx_priv->first_convert) { + fsl_easrc_stop_context(pair); + ctx_priv->first_convert = 1; + } + + return 0; +} + +static int fsl_easrc_m2m_check_format(u8 dir, u32 format) +{ + u64 support_format = FSL_EASRC_FORMATS; + + if (dir == OUT) + support_format |= SNDRV_PCM_FMTBIT_IEC958_SUBFRAME_LE; + + if (!(1 << format & support_format)) + return -EINVAL; + + return 0; +} + +static int fsl_easrc_m2m_check_rate(u8 dir, u32 rate) +{ + if (rate < 8000 || rate > 768000) + return -EINVAL; + + return 0; +} + +static int fsl_easrc_m2m_check_channel(u8 dir, u32 channels) +{ + if (channels < 1 || channels > 32) + return -EINVAL; + + return 0; +} + +/* calculate capture data length according to output data length and sample rate */ +static int fsl_easrc_m2m_calc_out_len(struct fsl_asrc_pair *pair, int input_buffer_length) +{ + struct fsl_asrc *easrc = pair->asrc; + struct fsl_easrc_priv *easrc_priv = easrc->private; + struct fsl_easrc_ctx_priv *ctx_priv = pair->private; + unsigned int in_rate = ctx_priv->in_params.norm_rate; + unsigned int out_rate = ctx_priv->out_params.norm_rate; + unsigned int channels = pair->channels; + unsigned int in_samples, out_samples; + unsigned int in_width, out_width; + unsigned int out_length; + unsigned int frac_bits; + u64 val1, val2; + + switch (easrc_priv->rs_num_taps) { + case EASRC_RS_32_TAPS: +
[RFC PATCH v3 0/9] Add audio support in v4l2 framework
Audio signal processing also has the requirement for memory to memory similar as Video. This asrc memory to memory (memory ->asrc->memory) case is a non real time use case. User fills the input buffer to the asrc module, after conversion, then asrc sends back the output buffer to user. So it is not a traditional ALSA playback and capture case. It is a specific use case, there is no reference in current kernel. v4l2 memory to memory is the closed implementation, v4l2 current support video, image, radio, tuner, touch devices, so it is not complicated to add support for this specific audio case. Because we had implemented the "memory -> asrc ->i2s device-> codec" use case in ALSA. Now the "memory->asrc->memory" needs to reuse the code in asrc driver, so the first 3 patches is for refining the code to make it can be shared by the "memory->asrc->memory" driver. The main change is in the v4l2 side, A /dev/vl4-audioX will be created, user applications only use the ioctl of v4l2 framework. Other change is to add memory to memory support for two kinds of i.MX ASRC module. changes in v3: - Modify documents for adding audio m2m support - Add audio virtual m2m driver - Defined V4L2_AUDIO_FMT_LPCM format type for audio. - Defined V4L2_CAP_AUDIO_M2M capability type for audio m2m case. - with modification in v4l-utils, pass v4l2-compliance test. changes in v2: - decouple the implementation in v4l2 and ALSA - implement the memory to memory driver as a platfrom driver and move it to driver/media - move fsl_asrc_common.h to include/sound folder Shengjiu Wang (9): ASoC: fsl_asrc: define functions for memory to memory usage ASoC: fsl_easrc: define functions for memory to memory usage ASoC: fsl_asrc: move fsl_asrc_common.h to include/sound ASoC: fsl_asrc: register m2m platform device ASoC: fsl_easrc: register m2m platform device media: v4l2: Add audio capture and output support media: uapi: Add V4L2_CID_USER_IMX_ASRC_RATIO_MOD control media: audm2m: add virtual driver for audio memory to memory media: imx-asrc: Add memory to memory driver .../userspace-api/media/v4l/audio-formats.rst | 15 + .../userspace-api/media/v4l/buffer.rst|6 + .../userspace-api/media/v4l/control.rst |5 + .../userspace-api/media/v4l/dev-audio.rst | 63 + .../userspace-api/media/v4l/devices.rst |1 + .../media/v4l/pixfmt-aud-lpcm.rst | 31 + .../userspace-api/media/v4l/pixfmt.rst|1 + .../media/v4l/vidioc-enum-fmt.rst |2 + .../userspace-api/media/v4l/vidioc-g-fmt.rst |4 + .../media/v4l/vidioc-querycap.rst |3 + .../media/videodev2.h.rst.exceptions |2 + .../media/common/videobuf2/videobuf2-v4l2.c |4 + drivers/media/platform/nxp/Kconfig| 12 + drivers/media/platform/nxp/Makefile |1 + drivers/media/platform/nxp/imx-asrc.c | 1058 + drivers/media/test-drivers/Kconfig|9 + drivers/media/test-drivers/Makefile |1 + drivers/media/test-drivers/audm2m.c | 767 drivers/media/v4l2-core/v4l2-ctrls-defs.c |1 + drivers/media/v4l2-core/v4l2-dev.c| 17 + drivers/media/v4l2-core/v4l2-ioctl.c | 53 + include/media/v4l2-dev.h |2 + include/media/v4l2-ioctl.h| 34 + .../fsl => include/sound}/fsl_asrc_common.h | 54 + include/uapi/linux/v4l2-controls.h|1 + include/uapi/linux/videodev2.h| 25 + sound/soc/fsl/fsl_asrc.c | 162 +++ sound/soc/fsl/fsl_asrc.h |4 +- sound/soc/fsl/fsl_asrc_dma.c |2 +- sound/soc/fsl/fsl_easrc.c | 239 sound/soc/fsl/fsl_easrc.h |8 +- 31 files changed, 2584 insertions(+), 3 deletions(-) create mode 100644 Documentation/userspace-api/media/v4l/audio-formats.rst create mode 100644 Documentation/userspace-api/media/v4l/dev-audio.rst create mode 100644 Documentation/userspace-api/media/v4l/pixfmt-aud-lpcm.rst create mode 100644 drivers/media/platform/nxp/imx-asrc.c create mode 100644 drivers/media/test-drivers/audm2m.c rename {sound/soc/fsl => include/sound}/fsl_asrc_common.h (60%) -- 2.34.1
Re: [PATCH v4 8/8] bpf ppc32: Access only if addr is kernel address
Hi, Le 29/09/2021 à 13:18, Hari Bathini a écrit : > With KUAP enabled, any kernel code which wants to access userspace > needs to be surrounded by disable-enable KUAP. But that is not > happening for BPF_PROBE_MEM load instruction. Though PPC32 does not > support read protection, considering the fact that PTR_TO_BTF_ID > (which uses BPF_PROBE_MEM mode) could either be a valid kernel pointer > or NULL but should never be a pointer to userspace address, execute > BPF_PROBE_MEM load only if addr is kernel address, otherwise set > dst_reg=0 and move on. While looking at the series "bpf: verifier: stop emitting zext for LDX" from Puranjay I got a question on this old commit, see below. > > This will catch NULL, valid or invalid userspace pointers. Only bad > kernel pointer will be handled by BPF exception table. > > [Alexei suggested for x86] > Suggested-by: Alexei Starovoitov > Signed-off-by: Hari Bathini > --- > > Changes in v4: > * Adjusted the emit code to avoid using temporary reg. > > > arch/powerpc/net/bpf_jit_comp32.c | 34 +++ > 1 file changed, 34 insertions(+) > > diff --git a/arch/powerpc/net/bpf_jit_comp32.c > b/arch/powerpc/net/bpf_jit_comp32.c > index 6ee13a09c70d..2ac81563c78d 100644 > --- a/arch/powerpc/net/bpf_jit_comp32.c > +++ b/arch/powerpc/net/bpf_jit_comp32.c > @@ -818,6 +818,40 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, > struct codegen_context * > case BPF_LDX | BPF_PROBE_MEM | BPF_W: > case BPF_LDX | BPF_MEM | BPF_DW: /* dst = *(u64 *)(ul) (src + > off) */ > case BPF_LDX | BPF_PROBE_MEM | BPF_DW: > + /* > + * As PTR_TO_BTF_ID that uses BPF_PROBE_MEM mode could > either be a valid > + * kernel pointer or NULL but not a userspace address, > execute BPF_PROBE_MEM > + * load only if addr is kernel address (see > is_kernel_addr()), otherwise > + * set dst_reg=0 and move on. > + */ > + if (BPF_MODE(code) == BPF_PROBE_MEM) { > + PPC_LI32(_R0, TASK_SIZE - off); > + EMIT(PPC_RAW_CMPLW(src_reg, _R0)); > + PPC_BCC(COND_GT, (ctx->idx + 5) * 4); > + EMIT(PPC_RAW_LI(dst_reg, 0)); > + /* > + * For BPF_DW case, "li reg_h,0" would be > needed when > + * !fp->aux->verifier_zext. Emit NOP otherwise. > + * > + * Note that "li reg_h,0" is emitted for > BPF_B/H/W case, > + * if necessary. So, jump there insted of > emitting an > + * additional "li reg_h,0" instruction. > + */ > + if (size == BPF_DW && !fp->aux->verifier_zext) > + EMIT(PPC_RAW_LI(dst_reg_h, 0)); > + else > + EMIT(PPC_RAW_NOP()); While do you need a NOP in the else case ? Can't we just emit no instruction in that case ? > + /* > + * Need to jump two instructions instead of one > for BPF_DW case > + * as there are two load instructions for > dst_reg_h & dst_reg > + * respectively. > + */ > + if (size == BPF_DW) > + PPC_JMP((ctx->idx + 3) * 4); > + else > + PPC_JMP((ctx->idx + 2) * 4); > + } > + > switch (size) { > case BPF_B: > EMIT(PPC_RAW_LBZ(dst_reg, src_reg, off));
Re: [PATCH] powerpc: add `cur_cpu_spec` symbol to vmcoreinfo
Hello Sachin, On Wed, Sep 13, 2023 at 06:19:11PM +0530, Sachin Sant wrote: > > > > On 11-Sep-2023, at 2:44 PM, Aditya Gupta wrote: > > > > Presently, while reading a vmcore, makedumpfile uses > > `cur_cpu_spec.mmu_features` to decide whether the crashed system had > > RADIX MMU or not. > > > > Currently, makedumpfile fails to get the `cur_cpu_spec` symbol (unless > > a vmlinux is passed with the `-x` flag to makedumpfile), and hence > > assigns offsets and shifts (such as pgd_offset_l4) incorrecly considering > > MMU to be hash MMU. > > > > Add `cur_cpu_spec` symbol and offset of `mmu_features` in the > > `cpu_spec` struct, to VMCOREINFO, so that the symbol address and offset > > is accessible to makedumpfile, without needing the vmlinux file > > > > Signed-off-by: Aditya Gupta > > --- > > Thanks for the patch. With this patch applied (along with makedumpfile > changes) > I am able to capture vmcore against a kernel which contains commit > 8dc9a0ad0c3e > > Reported-by: Sachin Sant > Tested-by: Sachin Sant Thanks for testing this, and for providing the tags. - Aditya Gupta