linux-kernel - Re: [PATCH v6 1/1] MIPS: Fix idle VS timer enqueue

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <alpine.DEB.2.21.2504021933160.53907@angie.orcam.me.uk>
Date: Thu, 3 Apr 2025 13:00:15 +0100 (BST)
From: "Maciej W. Rozycki" <macro@...am.me.uk>
To: Marco Crivellari <marco.crivellari@...e.com>
cc: linux-mips@...r.kernel.org, linux-kernel@...r.kernel.org, 
    Thomas Bogendoerfer <tsbogend@...ha.franken.de>, 
    Frederic Weisbecker <frederic@...nel.org>, 
    Anna-Maria Behnsen <anna-maria@...utronix.de>, 
    Thomas Gleixner <tglx@...utronix.de>, 
    Peter Zijlstra <peterz@...radead.org>, Huacai Chen <chenhuacai@...nel.org>
Subject: Re: [PATCH v6 1/1] MIPS: Fix idle VS timer enqueue

On Wed, 2 Apr 2025, Marco Crivellari wrote:

> > Well, you should be able to set a breakpoint at `rollback_handle_int' and
> > fiddle with $epc by hand to see if the code sequence correctly skips over
> > WAIT.  Though I reckon QEMU used to have an issue with presenting the MIPS
> > privileged context over its debug stub.  Has the issue been fixed?  Either
> > way you should be able to just operate on the copy in $k0 retrieved with
> > (D)MFC0.
> 
> Nope, seems not fixed so far. But yes, changing $k0 is working fine.
> With the cpu in idle code (executing "wait"), ctrl+c, then placed a bp
> in rollback_handle_init+4. Then "c" to hit the bp.
> When the bp is hit, I can see $k0 = r4k_wait_exit.
> 
> I changed $k0 with an address inside the region, and setting a bp on "bne",
>  the value is equal to $k1. I'm assuming the value is also saved
> correctly in $epc,
> considering it points correctly to r4k_wait_exit.

 I think it's enough evidence to prove the code works as expected.

>  > Hmm, "skipover" maybe?
> 
> Now that I'm looking at the code, shouldn't it be better to address this in a
> separate patch or another time?
> 
> I can see the rollback_handler* is exported also in arch/mips/kernel/traps.c
> And there are a few parts that makes use of the "rollback" name; I'm
> wondering if also
> the code should be refactored a bit then,
> eg arch/mips /include/asm/idle.h:using_rollback_handler()

 Yeah, I agree cluttering the semantics change with the rename is not the 
best idea, so a follow-up patch approach will be better.

 Also I wonder if we could come up with yet another name that does not 
have the exact semantics implied, so that if in 20 years' time we change 
our minds again and decide: "Oh, let's do the rollback thing after all," 
we don't have to do the rename again.  Though it's like I'm diverging into 
the bikeshedding area here, so perhaps please just use whatever name you 
have found most reasonable at the time you're about to wrap up with this 
effort.

  Maciej