[v2] hw/acpi: bump MADT to revision 5

[PATCH v2 2/4] hw/acpi: arm: bump MADT to revision 5

Posted by Eric DeVolder 2 years, 9 months ago

Currently ARM QEMU generates, and reports, MADT revision 4. ACPI 6.3
introduces MADT revision 5.

For MADT revision 5, the GICC structure adds an SPE Overflow Interrupt
field. This new 2-byte field is created from the existing 3-byte
Reserved field. The spec indicates if the SPE overflow interrupt is
not supported, to zero the field.

Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
---
 hw/arm/virt-acpi-build.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 4156111d49..23268dd981 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -705,7 +705,7 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
     int i;
     VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(vms);
     const MemMapEntry *memmap = vms->memmap;
-    AcpiTable table = { .sig = "APIC", .rev = 4, .oem_id = vms->oem_id,
+    AcpiTable table = { .sig = "APIC", .rev = 5, .oem_id = vms->oem_id,
                         .oem_table_id = vms->oem_table_id };
 
     acpi_table_begin(&table, table_data);
@@ -763,7 +763,9 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
         /* Processor Power Efficiency Class */
         build_append_int_noprefix(table_data, 0, 1);
         /* Reserved */
-        build_append_int_noprefix(table_data, 0, 3);
+        build_append_int_noprefix(table_data, 0, 1);
+        /* SPE overflow Interrupt */
+        build_append_int_noprefix(table_data, 0, 2);
     }
 
     if (vms->gic_version != VIRT_GIC_VERSION_2) {
-- 
2.31.1

Re: [PATCH v2 2/4] hw/acpi: arm: bump MADT to revision 5

Posted by Michael S. Tsirkin 2 years, 9 months ago

On Tue, Apr 18, 2023 at 12:52:17PM -0400, Eric DeVolder wrote:
> Currently ARM QEMU generates, and reports, MADT revision 4. ACPI 6.3
> introduces MADT revision 5.
> 
> For MADT revision 5, the GICC structure adds an SPE Overflow Interrupt
> field. This new 2-byte field is created from the existing 3-byte
> Reserved field. The spec indicates if the SPE overflow interrupt is
> not supported, to zero the field.
> 
> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>

So why do we bother changing this? I'd rather defer until
we actually intend to fill this field.

> ---
>  hw/arm/virt-acpi-build.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 4156111d49..23268dd981 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -705,7 +705,7 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>      int i;
>      VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(vms);
>      const MemMapEntry *memmap = vms->memmap;
> -    AcpiTable table = { .sig = "APIC", .rev = 4, .oem_id = vms->oem_id,
> +    AcpiTable table = { .sig = "APIC", .rev = 5, .oem_id = vms->oem_id,
>                          .oem_table_id = vms->oem_table_id };
>  
>      acpi_table_begin(&table, table_data);
> @@ -763,7 +763,9 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>          /* Processor Power Efficiency Class */
>          build_append_int_noprefix(table_data, 0, 1);
>          /* Reserved */
> -        build_append_int_noprefix(table_data, 0, 3);
> +        build_append_int_noprefix(table_data, 0, 1);
> +        /* SPE overflow Interrupt */
> +        build_append_int_noprefix(table_data, 0, 2);
>      }
>  
>      if (vms->gic_version != VIRT_GIC_VERSION_2) {
> -- 
> 2.31.1

Re: [PATCH v2 2/4] hw/acpi: arm: bump MADT to revision 5

Posted by Eric DeVolder 2 years, 9 months ago


On 4/19/23 00:30, Michael S. Tsirkin wrote:
> On Tue, Apr 18, 2023 at 12:52:17PM -0400, Eric DeVolder wrote:
>> Currently ARM QEMU generates, and reports, MADT revision 4. ACPI 6.3
>> introduces MADT revision 5.
>>
>> For MADT revision 5, the GICC structure adds an SPE Overflow Interrupt
>> field. This new 2-byte field is created from the existing 3-byte
>> Reserved field. The spec indicates if the SPE overflow interrupt is
>> not supported, to zero the field.
>>
>> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
> 
> So why do we bother changing this? I'd rather defer until
> we actually intend to fill this field.

Perfectly reasonable to me. I'll drop the ARM change going forward.
eric

> 
>> ---
>>   hw/arm/virt-acpi-build.c | 6 ++++--
>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
>> index 4156111d49..23268dd981 100644
>> --- a/hw/arm/virt-acpi-build.c
>> +++ b/hw/arm/virt-acpi-build.c
>> @@ -705,7 +705,7 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>>       int i;
>>       VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(vms);
>>       const MemMapEntry *memmap = vms->memmap;
>> -    AcpiTable table = { .sig = "APIC", .rev = 4, .oem_id = vms->oem_id,
>> +    AcpiTable table = { .sig = "APIC", .rev = 5, .oem_id = vms->oem_id,
>>                           .oem_table_id = vms->oem_table_id };
>>   
>>       acpi_table_begin(&table, table_data);
>> @@ -763,7 +763,9 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>>           /* Processor Power Efficiency Class */
>>           build_append_int_noprefix(table_data, 0, 1);
>>           /* Reserved */
>> -        build_append_int_noprefix(table_data, 0, 3);
>> +        build_append_int_noprefix(table_data, 0, 1);
>> +        /* SPE overflow Interrupt */
>> +        build_append_int_noprefix(table_data, 0, 2);
>>       }
>>   
>>       if (vms->gic_version != VIRT_GIC_VERSION_2) {
>> -- 
>> 2.31.1
>