linux-kernel - Re: [PATCH] mei: vsc: Use "wakeuphostint" when getting the host wakeup GPIO

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

Message-ID: <Z7RDpYVCLIdMiytw@kekkonen.localdomain>
Date: Tue, 18 Feb 2025 08:24:05 +0000
From: Sakari Ailus <sakari.ailus@...ux.intel.com>
To: Hans de Goede <hdegoede@...hat.com>
Cc: Wentong Wu <wentong.wu@...el.com>, Jason Chen <jason.z.chen@...el.com>,
	Alexander Usyskin <alexander.usyskin@...el.com>,
	Arnd Bergmann <arnd@...db.de>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Stanislaw Gruszka <stanislaw.gruszka@...ux.intel.com>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] mei: vsc: Use "wakeuphostint" when getting the host
 wakeup GPIO

Hi Hans,

On Fri, Feb 14, 2025 at 10:24:25PM +0100, Hans de Goede wrote:
> The _CRS ACPI resources table has 2 entries for the host wakeup GPIO,
> the first one being a regular GpioIo () resource while the second one
> is a GpioInt () resource for the same pin.
> 
> The acpi_gpio_mapping table used by vsc-tp.c maps the first Gpio ()
> resource to "wakeuphost-gpios" where as the second GpioInt () entry
> is mapped to "wakeuphostint-gpios".
> 
> Using "wakeuphost" to request the GPIO as was done until now, means
> that the gpiolib-acpi code does not know that the GPIO is active-low
> as that info is only available in the GpioInt () entry.
> 
> Things were still working before due to the following happening:
> 
> 1. Since the 2 entries point to the same pin they share a struct gpio_desc
> 2. The SPI core creates the SPI device vsc-tp.c binds to and calls
>    acpi_dev_gpio_irq_get(). This does use the second entry and sets
>    FLAG_ACTIVE_LOW in gpio_desc.flags .
> 3. vsc_tp_probe() requests the "wakeuphost" GPIO and inherits the
>    active-low flag set by acpi_dev_gpio_irq_get()
> 
> But there is a possible scenario where things do not work:
> 
> 1. - 3. happen as above
> 4. After requesting the "wakeuphost" GPIO, the "resetfw" GPIO is requested
>    next, but its USB GPIO controller is not available yet, so this call
>    returns -EPROBE_DEFER.
> 5. The gpio_desc for "wakeuphost" is put() and during this the active-low
>    flag is cleared from gpio_desc.flags .
> 6. Later on vsc_tp_probe() requests the "wakeuphost" GPIO again, but now it
>    is not marked active-low.
> 
> The difference can also be seen in /sys/kernel/debug/gpio, which contains
> the following line for this GPIO:
> 
>  gpio-535 (                    |wakeuphost          ) in  hi IRQ ACTIVE LOW
> 
> If the second scenario is hit the "ACTIVE LOW" at the end disappears and
> things do not work.
> 
> Fix this by requesting the GPIO through the "wakeuphostint" mapping instead
> which provides active-low info without relying on acpi_dev_gpio_irq_get()
> pre-populating this info in the gpio_desc.
> 
> Link: https://bugzilla.redhat.com/show_bug.cgi?id=2316918
> Signed-off-by: Hans de Goede <hdegoede@...hat.com>

Great detective work! :-)

Tested-by: Sakari Ailus <sakari.ailus@...ux.intel.com>

This was on Arch Linux where I've seen this only, never on Debian.

-- 
Regards,

Sakari Ailus