linux-kernel - Re: [PATCH v5] ACPI:PRM: Reduce unnecessary printing to avoid the worries of regular users

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: <CAJZ5v0j9J-4-dv=0QxEPxYDS_epnjT9dGCNoZA0LpiDZa7sy8Q@mail.gmail.com>
Date: Mon, 30 Jun 2025 22:06:57 +0200
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Zhu Qiyu <qiyuzhu2@....com>
Cc: rafael@...nel.org, lenb@...nel.org, linux-acpi@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5] ACPI:PRM: Reduce unnecessary printing to avoid the
 worries of regular users

On Tue, May 27, 2025 at 4:43 AM Zhu Qiyu <qiyuzhu2@....com> wrote:
>
> Commit 088984c8d54c ("ACPI: PRM: Find EFI_MEMORY_RUNTIME block for PRM
> handler and context") introduces non-essential printing "Failed to find
> VA for GUID: xxxx, PA: 0x0" which causes unnecessary worry for regular
> users.
>
> Refer to PRM Spec Section 4.1.2[1], both static data buffer address

s/Refer/According to/

> and ACPI parameter buffer address may be NULL if they are not needed.
> So there is no need to print out "Failed to find VA ... " to intimidate
> regular users.

s/intimidate regular users/confuse users/

>
> Link: https://uefi.org/sites/default/files/resources/Platform%20Runtime%20Mechanism%20-%20with%20legal%20notice.pdf # [1]
>
> Signed-off-by: Zhu Qiyu <qiyuzhu2@....com>
> ---
> Previous versions can be found at:
> v1: https://lore.kernel.org/linux-acpi/20250427075317.42687-1-qiyuzhu2@amd.com/
> v2: https://lore.kernel.org/linux-acpi/20250512010620.142155-1-qiyuzhu2@amd.com/#r
> v3: https://lore.kernel.org/linux-acpi/20250512011833.142204-1-qiyuzhu2@amd.com/t/#u
> v4: https://lore.kernel.org/linux-acpi/20250518062507.218855-1-qiyuzhu2@amd.com/
>
> Changes in v2:
>  - Reduce the code changes.
> Changes in v3:
>  - Fixed title letters not showing.
> Changes in v4:
>  - Only print the necessary warnings.
> Changes in v5:
>  - Add more cases of print warnings.
>
>  drivers/acpi/prmt.c | 22 ++++++++++++++++++++--
>  1 file changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/acpi/prmt.c b/drivers/acpi/prmt.c
> index e549914a636c..ed39502ed86f 100644
> --- a/drivers/acpi/prmt.c
> +++ b/drivers/acpi/prmt.c
> @@ -85,8 +85,6 @@ static u64 efi_pa_va_lookup(efi_guid_t *guid, u64 pa)
>                 }
>         }
>
> -       pr_warn("Failed to find VA for GUID: %pUL, PA: 0x%llx", guid, pa);
> -
>         return 0;
>  }
>
> @@ -154,13 +152,33 @@ acpi_parse_prmt(union acpi_subtable_headers *header, const unsigned long end)
>                 guid_copy(&th->guid, (guid_t *)handler_info->handler_guid);
>                 th->handler_addr =
>                         (void *)efi_pa_va_lookup(&th->guid, handler_info->handler_address);
> +               /*
> +                * Refer to PRM Spec, both static data buffer address and
> +                * ACPI parameter buffer address may be NULL if they are not
> +                * needed, so there print out warning if handler_addr is zero.
> +                */

Change the comment above to something like "Print a warning message if
handler_addr is zero which is not expected to ever happen."

> +               if (unlikely(!th->handler_addr))
> +                       pr_warn("Failed to find VA of handler for GUID: %pUL, PA: 0x%llx",
> +                               &th->guid, handler_info->handler_address);
>
>                 th->static_data_buffer_addr =
>                         efi_pa_va_lookup(&th->guid, handler_info->static_data_buffer_address);
> +               /*
> +                * As for the static_data_buffer_addr/acpi_param_buffer_addr,
> +                * if the physical address passed to efi_pa_va_lookup() is nonzero
> +                * and the return value is zero, there should print out the warnings.
> +                */

Change the comment above to something like "According to the PRM
specification, static_data_buffer_address can be zero, so avoid
printing a warning message in that case."

> +               if (unlikely(!th->static_data_buffer_addr && handler_info->static_data_buffer_address))
> +                       pr_warn("Failed to find VA of static data buffer for GUID: %pUL, PA: 0x%llx",
> +                               &th->guid, handler_info->static_data_buffer_address);
>
>                 th->acpi_param_buffer_addr =
>                         efi_pa_va_lookup(&th->guid, handler_info->acpi_param_buffer_address);
>
> +               if (unlikely(!th->acpi_param_buffer_addr && handler_info->acpi_param_buffer_address))
> +                       pr_warn("Failed to find VA of acpi param buffer for GUID: %pUL, PA: 0x%llx",
> +                               &th->guid, handler_info->acpi_param_buffer_address);

And analogously here.

> +
>         } while (++cur_handler < tm->handler_count && (handler_info = get_next_handler(handler_info)));
>
>         return 0;
>

Thanks!