| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Mon, 9 May 2022 19:29:31 +0900
From: Chanwoo Choi <cwchoi00@...il.com>
To: Chen-Yu Tsai <wenst@...omium.org>
Cc: linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org,
cw00.choi@...sung.com, myungjoo.ham@...sung.com,
kyungmin.park@...sung.com, johnson.wang@...iatek.com,
mka@...omium.org, jia-wei.chang@...iatek.com,
andrew-sh.cheng@...iatek.com, hsinyi@...omium.org,
sibis@...eaurora.org, saravanak@...gle.com
Subject: Re: [PATCH v2 5/5] PM / devfreq: passive: Update frequency when start
governor
Hi,
On 22. 5. 9. 13:25, Chen-Yu Tsai wrote:
> Hi,
>
> On Sun, May 08, 2022 at 12:01:45AM +0900, Chanwoo Choi wrote:
>> If the parent device changes the their frequency before registering
>> the passive device, the passive device cannot receive the notification
>> from parent device and then the passive device cannot be able to
>> set the proper frequency according to the frequency of parent device.
>>
>> So, when start the passive governor, update the frequency
>> according to the frequency of parent device.
>>
>> Signed-off-by: Chanwoo Choi <cw00.choi@...sung.com>
>> Link: https://lore.kernel.org/r/20220507150145.531864-6-cw00.choi@samsung.com
>> ---
>> drivers/devfreq/governor_passive.c | 17 +++++++++++++++++
>> 1 file changed, 17 insertions(+)
>>
>> diff --git a/drivers/devfreq/governor_passive.c b/drivers/devfreq/governor_passive.c
>> index b34dbe750c0a..74d26c193fdb 100644
>> --- a/drivers/devfreq/governor_passive.c
>> +++ b/drivers/devfreq/governor_passive.c
>> @@ -412,6 +412,23 @@ static int devfreq_passive_event_handler(struct devfreq *devfreq,
>> if (!p_data->this)
>> p_data->this = devfreq;
>>
>> + /*
>> + * If the parent device changes the their frequency before
>> + * registering the passive device, the passive device cannot
>> + * receive the notification from parent device and then the
>> + * passive device cannot be able to set the proper frequency
>> + * according to the frequency of parent device.
>> + *
>> + * When start the passive governor, update the frequency
>> + * according to the frequency of parent device.
>> + */
>> + mutex_lock(&devfreq->lock);
>> + ret = devfreq_update_target(devfreq, parent->previous_freq);
>
> This crashes when parent is NULL, in the case where parent is cpufreq.
> This is the case with the MTK ccifreq driver, which produces the panic
> and backtrace below [1].
>
> I made a fix for a previous version of this patch:
>
> https://github.com/wens/linux/commit/f85c1834dd07388abb57a00200c80f7440823a03
>
> BTW, could you CC me on future revisions? I'm not subscribed to the
> linux-pm mailing list.
OK. I'll.
>
>
> Regards
> ChenYu
>
> [1]
>
> Unable to handle kernel read from unreadable memory at virtual address 0000000000000420
> Mem abort info:
> ESR = 0x0000000096000005
> EC = 0x25: DABT (current EL), IL = 32 bits
> SET = 0, FnV = 0
> EA = 0, S1PTW = 0
> FSC = 0x05: level 1 translation fault
> Data abort info:
> ISV = 0, ISS = 0x00000005
> CM = 0, WnR = 0
> [0000000000000420] user address but active_mm is swapper
> Internal error: Oops: 96000005 [#1] PREEMPT SMP
> Modules linked in:
> CPU: 7 PID: 1 Comm: swapper/0 Not tainted 5.18.0-rc5-next-20220505-09393-g38dc825c1d73 #155 b348fdb8d61a403eef7a9c5857bc02a261fcb213
> Hardware name: Google juniper sku16 board (DT)
> pstate: 60400005 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> pc : devfreq_passive_event_handler (drivers/devfreq/governor_passive.c:426)
> lr : devfreq_passive_event_handler (drivers/devfreq/governor_passive.c:426)
> sp : ffffffc00808ba80
> x29: ffffffc00808ba80 x28: 0000000000000000 x27: ffffffe99bb90458
> x26: 0000000000000010 x25: ffffff80c1843848 x24: ffffff80c1843810
> x23: ffffffe99babf3f5 x22: ffffffe99c278190 x21: ffffff80c0924d80
> x20: ffffff80c1843800 x19: 0000000000000000 x18: 0000000000000000
> x17: 0000000065516d0e x16: 00000000fc90660b x15: 0000000000000018
> x14: 0000000000000000 x13: ffffffffff000000 x12: 0000000000000038
> x11: 0101010101010101 x10: 8000000000000000 x9 : ffffffe99acb8458
> x8 : 0065766973000000 x7 : 0000000000000080 x6 : 0000000000000000
> x5 : 8000000000000000 x4 : 0000000000000000 x3 : ffffff80c1843810
> x2 : ffffff80c0228000 x1 : 0000000000000000 x0 : 0000000000000000
> Call trace:
> devfreq_passive_event_handler (drivers/devfreq/governor_passive.c:426)
> devfreq_add_device (drivers/devfreq/devfreq.c:932)
> devm_devfreq_add_device (drivers/devfreq/devfreq.c:1028)
> mtk_ccifreq_probe (drivers/devfreq/mtk-cci-devfreq.c:366)
> platform_probe (drivers/base/platform.c:1398)
> really_probe (drivers/base/dd.c:542 drivers/base/dd.c:621 drivers/base/dd.c:566)
> __driver_probe_device (drivers/base/dd.c:752)
> driver_probe_device (drivers/base/dd.c:782)
> __driver_attach (drivers/base/dd.c:1143 drivers/base/dd.c:1094)
> bus_for_each_dev (drivers/base/bus.c:301)
> driver_attach (drivers/base/dd.c:1160)
> bus_add_driver (drivers/base/bus.c:619)
> driver_register (drivers/base/driver.c:240)
> __platform_driver_register (drivers/base/platform.c:866)
> mtk_ccifreq_platdrv_init (drivers/devfreq/mtk-cci-devfreq.c:468)
> do_one_initcall (init/main.c:1301)
> kernel_init_freeable (init/main.c:1375 init/main.c:1392 init/main.c:1411 init/main.c:1618)
> kernel_init (init/main.c:1511)
> ret_from_fork (arch/arm64/kernel/entry.S:868)
> Code: f9000eb4 91004298 aa1803e0 940979d4 (f9421261)
> All code
> ========
> 0: f9000eb4 str x20, [x21, #24]
> 4: 91004298 add x24, x20, #0x10
> 8: aa1803e0 mov x0, x24
> c: 940979d4 bl 0x25e75c
> 10:* f9421261 ldr x1, [x19, #1056] <-- trapping instruction
>
> Code starting with the faulting instruction
> ===========================================
> 0: f9421261 ldr x1, [x19, #1056]
> ---[ end trace 0000000000000000 ]---
> Kernel panic - not syncing: Oops: Fatal exception
> SMP: stopping secondary CPUs
> Kernel Offset: 0x2992c00000 from 0xffffffc008000000
> PHYS_OFFSET: 0x40000000
> CPU features: 0x000,00324811,00001086
> Memory Limit: none
> PANIC in EL3.
>
>> + if (ret < 0)
>> + dev_warn(&devfreq->dev,
>> + "failed to update devfreq using passive governor\n");
>> + mutex_unlock(&devfreq->lock);
>> +
>> if (p_data->parent_type == DEVFREQ_PARENT_DEV)
>> ret = devfreq_passive_register_notifier(devfreq);
>> else if (p_data->parent_type == CPUFREQ_PARENT_DEV)
>>
>> --
>> 2.25.1
>>
Thanks for the testing. I'll drop the last patch
and then send next version.
--
Best Regards,
Samsung Electronics
Chanwoo Choi
Powered by blists - more mailing lists