linux-kernel - RE: [PATCH V2] ACPI: APEI: Use ERST timeout for slow devices

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: <MN2PR12MB33738FA73A69BC6AEB64BD63DBDEA@MN2PR12MB3373.namprd12.prod.outlook.com>
Date:   Wed, 25 Oct 2023 14:09:37 +0000
From:   Jeshua Smith <jeshuas@...dia.com>
To:     Borislav Petkov <bp@...en8.de>,
        "Rafael J. Wysocki" <rafael@...nel.org>
CC:     "Luck, Tony" <tony.luck@...el.com>,
        "james.morse@....com" <james.morse@....com>,
        "keescook@...omium.org" <keescook@...omium.org>,
        "gpiccoli@...lia.com" <gpiccoli@...lia.com>,
        "lenb@...nel.org" <lenb@...nel.org>,
        "linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-hardening@...r.kernel.org" <linux-hardening@...r.kernel.org>,
        "linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>,
        Thierry Reding <treding@...dia.com>,
        Jonathan Hunter <jonathanh@...dia.com>
Subject: RE: [PATCH V2] ACPI: APEI: Use ERST timeout for slow devices

Hi Boris,

You asked several questions, and it's not clear to me if you are suggesting the answers be sent as email reply, or if you're asking for the answers to be added to the commit message. Below are my email replies to those questions.

Borislav Petkov wrote:
> When I see "may" in commit messages "Slow devices such as flash may not meet the default 1ms timeout value" then I wanna know what devices are those?

The ERST table specifies an interface for how the OS can serialize error records to a "persistent store". The details of the persistent storage device are implementation defined. The ERST table provides microsecond values for the "nominal" and "maximum" amount of time it takes for the implemented device to process and complete an EXECUTE_OPERATION (a record write, read, or clear request). The current APEI ERST code hardcodes the timeout to 1ms, and ignores the actual timing information that the platform has provided in the ERST table for the platform's implementation. This is a problem for any device that can or will take more than 1ms worst case to process and complete requested operations. On a platform that uses NOR flash as the "persistent store", for example, it can easily take longer than 1ms.

Detailed NOR flash example:
A Micron nor-flash spec sheet: https://media-www.micron.com/-/media/client/global/documents/products/data-sheet/nor-flash/serial-nor/mt25q/die-rev-b/mt25q_qlkt_u_02g_cbb_0.pdf?rev=9b167fbf2b3645efba6385949a72e453

Page 82 lists "Page program time (256 bytes)" as 120us typical, 1800us max.

A 32KB error log would be (32K/256) = 128 nor-flash pages.

Writing 128 nor-flash pages would then take 120us * 128 = 15ms typical, or 1800us * 128 = 230.4ms max.

> What is the actual use case here?

Actual use case:
Kernel panic -> Pstore calls APEI's ERST code to write the ~32KB error log to persistent store -> ERST code writes the error log to nor-flash, which takes more than 1ms to complete. This is expected, as communicated by the platform to the OS via the maximum time field in the ERST table.

Currently APIE's ERST code will flag a timeout of the write operation after 1ms and return an error to Pstore. My patch will let the write (or read or clear) operation take as long as the maximum time ERST says it could take before flagging a timeout and returning an error. The ERST table has a "attributes" field that includes a "slow" bit to allow the platform to indicate that the address range for the error log "has slow access times", but the spec doesn't define what is considered a slow access time. My patch assumes that since the current timeout is 1ms, any access times greater than 1ms would reasonably be considered "slow", and therefore the extended (ERST-defined) timeout is only applied for implementations that indicate that they are "slow". I assume that platforms which bother to set the "slow" bit will also specify actual timings, and platforms which don't are OK with the current 1ms timeout.

> Upthread there's a question about the ACPI spec. That should be explained too. Because I have no clue what "the ERST max execution time value" is.

As I replied to Tony upthread, the Serialization Actions table entry for OPERATION_TIMINGS (https://uefi.org/specs/ACPI/6.5/18_Platform_Error_Interfaces.html#serialization-actions) says that it is the "value in microseconds that the platform expects would be the maximum amount of time it will take to process and complete an EXECUTE_OPERATION." Based on the spec, I take that to be the worst case time between when the OS does "EXECUTE_OPERATION" and when the OS sees "CHECK_BUSY_STATUS" return FALSE rather than TRUE, for the worst case time of a read/write/clear operation for the maximum size supported by the platform's ERST implementation.

Does that answer your questions?