[Qemu-devel] [PATCH v8 0/5] add ACPI node for fw_cfg on pc and arm

[Qemu-devel] [PATCH v8 0/5] add ACPI node for fw_cfg on pc and arm

[Gabriel L. Somlo]

Generate an ACPI DSDT node for fw_cfg on pc and arm guests.

New since v7:

	- edited commit blurb on 3/5 to match updated content, i.e. that
	  the ACPI node is now inserted into the DSDT (no longer the SSDT).
	  (Thanks to Igor Mammedov for catching that!)

Thanks,
  --Gabriel

>New since v6:
>	- rebased to fit on top of fb306ff and f264d36, which moved things
>	  around in pc's acpi-build.c (only patch 3/5 affected);
>	- kernel-side fw_cfg sysfs driver accepted into upstream linux
>
>>New since v5:
>>
>>	- rebased on top of latest QEMU git master
>>
>>>New since v4:
>>>
>>>	- rebased on top of Marc's DMA series
>>>	- drop machine compat dependency for insertion into x86/ssdt
>>>	  (patch 3/5), following agreement between Igor and Eduardo
>>>	- [mm]io register range now covers DMA register as well, if
>>>	  available.
>>>	- s/bios/firmware in doc file updates
>>>
>>>>New since v3:
>>>>
>>>>	- rebased to work on top of 87e896ab (introducing pc-*-25 classes),
>>>>	  inserting fw_cfg acpi node only for machines >= 2.5.
>>>>
>>>>	- reintroduce _STA with value 0x0B (bit 2 for u/i visibility turned
>>>>	  off to avoid Windows complaining -- thanks Igor for catching that!)
>>>>
>>>>If there's any other feedback besides questions regarding the
>>>>appropriateness of "QEMU0002" as the value of _HID, please don't hesitate!
>>>>
>>>>>New since v2:
>>>>>
>>>>>	- pc/i386 node in ssdt only on machine types *newer* than 2.4
>>>>>	  (as suggested by Eduardo)
>>>>>
>>>>>I appreciate any further comments and reviews. Hopefully we can make
>>>>>this palatable for upstream, modulo the lingering concerns about whether
>>>>>"QEMU0002" is ok to use as the value of _HID, which I'll hopefully get
>>>>>sorted out with the kernel crew...
>>>>>
>>>>>>New since v1:
>>>>>>
>>>>>>	- expose control register size (suggested by Marc Marí)
>>>>>>
>>>>>>	- leaving out _UID and _STA fields (thanks Shannon & Igor)
>>>>>>
>>>>>>	- using "QEMU0002" as the value of _HID (thanks Michael)
>>>>>>
>>>>>>	- added documentation blurb to docs/specs/fw_cfg.txt
>>>>>>	  (mainly to record usage of the "QEMU0002" string with fw_cfg).
>>>>>>
>>>>>>> This series adds a fw_cfg device node to the SSDT (on pc), or to the
>>>>>>> DSDT (on arm).
>>>>>>>
>>>>>>> 	- Patch 1/3 moves (and renames) the BIOS_CFG_IOPORT (0x510)
>>>>>>> 	  define from pc.c to pc.h, so that it could be used from
>>>>>>> 	  acpi-build.c in patch 2/3.
>>>>>>> 
>>>>>>> 	- Patch 2/3 adds a fw_cfg node to the pc SSDT.
>>>>>>> 
>>>>>>> 	- Patch 3/3 adds a fw_cfg node to the arm DSDT.
>>>>>>>
>>>>>>> I made up some names - "FWCF" for the node name, and "FWCF0001"
>>>>>>> for _HID; no idea whether that's appropriate, or how else I should
>>>>>>> figure out what to use instead...
>>>>>>>
>>>>>>> Also, using scope "\\_SB", based on where fw_cfg shows up in the
>>>>>>> output of "info qtree". Again, if that's wrong, please point me in
>>>>>>> the right direction.
>>>>>>>
>>>>>>> Re. 3/3 (also mentioned after the commit blurb in the patch itself),
>>>>>>> I noticed none of the other DSDT entries contain a _STA field, wondering
>>>>>>> why it would (not) make sense to include that, same as on the PC.

Gabriel L. Somlo (5):
  fw_cfg: expose control register size in fw_cfg.h
  pc: fw_cfg: move ioport base constant to pc.h
  acpi: pc: add fw_cfg device node to dsdt
  acpi: arm: add fw_cfg device node to dsdt
  fw_cfg: document ACPI device node information

 docs/specs/fw_cfg.txt     |  9 +++++++++
 hw/arm/virt-acpi-build.c  | 15 +++++++++++++++
 hw/i386/acpi-build.c      | 29 +++++++++++++++++++++++++++++
 hw/i386/pc.c              |  5 ++---
 hw/nvram/fw_cfg.c         |  4 +++-
 include/hw/i386/pc.h      |  2 ++
 include/hw/nvram/fw_cfg.h |  3 +++
 7 files changed, 63 insertions(+), 4 deletions(-)

-- 
2.4.3

[Qemu-devel] [PATCH v8 1/5] fw_cfg: expose control register size in fw_cfg.h

[Gabriel L. Somlo]

Expose the size of the control register (FW_CFG_CTL_SIZE) in fw_cfg.h.
Add comment to fw_cfg_io_realize() pointing out that since the
8-bit data register is always subsumed by the 16-bit control
register in the port I/O case, we use the control register width
as the *total* width of the (classic, non-DMA) port I/O region reserved
for the device.

Cc: Marc Marí <markmb@redhat.com>
Signed-off-by: Gabriel Somlo <somlo@cmu.edu>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Marc Marí <markmb@redhat.com>
---
 hw/nvram/fw_cfg.c         | 4 +++-
 include/hw/nvram/fw_cfg.h | 3 +++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index 79c5742..ef2a219 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -32,7 +32,6 @@
 #include "qemu/error-report.h"
 #include "qemu/config-file.h"
 
-#define FW_CFG_CTL_SIZE 2
 #define FW_CFG_NAME "fw_cfg"
 #define FW_CFG_PATH "/machine/" FW_CFG_NAME
 
@@ -882,6 +881,9 @@ static void fw_cfg_io_realize(DeviceState *dev, Error **errp)
     FWCfgIoState *s = FW_CFG_IO(dev);
     SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
 
+    /* when using port i/o, the 8-bit data register ALWAYS overlaps
+     * with half of the 16-bit control register. Hence, the total size
+     * of the i/o region used is FW_CFG_CTL_SIZE */
     memory_region_init_io(&s->comb_iomem, OBJECT(s), &fw_cfg_comb_mem_ops,
                           FW_CFG(s), "fwcfg", FW_CFG_CTL_SIZE);
     sysbus_add_io(sbd, s->iobase, &s->comb_iomem);
diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h
index 664eaf6..2667ca9 100644
--- a/include/hw/nvram/fw_cfg.h
+++ b/include/hw/nvram/fw_cfg.h
@@ -46,6 +46,9 @@
 
 #define FW_CFG_INVALID          0xffff
 
+/* width in bytes of fw_cfg control register */
+#define FW_CFG_CTL_SIZE         0x02
+
 #define FW_CFG_MAX_FILE_PATH    56
 
 #ifndef NO_QEMU_PROTOS
-- 
2.4.3

[Qemu-devel] [PATCH v8 2/5] pc: fw_cfg: move ioport base constant to pc.h

[Gabriel L. Somlo]

Move BIOS_CFG_IOPORT define from pc.c to pc.h, and rename
it to FW_CFG_IO_BASE.

Cc: Marc Marí <markmb@redhat.com>
Signed-off-by: Gabriel Somlo <somlo@cmu.edu>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Marc Marí <markmb@redhat.com>
---
 hw/i386/pc.c         | 5 ++---
 include/hw/i386/pc.h | 2 ++
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 0aeefd2..56ec6cd 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -78,7 +78,6 @@
 #define DPRINTF(fmt, ...)
 #endif
 
-#define BIOS_CFG_IOPORT 0x510
 #define FW_CFG_ACPI_TABLES (FW_CFG_ARCH_LOCAL + 0)
 #define FW_CFG_SMBIOS_ENTRIES (FW_CFG_ARCH_LOCAL + 1)
 #define FW_CFG_IRQ0_OVERRIDE (FW_CFG_ARCH_LOCAL + 2)
@@ -756,7 +755,7 @@ static FWCfgState *bochs_bios_init(AddressSpace *as)
     int i, j;
     unsigned int apic_id_limit = pc_apic_id_limit(max_cpus);
 
-    fw_cfg = fw_cfg_init_io_dma(BIOS_CFG_IOPORT, BIOS_CFG_IOPORT + 4, as);
+    fw_cfg = fw_cfg_init_io_dma(FW_CFG_IO_BASE, FW_CFG_IO_BASE + 4, as);
 
     /* FW_CFG_MAX_CPUS is a bit confusing/problematic on x86:
      *
@@ -1258,7 +1257,7 @@ void xen_load_linux(PCMachineState *pcms)
 
     assert(MACHINE(pcms)->kernel_filename != NULL);
 
-    fw_cfg = fw_cfg_init_io(BIOS_CFG_IOPORT);
+    fw_cfg = fw_cfg_init_io(FW_CFG_IO_BASE);
     rom_set_fw(fw_cfg);
 
     load_linux(pcms, fw_cfg);
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 8b3546e..79ffe5b 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -266,6 +266,8 @@ void ioapic_init_gsi(GSIState *gsi_state, const char *parent_name);
 
 ISADevice *pc_find_fdc0(void);
 
+#define FW_CFG_IO_BASE     0x510
+
 /* acpi_piix.c */
 
 I2CBus *piix4_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base,
-- 
2.4.3

[Qemu-devel] [PATCH v8 3/5] acpi: pc: add fw_cfg device node to dsdt

[Gabriel L. Somlo]

Add a fw_cfg device node to the ACPI DSDT. While the guest-side
firmware can't utilize this information (since it has to access
the hard-coded fw_cfg device to extract ACPI tables to begin with),
having fw_cfg listed in ACPI will help the guest kernel keep a more
accurate inventory of in-use IO port regions.

Signed-off-by: Gabriel Somlo <somlo@cmu.edu>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Marc Marí <markmb@redhat.com>
---
 hw/i386/acpi-build.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 4554eb8..4762fd2 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2190,6 +2190,35 @@ build_dsdt(GArray *table_data, GArray *linker,
     aml_append(scope, aml_name_decl("_S5", pkg));
     aml_append(dsdt, scope);
 
+    /* create fw_cfg node, unconditionally */
+    {
+        /* when using port i/o, the 8-bit data register *always* overlaps
+         * with half of the 16-bit control register. Hence, the total size
+         * of the i/o region used is FW_CFG_CTL_SIZE; when using DMA, the
+         * DMA control register is located at FW_CFG_DMA_IO_BASE + 4 */
+        uint8_t io_size = object_property_get_bool(OBJECT(pcms->fw_cfg),
+                                                   "dma_enabled", NULL) ?
+                          ROUND_UP(FW_CFG_CTL_SIZE, 4) + sizeof(dma_addr_t) :
+                          FW_CFG_CTL_SIZE;
+
+        scope = aml_scope("\\_SB");
+        dev = aml_device("FWCF");
+
+        aml_append(dev, aml_name_decl("_HID", aml_string("QEMU0002")));
+
+        /* device present, functioning, decoding, not shown in UI */
+        aml_append(dev, aml_name_decl("_STA", aml_int(0xB)));
+
+        crs = aml_resource_template();
+        aml_append(crs,
+            aml_io(AML_DECODE16, FW_CFG_IO_BASE, FW_CFG_IO_BASE, 0x01, io_size)
+        );
+        aml_append(dev, aml_name_decl("_CRS", crs));
+
+        aml_append(scope, dev);
+        aml_append(dsdt, scope);
+    }
+
     if (misc->applesmc_io_base) {
         scope = aml_scope("\\_SB.PCI0.ISA");
         dev = aml_device("SMC");
-- 
2.4.3

[Qemu-devel] [PATCH v8 4/5] acpi: arm: add fw_cfg device node to dsdt

[Gabriel L. Somlo]

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 <somlo@cmu.edu>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Marc Marí <markmb@redhat.com>
---
 hw/arm/virt-acpi-build.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 8cf9a21..7d7998b 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -81,6 +81,20 @@ static void acpi_dsdt_add_uart(Aml *scope, const MemMapEntry *uart_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("QEMU0002")));
+    /* device present, functioning, decoding, not shown in UI */
+    aml_append(dev, aml_name_decl("_STA", aml_int(0xB)));
+
+    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;
@@ -548,6 +562,7 @@ build_dsdt(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
     acpi_dsdt_add_uart(scope, &memmap[VIRT_UART],
                        (irqmap[VIRT_UART] + ARM_SPI_BASE));
     acpi_dsdt_add_flash(scope, &memmap[VIRT_FLASH]);
+    acpi_dsdt_add_fw_cfg(scope, &memmap[VIRT_FW_CFG]);
     acpi_dsdt_add_virtio(scope, &memmap[VIRT_MMIO],
                     (irqmap[VIRT_MMIO] + ARM_SPI_BASE), NUM_VIRTIO_TRANSPORTS);
     acpi_dsdt_add_pci(scope, memmap, (irqmap[VIRT_PCIE] + ARM_SPI_BASE),
-- 
2.4.3

[Qemu-devel] [PATCH v8 5/5] fw_cfg: document ACPI device node information

[Gabriel L. Somlo]

Signed-off-by: Gabriel Somlo <somlo@cmu.edu>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Marc Marí <markmb@redhat.com>
---
 docs/specs/fw_cfg.txt | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/docs/specs/fw_cfg.txt b/docs/specs/fw_cfg.txt
index 2099ad9..5414140 100644
--- a/docs/specs/fw_cfg.txt
+++ b/docs/specs/fw_cfg.txt
@@ -84,6 +84,15 @@ Selector Register address: Base + 8 (2 bytes)
 Data Register address:     Base + 0 (8 bytes)
 DMA Address address:       Base + 16 (8 bytes)
 
+== ACPI Interface ==
+
+The fw_cfg device is defined with ACPI ID "QEMU0002". Since we expect
+ACPI tables to be passed into the guest through the fw_cfg device itself,
+the guest-side firmware can not use ACPI to find fw_cfg. However, once the
+firmware is finished setting up ACPI tables and hands control over to the
+guest kernel, the latter can use the fw_cfg ACPI node for a more accurate
+inventory of in-use IOport or MMIO regions.
+
 == Firmware Configuration Items ==
 
 === Signature (Key 0x0000, FW_CFG_SIGNATURE) ===
-- 
2.4.3

Re: [Qemu-devel] [PATCH v8 0/5] add ACPI node for fw_cfg on pc and arm

[Gabriel L. Somlo]

On Thu, Feb 11, 2016 at 05:06:00PM -0500, Gabriel L. Somlo wrote:
> Generate an ACPI DSDT node for fw_cfg on pc and arm guests.
> 
> New since v7:
> 
> 	- edited commit blurb on 3/5 to match updated content, i.e. that
> 	  the ACPI node is now inserted into the DSDT (no longer the SSDT).
> 	  (Thanks to Igor Mammedov for catching that!)

BTW, regarding Igor's question about Windows starting to search for a
driver: Just for grins, I installed Windows 10 (with qemu git master
*before* this series was applied). Then, after applying the series,
DeviceManager was happy and had no unknown hardware listed.

Only after setting the fw_cfg _STA to 0x0F did I get an unknown device
like so: http://imagebin.ca/v/2XDUfONVF3bY

So, on XP, Windows 7, and Windows 10, the fw_cfg device showing up in
ACPI with a _STA set to 0x0B will *NOT* prompt the device manager to 
start making trouble :)

Thanks,
--Gabriel

> >New since v6:
> >	- rebased to fit on top of fb306ff and f264d36, which moved things
> >	  around in pc's acpi-build.c (only patch 3/5 affected);
> >	- kernel-side fw_cfg sysfs driver accepted into upstream linux
> >
> >>New since v5:
> >>
> >>	- rebased on top of latest QEMU git master
> >>
> >>>New since v4:
> >>>
> >>>	- rebased on top of Marc's DMA series
> >>>	- drop machine compat dependency for insertion into x86/ssdt
> >>>	  (patch 3/5), following agreement between Igor and Eduardo
> >>>	- [mm]io register range now covers DMA register as well, if
> >>>	  available.
> >>>	- s/bios/firmware in doc file updates
> >>>
> >>>>New since v3:
> >>>>
> >>>>	- rebased to work on top of 87e896ab (introducing pc-*-25 classes),
> >>>>	  inserting fw_cfg acpi node only for machines >= 2.5.
> >>>>
> >>>>	- reintroduce _STA with value 0x0B (bit 2 for u/i visibility turned
> >>>>	  off to avoid Windows complaining -- thanks Igor for catching that!)
> >>>>
> >>>>If there's any other feedback besides questions regarding the
> >>>>appropriateness of "QEMU0002" as the value of _HID, please don't hesitate!
> >>>>
> >>>>>New since v2:
> >>>>>
> >>>>>	- pc/i386 node in ssdt only on machine types *newer* than 2.4
> >>>>>	  (as suggested by Eduardo)
> >>>>>
> >>>>>I appreciate any further comments and reviews. Hopefully we can make
> >>>>>this palatable for upstream, modulo the lingering concerns about whether
> >>>>>"QEMU0002" is ok to use as the value of _HID, which I'll hopefully get
> >>>>>sorted out with the kernel crew...
> >>>>>
> >>>>>>New since v1:
> >>>>>>
> >>>>>>	- expose control register size (suggested by Marc Marí)
> >>>>>>
> >>>>>>	- leaving out _UID and _STA fields (thanks Shannon & Igor)
> >>>>>>
> >>>>>>	- using "QEMU0002" as the value of _HID (thanks Michael)
> >>>>>>
> >>>>>>	- added documentation blurb to docs/specs/fw_cfg.txt
> >>>>>>	  (mainly to record usage of the "QEMU0002" string with fw_cfg).
> >>>>>>
> >>>>>>> This series adds a fw_cfg device node to the SSDT (on pc), or to the
> >>>>>>> DSDT (on arm).
> >>>>>>>
> >>>>>>> 	- Patch 1/3 moves (and renames) the BIOS_CFG_IOPORT (0x510)
> >>>>>>> 	  define from pc.c to pc.h, so that it could be used from
> >>>>>>> 	  acpi-build.c in patch 2/3.
> >>>>>>> 
> >>>>>>> 	- Patch 2/3 adds a fw_cfg node to the pc SSDT.
> >>>>>>> 
> >>>>>>> 	- Patch 3/3 adds a fw_cfg node to the arm DSDT.
> >>>>>>>
> >>>>>>> I made up some names - "FWCF" for the node name, and "FWCF0001"
> >>>>>>> for _HID; no idea whether that's appropriate, or how else I should
> >>>>>>> figure out what to use instead...
> >>>>>>>
> >>>>>>> Also, using scope "\\_SB", based on where fw_cfg shows up in the
> >>>>>>> output of "info qtree". Again, if that's wrong, please point me in
> >>>>>>> the right direction.
> >>>>>>>
> >>>>>>> Re. 3/3 (also mentioned after the commit blurb in the patch itself),
> >>>>>>> I noticed none of the other DSDT entries contain a _STA field, wondering
> >>>>>>> why it would (not) make sense to include that, same as on the PC.
> 
> Gabriel L. Somlo (5):
>   fw_cfg: expose control register size in fw_cfg.h
>   pc: fw_cfg: move ioport base constant to pc.h
>   acpi: pc: add fw_cfg device node to dsdt
>   acpi: arm: add fw_cfg device node to dsdt
>   fw_cfg: document ACPI device node information
> 
>  docs/specs/fw_cfg.txt     |  9 +++++++++
>  hw/arm/virt-acpi-build.c  | 15 +++++++++++++++
>  hw/i386/acpi-build.c      | 29 +++++++++++++++++++++++++++++
>  hw/i386/pc.c              |  5 ++---
>  hw/nvram/fw_cfg.c         |  4 +++-
>  include/hw/i386/pc.h      |  2 ++
>  include/hw/nvram/fw_cfg.h |  3 +++
>  7 files changed, 63 insertions(+), 4 deletions(-)
> 
> -- 
> 2.4.3
> 

Re: [Qemu-devel] [PATCH v8 0/5] add ACPI node for fw_cfg on pc and arm

[Gerd Hoffmann]

On Di, 2016-02-16 at 18:49 -0500, Gabriel L. Somlo wrote:
> On Thu, Feb 11, 2016 at 05:06:00PM -0500, Gabriel L. Somlo wrote:
> > Generate an ACPI DSDT node for fw_cfg on pc and arm guests.
> > 
> > New since v7:
> > 
> > 	- edited commit blurb on 3/5 to match updated content, i.e. that
> > 	  the ACPI node is now inserted into the DSDT (no longer the SSDT).
> > 	  (Thanks to Igor Mammedov for catching that!)
> 
> BTW, regarding Igor's question about Windows starting to search for a
> driver: Just for grins, I installed Windows 10 (with qemu git master
> *before* this series was applied). Then, after applying the series,
> DeviceManager was happy and had no unknown hardware listed.

Igor?  Could I have a review for the acpi bits?  Then I can prepare a
fw_cfg pull request early next week with this and the fw-cfg-dma
migration fix from laszlo

thanks,
  Gerd

Re: [Qemu-devel] [PATCH v8 3/5] acpi: pc: add fw_cfg device node to dsdt

[Igor Mammedov]

On Thu, 11 Feb 2016 17:06:03 -0500
"Gabriel L. Somlo" <somlo@cmu.edu> wrote:

> Add a fw_cfg device node to the ACPI DSDT. While the guest-side
> firmware can't utilize this information (since it has to access
> the hard-coded fw_cfg device to extract ACPI tables to begin with),
> having fw_cfg listed in ACPI will help the guest kernel keep a more
> accurate inventory of in-use IO port regions.
> 
> Signed-off-by: Gabriel Somlo <somlo@cmu.edu>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> Reviewed-by: Marc Marí <markmb@redhat.com>
> ---
>  hw/i386/acpi-build.c | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 4554eb8..4762fd2 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -2190,6 +2190,35 @@ build_dsdt(GArray *table_data, GArray *linker,
>      aml_append(scope, aml_name_decl("_S5", pkg));
>      aml_append(dsdt, scope);
>  
> +    /* create fw_cfg node, unconditionally */
> +    {
> +        /* when using port i/o, the 8-bit data register *always* overlaps
> +         * with half of the 16-bit control register. Hence, the total size
> +         * of the i/o region used is FW_CFG_CTL_SIZE; when using DMA, the
> +         * DMA control register is located at FW_CFG_DMA_IO_BASE + 4 */
> +        uint8_t io_size = object_property_get_bool(OBJECT(pcms->fw_cfg),
> +                                                   "dma_enabled", NULL) ?
> +                          ROUND_UP(FW_CFG_CTL_SIZE, 4) + sizeof(dma_addr_t) :
> +                          FW_CFG_CTL_SIZE;
> +
> +        scope = aml_scope("\\_SB");
> +        dev = aml_device("FWCF");
> +
> +        aml_append(dev, aml_name_decl("_HID", aml_string("QEMU0002")));
From my tests with different Windows versions, it doesn't prevent
WS2003 and WS2008 from showing a prompt for unknown device that asks for a driver.

One however could shut it up by allowing OS look for a driver
which is not found and then tell  not to ask for it anymore.

So existing users will have to perform this action once
they have migrated to newer QEMU regardless of machine version.

> +
> +        /* device present, functioning, decoding, not shown in UI */
> +        aml_append(dev, aml_name_decl("_STA", aml_int(0xB)));
> +
> +        crs = aml_resource_template();
> +        aml_append(crs,
> +            aml_io(AML_DECODE16, FW_CFG_IO_BASE, FW_CFG_IO_BASE, 0x01, io_size)
> +        );
that creates \_SB.FWCF._CRS 
+            Name (_CRS, ResourceTemplate ()  // _CRS: Current Resource Settings
+            {
+                IO (Decode16,
+                    0x0510,             // Range Minimum
+                    0x0510,             // Range Maximum
+                    0x01,               // Alignment
+                    0x0C,               // Length
+                    )
+            })

which collides with \_SB.PCI0._CRS
            WordIO (ResourceProducer, MinFixed, MaxFixed, PosDecode, EntireRange,
                0x0000,             // Granularity
                0x0000,             // Range Minimum
                0x0CF7,             // Range Maximum
                0x0000,             // Translation Offset
                0x0CF8,

that shouldn't happen, and solution for this is to put
FWCF device descriptor in \_SB.PCI0 scope so FWCF would consume
from PCI0 IO range.

That won't help Windows to notice a conflict, if there is any,
as there aren't any drivers for QEMU0002 device but at least
when Windows gets a driver it won't crash due above conflict.

> +        aml_append(dev, aml_name_decl("_CRS", crs));
> +
> +        aml_append(scope, dev);
> +        aml_append(dsdt, scope);
> +    }
> +
>      if (misc->applesmc_io_base) {
>          scope = aml_scope("\\_SB.PCI0.ISA");
>          dev = aml_device("SMC");

Re: [Qemu-devel] [PATCH v8 3/5] acpi: pc: add fw_cfg device node to dsdt

[Gabriel L. Somlo]

On Fri, Feb 19, 2016 at 02:49:53PM +0100, Igor Mammedov wrote:
> On Thu, 11 Feb 2016 17:06:03 -0500
> "Gabriel L. Somlo" <somlo@cmu.edu> wrote:
> 
> > Add a fw_cfg device node to the ACPI DSDT. While the guest-side
> > firmware can't utilize this information (since it has to access
> > the hard-coded fw_cfg device to extract ACPI tables to begin with),
> > having fw_cfg listed in ACPI will help the guest kernel keep a more
> > accurate inventory of in-use IO port regions.
> > 
> > Signed-off-by: Gabriel Somlo <somlo@cmu.edu>
> > Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> > Reviewed-by: Marc Marí <markmb@redhat.com>
> > ---
> >  hw/i386/acpi-build.c | 29 +++++++++++++++++++++++++++++
> >  1 file changed, 29 insertions(+)
> > 
> > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > index 4554eb8..4762fd2 100644
> > --- a/hw/i386/acpi-build.c
> > +++ b/hw/i386/acpi-build.c
> > @@ -2190,6 +2190,35 @@ build_dsdt(GArray *table_data, GArray *linker,
> >      aml_append(scope, aml_name_decl("_S5", pkg));
> >      aml_append(dsdt, scope);
> >  
> > +    /* create fw_cfg node, unconditionally */
> > +    {
> > +        /* when using port i/o, the 8-bit data register *always* overlaps
> > +         * with half of the 16-bit control register. Hence, the total size
> > +         * of the i/o region used is FW_CFG_CTL_SIZE; when using DMA, the
> > +         * DMA control register is located at FW_CFG_DMA_IO_BASE + 4 */
> > +        uint8_t io_size = object_property_get_bool(OBJECT(pcms->fw_cfg),
> > +                                                   "dma_enabled", NULL) ?
> > +                          ROUND_UP(FW_CFG_CTL_SIZE, 4) + sizeof(dma_addr_t) :
> > +                          FW_CFG_CTL_SIZE;
> > +
> > +        scope = aml_scope("\\_SB");
> > +        dev = aml_device("FWCF");
> > +
> > +        aml_append(dev, aml_name_decl("_HID", aml_string("QEMU0002")));
> From my tests with different Windows versions, it doesn't prevent
> WS2003 and WS2008 from showing a prompt for unknown device that asks for a driver.

I installed windows 2008 server r2 64bit, on both pc and q35 machines,
and still only saw the "unknown device" in the Device Manager after
forcing _STA to 0xF (instead of 0xB).

> One however could shut it up by allowing OS look for a driver
> which is not found and then tell  not to ask for it anymore.

It's entirely possible that "after a while" something would pop up and
ask for a driver anyway. I did try to "humor it" by allowing it to
search for a driver, and it failed, and the device was still showing
up as unknown (remember, this is with _STA set to 0xF, which I did
only to force it to show up :)

I never waited around long enough for it to prompt me, so that may be
the difference between our tests...

> 
> So existing users will have to perform this action once
> they have migrated to newer QEMU regardless of machine version.
> 
> > +
> > +        /* device present, functioning, decoding, not shown in UI */
> > +        aml_append(dev, aml_name_decl("_STA", aml_int(0xB)));
> > +
> > +        crs = aml_resource_template();
> > +        aml_append(crs,
> > +            aml_io(AML_DECODE16, FW_CFG_IO_BASE, FW_CFG_IO_BASE, 0x01, io_size)
> > +        );
> that creates \_SB.FWCF._CRS 
> +            Name (_CRS, ResourceTemplate ()  // _CRS: Current Resource Settings
> +            {
> +                IO (Decode16,
> +                    0x0510,             // Range Minimum
> +                    0x0510,             // Range Maximum
> +                    0x01,               // Alignment
> +                    0x0C,               // Length
> +                    )
> +            })
> 
> which collides with \_SB.PCI0._CRS
>             WordIO (ResourceProducer, MinFixed, MaxFixed, PosDecode, EntireRange,
>                 0x0000,             // Granularity
>                 0x0000,             // Range Minimum
>                 0x0CF7,             // Range Maximum
>                 0x0000,             // Translation Offset
>                 0x0CF8,
> 
> that shouldn't happen, and solution for this is to put
> FWCF device descriptor in \_SB.PCI0 scope so FWCF would consume
> from PCI0 IO range.
> 
> That won't help Windows to notice a conflict, if there is any,
> as there aren't any drivers for QEMU0002 device but at least
> when Windows gets a driver it won't crash due above conflict.

OK, so the difference between "\_SB" and "\_SB.PCI0" is that, with
_STA forced to 0xF, the "resources" tab of the "unknown device" no
longer shows conflicts on the IO range of 510-518.

With that being said, I still hope most people won't ever make it this
far (with _STA set to 0xB). But in case they ever do, yes, placing
FWCF in \_SB.PCI0 should take care of it -- thanks for showing me that!

V9 (coming up soon) should reflect that change.

Thanks much,
--Gabriel

> 
> > +        aml_append(dev, aml_name_decl("_CRS", crs));
> > +
> > +        aml_append(scope, dev);
> > +        aml_append(dsdt, scope);
> > +    }
> > +
> >      if (misc->applesmc_io_base) {
> >          scope = aml_scope("\\_SB.PCI0.ISA");
> >          dev = aml_device("SMC");
>