diff mbox series

[PULL,11/41] hw/acpi/ghes: Make ghes_record_cper_errors() static

Message ID 20250305012157.96463-12-philmd@linaro.org
State New
Headers show
Series [PULL,01/41] hw/intc: Remove TCG dependency on ARM_GICV3 | expand

Commit Message

Philippe Mathieu-Daudé March 5, 2025, 1:21 a.m. UTC
From: Gavin Shan <gshan@redhat.com>

acpi_ghes_memory_errors() is the only caller, no need to expose
the function. Besides, the last 'return' in this function isn't
necessary and remove it.

No functional changes intended.

Signed-off-by: Gavin Shan <gshan@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-ID: <20250214041635.608012-2-gshan@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/acpi/ghes.h | 2 --
 hw/acpi/ghes.c         | 6 ++----
 2 files changed, 2 insertions(+), 6 deletions(-)

Comments

Mauro Carvalho Chehab March 6, 2025, 10:36 p.m. UTC | #1
Em Wed,  5 Mar 2025 02:21:26 +0100
Philippe Mathieu-Daudé <philmd@linaro.org> escreveu:

> From: Gavin Shan <gshan@redhat.com>
> 
> acpi_ghes_memory_errors() is the only caller, no need to expose
> the function. Besides, the last 'return' in this function isn't
> necessary and remove it.
> 
> No functional changes intended.

Please revert this patch, as ghes_record_cper_errors() was written
to be used for error injection. As agreed last year with some ACPI
maintainers, we ended splitting the error injection series on two parts
to make easier for people to review it.

The followup series:

	https://lore.kernel.org/qemu-devel/cover.1740903110.git.mchehab+huawei@kernel.org/

Need this function to be not static, as this will be used by a
QMP caller.

The usage itself is on this patch:

	https://lore.kernel.org/qemu-devel/6ef8d6a3f42e3347ed6fd3d1fc29ab5ff2a070df.1740903110.git.mchehab+huawei@kernel.org/

but this one causes conflict since patch 01 of the series.

Regards,
Mauro


> 
> Signed-off-by: Gavin Shan <gshan@redhat.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Message-ID: <20250214041635.608012-2-gshan@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  include/hw/acpi/ghes.h | 2 --
>  hw/acpi/ghes.c         | 6 ++----
>  2 files changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/include/hw/acpi/ghes.h b/include/hw/acpi/ghes.h
> index 39619a2457c..578a582203c 100644
> --- a/include/hw/acpi/ghes.h
> +++ b/include/hw/acpi/ghes.h
> @@ -75,8 +75,6 @@ void acpi_build_hest(GArray *table_data, GArray *hardware_errors,
>  void acpi_ghes_add_fw_cfg(AcpiGhesState *vms, FWCfgState *s,
>                            GArray *hardware_errors);
>  int acpi_ghes_memory_errors(uint16_t source_id, uint64_t error_physical_addr);
> -void ghes_record_cper_errors(const void *cper, size_t len,
> -                             uint16_t source_id, Error **errp);
>  
>  /**
>   * acpi_ghes_present: Report whether ACPI GHES table is present
> diff --git a/hw/acpi/ghes.c b/hw/acpi/ghes.c
> index b709c177cde..b85bb48195a 100644
> --- a/hw/acpi/ghes.c
> +++ b/hw/acpi/ghes.c
> @@ -390,8 +390,8 @@ static void get_hw_error_offsets(uint64_t ghes_addr,
>      *read_ack_register_addr = ghes_addr + sizeof(uint64_t);
>  }
>  
> -void ghes_record_cper_errors(const void *cper, size_t len,
> -                             uint16_t source_id, Error **errp)
> +static void ghes_record_cper_errors(const void *cper, size_t len,
> +                                    uint16_t source_id, Error **errp)
>  {
>      uint64_t cper_addr = 0, read_ack_register_addr = 0, read_ack_register;
>      AcpiGedState *acpi_ged_state;
> @@ -440,8 +440,6 @@ void ghes_record_cper_errors(const void *cper, size_t len,
>  
>      /* Write the generic error data entry into guest memory */
>      cpu_physical_memory_write(cper_addr, cper, len);
> -
> -    return;
>  }
>  
>  int acpi_ghes_memory_errors(uint16_t source_id, uint64_t physical_address)



Thanks,
Mauro
Philippe Mathieu-Daudé March 7, 2025, 12:41 p.m. UTC | #2
Hi Mauro,

On 6/3/25 23:36, Mauro Carvalho Chehab wrote:
> Em Wed,  5 Mar 2025 02:21:26 +0100
> Philippe Mathieu-Daudé <philmd@linaro.org> escreveu:
> 
>> From: Gavin Shan <gshan@redhat.com>
>>
>> acpi_ghes_memory_errors() is the only caller, no need to expose
>> the function. Besides, the last 'return' in this function isn't
>> necessary and remove it.
>>
>> No functional changes intended.
> 
> Please revert this patch, as ghes_record_cper_errors() was written
> to be used for error injection. As agreed last year with some ACPI
> maintainers, we ended splitting the error injection series on two parts
> to make easier for people to review it.
> 
> The followup series:
> 
> 	https://lore.kernel.org/qemu-devel/cover.1740903110.git.mchehab+huawei@kernel.org/
> 
> Need this function to be not static, as this will be used by a
> QMP caller.
> 
> The usage itself is on this patch:
> 
> 	https://lore.kernel.org/qemu-devel/6ef8d6a3f42e3347ed6fd3d1fc29ab5ff2a070df.1740903110.git.mchehab+huawei@kernel.org/
> 
> but this one causes conflict since patch 01 of the series.

As of this commit this method isn't used elsewhere in the tree,
so the reversion has to happen in the series using it, if it
eventually lands. Otherwise it is too costly to maintain
incomplete features.

Regards,

Phil.
diff mbox series

Patch

diff --git a/include/hw/acpi/ghes.h b/include/hw/acpi/ghes.h
index 39619a2457c..578a582203c 100644
--- a/include/hw/acpi/ghes.h
+++ b/include/hw/acpi/ghes.h
@@ -75,8 +75,6 @@  void acpi_build_hest(GArray *table_data, GArray *hardware_errors,
 void acpi_ghes_add_fw_cfg(AcpiGhesState *vms, FWCfgState *s,
                           GArray *hardware_errors);
 int acpi_ghes_memory_errors(uint16_t source_id, uint64_t error_physical_addr);
-void ghes_record_cper_errors(const void *cper, size_t len,
-                             uint16_t source_id, Error **errp);
 
 /**
  * acpi_ghes_present: Report whether ACPI GHES table is present
diff --git a/hw/acpi/ghes.c b/hw/acpi/ghes.c
index b709c177cde..b85bb48195a 100644
--- a/hw/acpi/ghes.c
+++ b/hw/acpi/ghes.c
@@ -390,8 +390,8 @@  static void get_hw_error_offsets(uint64_t ghes_addr,
     *read_ack_register_addr = ghes_addr + sizeof(uint64_t);
 }
 
-void ghes_record_cper_errors(const void *cper, size_t len,
-                             uint16_t source_id, Error **errp)
+static void ghes_record_cper_errors(const void *cper, size_t len,
+                                    uint16_t source_id, Error **errp)
 {
     uint64_t cper_addr = 0, read_ack_register_addr = 0, read_ack_register;
     AcpiGedState *acpi_ged_state;
@@ -440,8 +440,6 @@  void ghes_record_cper_errors(const void *cper, size_t len,
 
     /* Write the generic error data entry into guest memory */
     cpu_physical_memory_write(cper_addr, cper, len);
-
-    return;
 }
 
 int acpi_ghes_memory_errors(uint16_t source_id, uint64_t physical_address)