Re: [PATCH v5 23/23] hw/i386/pc: Replace PCMachineClass::acpi_data_size by PC_ACPI_DATA_SIZE

2024-05-29 Thread Igor Mammedov
On Wed, 29 May 2024 07:15:39 +0200
Philippe Mathieu-Daudé  wrote:

> PCMachineClass::acpi_data_size was only used by the pc-i440fx-2.0
> machine, which got removed. Since it is constant, replace the class
> field by a definition (local to hw/i386/pc.c, since not used
> elsewhere).
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> Reviewed-by: Thomas Huth 
> Reviewed-by: Zhao Liu 
> ---
>  include/hw/i386/pc.h |  4 
>  hw/i386/pc.c | 19 ---
>  hw/i386/pc_piix.c|  7 ---
>  3 files changed, 12 insertions(+), 18 deletions(-)
> 
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 63568eb9e9..db26368ace 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -74,9 +74,6 @@ typedef struct PCMachineState {
>   *
>   * Compat fields:
>   *
> - * @acpi_data_size: Size of the chunk of memory at the top of RAM
> - *  for the BIOS ACPI tables and other BIOS
> - *  datastructures.
>   * @gigabyte_align: Make sure that guest addresses aligned at
>   *  1Gbyte boundaries get mapped to host
>   *  addresses aligned at 1Gbyte boundaries. This
> @@ -100,7 +97,6 @@ struct PCMachineClass {
>  
>  /* ACPI compat: */
>  bool has_acpi_build;
> -unsigned acpi_data_size;
>  int pci_root_uid;
>  
>  /* SMBIOS compat: */
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index b84c8ddba0..9dca3f0354 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -264,6 +264,16 @@ GlobalProperty pc_compat_2_4[] = {
>  };
>  const size_t pc_compat_2_4_len = G_N_ELEMENTS(pc_compat_2_4);
>  
> +/*
> + * @PC_ACPI_DATA_SIZE:
> + * Size of the chunk of memory at the top of RAM for the BIOS ACPI tables
> + * and other BIOS datastructures.
> + *
> + * BIOS ACPI tables: 128K. Other BIOS datastructures: less than 4K
> + * reported to be used at the moment, 32K should be enough for a while.
> + */
> +#define PC_ACPI_DATA_SIZE (0x2 + 0x8000)

PC_FW_DATA would be a better name,
with a correspond x86_load_linux() argument rename

> +
>  GSIState *pc_gsi_create(qemu_irq **irqs, bool pci_enabled)
>  {
>  GSIState *s;
> @@ -645,8 +655,7 @@ void xen_load_linux(PCMachineState *pcms)
>  fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, x86ms->boot_cpus);
>  rom_set_fw(fw_cfg);
>  
> -x86_load_linux(x86ms, fw_cfg, pcmc->acpi_data_size,
> -   pcmc->pvh_enabled);
> +x86_load_linux(x86ms, fw_cfg, PC_ACPI_DATA_SIZE, pcmc->pvh_enabled);
>  for (i = 0; i < nb_option_roms; i++) {
>  assert(!strcmp(option_rom[i].name, "linuxboot.bin") ||
> !strcmp(option_rom[i].name, "linuxboot_dma.bin") ||
> @@ -980,8 +989,7 @@ void pc_memory_init(PCMachineState *pcms,
>  }
>  
>  if (linux_boot) {
> -x86_load_linux(x86ms, fw_cfg, pcmc->acpi_data_size,
> -   pcmc->pvh_enabled);
> +x86_load_linux(x86ms, fw_cfg, PC_ACPI_DATA_SIZE, pcmc->pvh_enabled);
>  }
>  
>  for (i = 0; i < nb_option_roms; i++) {
> @@ -1737,9 +1745,6 @@ static void pc_machine_class_init(ObjectClass *oc, void 
> *data)
>  pcmc->has_reserved_memory = true;
>  pcmc->enforce_amd_1tb_hole = true;
>  pcmc->isa_bios_alias = true;
> -/* BIOS ACPI tables: 128K. Other BIOS datastructures: less than 4K 
> reported
> - * to be used at the moment, 32K should be enough for a while.  */
> -pcmc->acpi_data_size = 0x2 + 0x8000;
>  pcmc->pvh_enabled = true;
>  pcmc->kvmclock_create_always = true;
>  x86mc->apic_xrupt_override = true;
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index e7f51a5f2c..e4930b7f48 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -414,13 +414,6 @@ static void pc_set_south_bridge(Object *obj, int value, 
> Error **errp)
>  pcms->south_bridge = PCSouthBridgeOption_lookup.array[value];
>  }



> -/* Looking for a pc_compat_2_4() function? It doesn't exist.
> - * pc_compat_*() functions that run on machine-init time and
> - * change global QEMU state are deprecated. Please don't create
> - * one, and implement any pc-*-2.4 (and newer) compat code in
> - * hw_compat_*, pc_compat_*, or * pc_*_machine_options().
> - */
> 

misplaced hunk, should be a part of [21/23]

>  #ifdef CONFIG_ISAPC
>  static void pc_init_isa(MachineState *machine)
>  {




[PATCH v5 23/23] hw/i386/pc: Replace PCMachineClass::acpi_data_size by PC_ACPI_DATA_SIZE

2024-05-28 Thread Philippe Mathieu-Daudé
PCMachineClass::acpi_data_size was only used by the pc-i440fx-2.0
machine, which got removed. Since it is constant, replace the class
field by a definition (local to hw/i386/pc.c, since not used
elsewhere).

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Thomas Huth 
Reviewed-by: Zhao Liu 
---
 include/hw/i386/pc.h |  4 
 hw/i386/pc.c | 19 ---
 hw/i386/pc_piix.c|  7 ---
 3 files changed, 12 insertions(+), 18 deletions(-)

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 63568eb9e9..db26368ace 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -74,9 +74,6 @@ typedef struct PCMachineState {
  *
  * Compat fields:
  *
- * @acpi_data_size: Size of the chunk of memory at the top of RAM
- *  for the BIOS ACPI tables and other BIOS
- *  datastructures.
  * @gigabyte_align: Make sure that guest addresses aligned at
  *  1Gbyte boundaries get mapped to host
  *  addresses aligned at 1Gbyte boundaries. This
@@ -100,7 +97,6 @@ struct PCMachineClass {
 
 /* ACPI compat: */
 bool has_acpi_build;
-unsigned acpi_data_size;
 int pci_root_uid;
 
 /* SMBIOS compat: */
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index b84c8ddba0..9dca3f0354 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -264,6 +264,16 @@ GlobalProperty pc_compat_2_4[] = {
 };
 const size_t pc_compat_2_4_len = G_N_ELEMENTS(pc_compat_2_4);
 
+/*
+ * @PC_ACPI_DATA_SIZE:
+ * Size of the chunk of memory at the top of RAM for the BIOS ACPI tables
+ * and other BIOS datastructures.
+ *
+ * BIOS ACPI tables: 128K. Other BIOS datastructures: less than 4K
+ * reported to be used at the moment, 32K should be enough for a while.
+ */
+#define PC_ACPI_DATA_SIZE (0x2 + 0x8000)
+
 GSIState *pc_gsi_create(qemu_irq **irqs, bool pci_enabled)
 {
 GSIState *s;
@@ -645,8 +655,7 @@ void xen_load_linux(PCMachineState *pcms)
 fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, x86ms->boot_cpus);
 rom_set_fw(fw_cfg);
 
-x86_load_linux(x86ms, fw_cfg, pcmc->acpi_data_size,
-   pcmc->pvh_enabled);
+x86_load_linux(x86ms, fw_cfg, PC_ACPI_DATA_SIZE, pcmc->pvh_enabled);
 for (i = 0; i < nb_option_roms; i++) {
 assert(!strcmp(option_rom[i].name, "linuxboot.bin") ||
!strcmp(option_rom[i].name, "linuxboot_dma.bin") ||
@@ -980,8 +989,7 @@ void pc_memory_init(PCMachineState *pcms,
 }
 
 if (linux_boot) {
-x86_load_linux(x86ms, fw_cfg, pcmc->acpi_data_size,
-   pcmc->pvh_enabled);
+x86_load_linux(x86ms, fw_cfg, PC_ACPI_DATA_SIZE, pcmc->pvh_enabled);
 }
 
 for (i = 0; i < nb_option_roms; i++) {
@@ -1737,9 +1745,6 @@ static void pc_machine_class_init(ObjectClass *oc, void 
*data)
 pcmc->has_reserved_memory = true;
 pcmc->enforce_amd_1tb_hole = true;
 pcmc->isa_bios_alias = true;
-/* BIOS ACPI tables: 128K. Other BIOS datastructures: less than 4K reported
- * to be used at the moment, 32K should be enough for a while.  */
-pcmc->acpi_data_size = 0x2 + 0x8000;
 pcmc->pvh_enabled = true;
 pcmc->kvmclock_create_always = true;
 x86mc->apic_xrupt_override = true;
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index e7f51a5f2c..e4930b7f48 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -414,13 +414,6 @@ static void pc_set_south_bridge(Object *obj, int value, 
Error **errp)
 pcms->south_bridge = PCSouthBridgeOption_lookup.array[value];
 }
 
-/* Looking for a pc_compat_2_4() function? It doesn't exist.
- * pc_compat_*() functions that run on machine-init time and
- * change global QEMU state are deprecated. Please don't create
- * one, and implement any pc-*-2.4 (and newer) compat code in
- * hw_compat_*, pc_compat_*, or * pc_*_machine_options().
- */
-
 #ifdef CONFIG_ISAPC
 static void pc_init_isa(MachineState *machine)
 {
-- 
2.41.0