linux-kernel - Re: [PATCH RFC 1/2] acpi: typec: ucsi: Introduce a ->poll

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: <CAA8EJpr=SBQ29m8_iCigUKMHzrdbTFRSpTHv4Aapo55hMVOcaw@mail.gmail.com>
Date: Fri, 7 Feb 2025 02:38:03 +0200
From: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To: Fedor Pchelkin <boddah8794@...il.com>
Cc: Heikki Krogerus <heikki.krogerus@...ux.intel.com>, "Christian A. Ehrhardt" <lk@...e.de>, 
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>, Benson Leung <bleung@...omium.org>, 
	Jameson Thies <jthies@...gle.com>, Saranya Gopal <saranya.gopal@...el.com>, linux-usb@...r.kernel.org, 
	linux-kernel@...r.kernel.org, Mark Pearson <mpearson@...ebb.ca>, stable@...r.kernel.org
Subject: Re: [PATCH RFC 1/2] acpi: typec: ucsi: Introduce a ->poll_cci method

On Thu, 6 Feb 2025 at 20:43, Fedor Pchelkin <boddah8794@...il.com> wrote:
>
> From: "Christian A. Ehrhardt" <lk@...e.de>
>
> For the ACPI backend of UCSI the UCSI "registers" are just a memory copy
> of the register values in an opregion. The ACPI implementation in the
> BIOS ensures that the opregion contents are synced to the embedded
> controller and it ensures that the registers (in particular CCI) are
> synced back to the opregion on notifications. While there is an ACPI call
> that syncs the actual registers to the opregion there is rarely a need to
> do this and on some ACPI implementations it actually breaks in various
> interesting ways.
>
> The only reason to force a sync from the embedded controller is to poll
> CCI while notifications are disabled. Only the ucsi core knows if this
> is the case and guessing based on the current command is suboptimal, i.e.
> leading to the following spurious assertion splat:
>
> WARNING: CPU: 3 PID: 76 at drivers/usb/typec/ucsi/ucsi.c:1388 ucsi_reset_ppm+0x1b4/0x1c0 [typec_ucsi]
> CPU: 3 UID: 0 PID: 76 Comm: kworker/3:0 Not tainted 6.12.11-200.fc41.x86_64 #1
> Hardware name: LENOVO 21D0/LNVNB161216, BIOS J6CN45WW 03/17/2023
> Workqueue: events_long ucsi_init_work [typec_ucsi]
> RIP: 0010:ucsi_reset_ppm+0x1b4/0x1c0 [typec_ucsi]
> Call Trace:
>  <TASK>
>  ucsi_init_work+0x3c/0xac0 [typec_ucsi]
>  process_one_work+0x179/0x330
>  worker_thread+0x252/0x390
>  kthread+0xd2/0x100
>  ret_from_fork+0x34/0x50
>  ret_from_fork_asm+0x1a/0x30
>  </TASK>
>
> Thus introduce a ->poll_cci() method that works like ->read_cci() with an
> additional forced sync and document that this should be used when polling
> with notifications disabled. For all other backends that presumably don't
> have this issue use the same implementation for both methods.

Should the ucsi_init() also use ->poll_cci instead of ->read_cci?
Although it's executed with notifications enabled, it looks as if it
might need the additional resync.

>
> Fixes: fa48d7e81624 ("usb: typec: ucsi: Do not call ACPI _DSM method for UCSI read operations")
> Cc: stable@...r.kernel.org
> Signed-off-by: Christian A. Ehrhardt <lk@...e.de>
> Tested-by: Fedor Pchelkin <boddah8794@...il.com>
> Signed-off-by: Fedor Pchelkin <boddah8794@...il.com>
> ---



-- 
With best wishes
Dmitry