1. Patchew
  2. QEMU
  3. [PATCH-for-10.1 v4 0/8] hw/arm: GIC 'its=off' ACPI table fixes
  4. View patch

[PATCH v4 1/8] hw/intc/gicv3_its: Do not check its_class_name()

Gustavo Romero posted 8 patches 5 months ago
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, Igor Mammedov <imammedo@redhat.com>, Ani Sinha <anisinha@redhat.com>, Peter Maydell <peter.maydell@linaro.org>, Shannon Zhao <shannon.zhaosl@gmail.com>
There is a newer version of this series
[PATCH v4 1/8] hw/intc/gicv3_its: Do not check its_class_name()
Posted by Gustavo Romero 5 months ago 
Since commit cc5e719e2c8 ("kvm: require KVM_CAP_SIGNAL_MSI"), the single
implementation of its_class_name() no longer returns NULL (it now always
returns a valid char pointer). Hence, update the prototype docstring and
remove the tautological checks that use the its_class_name() returned
value.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
---
 hw/arm/virt-acpi-build.c               | 32 +++++++++++---------------
 include/hw/intc/arm_gicv3_its_common.h |  2 +-
 2 files changed, 15 insertions(+), 19 deletions(-)

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 7e8e0f0298..9eee284c80 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -737,20 +737,18 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
                                           memmap[VIRT_HIGH_GIC_REDIST2].size);
         }
 
-        if (its_class_name()) {
-            /*
-             * ACPI spec, Revision 6.0 Errata A
-             * (original 6.0 definition has invalid Length)
-             * 5.2.12.18 GIC ITS Structure
-             */
-            build_append_int_noprefix(table_data, 0xF, 1);  /* Type */
-            build_append_int_noprefix(table_data, 20, 1);   /* Length */
-            build_append_int_noprefix(table_data, 0, 2);    /* Reserved */
-            build_append_int_noprefix(table_data, 0, 4);    /* GIC ITS ID */
-            /* Physical Base Address */
-            build_append_int_noprefix(table_data, memmap[VIRT_GIC_ITS].base, 8);
-            build_append_int_noprefix(table_data, 0, 4);    /* Reserved */
-        }
+        /*
+         * ACPI spec, Revision 6.0 Errata A
+         * (original 6.0 definition has invalid Length)
+         * 5.2.12.18 GIC ITS Structure
+         */
+        build_append_int_noprefix(table_data, 0xF, 1);  /* Type */
+        build_append_int_noprefix(table_data, 20, 1);   /* Length */
+        build_append_int_noprefix(table_data, 0, 2);    /* Reserved */
+        build_append_int_noprefix(table_data, 0, 4);    /* GIC ITS ID */
+        /* Physical Base Address */
+        build_append_int_noprefix(table_data, memmap[VIRT_GIC_ITS].base, 8);
+        build_append_int_noprefix(table_data, 0, 4);    /* Reserved */
     } else {
         const uint16_t spi_base = vms->irqmap[VIRT_GIC_V2M] + ARM_SPI_BASE;
 
@@ -969,10 +967,8 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables)
                           vms->oem_table_id);
     }
 
-    if (its_class_name()) {
-        acpi_add_table(table_offsets, tables_blob);
-        build_iort(tables_blob, tables->linker, vms);
-    }
+    acpi_add_table(table_offsets, tables_blob);
+    build_iort(tables_blob, tables->linker, vms);
 
 #ifdef CONFIG_TPM
     if (tpm_get_version(tpm_find()) == TPM_VERSION_2_0) {
diff --git a/include/hw/intc/arm_gicv3_its_common.h b/include/hw/intc/arm_gicv3_its_common.h
index 7dc712b38d..3c7b543b01 100644
--- a/include/hw/intc/arm_gicv3_its_common.h
+++ b/include/hw/intc/arm_gicv3_its_common.h
@@ -128,7 +128,7 @@ struct GICv3ITSCommonClass {
  * Return the ITS class name to use depending on whether KVM acceleration
  * and KVM CAP_SIGNAL_MSI are supported
  *
- * Returns: class name to use or NULL
+ * Returns: class name to use
  */
 const char *its_class_name(void);
 
-- 
2.34.1
Re: [PATCH v4 1/8] hw/intc/gicv3_its: Do not check its_class_name()
Posted by Philippe Mathieu-Daudé 5 months ago 
Hi Gustavo, why reset authorship?

On 16/6/25 15:18, Gustavo Romero wrote:
> Since commit cc5e719e2c8 ("kvm: require KVM_CAP_SIGNAL_MSI"), the single
> implementation of its_class_name() no longer returns NULL (it now always
> returns a valid char pointer). Hence, update the prototype docstring and
> remove the tautological checks that use the its_class_name() returned
> value.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Reviewed-by: Eric Auger <eric.auger@redhat.com>
> ---
>   hw/arm/virt-acpi-build.c               | 32 +++++++++++---------------
>   include/hw/intc/arm_gicv3_its_common.h |  2 +-
>   2 files changed, 15 insertions(+), 19 deletions(-)
> 
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 7e8e0f0298..9eee284c80 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -737,20 +737,18 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>                                             memmap[VIRT_HIGH_GIC_REDIST2].size);
>           }
>   
> -        if (its_class_name()) {
> -            /*
> -             * ACPI spec, Revision 6.0 Errata A
> -             * (original 6.0 definition has invalid Length)
> -             * 5.2.12.18 GIC ITS Structure
> -             */
> -            build_append_int_noprefix(table_data, 0xF, 1);  /* Type */
> -            build_append_int_noprefix(table_data, 20, 1);   /* Length */
> -            build_append_int_noprefix(table_data, 0, 2);    /* Reserved */
> -            build_append_int_noprefix(table_data, 0, 4);    /* GIC ITS ID */
> -            /* Physical Base Address */
> -            build_append_int_noprefix(table_data, memmap[VIRT_GIC_ITS].base, 8);
> -            build_append_int_noprefix(table_data, 0, 4);    /* Reserved */
> -        }
> +        /*
> +         * ACPI spec, Revision 6.0 Errata A
> +         * (original 6.0 definition has invalid Length)
> +         * 5.2.12.18 GIC ITS Structure
> +         */
> +        build_append_int_noprefix(table_data, 0xF, 1);  /* Type */
> +        build_append_int_noprefix(table_data, 20, 1);   /* Length */
> +        build_append_int_noprefix(table_data, 0, 2);    /* Reserved */
> +        build_append_int_noprefix(table_data, 0, 4);    /* GIC ITS ID */
> +        /* Physical Base Address */
> +        build_append_int_noprefix(table_data, memmap[VIRT_GIC_ITS].base, 8);
> +        build_append_int_noprefix(table_data, 0, 4);    /* Reserved */
>       } else {
>           const uint16_t spi_base = vms->irqmap[VIRT_GIC_V2M] + ARM_SPI_BASE;
>   
> @@ -969,10 +967,8 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables)
>                             vms->oem_table_id);
>       }
>   
> -    if (its_class_name()) {
> -        acpi_add_table(table_offsets, tables_blob);
> -        build_iort(tables_blob, tables->linker, vms);
> -    }
> +    acpi_add_table(table_offsets, tables_blob);
> +    build_iort(tables_blob, tables->linker, vms);
>   
>   #ifdef CONFIG_TPM
>       if (tpm_get_version(tpm_find()) == TPM_VERSION_2_0) {
> diff --git a/include/hw/intc/arm_gicv3_its_common.h b/include/hw/intc/arm_gicv3_its_common.h
> index 7dc712b38d..3c7b543b01 100644
> --- a/include/hw/intc/arm_gicv3_its_common.h
> +++ b/include/hw/intc/arm_gicv3_its_common.h
> @@ -128,7 +128,7 @@ struct GICv3ITSCommonClass {
>    * Return the ITS class name to use depending on whether KVM acceleration
>    * and KVM CAP_SIGNAL_MSI are supported
>    *
> - * Returns: class name to use or NULL
> + * Returns: class name to use
>    */
>   const char *its_class_name(void);
>
Re: [PATCH v4 1/8] hw/intc/gicv3_its: Do not check its_class_name()
Posted by Gustavo Romero 5 months ago 
Hi Phil,

On 6/16/25 10:33, Philippe Mathieu-Daudé wrote:
> Hi Gustavo, why reset authorship?

It was not intentional. hmm maybe it changed because I tweaked
the commit message and the code due to rebasing as well? Do you
know how to restore it?


Cheers,
Gustavo

> On 16/6/25 15:18, Gustavo Romero wrote:
>> Since commit cc5e719e2c8 ("kvm: require KVM_CAP_SIGNAL_MSI"), the single
>> implementation of its_class_name() no longer returns NULL (it now always
>> returns a valid char pointer). Hence, update the prototype docstring and
>> remove the tautological checks that use the its_class_name() returned
>> value.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>> Reviewed-by: Eric Auger <eric.auger@redhat.com>
>> ---
>>   hw/arm/virt-acpi-build.c               | 32 +++++++++++---------------
>>   include/hw/intc/arm_gicv3_its_common.h |  2 +-
>>   2 files changed, 15 insertions(+), 19 deletions(-)
>>
>> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
>> index 7e8e0f0298..9eee284c80 100644
>> --- a/hw/arm/virt-acpi-build.c
>> +++ b/hw/arm/virt-acpi-build.c
>> @@ -737,20 +737,18 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>>                                             memmap[VIRT_HIGH_GIC_REDIST2].size);
>>           }
>> -        if (its_class_name()) {
>> -            /*
>> -             * ACPI spec, Revision 6.0 Errata A
>> -             * (original 6.0 definition has invalid Length)
>> -             * 5.2.12.18 GIC ITS Structure
>> -             */
>> -            build_append_int_noprefix(table_data, 0xF, 1);  /* Type */
>> -            build_append_int_noprefix(table_data, 20, 1);   /* Length */
>> -            build_append_int_noprefix(table_data, 0, 2);    /* Reserved */
>> -            build_append_int_noprefix(table_data, 0, 4);    /* GIC ITS ID */
>> -            /* Physical Base Address */
>> -            build_append_int_noprefix(table_data, memmap[VIRT_GIC_ITS].base, 8);
>> -            build_append_int_noprefix(table_data, 0, 4);    /* Reserved */
>> -        }
>> +        /*
>> +         * ACPI spec, Revision 6.0 Errata A
>> +         * (original 6.0 definition has invalid Length)
>> +         * 5.2.12.18 GIC ITS Structure
>> +         */
>> +        build_append_int_noprefix(table_data, 0xF, 1);  /* Type */
>> +        build_append_int_noprefix(table_data, 20, 1);   /* Length */
>> +        build_append_int_noprefix(table_data, 0, 2);    /* Reserved */
>> +        build_append_int_noprefix(table_data, 0, 4);    /* GIC ITS ID */
>> +        /* Physical Base Address */
>> +        build_append_int_noprefix(table_data, memmap[VIRT_GIC_ITS].base, 8);
>> +        build_append_int_noprefix(table_data, 0, 4);    /* Reserved */
>>       } else {
>>           const uint16_t spi_base = vms->irqmap[VIRT_GIC_V2M] + ARM_SPI_BASE;
>> @@ -969,10 +967,8 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables)
>>                             vms->oem_table_id);
>>       }
>> -    if (its_class_name()) {
>> -        acpi_add_table(table_offsets, tables_blob);
>> -        build_iort(tables_blob, tables->linker, vms);
>> -    }
>> +    acpi_add_table(table_offsets, tables_blob);
>> +    build_iort(tables_blob, tables->linker, vms);
>>   #ifdef CONFIG_TPM
>>       if (tpm_get_version(tpm_find()) == TPM_VERSION_2_0) {
>> diff --git a/include/hw/intc/arm_gicv3_its_common.h b/include/hw/intc/arm_gicv3_its_common.h
>> index 7dc712b38d..3c7b543b01 100644
>> --- a/include/hw/intc/arm_gicv3_its_common.h
>> +++ b/include/hw/intc/arm_gicv3_its_common.h
>> @@ -128,7 +128,7 @@ struct GICv3ITSCommonClass {
>>    * Return the ITS class name to use depending on whether KVM acceleration
>>    * and KVM CAP_SIGNAL_MSI are supported
>>    *
>> - * Returns: class name to use or NULL
>> + * Returns: class name to use
>>    */
>>   const char *its_class_name(void);
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.