[PATCH] hw/acpi: limit warning on acpi table size to pc machines older than version 2.3

Ani Sinha posted 1 patch 1 year ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20230320112902.90160-1-anisinha@redhat.com
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, Igor Mammedov <imammedo@redhat.com>, Ani Sinha <ani@anisinha.ca>, Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, Eduardo Habkost <eduardo@habkost.net>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
There is a newer version of this series
hw/i386/acpi-build.c | 6 ++++--
hw/i386/pc.c         | 1 +
hw/i386/pc_piix.c    | 1 +
include/hw/i386/pc.h | 3 +++
4 files changed, 9 insertions(+), 2 deletions(-)
[PATCH] hw/acpi: limit warning on acpi table size to pc machines older than version 2.3
Posted by Ani Sinha 1 year ago
i440fx machine versions 2.3 and newer and q35 machines supports dynamic ram
resizing. Please see commit a1666142db6233 ("acpi-build: make ROMs RAM blocks resizeable") .
Hence the warning when the ACPI table size exceeds a pre-defined value does
not apply to those machines. Add a check limiting the warning message to only
those machines that does not support expandable ram blocks, that is, i440fx
machines with version 2.2 and older.

Signed-off-by: Ani Sinha <anisinha@redhat.com>
---
 hw/i386/acpi-build.c | 6 ++++--
 hw/i386/pc.c         | 1 +
 hw/i386/pc_piix.c    | 1 +
 include/hw/i386/pc.h | 3 +++
 4 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index b19fb4259e..2311bea082 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2616,7 +2616,8 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
         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) {
+        if ((tables_blob->len > legacy_table_size) &&
+            !pcmc->resizable_ram_block) {
             /* 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",
@@ -2627,7 +2628,8 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
         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) {
+        if ((tables_blob->len > ACPI_BUILD_TABLE_SIZE / 2) &&
+            !pcmc->resizable_ram_block) {
             /* 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",
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 7bebea57e3..822d5de333 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1936,6 +1936,7 @@ static void pc_machine_class_init(ObjectClass *oc, void *data)
     pcmc->acpi_data_size = 0x20000 + 0x8000;
     pcmc->pvh_enabled = true;
     pcmc->kvmclock_create_always = true;
+    pcmc->resizable_ram_block = true;
     assert(!mc->get_hotplug_handler);
     mc->get_hotplug_handler = pc_get_hotplug_handler;
     mc->hotplug_allowed = pc_hotplug_allowed;
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 2f16011bab..3c74dfcfb4 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -745,6 +745,7 @@ static void pc_i440fx_2_2_machine_options(MachineClass *m)
     compat_props_add(m->compat_props, hw_compat_2_2, hw_compat_2_2_len);
     compat_props_add(m->compat_props, pc_compat_2_2, pc_compat_2_2_len);
     pcmc->rsdp_in_ram = false;
+    pcmc->resizable_ram_block = false;
 }
 
 DEFINE_I440FX_MACHINE(v2_2, "pc-i440fx-2.2", pc_compat_2_2_fn,
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 8206d5405a..3427a35f73 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -127,6 +127,9 @@ struct PCMachineClass {
 
     /* create kvmclock device even when KVM PV features are not exposed */
     bool kvmclock_create_always;
+
+    /* resizable memory block compat */
+    bool resizable_ram_block;
 };
 
 #define TYPE_PC_MACHINE "generic-pc-machine"
-- 
2.39.2
Re: [PATCH] hw/acpi: limit warning on acpi table size to pc machines older than version 2.3
Posted by Igor Mammedov 1 year ago
On Mon, 20 Mar 2023 16:59:02 +0530
Ani Sinha <anisinha@redhat.com> wrote:

> i440fx machine versions 2.3 and newer and q35 machines supports dynamic ram
> resizing. Please see commit a1666142db6233 ("acpi-build: make ROMs RAM blocks resizeable") .
> Hence the warning when the ACPI table size exceeds a pre-defined value does
> not apply to those machines. Add a check limiting the warning message to only
> those machines that does not support expandable ram blocks, that is, i440fx
> machines with version 2.2 and older.
So q35 is not affected at all? If yes, then mention it here.

> 
> Signed-off-by: Ani Sinha <anisinha@redhat.com>
> ---
>  hw/i386/acpi-build.c | 6 ++++--
>  hw/i386/pc.c         | 1 +
>  hw/i386/pc_piix.c    | 1 +
>  include/hw/i386/pc.h | 3 +++
>  4 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index b19fb4259e..2311bea082 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -2616,7 +2616,8 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
>          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) {
> +        if ((tables_blob->len > legacy_table_size) &&
> +            !pcmc->resizable_ram_block) {
>              /* 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",
> @@ -2627,7 +2628,8 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
>          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) {
> +        if ((tables_blob->len > ACPI_BUILD_TABLE_SIZE / 2) &&
> +            !pcmc->resizable_ram_block) {
>              /* 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",
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 7bebea57e3..822d5de333 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1936,6 +1936,7 @@ static void pc_machine_class_init(ObjectClass *oc, void *data)
>      pcmc->acpi_data_size = 0x20000 + 0x8000;
>      pcmc->pvh_enabled = true;
>      pcmc->kvmclock_create_always = true;
> +    pcmc->resizable_ram_block = true;
>      assert(!mc->get_hotplug_handler);
>      mc->get_hotplug_handler = pc_get_hotplug_handler;
>      mc->hotplug_allowed = pc_hotplug_allowed;
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 2f16011bab..3c74dfcfb4 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -745,6 +745,7 @@ static void pc_i440fx_2_2_machine_options(MachineClass *m)
>      compat_props_add(m->compat_props, hw_compat_2_2, hw_compat_2_2_len);
>      compat_props_add(m->compat_props, pc_compat_2_2, pc_compat_2_2_len);
>      pcmc->rsdp_in_ram = false;
> +    pcmc->resizable_ram_block = false;
>  }
>  
>  DEFINE_I440FX_MACHINE(v2_2, "pc-i440fx-2.2", pc_compat_2_2_fn,
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 8206d5405a..3427a35f73 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -127,6 +127,9 @@ struct PCMachineClass {
>  
>      /* create kvmclock device even when KVM PV features are not exposed */
>      bool kvmclock_create_always;
> +
> +    /* resizable memory block compat */
> +    bool resizable_ram_block;
perhaps more specific
   resizeable_acpi_blob
would be better

>  };
>  
>  #define TYPE_PC_MACHINE "generic-pc-machine"
Re: [PATCH] hw/acpi: limit warning on acpi table size to pc machines older than version 2.3
Posted by Anirban Sinha 1 year ago

> On 28-Mar-2023, at 5:46 PM, Igor Mammedov <imammedo@redhat.com> wrote:
> 
> On Mon, 20 Mar 2023 16:59:02 +0530
> Ani Sinha <anisinha@redhat.com> wrote:
> 
>> i440fx machine versions 2.3 and newer and q35 machines supports dynamic ram
>> resizing. Please see commit a1666142db6233 ("acpi-build: make ROMs RAM blocks resizeable") .
>> Hence the warning when the ACPI table size exceeds a pre-defined value does
>> not apply to those machines. Add a check limiting the warning message to only
>> those machines that does not support expandable ram blocks, that is, i440fx
>> machines with version 2.2 and older.
> So q35 is not affected at all? If yes, then mention it here.

Hmm, initially I thought q35 was unaffected but seems I was mistaken. Looking at 94dec5948aeb240 ("pc: rename machine types”), seems q35 machine types started with version 1.4 and so any version earlier than 2.3 in q35 should also be affected. I will take care of this.

 
> 
>> 
>> Signed-off-by: Ani Sinha <anisinha@redhat.com>
>> ---
>> hw/i386/acpi-build.c | 6 ++++--
>> hw/i386/pc.c         | 1 +
>> hw/i386/pc_piix.c    | 1 +
>> include/hw/i386/pc.h | 3 +++
>> 4 files changed, 9 insertions(+), 2 deletions(-)
>> 
>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>> index b19fb4259e..2311bea082 100644
>> --- a/hw/i386/acpi-build.c
>> +++ b/hw/i386/acpi-build.c
>> @@ -2616,7 +2616,8 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
>>         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) {
>> +        if ((tables_blob->len > legacy_table_size) &&
>> +            !pcmc->resizable_ram_block) {
>>             /* 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",
>> @@ -2627,7 +2628,8 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
>>         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) {
>> +        if ((tables_blob->len > ACPI_BUILD_TABLE_SIZE / 2) &&
>> +            !pcmc->resizable_ram_block) {
>>             /* 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",
>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>> index 7bebea57e3..822d5de333 100644
>> --- a/hw/i386/pc.c
>> +++ b/hw/i386/pc.c
>> @@ -1936,6 +1936,7 @@ static void pc_machine_class_init(ObjectClass *oc, void *data)
>>     pcmc->acpi_data_size = 0x20000 + 0x8000;
>>     pcmc->pvh_enabled = true;
>>     pcmc->kvmclock_create_always = true;
>> +    pcmc->resizable_ram_block = true;
>>     assert(!mc->get_hotplug_handler);
>>     mc->get_hotplug_handler = pc_get_hotplug_handler;
>>     mc->hotplug_allowed = pc_hotplug_allowed;
>> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
>> index 2f16011bab..3c74dfcfb4 100644
>> --- a/hw/i386/pc_piix.c
>> +++ b/hw/i386/pc_piix.c
>> @@ -745,6 +745,7 @@ static void pc_i440fx_2_2_machine_options(MachineClass *m)
>>     compat_props_add(m->compat_props, hw_compat_2_2, hw_compat_2_2_len);
>>     compat_props_add(m->compat_props, pc_compat_2_2, pc_compat_2_2_len);
>>     pcmc->rsdp_in_ram = false;
>> +    pcmc->resizable_ram_block = false;
>> }
>> 
>> DEFINE_I440FX_MACHINE(v2_2, "pc-i440fx-2.2", pc_compat_2_2_fn,
>> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
>> index 8206d5405a..3427a35f73 100644
>> --- a/include/hw/i386/pc.h
>> +++ b/include/hw/i386/pc.h
>> @@ -127,6 +127,9 @@ struct PCMachineClass {
>> 
>>     /* create kvmclock device even when KVM PV features are not exposed */
>>     bool kvmclock_create_always;
>> +
>> +    /* resizable memory block compat */
>> +    bool resizable_ram_block;
> perhaps more specific
>   resizeable_acpi_blob
> would be better

Sure, will rename accordingly.

> 
>> };
>> 
>> #define TYPE_PC_MACHINE "generic-pc-machine"
Re: [PATCH] hw/acpi: limit warning on acpi table size to pc machines older than version 2.3
Posted by Ani Sinha 1 year ago

> On 28-Mar-2023, at 7:35 PM, Anirban Sinha <anisinha@redhat.com> wrote:
> 
> 
> 
>> On 28-Mar-2023, at 5:46 PM, Igor Mammedov <imammedo@redhat.com> wrote:
>> 
>> On Mon, 20 Mar 2023 16:59:02 +0530
>> Ani Sinha <anisinha@redhat.com> wrote:
>> 
>>> i440fx machine versions 2.3 and newer and q35 machines supports dynamic ram
>>> resizing. Please see commit a1666142db6233 ("acpi-build: make ROMs RAM blocks resizeable") .
>>> Hence the warning when the ACPI table size exceeds a pre-defined value does
>>> not apply to those machines. Add a check limiting the warning message to only
>>> those machines that does not support expandable ram blocks, that is, i440fx
>>> machines with version 2.2 and older.
>> So q35 is not affected at all? If yes, then mention it here.
> 
> Hmm, initially I thought q35 was unaffected but seems I was mistaken. Looking at 94dec5948aeb240 ("pc: rename machine types”), seems q35 machine types started with version 1.4 and so any version earlier than 2.3 in q35 should also be affected.

Ah never mind. We have deprecated all q35 machines older than version 2.4:

commit 86165b499edf8b03bb2d0e926d116c2f12a95bfe
Author: Eduardo Habkost <ehabkost@redhat.com>
Date:   Sat Jan 23 14:02:09 2016 -0200

    q35: Remove old machine versions
    
    Migration with q35 was not possible before commit
    04329029a8c539eb5f75dcb6d8b016f0c53a031a, because q35
    unconditionally creates an ich9-ahci device, that was marked as
    unmigratable. So all q35 machine classes before pc-q35-2.4 were
    not migratable, so there's no point in keeping compatibility code
    for them.
    
    Remove all old pc-q35 machine classes and keep only pc-q35-2.4
    and newer.
    
    Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
    Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
    Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
    Reviewed-by: Laszlo Ersek <lersek@redhat.com>
    Reviewed-by: Markus Armbruster <armbru@redhat.com>
    Reviewed-by: Igor Mammedov <imammedo@redhat.com>

So yes q35 is unaffected. I will update the commit log accordingly.


> I will take care of this.
> 
> 
>> 
>>> 
>>> Signed-off-by: Ani Sinha <anisinha@redhat.com>
>>> ---
>>> hw/i386/acpi-build.c | 6 ++++--
>>> hw/i386/pc.c         | 1 +
>>> hw/i386/pc_piix.c    | 1 +
>>> include/hw/i386/pc.h | 3 +++
>>> 4 files changed, 9 insertions(+), 2 deletions(-)
>>> 
>>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>>> index b19fb4259e..2311bea082 100644
>>> --- a/hw/i386/acpi-build.c
>>> +++ b/hw/i386/acpi-build.c
>>> @@ -2616,7 +2616,8 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
>>>        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) {
>>> +        if ((tables_blob->len > legacy_table_size) &&
>>> +            !pcmc->resizable_ram_block) {
>>>            /* 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",
>>> @@ -2627,7 +2628,8 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
>>>        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) {
>>> +        if ((tables_blob->len > ACPI_BUILD_TABLE_SIZE / 2) &&
>>> +            !pcmc->resizable_ram_block) {
>>>            /* 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",
>>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>>> index 7bebea57e3..822d5de333 100644
>>> --- a/hw/i386/pc.c
>>> +++ b/hw/i386/pc.c
>>> @@ -1936,6 +1936,7 @@ static void pc_machine_class_init(ObjectClass *oc, void *data)
>>>    pcmc->acpi_data_size = 0x20000 + 0x8000;
>>>    pcmc->pvh_enabled = true;
>>>    pcmc->kvmclock_create_always = true;
>>> +    pcmc->resizable_ram_block = true;
>>>    assert(!mc->get_hotplug_handler);
>>>    mc->get_hotplug_handler = pc_get_hotplug_handler;
>>>    mc->hotplug_allowed = pc_hotplug_allowed;
>>> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
>>> index 2f16011bab..3c74dfcfb4 100644
>>> --- a/hw/i386/pc_piix.c
>>> +++ b/hw/i386/pc_piix.c
>>> @@ -745,6 +745,7 @@ static void pc_i440fx_2_2_machine_options(MachineClass *m)
>>>    compat_props_add(m->compat_props, hw_compat_2_2, hw_compat_2_2_len);
>>>    compat_props_add(m->compat_props, pc_compat_2_2, pc_compat_2_2_len);
>>>    pcmc->rsdp_in_ram = false;
>>> +    pcmc->resizable_ram_block = false;
>>> }
>>> 
>>> DEFINE_I440FX_MACHINE(v2_2, "pc-i440fx-2.2", pc_compat_2_2_fn,
>>> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
>>> index 8206d5405a..3427a35f73 100644
>>> --- a/include/hw/i386/pc.h
>>> +++ b/include/hw/i386/pc.h
>>> @@ -127,6 +127,9 @@ struct PCMachineClass {
>>> 
>>>    /* create kvmclock device even when KVM PV features are not exposed */
>>>    bool kvmclock_create_always;
>>> +
>>> +    /* resizable memory block compat */
>>> +    bool resizable_ram_block;
>> perhaps more specific
>>  resizeable_acpi_blob
>> would be better
> 
> Sure, will rename accordingly.
> 
>> 
>>> };
>>> 
>>> #define TYPE_PC_MACHINE "generic-pc-machine"
>