Re: [PATCH V3 2/2] tools/perf/tests: Fix object code reading to skip address that falls out of text section

2023-09-14 Thread Athira Rajeev



> 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

2023-09-14 Thread Athira Rajeev
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

2023-09-14 Thread Athira Rajeev
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

2023-09-14 Thread Guenter Roeck

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

2023-09-14 Thread Adrian Hunter
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

2023-09-14 Thread Kees Cook
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

2023-09-14 Thread Athira Rajeev
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

2023-09-14 Thread Athira Rajeev
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

2023-09-14 Thread Athira Rajeev



> 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

2023-09-14 Thread Athira Rajeev



> 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

2023-09-14 Thread Christophe Leroy


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

2023-09-14 Thread Benjamin Gray
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

2023-09-14 Thread Yuanjun Gong
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

2023-09-14 Thread Michael Ellerman
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

2023-09-14 Thread Michael Ellerman
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

2023-09-14 Thread Michael Ellerman
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

2023-09-14 Thread Sean Christopherson
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

2023-09-14 Thread Justin Stitt
`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

2023-09-14 Thread Ackerley Tng
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

2023-09-14 Thread Edgecombe, Rick P
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

2023-09-14 Thread Maciej W. Rozycki
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

2023-09-14 Thread Sohil Mehta
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

2023-09-14 Thread John Ogness
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

2023-09-14 Thread Yuan Tan

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

2023-09-14 Thread Michael Ellerman
"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

2023-09-14 Thread Disha Goel

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

2023-09-14 Thread Kent Overstreet
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

2023-09-14 Thread Randy Dunlap



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

2023-09-14 Thread Sean Christopherson
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

2023-09-14 Thread John Ogness
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

2023-09-14 Thread John Ogness
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

2023-09-14 Thread Adrian Hunter
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

2023-09-14 Thread Adrian Hunter
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

2023-09-14 Thread Sean Christopherson
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

2023-09-14 Thread Athira Rajeev



> 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

2023-09-14 Thread Athira Rajeev



> 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

2023-09-14 Thread Athira Rajeev
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

2023-09-14 Thread Athira Rajeev
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

2023-09-14 Thread Athira Rajeev
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

2023-09-14 Thread Athira Rajeev
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 "@"

2023-09-14 Thread Athira Rajeev
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

2023-09-14 Thread Tyrel Datwyler
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

2023-09-14 Thread Christophe Leroy
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

2023-09-14 Thread Sachin Sant



> 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

2023-09-14 Thread Mark Brown
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

2023-09-14 Thread Sean Christopherson
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

2023-09-14 Thread Michael Ellerman
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

2023-09-14 Thread Michael Ellerman
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

2023-09-14 Thread Michael Ellerman
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

2023-09-14 Thread Michael Ellerman
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)

2023-09-14 Thread Erhard Furtner
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

2023-09-14 Thread Michael Ellerman
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

2023-09-14 Thread Michal Suchánek
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

2023-09-14 Thread Sakari Ailus
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

2023-09-14 Thread Yuan Tan
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

2023-09-14 Thread Yuan Tan
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

2023-09-14 Thread Yuan Tan
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

2023-09-14 Thread Yuan Tan
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

2023-09-14 Thread Yuanjun Gong
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

2023-09-14 Thread Hari Bathini




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

2023-09-14 Thread Yuan Tan



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

2023-09-14 Thread Shengjiu Wang
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

2023-09-14 Thread Shengjiu Wang
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

2023-09-14 Thread Shengjiu Wang
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

2023-09-14 Thread Shengjiu Wang
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

2023-09-14 Thread Shengjiu Wang
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

2023-09-14 Thread Shengjiu Wang
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

2023-09-14 Thread Shengjiu Wang
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

2023-09-14 Thread Shengjiu Wang
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

2023-09-14 Thread Shengjiu Wang
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

2023-09-14 Thread Shengjiu Wang
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

2023-09-14 Thread Christophe Leroy
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

2023-09-14 Thread Aditya Gupta
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