1. Patchew
  2. QEMU
  3. [PATCH 0/7] hw/i386: Cleanups around 'hw/i386/pc.h'
  4. View patch

[PATCH 4/7] hw/i386/acpi: Declare pc_madt_cpu_entry() in 'acpi-common.h'

Philippe Mathieu-Daudé posted 7 patches 9 months, 2 weeks ago
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, Igor Mammedov <imammedo@redhat.com>, Ani Sinha <anisinha@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, Eduardo Habkost <eduardo@habkost.net>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
[PATCH 4/7] hw/i386/acpi: Declare pc_madt_cpu_entry() in 'acpi-common.h'
Posted by Philippe Mathieu-Daudé 9 months, 2 weeks ago 
Since pc_madt_cpu_entry() is only used by:
 - hw/i386/acpi-build.c   // single call
 - hw/i386/acpi-common.c  // definition
there is no need to expose it outside of hw/i386/.
Declare it in "acpi-common.h".
acpi-build.c doesn't need "hw/i386/pc.h" anymore.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/i386/acpi-common.h | 3 +++
 include/hw/i386/pc.h  | 4 ----
 hw/i386/acpi-common.c | 1 -
 3 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/hw/i386/acpi-common.h b/hw/i386/acpi-common.h
index b3c56ee014..e305aaac15 100644
--- a/hw/i386/acpi-common.h
+++ b/hw/i386/acpi-common.h
@@ -1,12 +1,15 @@
 #ifndef HW_I386_ACPI_COMMON_H
 #define HW_I386_ACPI_COMMON_H
 
+#include "hw/boards.h"
 #include "hw/acpi/bios-linker-loader.h"
 #include "hw/i386/x86.h"
 
 /* Default IOAPIC ID */
 #define ACPI_BUILD_IOAPIC_ID 0x0
 
+void pc_madt_cpu_entry(int uid, const CPUArchIdList *apic_ids,
+                       GArray *entry, bool force_enabled);
 void acpi_build_madt(GArray *table_data, BIOSLinker *linker,
                      X86MachineState *x86ms,
                      const char *oem_id, const char *oem_table_id);
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index f9fc42c2be..ce442372ac 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -199,10 +199,6 @@ bool pc_system_ovmf_table_find(const char *entry, uint8_t **data,
                                int *data_len);
 void pc_system_parse_ovmf_flash(uint8_t *flash_ptr, size_t flash_size);
 
-/* hw/i386/acpi-common.c */
-void pc_madt_cpu_entry(int uid, const CPUArchIdList *apic_ids,
-                       GArray *entry, bool force_enabled);
-
 /* sgx.c */
 void pc_machine_init_sgx_epc(PCMachineState *pcms);
 
diff --git a/hw/i386/acpi-common.c b/hw/i386/acpi-common.c
index 43dc23f7e0..f1a11f833a 100644
--- a/hw/i386/acpi-common.c
+++ b/hw/i386/acpi-common.c
@@ -27,7 +27,6 @@
 #include "hw/acpi/acpi.h"
 #include "hw/acpi/aml-build.h"
 #include "hw/acpi/utils.h"
-#include "hw/i386/pc.h"
 #include "target/i386/cpu.h"
 
 #include "acpi-build.h"
-- 
2.41.0
Re: [PATCH 4/7] hw/i386/acpi: Declare pc_madt_cpu_entry() in 'acpi-common.h'
Posted by Bernhard Beschow 9 months, 2 weeks ago 

Am 13. Februar 2024 12:01:49 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>Since pc_madt_cpu_entry() is only used by:
> - hw/i386/acpi-build.c   // single call
> - hw/i386/acpi-common.c  // definition
>there is no need to expose it outside of hw/i386/.
>Declare it in "acpi-common.h".
>acpi-build.c doesn't need "hw/i386/pc.h" anymore.
>
>Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>---
> hw/i386/acpi-common.h | 3 +++
> include/hw/i386/pc.h  | 4 ----
> hw/i386/acpi-common.c | 1 -
> 3 files changed, 3 insertions(+), 5 deletions(-)
>
>diff --git a/hw/i386/acpi-common.h b/hw/i386/acpi-common.h
>index b3c56ee014..e305aaac15 100644
>--- a/hw/i386/acpi-common.h
>+++ b/hw/i386/acpi-common.h
>@@ -1,12 +1,15 @@
> #ifndef HW_I386_ACPI_COMMON_H
> #define HW_I386_ACPI_COMMON_H
> 
>+#include "hw/boards.h"
> #include "hw/acpi/bios-linker-loader.h"
> #include "hw/i386/x86.h"
> 
> /* Default IOAPIC ID */
> #define ACPI_BUILD_IOAPIC_ID 0x0
> 
>+void pc_madt_cpu_entry(int uid, const CPUArchIdList *apic_ids,

Since the function is apparently not pc but rather x86-specific: Does it make sense to rename the function as well, e.g. to x86_madt_cpu_entry()?

Best regards,
Bernhard

>+                       GArray *entry, bool force_enabled);
> void acpi_build_madt(GArray *table_data, BIOSLinker *linker,
>                      X86MachineState *x86ms,
>                      const char *oem_id, const char *oem_table_id);
>diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
>index f9fc42c2be..ce442372ac 100644
>--- a/include/hw/i386/pc.h
>+++ b/include/hw/i386/pc.h
>@@ -199,10 +199,6 @@ bool pc_system_ovmf_table_find(const char *entry, uint8_t **data,
>                                int *data_len);
> void pc_system_parse_ovmf_flash(uint8_t *flash_ptr, size_t flash_size);
> 
>-/* hw/i386/acpi-common.c */
>-void pc_madt_cpu_entry(int uid, const CPUArchIdList *apic_ids,
>-                       GArray *entry, bool force_enabled);
>-
> /* sgx.c */
> void pc_machine_init_sgx_epc(PCMachineState *pcms);
> 
>diff --git a/hw/i386/acpi-common.c b/hw/i386/acpi-common.c
>index 43dc23f7e0..f1a11f833a 100644
>--- a/hw/i386/acpi-common.c
>+++ b/hw/i386/acpi-common.c
>@@ -27,7 +27,6 @@
> #include "hw/acpi/acpi.h"
> #include "hw/acpi/aml-build.h"
> #include "hw/acpi/utils.h"
>-#include "hw/i386/pc.h"
> #include "target/i386/cpu.h"
> 
> #include "acpi-build.h"
Re: [PATCH 4/7] hw/i386/acpi: Declare pc_madt_cpu_entry() in 'acpi-common.h'
Posted by Philippe Mathieu-Daudé 9 months, 2 weeks ago 
On 13/2/24 19:57, Bernhard Beschow wrote:
> 
> 
> Am 13. Februar 2024 12:01:49 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>> Since pc_madt_cpu_entry() is only used by:
>> - hw/i386/acpi-build.c   // single call
>> - hw/i386/acpi-common.c  // definition
>> there is no need to expose it outside of hw/i386/.
>> Declare it in "acpi-common.h".
>> acpi-build.c doesn't need "hw/i386/pc.h" anymore.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>> hw/i386/acpi-common.h | 3 +++
>> include/hw/i386/pc.h  | 4 ----
>> hw/i386/acpi-common.c | 1 -
>> 3 files changed, 3 insertions(+), 5 deletions(-)
>>
>> diff --git a/hw/i386/acpi-common.h b/hw/i386/acpi-common.h
>> index b3c56ee014..e305aaac15 100644
>> --- a/hw/i386/acpi-common.h
>> +++ b/hw/i386/acpi-common.h
>> @@ -1,12 +1,15 @@
>> #ifndef HW_I386_ACPI_COMMON_H
>> #define HW_I386_ACPI_COMMON_H
>>
>> +#include "hw/boards.h"
>> #include "hw/acpi/bios-linker-loader.h"
>> #include "hw/i386/x86.h"
>>
>> /* Default IOAPIC ID */
>> #define ACPI_BUILD_IOAPIC_ID 0x0
>>
>> +void pc_madt_cpu_entry(int uid, const CPUArchIdList *apic_ids,
> 
> Since the function is apparently not pc but rather x86-specific: Does it make sense to rename the function as well, e.g. to x86_madt_cpu_entry()?

I don't know much about ACPI tables. Is it?
Ani, can you confirm and do you mind posting a cleanup patch on top? :)

Regards,

Phil.

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.