diff mbox series

[v4] ACPI:PRM: Reduce unnecessary printing to avoid the worries of regular users

Message ID 20250518062507.218855-1-qiyuzhu2@amd.com
State New
Headers show
Series [v4] ACPI:PRM: Reduce unnecessary printing to avoid the worries of regular users | expand

Commit Message

Zhu Qiyu May 18, 2025, 6:25 a.m. UTC
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
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.

Link: https://uefi.org/sites/default/files/resources/Platform%20Runtime%20Mechanism%20-%20with%20legal%20notice.pdf # [1]

Signed-off-by: Zhu Qiyu <qiyuzhu2@amd.com>
---

Previous versions can be found at:
v1: https://lore.kernel.org/linux-acpi/CAJZ5v0hv0WKd-SXFhUgYs-Zpc+-PsSNOBu0r7L5TzJWgddtsKA@mail.gmail.com/t/#u
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

Changes in v2:
 - Reduce the code changes.
Changes in v3:
 - Fixed title letters not showing.
Changes in v4:
 - Only print the necessary warnings.

 drivers/acpi/prmt.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)


base-commit: 82f2b0b97b36ee3fcddf0f0780a9a0825d52fec3

Comments

Rafael J. Wysocki May 23, 2025, 2:52 p.m. UTC | #1
On Sun, May 18, 2025 at 8:25 AM Zhu Qiyu <qiyuzhu2@amd.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
> 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.
>
> Link: https://uefi.org/sites/default/files/resources/Platform%20Runtime%20Mechanism%20-%20with%20legal%20notice.pdf # [1]
>
> Signed-off-by: Zhu Qiyu <qiyuzhu2@amd.com>
> ---
>
> Previous versions can be found at:
> v1: https://lore.kernel.org/linux-acpi/CAJZ5v0hv0WKd-SXFhUgYs-Zpc+-PsSNOBu0r7L5TzJWgddtsKA@mail.gmail.com/t/#u
> 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
>
> Changes in v2:
>  - Reduce the code changes.
> Changes in v3:
>  - Fixed title letters not showing.
> Changes in v4:
>  - Only print the necessary warnings.
>
>  drivers/acpi/prmt.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/acpi/prmt.c b/drivers/acpi/prmt.c
> index e549914a636c..bee450869cce 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,6 +152,15 @@ 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 only print out warning if handler address
> +                * is zero.
> +                */
> +               if (!th->handler_addr)
> +                       pr_warn("Failed to find VA for GUID: %pUL, PA: 0x%llx",
> +                               &th->guid, handler_info->handler_address);

I think that the message should still be printed in the other cases if
the physical address passed to efi_pa_va_lookup() is nonzero and the
lookup fails.

>
>                 th->static_data_buffer_addr =
>                         efi_pa_va_lookup(&th->guid, handler_info->static_data_buffer_address);
>
> base-commit: 82f2b0b97b36ee3fcddf0f0780a9a0825d52fec3
> --
Zhu Qiyu May 27, 2025, 2:51 a.m. UTC | #2
Hi,Rafael

  Thanks for the review, and your consideration is correct.
  Patch v5 has been sent, please review, thanks!
  https://lore.kernel.org/linux-acpi/20250527024259.56996-1-qiyuzhu2@amd.com/T/#u

Qiyu
diff mbox series

Patch

diff --git a/drivers/acpi/prmt.c b/drivers/acpi/prmt.c
index e549914a636c..bee450869cce 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,6 +152,15 @@  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 only print out warning if handler address
+		 * is zero.
+		 */
+		if (!th->handler_addr)
+			pr_warn("Failed to find VA 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);