On Tue, May 27, 2025 at 4:43 AM Zhu Qiyu <qiyuzhu2@xxxxxxx> 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@xxxxxxx> > --- > Previous versions can be found at: > v1: https://lore.kernel.org/linux-acpi/20250427075317.42687-1-qiyuzhu2@xxxxxxx/ > v2: https://lore.kernel.org/linux-acpi/20250512010620.142155-1-qiyuzhu2@xxxxxxx/#r > v3: https://lore.kernel.org/linux-acpi/20250512011833.142204-1-qiyuzhu2@xxxxxxx/t/#u > v4: https://lore.kernel.org/linux-acpi/20250518062507.218855-1-qiyuzhu2@xxxxxxx/ > > 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!