linux-kernel - Re: [PATCH] perf: arm_cspmu: Don't touch interrupt registers if no interrupt was assigned

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] [day] [month] [year] [list]

Message-ID: <ebeef2f-8326-3752-7374-7b2514fe9563@os.amperecomputing.com>
Date: Thu, 11 Apr 2024 00:35:44 -0700 (PDT)
From: Ilkka Koskinen <ilkka@...amperecomputing.com>
To: Robin Murphy <robin.murphy@....com>
cc: Ilkka Koskinen <ilkka@...amperecomputing.com>, 
    Besar Wicaksono <bwicaksono@...dia.com>, 
    Suzuki K Poulose <suzuki.poulose@....com>, Will Deacon <will@...nel.org>, 
    Mark Rutland <mark.rutland@....com>, Raag Jadav <raag.jadav@...el.com>, 
    linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] perf: arm_cspmu: Don't touch interrupt registers if no
 interrupt was assigned



On Tue, 9 Apr 2024, Robin Murphy wrote:
> On 09/04/2024 2:05 am, Ilkka Koskinen wrote:
>> 
>> On Mon, 8 Apr 2024, Robin Murphy wrote:
>>> On 2024-04-05 11:33 pm, Ilkka Koskinen wrote:
>>>> 
>>>> On Fri, 5 Apr 2024, Robin Murphy wrote:
>>>>> On 2024-03-07 7:31 pm, Ilkka Koskinen wrote:
>>>>>> The driver enabled and disabled interrupts even if no interrupt was
>>>>>> assigned to the device.
>>>>> 
>>>>> Why's that a concern - if the interrupt isn't routed anywhere, surely it 
>>>>> makes no difference what happens at the source end?
>>>> 
>>>> The issue is that we have two PMUs attached to the same interrupt line.
>>>> Unfortunately, I just don't seem to find time to add support for shared 
>>>> interrupts to the cspmu driver. Meanwhile, I assigned the interrupt to 
>>>> one of the PMUs while the other one has zero in the APMT table.
>>> 
>>> I suspected something like that ;)
>>> 
>>>> Without the patch, I can trigger "ghost interrupt" in the latter PMU.
>>> 
>>> An occasional spurious interrupt should be no big deal. If it ends up as a 
>>> screaming spurious interrupt because we never handle the overflow 
>>> condition on the "other" PMU, then what matters most is that we never 
>>> handle the overflow, thus the "other" PMU is still useless since you can't 
>>> assume the user is going to read it frequently enough to avoid losing 
>>> information and getting nonsense counts back. So this hack really isn't a 
>>> viable solution for anything.
>> 
>> IIRC, what happens is that kernel will disable the interrupt eventually due 
>> to unhandled spurious interrupts making the "working" PMU also useless.
>
> Indeed, but if having one inaccurate PMU is fine, having more than one is no 
> big deal either, right? The moral of the story is that hacking the firmware 
> to lie about the hardware is just not a great idea.

Depends on the use case, of course :D

>
> TBH it's always seemed a bit broken that we allow probing without an IRQ but 
> then have no accommodation for overflow if so. Fixing that would be a good 
> thing in itself, and would at least have the side-effect of allowing your 
> hack to work, however much I may disapprove of that :)
>
> FWIW it is still lingering some way down my to-do list to factor out the 
> fiddly IRQ-sharing/migration code into at least a helper library (if not 
> further into perf core itself) before it gets copy-pasted much more, and it 
> occurs to me that I could then easily factor the IRQ-substitute timer 
> approach from e.g. arm-ccn into that as well... The more I think about it the 
> more I might just convince myself that I want it for the driver I'm currently 
> working on and justify bumping it up the list, let's see...

That sounds like a great idea. Even plain helper library would keep the 
drivers a lot cleaner.

Cheers, Ilkka