| Message ID | 1455228365-13666-4-git-send-email-somlo@cmu.edu |
|---|---|
| State | New |
| Headers | show |
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");
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 --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");