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 |
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
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 --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)