Index | Thread | Search

From:
Vitaliy Makkoveev <otto@bsdbox.dev>
Subject:
Re: sys/ihidev: prevent crash on interrupt storm
To:
"Kirill A. Korinsky" <kirill@korins.ky>
Cc:
OpenBSD tech <tech@openbsd.org>
Date:
Mon, 23 Dec 2024 14:31:05 +0300

Download raw body.

Thread

> On 23 Dec 2024, at 14:23, Kirill A. Korinsky <kirill@korins.ky> wrote:
> 
> On Mon, 23 Dec 2024 12:06:45 +0100,
> Vitaliy Makkoveev <otto@bsdbox.dev> wrote:
>> 
>>> On 23 Dec 2024, at 13:21, Kirill A. Korinsky <kirill@korins.ky> wrote:
>>> 
>>> tech@,
>>> 
>>> Before I move forward with quirks, I'd like to fix the cause of a crash in a
>>> system on HONOR MagicBook Art 14 Snapdragon.
>>> 
>>> Let's assume that we have a device on a machine I2C bus which goes into a
>>> so-called interrupt storm for some reason. Under this condition, the system
>>> crashes if an interrupt arrives before we allocate a buffer to read it.
>>> 
>>> Here's an example of a trace:
>>> 
>>>       panic: attempt to access user address 0x0 from EL1
>>>       Stopped at panic+0x140: cmp w21, #0x0
>>> 
>>>       TID PID UID PRFLAGS PFLAGS CPU COMMAND
>>>       db_enter() at panic+0x13c
>>>       panic() at kdata_abort+0x180
>>>       do_el0_sync() at handle_el1h_sync+0x68
>>>       handle_el1h_sync() at qciic_exec+0x2d4
>>>       qciic_exec() at ihidev_intr+0x70
>>>       ihidev_intr() at qcgpio_intr+0xac
>>>       qcgpio_intr() at agintc_irq_handler+0x2bc
>>> 
>>> Tested on the same machine, without any additional patches. System boots,
>>> but touchpad doesn't work.
>>> 
>>> Ok?
>>> 
>> 
>> Why don't establish interrupt handler after buffer allocation?
>> 
> 
> Mainly to reuse
> 
> 	if (sc->sc_poll && !sc->sc_frompoll) {
> 		DPRINTF(("%s: received interrupt while polling, disabling "
> 		    "polling\n", sc->sc_dev.dv_xname));
> 		sc->sc_poll = 0;
> 		timeout_del_barrier(&sc->sc_timer);
> 	}
> 
> to disable this device on the case of storm.
> 
> When I move iic_intr_establish to the end of ihidev_attach, a system doesn't
> crash, but it also stuck on boot. I assume that it will boot, eventually,
> because it is wasting a lot of CPU to process a stream of useless interrupts
> from touchpad.

This code only disables polling, and I see it is wrong.