Re: [PATCH v5 05/23] hw/i386/acpi: Remove PCMachineClass::legacy_acpi_table_size

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

> PCMachineClass::legacy_acpi_table_size was only used by the
> pc-i440fx-2.0 machine, which got removed. Remove it and simplify
> acpi_build().
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> Reviewed-by: Zhao Liu 

nit below
otherwise

Reviewed-by: Igor Mammedov 

> ---
>  include/hw/i386/pc.h |  1 -
>  hw/i386/acpi-build.c | 62 +---
>  2 files changed, 12 insertions(+), 51 deletions(-)
> 
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 7347636d47..01fdcfaeb6 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -103,7 +103,6 @@ struct PCMachineClass {
>  /* ACPI compat: */
>  bool has_acpi_build;
>  bool rsdp_in_ram;
> -int legacy_acpi_table_size;
>  unsigned acpi_data_size;
>  int pci_root_uid;
>  
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 53f804ac16..a6f8203460 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -2499,13 +2499,12 @@ void acpi_build(AcpiBuildTables *tables, MachineState 
> *machine)
>  X86MachineState *x86ms = X86_MACHINE(machine);
>  DeviceState *iommu = pcms->iommu;
>  GArray *table_offsets;
> -unsigned facs, dsdt, rsdt, fadt;
> +unsigned facs, dsdt, rsdt;
>  AcpiPmInfo pm;
>  AcpiMiscInfo misc;
>  AcpiMcfgInfo mcfg;
>  Range pci_hole = {}, pci_hole64 = {};
>  uint8_t *u;
> -size_t aml_len = 0;
>  GArray *tables_blob = tables->table_data;
>  AcpiSlicOem slic_oem = { .id = NULL, .table_id = NULL };
>  Object *vmgenid_dev;
> @@ -2551,19 +2550,12 @@ void acpi_build(AcpiBuildTables *tables, MachineState 
> *machine)
>  build_dsdt(tables_blob, tables->linker, , ,
> _hole, _hole64, machine);
>  
> -/* Count the size of the DSDT and SSDT, we will need it for legacy
> - * sizing of ACPI tables.
> - */
> -aml_len += tables_blob->len - dsdt;
> -
>  /* ACPI tables pointed to by RSDT */
> -fadt = tables_blob->len;
>  acpi_add_table(table_offsets, tables_blob);
>  pm.fadt.facs_tbl_offset = 
>  pm.fadt.dsdt_tbl_offset = 
>  pm.fadt.xdsdt_tbl_offset = 
>  build_fadt(tables_blob, tables->linker, , oem_id, oem_table_id);
> -aml_len += tables_blob->len - fadt;
>  
>  acpi_add_table(table_offsets, tables_blob);
>  acpi_build_madt(tables_blob, tables->linker, x86ms,
> @@ -2694,49 +2686,19 @@ void acpi_build(AcpiBuildTables *tables, MachineState 
> *machine)
>   * too simple to be enough.  4k turned out to be too small an
>   * alignment very soon, and in fact it is almost impossible to
>   * keep the table size stable for all (max_cpus, max_memory_slots)
> - * combinations.  So the table size is always 64k for pc-i440fx-2.1
> - * and we give an error if the table grows beyond that limit.
> - *
> - * We still have the problem of migrating from "-M pc-i440fx-2.0".  For
> - * that, we exploit the fact that QEMU 2.1 generates _smaller_ tables
> - * than 2.0 and we can always pad the smaller tables with zeros.  We can
> - * then use the exact size of the 2.0 tables.
> - *
> - * All this is for PIIX4, since QEMU 2.0 didn't support Q35 migration.
> + * combinations.
>   */
> -if (pcmc->legacy_acpi_table_size) {
> -/* Subtracting aml_len gives the size of fixed tables.  Then add the
> - * size of the PIIX4 DSDT/SSDT in QEMU 2.0.
> - */
> -int legacy_aml_len =
> -pcmc->legacy_acpi_table_size +
> -ACPI_BUILD_LEGACY_CPU_AML_SIZE * x86ms->apic_id_limit;

missed removal of ACPI_BUILD_LEGACY_CPU_AML_SIZE definition at the beginning of 
the file.

> -int legacy_table_size =
> -ROUND_UP(tables_blob->len - aml_len + legacy_aml_len,
> - ACPI_BUILD_ALIGN_SIZE);
> -if ((tables_blob->len > legacy_table_size) &&
> -!pcmc->resizable_acpi_blob) {
> -/* Should happen only with PCI bridges and -M pc-i440fx-2.0.  */
> -warn_report("ACPI table size %u exceeds %d bytes,"
> -" migration may not work",
> -tables_blob->len, legacy_table_size);
> -error_printf("Try removing CPUs, NUMA nodes, memory slots"
> - " or PCI bridges.\n");
> -}
> -g_array_set_size(tables_blob, legacy_table_size);
> -} else {
> -/* Make sure we have a buffer in case we need to resize the tables. 
> */
> -if ((tables_blob->len > ACPI_BUILD_TABLE_SIZE / 2) &&
> -!pcmc->resizable_acpi_blob) {
> -/* As of QEMU 2.1, this fires with 160 VCPUs and 255 memory 
> slots.  */
> -warn_report("ACPI table size %u exceeds %d bytes,"
> -" migration may not work",
> -tables_blob->len, ACPI_BUILD_TABLE_SIZE / 2);
> -error_printf("Try 

[PATCH v5 05/23] hw/i386/acpi: Remove PCMachineClass::legacy_acpi_table_size

2024-05-28 Thread Philippe Mathieu-Daudé
PCMachineClass::legacy_acpi_table_size was only used by the
pc-i440fx-2.0 machine, which got removed. Remove it and simplify
acpi_build().

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Zhao Liu 
---
 include/hw/i386/pc.h |  1 -
 hw/i386/acpi-build.c | 62 +---
 2 files changed, 12 insertions(+), 51 deletions(-)

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 7347636d47..01fdcfaeb6 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -103,7 +103,6 @@ struct PCMachineClass {
 /* ACPI compat: */
 bool has_acpi_build;
 bool rsdp_in_ram;
-int legacy_acpi_table_size;
 unsigned acpi_data_size;
 int pci_root_uid;
 
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 53f804ac16..a6f8203460 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2499,13 +2499,12 @@ void acpi_build(AcpiBuildTables *tables, MachineState 
*machine)
 X86MachineState *x86ms = X86_MACHINE(machine);
 DeviceState *iommu = pcms->iommu;
 GArray *table_offsets;
-unsigned facs, dsdt, rsdt, fadt;
+unsigned facs, dsdt, rsdt;
 AcpiPmInfo pm;
 AcpiMiscInfo misc;
 AcpiMcfgInfo mcfg;
 Range pci_hole = {}, pci_hole64 = {};
 uint8_t *u;
-size_t aml_len = 0;
 GArray *tables_blob = tables->table_data;
 AcpiSlicOem slic_oem = { .id = NULL, .table_id = NULL };
 Object *vmgenid_dev;
@@ -2551,19 +2550,12 @@ void acpi_build(AcpiBuildTables *tables, MachineState 
*machine)
 build_dsdt(tables_blob, tables->linker, , ,
_hole, _hole64, machine);
 
-/* Count the size of the DSDT and SSDT, we will need it for legacy
- * sizing of ACPI tables.
- */
-aml_len += tables_blob->len - dsdt;
-
 /* ACPI tables pointed to by RSDT */
-fadt = tables_blob->len;
 acpi_add_table(table_offsets, tables_blob);
 pm.fadt.facs_tbl_offset = 
 pm.fadt.dsdt_tbl_offset = 
 pm.fadt.xdsdt_tbl_offset = 
 build_fadt(tables_blob, tables->linker, , oem_id, oem_table_id);
-aml_len += tables_blob->len - fadt;
 
 acpi_add_table(table_offsets, tables_blob);
 acpi_build_madt(tables_blob, tables->linker, x86ms,
@@ -2694,49 +2686,19 @@ void acpi_build(AcpiBuildTables *tables, MachineState 
*machine)
  * too simple to be enough.  4k turned out to be too small an
  * alignment very soon, and in fact it is almost impossible to
  * keep the table size stable for all (max_cpus, max_memory_slots)
- * combinations.  So the table size is always 64k for pc-i440fx-2.1
- * and we give an error if the table grows beyond that limit.
- *
- * We still have the problem of migrating from "-M pc-i440fx-2.0".  For
- * that, we exploit the fact that QEMU 2.1 generates _smaller_ tables
- * than 2.0 and we can always pad the smaller tables with zeros.  We can
- * then use the exact size of the 2.0 tables.
- *
- * All this is for PIIX4, since QEMU 2.0 didn't support Q35 migration.
+ * combinations.
  */
-if (pcmc->legacy_acpi_table_size) {
-/* Subtracting aml_len gives the size of fixed tables.  Then add the
- * size of the PIIX4 DSDT/SSDT in QEMU 2.0.
- */
-int legacy_aml_len =
-pcmc->legacy_acpi_table_size +
-ACPI_BUILD_LEGACY_CPU_AML_SIZE * x86ms->apic_id_limit;
-int legacy_table_size =
-ROUND_UP(tables_blob->len - aml_len + legacy_aml_len,
- ACPI_BUILD_ALIGN_SIZE);
-if ((tables_blob->len > legacy_table_size) &&
-!pcmc->resizable_acpi_blob) {
-/* Should happen only with PCI bridges and -M pc-i440fx-2.0.  */
-warn_report("ACPI table size %u exceeds %d bytes,"
-" migration may not work",
-tables_blob->len, legacy_table_size);
-error_printf("Try removing CPUs, NUMA nodes, memory slots"
- " or PCI bridges.\n");
-}
-g_array_set_size(tables_blob, legacy_table_size);
-} else {
-/* Make sure we have a buffer in case we need to resize the tables. */
-if ((tables_blob->len > ACPI_BUILD_TABLE_SIZE / 2) &&
-!pcmc->resizable_acpi_blob) {
-/* As of QEMU 2.1, this fires with 160 VCPUs and 255 memory slots. 
 */
-warn_report("ACPI table size %u exceeds %d bytes,"
-" migration may not work",
-tables_blob->len, ACPI_BUILD_TABLE_SIZE / 2);
-error_printf("Try removing CPUs, NUMA nodes, memory slots"
- " or PCI bridges.\n");
-}
-acpi_align_size(tables_blob, ACPI_BUILD_TABLE_SIZE);
+/* Make sure we have a buffer in case we need to resize the tables. */
+if ((tables_blob->len > ACPI_BUILD_TABLE_SIZE / 2) &&
+!pcmc->resizable_acpi_blob) {
+/* As of QEMU 2.1, this fires with 160 VCPUs and 255 memory slots.  */
+