[PATCH v2] powerpc/pseries/vas: Use usleep_range() to support HCALL delay

[Haren Myneni]

VAS allocate, modify and deallocate HCALLs returns
H_LONG_BUSY_ORDER_1_MSEC or H_LONG_BUSY_ORDER_10_MSEC for busy
delay and expects OS to reissue HCALL after that delay. But using
msleep() will often sleep at least 20 msecs even though the
hypervisor expects to reissue these HCALLs after 1 or 10msecs.
It might cause these HCALLs takes longer when multiple threads
issue open or close VAS windows simultaneously.

So instead of msleep(), use usleep_range() to ensure sleep with
the expected value before issuing HCALL again.

Signed-off-by: Haren Myneni 
Suggested-by: Nathan Lynch 

---
v1 -> v2:
- Use usleep_range instead of using RTAS sleep routine as
  suggested by Nathan
---
 arch/powerpc/platforms/pseries/vas.c | 24 +++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/pseries/vas.c 
b/arch/powerpc/platforms/pseries/vas.c
index 71d52a670d95..bade4402741f 100644
--- a/arch/powerpc/platforms/pseries/vas.c
+++ b/arch/powerpc/platforms/pseries/vas.c
@@ -36,9 +36,31 @@ static bool migration_in_progress;
 
 static long hcall_return_busy_check(long rc)
 {
+   unsigned int ms;
+
/* Check if we are stalled for some time */
if (H_IS_LONG_BUSY(rc)) {
-   msleep(get_longbusy_msecs(rc));
+   ms = get_longbusy_msecs(rc);
+   /*
+* Allocate, Modify and Deallocate HCALLs returns
+* H_LONG_BUSY_ORDER_1_MSEC or H_LONG_BUSY_ORDER_10_MSEC
+* for the long delay. So the delay should always be 1
+* or 10msecs, but sleeps 1msec in case if the long
+* delay is > H_LONG_BUSY_ORDER_10_MSEC.
+*/
+   if (ms > 10)
+   ms = 1;
+
+   /*
+* msleep() will often sleep at least 20 msecs even
+* though the hypervisor expects to reissue these
+* HCALLs after 1 or 10msecs. So use usleep_range()
+* to sleep with the expected value.
+*
+* See Documentation/timers/timers-howto.rst on using
+* the value range in usleep_range().
+*/
+   usleep_range(ms * 100, ms * 1000);
rc = H_BUSY;
} else if (rc == H_BUSY) {
cond_resched();
-- 
2.26.3

Re: [PATCH v1] powerpc: Add PVN support for HeXin C2000 processor

[Qu Shenghui 瞿盛辉]

Hi, Nick

Thanks for your comments.

On 2023/11/25 7:35, Nicholas Piggin wrote:

On Thu Nov 23, 2023 at 7:36 PM AEST, Zhao Ke wrote:

HeXin Tech Co. has applied for a new PVN from the OpenPower Community
for its new processor C2000. The OpenPower has assigned a new PVN
and this newly assigned PVN is 0x0066, add pvr register related
support for this PVN.

Signed-off-by: Zhao Ke
Link:https://discuss.openpower.foundation/t/how-to-get-a-new-pvr-for-processors-follow-power-isa/477/10
---
v0 -> v1:
- Fix .cpu_name with the correct description
---
---
  arch/powerpc/include/asm/reg.h|  1 +
  arch/powerpc/kernel/cpu_specs_book3s_64.h | 15 +++
  arch/powerpc/kvm/book3s_pr.c  |  1 +
  arch/powerpc/mm/book3s64/pkeys.c  |  3 ++-
  arch/powerpc/platforms/powernv/subcore.c  |  3 ++-
  drivers/misc/cxl/cxl.h|  3 ++-
  6 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
index 4ae4ab9090a2..7fd09f25452d 100644
--- a/arch/powerpc/include/asm/reg.h
+++ b/arch/powerpc/include/asm/reg.h
@@ -1361,6 +1361,7 @@
  #define PVR_POWER8E   0x004B
  #define PVR_POWER8NVL 0x004C
  #define PVR_POWER80x004D
+#define PVR_HX_C2000   0x0066
  #define PVR_POWER90x004E
  #define PVR_POWER10   0x0080
  #define PVR_BE0x0070
diff --git a/arch/powerpc/kernel/cpu_specs_book3s_64.h 
b/arch/powerpc/kernel/cpu_specs_book3s_64.h
index c370c1b804a9..367c9f6d9be5 100644
--- a/arch/powerpc/kernel/cpu_specs_book3s_64.h
+++ b/arch/powerpc/kernel/cpu_specs_book3s_64.h
@@ -238,6 +238,21 @@ static struct cpu_spec cpu_specs[] __initdata = {
.machine_check_early= __machine_check_early_realmode_p8,
.platform   = "power8",
},
+   {   /* 2.07-compliant processor, HeXin C2000 processor */
+   .pvr_mask   = 0x,
+   .pvr_value  = 0x0066,
+   .cpu_name   = "POWER8 (raw)",

If this is a raw mode, it should go with the raw POWER8 entry.
The raw vs architected entries are already out of order with
POWER6, but we should fix that too.

You may want your PVR mask to follow the other raw examples too,
but it depends on how you foresee PVR being used. Using 0x
allows you to increment the low part of the PVR and existing
kernels will continue to match it. You can then add a specific
match for the older version if you need to add special handling
for it (e.g., see how POWER9 is handled).

Do you want .cpu_name to be "POWER8 (raw)"? You could call it
"HX-C2000", as Michael suggested earlier.


**Following your suggestion, we have discussed internally and decided to 
modify the cpu_name and pvr_mask.





+   .cpu_features   = CPU_FTRS_POWER8,
+   .cpu_user_features  = COMMON_USER_POWER8,
+   .cpu_user_features2 = COMMON_USER2_POWER8,
+   .mmu_features   = MMU_FTRS_POWER8,
+   .icache_bsize   = 128,
+   .dcache_bsize   = 128,
+   .cpu_setup  = __setup_cpu_power8,
+   .cpu_restore= __restore_cpu_power8,
+   .machine_check_early= __machine_check_early_realmode_p8,
+   .platform   = "power8",
+   },
{   /* 3.00-compliant processor, i.e. Power9 "architected" mode */
.pvr_mask   = 0x,
.pvr_value  = 0x0f05,
diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c
index 9118242063fb..5b92619a05fd 100644
--- a/arch/powerpc/kvm/book3s_pr.c
+++ b/arch/powerpc/kvm/book3s_pr.c
@@ -604,6 +604,7 @@ static void kvmppc_set_pvr_pr(struct kvm_vcpu *vcpu, u32 
pvr)
case PVR_POWER8:
case PVR_POWER8E:
case PVR_POWER8NVL:
+   case PVR_HX_C2000:
case PVR_POWER9:
vcpu->arch.hflags |= BOOK3S_HFLAG_MULTI_PGSIZE |
BOOK3S_HFLAG_NEW_TLBIE;
diff --git a/arch/powerpc/mm/book3s64/pkeys.c b/arch/powerpc/mm/book3s64/pkeys.c
index 125733962033..c38f378e1942 100644
--- a/arch/powerpc/mm/book3s64/pkeys.c
+++ b/arch/powerpc/mm/book3s64/pkeys.c
@@ -89,7 +89,8 @@ static int __init scan_pkey_feature(void)
unsigned long pvr = mfspr(SPRN_PVR);
  
  			if (PVR_VER(pvr) == PVR_POWER8 || PVR_VER(pvr) == PVR_POWER8E ||

-   PVR_VER(pvr) == PVR_POWER8NVL || PVR_VER(pvr) == 
PVR_POWER9)
+   PVR_VER(pvr) == PVR_POWER8NVL || PVR_VER(pvr) == 
PVR_POWER9 ||
+   PVR_VER(pvr) == PVR_HX_C2000)
pkeys_total = 32;
}
}
diff --git a/arch/powerpc/platforms/powernv/subcore.c 
b/arch/powerpc/platforms/powernv/subcore.c
index 191424468f10..58e7331e1e7e 100644
--- a/arch/powerpc/platforms/powe

Re: [PATCH] perf vendor events: Update datasource event name to fix duplicate events

[Athira Rajeev]

> On 27-Nov-2023, at 5:32 PM, Disha Goel  wrote:
> 
> On 23/11/23 9:31 pm, Athira Rajeev wrote:
> 
>> Running "perf list" on powerpc fails with segfault
>> as below:
>> 
>>./perf list
>>Segmentation fault (core dumped)
>> 
>> This happens because of duplicate events in the json list.
>> The powerpc Json event list contains some event with same
>> event name, but different event code. They are:
>> - PM_INST_FROM_L3MISS (Present in datasource and frontend)
>> - PM_MRK_DATA_FROM_L2MISS (Present in datasource and marked)
>> - PM_MRK_INST_FROM_L3MISS (Present in datasource and marked)
>> - PM_MRK_DATA_FROM_L3MISS (Present in datasource and marked)
>> 
>> pmu_events_table__num_events uses the value from
>> table_pmu->num_entries which includes duplicate events as
>> well. This causes issue during "perf list" and results in
>> segmentation fault.
>> 
>> Since both event codes are valid, append _DSRC to the Data
>> Source events (datasource.json), so that they would have a
>> unique name. Also add PM_DATA_FROM_L2MISS_DSRC and
>> PM_DATA_FROM_L3MISS_DSRC events. With the fix, perf list
>> works as expected.
>> 
>> Fixes: fc1435807533 ("perf vendor events power10: Update JSON/events")
>> Signed-off-by: Athira Rajeev 
> 
> I have tested the patch on Power10 machine. Perf list works correctly without 
> any segfault now.
> 
> # ./perf list
> 
> List of pre-defined events (to be used in -e or -M):
> 
>   branch-instructions OR branches[Hardware event]
>   branch-misses  [Hardware event]
> 
> Tested-by: Disha Goel 
> 

Thanks Disha for testing

Athira
>> ---
>>  .../arch/powerpc/power10/datasource.json   | 18 ++
>>  1 file changed, 14 insertions(+), 4 deletions(-)
>> 
>> diff --git a/tools/perf/pmu-events/arch/powerpc/power10/datasource.json 
>> b/tools/perf/pmu-events/arch/powerpc/power10/datasource.json
>> index 6b0356f2d301..0eeaaf1a95b8 100644
>> --- a/tools/perf/pmu-events/arch/powerpc/power10/datasource.json
>> +++ b/tools/perf/pmu-events/arch/powerpc/power10/datasource.json
>> @@ -99,6 +99,11 @@
>>  "EventName": "PM_INST_FROM_L2MISS",
>>  "BriefDescription": "The processor's instruction cache was reloaded 
>> from a source beyond the local core's L2 due to a demand miss."
>>},
>> +  {
>> +"EventCode": "0x0003C000C040",
>> +"EventName": "PM_DATA_FROM_L2MISS_DSRC",
>> +"BriefDescription": "The processor's L1 data cache was reloaded from a 
>> source beyond the local core's L2 due to a demand miss."
>> +  },
>>{
>>  "EventCode": "0x00038010C040",
>>  "EventName": "PM_INST_FROM_L2MISS_ALL",
>> @@ -161,9 +166,14 @@
>>},
>>{
>>  "EventCode": "0x00078000C040",
>> -"EventName": "PM_INST_FROM_L3MISS",
>> +"EventName": "PM_INST_FROM_L3MISS_DSRC",
>>  "BriefDescription": "The processor's instruction cache was reloaded 
>> from beyond the local core's L3 due to a demand miss."
>>},
>> +  {
>> +"EventCode": "0x0007C000C040",
>> +"EventName": "PM_DATA_FROM_L3MISS_DSRC",
>> +"BriefDescription": "The processor's L1 data cache was reloaded from 
>> beyond the local core's L3 due to a demand miss."
>> +  },
>>{
>>  "EventCode": "0x00078010C040",
>>  "EventName": "PM_INST_FROM_L3MISS_ALL",
>> @@ -981,7 +991,7 @@
>>},
>>{
>>  "EventCode": "0x0003C000C142",
>> -"EventName": "PM_MRK_DATA_FROM_L2MISS",
>> +"EventName": "PM_MRK_DATA_FROM_L2MISS_DSRC",
>>  "BriefDescription": "The processor's L1 data cache was reloaded from a 
>> source beyond the local core's L2 due to a demand miss for a marked 
>> instruction."
>>},
>>{
>> @@ -1046,12 +1056,12 @@
>>},
>>{
>>  "EventCode": "0x00078000C142",
>> -"EventName": "PM_MRK_INST_FROM_L3MISS",
>> +"EventName": "PM_MRK_INST_FROM_L3MISS_DSRC",
>>  "BriefDescription": "The processor's instruction cache was reloaded 
>> from beyond the local core's L3 due to a demand miss for a marked 
>> instruction."
>>},
>>{
>>  "EventCode": "0x0007C000C142",
>> -"EventName": "PM_MRK_DATA_FROM_L3MISS",
>> +"EventName": "PM_MRK_DATA_FROM_L3MISS_DSRC",
>>  "BriefDescription": "The processor's L1 data cache was reloaded from 
>> beyond the local core's L3 due to a demand miss for a marked instruction."
>>},
>>{

Re: [PATCH] powerpc: Add PVN support for HeXin C2000 processor

[Michael Ellerman]

Zhao Ke 赵 可  writes:
> On 2023/11/22 9:46, Michael Ellerman wrote:
>> Zhao Ke  writes:
>>> HeXin Tech Co. has applied for a new PVN from the OpenPower Community
>>> for its new processor C2000. The OpenPower has assigned a new PVN
>>> and this newly assigned PVN is 0x0066, add pvr register related
>>> support for this PVN.
>>>
>>> Signed-off-by: Zhao Ke 
>>> Link: 
>>> https://discuss.openpower.foundation/t/how-to-get-a-new-pvr-for-processors-follow-power-isa/477/10
>>   
>> Hi Zhao Ke,
>>
>> Thanks for the patch. Just a few questions.
>>
>> Are you able to provide any further detail on the processor?
>>
>> Your cputable entry claims that it's identical to the original Power8
>> core, can you comment at all on how true that is in practice?
>
> Basically, we made lots of design change for the new processor.
>
> For example:
>
>      1. redesign the interconnect of the fabric, from crossbar to mesh
>
>      2. redesign the memory subsystem, including the modification of L2 
> and L3 architecture
>
>      3. redesign the SMP bus
>
>      4. upgrade PCIe to gen5 and increase the number of lanes
>
>      5. upgrade ddr to DDR5, dimm direct connected, and the number of 
> channels
>
>      6. redesign the pervasive architecture, including debug/trace, 
> clock&power management, etc.

OK thanks for the detail.

Given all those changes I think you should not use "Power8" as the CPU
name. Whatever the lineage of the core design is, it's no longer a
literal "Power8", not even the same design using a different process
node.

So I think you should call it "HeXin C2000" or similar.

cheers

Re: [PATCH] perf test record+probe_libc_inet_pton: Fix call chain match on powerpc

[Athira Rajeev]

> On 26-Nov-2023, at 12:39 PM, Likhitha Korrapati  
> wrote:
> 
> The perf test "probe libc's inet_pton & backtrace it with ping" fails on
> powerpc as below:
> 
> root@xxx perf]# perf test -v "probe libc's inet_pton & backtrace it with
> ping"
> 85: probe libc's inet_pton & backtrace it with ping :
> --- start ---
> test child forked, pid 96028
> ping 96056 [002] 127271.101961: probe_libc:inet_pton: (7fffa1779a60)
> 7fffa1779a60 __GI___inet_pton+0x0
> (/usr/lib64/glibc-hwcaps/power10/libc.so.6)
> 7fffa172a73c getaddrinfo+0x121c
> (/usr/lib64/glibc-hwcaps/power10/libc.so.6)
> FAIL: expected backtrace entry
> "gaih_inet.*\+0x[[:xdigit:]]+[[:space:]]\(/usr/lib64/glibc-hwcaps/power10/libc.so.6\)$"
> got "7fffa172a73c getaddrinfo+0x121c
> (/usr/lib64/glibc-hwcaps/power10/libc.so.6)"
> test child finished with -1
>  end 
> probe libc's inet_pton & backtrace it with ping: FAILED!

Reviewed-by: Athira Rajeev 

Thanks
Athira
> 
> This test installs a probe on libc's inet_pton function, which will use
> uprobes and then uses perf trace on a ping to localhost. It gets 3
> levels deep backtrace and checks whether it is what we expected or not.
> 
> The test started failing from RHEL 9.4 where as it works in previous
> distro version (RHEL 9.2). Test expects gaih_inet function to be part of
> backtrace. But in the glibc version (2.34-86) which is part of distro
> where it fails, this function is missing and hence the test is failing.
> 
> From nm and ping command output we can confirm that gaih_inet function
> is not present in the expected backtrace for glibc version glibc-2.34-86
> 
> [root@xxx perf]# nm /usr/lib64/glibc-hwcaps/power10/libc.so.6 | grep gaih_inet
> 001273e0 t gaih_inet_serv
> 001cd8d8 r gaih_inet_typeproto
> 
> [root@xxx perf]# perf script -i /tmp/perf.data.6E8
> ping  104048 [000] 128582.508976: probe_libc:inet_pton: (7fff83779a60)
>7fff83779a60 __GI___inet_pton+0x0
> (/usr/lib64/glibc-hwcaps/power10/libc.so.6)
>7fff8372a73c getaddrinfo+0x121c
> (/usr/lib64/glibc-hwcaps/power10/libc.so.6)
>   11dc73534 [unknown] (/usr/bin/ping)
>7fff8362a8c4 __libc_start_call_main+0x84
> (/usr/lib64/glibc-hwcaps/power10/libc.so.6)
> 
> FAIL: expected backtrace entry
> "gaih_inet.*\+0x[[:xdigit:]]+[[:space:]]\(/usr/lib64/glibc-hwcaps/power10/libc.so.6\)$"
> got "7fff9d52a73c getaddrinfo+0x121c
> (/usr/lib64/glibc-hwcaps/power10/libc.so.6)"
> 
> With version glibc-2.34-60 gaih_inet function is present as part of the
> expected backtrace. So we cannot just remove the gaih_inet function from
> the backtrace.
> 
> [root@xxx perf]# nm /usr/lib64/glibc-hwcaps/power10/libc.so.6 | grep gaih_inet
> 00130490 t gaih_inet.constprop.0
> 0012e830 t gaih_inet_serv
> 001d45e4 r gaih_inet_typeproto
> 
> [root@xxx perf]# ./perf script -i /tmp/perf.data.b6S
> ping   67906 [000] 22699.591699: probe_libc:inet_pton_3: (7fffbdd80820)
>7fffbdd80820 __GI___inet_pton+0x0
> (/usr/lib64/glibc-hwcaps/power10/libc.so.6)
>7fffbdd31160 gaih_inet.constprop.0+0xcd0
> (/usr/lib64/glibc-hwcaps/power10/libc.so.6)
>7fffbdd31c7c getaddrinfo+0x14c
> (/usr/lib64/glibc-hwcaps/power10/libc.so.6)
>   1140d3558 [unknown] (/usr/bin/ping)
> 
> This patch solves this issue by doing a conditional skip. If there is a
> gaih_inet function present in the libc then it will be added to the
> expected backtrace else the function will be skipped from being added
> to the expected backtrace.
> 
> Output with the patch
> 
> [root@xxx perf]# ./perf test -v "probe libc's inet_pton & backtrace it
> with ping"
> 83: probe libc's inet_pton & backtrace it with ping :
> --- start ---
> test child forked, pid 102662
> ping 102692 [000] 127935.549973: probe_libc:inet_pton: (7fff93379a60)
> 7fff93379a60 __GI___inet_pton+0x0
> (/usr/lib64/glibc-hwcaps/power10/libc.so.6)
> 7fff9332a73c getaddrinfo+0x121c
> (/usr/lib64/glibc-hwcaps/power10/libc.so.6)
> 11ef03534 [unknown] (/usr/bin/ping)
> test child finished with 0
>  end 
> probe libc's inet_pton & backtrace it with ping: Ok
> 
> Signed-off-by: Likhitha Korrapati 
> Reported-by: Disha Goel 
> ---
> tools/perf/tests/shell/record+probe_libc_inet_pton.sh | 5 -
> 1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/tests/shell/record+probe_libc_inet_pton.sh 
> b/tools/perf/tests/shell/record+probe_libc_inet_pton.sh
> index eebeea6bdc76..72c65570db37 100755
> --- a/tools/perf/tests/shell/record+probe_libc_inet_pton.sh
> +++ b/tools/perf/tests/shell/record+probe_libc_inet_pton.sh
> @@ -45,7 +45,10 @@ trace_libc_inet_pton_backtrace() {
> ;;
> ppc64|ppc64le)
> eventattr='max-stack=4'
> - echo "gaih_inet.*\+0x[[:xdigit:]]+[[:space:]]\($libc\)$" >> $expected
> + # Add gaih_inet to expected backtrace only if it is part of libc.
> + if nm $libc | grep -F -q gaih_inet.; then
> + echo "gaih_inet.*\+0x[[:xdigit:]]+[[:space:]]\($libc\)$" >>

Re: [PATCH V4] tools/perf: Add perf binary dependent rule for shellcheck log in Makefile.perf

[Athira Rajeev]

> On 27-Nov-2023, at 8:21 PM, Arnaldo Carvalho de Melo  wrote:
> 
> Em Mon, Nov 27, 2023 at 11:12:57AM +, James Clark escreveu:
>> On 23/11/2023 16:02, Athira Rajeev wrote:
>>> Add rule in new Makefile "tests/Makefile.tests" for running
>>> shellcheck on shell test scripts. This automates below shellcheck
>>> into the build.
> 
>> Seems to work really well. I also tested it on Ubuntu, and checked
>> NO_SHELLCHECK, cleaning and with and without shellcheck installed etc.
> 
>> Reviewed-by: James Clark 
> 
> Tested on Fedora 38, works as advertised, applied.
> 
> - Arnaldo

Hi James, Arnaldo

Thanks for testing the patch and comments.

Athira Rajeev

Re: [PATCH v4 05/13] powerpc/rtas: Facilitate high-level call sequences

[Michael Ellerman]

Nathan Lynch  writes:
> Michael Ellerman  writes:
>> Nathan Lynch via B4 Relay 
>> writes:
>>> diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
>>> index 1fc0b3fffdd1..52f2242d0c28 100644
>>> --- a/arch/powerpc/kernel/rtas.c
>>> +++ b/arch/powerpc/kernel/rtas.c
>>> @@ -196,6 +224,12 @@ static struct rtas_function rtas_function_table[] 
>>> __ro_after_init = {
>>> .buf_idx1 = 1, .size_idx1 = -1,
>>> .buf_idx2 = -1, .size_idx2 = -1,
>>> },
>>> +   /*
>>> +* PAPR+ R1–7.3.19–3 is explicit that the OS must not
>>
>> When you cite PAPR+ can you please include the version number?
>>
>> That's a general comment on this patch and in some other places in the
>> series too.
>
> OK. I assume v2.13 is fine even though most of the citations refer to
> passages that significantly predate that version.

Yeah whatever version you are referring to.

It just means if there's ever confusion about what's in the kernel
comments vs the then current version of PAPR, we can go back and refer
to the exact version you were using.

It also avoids confusion vs LoPAPR, which is simliar but has some
differently numbered chapters.

cheers

Re: [PATCH v4 09/13] powerpc/pseries: Add papr-vpd character driver for VPD retrieval

[Michael Ellerman]

Nathan Lynch  writes:
> Michael Ellerman  writes:
>
>> Nathan Lynch via B4 Relay 
>> writes:
>>> From: Nathan Lynch 
>>>
>>> PowerVM LPARs may retrieve Vital Product Data (VPD) for system
>>> components using the ibm,get-vpd RTAS function.
>> ...
>>>
>>> diff --git a/Documentation/userspace-api/ioctl/ioctl-number.rst 
>>> b/Documentation/userspace-api/ioctl/ioctl-number.rst
>>> index 4ea5b837399a..a950545bf7cd 100644
>>> --- a/Documentation/userspace-api/ioctl/ioctl-number.rst
>>> +++ b/Documentation/userspace-api/ioctl/ioctl-number.rst
>>> @@ -349,6 +349,8 @@ Code  Seq#Include File  
>>>  Comments
>>>   
>>> 
>>>  0xB1  00-1F  PPPoX
>>>   
>>> 
>>> +0xB2  00 arch/powerpc/include/uapi/asm/papr-vpd.h
>>> powerpc/pseries VPD API
>>> + 
>>> 
>>  
>> This hunk should probably go in the previous patch.
>
> The papr-sysparm driver (patch 11/13 "powerpc/pseries/papr-sysparm:
> Expose character device to user space") also adds a line to
> ioctl-number.rst. Are you saying all the additions to ioctl-number.rst
> should be contained in a single patch?

No.

I just meant that the previous patch is where we initially expose the
0xB2 value via uapi, which is the point of no return. So preferably the
documentation is updated by or before that point to reflect that the
0xB2 value is now reserved.

The change log of that patch also talks about allocating a value from
the ioctl-number table, but then doesn't update the table.

cheers

Re: [PATCH v4 09/13] powerpc/pseries: Add papr-vpd character driver for VPD retrieval

[Nathan Lynch]

Michael Ellerman  writes:

> Nathan Lynch via B4 Relay 
> writes:
>> From: Nathan Lynch 
>>
>> PowerVM LPARs may retrieve Vital Product Data (VPD) for system
>> components using the ibm,get-vpd RTAS function.
> ...
>>
>> diff --git a/Documentation/userspace-api/ioctl/ioctl-number.rst 
>> b/Documentation/userspace-api/ioctl/ioctl-number.rst
>> index 4ea5b837399a..a950545bf7cd 100644
>> --- a/Documentation/userspace-api/ioctl/ioctl-number.rst
>> +++ b/Documentation/userspace-api/ioctl/ioctl-number.rst
>> @@ -349,6 +349,8 @@ Code  Seq#Include File   
>> Comments
>>   
>> 
>>  0xB1  00-1F  PPPoX
>>   
>> 
>> +0xB2  00 arch/powerpc/include/uapi/asm/papr-vpd.h
>> powerpc/pseries VPD API
>> + 
>> 
>  
> This hunk should probably go in the previous patch.

The papr-sysparm driver (patch 11/13 "powerpc/pseries/papr-sysparm:
Expose character device to user space") also adds a line to
ioctl-number.rst. Are you saying all the additions to ioctl-number.rst
should be contained in a single patch?

Re: [PATCH v4 05/13] powerpc/rtas: Facilitate high-level call sequences

[Nathan Lynch]

Michael Ellerman  writes:
> Nathan Lynch via B4 Relay 
> writes:
>> diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
>> index 1fc0b3fffdd1..52f2242d0c28 100644
>> --- a/arch/powerpc/kernel/rtas.c
>> +++ b/arch/powerpc/kernel/rtas.c
>> @@ -196,6 +224,12 @@ static struct rtas_function rtas_function_table[] 
>> __ro_after_init = {
>>  .buf_idx1 = 1, .size_idx1 = -1,
>>  .buf_idx2 = -1, .size_idx2 = -1,
>>  },
>> +/*
>> + * PAPR+ R1–7.3.19–3 is explicit that the OS must not
>
> When you cite PAPR+ can you please include the version number?
>
> That's a general comment on this patch and in some other places in the
> series too.

OK. I assume v2.13 is fine even though most of the citations refer to
passages that significantly predate that version.

Re: Ping? Re: [PATCH rc] kvm: Prevent compiling virt/kvm/vfio.c unless VFIO is selected

[Sean Christopherson]

On Fri, Nov 10, 2023, Michael Ellerman wrote:
> Jason Gunthorpe  writes:
> > There are a bunch of reported randconfig failures now because of this,
> > something like:
> >
> >>> arch/powerpc/kvm/../../../virt/kvm/vfio.c:89:7: warning: attribute 
> >>> declaration must precede definition [-Wignored-attributes]
> >fn = symbol_get(vfio_file_iommu_group);
> > ^
> >include/linux/module.h:805:60: note: expanded from macro 'symbol_get'
> >#define symbol_get(x) ({ extern typeof(x) x 
> > __attribute__((weak,visibility("hidden"))); &(x); })
> >
> > It happens because the arch forces KVM_VFIO without knowing if VFIO is
> > even enabled.
> 
> This is still breaking some builds. Can we get this fix in please?
> 
> cheers
> 
> > Split the kconfig so the arch selects the usual HAVE_KVM_ARCH_VFIO and
> > then KVM_VFIO is only enabled if the arch wants it and VFIO is turned on.

Heh, so I was trying to figure out why things like vfio_file_set_kvm() aren't
problematic, i.e. why the existing mess didn't cause failures.  I can't repro 
the
warning (requires clang-16?), but IIUC the reason only the group code is 
problematic
is that vfio.h creates a stub for vfio_file_iommu_group() and thus there's no 
symbol,
whereas vfio.h declares vfio_file_set_kvm() unconditionally.

Because KVM is doing symbol_get() and not taking a direct dependency, the lack 
of
an exported symbol doesn't cause problems, i.e. simply declaring the symbol 
makes
the compiler happy.

Given that the vfio_file_iommu_group() stub shouldn't exist (KVM is the only 
user,
and so if I'm correct the stub is worthless), what about this as a temporary 
"fix"?

I'm 100% on-board with fixing KVM properly, my motivation is purely to minimize
the total amount of churn.  E.g. if this works, then the only extra churn is to
move the declaration of vfio_file_iommu_group() back under the #if, versus 
having
to churn all of the KVM Kconfigs twice (once now, and again for the full 
cleanup).

diff --git a/include/linux/vfio.h b/include/linux/vfio.h
index 454e9295970c..a65b2513f8cd 100644
--- a/include/linux/vfio.h
+++ b/include/linux/vfio.h
@@ -289,16 +289,12 @@ void vfio_combine_iova_ranges(struct rb_root_cached 
*root, u32 cur_nodes,
 /*
  * External user API
  */
-#if IS_ENABLED(CONFIG_VFIO_GROUP)
 struct iommu_group *vfio_file_iommu_group(struct file *file);
+
+#if IS_ENABLED(CONFIG_VFIO_GROUP)
 bool vfio_file_is_group(struct file *file);
 bool vfio_file_has_dev(struct file *file, struct vfio_device *device);
 #else
-static inline struct iommu_group *vfio_file_iommu_group(struct file *file)
-{
-   return NULL;
-}
-
 static inline bool vfio_file_is_group(struct file *file)
 {
return false;

Re: [PATCH v4 05/13] powerpc/rtas: Facilitate high-level call sequences

[Michael Ellerman]

Nathan Lynch via B4 Relay 
writes:
> diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
> index 1fc0b3fffdd1..52f2242d0c28 100644
> --- a/arch/powerpc/kernel/rtas.c
> +++ b/arch/powerpc/kernel/rtas.c
> @@ -196,6 +224,12 @@ static struct rtas_function rtas_function_table[] 
> __ro_after_init = {
>   .buf_idx1 = 1, .size_idx1 = -1,
>   .buf_idx2 = -1, .size_idx2 = -1,
>   },
> + /*
> +  * PAPR+ R1–7.3.19–3 is explicit that the OS must not

When you cite PAPR+ can you please include the version number?

That's a general comment on this patch and in some other places in the
series too.

cheers

Re: [PATCH v4 09/13] powerpc/pseries: Add papr-vpd character driver for VPD retrieval

[Michael Ellerman]

Nathan Lynch via B4 Relay 
writes:
> From: Nathan Lynch 
>
> PowerVM LPARs may retrieve Vital Product Data (VPD) for system
> components using the ibm,get-vpd RTAS function.
...
>
> diff --git a/Documentation/userspace-api/ioctl/ioctl-number.rst 
> b/Documentation/userspace-api/ioctl/ioctl-number.rst
> index 4ea5b837399a..a950545bf7cd 100644
> --- a/Documentation/userspace-api/ioctl/ioctl-number.rst
> +++ b/Documentation/userspace-api/ioctl/ioctl-number.rst
> @@ -349,6 +349,8 @@ Code  Seq#Include File
>Comments
>   
> 
>  0xB1  00-1F  PPPoX
>   
> 
> +0xB2  00 arch/powerpc/include/uapi/asm/papr-vpd.h
> powerpc/pseries VPD API
> + 
> 
 
This hunk should probably go in the previous patch.

cheers

Re: [PATCHv9 2/2] powerpc/setup: Loosen the mapping between cpu logical id and its seq in dt

[Pingfan Liu]

Hi Hari,


On Mon, Nov 27, 2023 at 12:30 PM Hari Bathini  wrote:
>
> Hi Pingfan, Michael,
>
> On 17/10/23 4:03 pm, Hari Bathini wrote:
> >
> >
> > On 17/10/23 7:58 am, Pingfan Liu wrote:
> >> *** Idea ***
> >> For kexec -p, the boot cpu can be not the cpu0, this causes the problem
> >> of allocating memory for paca_ptrs[]. However, in theory, there is no
> >> requirement to assign cpu's logical id as its present sequence in the
> >> device tree. But there is something like cpu_first_thread_sibling(),
> >> which makes assumption on the mapping inside a core. Hence partially
> >> loosening the mapping, i.e. unbind the mapping of core while keep the
> >> mapping inside a core.
> >>
> >> *** Implement ***
> >> At this early stage, there are plenty of memory to utilize. Hence, this
> >> patch allocates interim memory to link the cpu info on a list, then
> >> reorder cpus by changing the list head. As a result, there is a rotate
> >> shift between the sequence number in dt and the cpu logical number.
> >>
> >> *** Result ***
> >> After this patch, a boot-cpu's logical id will always be mapped into the
> >> range [0,threads_per_core).
> >>
> >> Besides this, at this phase, all threads in the boot core are forced to
> >> be onlined. This restriction will be lifted in a later patch with
> >> extra effort.
> >>
> >> Signed-off-by: Pingfan Liu 
> >> Cc: Michael Ellerman 
> >> Cc: Nicholas Piggin 
> >> Cc: Christophe Leroy 
> >> Cc: Mahesh Salgaonkar 
> >> Cc: Wen Xiong 
> >> Cc: Baoquan He 
> >> Cc: Ming Lei 
> >> Cc: Sourabh Jain 
> >> Cc: Hari Bathini 
> >> Cc: ke...@lists.infradead.org
> >> To: linuxppc-dev@lists.ozlabs.org
> >
> > Thanks for working on this, Pingfan.
> > Looks good to me.
> >
> > Acked-by: Hari Bathini 
> >
>
> On second thoughts, probably better off with no impact for
> bootcpu < nr_cpu_ids case and changing only two cores logical
> numbering otherwise. Something like the below (Please share
> your thoughts):
>

I am afraid that it may not be as ideal as it looks, considering the
following factors:
-1. For the case of 'bootcpu < nr_cpu_ids', crash can happen evenly
across any cpu in the system, which seriously undermines the
protection intended here (Under the most optimistic scenario, there is
a 50% chance of success)

-2. For the re-ordering of logical numbering, IMHO, if there is
concern that re-ordering will break something, the partial re-ordering
can not avoid that.  We ought to spot probable hazards so as to ease
worries.


Thanks,

Pingfan

> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> index ec82f5bda908..78a8312aa8c4 100644
> --- a/arch/powerpc/kernel/prom.c
> +++ b/arch/powerpc/kernel/prom.c
> @@ -76,7 +76,9 @@ u64 ppc64_rma_size;
>   unsigned int boot_cpu_node_count __ro_after_init;
>   #endif
>   static phys_addr_t first_memblock_size;
> +#ifdef CONFIG_SMP
>   static int __initdata boot_cpu_count;
> +#endif
>
>   static int __init early_parse_mem(char *p)
>   {
> @@ -357,6 +359,25 @@ static int __init early_init_dt_scan_cpus(unsigned
> long node,
> fdt_boot_cpuid_phys(initial_boot_params)) {
> found = boot_cpu_count;
> found_thread = i;
> +   /*
> +* Map boot-cpu logical id into the range
> +* of [0, thread_per_core) if it can't be
> +* accommodated within nr_cpu_ids.
> +*/
> +   if (i != boot_cpu_count && boot_cpu_count >= 
> nr_cpu_ids) {
> +   boot_cpuid = i;
> +   DBG("Logical CPU number for boot CPU changed 
> from %d to %d\n",
> +   boot_cpu_count, i);
> +   } else {
> +   boot_cpuid = boot_cpu_count;
> +   }
> +
> +   /* Ensure boot thread is acconted for in nr_cpu_ids */
> +   if (boot_cpuid >= nr_cpu_ids) {
> +   set_nr_cpu_ids(boot_cpuid + 1);
> +   DBG("Adjusted nr_cpu_ids to %u, to include 
> boot CPU.\n",
> +   nr_cpu_ids);
> +   }
> }
>   #ifdef CONFIG_SMP
> /* logical cpu id is always 0 on UP kernels */
> @@ -368,9 +389,8 @@ static int __init early_init_dt_scan_cpus(unsigned
> long node,
> if (found < 0)
> return 0;
>
> -   DBG("boot cpu: logical %d physical %d\n", found,
> +   DBG("boot cpu: logical %d physical %d\n", boot_cpuid,
> be32_to_cpu(intserv[found_thread]));
> -   boot_cpuid = found;
>
> boot_cpu_hwid = be32_to_cpu(intserv[found_thread]);
>
> diff --git a/arch/powerpc/kernel/setup-common.c
> b/arch/powerpc/kernel/setup-common.c
> index b7b733474b60..f7179525c774 100644
> --- a/arch/powerpc/kernel/setup-common.c
> +++ b/arch/powerpc/kernel/setu

linux-next: duplicate patches in the char-misc tree

[Stephen Rothwell]

Hi all,

The following commits are also in the powerpc tree as different commits
(but the same patches):

  bc1183a63057 ("misc: ocxl: main: Remove unnecessary ‘0’ values from rc")
  29eb0dc7bd1e ("misc: ocxl: link: Remove unnecessary (void*) conversions")
  0e425d703c30 ("misc: ocxl: afu_irq: Remove unnecessary (void*) conversions")
  62df29a542f9 ("misc: ocxl: context: Remove unnecessary (void*) conversions")

These are commits

  29685ea5754f ("misc: ocxl: main: Remove unnecessary ‘0’ values from rc")
  220f3ced8e42 ("misc: ocxl: link: Remove unnecessary (void*) conversions")
  84ba5d3675e2 ("misc: ocxl: afu_irq: Remove unnecessary (void*) conversions")
  82d30723d58f ("misc: ocxl: context: Remove unnecessary (void*) conversions")

in the powerpc tree.

-- 
Cheers,
Stephen Rothwell


pgpCthRn5ODId.pgp
Description: OpenPGP digital signature

Re: [PATCH v4 13/13] powerpc/selftests: Add test for papr-sysparm

[Michael Ellerman]

Nathan Lynch via B4 Relay 
writes:
> From: Nathan Lynch 
>
> Consistently testing system parameter access is a bit difficult by
> nature -- the set of parameters available depends on the model and
> system configuration, and updating a parameter should be considered a
> destructive operation reserved for the admin.
...
> diff --git a/tools/testing/selftests/powerpc/papr_sysparm/papr_sysparm.c 
> b/tools/testing/selftests/powerpc/papr_sysparm/papr_sysparm.c
> new file mode 100644
> index ..fc25c03e8bc7
> --- /dev/null
> +++ b/tools/testing/selftests/powerpc/papr_sysparm/papr_sysparm.c
> @@ -0,0 +1,164 @@
...
> +static int set_hmc0(void)
> +{
> + struct papr_sysparm_io_block sp = {
> + .parameter = 0, // HMC0, not a settable parameter
> + };
> + const int devfd = open(DEVPATH, O_RDONLY);
> +
> + SKIP_IF_MSG(devfd < 0 && errno == ENOENT,
> + DEVPATH " not present");
> +
> + FAIL_IF(devfd < 0);
> +
> + // Ensure expected error
> + FAIL_IF(ioctl(devfd, PAPR_SYSPARM_IOC_SET, &sp) != -1);
> + FAIL_IF(errno != EPERM);
> +
> + FAIL_IF(close(devfd) != 0);
> +
> + return 0;
> +}

This one fails when run with qemu/KVM.

# selftests: powerpc: papr_sysparm
# test: open and close /dev/papr-sysparm without issuing commands
# tags: git_version:v6.7-rc2-35-g41ada9f713ae
# success: open and close /dev/papr-sysparm without issuing commands
# test: retrieve SPLPAR characteristics
# tags: git_version:v6.7-rc2-35-g41ada9f713ae
# success: retrieve SPLPAR characteristics
# test: verify EOPNOTSUPP for known-bad parameter
# tags: git_version:v6.7-rc2-35-g41ada9f713ae
# success: verify EOPNOTSUPP for known-bad parameter
# test: PAPR_SYSPARM_IOC_GET returns EFAULT on bad address
# tags: git_version:v6.7-rc2-35-g41ada9f713ae
# success: PAPR_SYSPARM_IOC_GET returns EFAULT on bad address
# test: PAPR_SYSPARM_IOC_SET returns EFAULT on bad address
# tags: git_version:v6.7-rc2-35-g41ada9f713ae
# success: PAPR_SYSPARM_IOC_SET returns EFAULT on bad address
# test: ensure EPERM on attempt to update HMC0
# tags: git_version:v6.7-rc2-35-g41ada9f713ae
# [FAIL] Test FAILED on line 113
# failure: ensure EPERM on attempt to update HMC0

It's returning EOPNOTSUPP.

Something like below would work to fix it.

cheers

diff --git a/tools/testing/selftests/powerpc/papr_sysparm/papr_sysparm.c 
b/tools/testing/selftests/powerpc/papr_sysparm/papr_sysparm.c
index fc25c03e8bc7..9d4850c25aed 100644
--- a/tools/testing/selftests/powerpc/papr_sysparm/papr_sysparm.c
+++ b/tools/testing/selftests/powerpc/papr_sysparm/papr_sysparm.c
@@ -110,6 +110,7 @@ static int set_hmc0(void)

// Ensure expected error
FAIL_IF(ioctl(devfd, PAPR_SYSPARM_IOC_SET, &sp) != -1);
+   SKIP_IF_MSG(errno == EOPNOTSUPP, "operation not supported");
FAIL_IF(errno != EPERM);

FAIL_IF(close(devfd) != 0);

Re: [PATCH v4 05/13] powerpc/rtas: Facilitate high-level call sequences

[Nathan Lynch]

Michael Ellerman  writes:

> Nathan Lynch via B4 Relay 
> writes:
>> From: Nathan Lynch 
>>
>> On RTAS platforms there is a general restriction that the OS must not
>> enter RTAS on more than one CPU at a time. This low-level
>> serialization requirement is satisfied by holding a spin
>> lock (rtas_lock) across most RTAS function invocations.
> ...
>> diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
>> index 1fc0b3fffdd1..52f2242d0c28 100644
>> --- a/arch/powerpc/kernel/rtas.c
>> +++ b/arch/powerpc/kernel/rtas.c
>> @@ -581,6 +652,28 @@ static const struct rtas_function 
>> *rtas_token_to_function(s32 token)
>>  return NULL;
>>  }
>>  
>> +static void __rtas_function_lock(struct rtas_function *func)
>> +{
>> +if (func && func->lock)
>> +mutex_lock(func->lock);
>> +}
>
> This is obviously going to defeat most static analysis tools.

I guess it's not that obvious to me :-) Is it because the mutex_lock()
is conditional? I'll improve this if it's possible.

> I assume lockdep is OK with it though?

Seems to be, yes.

Re: [PATCH v4 05/13] powerpc/rtas: Facilitate high-level call sequences

[Michael Ellerman]

Nathan Lynch via B4 Relay 
writes:
> From: Nathan Lynch 
>
> On RTAS platforms there is a general restriction that the OS must not
> enter RTAS on more than one CPU at a time. This low-level
> serialization requirement is satisfied by holding a spin
> lock (rtas_lock) across most RTAS function invocations.
...
> diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
> index 1fc0b3fffdd1..52f2242d0c28 100644
> --- a/arch/powerpc/kernel/rtas.c
> +++ b/arch/powerpc/kernel/rtas.c
> @@ -581,6 +652,28 @@ static const struct rtas_function 
> *rtas_token_to_function(s32 token)
>   return NULL;
>  }
>  
> +static void __rtas_function_lock(struct rtas_function *func)
> +{
> + if (func && func->lock)
> + mutex_lock(func->lock);
> +}

This is obviously going to defeat most static analysis tools. I assume
lockdep is OK with it though?

cheers

Re: [PATCH] perf test record+probe_libc_inet_pton: Fix call chain match on powerpc

[Ian Rogers]

On Tue, Nov 28, 2023 at 1:57 AM Disha Goel  wrote:
>
> On 26/11/23 12:39 pm, Likhitha Korrapati wrote:
>
> > The perf test "probe libc's inet_pton & backtrace it with ping" fails on
> > powerpc as below:
> >
> > root@xxx perf]# perf test -v "probe libc's inet_pton & backtrace it with
> > ping"
> >   85: probe libc's inet_pton & backtrace it with ping :
> > --- start ---
> > test child forked, pid 96028
> > ping 96056 [002] 127271.101961: probe_libc:inet_pton: (7fffa1779a60)
> > 7fffa1779a60 __GI___inet_pton+0x0
> > (/usr/lib64/glibc-hwcaps/power10/libc.so.6)
> > 7fffa172a73c getaddrinfo+0x121c
> > (/usr/lib64/glibc-hwcaps/power10/libc.so.6)
> > FAIL: expected backtrace entry
> > "gaih_inet.*\+0x[[:xdigit:]]+[[:space:]]\(/usr/lib64/glibc-hwcaps/power10/libc.so.6\)$"
> > got "7fffa172a73c getaddrinfo+0x121c
> > (/usr/lib64/glibc-hwcaps/power10/libc.so.6)"
> > test child finished with -1
> >  end 
> > probe libc's inet_pton & backtrace it with ping: FAILED!
> >
> > This test installs a probe on libc's inet_pton function, which will use
> > uprobes and then uses perf trace on a ping to localhost. It gets 3
> > levels deep backtrace and checks whether it is what we expected or not.
> >
> > The test started failing from RHEL 9.4 where as it works in previous
> > distro version (RHEL 9.2). Test expects gaih_inet function to be part of
> > backtrace. But in the glibc version (2.34-86) which is part of distro
> > where it fails, this function is missing and hence the test is failing.
> >
> >  From nm and ping command output we can confirm that gaih_inet function
> > is not present in the expected backtrace for glibc version glibc-2.34-86
> >
> > [root@xxx perf]# nm /usr/lib64/glibc-hwcaps/power10/libc.so.6 | grep 
> > gaih_inet
> > 001273e0 t gaih_inet_serv
> > 001cd8d8 r gaih_inet_typeproto
> >
> > [root@xxx perf]# perf script -i /tmp/perf.data.6E8
> > ping  104048 [000] 128582.508976: probe_libc:inet_pton: (7fff83779a60)
> >  7fff83779a60 __GI___inet_pton+0x0
> > (/usr/lib64/glibc-hwcaps/power10/libc.so.6)
> >  7fff8372a73c getaddrinfo+0x121c
> > (/usr/lib64/glibc-hwcaps/power10/libc.so.6)
> > 11dc73534 [unknown] (/usr/bin/ping)
> >  7fff8362a8c4 __libc_start_call_main+0x84
> > (/usr/lib64/glibc-hwcaps/power10/libc.so.6)
> >
> > FAIL: expected backtrace entry
> > "gaih_inet.*\+0x[[:xdigit:]]+[[:space:]]\(/usr/lib64/glibc-hwcaps/power10/libc.so.6\)$"
> > got "7fff9d52a73c getaddrinfo+0x121c
> > (/usr/lib64/glibc-hwcaps/power10/libc.so.6)"
> >
> > With version glibc-2.34-60 gaih_inet function is present as part of the
> > expected backtrace. So we cannot just remove the gaih_inet function from
> > the backtrace.
> >
> > [root@xxx perf]# nm /usr/lib64/glibc-hwcaps/power10/libc.so.6 | grep 
> > gaih_inet
> > 00130490 t gaih_inet.constprop.0
> > 0012e830 t gaih_inet_serv
> > 001d45e4 r gaih_inet_typeproto
> >
> > [root@xxx perf]# ./perf script -i /tmp/perf.data.b6S
> > ping   67906 [000] 22699.591699: probe_libc:inet_pton_3: (7fffbdd80820)
> >  7fffbdd80820 __GI___inet_pton+0x0
> > (/usr/lib64/glibc-hwcaps/power10/libc.so.6)
> >  7fffbdd31160 gaih_inet.constprop.0+0xcd0
> > (/usr/lib64/glibc-hwcaps/power10/libc.so.6)
> >  7fffbdd31c7c getaddrinfo+0x14c
> > (/usr/lib64/glibc-hwcaps/power10/libc.so.6)
> > 1140d3558 [unknown] (/usr/bin/ping)
> >
> > This patch solves this issue by doing a conditional skip. If there is a
> > gaih_inet function present in the libc then it will be added to the
> > expected backtrace else the function will be skipped from being added
> > to the expected backtrace.
> >
> > Output with the patch
> >
> > [root@xxx perf]# ./perf test -v "probe libc's inet_pton & backtrace it
> > with ping"
> >   83: probe libc's inet_pton & backtrace it with ping :
> > --- start ---
> > test child forked, pid 102662
> > ping 102692 [000] 127935.549973: probe_libc:inet_pton: (7fff93379a60)
> > 7fff93379a60 __GI___inet_pton+0x0
> > (/usr/lib64/glibc-hwcaps/power10/libc.so.6)
> > 7fff9332a73c getaddrinfo+0x121c
> > (/usr/lib64/glibc-hwcaps/power10/libc.so.6)
> > 11ef03534 [unknown] (/usr/bin/ping)
> > test child finished with 0
> >  end 
> > probe libc's inet_pton & backtrace it with ping: Ok
> >
> > Signed-off-by: Likhitha Korrapati 
> > Reported-by: Disha Goel 
>
> Thanks for the fix patch.
> I have tested on a Power10 machine, "probe libc's inet_pton & backtrace it 
> with ping"
> perf test passes with the patch applied.
>
> Output where gaih_inet function is not present
>
> # perf test -v "probe libc's inet_pton & backtrace it with ping"
>  85: probe libc's inet_pton & backtrace it with ping :
> --- start ---
> test child forked, pid 4622
> ping 4652 [011] 58.987631: probe_libc:inet_pton: (7fff91b79a60)
> 7fff91b79a60 __GI___inet_pton+0x0 
> (/usr/lib64/glibc-

Re: [PATCH v5 4/5] tty: Add SBI debug console support to HVC SBI driver

[Greg Kroah-Hartman]

On Fri, Nov 24, 2023 at 12:39:04PM +0530, Anup Patel wrote:
> From: Atish Patra 
> 
> RISC-V SBI specification supports advanced debug console
> support via SBI DBCN extension.
> 
> Extend the HVC SBI driver to support it.
> 
> Signed-off-by: Atish Patra 
> Signed-off-by: Anup Patel 
> ---
>  drivers/tty/hvc/Kconfig |  2 +-
>  drivers/tty/hvc/hvc_riscv_sbi.c | 37 ++---
>  2 files changed, 31 insertions(+), 8 deletions(-)

Acked-by: Greg Kroah-Hartman 

Re: [PATCH v5 3/5] tty/serial: Add RISC-V SBI debug console based earlycon

[Greg Kroah-Hartman]

On Fri, Nov 24, 2023 at 12:39:03PM +0530, Anup Patel wrote:
> We extend the existing RISC-V SBI earlycon support to use the new
> RISC-V SBI debug console extension.
> 
> Signed-off-by: Anup Patel 
> Reviewed-by: Andrew Jones 
> ---
>  drivers/tty/serial/Kconfig  |  2 +-
>  drivers/tty/serial/earlycon-riscv-sbi.c | 27 ++---
>  2 files changed, 25 insertions(+), 4 deletions(-)

Acked-by: Greg Kroah-Hartman 

Re: [PATCH v4 06/13] powerpc/rtas: Serialize firmware activation sequences

[Nathan Lynch]

Nathan Lynch  writes:
> "Aneesh Kumar K.V (IBM)"  writes:
>
>> Nathan Lynch  writes:
>>
>>> "Aneesh Kumar K.V (IBM)"  writes:
 Nathan Lynch via B4 Relay 
 writes:

>
> Use the function lock API to prevent interleaving call sequences of
> the ibm,activate-firmware RTAS function, which typically requires
> multiple calls to complete the update. While the spec does not
> specifically prohibit interleaved sequences, there's almost certainly
> no advantage to allowing them.
>

 Can we document what is the equivalent thing the userspace does?
>>>
>>> I'm not sure what we would document.
>>>
>>> As best I can tell, the activate_firmware command in powerpc-utils does
>>> not make any effort to protect its use of the ibm,activate-firmware RTAS
>>> function. The command is not intended to be run manually and I guess
>>> it's relying on the platform's management console to serialize its
>>> invocations.
>>>
>>> drmgr (also from powerpc-utils) has some dead code for LPM that calls
>>> ibm,activate-firmware; it should probably be removed. The command uses a
>>> lock file to serialize all of its executions.
>>>
>>> Something that could happen with interleaved ibm,activate-firmware
>>> sequences is something like this:
>>>
>>> 1. Process A initiates an ibm,activate-firmware sequence and receives a
>>>"retry" status (-2/990x).
>>> 2. Process B calls ibm,activate-firmware and receives the "done" status
>>>(0), concluding the sequence A began.
>>> 3. Process A, unaware of B, calls ibm,activate-firmware again,
>>>inadvertently beginning a new sequence.
>>>
>>
>> So this patch won't protect us against a parallel userspace
>> invocation.
>
> It does protect in-kernel sequences from disruption by sys_rtas-based
> sequences. Patch 5/13 "Facilitate high-level call sequences" makes it so
> sys_rtas-based invocations of ibm,activate-firmware acquire
> rtas_ibm_activate_firmware_lock.
>
>> We can add static bool call_in_progress to track the ongoing
>> ibm,activate-firmware call from userspace?
>
> We can't reliably maintain any such state in the kernel. A user of
> sys_rtas could exit with a sequence in progress, or it could simply
> decline to complete a sequence it has initiated for any reason. This is
> one of the fundamental problems with directly exposing more complex RTAS
> functions to user space.

That said, I should resurrect "powerpc/rtas: consume retry statuses in
sys_rtas()":

https://lore.kernel.org/linuxppc-dev/20230220-rtas-queue-for-6-4-v1-8-010e4416f...@linux.ibm.com/

That ought to have the effect of perfectly serializing all
ibm,activate-firmware sequences regardless of how they're initiated.

But I'd like to leave that until later instead of adding to this series.

Re: [PATCH v4 06/13] powerpc/rtas: Serialize firmware activation sequences

[Nathan Lynch]

"Aneesh Kumar K.V (IBM)"  writes:

> Nathan Lynch  writes:
>
>> "Aneesh Kumar K.V (IBM)"  writes:
>>> Nathan Lynch via B4 Relay 
>>> writes:
>>>

 Use the function lock API to prevent interleaving call sequences of
 the ibm,activate-firmware RTAS function, which typically requires
 multiple calls to complete the update. While the spec does not
 specifically prohibit interleaved sequences, there's almost certainly
 no advantage to allowing them.

>>>
>>> Can we document what is the equivalent thing the userspace does?
>>
>> I'm not sure what we would document.
>>
>> As best I can tell, the activate_firmware command in powerpc-utils does
>> not make any effort to protect its use of the ibm,activate-firmware RTAS
>> function. The command is not intended to be run manually and I guess
>> it's relying on the platform's management console to serialize its
>> invocations.
>>
>> drmgr (also from powerpc-utils) has some dead code for LPM that calls
>> ibm,activate-firmware; it should probably be removed. The command uses a
>> lock file to serialize all of its executions.
>>
>> Something that could happen with interleaved ibm,activate-firmware
>> sequences is something like this:
>>
>> 1. Process A initiates an ibm,activate-firmware sequence and receives a
>>"retry" status (-2/990x).
>> 2. Process B calls ibm,activate-firmware and receives the "done" status
>>(0), concluding the sequence A began.
>> 3. Process A, unaware of B, calls ibm,activate-firmware again,
>>inadvertently beginning a new sequence.
>>
>
> So this patch won't protect us against a parallel userspace
> invocation.

It does protect in-kernel sequences from disruption by sys_rtas-based
sequences. Patch 5/13 "Facilitate high-level call sequences" makes it so
sys_rtas-based invocations of ibm,activate-firmware acquire
rtas_ibm_activate_firmware_lock.

> We can add static bool call_in_progress to track the ongoing
> ibm,activate-firmware call from userspace?

We can't reliably maintain any such state in the kernel. A user of
sys_rtas could exit with a sequence in progress, or it could simply
decline to complete a sequence it has initiated for any reason. This is
one of the fundamental problems with directly exposing more complex RTAS
functions to user space.

> My only concern is we are adding locks to protect against parallel
> calls in the kernel, but at the same time, we ignore any userspace
> call regarding the same. We should at least document this if this is
> not important to be fixed.

It's not accurate to say we're ignoring user space calls. Patch 5/13
makes it so that sys_rtas(ibm,activate-firmware) will serialize on the
same lock used here.

Re: [linus:master] [file] 0ede61d858: will-it-scale.per_thread_ops -2.9% regression

[Christian Brauner]

On Mon, Nov 27, 2023 at 09:10:54AM -0800, Linus Torvalds wrote:
> On Mon, 27 Nov 2023 at 02:27, Christian Brauner  wrote:
> >
> > So I've picked up your patch (vfs.misc). It's clever alright so thanks
> > for the comments in there otherwise I would've stared at this for far
> > too long.
> 
> Note that I should probably have commented on one other thing: that
> whole "just load from fd[0] is always safe, because the fd[] array
> always exists".

I added a comment to that effect in the code.

> 
> IOW, that whole "load and mask" thing only works when you know the
> array exists at all.
> 
> Doing that "just mask the index" wouldn't be valid if "size = 0" is an
> option and might mean that we don't have an array at all (ie if "->fd"
> itself could be NULL.
> 
> But we never have a completely empty file descriptor array, and
> fdp->fd is never NULL.  At a minimum 'max_fds' is NR_OPEN_DEFAULT.
> 
> (The whole 'tsk->files' could be NULL, but only for kernel threads or
> when exiting, so fget_task() will check for *that*, but it's a
> separate thing)

Yep.

> 
> So that's why it's safe to *entirely* remove the whole
> 
> if (unlikely(fd >= fdt->max_fds))
> 
> test, and do it *all* with just "mask the index, and mask the resulting load".

Yep.

Re: [PATCH v4 06/13] powerpc/rtas: Serialize firmware activation sequences

[IBM]

Nathan Lynch  writes:

> "Aneesh Kumar K.V (IBM)"  writes:
>> Nathan Lynch via B4 Relay 
>> writes:
>>
>>>
>>> Use the function lock API to prevent interleaving call sequences of
>>> the ibm,activate-firmware RTAS function, which typically requires
>>> multiple calls to complete the update. While the spec does not
>>> specifically prohibit interleaved sequences, there's almost certainly
>>> no advantage to allowing them.
>>>
>>
>> Can we document what is the equivalent thing the userspace does?
>
> I'm not sure what we would document.
>
> As best I can tell, the activate_firmware command in powerpc-utils does
> not make any effort to protect its use of the ibm,activate-firmware RTAS
> function. The command is not intended to be run manually and I guess
> it's relying on the platform's management console to serialize its
> invocations.
>
> drmgr (also from powerpc-utils) has some dead code for LPM that calls
> ibm,activate-firmware; it should probably be removed. The command uses a
> lock file to serialize all of its executions.
>
> Something that could happen with interleaved ibm,activate-firmware
> sequences is something like this:
>
> 1. Process A initiates an ibm,activate-firmware sequence and receives a
>"retry" status (-2/990x).
> 2. Process B calls ibm,activate-firmware and receives the "done" status
>(0), concluding the sequence A began.
> 3. Process A, unaware of B, calls ibm,activate-firmware again,
>inadvertently beginning a new sequence.
>

So this patch won't protect us against a parallel userspace invocation.
We can add static bool call_in_progress to track the ongoing
ibm,activate-firmware call from userspace? My only concern is we are
adding locks to protect against parallel calls in the kernel, but at the
same time, we ignore any userspace call regarding the same. We should at
least document this if this is not important to be fixed.

-aneesh

[powerpc:next] BUILD SUCCESS 0d555b57ee660d8a871781c0eebf006e855e918d

[kernel test robot]

tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git 
next
branch HEAD: 0d555b57ee660d8a871781c0eebf006e855e918d  powerpc: 
pmd_move_must_withdraw() is only needed for CONFIG_TRANSPARENT_HUGEPAGE

elapsed time: 1466m

configs tested: 82
configs skipped: 2

The following configs have been built successfully.
More configs may be tested in the coming days.

tested configs:
alpha allnoconfig   gcc  
alphaallyesconfig   gcc  
alpha   defconfig   gcc  
arc  allmodconfig   gcc  
arc   allnoconfig   gcc  
arc  allyesconfig   gcc  
arc defconfig   gcc  
arm  allmodconfig   gcc  
arm   allnoconfig   gcc  
arm  allyesconfig   gcc  
arm defconfig   clang
arm64allmodconfig   clang
arm64 allnoconfig   gcc  
arm64   defconfig   gcc  
csky allmodconfig   gcc  
csky  allnoconfig   gcc  
csky allyesconfig   gcc  
cskydefconfig   gcc  
hexagon  allmodconfig   clang
hexagon   allnoconfig   clang
hexagon  allyesconfig   clang
hexagon defconfig   clang
i386 allmodconfig   clang
i386  allnoconfig   clang
i386 allyesconfig   clang
i386defconfig   gcc  
loongarchallmodconfig   gcc  
loongarch allnoconfig   gcc  
loongarch   defconfig   gcc  
m68k allmodconfig   gcc  
m68k  allnoconfig   gcc  
m68k allyesconfig   gcc  
m68kdefconfig   gcc  
microblaze   allmodconfig   gcc  
microblazeallnoconfig   gcc  
microblaze   allyesconfig   gcc  
microblaze  defconfig   gcc  
mips  allnoconfig   clang
mips allyesconfig   gcc  
nios2allmodconfig   gcc  
nios2 allnoconfig   gcc  
nios2allyesconfig   gcc  
nios2   defconfig   gcc  
openrisc  allnoconfig   gcc  
openrisc allyesconfig   gcc  
openriscdefconfig   gcc  
parisc   allmodconfig   gcc  
pariscallnoconfig   gcc  
parisc   allyesconfig   gcc  
parisc  defconfig   gcc  
parisc64defconfig   gcc  
powerpc  allmodconfig   clang
powerpc   allnoconfig   gcc  
powerpc  allyesconfig   clang
riscvallmodconfig   gcc  
riscv allnoconfig   clang
riscvallyesconfig   gcc  
riscv   defconfig   gcc  
riscv  rv32_defconfig   clang
s390 allmodconfig   gcc  
s390  allnoconfig   gcc  
s390 allyesconfig   gcc  
s390defconfig   gcc  
sh   allmodconfig   gcc  
shallnoconfig   gcc  
sh   allyesconfig   gcc  
sh  defconfig   gcc  
sparcallmodconfig   gcc  
sparc64  allmodconfig   gcc  
sparc64  allyesconfig   gcc  
sparc64 defconfig   gcc  
um   allmodconfig   clang
umallnoconfig   clang
um   allyesconfig   clang
um  defconfig   gcc  
um i386_defconfig   gcc  
um   x86_64_defconfig   gcc  
x86_64allnoconfig   gcc  
x86_64   allyesconfig   clang
x86_64  defconfig   gcc  
x86_64  rhel-8.3-rust   clang
xtensaallnoconfig   gcc  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v4 09/13] powerpc/pseries: Add papr-vpd character driver for VPD retrieval

[Nathan Lynch]

Michal Suchánek  writes:
>
> On Fri, Nov 17, 2023 at 11:14:27PM -0600, Nathan Lynch via B4 Relay wrote:
>> +do {
>> +blob = papr_vpd_run_sequence(loc_code);
>> +if (!IS_ERR(blob)) /* Success. */
>> +break;
>> +if (PTR_ERR(blob) != -EAGAIN) /* Hard error. */
>> +break;
>> +pr_info_ratelimited("VPD changed during retrieval, retrying\n");
>> +cond_resched();
>> +} while (!fatal_signal_pending(current));
>
> this is defined in linux/sched/signal.h which is not included.
>


>> +static long papr_vpd_create_handle(struct papr_location_code __user *ulc)
>> +{
>> +struct papr_location_code klc;
>> +const struct vpd_blob *blob;
>> +struct file *file;
>> +long err;
>> +int fd;
>> +
>> +if (copy_from_user(&klc, ulc, sizeof(klc)))
>> +return -EFAULT;
>
> This is defined in linux/uaccess.h which is not included.
>
> Same for the sysparm driver.
>
> Tested-by: Michal Suchánek 

Thanks, I'll fix these issues and add your T-B to this patch.

Re: [PATCH v4 05/13] powerpc/rtas: Facilitate high-level call sequences

[Nathan Lynch]

"Aneesh Kumar K.V (IBM)"  writes:
> Nathan Lynch via B4 Relay 
> writes:
>> There should be no perceivable change introduced here except that
>> concurrent callers of the same RTAS function via sys_rtas() may block
>> on a mutex instead of spinning on rtas_lock. Changes to follow will
>> add rtas_function_lock()/unlock() pairs to kernel-based call
>> sequences.
>>
>
> Can you add an example of the last part. I did look at to find 06 to
> find the details
>
>   rtas_function_lock(RTAS_FN_IBM_ACTIVATE_FIRMWARE);
>
>   do {
>   fwrc = rtas_call(token, 0, 1, NULL);
>   } while (rtas_busy_delay(fwrc));
>
>   rtas_function_unlock(RTAS_FN_IBM_ACTIVATE_FIRMWARE);

Sure, I'll add a simple example of the API usage in the commit message,
thanks.

Re: [PATCH v4 06/13] powerpc/rtas: Serialize firmware activation sequences

[Nathan Lynch]

"Aneesh Kumar K.V (IBM)"  writes:
> Nathan Lynch via B4 Relay 
> writes:
>
>>
>> Use the function lock API to prevent interleaving call sequences of
>> the ibm,activate-firmware RTAS function, which typically requires
>> multiple calls to complete the update. While the spec does not
>> specifically prohibit interleaved sequences, there's almost certainly
>> no advantage to allowing them.
>>
>
> Can we document what is the equivalent thing the userspace does?

I'm not sure what we would document.

As best I can tell, the activate_firmware command in powerpc-utils does
not make any effort to protect its use of the ibm,activate-firmware RTAS
function. The command is not intended to be run manually and I guess
it's relying on the platform's management console to serialize its
invocations.

drmgr (also from powerpc-utils) has some dead code for LPM that calls
ibm,activate-firmware; it should probably be removed. The command uses a
lock file to serialize all of its executions.

Something that could happen with interleaved ibm,activate-firmware
sequences is something like this:

1. Process A initiates an ibm,activate-firmware sequence and receives a
   "retry" status (-2/990x).
2. Process B calls ibm,activate-firmware and receives the "done" status
   (0), concluding the sequence A began.
3. Process A, unaware of B, calls ibm,activate-firmware again,
   inadvertently beginning a new sequence.

Seems mostly benign to me except that process A could fail to make
progress indefinitely under the right circumstances.

Re: [PATCH 2/3] powerpc/pseries/memhp: Remove unbalanced dlpar_release_drc() call

[Nathan Lynch]

Nick Child  writes:
> Hi Nathan,
> Patches 1 and 3 LGTM

thanks.

> Regarding this patch, dlpar_memory_remove_by_count() calls 
> dlpar_add_lmb() and does not free drc on add error.
> dlpar_add_lmb() is called here in error recovery so probably
> not a big deal.
>
> This is all new code to me but it looks like if the requested
> number of lmbs cannot be removed then it attempts to add back
> the ones that were successfully removed. So if you cannot add
> an lmb that WAS successfully removed, it seems sane to also
> release the drc.

Maybe I'll drop this one for now and turn my attention to removing all
the high-level rollback logic in this code. There's no reliable way to
accomplish what it's trying to do (i.e. restore the original quantity of
LMBs) and it just complicates things.


> On 11/14/23 11:01, Nathan Lynch via B4 Relay wrote:
>> From: Nathan Lynch 
>> 
>> Callers of dlpar_add_lmb() are responsible for first acquiring the DRC
>> and releasing it if dlpar_add_lmb() fails.
>> 
>> However, dlpar_add_lmb() performs a dlpar_release_drc() in one error
>> branch.  There is no corresponding dlpar_acquire_drc() in the
>> function, nor is there any stated justification. None of the other
>> error paths in dlpar_add_lmb() release the DRC.
>> 
>> This is a potential source of redundant attempts to release DRCs,
>> which is likely benign, but is confusing and inconsistent. Remove it.
>> 
>> Signed-off-by: Nathan Lynch 
>> ---
>>   arch/powerpc/platforms/pseries/hotplug-memory.c | 1 -
>>   1 file changed, 1 deletion(-)
>> 
>> diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c 
>> b/arch/powerpc/platforms/pseries/hotplug-memory.c
>> index 6f2eebae7bee..ba883c1b9f6d 100644
>> --- a/arch/powerpc/platforms/pseries/hotplug-memory.c
>> +++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
>> @@ -575,7 +575,6 @@ static int dlpar_add_lmb(struct drmem_lmb *lmb)
>>   
>>  rc = update_lmb_associativity_index(lmb);
>>  if (rc) {
>> -dlpar_release_drc(lmb->drc_index);
>>  return rc;
>>  }
>>   
>> 

[PATCH 17/17] soc: fsl: cpm1: qmc: Introduce functions to change timeslots at runtime

[Herve Codina]

Introduce qmc_chan_{get,set}_ts_info() function to allow timeslots
modification at runtime.

The modification is provided using qmc_chan_set_ts_info() and will be
applied on next qmc_chan_start().
qmc_chan_set_ts_info() must be called with the channel rx and/or tx
stopped.

Signed-off-by: Herve Codina 
Reviewed-by: Christophe Leroy 
---
 drivers/soc/fsl/qe/qmc.c | 51 
 include/soc/fsl/qe/qmc.h | 10 
 2 files changed, 61 insertions(+)

diff --git a/drivers/soc/fsl/qe/qmc.c b/drivers/soc/fsl/qe/qmc.c
index 73903ce31695..79fe79b9464f 100644
--- a/drivers/soc/fsl/qe/qmc.c
+++ b/drivers/soc/fsl/qe/qmc.c
@@ -290,6 +290,57 @@ int qmc_chan_get_info(struct qmc_chan *chan, struct 
qmc_chan_info *info)
 }
 EXPORT_SYMBOL(qmc_chan_get_info);
 
+int qmc_chan_get_ts_info(struct qmc_chan *chan, struct qmc_chan_ts_info 
*ts_info)
+{
+   unsigned long flags;
+
+   spin_lock_irqsave(&chan->ts_lock, flags);
+
+   ts_info->rx_ts_mask_avail = chan->rx_ts_mask_avail;
+   ts_info->tx_ts_mask_avail = chan->tx_ts_mask_avail;
+   ts_info->rx_ts_mask = chan->rx_ts_mask;
+   ts_info->tx_ts_mask = chan->tx_ts_mask;
+
+   spin_unlock_irqrestore(&chan->ts_lock, flags);
+
+   return 0;
+}
+EXPORT_SYMBOL(qmc_chan_get_ts_info);
+
+int qmc_chan_set_ts_info(struct qmc_chan *chan, const struct qmc_chan_ts_info 
*ts_info)
+{
+   unsigned long flags;
+   int ret;
+
+   /* Only a subset of available timeslots is allowed */
+   if ((ts_info->rx_ts_mask & chan->rx_ts_mask_avail) != 
ts_info->rx_ts_mask)
+   return -EINVAL;
+   if ((ts_info->tx_ts_mask & chan->tx_ts_mask_avail) != 
ts_info->tx_ts_mask)
+   return -EINVAL;
+
+   /* In case of common rx/tx table, rx/tx masks must be identical */
+   if (chan->qmc->is_tsa_64rxtx) {
+   if (ts_info->rx_ts_mask != ts_info->tx_ts_mask)
+   return -EINVAL;
+   }
+
+   spin_lock_irqsave(&chan->ts_lock, flags);
+
+   if ((chan->tx_ts_mask != ts_info->tx_ts_mask && !chan->is_tx_stopped) ||
+   (chan->rx_ts_mask != ts_info->rx_ts_mask && !chan->is_rx_stopped)) {
+   dev_err(chan->qmc->dev, "Channel rx and/or tx not stopped\n");
+   ret = -EBUSY;
+   } else {
+   chan->tx_ts_mask = ts_info->tx_ts_mask;
+   chan->rx_ts_mask = ts_info->rx_ts_mask;
+   ret = 0;
+   }
+   spin_unlock_irqrestore(&chan->ts_lock, flags);
+
+   return ret;
+}
+EXPORT_SYMBOL(qmc_chan_set_ts_info);
+
 int qmc_chan_set_param(struct qmc_chan *chan, const struct qmc_chan_param 
*param)
 {
if (param->mode != chan->mode)
diff --git a/include/soc/fsl/qe/qmc.h b/include/soc/fsl/qe/qmc.h
index 166484bb4294..2a333fc1ea81 100644
--- a/include/soc/fsl/qe/qmc.h
+++ b/include/soc/fsl/qe/qmc.h
@@ -40,6 +40,16 @@ struct qmc_chan_info {
 
 int qmc_chan_get_info(struct qmc_chan *chan, struct qmc_chan_info *info);
 
+struct qmc_chan_ts_info {
+   u64 rx_ts_mask_avail;
+   u64 tx_ts_mask_avail;
+   u64 rx_ts_mask;
+   u64 tx_ts_mask;
+};
+
+int qmc_chan_get_ts_info(struct qmc_chan *chan, struct qmc_chan_ts_info 
*ts_info);
+int qmc_chan_set_ts_info(struct qmc_chan *chan, const struct qmc_chan_ts_info 
*ts_info);
+
 struct qmc_chan_param {
enum qmc_mode mode;
union {
-- 
2.42.0

[PATCH 16/17] soc: fsl: cpm1: qmc: Remove timeslots handling from setup_chan()

[Herve Codina]

Timeslots setting is done at channel start() and stop().
There is no more need to do that during setup_chan().

Simply remove timeslot setting from setup_chan().

Signed-off-by: Herve Codina 
Reviewed-by: Christophe Leroy 
---
 drivers/soc/fsl/qe/qmc.c | 28 
 1 file changed, 28 deletions(-)

diff --git a/drivers/soc/fsl/qe/qmc.c b/drivers/soc/fsl/qe/qmc.c
index e56aea5803bf..73903ce31695 100644
--- a/drivers/soc/fsl/qe/qmc.c
+++ b/drivers/soc/fsl/qe/qmc.c
@@ -723,30 +723,6 @@ static int qmc_chan_setup_tsa_rx(struct qmc_chan *chan, 
bool enable)
return qmc_chan_setup_tsa_32rx(chan, &info, enable);
 }
 
-static int qmc_chan_setup_tsa(struct qmc_chan *chan, bool enable)
-{
-   struct tsa_serial_info info;
-   int ret;
-
-   /* Retrieve info from the TSA related serial */
-   ret = tsa_serial_get_info(chan->qmc->tsa_serial, &info);
-   if (ret)
-   return ret;
-
-   /*
-* Setup one common 64 entries table or two 32 entries (one for Tx
-* and one for Tx) according to assigned TS numbers.
-*/
-   if (chan->qmc->is_tsa_64rxtx)
-   return qmc_chan_setup_tsa_64rxtx(chan, &info, enable);
-
-   ret = qmc_chan_setup_tsa_32rx(chan, &info, enable);
-   if (ret)
-   return ret;
-
-   return qmc_chan_setup_tsa_32tx(chan, &info, enable);
-}
-
 static int qmc_chan_command(struct qmc_chan *chan, u8 qmc_opcode)
 {
return cpm_command(chan->id << 2, (qmc_opcode << 4) | 0x0E);
@@ -1323,10 +1299,6 @@ static int qmc_setup_chan(struct qmc *qmc, struct 
qmc_chan *chan)
 
chan->qmc = qmc;
 
-   ret = qmc_chan_setup_tsa(chan, true);
-   if (ret)
-   return ret;
-
/* Set channel specific parameter base address */
chan->s_param = qmc->dpram + (chan->id * 64);
/* 16 bd per channel (8 rx and 8 tx) */
-- 
2.42.0

[PATCH 15/17] soc: fsl: cpm1: qmc: Handle timeslot entries at channel start() and stop()

[Herve Codina]

In order to support runtime timeslot route changes, enable the
channel timeslot entries at channel start() and disable them at
channel stop().

Signed-off-by: Herve Codina 
Reviewed-by: Christophe Leroy 
---
 drivers/soc/fsl/qe/qmc.c | 175 ---
 1 file changed, 163 insertions(+), 12 deletions(-)

diff --git a/drivers/soc/fsl/qe/qmc.c b/drivers/soc/fsl/qe/qmc.c
index e651b3bba1ca..e56aea5803bf 100644
--- a/drivers/soc/fsl/qe/qmc.c
+++ b/drivers/soc/fsl/qe/qmc.c
@@ -177,6 +177,7 @@ struct qmc_chan {
struct qmc *qmc;
void __iomem *s_param;
enum qmc_mode mode;
+   spinlock_t  ts_lock; /* Protect timeslots */
u64 tx_ts_mask_avail;
u64 tx_ts_mask;
u64 rx_ts_mask_avail;
@@ -265,6 +266,7 @@ static void qmc_setbits32(void __iomem *addr, u32 set)
 int qmc_chan_get_info(struct qmc_chan *chan, struct qmc_chan_info *info)
 {
struct tsa_serial_info tsa_info;
+   unsigned long flags;
int ret;
 
/* Retrieve info from the TSA related serial */
@@ -272,6 +274,8 @@ int qmc_chan_get_info(struct qmc_chan *chan, struct 
qmc_chan_info *info)
if (ret)
return ret;
 
+   spin_lock_irqsave(&chan->ts_lock, flags);
+
info->mode = chan->mode;
info->rx_fs_rate = tsa_info.rx_fs_rate;
info->rx_bit_rate = tsa_info.rx_bit_rate;
@@ -280,6 +284,8 @@ int qmc_chan_get_info(struct qmc_chan *chan, struct 
qmc_chan_info *info)
info->tx_bit_rate = tsa_info.tx_bit_rate;
info->nb_rx_ts = hweight64(chan->rx_ts_mask);
 
+   spin_unlock_irqrestore(&chan->ts_lock, flags);
+
return 0;
 }
 EXPORT_SYMBOL(qmc_chan_get_info);
@@ -683,6 +689,40 @@ static int qmc_chan_setup_tsa_32tx(struct qmc_chan *chan, 
const struct tsa_seria
return 0;
 }
 
+static int qmc_chan_setup_tsa_tx(struct qmc_chan *chan, bool enable)
+{
+   struct tsa_serial_info info;
+   int ret;
+
+   /* Retrieve info from the TSA related serial */
+   ret = tsa_serial_get_info(chan->qmc->tsa_serial, &info);
+   if (ret)
+   return ret;
+
+   /* Setup entries */
+   if (chan->qmc->is_tsa_64rxtx)
+   return qmc_chan_setup_tsa_64rxtx(chan, &info, enable);
+
+   return qmc_chan_setup_tsa_32tx(chan, &info, enable);
+}
+
+static int qmc_chan_setup_tsa_rx(struct qmc_chan *chan, bool enable)
+{
+   struct tsa_serial_info info;
+   int ret;
+
+   /* Retrieve info from the TSA related serial */
+   ret = tsa_serial_get_info(chan->qmc->tsa_serial, &info);
+   if (ret)
+   return ret;
+
+   /* Setup entries */
+   if (chan->qmc->is_tsa_64rxtx)
+   return qmc_chan_setup_tsa_64rxtx(chan, &info, enable);
+
+   return qmc_chan_setup_tsa_32rx(chan, &info, enable);
+}
+
 static int qmc_chan_setup_tsa(struct qmc_chan *chan, bool enable)
 {
struct tsa_serial_info info;
@@ -719,6 +759,12 @@ static int qmc_chan_stop_rx(struct qmc_chan *chan)
 
spin_lock_irqsave(&chan->rx_lock, flags);
 
+   if (chan->is_rx_stopped) {
+   /* The channel is already stopped -> simply return ok */
+   ret = 0;
+   goto end;
+   }
+
/* Send STOP RECEIVE command */
ret = qmc_chan_command(chan, 0x0);
if (ret) {
@@ -729,6 +775,15 @@ static int qmc_chan_stop_rx(struct qmc_chan *chan)
 
chan->is_rx_stopped = true;
 
+   if (!chan->qmc->is_tsa_64rxtx || chan->is_tx_stopped) {
+   ret = qmc_chan_setup_tsa_rx(chan, false);
+   if (ret) {
+   dev_err(chan->qmc->dev, "chan %u: Disable tsa entries 
failed (%d)\n",
+   chan->id, ret);
+   goto end;
+   }
+   }
+
 end:
spin_unlock_irqrestore(&chan->rx_lock, flags);
return ret;
@@ -741,6 +796,12 @@ static int qmc_chan_stop_tx(struct qmc_chan *chan)
 
spin_lock_irqsave(&chan->tx_lock, flags);
 
+   if (chan->is_tx_stopped) {
+   /* The channel is already stopped -> simply return ok */
+   ret = 0;
+   goto end;
+   }
+
/* Send STOP TRANSMIT command */
ret = qmc_chan_command(chan, 0x1);
if (ret) {
@@ -751,37 +812,82 @@ static int qmc_chan_stop_tx(struct qmc_chan *chan)
 
chan->is_tx_stopped = true;
 
+   if (!chan->qmc->is_tsa_64rxtx || chan->is_rx_stopped) {
+   ret = qmc_chan_setup_tsa_tx(chan, false);
+   if (ret) {
+   dev_err(chan->qmc->dev, "chan %u: Disable tsa entries 
failed (%d)\n",
+   chan->id, ret);
+   goto end;
+   }
+   }
+
 end:
spin_unlock_irqrestore(&chan->tx_lock, flags);
return ret;
 }
 
+static int qmc_chan_start_rx(struct qmc_chan *chan);
+
 int qmc_chan_stop(struct qmc_chan *chan, int direction)
 {
-   int ret;
+   bool is_r

[PATCH 14/17] soc: fsl: cpm1: qmc: Introduce is_tsa_64rxtx flag

[Herve Codina]

In order to support runtime timeslot route changes, some operations will
be different according the routing table used (common Rx and Tx table or
one table for Rx and one for Tx).

The is_tsa_64rxtx flag is introduced to avoid extra computation to
determine the table format each time we need it.
It is set once at initialization.

Signed-off-by: Herve Codina 
Reviewed-by: Christophe Leroy 
---
 drivers/soc/fsl/qe/qmc.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/soc/fsl/qe/qmc.c b/drivers/soc/fsl/qe/qmc.c
index 5ca4120779f8..e651b3bba1ca 100644
--- a/drivers/soc/fsl/qe/qmc.c
+++ b/drivers/soc/fsl/qe/qmc.c
@@ -216,6 +216,7 @@ struct qmc {
u16 __iomem *int_curr;
dma_addr_t int_dma_addr;
size_t int_size;
+   bool is_tsa_64rxtx;
struct list_head chan_head;
struct qmc_chan *chans[64];
 };
@@ -696,7 +697,7 @@ static int qmc_chan_setup_tsa(struct qmc_chan *chan, bool 
enable)
 * Setup one common 64 entries table or two 32 entries (one for Tx
 * and one for Tx) according to assigned TS numbers.
 */
-   if (info.nb_tx_ts > 32 || info.nb_rx_ts > 32)
+   if (chan->qmc->is_tsa_64rxtx)
return qmc_chan_setup_tsa_64rxtx(chan, &info, enable);
 
ret = qmc_chan_setup_tsa_32rx(chan, &info, enable);
@@ -1053,6 +1054,7 @@ static int qmc_init_tsa_64rxtx(struct qmc *qmc, const 
struct tsa_serial_info *in
 * Everything was previously checked, Tx and Rx related stuffs are
 * identical -> Used Rx related stuff to build the table
 */
+   qmc->is_tsa_64rxtx = true;
 
/* Invalidate all entries */
for (i = 0; i < 64; i++)
@@ -1081,6 +1083,7 @@ static int qmc_init_tsa_32rx_32tx(struct qmc *qmc, const 
struct tsa_serial_info
 * Use a Tx 32 entries table and a Rx 32 entries table.
 * Everything was previously checked.
 */
+   qmc->is_tsa_64rxtx = false;
 
/* Invalidate all entries */
for (i = 0; i < 32; i++) {
-- 
2.42.0

[PATCH 13/17] soc: fsl: cpm1: qmc: Split Tx and Rx TSA entries setup

[Herve Codina]

The Tx and Rx entries for a given channel are set in one function.

In order to modify Rx entries and Tx entries independently of one other,
split this function in one for the Rx part and one for the Tx part.

Signed-off-by: Herve Codina 
Reviewed-by: Christophe Leroy 
---
 drivers/soc/fsl/qe/qmc.c | 49 
 1 file changed, 35 insertions(+), 14 deletions(-)

diff --git a/drivers/soc/fsl/qe/qmc.c b/drivers/soc/fsl/qe/qmc.c
index c1318fad296b..5ca4120779f8 100644
--- a/drivers/soc/fsl/qe/qmc.c
+++ b/drivers/soc/fsl/qe/qmc.c
@@ -610,14 +610,14 @@ static int qmc_chan_setup_tsa_64rxtx(struct qmc_chan 
*chan, const struct tsa_ser
return 0;
 }
 
-static int qmc_chan_setup_tsa_32rx_32tx(struct qmc_chan *chan, const struct 
tsa_serial_info *info,
-   bool enable)
+static int qmc_chan_setup_tsa_32rx(struct qmc_chan *chan, const struct 
tsa_serial_info *info,
+  bool enable)
 {
unsigned int i;
u16 curr;
u16 val;
 
-   /* Use a Tx 32 entries table and a Rx 32 entries table */
+   /* Use a Rx 32 entries table */
 
val = QMC_TSA_VALID | QMC_TSA_MASK | QMC_TSA_CHANNEL(chan->id);
 
@@ -633,6 +633,30 @@ static int qmc_chan_setup_tsa_32rx_32tx(struct qmc_chan 
*chan, const struct tsa_
return -EBUSY;
}
}
+
+   /* Set entries based on Rx stuff */
+   for (i = 0; i < info->nb_rx_ts; i++) {
+   if (!(chan->rx_ts_mask & (((u64)1) << i)))
+   continue;
+
+   qmc_clrsetbits16(chan->qmc->scc_pram + QMC_GBL_TSATRX + (i * 2),
+~QMC_TSA_WRAP, enable ? val : 0x);
+   }
+
+   return 0;
+}
+
+static int qmc_chan_setup_tsa_32tx(struct qmc_chan *chan, const struct 
tsa_serial_info *info,
+  bool enable)
+{
+   unsigned int i;
+   u16 curr;
+   u16 val;
+
+   /* Use a Tx 32 entries table */
+
+   val = QMC_TSA_VALID | QMC_TSA_MASK | QMC_TSA_CHANNEL(chan->id);
+
/* Check entries based on Tx stuff */
for (i = 0; i < info->nb_tx_ts; i++) {
if (!(chan->tx_ts_mask & (((u64)1) << i)))
@@ -646,14 +670,6 @@ static int qmc_chan_setup_tsa_32rx_32tx(struct qmc_chan 
*chan, const struct tsa_
}
}
 
-   /* Set entries based on Rx stuff */
-   for (i = 0; i < info->nb_rx_ts; i++) {
-   if (!(chan->rx_ts_mask & (((u64)1) << i)))
-   continue;
-
-   qmc_clrsetbits16(chan->qmc->scc_pram + QMC_GBL_TSATRX + (i * 2),
-~QMC_TSA_WRAP, enable ? val : 0x);
-   }
/* Set entries based on Tx stuff */
for (i = 0; i < info->nb_tx_ts; i++) {
if (!(chan->tx_ts_mask & (((u64)1) << i)))
@@ -680,9 +696,14 @@ static int qmc_chan_setup_tsa(struct qmc_chan *chan, bool 
enable)
 * Setup one common 64 entries table or two 32 entries (one for Tx
 * and one for Tx) according to assigned TS numbers.
 */
-   return ((info.nb_tx_ts > 32) || (info.nb_rx_ts > 32)) ?
-   qmc_chan_setup_tsa_64rxtx(chan, &info, enable) :
-   qmc_chan_setup_tsa_32rx_32tx(chan, &info, enable);
+   if (info.nb_tx_ts > 32 || info.nb_rx_ts > 32)
+   return qmc_chan_setup_tsa_64rxtx(chan, &info, enable);
+
+   ret = qmc_chan_setup_tsa_32rx(chan, &info, enable);
+   if (ret)
+   return ret;
+
+   return qmc_chan_setup_tsa_32tx(chan, &info, enable);
 }
 
 static int qmc_chan_command(struct qmc_chan *chan, u8 qmc_opcode)
-- 
2.42.0

[PATCH 12/17] soc: fsl: cpm1: qmc: Add support for disabling channel TSA entries

[Herve Codina]

In order to allow runtime timeslot route changes, disabling channel TSA
entries needs to be supported.

Add support for this new feature.

Signed-off-by: Herve Codina 
Reviewed-by: Christophe Leroy 
---
 drivers/soc/fsl/qe/qmc.c | 20 +++-
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/soc/fsl/qe/qmc.c b/drivers/soc/fsl/qe/qmc.c
index 8d71e63d0f21..c1318fad296b 100644
--- a/drivers/soc/fsl/qe/qmc.c
+++ b/drivers/soc/fsl/qe/qmc.c
@@ -567,7 +567,8 @@ static void qmc_chan_read_done(struct qmc_chan *chan)
spin_unlock_irqrestore(&chan->rx_lock, flags);
 }
 
-static int qmc_chan_setup_tsa_64rxtx(struct qmc_chan *chan, const struct 
tsa_serial_info *info)
+static int qmc_chan_setup_tsa_64rxtx(struct qmc_chan *chan, const struct 
tsa_serial_info *info,
+bool enable)
 {
unsigned int i;
u16 curr;
@@ -603,13 +604,14 @@ static int qmc_chan_setup_tsa_64rxtx(struct qmc_chan 
*chan, const struct tsa_ser
continue;
 
qmc_clrsetbits16(chan->qmc->scc_pram + QMC_GBL_TSATRX + (i * 2),
-~QMC_TSA_WRAP, val);
+~QMC_TSA_WRAP, enable ? val : 0x);
}
 
return 0;
 }
 
-static int qmc_chan_setup_tsa_32rx_32tx(struct qmc_chan *chan, const struct 
tsa_serial_info *info)
+static int qmc_chan_setup_tsa_32rx_32tx(struct qmc_chan *chan, const struct 
tsa_serial_info *info,
+   bool enable)
 {
unsigned int i;
u16 curr;
@@ -650,7 +652,7 @@ static int qmc_chan_setup_tsa_32rx_32tx(struct qmc_chan 
*chan, const struct tsa_
continue;
 
qmc_clrsetbits16(chan->qmc->scc_pram + QMC_GBL_TSATRX + (i * 2),
-~QMC_TSA_WRAP, val);
+~QMC_TSA_WRAP, enable ? val : 0x);
}
/* Set entries based on Tx stuff */
for (i = 0; i < info->nb_tx_ts; i++) {
@@ -658,13 +660,13 @@ static int qmc_chan_setup_tsa_32rx_32tx(struct qmc_chan 
*chan, const struct tsa_
continue;
 
qmc_clrsetbits16(chan->qmc->scc_pram + QMC_GBL_TSATTX + (i * 2),
-~QMC_TSA_WRAP, val);
+~QMC_TSA_WRAP, enable ? val : 0x);
}
 
return 0;
 }
 
-static int qmc_chan_setup_tsa(struct qmc_chan *chan)
+static int qmc_chan_setup_tsa(struct qmc_chan *chan, bool enable)
 {
struct tsa_serial_info info;
int ret;
@@ -679,8 +681,8 @@ static int qmc_chan_setup_tsa(struct qmc_chan *chan)
 * and one for Tx) according to assigned TS numbers.
 */
return ((info.nb_tx_ts > 32) || (info.nb_rx_ts > 32)) ?
-   qmc_chan_setup_tsa_64rxtx(chan, &info) :
-   qmc_chan_setup_tsa_32rx_32tx(chan, &info);
+   qmc_chan_setup_tsa_64rxtx(chan, &info, enable) :
+   qmc_chan_setup_tsa_32rx_32tx(chan, &info, enable);
 }
 
 static int qmc_chan_command(struct qmc_chan *chan, u8 qmc_opcode)
@@ -1146,7 +1148,7 @@ static int qmc_setup_chan(struct qmc *qmc, struct 
qmc_chan *chan)
 
chan->qmc = qmc;
 
-   ret = qmc_chan_setup_tsa(chan);
+   ret = qmc_chan_setup_tsa(chan, true);
if (ret)
return ret;
 
-- 
2.42.0

[PATCH 08/17] soc: fsl: cpm1: qmc: Rename qmc_setup_tsa* to qmc_init_tsa*

[Herve Codina]

qmc_setup_tsa* are called once at initialisation.
They initialize the QMC TSA table.
In order to introduce setup function later on for dynamic timeslots
management, rename the function to avoid later confusion.

Signed-off-by: Herve Codina 
Reviewed-by: Christophe Leroy 
---
 drivers/soc/fsl/qe/qmc.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/soc/fsl/qe/qmc.c b/drivers/soc/fsl/qe/qmc.c
index 0413e25d4c67..e3f2afb8fa4d 100644
--- a/drivers/soc/fsl/qe/qmc.c
+++ b/drivers/soc/fsl/qe/qmc.c
@@ -919,7 +919,7 @@ static int qmc_of_parse_chans(struct qmc *qmc, struct 
device_node *np)
return qmc_check_chans(qmc);
 }
 
-static int qmc_setup_tsa_64rxtx(struct qmc *qmc, const struct tsa_serial_info 
*info)
+static int qmc_init_tsa_64rxtx(struct qmc *qmc, const struct tsa_serial_info 
*info)
 {
struct qmc_chan *chan;
unsigned int i;
@@ -961,7 +961,7 @@ static int qmc_setup_tsa_64rxtx(struct qmc *qmc, const 
struct tsa_serial_info *i
return 0;
 }
 
-static int qmc_setup_tsa_32rx_32tx(struct qmc *qmc, const struct 
tsa_serial_info *info)
+static int qmc_init_tsa_32rx_32tx(struct qmc *qmc, const struct 
tsa_serial_info *info)
 {
struct qmc_chan *chan;
unsigned int i;
@@ -1019,7 +1019,7 @@ static int qmc_setup_tsa_32rx_32tx(struct qmc *qmc, const 
struct tsa_serial_info
return 0;
 }
 
-static int qmc_setup_tsa(struct qmc *qmc)
+static int qmc_init_tsa(struct qmc *qmc)
 {
struct tsa_serial_info info;
int ret;
@@ -1030,12 +1030,12 @@ static int qmc_setup_tsa(struct qmc *qmc)
return ret;
 
/*
-* Setup one common 64 entries table or two 32 entries (one for Tx and
-* one for Tx) according to assigned TS numbers.
+* Initialize one common 64 entries table or two 32 entries (one for Tx
+* and one for Tx) according to assigned TS numbers.
 */
return ((info.nb_tx_ts > 32) || (info.nb_rx_ts > 32)) ?
-   qmc_setup_tsa_64rxtx(qmc, &info) :
-   qmc_setup_tsa_32rx_32tx(qmc, &info);
+   qmc_init_tsa_64rxtx(qmc, &info) :
+   qmc_init_tsa_32rx_32tx(qmc, &info);
 }
 
 static int qmc_setup_chan_trnsync(struct qmc *qmc, struct qmc_chan *chan)
@@ -1391,7 +1391,7 @@ static int qmc_probe(struct platform_device *pdev)
qmc_write32(qmc->scc_pram + QMC_GBL_C_MASK32, 0xDEBB20E3);
qmc_write16(qmc->scc_pram + QMC_GBL_C_MASK16, 0xF0B8);
 
-   ret = qmc_setup_tsa(qmc);
+   ret = qmc_init_tsa(qmc);
if (ret)
goto err_tsa_serial_disconnect;
 
-- 
2.42.0

[PATCH 01/17] soc: fsl: cpm1: tsa: Fix __iomem addresses declaration

[Herve Codina]

Running sparse (make C=1) on tsa.c raises a lot of warning such as:
  --- 8< ---
  warning: incorrect type in assignment (different address spaces)
 expected void *[noderef] si_regs
 got void [noderef] __iomem *
  --- 8< ---

Indeed, some variable were declared 'type *__iomem var' instead of
'type __iomem *var'.

Use the correct declaration to remove these warnings.

Fixes: 1d4ba0b81c1c ("soc: fsl: cpm1: Add support for TSA")
Signed-off-by: Herve Codina 
Reviewed-by: Christophe Leroy 
---
 drivers/soc/fsl/qe/tsa.c | 22 +++---
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/soc/fsl/qe/tsa.c b/drivers/soc/fsl/qe/tsa.c
index 3f9981335590..6c5741cf5e9d 100644
--- a/drivers/soc/fsl/qe/tsa.c
+++ b/drivers/soc/fsl/qe/tsa.c
@@ -98,9 +98,9 @@
 #define TSA_SIRP   0x10
 
 struct tsa_entries_area {
-   void *__iomem entries_start;
-   void *__iomem entries_next;
-   void *__iomem last_entry;
+   void __iomem *entries_start;
+   void __iomem *entries_next;
+   void __iomem *last_entry;
 };
 
 struct tsa_tdm {
@@ -117,8 +117,8 @@ struct tsa_tdm {
 
 struct tsa {
struct device *dev;
-   void *__iomem si_regs;
-   void *__iomem si_ram;
+   void __iomem *si_regs;
+   void __iomem *si_ram;
resource_size_t si_ram_sz;
spinlock_t  lock;
int tdms; /* TSA_TDMx ORed */
@@ -135,27 +135,27 @@ static inline struct tsa *tsa_serial_get_tsa(struct 
tsa_serial *tsa_serial)
return container_of(tsa_serial, struct tsa, serials[tsa_serial->id]);
 }
 
-static inline void tsa_write32(void *__iomem addr, u32 val)
+static inline void tsa_write32(void __iomem *addr, u32 val)
 {
iowrite32be(val, addr);
 }
 
-static inline void tsa_write8(void *__iomem addr, u32 val)
+static inline void tsa_write8(void __iomem *addr, u32 val)
 {
iowrite8(val, addr);
 }
 
-static inline u32 tsa_read32(void *__iomem addr)
+static inline u32 tsa_read32(void __iomem *addr)
 {
return ioread32be(addr);
 }
 
-static inline void tsa_clrbits32(void *__iomem addr, u32 clr)
+static inline void tsa_clrbits32(void __iomem *addr, u32 clr)
 {
tsa_write32(addr, tsa_read32(addr) & ~clr);
 }
 
-static inline void tsa_clrsetbits32(void *__iomem addr, u32 clr, u32 set)
+static inline void tsa_clrsetbits32(void __iomem *addr, u32 clr, u32 set)
 {
tsa_write32(addr, (tsa_read32(addr) & ~clr) | set);
 }
@@ -313,7 +313,7 @@ static u32 tsa_serial_id2csel(struct tsa *tsa, u32 
serial_id)
 static int tsa_add_entry(struct tsa *tsa, struct tsa_entries_area *area,
 u32 count, u32 serial_id)
 {
-   void *__iomem addr;
+   void __iomem *addr;
u32 left;
u32 val;
u32 cnt;
-- 
2.42.0

[PATCH 06/17] soc: fsl: cpm1: qmc: Add support for child devices

[Herve Codina]

QMC child devices support is needed to avoid orphan DT nodes that use a
simple DT phandle to reference a QMC channel.

Allow to instantiate child devices and also extend the API to get the
qmc_chan using a child device.

Signed-off-by: Herve Codina 
---
 drivers/soc/fsl/qe/qmc.c | 91 +++-
 include/soc/fsl/qe/qmc.h |  2 +
 2 files changed, 73 insertions(+), 20 deletions(-)

diff --git a/drivers/soc/fsl/qe/qmc.c b/drivers/soc/fsl/qe/qmc.c
index 27f2f16deac9..e716f13669a0 100644
--- a/drivers/soc/fsl/qe/qmc.c
+++ b/drivers/soc/fsl/qe/qmc.c
@@ -1425,8 +1425,16 @@ static int qmc_probe(struct platform_device *pdev)
 
platform_set_drvdata(pdev, qmc);
 
+   /* Populate channel related devices */
+   ret = devm_of_platform_populate(qmc->dev);
+   if (ret)
+   goto err_disable_txrx;
+
return 0;
 
+err_disable_txrx:
+   qmc_setbits32(qmc->scc_regs + SCC_GSMRL, 0);
+
 err_disable_intr:
qmc_write16(qmc->scc_regs + SCC_SCCM, 0);
 
@@ -1465,26 +1473,16 @@ static struct platform_driver qmc_driver = {
 };
 module_platform_driver(qmc_driver);
 
-struct qmc_chan *qmc_chan_get_byphandle(struct device_node *np, const char 
*phandle_name)
+static struct qmc_chan *qmc_chan_get_from_qmc(struct device_node *qmc_np, 
unsigned int chan_index)
 {
-   struct of_phandle_args out_args;
struct platform_device *pdev;
struct qmc_chan *qmc_chan;
struct qmc *qmc;
-   int ret;
 
-   ret = of_parse_phandle_with_fixed_args(np, phandle_name, 1, 0,
-  &out_args);
-   if (ret < 0)
-   return ERR_PTR(ret);
-
-   if (!of_match_node(qmc_driver.driver.of_match_table, out_args.np)) {
-   of_node_put(out_args.np);
+   if (!of_match_node(qmc_driver.driver.of_match_table, qmc_np))
return ERR_PTR(-EINVAL);
-   }
 
-   pdev = of_find_device_by_node(out_args.np);
-   of_node_put(out_args.np);
+   pdev = of_find_device_by_node(qmc_np);
if (!pdev)
return ERR_PTR(-ENODEV);
 
@@ -1494,17 +1492,12 @@ struct qmc_chan *qmc_chan_get_byphandle(struct 
device_node *np, const char *phan
return ERR_PTR(-EPROBE_DEFER);
}
 
-   if (out_args.args_count != 1) {
+   if (chan_index >= ARRAY_SIZE(qmc->chans)) {
platform_device_put(pdev);
return ERR_PTR(-EINVAL);
}
 
-   if (out_args.args[0] >= ARRAY_SIZE(qmc->chans)) {
-   platform_device_put(pdev);
-   return ERR_PTR(-EINVAL);
-   }
-
-   qmc_chan = qmc->chans[out_args.args[0]];
+   qmc_chan = qmc->chans[chan_index];
if (!qmc_chan) {
platform_device_put(pdev);
return ERR_PTR(-ENOENT);
@@ -1512,8 +1505,44 @@ struct qmc_chan *qmc_chan_get_byphandle(struct 
device_node *np, const char *phan
 
return qmc_chan;
 }
+
+struct qmc_chan *qmc_chan_get_byphandle(struct device_node *np, const char 
*phandle_name)
+{
+   struct of_phandle_args out_args;
+   struct qmc_chan *qmc_chan;
+   int ret;
+
+   ret = of_parse_phandle_with_fixed_args(np, phandle_name, 1, 0,
+  &out_args);
+   if (ret < 0)
+   return ERR_PTR(ret);
+
+   if (out_args.args_count != 1) {
+   of_node_put(out_args.np);
+   return ERR_PTR(-EINVAL);
+   }
+
+   qmc_chan = qmc_chan_get_from_qmc(out_args.np, out_args.args[0]);
+   of_node_put(out_args.np);
+   return qmc_chan;
+}
 EXPORT_SYMBOL(qmc_chan_get_byphandle);
 
+struct qmc_chan *qmc_chan_get_bychild(struct device_node *np)
+{
+   struct device_node *qmc_np;
+   u32 chan_index;
+   int ret;
+
+   qmc_np = np->parent;
+   ret = of_property_read_u32(np, "reg", &chan_index);
+   if (ret)
+   return ERR_PTR(-EINVAL);
+
+   return qmc_chan_get_from_qmc(qmc_np, chan_index);
+}
+EXPORT_SYMBOL(qmc_chan_get_bychild);
+
 void qmc_chan_put(struct qmc_chan *chan)
 {
put_device(chan->qmc->dev);
@@ -1550,6 +1579,28 @@ struct qmc_chan *devm_qmc_chan_get_byphandle(struct 
device *dev,
 }
 EXPORT_SYMBOL(devm_qmc_chan_get_byphandle);
 
+struct qmc_chan *devm_qmc_chan_get_bychild(struct device *dev,
+  struct device_node *np)
+{
+   struct qmc_chan *qmc_chan;
+   struct qmc_chan **dr;
+
+   dr = devres_alloc(devm_qmc_chan_release, sizeof(*dr), GFP_KERNEL);
+   if (!dr)
+   return ERR_PTR(-ENOMEM);
+
+   qmc_chan = qmc_chan_get_bychild(np);
+   if (!IS_ERR(qmc_chan)) {
+   *dr = qmc_chan;
+   devres_add(dev, dr);
+   } else {
+   devres_free(dr);
+   }
+
+   return qmc_chan;
+}
+EXPORT_SYMBOL(devm_qmc_chan_get_bychild);
+
 MODULE_AUTHOR("Herve Codina ");
 MODULE_DESCRIPTION("CPM QMC driver");
 MODULE_LICENSE("GPL");
diff --git a/include/

[PATCH 02/17] soc: fsl: cpm1: qmc: Fix __iomem addresses declaration

[Herve Codina]

Running sparse (make C=1) on qmc.c raises a lot of warning such as:
  ...
  warning: incorrect type in assignment (different address spaces)
 expected struct cpm_buf_desc [usertype] *[noderef] __iomem bd
 got struct cpm_buf_desc [noderef] [usertype] __iomem *txbd_free
  ...

Indeed, some variable were declared 'type *__iomem var' instead of
'type __iomem *var'.

Use the correct declaration to remove these warnings.

Fixes: 3178d58e0b97 ("soc: fsl: cpm1: Add support for QMC")
Signed-off-by: Herve Codina 
Reviewed-by: Christophe Leroy 
---
 drivers/soc/fsl/qe/qmc.c | 34 +-
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/soc/fsl/qe/qmc.c b/drivers/soc/fsl/qe/qmc.c
index 92ec76c03965..3f3de1351c96 100644
--- a/drivers/soc/fsl/qe/qmc.c
+++ b/drivers/soc/fsl/qe/qmc.c
@@ -175,7 +175,7 @@ struct qmc_chan {
struct list_head list;
unsigned int id;
struct qmc *qmc;
-   void *__iomem s_param;
+   void __iomem *s_param;
enum qmc_mode mode;
u64 tx_ts_mask;
u64 rx_ts_mask;
@@ -203,9 +203,9 @@ struct qmc_chan {
 struct qmc {
struct device *dev;
struct tsa_serial *tsa_serial;
-   void *__iomem scc_regs;
-   void *__iomem scc_pram;
-   void *__iomem dpram;
+   void __iomem *scc_regs;
+   void __iomem *scc_pram;
+   void __iomem *dpram;
u16 scc_pram_offset;
cbd_t __iomem *bd_table;
dma_addr_t bd_dma_addr;
@@ -218,37 +218,37 @@ struct qmc {
struct qmc_chan *chans[64];
 };
 
-static inline void qmc_write16(void *__iomem addr, u16 val)
+static inline void qmc_write16(void __iomem *addr, u16 val)
 {
iowrite16be(val, addr);
 }
 
-static inline u16 qmc_read16(void *__iomem addr)
+static inline u16 qmc_read16(void __iomem *addr)
 {
return ioread16be(addr);
 }
 
-static inline void qmc_setbits16(void *__iomem addr, u16 set)
+static inline void qmc_setbits16(void __iomem *addr, u16 set)
 {
qmc_write16(addr, qmc_read16(addr) | set);
 }
 
-static inline void qmc_clrbits16(void *__iomem addr, u16 clr)
+static inline void qmc_clrbits16(void __iomem *addr, u16 clr)
 {
qmc_write16(addr, qmc_read16(addr) & ~clr);
 }
 
-static inline void qmc_write32(void *__iomem addr, u32 val)
+static inline void qmc_write32(void __iomem *addr, u32 val)
 {
iowrite32be(val, addr);
 }
 
-static inline u32 qmc_read32(void *__iomem addr)
+static inline u32 qmc_read32(void __iomem *addr)
 {
return ioread32be(addr);
 }
 
-static inline void qmc_setbits32(void *__iomem addr, u32 set)
+static inline void qmc_setbits32(void __iomem *addr, u32 set)
 {
qmc_write32(addr, qmc_read32(addr) | set);
 }
@@ -318,7 +318,7 @@ int qmc_chan_write_submit(struct qmc_chan *chan, dma_addr_t 
addr, size_t length,
 {
struct qmc_xfer_desc *xfer_desc;
unsigned long flags;
-   cbd_t *__iomem bd;
+   cbd_t __iomem *bd;
u16 ctrl;
int ret;
 
@@ -374,7 +374,7 @@ static void qmc_chan_write_done(struct qmc_chan *chan)
void (*complete)(void *context);
unsigned long flags;
void *context;
-   cbd_t *__iomem bd;
+   cbd_t __iomem *bd;
u16 ctrl;
 
/*
@@ -425,7 +425,7 @@ int qmc_chan_read_submit(struct qmc_chan *chan, dma_addr_t 
addr, size_t length,
 {
struct qmc_xfer_desc *xfer_desc;
unsigned long flags;
-   cbd_t *__iomem bd;
+   cbd_t __iomem *bd;
u16 ctrl;
int ret;
 
@@ -488,7 +488,7 @@ static void qmc_chan_read_done(struct qmc_chan *chan)
void (*complete)(void *context, size_t size);
struct qmc_xfer_desc *xfer_desc;
unsigned long flags;
-   cbd_t *__iomem bd;
+   cbd_t __iomem *bd;
void *context;
u16 datalen;
u16 ctrl;
@@ -663,7 +663,7 @@ static void qmc_chan_reset_rx(struct qmc_chan *chan)
 {
struct qmc_xfer_desc *xfer_desc;
unsigned long flags;
-   cbd_t *__iomem bd;
+   cbd_t __iomem *bd;
u16 ctrl;
 
spin_lock_irqsave(&chan->rx_lock, flags);
@@ -694,7 +694,7 @@ static void qmc_chan_reset_tx(struct qmc_chan *chan)
 {
struct qmc_xfer_desc *xfer_desc;
unsigned long flags;
-   cbd_t *__iomem bd;
+   cbd_t __iomem *bd;
u16 ctrl;
 
spin_lock_irqsave(&chan->tx_lock, flags);
-- 
2.42.0

[PATCH 03/17] soc: fsl: cpm1: qmc: Fix rx channel reset

[Herve Codina]

The qmc_chan_reset_rx() set the is_rx_stopped flag. This leads to an
inconsistent state in the following sequence.
qmc_chan_stop()
qmc_chan_reset()
Indeed, after the qmc_chan_reset() call, the channel must still be
stopped. Only a qmc_chan_start() call can move the channel from stopped
state to started state.

Fix the issue removing the is_rx_stopped flag setting from
qmc_chan_reset()

Fixes: 3178d58e0b97 ("soc: fsl: cpm1: Add support for QMC")
Signed-off-by: Herve Codina 
Reviewed-by: Christophe Leroy 
---
 drivers/soc/fsl/qe/qmc.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/soc/fsl/qe/qmc.c b/drivers/soc/fsl/qe/qmc.c
index 3f3de1351c96..2312152a44b3 100644
--- a/drivers/soc/fsl/qe/qmc.c
+++ b/drivers/soc/fsl/qe/qmc.c
@@ -685,7 +685,6 @@ static void qmc_chan_reset_rx(struct qmc_chan *chan)
qmc_read16(chan->s_param + QMC_SPE_RBASE));
 
chan->rx_pending = 0;
-   chan->is_rx_stopped = false;
 
spin_unlock_irqrestore(&chan->rx_lock, flags);
 }
-- 
2.42.0

[PATCH 09/17] soc: fsl: cpm1: qmc: Introduce qmc_chan_setup_tsa*

[Herve Codina]

Introduce the qmc_chan_setup_tsa* functions to setup entries related
to the given channel.
Use them during QMC channels setup.

Signed-off-by: Herve Codina 
Reviewed-by: Christophe Leroy 
---
 drivers/soc/fsl/qe/qmc.c | 161 ++-
 1 file changed, 125 insertions(+), 36 deletions(-)

diff --git a/drivers/soc/fsl/qe/qmc.c b/drivers/soc/fsl/qe/qmc.c
index e3f2afb8fa4d..5d7e2ecdd933 100644
--- a/drivers/soc/fsl/qe/qmc.c
+++ b/drivers/soc/fsl/qe/qmc.c
@@ -240,6 +240,11 @@ static void qmc_clrbits16(void __iomem *addr, u16 clr)
qmc_write16(addr, qmc_read16(addr) & ~clr);
 }
 
+static void qmc_clrsetbits16(void __iomem *addr, u16 clr, u16 set)
+{
+   qmc_write16(addr, (qmc_read16(addr) & ~clr) | set);
+}
+
 static void qmc_write32(void __iomem *addr, u32 val)
 {
iowrite32be(val, addr);
@@ -562,6 +567,122 @@ static void qmc_chan_read_done(struct qmc_chan *chan)
spin_unlock_irqrestore(&chan->rx_lock, flags);
 }
 
+static int qmc_chan_setup_tsa_64rxtx(struct qmc_chan *chan, const struct 
tsa_serial_info *info)
+{
+   unsigned int i;
+   u16 curr;
+   u16 val;
+
+   /*
+* Use a common Tx/Rx 64 entries table.
+* Tx and Rx related stuffs must be identical
+*/
+   if (chan->tx_ts_mask != chan->rx_ts_mask) {
+   dev_err(chan->qmc->dev, "chan %u uses different Rx and Tx 
TS\n", chan->id);
+   return -EINVAL;
+   }
+
+   val = QMC_TSA_VALID | QMC_TSA_MASK | QMC_TSA_CHANNEL(chan->id);
+
+   /* Check entries based on Rx stuff*/
+   for (i = 0; i < info->nb_rx_ts; i++) {
+   if (!(chan->rx_ts_mask & (((u64)1) << i)))
+   continue;
+
+   curr = qmc_read16(chan->qmc->scc_pram + QMC_GBL_TSATRX + (i * 
2));
+   if (curr & QMC_TSA_VALID && (curr & ~QMC_TSA_WRAP) != val) {
+   dev_err(chan->qmc->dev, "chan %u TxRx entry %d already 
used\n",
+   chan->id, i);
+   return -EBUSY;
+   }
+   }
+
+   /* Set entries based on Rx stuff*/
+   for (i = 0; i < info->nb_rx_ts; i++) {
+   if (!(chan->rx_ts_mask & (((u64)1) << i)))
+   continue;
+
+   qmc_clrsetbits16(chan->qmc->scc_pram + QMC_GBL_TSATRX + (i * 2),
+~QMC_TSA_WRAP, val);
+   }
+
+   return 0;
+}
+
+static int qmc_chan_setup_tsa_32rx_32tx(struct qmc_chan *chan, const struct 
tsa_serial_info *info)
+{
+   unsigned int i;
+   u16 curr;
+   u16 val;
+
+   /* Use a Tx 32 entries table and a Rx 32 entries table */
+
+   val = QMC_TSA_VALID | QMC_TSA_MASK | QMC_TSA_CHANNEL(chan->id);
+
+   /* Check entries based on Rx stuff */
+   for (i = 0; i < info->nb_rx_ts; i++) {
+   if (!(chan->rx_ts_mask & (((u64)1) << i)))
+   continue;
+
+   curr = qmc_read16(chan->qmc->scc_pram + QMC_GBL_TSATRX + (i * 
2));
+   if (curr & QMC_TSA_VALID && (curr & ~QMC_TSA_WRAP) != val) {
+   dev_err(chan->qmc->dev, "chan %u Rx entry %d already 
used\n",
+   chan->id, i);
+   return -EBUSY;
+   }
+   }
+   /* Check entries based on Tx stuff */
+   for (i = 0; i < info->nb_tx_ts; i++) {
+   if (!(chan->tx_ts_mask & (((u64)1) << i)))
+   continue;
+
+   curr = qmc_read16(chan->qmc->scc_pram + QMC_GBL_TSATTX + (i * 
2));
+   if (curr & QMC_TSA_VALID && (curr & ~QMC_TSA_WRAP) != val) {
+   dev_err(chan->qmc->dev, "chan %u Tx entry %d already 
used\n",
+   chan->id, i);
+   return -EBUSY;
+   }
+   }
+
+   /* Set entries based on Rx stuff */
+   for (i = 0; i < info->nb_rx_ts; i++) {
+   if (!(chan->rx_ts_mask & (((u64)1) << i)))
+   continue;
+
+   qmc_clrsetbits16(chan->qmc->scc_pram + QMC_GBL_TSATRX + (i * 2),
+~QMC_TSA_WRAP, val);
+   }
+   /* Set entries based on Tx stuff */
+   for (i = 0; i < info->nb_tx_ts; i++) {
+   if (!(chan->tx_ts_mask & (((u64)1) << i)))
+   continue;
+
+   qmc_clrsetbits16(chan->qmc->scc_pram + QMC_GBL_TSATTX + (i * 2),
+~QMC_TSA_WRAP, val);
+   }
+
+   return 0;
+}
+
+static int qmc_chan_setup_tsa(struct qmc_chan *chan)
+{
+   struct tsa_serial_info info;
+   int ret;
+
+   /* Retrieve info from the TSA related serial */
+   ret = tsa_serial_get_info(chan->qmc->tsa_serial, &info);
+   if (ret)
+   return ret;
+
+   /*
+* Setup one common 64 entries table or two 32 entries (one for Tx
+* and one for Tx) according to assigned TS numbers.
+*/
+   return ((info.nb_tx_ts

[PATCH 11/17] soc: fsl: cpm1: qmc: Check available timeslots in qmc_check_chans()

[Herve Codina]

The timeslots checked in qmc_check_chans() are the timeslots used.
With the introduction of the available timeslots, the used timeslots
are a subset of the available timeslots. The timeslots checked during
the qmc_check_chans() call should be the available ones.

Simply update and check the available timeslots instead of the used
timeslots in qmc_check_chans().

Signed-off-by: Herve Codina 
Reviewed-by: Christophe Leroy 
---
 drivers/soc/fsl/qe/qmc.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/soc/fsl/qe/qmc.c b/drivers/soc/fsl/qe/qmc.c
index f2a71a140db7..8d71e63d0f21 100644
--- a/drivers/soc/fsl/qe/qmc.c
+++ b/drivers/soc/fsl/qe/qmc.c
@@ -914,13 +914,13 @@ static int qmc_check_chans(struct qmc *qmc)
rx_ts_assigned_mask = info.nb_rx_ts == 64 ? U64_MAX : (((u64)1) << 
info.nb_rx_ts) - 1;
 
list_for_each_entry(chan, &qmc->chan_head, list) {
-   if (chan->tx_ts_mask > tx_ts_assigned_mask) {
-   dev_err(qmc->dev, "chan %u uses TSA unassigned Tx 
TS\n", chan->id);
+   if (chan->tx_ts_mask_avail > tx_ts_assigned_mask) {
+   dev_err(qmc->dev, "chan %u can use TSA unassigned Tx 
TS\n", chan->id);
return -EINVAL;
}
 
-   if (chan->rx_ts_mask > rx_ts_assigned_mask) {
-   dev_err(qmc->dev, "chan %u uses TSA unassigned Rx 
TS\n", chan->id);
+   if (chan->rx_ts_mask_avail > rx_ts_assigned_mask) {
+   dev_err(qmc->dev, "chan %u can use TSA unassigned Rx 
TS\n", chan->id);
return -EINVAL;
}
}
-- 
2.42.0

[PATCH 10/17] soc: fsl: cpm1: qmc: Remove no more needed checks from qmc_check_chans()

[Herve Codina]

The newly introduced qmc_chan_setup_tsa* functions check that the
channel entries are not already used.
These checks are also performed by qmc_check_chans() and are no more
needed.

Remove them from qmc_check_chans().

Signed-off-by: Herve Codina 
Reviewed-by: Christophe Leroy 
---
 drivers/soc/fsl/qe/qmc.c | 20 
 1 file changed, 20 deletions(-)

diff --git a/drivers/soc/fsl/qe/qmc.c b/drivers/soc/fsl/qe/qmc.c
index 5d7e2ecdd933..f2a71a140db7 100644
--- a/drivers/soc/fsl/qe/qmc.c
+++ b/drivers/soc/fsl/qe/qmc.c
@@ -884,10 +884,7 @@ EXPORT_SYMBOL(qmc_chan_reset);
 static int qmc_check_chans(struct qmc *qmc)
 {
struct tsa_serial_info info;
-   bool is_one_table = false;
struct qmc_chan *chan;
-   u64 tx_ts_mask = 0;
-   u64 rx_ts_mask = 0;
u64 tx_ts_assigned_mask;
u64 rx_ts_assigned_mask;
int ret;
@@ -911,7 +908,6 @@ static int qmc_check_chans(struct qmc *qmc)
dev_err(qmc->dev, "Number of TSA Tx/Rx TS assigned are 
not equal\n");
return -EINVAL;
}
-   is_one_table = true;
}
 
tx_ts_assigned_mask = info.nb_tx_ts == 64 ? U64_MAX : (((u64)1) << 
info.nb_tx_ts) - 1;
@@ -922,27 +918,11 @@ static int qmc_check_chans(struct qmc *qmc)
dev_err(qmc->dev, "chan %u uses TSA unassigned Tx 
TS\n", chan->id);
return -EINVAL;
}
-   if (tx_ts_mask & chan->tx_ts_mask) {
-   dev_err(qmc->dev, "chan %u uses an already used Tx 
TS\n", chan->id);
-   return -EINVAL;
-   }
 
if (chan->rx_ts_mask > rx_ts_assigned_mask) {
dev_err(qmc->dev, "chan %u uses TSA unassigned Rx 
TS\n", chan->id);
return -EINVAL;
}
-   if (rx_ts_mask & chan->rx_ts_mask) {
-   dev_err(qmc->dev, "chan %u uses an already used Rx 
TS\n", chan->id);
-   return -EINVAL;
-   }
-
-   if (is_one_table && (chan->tx_ts_mask != chan->rx_ts_mask)) {
-   dev_err(qmc->dev, "chan %u uses different Rx and Tx 
TS\n", chan->id);
-   return -EINVAL;
-   }
-
-   tx_ts_mask |= chan->tx_ts_mask;
-   rx_ts_mask |= chan->rx_ts_mask;
}
 
return 0;
-- 
2.42.0

[PATCH 07/17] soc: fsl: cpm1: qmc: Introduce available timeslots masks

[Herve Codina]

Available timeslots masks define timeslots available for the related
channel. These timeslots are defined by the QMC binding.

Timeslots used are initialized to available timeslots but can be a
subset of available timeslots.
This prepares the dynamic timeslots management (ie. changing timeslots
at runtime).

Signed-off-by: Herve Codina 
Reviewed-by: Christophe Leroy 
---
 drivers/soc/fsl/qe/qmc.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/soc/fsl/qe/qmc.c b/drivers/soc/fsl/qe/qmc.c
index e716f13669a0..0413e25d4c67 100644
--- a/drivers/soc/fsl/qe/qmc.c
+++ b/drivers/soc/fsl/qe/qmc.c
@@ -177,7 +177,9 @@ struct qmc_chan {
struct qmc *qmc;
void __iomem *s_param;
enum qmc_mode mode;
+   u64 tx_ts_mask_avail;
u64 tx_ts_mask;
+   u64 rx_ts_mask_avail;
u64 rx_ts_mask;
bool is_reverse_data;
 
@@ -875,7 +877,8 @@ static int qmc_of_parse_chans(struct qmc *qmc, struct 
device_node *np)
of_node_put(chan_np);
return ret;
}
-   chan->tx_ts_mask = ts_mask;
+   chan->tx_ts_mask_avail = ts_mask;
+   chan->tx_ts_mask = chan->tx_ts_mask_avail;
 
ret = of_property_read_u64(chan_np, "fsl,rx-ts-mask", &ts_mask);
if (ret) {
@@ -884,7 +887,8 @@ static int qmc_of_parse_chans(struct qmc *qmc, struct 
device_node *np)
of_node_put(chan_np);
return ret;
}
-   chan->rx_ts_mask = ts_mask;
+   chan->rx_ts_mask_avail = ts_mask;
+   chan->rx_ts_mask = chan->rx_ts_mask_avail;
 
mode = "transparent";
ret = of_property_read_string(chan_np, "fsl,operational-mode", 
&mode);
-- 
2.42.0

[PATCH 04/17] soc: fsl: cpm1: qmc: Extend the API to provide Rx status

[Herve Codina]

In HDLC mode, some status flags related to the data read transfer can be
set by the hardware and need to be known by a QMC consumer for further
analysis.

Extend the API in order to provide these transfer status flags at the
read complete() call.

In TRANSPARENT mode, these flags have no meaning. Keep only one read
complete() API and update the consumers working in transparent mode.
In this case, the newly introduced flags parameter is simply unused.

Signed-off-by: Herve Codina 
Reviewed-by: Christophe Leroy 
---
 drivers/soc/fsl/qe/qmc.c  | 29 +
 include/soc/fsl/qe/qmc.h  | 15 ++-
 sound/soc/fsl/fsl_qmc_audio.c |  2 +-
 3 files changed, 40 insertions(+), 6 deletions(-)

diff --git a/drivers/soc/fsl/qe/qmc.c b/drivers/soc/fsl/qe/qmc.c
index 2312152a44b3..4b4832d93c9b 100644
--- a/drivers/soc/fsl/qe/qmc.c
+++ b/drivers/soc/fsl/qe/qmc.c
@@ -166,7 +166,7 @@
 struct qmc_xfer_desc {
union {
void (*tx_complete)(void *context);
-   void (*rx_complete)(void *context, size_t length);
+   void (*rx_complete)(void *context, size_t length, unsigned int 
flags);
};
void *context;
 };
@@ -421,7 +421,8 @@ static void qmc_chan_write_done(struct qmc_chan *chan)
 }
 
 int qmc_chan_read_submit(struct qmc_chan *chan, dma_addr_t addr, size_t length,
-void (*complete)(void *context, size_t length), void 
*context)
+void (*complete)(void *context, size_t length, 
unsigned int flags),
+void *context)
 {
struct qmc_xfer_desc *xfer_desc;
unsigned long flags;
@@ -454,6 +455,10 @@ int qmc_chan_read_submit(struct qmc_chan *chan, dma_addr_t 
addr, size_t length,
xfer_desc->rx_complete = complete;
xfer_desc->context = context;
 
+   /* Clear previous status flags */
+   ctrl &= ~(QMC_BD_RX_L | QMC_BD_RX_F | QMC_BD_RX_LG | QMC_BD_RX_NO |
+ QMC_BD_RX_AB | QMC_BD_RX_CR);
+
/* Activate the descriptor */
ctrl |= (QMC_BD_RX_E | QMC_BD_RX_UB);
wmb(); /* Be sure to flush data before descriptor activation */
@@ -485,7 +490,7 @@ EXPORT_SYMBOL(qmc_chan_read_submit);
 
 static void qmc_chan_read_done(struct qmc_chan *chan)
 {
-   void (*complete)(void *context, size_t size);
+   void (*complete)(void *context, size_t size, unsigned int flags);
struct qmc_xfer_desc *xfer_desc;
unsigned long flags;
cbd_t __iomem *bd;
@@ -527,7 +532,23 @@ static void qmc_chan_read_done(struct qmc_chan *chan)
 
if (complete) {
spin_unlock_irqrestore(&chan->rx_lock, flags);
-   complete(context, datalen);
+
+   /*
+* Avoid conversion between internal hardware flags and
+* the software API flags.
+* -> Be sure that the software API flags are consistent
+*with the hardware flags
+*/
+   BUILD_BUG_ON(QMC_RX_FLAG_HDLC_LAST  != QMC_BD_RX_L);
+   BUILD_BUG_ON(QMC_RX_FLAG_HDLC_FIRST != QMC_BD_RX_F);
+   BUILD_BUG_ON(QMC_RX_FLAG_HDLC_OVF   != QMC_BD_RX_LG);
+   BUILD_BUG_ON(QMC_RX_FLAG_HDLC_UNA   != QMC_BD_RX_NO);
+   BUILD_BUG_ON(QMC_RX_FLAG_HDLC_ABORT != QMC_BD_RX_AB);
+   BUILD_BUG_ON(QMC_RX_FLAG_HDLC_CRC   != QMC_BD_RX_CR);
+
+   complete(context, datalen,
+ctrl & (QMC_BD_RX_L | QMC_BD_RX_F | 
QMC_BD_RX_LG |
+QMC_BD_RX_NO | QMC_BD_RX_AB | 
QMC_BD_RX_CR));
spin_lock_irqsave(&chan->rx_lock, flags);
}
 
diff --git a/include/soc/fsl/qe/qmc.h b/include/soc/fsl/qe/qmc.h
index 3c61a50d2ae2..6f1d6cebc9fe 100644
--- a/include/soc/fsl/qe/qmc.h
+++ b/include/soc/fsl/qe/qmc.h
@@ -9,6 +9,7 @@
 #ifndef __SOC_FSL_QMC_H__
 #define __SOC_FSL_QMC_H__
 
+#include 
 #include 
 
 struct device_node;
@@ -56,8 +57,20 @@ int qmc_chan_set_param(struct qmc_chan *chan, const struct 
qmc_chan_param *param
 int qmc_chan_write_submit(struct qmc_chan *chan, dma_addr_t addr, size_t 
length,
  void (*complete)(void *context), void *context);
 
+/* Flags available (ORed) for read complete() flags parameter in HDLC mode.
+ * No flags are available in transparent mode and the read complete() flags
+ * parameter has no meaning in transparent mode.
+ */
+#define QMC_RX_FLAG_HDLC_LAST  BIT(11) /* Last in frame */
+#define QMC_RX_FLAG_HDLC_FIRST BIT(10) /* First in frame */
+#define QMC_RX_FLAG_HDLC_OVF   BIT(5)  /* Data overflow */
+#define QMC_RX_FLAG_HDLC_UNA   BIT(4)  /* Unaligned (ie. bits received not 
multiple of 8) */
+#define QMC_RX_FLAG_HDLC_ABORT BIT(3)  /* Received an abort sequence (seven 
consecutive ones) */
+#define QMC_RX_FLAG_HDLC_CR

[PATCH 00/17] Prepare the PowerQUICC QMC and TSA for the HDLC QMC driver

[Herve Codina]

Hi,

This series updates PowerQUICC QMC and TSA drivers to prepare the
support for the QMC HDLC driver.

Patches were previously sent as part of a full feature series:
"Add support for QMC HDLC, framer infrastructure and PEF2256 framer" [1]

The full feature series reached the v9 iteration.
The v1 was sent the 07/25/2023 followed by the other iterations
(07/26/2023, 08/09/2023, 08/18/2023, 09/12/2023, 09/22/2023, 09/28/2023,
10/11/23, 11/15/2023) and was ready to be merged in its v8.
  https://lore.kernel.org/linux-kernel/20231025123215.5caca...@kernel.org/

The lack of feedback from the Freescale SoC and the Quicc Engine
maintainers (i.e. drivers/soc/fsl/qe/ to which the QMC and TSA drivers
belong) blocks the entire full feature series.
These patches are fixes and improvements to TSA and QMC drivers.
These drivers were previously acked by Li Yang but without any feedback
from Li Yang nor Qiang Zhao the series cannot move forward.

In order to ease the review/merge, the full feature series has been
split and this series contains patches related to the PowerQUICC SoC
part (QMC and TSA).
 - Perform some fixes (patches 1 to 5)
 - Add support for child devices (patch 6)
 - Add QMC dynamic timeslot support (patches 7 to 17)

>From the original full feature series, a patches extraction without any
modification was done.

Best regards,
Hervé

[1]: 
https://lore.kernel.org/linux-kernel/20231115144007.478111-1-herve.cod...@bootlin.com/

Patches extracted:
  - Patch 1..6 : full feature series patch 1..6
  - Patch 7..17 : full feature series patch 9..19

Herve Codina (17):
  soc: fsl: cpm1: tsa: Fix __iomem addresses declaration
  soc: fsl: cpm1: qmc: Fix __iomem addresses declaration
  soc: fsl: cpm1: qmc: Fix rx channel reset
  soc: fsl: cpm1: qmc: Extend the API to provide Rx status
  soc: fsl: cpm1: qmc: Remove inline function specifiers
  soc: fsl: cpm1: qmc: Add support for child devices
  soc: fsl: cpm1: qmc: Introduce available timeslots masks
  soc: fsl: cpm1: qmc: Rename qmc_setup_tsa* to qmc_init_tsa*
  soc: fsl: cpm1: qmc: Introduce qmc_chan_setup_tsa*
  soc: fsl: cpm1: qmc: Remove no more needed checks from
qmc_check_chans()
  soc: fsl: cpm1: qmc: Check available timeslots in qmc_check_chans()
  soc: fsl: cpm1: qmc: Add support for disabling channel TSA entries
  soc: fsl: cpm1: qmc: Split Tx and Rx TSA entries setup
  soc: fsl: cpm1: qmc: Introduce is_tsa_64rxtx flag
  soc: fsl: cpm1: qmc: Handle timeslot entries at channel start() and
stop()
  soc: fsl: cpm1: qmc: Remove timeslots handling from setup_chan()
  soc: fsl: cpm1: qmc: Introduce functions to change timeslots at
runtime

 drivers/soc/fsl/qe/qmc.c  | 592 +++---
 drivers/soc/fsl/qe/tsa.c  |  22 +-
 include/soc/fsl/qe/qmc.h  |  27 +-
 sound/soc/fsl/fsl_qmc_audio.c |   2 +-
 4 files changed, 506 insertions(+), 137 deletions(-)

-- 
2.42.0

[PATCH 05/17] soc: fsl: cpm1: qmc: Remove inline function specifiers

[Herve Codina]

The inline function specifier is present on some functions but it is
better to let the compiler decide inlining or not these functions.

Remove inline specifiers.

Fixes: 3178d58e0b97 ("soc: fsl: cpm1: Add support for QMC")
Signed-off-by: Herve Codina 
Suggested-by: Andrew Lunn 
Reviewed-by: Christophe Leroy 
---
 drivers/soc/fsl/qe/qmc.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/soc/fsl/qe/qmc.c b/drivers/soc/fsl/qe/qmc.c
index 4b4832d93c9b..27f2f16deac9 100644
--- a/drivers/soc/fsl/qe/qmc.c
+++ b/drivers/soc/fsl/qe/qmc.c
@@ -218,37 +218,37 @@ struct qmc {
struct qmc_chan *chans[64];
 };
 
-static inline void qmc_write16(void __iomem *addr, u16 val)
+static void qmc_write16(void __iomem *addr, u16 val)
 {
iowrite16be(val, addr);
 }
 
-static inline u16 qmc_read16(void __iomem *addr)
+static u16 qmc_read16(void __iomem *addr)
 {
return ioread16be(addr);
 }
 
-static inline void qmc_setbits16(void __iomem *addr, u16 set)
+static void qmc_setbits16(void __iomem *addr, u16 set)
 {
qmc_write16(addr, qmc_read16(addr) | set);
 }
 
-static inline void qmc_clrbits16(void __iomem *addr, u16 clr)
+static void qmc_clrbits16(void __iomem *addr, u16 clr)
 {
qmc_write16(addr, qmc_read16(addr) & ~clr);
 }
 
-static inline void qmc_write32(void __iomem *addr, u32 val)
+static void qmc_write32(void __iomem *addr, u32 val)
 {
iowrite32be(val, addr);
 }
 
-static inline u32 qmc_read32(void __iomem *addr)
+static u32 qmc_read32(void __iomem *addr)
 {
return ioread32be(addr);
 }
 
-static inline void qmc_setbits32(void __iomem *addr, u32 set)
+static void qmc_setbits32(void __iomem *addr, u32 set)
 {
qmc_write32(addr, qmc_read32(addr) | set);
 }
-- 
2.42.0

[PATCH 1/5] selftests/powerpc: Fix error handling in FPU/VMX preemption tests

[Michael Ellerman]

The FPU & VMX preemption tests do not check for errors returned by the
low-level asm routines, preempt_fpu() / preempt_vsx() respectively.
That means any register corruption detected by the asm routines does not
result in a test failure.

Fix it by returning the return value of the asm routines from the
pthread child routines.

Fixes: e5ab8be68e44 ("selftests/powerpc: Test preservation of FPU and VMX regs 
across preemption")
Signed-off-by: Michael Ellerman 
---
 tools/testing/selftests/powerpc/math/fpu_preempt.c |  9 +
 tools/testing/selftests/powerpc/math/vmx_preempt.c | 10 ++
 2 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/tools/testing/selftests/powerpc/math/fpu_preempt.c 
b/tools/testing/selftests/powerpc/math/fpu_preempt.c
index 5235bdc8c0b1..3e5b5663d244 100644
--- a/tools/testing/selftests/powerpc/math/fpu_preempt.c
+++ b/tools/testing/selftests/powerpc/math/fpu_preempt.c
@@ -37,19 +37,20 @@ __thread double darray[] = {0.1, 0.2, 0.3, 0.4, 0.5, 0.6, 
0.7, 0.8, 0.9, 1.0,
 int threads_starting;
 int running;
 
-extern void preempt_fpu(double *darray, int *threads_starting, int *running);
+extern int preempt_fpu(double *darray, int *threads_starting, int *running);
 
 void *preempt_fpu_c(void *p)
 {
+   long rc;
int i;
+
srand(pthread_self());
for (i = 0; i < 21; i++)
darray[i] = rand();
 
-   /* Test failed if it ever returns */
-   preempt_fpu(darray, &threads_starting, &running);
+   rc = preempt_fpu(darray, &threads_starting, &running);
 
-   return p;
+   return (void *)rc;
 }
 
 int test_preempt_fpu(void)
diff --git a/tools/testing/selftests/powerpc/math/vmx_preempt.c 
b/tools/testing/selftests/powerpc/math/vmx_preempt.c
index 6761d6ce30ec..6f7cf400c687 100644
--- a/tools/testing/selftests/powerpc/math/vmx_preempt.c
+++ b/tools/testing/selftests/powerpc/math/vmx_preempt.c
@@ -37,19 +37,21 @@ __thread vector int varray[] = {{1, 2, 3, 4}, {5, 6, 7, 8}, 
{9, 10,11,12},
 int threads_starting;
 int running;
 
-extern void preempt_vmx(vector int *varray, int *threads_starting, int 
*running);
+extern int preempt_vmx(vector int *varray, int *threads_starting, int 
*running);
 
 void *preempt_vmx_c(void *p)
 {
int i, j;
+   long rc;
+
srand(pthread_self());
for (i = 0; i < 12; i++)
for (j = 0; j < 4; j++)
varray[i][j] = rand();
 
-   /* Test fails if it ever returns */
-   preempt_vmx(varray, &threads_starting, &running);
-   return p;
+   rc = preempt_vmx(varray, &threads_starting, &running);
+
+   return (void *)rc;
 }
 
 int test_preempt_vmx(void)
-- 
2.41.0

[PATCH 5/5] selftests/powerpc: Check all FPRs in fpu_syscall test

[Michael Ellerman]

There is a selftest that checks if FPRs are corrupted across a fork, aka
clone. It was added as part of the series that optimised the clone path
to save the parent's FP state without "giving up" (turning off FP).

See commit 8792468da5e1 ("powerpc: Add the ability to save FPU without
giving it up").

The test encodes the assumption that FPRs 0-13 are volatile across the
syscall, by only checking the volatile FPRs are not changed by the fork.
There was also a comment in the fpu_preempt test alluding to that:

  The check_fpu function in asm only checks the non volatile registers
  as it is reused from the syscall test

It is true that the function call ABI treats f0-f13 as volatile,
however the syscall ABI has since been documented as *not* treating those
registers as volatile. See commit 7b8845a2a2ec ("powerpc/64: Document
the syscall ABI").

So change the test to check all FPRs are not corrupted by the syscall.
Note that this currently fails, because save_fpu() etc. do not restore
f0/vsr0.

Signed-off-by: Michael Ellerman 
---
 tools/testing/selftests/powerpc/math/fpu_asm.S | 7 ---
 tools/testing/selftests/powerpc/math/fpu_syscall.c | 8 +---
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/powerpc/math/fpu_asm.S 
b/tools/testing/selftests/powerpc/math/fpu_asm.S
index 051392ad3ce7..efe1e1be4695 100644
--- a/tools/testing/selftests/powerpc/math/fpu_asm.S
+++ b/tools/testing/selftests/powerpc/math/fpu_asm.S
@@ -109,8 +109,9 @@ FUNC_START(test_fpu)
std r3,STACK_FRAME_PARAM(0)(sp) # Address of darray
std r4,STACK_FRAME_PARAM(1)(sp) # Address of pid
 
-   bl load_fpu
-   nop
+   // Load FPRs with expected values
+   OP_REGS lfd, 8, 0, 31, r3
+
li  r0,__NR_fork
sc
 
@@ -119,7 +120,7 @@ FUNC_START(test_fpu)
std r3,0(r9)
 
ld r3,STACK_FRAME_PARAM(0)(sp)
-   bl check_fpu
+   bl check_all_fprs
nop
 
POP_FPU(256)
diff --git a/tools/testing/selftests/powerpc/math/fpu_syscall.c 
b/tools/testing/selftests/powerpc/math/fpu_syscall.c
index 694f225c7e45..751d46b133fc 100644
--- a/tools/testing/selftests/powerpc/math/fpu_syscall.c
+++ b/tools/testing/selftests/powerpc/math/fpu_syscall.c
@@ -14,12 +14,11 @@
 #include 
 
 #include "utils.h"
+#include "fpu.h"
 
 extern int test_fpu(double *darray, pid_t *pid);
 
-double darray[] = {0.1, 0.2, 0.3, 0.4, 0.5, 0.6, 0.7, 0.8, 0.9, 1.0,
-1.1, 1.2, 1.3, 1.4, 1.5, 1.6, 1.7, 1.8, 1.9, 2.0,
-2.1};
+double darray[32];
 
 int syscall_fpu(void)
 {
@@ -27,6 +26,9 @@ int syscall_fpu(void)
int i;
int ret;
int child_ret;
+
+   randomise_darray(darray, ARRAY_SIZE(darray));
+
for (i = 0; i < 1000; i++) {
/* test_fpu will fork() */
ret = test_fpu(darray, &fork_pid);
-- 
2.41.0

[PATCH 4/5] selftests/powerpc: Run fpu_preempt test for 60 seconds

[Michael Ellerman]

The FPU preempt test only runs for 20 seconds, which is not particularly
long. Run it for 60 seconds to increase the chance of detecting
corruption.

Signed-off-by: Michael Ellerman 
---
 tools/testing/selftests/powerpc/math/fpu_preempt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/powerpc/math/fpu_preempt.c 
b/tools/testing/selftests/powerpc/math/fpu_preempt.c
index 97dff3690136..9ddede0770ed 100644
--- a/tools/testing/selftests/powerpc/math/fpu_preempt.c
+++ b/tools/testing/selftests/powerpc/math/fpu_preempt.c
@@ -22,7 +22,7 @@
 #include "fpu.h"
 
 /* Time to wait for workers to get preempted (seconds) */
-#define PREEMPT_TIME 20
+#define PREEMPT_TIME 60
 /*
  * Factor by which to multiply number of online CPUs for total number of
  * worker threads
-- 
2.41.0

[PATCH 3/5] selftests/powerpc: Generate better bit patterns for FPU tests

[Michael Ellerman]

The fpu_preempt test randomly initialises an array of doubles to try and
detect FPU register corruption.

However the values it generates do not occupy the full range of values
possible in the 64-bit double, meaning some partial register corruption
could go undetected.

Without getting too carried away, add some better initialisation to
generate values that occupy more bits.

Sample values before:

  f0 902677510   (raw 0x41cae6e20300)
  f1 325217596   (raw 0x41b3626d3c00)
  f2 1856578300  (raw 0x41dbaa48bf00)
  f3 1247189984  (raw 0x41d295a6f800)

And after:

  f0 1.1078153481413311e-09  (raw 0x3e13083932805cc2)
  f1 1.0576648474801922e+17  (raw 0x43777c20eb88c261)
  f2 -6.6245033413594075e-10 (raw 0xbe06c2f989facae9)
  f3 3.0085988827156291e+18  (raw 0x43c4e0585f2df37b)

Signed-off-by: Michael Ellerman 
---
 tools/testing/selftests/powerpc/math/fpu.h| 25 +++
 .../selftests/powerpc/math/fpu_preempt.c  |  6 ++---
 2 files changed, 27 insertions(+), 4 deletions(-)
 create mode 100644 tools/testing/selftests/powerpc/math/fpu.h

diff --git a/tools/testing/selftests/powerpc/math/fpu.h 
b/tools/testing/selftests/powerpc/math/fpu.h
new file mode 100644
index ..a8ad0d42604e
--- /dev/null
+++ b/tools/testing/selftests/powerpc/math/fpu.h
@@ -0,0 +1,25 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright 2023, Michael Ellerman, IBM Corporation.
+ */
+
+#ifndef _SELFTESTS_POWERPC_FPU_H
+#define _SELFTESTS_POWERPC_FPU_H
+
+static inline void randomise_darray(double *darray, int num)
+{
+   long val;
+
+   for (int i = 0; i < num; i++) {
+   val = random();
+   if (val & 1)
+   val *= -1;
+
+   if (val & 2)
+   darray[i] = 1.0 / val;
+   else
+   darray[i] = val * val;
+   }
+}
+
+#endif /* _SELFTESTS_POWERPC_FPU_H */
diff --git a/tools/testing/selftests/powerpc/math/fpu_preempt.c 
b/tools/testing/selftests/powerpc/math/fpu_preempt.c
index 24b5abacccdc..97dff3690136 100644
--- a/tools/testing/selftests/powerpc/math/fpu_preempt.c
+++ b/tools/testing/selftests/powerpc/math/fpu_preempt.c
@@ -19,6 +19,7 @@
 #include 
 
 #include "utils.h"
+#include "fpu.h"
 
 /* Time to wait for workers to get preempted (seconds) */
 #define PREEMPT_TIME 20
@@ -39,12 +40,9 @@ extern int preempt_fpu(double *darray, int 
*threads_starting, int *running);
 void *preempt_fpu_c(void *p)
 {
long rc;
-   int i;
 
srand(pthread_self());
-   for (i = 0; i < ARRAY_SIZE(darray); i++)
-   darray[i] = rand();
-
+   randomise_darray(darray, ARRAY_SIZE(darray));
rc = preempt_fpu(darray, &threads_starting, &running);
 
return (void *)rc;
-- 
2.41.0

[PATCH 2/5] selftests/powerpc: Check all FPRs in fpu_preempt

[Michael Ellerman]

There's a selftest that checks FPRs aren't corrupted by preemption, or
just process scheduling. However it only checks the non-volatile FPRs,
meaning corruption of the volatile FPRs could go undetected.

The check_fpu function it calls is used by several other tests, so for
now add a new routine to check all the FPRs. Increase the size of the
array of FPRs to 32, and initialise them all with random values.

Signed-off-by: Michael Ellerman 
---
 .../testing/selftests/powerpc/math/fpu_asm.S  | 41 +--
 .../selftests/powerpc/math/fpu_preempt.c  | 15 +++
 2 files changed, 43 insertions(+), 13 deletions(-)

diff --git a/tools/testing/selftests/powerpc/math/fpu_asm.S 
b/tools/testing/selftests/powerpc/math/fpu_asm.S
index 9dc0c158f871..051392ad3ce7 100644
--- a/tools/testing/selftests/powerpc/math/fpu_asm.S
+++ b/tools/testing/selftests/powerpc/math/fpu_asm.S
@@ -66,6 +66,40 @@ FUNC_START(check_fpu)
li  r3,0 # Success!!!
 1: blr
 
+
+// int check_all_fprs(double darray[32])
+FUNC_START(check_all_fprs)
+   PUSH_BASIC_STACK(8)
+   mr  r4, r3  // r4 = darray
+   li  r3, 1   // prepare for failure
+
+   stfdf31, STACK_FRAME_LOCAL(0, 0)(sp) // backup f31
+
+   // Check regs f0-f30, using f31 as scratch
+   .set i, 0
+   .rept 31
+   lfd f31, (8 * i)(r4)// load expected value
+   fcmpu   cr0, i, f31 // compare
+   bne cr0, 1f // bail if mismatch
+   .set i, i + 1
+   .endr
+
+   lfd f31, STACK_FRAME_LOCAL(0, 0)(sp) // reload f31
+   stfdf30, STACK_FRAME_LOCAL(0, 0)(sp) // backup f30
+
+   lfd f30, (8 * 31)(r4)   // load expected value of f31
+   fcmpu   cr0, f30, f31   // compare
+   bne cr0, 1f // bail if mismatch
+
+   lfd f30, STACK_FRAME_LOCAL(0, 0)(sp) // reload f30
+
+   // Success
+   li  r3, 0
+
+1: POP_BASIC_STACK(8)
+   blr
+FUNC_END(check_all_fprs)
+
 FUNC_START(test_fpu)
# r3 holds pointer to where to put the result of fork
# r4 holds pointer to the pid
@@ -104,8 +138,8 @@ FUNC_START(preempt_fpu)
std r4,STACK_FRAME_PARAM(1)(sp) # int *threads_starting
std r5,STACK_FRAME_PARAM(2)(sp) # int *running
 
-   bl load_fpu
-   nop
+   // Load FPRs with expected values
+   OP_REGS lfd, 8, 0, 31, r3
 
sync
# Atomic DEC
@@ -116,8 +150,7 @@ FUNC_START(preempt_fpu)
bne- 1b
 
 2: ld r3,STACK_FRAME_PARAM(0)(sp)
-   bl check_fpu
-   nop
+   bl check_all_fprs
cmpdi r3,0
bne 3f
ld r4,STACK_FRAME_PARAM(2)(sp)
diff --git a/tools/testing/selftests/powerpc/math/fpu_preempt.c 
b/tools/testing/selftests/powerpc/math/fpu_preempt.c
index 3e5b5663d244..24b5abacccdc 100644
--- a/tools/testing/selftests/powerpc/math/fpu_preempt.c
+++ b/tools/testing/selftests/powerpc/math/fpu_preempt.c
@@ -1,13 +1,12 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 /*
  * Copyright 2015, Cyril Bur, IBM Corp.
+ * Copyright 2023, Michael Ellerman, IBM Corp.
  *
  * This test attempts to see if the FPU registers change across preemption.
- * Two things should be noted here a) The check_fpu function in asm only checks
- * the non volatile registers as it is reused from the syscall test b) There is
- * no way to be sure preemption happened so this test just uses many threads
- * and a long wait. As such, a successful test doesn't mean much but a failure
- * is bad.
+ * There is no way to be sure preemption happened so this test just uses many
+ * threads and a long wait. As such, a successful test doesn't mean much but
+ * a failure is bad.
  */
 
 #include 
@@ -30,9 +29,7 @@
 #define THREAD_FACTOR 8
 
 
-__thread double darray[] = {0.1, 0.2, 0.3, 0.4, 0.5, 0.6, 0.7, 0.8, 0.9, 1.0,
-1.1, 1.2, 1.3, 1.4, 1.5, 1.6, 1.7, 1.8, 1.9, 2.0,
-2.1};
+__thread double darray[32];
 
 int threads_starting;
 int running;
@@ -45,7 +42,7 @@ void *preempt_fpu_c(void *p)
int i;
 
srand(pthread_self());
-   for (i = 0; i < 21; i++)
+   for (i = 0; i < ARRAY_SIZE(darray); i++)
darray[i] = rand();
 
rc = preempt_fpu(darray, &threads_starting, &running);
-- 
2.41.0

[PATCH] soc: fsl: cpm1: qmc: fix error return code in qmc_probe()

[Yang Yingliang]

If platform_get_irq() fails, it need return error code in
qmc_probe().

Fixes: 3178d58e0b97 ("soc: fsl: cpm1: Add support for QMC")
Signed-off-by: Yang Yingliang 
---
 drivers/soc/fsl/qe/qmc.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/soc/fsl/qe/qmc.c b/drivers/soc/fsl/qe/qmc.c
index 92ec76c03965..65109f279a6b 100644
--- a/drivers/soc/fsl/qe/qmc.c
+++ b/drivers/soc/fsl/qe/qmc.c
@@ -1386,8 +1386,10 @@ static int qmc_probe(struct platform_device *pdev)
qmc_write16(qmc->scc_regs + SCC_SCCM, 0x);
qmc_write16(qmc->scc_regs + SCC_SCCE, 0x000F);
irq = platform_get_irq(pdev, 0);
-   if (irq < 0)
+   if (irq < 0) {
+   ret = irq;
goto err_tsa_serial_disconnect;
+   }
ret = devm_request_irq(qmc->dev, irq, qmc_irq_handler, 0, "qmc", qmc);
if (ret < 0)
goto err_tsa_serial_disconnect;
-- 
2.25.1

Re: [PATCH] soc: fsl: cpm1: qmc: fix error return code in qmc_probe()

[Herve Codina]

Hi Yang,

On Tue, 28 Nov 2023 21:09:42 +0800
Yang Yingliang  wrote:

> If platform_get_irq() fails, it need return error code in
> qmc_probe().
> 
> Fixes: 3178d58e0b97 ("soc: fsl: cpm1: Add support for QMC")
> Signed-off-by: Yang Yingliang 
> ---
>  drivers/soc/fsl/qe/qmc.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/soc/fsl/qe/qmc.c b/drivers/soc/fsl/qe/qmc.c
> index 92ec76c03965..65109f279a6b 100644
> --- a/drivers/soc/fsl/qe/qmc.c
> +++ b/drivers/soc/fsl/qe/qmc.c
> @@ -1386,8 +1386,10 @@ static int qmc_probe(struct platform_device *pdev)
>   qmc_write16(qmc->scc_regs + SCC_SCCM, 0x);
>   qmc_write16(qmc->scc_regs + SCC_SCCE, 0x000F);
>   irq = platform_get_irq(pdev, 0);
> - if (irq < 0)
> + if (irq < 0) {
> + ret = irq;
>   goto err_tsa_serial_disconnect;
> + }
>   ret = devm_request_irq(qmc->dev, irq, qmc_irq_handler, 0, "qmc", qmc);
>   if (ret < 0)
>   goto err_tsa_serial_disconnect;

Thanks for this patch.

Acked-by: Herve Codina 

Best regards,
Hervé

Re: [PATCH v2] powerpc: Don't clobber fr0/vs0 during fp|altivec register save

[Michael Ellerman]

Michael Ellerman  writes:
> Timothy Pearson  writes:
>
>> Just wanted to check back and see if this patch was going to be queued
>> up soon?  We're still having to work around / advertise the data
>> destruction issues the underlying bug is causing on e.g. Debian
>> Stable.
>
> Yeah I'll apply it this week, so it will be in rc4.

I reworked the change log to include the exact call path I identified
instead of the more high level description you had. And tweaked a few
other bits of wording and so on, apparently fr0 is a kernelism, the ABI
and binutils calls it f0.

I'm not sure how wedded you were to your change log, so if you dislike
my edits let me know and we can come up with a joint one.

The actual patch is unchanged.

cheers


>From 5e1d824f9a283cbf90f25241b66d1f69adb3835b Mon Sep 17 00:00:00 2001
From: Timothy Pearson 
Date: Sun, 19 Nov 2023 09:18:02 -0600
Subject: [PATCH] powerpc: Don't clobber f0/vs0 during fp|altivec register save

During floating point and vector save to thread data f0/vs0 are
clobbered by the FPSCR/VSCR store routine. This has been obvserved to
lead to userspace register corruption and application data corruption
with io-uring.

Fix it by restoring f0/vs0 after FPSCR/VSCR store has completed for
all the FP, altivec, VMX register save paths.

Tested under QEMU in kvm mode, running on a Talos II workstation with
dual POWER9 DD2.2 CPUs.

Additional detail (mpe):

Typically save_fpu() is called from __giveup_fpu() which saves the FP
regs and also *turns off FP* in the tasks MSR, meaning the kernel will
reload the FP regs from the thread struct before letting the task use FP
again. So in that case save_fpu() is free to clobber f0 because the FP
regs no longer hold live values for the task.

There is another case though, which is the path via:
  sys_clone()
...
copy_process()
  dup_task_struct()
arch_dup_task_struct()
  flush_all_to_thread()
save_all()

That path saves the FP regs but leaves them live. That's meant as an
optimisation for a process that's using FP/VSX and then calls fork(),
leaving the regs live means the parent process doesn't have to take a
fault after the fork to get its FP regs back. The optimisation was added
in commit 8792468da5e1 ("powerpc: Add the ability to save FPU without
giving it up").

That path does clobber f0, but f0 is volatile across function calls,
and typically programs reach copy_process() from userspace via a syscall
wrapper function. So in normal usage f0 being clobbered across a
syscall doesn't cause visible data corruption.

But there is now a new path, because io-uring can call copy_process()
via create_io_thread() from the signal handling path. That's OK if the
signal is handled as part of syscall return, but it's not OK if the
signal is handled due to some other interrupt.

That path is:

interrupt_return_srr_user()
  interrupt_exit_user_prepare()
interrupt_exit_user_prepare_main()
  do_notify_resume()
get_signal()
  task_work_run()
create_worker_cb()
  create_io_worker()
copy_process()
  dup_task_struct()
arch_dup_task_struct()
  flush_all_to_thread()
save_all()
  if (tsk->thread.regs->msr & MSR_FP)
save_fpu()
# f0 is clobbered and potentially live in userspace

Note the above discussion applies equally to save_altivec().

Fixes: 8792468da5e1 ("powerpc: Add the ability to save FPU without giving it 
up")
Cc: sta...@vger.kernel.org # v4.6+
Closes: 
https://lore.kernel.org/all/480932026.45576726.1699374859845.javamail.zim...@raptorengineeringinc.com/
Closes: 
https://lore.kernel.org/linuxppc-dev/480221078.47953493.1700206777956.javamail.zim...@raptorengineeringinc.com/
Tested-by: Timothy Pearson 
Tested-by: Jens Axboe 
Signed-off-by: Timothy Pearson 
[mpe: Reword change log to describe exact path of corruption & other minor 
tweaks]
Signed-off-by: Michael Ellerman 
Link: 
https://msgid.link/1921539696.48534988.1700407082933.javamail.zim...@raptorengineeringinc.com
---
 arch/powerpc/kernel/fpu.S| 13 +
 arch/powerpc/kernel/vector.S |  2 ++
 2 files changed, 15 insertions(+)

diff --git a/arch/powerpc/kernel/fpu.S b/arch/powerpc/kernel/fpu.S
index 6a9acfb690c9..2f8f3f93cbb6 100644
--- a/arch/powerpc/kernel/fpu.S
+++ b/arch/powerpc/kernel/fpu.S
@@ -23,6 +23,15 @@
 #include 
 
 #ifdef CONFIG_VSX
+#define __REST_1FPVSR(n,c,base)
\
+BEGIN_FTR_SECTION  \
+   b   2f; \
+END_FTR_SECTION_IFSET(CPU_FTR_VSX);\
+   REST_FPR(n,base);   \
+   b   3f; \
+2: REST_VSR(n,c,base);  

Re: [PATCH] powerpc/mm: Fix null-pointer dereference in pgtable_cache_add

[Michael Ellerman]

Kunwu Chan  writes:
> Hi Christophe,
>
> Thanks for your reply.
> It's my bad. According your reply, i read the code in 
> sysfs_do_create_link_sd.There is a null pointer check indeed.
>
> My intention was to check null pointer after memory allocation.
> Whether we can add a comment here for someone like me, the null pointer 
> check is no need here?

I don't mind there being a NULL check for name.

But the code shouldn't silently return if name can't be allocated.
Notice that if we can't create the cache we *panic*. A failure to
allocate name, which causes us to skip the cache creation, needs to also
panic.

cheers

> On 2023/11/24 23:17, Christophe Leroy wrote:
>> 
>> 
>> Le 22/11/2023 à 10:00, Kunwu Chan a écrit :
>>> [Vous ne recevez pas souvent de courriers de chen...@kylinos.cn. Découvrez 
>>> pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification 
>>> ]
>>>
>>> kasprintf() returns a pointer to dynamically allocated memory
>>> which can be NULL upon failure. Ensure the allocation was successful
>>> by checking the pointer validity.
>> 
>> Are you sure this is needed ? Did you check what happens what name is NULL ?
>> 
>> If I followed stuff correctly, I end up in function
>> sysfs_do_create_link_sd() which already handles the NULL name case which
>> a big hammer warning.
>> 
>>>
>>> Signed-off-by: Kunwu Chan 
>>> ---
>>>arch/powerpc/mm/init-common.c | 2 ++
>>>1 file changed, 2 insertions(+)
>>>
>>> diff --git a/arch/powerpc/mm/init-common.c b/arch/powerpc/mm/init-common.c
>>> index 119ef491f797..0884fc601c46 100644
>>> --- a/arch/powerpc/mm/init-common.c
>>> +++ b/arch/powerpc/mm/init-common.c
>>> @@ -139,6 +139,8 @@ void pgtable_cache_add(unsigned int shift)
>>>
>>>   align = max_t(unsigned long, align, minalign);
>>>   name = kasprintf(GFP_KERNEL, "pgtable-2^%d", shift);
>>> +   if (!name)
>>> +   return;
>>>   new = kmem_cache_create(name, table_size, align, 0, ctor(shift));
>>>   if (!new)
>>>   panic("Could not allocate pgtable cache for order %d", 
>>> shift);
>>> --
>>> 2.34.1
>>>

Re: [PATCH] perf test record+probe_libc_inet_pton: Fix call chain match on powerpc

[Disha Goel]

On 26/11/23 12:39 pm, Likhitha Korrapati wrote:


The perf test "probe libc's inet_pton & backtrace it with ping" fails on
powerpc as below:

root@xxx perf]# perf test -v "probe libc's inet_pton & backtrace it with
ping"
  85: probe libc's inet_pton & backtrace it with ping :
--- start ---
test child forked, pid 96028
ping 96056 [002] 127271.101961: probe_libc:inet_pton: (7fffa1779a60)
7fffa1779a60 __GI___inet_pton+0x0
(/usr/lib64/glibc-hwcaps/power10/libc.so.6)
7fffa172a73c getaddrinfo+0x121c
(/usr/lib64/glibc-hwcaps/power10/libc.so.6)
FAIL: expected backtrace entry
"gaih_inet.*\+0x[[:xdigit:]]+[[:space:]]\(/usr/lib64/glibc-hwcaps/power10/libc.so.6\)$"
got "7fffa172a73c getaddrinfo+0x121c
(/usr/lib64/glibc-hwcaps/power10/libc.so.6)"
test child finished with -1
 end 
probe libc's inet_pton & backtrace it with ping: FAILED!

This test installs a probe on libc's inet_pton function, which will use
uprobes and then uses perf trace on a ping to localhost. It gets 3
levels deep backtrace and checks whether it is what we expected or not.

The test started failing from RHEL 9.4 where as it works in previous
distro version (RHEL 9.2). Test expects gaih_inet function to be part of
backtrace. But in the glibc version (2.34-86) which is part of distro
where it fails, this function is missing and hence the test is failing.

 From nm and ping command output we can confirm that gaih_inet function
is not present in the expected backtrace for glibc version glibc-2.34-86

[root@xxx perf]# nm /usr/lib64/glibc-hwcaps/power10/libc.so.6 | grep gaih_inet
001273e0 t gaih_inet_serv
001cd8d8 r gaih_inet_typeproto

[root@xxx perf]# perf script -i /tmp/perf.data.6E8
ping  104048 [000] 128582.508976: probe_libc:inet_pton: (7fff83779a60)
 7fff83779a60 __GI___inet_pton+0x0
(/usr/lib64/glibc-hwcaps/power10/libc.so.6)
 7fff8372a73c getaddrinfo+0x121c
(/usr/lib64/glibc-hwcaps/power10/libc.so.6)
11dc73534 [unknown] (/usr/bin/ping)
 7fff8362a8c4 __libc_start_call_main+0x84
(/usr/lib64/glibc-hwcaps/power10/libc.so.6)

FAIL: expected backtrace entry
"gaih_inet.*\+0x[[:xdigit:]]+[[:space:]]\(/usr/lib64/glibc-hwcaps/power10/libc.so.6\)$"
got "7fff9d52a73c getaddrinfo+0x121c
(/usr/lib64/glibc-hwcaps/power10/libc.so.6)"

With version glibc-2.34-60 gaih_inet function is present as part of the
expected backtrace. So we cannot just remove the gaih_inet function from
the backtrace.

[root@xxx perf]# nm /usr/lib64/glibc-hwcaps/power10/libc.so.6 | grep gaih_inet
00130490 t gaih_inet.constprop.0
0012e830 t gaih_inet_serv
001d45e4 r gaih_inet_typeproto

[root@xxx perf]# ./perf script -i /tmp/perf.data.b6S
ping   67906 [000] 22699.591699: probe_libc:inet_pton_3: (7fffbdd80820)
 7fffbdd80820 __GI___inet_pton+0x0
(/usr/lib64/glibc-hwcaps/power10/libc.so.6)
 7fffbdd31160 gaih_inet.constprop.0+0xcd0
(/usr/lib64/glibc-hwcaps/power10/libc.so.6)
 7fffbdd31c7c getaddrinfo+0x14c
(/usr/lib64/glibc-hwcaps/power10/libc.so.6)
1140d3558 [unknown] (/usr/bin/ping)

This patch solves this issue by doing a conditional skip. If there is a
gaih_inet function present in the libc then it will be added to the
expected backtrace else the function will be skipped from being added
to the expected backtrace.

Output with the patch

[root@xxx perf]# ./perf test -v "probe libc's inet_pton & backtrace it
with ping"
  83: probe libc's inet_pton & backtrace it with ping :
--- start ---
test child forked, pid 102662
ping 102692 [000] 127935.549973: probe_libc:inet_pton: (7fff93379a60)
7fff93379a60 __GI___inet_pton+0x0
(/usr/lib64/glibc-hwcaps/power10/libc.so.6)
7fff9332a73c getaddrinfo+0x121c
(/usr/lib64/glibc-hwcaps/power10/libc.so.6)
11ef03534 [unknown] (/usr/bin/ping)
test child finished with 0
 end 
probe libc's inet_pton & backtrace it with ping: Ok

Signed-off-by: Likhitha Korrapati 
Reported-by: Disha Goel 


Thanks for the fix patch.
I have tested on a Power10 machine, "probe libc's inet_pton & backtrace it with 
ping"
perf test passes with the patch applied.

Output where gaih_inet function is not present

# perf test -v "probe libc's inet_pton & backtrace it with ping"
 85: probe libc's inet_pton & backtrace it with ping :
--- start ---
test child forked, pid 4622
ping 4652 [011] 58.987631: probe_libc:inet_pton: (7fff91b79a60)
7fff91b79a60 __GI___inet_pton+0x0 
(/usr/lib64/glibc-hwcaps/power10/libc.so.6)
7fff91b2a73c getaddrinfo+0x121c 
(/usr/lib64/glibc-hwcaps/power10/libc.so.6)
119e53534 [unknown] (/usr/bin/ping)
test child finished with 0
 end 
probe libc's inet_pton & backtrace it with ping: Ok

Output where gaih_inet function is present

# ./perf test -v "probe libc's inet_pton & backtrace it with ping"
 83: probe libc's inet_pton & backtrace it with ping

Re: [PATCH v2 0/5] MODVERSIONS + RUST Redux

[Greg KH]

On Tue, Nov 28, 2023 at 08:05:26AM +, Greg KH wrote:
> On Mon, Nov 27, 2023 at 11:27:07AM -0800, Matthew Maurer wrote:
> > > > >
> > > > > > With regards to future directions that likely won't work for 
> > > > > > loosening it:
> > > > > > Unfortunately, the .rmeta format itself is not stable, so I 
> > > > > > wouldn't want to
> > > > > > teach genksyms to open it up and split out the pieces for specific 
> > > > > > functions.
> > > > > > Extending genksyms to parse Rust would also not solve the situation 
> > > > > > -
> > > > > > layouts are allowed to differ across compiler versions or even (in 
> > > > > > rare
> > > > > > cases) seemingly unrelated code changes.
> > > > >
> > > > > What do you mean by "layout" here?  Yes, the crcs can be different
> > > > > across compiler versions and seemingly unrelated code changes 
> > > > > (genksyms
> > > > > is VERY fragile) but that's ok, that's not what you are checking here.
> > > > > You want to know if the rust function signature changes or not from 
> > > > > the
> > > > > last time you built the code, with the same compiler and options, 
> > > > > that's
> > > > > all you are verifying.
> > What I mean by layout here is that if you write in Rust:
> > struct Foo {
> >   x: i32,
> >   y: i32,
> > }
> > it is not guaranteed to have the same layout across different compilations, 
> > even
> > within the same compiler. See
> > https://doc.rust-lang.org/reference/type-layout.html#the-rust-representation
> 
> Then you are going to have big problems, sorry.
> 
> > Specifically, the compiler is allowed to arbitrarily insert padding,
> > reorder fields, etc.
> > on the same code as long as the overall alignment of the struct and 
> > individual
> > alignment of the fields remains correct and non-overlapping.
> > 
> > This means the compiler is *explicitly* allowed to, for example, permute x 
> > and y
> > as an optimization. In the above example this is unlikely, but if you
> > instead consider
> > struct Bar {
> >   x: i8,
> >   y: i64,
> >   z: i8,
> > }
> > It's easy to see why the compiler might decide to structure this as
> > y,x,z to reduce the
> > size of the struct. Those optimization decisions may be affected by
> > any other part of
> > the code, PGO, etc.
> 
> Then you all need to figure out some way to determine how the compiler
> layed out the structure after it compiled/optimized it and be able to
> compare it to previous builds (or just generate a crc based on the
> layout it chose.)
> 
> > > > > > Future directions that might work for loosening it:
> > > > > > * Generating crcs from debuginfo + compiler + flags
> > > > > > * Adding a feature to the rust compiler to dump this information. 
> > > > > > This
> > > > > > is likely to
> > > > > >   get pushback because Rust's current stance is that there is no 
> > > > > > ability to load
> > > > > >   object code built against a different library.
> > > > >
> > > > > Why not parse the function signature like we do for C?
> > Because the function signature is insufficient to check the ABI, see above.
> > > > >
> > > > > > Would setting up Rust symbols so that they have a crc built out of 
> > > > > > .rmeta be
> > > > > > sufficient for you to consider this useful? If not, can you help me 
> > > > > > understand
> > > > > > what level of precision would be required?
> > > > >
> > > > > What exactly does .rmeta have to do with the function signature?  
> > > > > That's
> > > > > all you care about here.
> > The .rmeta file contains the decisions the compiler made about layout
> > in the crate
> > you're interfacing with. For example, the choice to encode Bar
> > with a yxz field order would be written into the .rmeta file.
> 
> Ok, then yes, can you parse the .rmeta file to get that information?
> 
> > > > rmeta is generated per crate.
> > > >
> > > > CRC is computed per symbol.
> > > >
> > > > They have different granularity.
> > > > It is weird to refuse a module for incompatibility
> > > > of a symbol that it is not using at all.
> > >
> > > I agree, this should be on a per-symbol basis, so the Rust
> > > infrastructure in the kernel needs to be fixed up to support this
> > > properly, not just ignored like this patchset does.
> > I agree there is a divergence here, I tried to point it out so that it
> > wouldn't be
> > a surprise later. The .rmeta file itself (which is the only way we
> > could know that
> > the ABI actually matches, because layout decisions are in there) is an 
> > unstable
> > format, which is why I would be reluctant to try to parse it and find only 
> > the
> > relevant portions to hash. This isn't just a "technically unstable"
> > format, but one
> > in which the compiler essentially just serializes out relevant internal data
> > structures, so any parser for it will involve significant alterations
> > on compiler
> > updates, which doesn't seem like a good plan.
> > >
> > > thanks,
> > >
> > > greg k-h
> > Given the above additional information, would you be interested i

Re: [PATCH v2 0/5] MODVERSIONS + RUST Redux

[Greg KH]

On Mon, Nov 27, 2023 at 11:27:07AM -0800, Matthew Maurer wrote:
> > > >
> > > > > With regards to future directions that likely won't work for 
> > > > > loosening it:
> > > > > Unfortunately, the .rmeta format itself is not stable, so I wouldn't 
> > > > > want to
> > > > > teach genksyms to open it up and split out the pieces for specific 
> > > > > functions.
> > > > > Extending genksyms to parse Rust would also not solve the situation -
> > > > > layouts are allowed to differ across compiler versions or even (in 
> > > > > rare
> > > > > cases) seemingly unrelated code changes.
> > > >
> > > > What do you mean by "layout" here?  Yes, the crcs can be different
> > > > across compiler versions and seemingly unrelated code changes (genksyms
> > > > is VERY fragile) but that's ok, that's not what you are checking here.
> > > > You want to know if the rust function signature changes or not from the
> > > > last time you built the code, with the same compiler and options, that's
> > > > all you are verifying.
> What I mean by layout here is that if you write in Rust:
> struct Foo {
>   x: i32,
>   y: i32,
> }
> it is not guaranteed to have the same layout across different compilations, 
> even
> within the same compiler. See
> https://doc.rust-lang.org/reference/type-layout.html#the-rust-representation

Then you are going to have big problems, sorry.

> Specifically, the compiler is allowed to arbitrarily insert padding,
> reorder fields, etc.
> on the same code as long as the overall alignment of the struct and individual
> alignment of the fields remains correct and non-overlapping.
> 
> This means the compiler is *explicitly* allowed to, for example, permute x 
> and y
> as an optimization. In the above example this is unlikely, but if you
> instead consider
> struct Bar {
>   x: i8,
>   y: i64,
>   z: i8,
> }
> It's easy to see why the compiler might decide to structure this as
> y,x,z to reduce the
> size of the struct. Those optimization decisions may be affected by
> any other part of
> the code, PGO, etc.

Then you all need to figure out some way to determine how the compiler
layed out the structure after it compiled/optimized it and be able to
compare it to previous builds (or just generate a crc based on the
layout it chose.)

> > > > > Future directions that might work for loosening it:
> > > > > * Generating crcs from debuginfo + compiler + flags
> > > > > * Adding a feature to the rust compiler to dump this information. This
> > > > > is likely to
> > > > >   get pushback because Rust's current stance is that there is no 
> > > > > ability to load
> > > > >   object code built against a different library.
> > > >
> > > > Why not parse the function signature like we do for C?
> Because the function signature is insufficient to check the ABI, see above.
> > > >
> > > > > Would setting up Rust symbols so that they have a crc built out of 
> > > > > .rmeta be
> > > > > sufficient for you to consider this useful? If not, can you help me 
> > > > > understand
> > > > > what level of precision would be required?
> > > >
> > > > What exactly does .rmeta have to do with the function signature?  That's
> > > > all you care about here.
> The .rmeta file contains the decisions the compiler made about layout
> in the crate
> you're interfacing with. For example, the choice to encode Bar
> with a yxz field order would be written into the .rmeta file.

Ok, then yes, can you parse the .rmeta file to get that information?

> > > rmeta is generated per crate.
> > >
> > > CRC is computed per symbol.
> > >
> > > They have different granularity.
> > > It is weird to refuse a module for incompatibility
> > > of a symbol that it is not using at all.
> >
> > I agree, this should be on a per-symbol basis, so the Rust
> > infrastructure in the kernel needs to be fixed up to support this
> > properly, not just ignored like this patchset does.
> I agree there is a divergence here, I tried to point it out so that it
> wouldn't be
> a surprise later. The .rmeta file itself (which is the only way we
> could know that
> the ABI actually matches, because layout decisions are in there) is an 
> unstable
> format, which is why I would be reluctant to try to parse it and find only the
> relevant portions to hash. This isn't just a "technically unstable"
> format, but one
> in which the compiler essentially just serializes out relevant internal data
> structures, so any parser for it will involve significant alterations
> on compiler
> updates, which doesn't seem like a good plan.
> >
> > thanks,
> >
> > greg k-h
> Given the above additional information, would you be interested in a patchset
> which either:
> 
> A. Computes the CRC off the Rust type signature, knowing the compiler is
> allowed to change the ABI based on information not contained in the CRC.

No.

> B. Uses the CRC of the .rmeta file, knowing, as was pointed out, that this
> effectively contains the ABI of every symbol in the compilation unit, as well
> as inline fu