| 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: Sat, 24 Jul 2021 21:36:11 +0800
From: Zhou Yanjie <zhouyanjie@...yeetech.com>
To: Paul Cercueil <paul@...pouillou.net>
Cc: rjw@...ysocki.net, daniel.lezcano@...aro.org,
linux-pm@...r.kernel.org, linux-mips@...r.kernel.org,
linux-kernel@...r.kernel.org, dongsheng.qiu@...enic.com,
aric.pzqi@...enic.com, rick.tyliu@...enic.com,
sihui.liu@...enic.com, jun.jiang@...enic.com,
sernia.zhou@...mail.com, Alex Smith <alex.smith@...tec.com>
Subject: Re: [PATCH] cpuidle: JZ4780: Add Ingenic JZ4780 cpuidle driver.
Hi Paul,
On 2021/7/24 下午7:16, Paul Cercueil wrote:
> Hi Zhou,
>
> Le sam., juil. 24 2021 at 17:19:59 +0800, 周琰杰 (Zhou Yanjie)
> <zhouyanjie@...yeetech.com> a écrit :
>> The JZ4780 has a high overhead to executing a MIPS wait on SMP, as a
>> core must flush out dirty cache lines from its data cache before doing
>> so. This is because the core clock is gated during a wait and if the
>> other core tries to access a dirty line from the waiting core's cache,
>> it will lock up.
>>
>> To mitigate some of this impact, this driver provides a simple polling
>> top level idle state, to try to avoid the cache flushing overhead when
>> the wait will only be short. The second level state is implemented with
>> the MIPS wait instruction.
>>
>> This patch first found in the github repository of CI20, the original
>> author is Alex Smith. Because there is a chance to cause kernel hang
>> scenarios which can occur within hours or even within days, so this
>> patch was abandoned, but now it is determined that this is not the
>> problem caused by this patch, but caused by the cache driver. With
>> the new Ingenic specific cache driver, it has been working properly
>> on CI20 v1 for more than one week.
>>
>> Tested-by: H. Nikolaus Schaller <hns@...delico.com>
>> Signed-off-by: Alex Smith <alex.smith@...tec.com>
>> Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@...yeetech.com>
>> ---
>> drivers/cpuidle/Kconfig.mips | 8 +++++
>> drivers/cpuidle/Makefile | 1 +
>> drivers/cpuidle/cpuidle-jz4780.c | 74
>> ++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 83 insertions(+)
>> create mode 100644 drivers/cpuidle/cpuidle-jz4780.c
>>
>> diff --git a/drivers/cpuidle/Kconfig.mips b/drivers/cpuidle/Kconfig.mips
>> index c3c011a..4a55d24 100644
>> --- a/drivers/cpuidle/Kconfig.mips
>> +++ b/drivers/cpuidle/Kconfig.mips
>> @@ -16,3 +16,11 @@ config MIPS_CPS_CPUIDLE
>> Processing System (CPS) architecture. In order to make use of
>> the deepest idle states you will need to ensure that you are
>> also using the CONFIG_MIPS_CPS SMP implementation.
>> +
>> +config MIPS_JZ4780_CPUIDLE
>> + bool "CPU Idle driver for Ingenic JZ4780"
>> + depends on MACH_JZ4780 && SMP
>> + default y
>> + help
>> + Select this option to enable CPU idle state management through
>> + cpuidle for Ingenic JZ4780 platforms.
>> diff --git a/drivers/cpuidle/Makefile b/drivers/cpuidle/Makefile
>> index 26bbc5e..1dd372f 100644
>> --- a/drivers/cpuidle/Makefile
>> +++ b/drivers/cpuidle/Makefile
>> @@ -29,6 +29,7 @@ obj-$(CONFIG_ARM_QCOM_SPM_CPUIDLE) +=
>> cpuidle-qcom-spm.o
>>
>> ###############################################################################
>>
>> # MIPS drivers
>> obj-$(CONFIG_MIPS_CPS_CPUIDLE) += cpuidle-cps.o
>> +obj-$(CONFIG_MIPS_JZ4780_CPUIDLE) += cpuidle-jz4780.o
>>
>>
>> ###############################################################################
>>
>> # POWERPC drivers
>> diff --git a/drivers/cpuidle/cpuidle-jz4780.c
>> b/drivers/cpuidle/cpuidle-jz4780.c
>> new file mode 100644
>> index 00000000..2025de4
>> --- /dev/null
>> +++ b/drivers/cpuidle/cpuidle-jz4780.c
>> @@ -0,0 +1,74 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * JZ4780 CPU idle driver
>> + * Copyright (C) 2015 Imagination Technologies
>> + * Author: Alex Smith <alex.smith@...tec.com>
>> + * Copyright (c) 2020 周琰杰 (Zhou Yanjie) <zhouyanjie@...yeetech.com>
>> + */
>> +
>> +#include <linux/cpuidle.h>
>> +#include <linux/init.h>
>> +#include <linux/sched.h>
>> +#include <linux/sched/idle.h>
>> +
>> +#include <asm/idle.h>
>> +#include <asm/mipsregs.h>
>> +
>> +/*
>> + * The JZ4780 has a high overhead to entering just the basic MIPS
>> wait on SMP,
>> + * due to the requirement to flush out dirty lines from the dcache
>> before
>> + * waiting. Therefore, we try to mitigate this overhead by using a
>> simple
>> + * polling loop for short waits.
>> + */
>> +static int jz4780_cpuidle_poll_enter(struct cpuidle_device *dev,
>> + struct cpuidle_driver *drv, int index)
>> +{
>> + if (!current_set_polling_and_test())
>> + while (!need_resched() && !(read_c0_cause() &
>> read_c0_status() & CAUSEF_IP))
>> + cpu_relax();
>> +
>> + current_clr_polling();
>> + local_irq_enable();
>> +
>> + return index;
>> +}
>> +
>> +static struct cpuidle_driver jz4780_cpuidle_driver = {
>> + .name = "jz4780_cpuidle",
>> + .owner = THIS_MODULE,
>> + .states = {
>> + {
>> + .enter = jz4780_cpuidle_poll_enter,
>> + .exit_latency = 1,
>> + .target_residency = 1,
>> + .power_usage = UINT_MAX,
>> + .name = "poll",
>> + .desc = "polling loop",
>> + },
>> + {
>> + .enter = mips_cpuidle_wait_enter,
>> + .exit_latency = 50,
>> + .target_residency = 300,
>> + .power_usage = UINT_MAX,
>> + .name = "wait",
>> + .desc = "MIPS wait",
>> + },
>> + },
>> + .state_count = 2,
>> +};
>> +
>> +static int __init jz4780_cpuidle_init(void)
>> +{
>> + int ret;
>> +
>> + ret = cpuidle_register(&jz4780_cpuidle_driver, NULL);
>
> You're missing something here - you never check that the kernel is
> actually running on a JZ4780.
Sure, I will try to fix it.
Thanks and best regards!
>
> Cheers,
> -Paul
>
>> + if (ret) {
>> + pr_err("Failed to register JZ4780 idle driver: %d\n", ret);
>> + return ret;
>> + }
>> +
>> + pr_info("JZ4780 idle driver registered\n");
>> +
>> + return 0;
>> +}
>> +device_initcall(jz4780_cpuidle_init);
>> --
>> 2.7.4
>>
>
Powered by blists - more mailing lists