From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36489) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZbPGZ-000815-OY for qemu-devel@nongnu.org; Mon, 14 Sep 2015 04:37:36 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZbPGU-0001LZ-OZ for qemu-devel@nongnu.org; Mon, 14 Sep 2015 04:37:35 -0400 Received: from szxga01-in.huawei.com ([58.251.152.64]:42535) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZbPGT-0001K8-U1 for qemu-devel@nongnu.org; Mon, 14 Sep 2015 04:37:30 -0400 Message-ID: <55F6871E.3090903@huawei.com> Date: Mon, 14 Sep 2015 16:36:46 +0800 From: Shannon Zhao MIME-Version: 1.0 References: <1442100642-7258-1-git-send-email-somlo@cmu.edu> <1442100642-7258-4-git-send-email-somlo@cmu.edu> In-Reply-To: <1442100642-7258-4-git-send-email-somlo@cmu.edu> Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC PATCH 3/3] acpi: arm: add fw_cfg device node to dsdt List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Gabriel L. Somlo" , qemu-devel@nongnu.org Cc: peter.maydell@linaro.org, drjones@redhat.com, matt.fleming@intel.com, ehabkost@redhat.com, mst@redhat.com, ard.biesheuvel@linaro.org, leif.lindholm@linaro.org, kevin@koconnor.net, kraxel@redhat.com, pbonzini@redhat.com, imammedo@redhat.com, markmb@redhat.com, lersek@redhat.com, rth@twiddle.net 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 > --- > > 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