Re: [PATCH v5 02/13] hw/arm/virt-acpi-build.c: Migrate virtio creation to common location

[Sunil V L]

On Thu, Nov 02, 2023 at 09:10:05AM -0300, Daniel Henrique Barboza wrote:
> Alistair, Sunil,
> 
> This patch is breaking riscv-to-apply.next build when using 'clang' and
> --enable-debug:
> 
> URCE=600 -DNCURSES_WIDECHAR=1 -DSTRUCT_IOVEC_DEFINED -MD -MQ 
> libcommon.fa.p/hw_virtio_virtio-acpi.c.o -MF 
> libcommon.fa.p/hw_virtio_virtio-acpi.c.o.d -o 
> libcommon.fa.p/hw_virtio_virtio-acpi.c.o -c ../hw/virtio/virtio-acpi.c
> ../hw/virtio/virtio-acpi.c:14:12: error: variable 'virtio_base' set but not 
> used [-Werror,-Wunused-but-set-variable]
>     hwaddr virtio_base = base;
>            ^
> 1 error generated.
> 
> 
> Looking at the code:
> 
> void virtio_acpi_dsdt_add(Aml *scope, const hwaddr base, const hwaddr size,
>                           uint32_t mmio_irq, long int start_index, int num)
> {
>     long int i;
>     hwaddr virtio_base = base;  <------
> 
>     for (i = start_index; i < start_index + num; i++) {
>         uint32_t irq = mmio_irq + i;
>         Aml *dev = aml_device("VR%02u", (unsigned)i);
>         aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0005")));
>         aml_append(dev, aml_name_decl("_UID", aml_int(i)));
>         aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
> 
>         Aml *crs = aml_resource_template();
>         aml_append(crs, aml_memory32_fixed(base, size, AML_READ_WRITE));
>         aml_append(crs,
>                    aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
>                                  AML_EXCLUSIVE, &irq, 1));
>         aml_append(dev, aml_name_decl("_CRS", crs));
>         aml_append(scope, dev);
>         virtio_base += size;   <------
>     }
> }
> 
> 'virtio_base' is initialized with 'base', and it is incremented in the loop, 
> but
> nothing else is done with it.
> 
> 
> This solves it:
> 
> 
> $ git diff
> diff --git a/hw/virtio/virtio-acpi.c b/hw/virtio/virtio-acpi.c
> index 682283800f..eaf6028e93 100644
> --- a/hw/virtio/virtio-acpi.c
> +++ b/hw/virtio/virtio-acpi.c
> @@ -11,7 +11,6 @@ void virtio_acpi_dsdt_add(Aml *scope, const hwaddr base, 
> const hwaddr size,
>                            uint32_t mmio_irq, long int start_index, int num)
>  {
>      long int i;
> -    hwaddr virtio_base = base;
>      for (i = start_index; i < start_index + num; i++) {
>          uint32_t irq = mmio_irq + i;
> @@ -27,6 +26,5 @@ void virtio_acpi_dsdt_add(Aml *scope, const hwaddr base, 
> const hwaddr size,
>                                   AML_EXCLUSIVE, &irq, 1));
>          aml_append(dev, aml_name_decl("_CRS", crs));
>          aml_append(scope, dev);
> -        virtio_base += size;

Thanks Daniel for catching this. But proper fix would be to use
virtio_base. Let me send next version with proper fix. Sorry about this.

Thanks,
Sunil