diff mbox

[v8,3/5] acpi: pc: add fw_cfg device node to dsdt

Message ID 1455228365-13666-4-git-send-email-somlo@cmu.edu (mailing list archive)
State New, archived
Headers show

Commit Message

Gabriel L. Somlo Feb. 11, 2016, 10:06 p.m. UTC
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(+)

Comments

Igor Mammedov Feb. 19, 2016, 1:49 p.m. UTC | #1
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");
Gabriel L. Somlo Feb. 19, 2016, 4:59 p.m. UTC | #2
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");
>
diff mbox

Patch

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");