Re: [Qemu-devel] [RFC PATCH 3/3] acpi: arm: add fw_cfg device node to dsdt

[Shannon Zhao]

On 2015/9/13 7:30, Gabriel L. Somlo wrote:
> Add a fw_cfg device node to the ACPI DSDT. This is mostly
> informational, as the authoritative fw_cfg MMIO region(s)
> are listed in the Device Tree. However, since we are building
> ACPI tables, we might as well be thorough while at it...
> 
> Signed-off-by: Gabriel Somlo <address@hidden>
> ---
> 
> I used surrounding examples to create acpi_dsdt_add_fw_cfg(), and
> noticed that none add a _STA method, and many include a 0 _UID even
> for nodes with a single instance. I wonder whether 1. I really need
> the _UID, 
The _UID is optional if you has other way to identify it. So for the
nodes with one instance, it's not necessary. But having this is harmless.

and 2. why would we be OK not including a _STA method ?

Look at 6.3.7 _STA (Status) in ACPI Spec 6.0, it says "OSPM evaluates
the _STA object before it evaluates a device _INI method." So if the
device doesn't have _INI method, _STA is not necessary. And "If the _STA
object does not exist for the device, the device is assumed to be both
present and functional."

> 
> Is the #2 answer "because no exisging arm OSPM does in fact check,
> and/or care about the absence of _STA" ? :)
> 
> Thanks,
>  --Gabriel
> 
>  hw/arm/virt-acpi-build.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 9088248..150c9f9 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -110,6 +110,23 @@ static void acpi_dsdt_add_rtc(Aml *scope, const 
> MemMapEntry *rtc_memmap,
>      aml_append(scope, dev);
>  }
>  
> +static void acpi_dsdt_add_fw_cfg(Aml *scope, const MemMapEntry 
> *fw_cfg_memmap)
> +{
> +    Aml *dev = aml_device("FWCF");
> +    aml_append(dev, aml_name_decl("_HID", aml_string("FWCF0001")));
> +
> +    /* FIXME: is this necessary ? */
> +    aml_append(dev, aml_name_decl("_UID", aml_int(0)));
> +    /* FIXME: why doesn't a _STA get added to any other node ? */
> +    aml_append(dev, aml_name_decl("_STA", aml_int(0x0B)));
> +
> +    Aml *crs = aml_resource_template();
> +    aml_append(crs, aml_memory32_fixed(fw_cfg_memmap->base,
> +                                       fw_cfg_memmap->size, AML_READ_WRITE));
> +    aml_append(dev, aml_name_decl("_CRS", crs));
> +    aml_append(scope, dev);
> +}
> +
>  static void acpi_dsdt_add_flash(Aml *scope, const MemMapEntry *flash_memmap)
>  {
>      Aml *dev, *crs;
> @@ -519,6 +536,7 @@ build_dsdt(GArray *table_data, GArray *linker, 
> VirtGuestInfo *guest_info)
>                         (irqmap[VIRT_UART] + ARM_SPI_BASE));
>      acpi_dsdt_add_rtc(scope, &memmap[VIRT_RTC],
>                        (irqmap[VIRT_RTC] + ARM_SPI_BASE));
> +    acpi_dsdt_add_fw_cfg(scope, &memmap[VIRT_FW_CFG]);
>      acpi_dsdt_add_flash(scope, &memmap[VIRT_FLASH]);
>      acpi_dsdt_add_virtio(scope, &memmap[VIRT_MMIO],
>                      (irqmap[VIRT_MMIO] + ARM_SPI_BASE), 
> NUM_VIRTIO_TRANSPORTS);
> 

-- 
Shannon