linux-kernel - Re: [PATCH] clocksource/drivers/riscv: Refuse to probe on T-Head

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

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 09 Feb 2023 15:39:32 -0800 (PST)
From:   Palmer Dabbelt <palmer@...osinc.com>
To:     samuel@...lland.org
CC:     linux-riscv@...ts.infradead.org, daniel.lezcano@...aro.org,
        tglx@...utronix.de, Paul Walmsley <paul.walmsley@...ive.com>,
        aou@...s.berkeley.edu, linux-kernel@...r.kernel.org,
        linux@...osinc.com
Subject:     Re: [PATCH] clocksource/drivers/riscv: Refuse to probe on T-Head

On Thu, 09 Feb 2023 15:35:53 PST (-0800), samuel@...lland.org wrote:
> Hi Palmer,
>
> On 2/9/23 17:23, Palmer Dabbelt wrote:
>> From: Palmer Dabbelt <palmer@...osinc.com>
>>
>> As of d9f15a9de44a ("Revert "clocksource/drivers/riscv: Events are
>> stopped during CPU suspend"") this driver no longer functions correctly
>> for the T-Head firmware.  That shouldn't impact any users, as we've got
>
> The current situation is that the C9xx CLINT binding was just accepted,
> so the CLINT is not yet described in any devicetree. So at least with
> upstream OpenSBI, which needs the CLINT DT node, the SBI timer extension
> never worked at all.
>
>> a functioning driver that's higher priority, but let's just be safe and
>> ban it from probing at all.
>>
>> Signed-off-by: Palmer Dabbelt <palmer@...osinc.com>
>> ---
>> This feel super ugly to me, but I'm not sure how to do this more
>> cleanly.  I'm not even sure if it's necessary, but I just ran back into
>> the driver reviewing some other patches so I figured I'd say something.
>
> This is not necessary as long as we add the riscv,timer node with the
> riscv,timer-cannot-wake-cpu property before we add the CLINT node. So it
> should not be a problem for any C9xx platform going forward.

Awesome, that sounds way better.

>
> Regards,
> Samuel
>
>> ---
>>  drivers/clocksource/timer-riscv.c | 16 ++++++++++++++++
>>  1 file changed, 16 insertions(+)
>>
>> diff --git a/drivers/clocksource/timer-riscv.c b/drivers/clocksource/timer-riscv.c
>> index a0d66fabf073..d2d0236d1ae6 100644
>> --- a/drivers/clocksource/timer-riscv.c
>> +++ b/drivers/clocksource/timer-riscv.c
>> @@ -139,6 +139,22 @@ static int __init riscv_timer_init_dt(struct device_node *n)
>>  	if (cpuid != smp_processor_id())
>>  		return 0;
>>
>> +	/*
>> +	 * The T-Head firmware does not route timer interrups to the core
>> +	 * during non-retentive suspend.  This is allowed by the specifications
>> +	 * (no interrupts are required to wake up the core during non-retentive
>> +	 * suspend), but most systems don't behave that way and Linux just
>> +	 * assumes that interrupts work.
>> +	 *
>> +	 * There's another timer for the T-Head sytems that behave this way
>> +	 * that is already probed by default, but just to be sure skip
>> +	 * initializing the SBI driver as it'll just break things later.
>> +	 */
>> +	if (sbi_get_mvendorid() == THEAD_VENDOR_ID) {
>> +		pr_debug_once("Skipping SBI timer on T-Head due to missed wakeups");
>> +		return 0;
>> +	}
>> +
>>  	domain = NULL;
>>  	child = of_get_compatible_child(n, "riscv,cpu-intc");
>>  	if (!child) {