From b01a49014a867dc75fdac610c436810ca62b335c Mon Sep 17 00:00:00 2001 From: Laurent Vivier Date: Fri, 12 Feb 2021 14:52:47 +0100 Subject: [PATCH 01/17] pci: cleanup failover sanity check Commit a1190ab628 has added a "allow_unplug_during_migration = true" at the end of the main "if" block, so it is not needed to set it anymore in the previous checking. Remove it, to have only sub-ifs that check for needed conditions and exit if one fails. Fixes: 4f5b6a05a4e7 ("pci: add option for net failover") Fixes: a1190ab628c0 ("migration: allow unplug during migration for failover devices") Cc: jfreimann@redhat.com Signed-off-by: Laurent Vivier Message-Id: <20210212135250.2738750-2-lvivier@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Reviewed-by: Jens Freimann Acked-by: Jason Wang --- hw/pci/pci.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/hw/pci/pci.c b/hw/pci/pci.c index a9ebef8a35..fa97a671d1 100644 --- a/hw/pci/pci.c +++ b/hw/pci/pci.c @@ -2127,10 +2127,8 @@ static void pci_qdev_realize(DeviceState *qdev, Error **errp) pci_qdev_unrealize(DEVICE(pci_dev)); return; } - if (!(pci_dev->cap_present & QEMU_PCI_CAP_MULTIFUNCTION) - && (PCI_FUNC(pci_dev->devfn) == 0)) { - qdev->allow_unplug_during_migration = true; - } else { + if ((pci_dev->cap_present & QEMU_PCI_CAP_MULTIFUNCTION) + || (PCI_FUNC(pci_dev->devfn) != 0)) { error_setg(errp, "failover: primary device must be in its own " "PCI slot"); pci_qdev_unrealize(DEVICE(pci_dev)); From 00e7b1299599384dfdda2a2a4570a0fb2d69eb6b Mon Sep 17 00:00:00 2001 From: Laurent Vivier Date: Fri, 12 Feb 2021 14:52:48 +0100 Subject: [PATCH 02/17] virtio-net: add missing object_unref() failover_add_primary() calls qdev_device_add() and doesn't unref the device. Because of that, when the device is unplugged a reference is remaining and prevents the cleanup of the object. This prevents to be able to plugin back the failover primary device, with errors like: (qemu) device_add vfio-pci,host=0000:41:00.0,id=hostdev0,bus=root.3,failover_pair_id=net0 (qemu) device_del hostdev0 We can check with "info qtree" and "info pci" that the device has been removed, and then: (qemu) device_add vfio-pci,host=0000:41:00.0,id=hostdev1,bus=root.3,failover_pair_id=net0 Error: vfio 0000:41:00.0: device is already attached (qemu) device_add vfio-pci,host=0000:41:00.0,id=hostdev0,bus=root.3,failover_pair_id=net0 qemu-kvm: Duplicate ID 'hostdev0' for device Fixes: 21e8709b29cd ("failover: Remove primary_dev member") Cc: quintela@redhat.com Signed-off-by: Laurent Vivier Message-Id: <20210212135250.2738750-3-lvivier@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Reviewed-by: Jens Freimann --- hw/net/virtio-net.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index 5150f295e8..1c5af08dc5 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -862,6 +862,8 @@ static void failover_add_primary(VirtIONet *n, Error **errp) dev = qdev_device_add(opts, &err); if (err) { qemu_opts_del(opts); + } else { + object_unref(OBJECT(dev)); } } else { error_setg(errp, "Primary device not found"); From 97ca9c5920362d5b7a9f96d4fa758e9f2ccb3301 Mon Sep 17 00:00:00 2001 From: Laurent Vivier Date: Fri, 12 Feb 2021 14:52:49 +0100 Subject: [PATCH 03/17] failover: really display a warning when the primary device is not found In failover_add_primary(), we search the id of the failover device by scanning the list of the devices in the opts list to find a device with a failover_pair_id equals to the id of the virtio-net device. If the failover_pair_id is not found, QEMU ignores the primary device silently (which also means it will not be hidden and it will be enabled directly at boot). After that, we search the id in the opts list to do a qdev_device_add() with it. The device will be always found as otherwise we had exited before, and thus the warning is never displayed. Fix that by moving the error report to the first exit condition. Also add a g_assert() to be sure the compiler will not complain about a possibly NULL pointer. Signed-off-by: Laurent Vivier Message-Id: <20210212135250.2738750-4-lvivier@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/net/virtio-net.c | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index 1c5af08dc5..439f823b19 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -855,21 +855,19 @@ static void failover_add_primary(VirtIONet *n, Error **errp) id = failover_find_primary_device_id(n); if (!id) { - return; - } - opts = qemu_opts_find(qemu_find_opts("device"), id); - if (opts) { - dev = qdev_device_add(opts, &err); - if (err) { - qemu_opts_del(opts); - } else { - object_unref(OBJECT(dev)); - } - } else { error_setg(errp, "Primary device not found"); error_append_hint(errp, "Virtio-net failover will not work. Make " "sure primary device has parameter" - " failover_pair_id=\n"); + " failover_pair_id=%s\n", n->netclient_name); + return; + } + opts = qemu_opts_find(qemu_find_opts("device"), id); + g_assert(opts); /* cannot be NULL because id was found using opts list */ + dev = qdev_device_add(opts, &err); + if (err) { + qemu_opts_del(opts); + } else { + object_unref(OBJECT(dev)); } error_propagate(errp, err); } From df72184ec15829053b3bb5a0d5801773b6d9ec25 Mon Sep 17 00:00:00 2001 From: Laurent Vivier Date: Fri, 12 Feb 2021 14:52:50 +0100 Subject: [PATCH 04/17] pcie: don't set link state active if the slot is empty When the pcie slot is initialized, by default PCI_EXP_LNKSTA_DLLLA (Data Link Layer Link Active) is set in PCI_EXP_LNKSTA (Link Status) without checking if the slot is empty or not. This is confusing for the kernel because as it sees the link is up it tries to read the vendor ID and fails: (From https://bugzilla.kernel.org/show_bug.cgi?id=211691) [ 1.661105] pcieport 0000:00:02.2: pciehp: Slot Capabilities : 0x0002007b [ 1.661115] pcieport 0000:00:02.2: pciehp: Slot Status : 0x0010 [ 1.661123] pcieport 0000:00:02.2: pciehp: Slot Control : 0x07c0 [ 1.661138] pcieport 0000:00:02.2: pciehp: Slot #0 AttnBtn+ PwrCtrl+ MRL- AttnInd+ PwrInd+ HotPlug+ Surprise+ Interlock+ NoCompl- IbPresDis- LLActRep+ [ 1.662581] pcieport 0000:00:02.2: pciehp: pciehp_get_power_status: SLOTCTRL 6c value read 7c0 [ 1.662597] pcieport 0000:00:02.2: pciehp: pciehp_check_link_active: lnk_status = 2204 [ 1.662703] pcieport 0000:00:02.2: pciehp: pending interrupts 0x0010 from Slot Status [ 1.662706] pcieport 0000:00:02.2: pciehp: pcie_enable_notification: SLOTCTRL 6c write cmd 1031 [ 1.662730] pcieport 0000:00:02.2: pciehp: pciehp_check_link_active: lnk_status = 2204 [ 1.662748] pcieport 0000:00:02.2: pciehp: pciehp_check_link_active: lnk_status = 2204 [ 1.662750] pcieport 0000:00:02.2: pciehp: Slot(0-2): Link Up [ 2.896132] pcieport 0000:00:02.2: pciehp: pciehp_check_link_status: lnk_status = 2204 [ 2.896135] pcieport 0000:00:02.2: pciehp: Slot(0-2): No device found [ 2.896900] pcieport 0000:00:02.2: pciehp: pending interrupts 0x0010 from Slot Status [ 2.896903] pcieport 0000:00:02.2: pciehp: pciehp_power_off_slot: SLOTCTRL 6c write cmd 400 [ 3.656901] pcieport 0000:00:02.2: pciehp: pending interrupts 0x0009 from Slot Status This is really a problem with virtio-net failover that hotplugs a VFIO card during the boot process. The kernel can shutdown the slot while QEMU is hotplugging it, and this likely ends by an automatic unplug of the card. At the end of the boot sequence the card has disappeared. To fix that, don't set the "Link Active" state in the init function, but rely on the plug function to do it, as the mechanism has already been introduced by 2f2b18f60bf1. Fixes: 2f2b18f60bf1 ("pcie: set link state inactive/active after hot unplug/plug") Cc: zhengxiang9@huawei.com Fixes: 3d67447fe7c2 ("pcie: Fill PCIESlot link fields to support higher speeds and widths") Cc: alex.williamson@redhat.com Fixes: b2101eae63ea ("pcie: Set the "link active" in the link status register") Cc: benh@kernel.crashing.org Signed-off-by: Laurent Vivier Message-Id: <20210212135250.2738750-5-lvivier@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/pci/pcie.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c index d4010cf8f3..a733e2fb87 100644 --- a/hw/pci/pcie.c +++ b/hw/pci/pcie.c @@ -75,11 +75,6 @@ pcie_cap_v1_fill(PCIDevice *dev, uint8_t port, uint8_t type, uint8_t version) QEMU_PCI_EXP_LNKSTA_NLW(QEMU_PCI_EXP_LNK_X1) | QEMU_PCI_EXP_LNKSTA_CLS(QEMU_PCI_EXP_LNK_2_5GT)); - if (dev->cap_present & QEMU_PCIE_LNKSTA_DLLLA) { - pci_word_test_and_set_mask(exp_cap + PCI_EXP_LNKSTA, - PCI_EXP_LNKSTA_DLLLA); - } - /* We changed link status bits over time, and changing them across * migrations is generally fine as hardware changes them too. * Let's not bother checking. @@ -125,8 +120,7 @@ static void pcie_cap_fill_slot_lnk(PCIDevice *dev) */ pci_long_test_and_set_mask(exp_cap + PCI_EXP_LNKCAP, PCI_EXP_LNKCAP_DLLLARC); - pci_word_test_and_set_mask(exp_cap + PCI_EXP_LNKSTA, - PCI_EXP_LNKSTA_DLLLA); + /* the PCI_EXP_LNKSTA_DLLLA will be set in the hotplug function */ /* * Target Link Speed defaults to the highest link speed supported by @@ -427,6 +421,7 @@ void pcie_cap_slot_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev, PCIDevice *hotplug_pdev = PCI_DEVICE(hotplug_dev); uint8_t *exp_cap = hotplug_pdev->config + hotplug_pdev->exp.exp_cap; PCIDevice *pci_dev = PCI_DEVICE(dev); + uint32_t lnkcap = pci_get_long(exp_cap + PCI_EXP_LNKCAP); /* Don't send event when device is enabled during qemu machine creation: * it is present on boot, no hotplug event is necessary. We do send an @@ -434,7 +429,8 @@ void pcie_cap_slot_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev, if (!dev->hotplugged) { pci_word_test_and_set_mask(exp_cap + PCI_EXP_SLTSTA, PCI_EXP_SLTSTA_PDS); - if (pci_dev->cap_present & QEMU_PCIE_LNKSTA_DLLLA) { + if (pci_dev->cap_present & QEMU_PCIE_LNKSTA_DLLLA || + (lnkcap & PCI_EXP_LNKCAP_DLLLARC)) { pci_word_test_and_set_mask(exp_cap + PCI_EXP_LNKSTA, PCI_EXP_LNKSTA_DLLLA); } @@ -448,7 +444,8 @@ void pcie_cap_slot_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev, if (pci_get_function_0(pci_dev)) { pci_word_test_and_set_mask(exp_cap + PCI_EXP_SLTSTA, PCI_EXP_SLTSTA_PDS); - if (pci_dev->cap_present & QEMU_PCIE_LNKSTA_DLLLA) { + if (pci_dev->cap_present & QEMU_PCIE_LNKSTA_DLLLA || + (lnkcap & PCI_EXP_LNKCAP_DLLLARC)) { pci_word_test_and_set_mask(exp_cap + PCI_EXP_LNKSTA, PCI_EXP_LNKSTA_DLLLA); } @@ -640,6 +637,7 @@ void pcie_cap_slot_write_config(PCIDevice *dev, uint32_t pos = dev->exp.exp_cap; uint8_t *exp_cap = dev->config + pos; uint16_t sltsta = pci_get_word(exp_cap + PCI_EXP_SLTSTA); + uint32_t lnkcap = pci_get_long(exp_cap + PCI_EXP_LNKCAP); if (ranges_overlap(addr, len, pos + PCI_EXP_SLTSTA, 2)) { /* @@ -695,7 +693,8 @@ void pcie_cap_slot_write_config(PCIDevice *dev, pci_word_test_and_clear_mask(exp_cap + PCI_EXP_SLTSTA, PCI_EXP_SLTSTA_PDS); - if (dev->cap_present & QEMU_PCIE_LNKSTA_DLLLA) { + if (dev->cap_present & QEMU_PCIE_LNKSTA_DLLLA || + (lnkcap & PCI_EXP_LNKCAP_DLLLARC)) { pci_word_test_and_clear_mask(exp_cap + PCI_EXP_LNKSTA, PCI_EXP_LNKSTA_DLLLA); } From 451730cfe455a00c6d36fc110ef9b7ab6d646a82 Mon Sep 17 00:00:00 2001 From: Xingang Wang Date: Fri, 5 Feb 2021 01:56:43 +0000 Subject: [PATCH 05/17] acpi: Allow pxb DSDT acpi table changes Signed-off-by: Jiahui Cen Signed-off-by: Xingang Wang Message-Id: <1612490205-48788-2-git-send-email-wangxingang5@huawei.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Reviewed-by: Igor Mammedov --- tests/qtest/bios-tables-test-allowed-diff.h | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h index dfb8523c8b..90c53925fc 100644 --- a/tests/qtest/bios-tables-test-allowed-diff.h +++ b/tests/qtest/bios-tables-test-allowed-diff.h @@ -1 +1,2 @@ /* List of comma-separated changed AML files to ignore */ +"tests/data/acpi/virt/DSDT.pxb", From b48088d60e8466eea2cc517dedb7ee0d97b7feab Mon Sep 17 00:00:00 2001 From: Xingang Wang Date: Fri, 5 Feb 2021 01:56:44 +0000 Subject: [PATCH 06/17] acpi/gpex: Fix cca attribute check for pxb device When check DMA support for device attached to pxb, the cache coherency attribute need to be set. This add _CCA attribute for pxb DSDT. Fixes: 6f9765fbad ("acpi/gpex: Build tables for pxb") Signed-off-by: Jiahui Cen Signed-off-by: Xingang Wang Message-Id: <1612490205-48788-3-git-send-email-wangxingang5@huawei.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Reviewed-by: Igor Mammedov --- hw/pci-host/gpex-acpi.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/pci-host/gpex-acpi.c b/hw/pci-host/gpex-acpi.c index 446912d771..0f01f13a6e 100644 --- a/hw/pci-host/gpex-acpi.c +++ b/hw/pci-host/gpex-acpi.c @@ -175,6 +175,7 @@ void acpi_dsdt_add_gpex(Aml *scope, struct GPEXConfig *cfg) aml_append(dev, aml_name_decl("_BBN", aml_int(bus_num))); aml_append(dev, aml_name_decl("_UID", aml_int(bus_num))); aml_append(dev, aml_name_decl("_STR", aml_unicode("pxb Device"))); + aml_append(dev, aml_name_decl("_CCA", aml_int(1))); if (numa_node != NUMA_NODE_UNASSIGNED) { aml_append(dev, aml_name_decl("_PXM", aml_int(numa_node))); } From 2182e4058c5ac6a6fa24353c110131333c864648 Mon Sep 17 00:00:00 2001 From: Xingang Wang Date: Tue, 23 Feb 2021 10:01:31 -0500 Subject: [PATCH 07/17] tests/data/acpi/virt/DSDT.pxb: update with _CCA Update expected DSDT files accordingly, and re-enable their testing. diff of disassembly of changed expected files: diff -ru -IDisassembly old/tests/data/acpi/virt/DSDT.pxb.dsl new/tests/data/acpi/virt/DSDT.pxb.dsl --- old/tests/data/acpi/virt/DSDT.pxb.dsl 2021-02-23 09:54:18.566781350 -0500 +++ new/tests/data/acpi/virt/DSDT.pxb.dsl 2021-02-23 09:57:51.952816428 -0500 Name (_BBN, 0x80) // _BBN: BIOS Bus Number Name (_UID, 0x80) // _UID: Unique ID Name (_STR, Unicode ("pxb Device")) // _STR: Description String + Name (_CCA, One) // _CCA: Cache Coherency Attribute Name (_PRT, Package (0x80) // _PRT: PCI Routing Table { Package (0x04) Signed-off-by: Jiahui Cen Signed-off-by: Xingang Wang Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- tests/data/acpi/virt/DSDT.pxb | Bin 7689 -> 7695 bytes tests/qtest/bios-tables-test-allowed-diff.h | 1 - 2 files changed, 1 deletion(-) diff --git a/tests/data/acpi/virt/DSDT.pxb b/tests/data/acpi/virt/DSDT.pxb index eaa507b4bba45186d58c03695e46c5bba6a2695a..46b9c4cad5d65cb1b578410fb3168b70a05021be 100644 GIT binary patch delta 40 wcmeCQ>9^r>33dtLmt$aHnm3V4nz3u6nj|Nq=VS-YfX)8GB^;ao33BrQ0MsA~u>b%7 delta 40 vcmeCT>9pZ;33dtLlw)9Ex<8Rinz3c0nj|Nq^JE9kfX&guc^sQrgt&PC)sYG9 diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h index 90c53925fc..dfb8523c8b 100644 --- a/tests/qtest/bios-tables-test-allowed-diff.h +++ b/tests/qtest/bios-tables-test-allowed-diff.h @@ -1,2 +1 @@ /* List of comma-separated changed AML files to ignore */ -"tests/data/acpi/virt/DSDT.pxb", From d2f1af0e41205269bd87e56e133a4353dc8de664 Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Wed, 17 Feb 2021 21:51:09 -0800 Subject: [PATCH 08/17] checkpatch: don't emit warning on newly created acpi data files Newly created acpi data files(tests/data/acpi/) cause false positive warning. If file names are acpi expected file, don't emit warning. Fixes: e625ba2a41 ("checkpatch: fix acpi check with multiple file name") Signed-off-by: Isaku Yamahata Message-Id: <6899f9ad54cab8e7deca94ff0eeab641680e2b5e.1613615732.git.isaku.yamahata@intel.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- scripts/checkpatch.pl | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 7f194c842b..8f7053ec9b 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -1530,7 +1530,9 @@ sub process { ($line =~ /^(?:new|deleted) file mode\s*\d+\s*$/ || $line =~ /^rename (?:from|to) [\w\/\.\-]+\s*$/ || ($line =~ /\{\s*([\w\/\.\-]*)\s*\=\>\s*([\w\/\.\-]*)\s*\}/ && - (defined($1) || defined($2))))) { + (defined($1) || defined($2)))) && + !(($realfile ne '') && + ($realfile eq $acpi_testexpected))) { $reported_maintainer_file = 1; WARN("added, moved or deleted file(s), does MAINTAINERS need updating?\n" . $herecurr); } From 7995d9a399ecb7f509e2862b96bc2dcb249a8d44 Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Wed, 17 Feb 2021 21:51:10 -0800 Subject: [PATCH 09/17] qtest: update tests/qtest/bios-tables-test-allowed-diff.h The following tests will modify acpi tables. prepare qtests to allow acpi table change. add new tables for new tests. - tests/data/acpi/pc/DSDT.nohpet - tests/data/acpi/pc/FACP.nosmm - tests/data/acpi/q35/DSDT.nohpet - tests/data/acpi/q35/FACP.nosmm Acked-by: Igor Mammedov Signed-off-by: Isaku Yamahata Message-Id: Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- tests/data/acpi/pc/DSDT.nohpet | 0 tests/data/acpi/pc/FACP.nosmm | 0 tests/data/acpi/q35/DSDT.nohpet | 0 tests/data/acpi/q35/FACP.nosmm | 0 tests/qtest/bios-tables-test-allowed-diff.h | 14 ++++++++++++++ 5 files changed, 14 insertions(+) create mode 100644 tests/data/acpi/pc/DSDT.nohpet create mode 100644 tests/data/acpi/pc/FACP.nosmm create mode 100644 tests/data/acpi/q35/DSDT.nohpet create mode 100644 tests/data/acpi/q35/FACP.nosmm diff --git a/tests/data/acpi/pc/DSDT.nohpet b/tests/data/acpi/pc/DSDT.nohpet new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/data/acpi/pc/FACP.nosmm b/tests/data/acpi/pc/FACP.nosmm new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/data/acpi/q35/DSDT.nohpet b/tests/data/acpi/q35/DSDT.nohpet new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/data/acpi/q35/FACP.nosmm b/tests/data/acpi/q35/FACP.nosmm new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h index dfb8523c8b..95592459c5 100644 --- a/tests/qtest/bios-tables-test-allowed-diff.h +++ b/tests/qtest/bios-tables-test-allowed-diff.h @@ -1 +1,15 @@ /* List of comma-separated changed AML files to ignore */ +"tests/data/acpi/pc/FACP.nosmm", +"tests/data/acpi/pc/DSDT.nohpet", +"tests/data/acpi/q35/DSDT", +"tests/data/acpi/q35/DSDT.tis", +"tests/data/acpi/q35/DSDT.bridge", +"tests/data/acpi/q35/DSDT.mmio64", +"tests/data/acpi/q35/DSDT.ipmibt", +"tests/data/acpi/q35/DSDT.cphp", +"tests/data/acpi/q35/DSDT.memhp", +"tests/data/acpi/q35/DSDT.numamem", +"tests/data/acpi/q35/FACP.nosmm", +"tests/data/acpi/q35/DSDT.nohpet", +"tests/data/acpi/q35/DSDT.dimmpxm", +"tests/data/acpi/q35/DSDT.acpihmat", From 24cd04fce06b0d54e5ca2c12f20f714894a78b95 Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Wed, 17 Feb 2021 21:51:11 -0800 Subject: [PATCH 10/17] ich9, piix4: add property, smm-compat, to keep compatibility of SMM The following patch will introduce incompatible behavior of SMM. Introduce a property to keep the old behavior for compatibility. To enable smm compat, use "-global ICH9-LPC.smm-compat=on" or "-global PIIX4_PM.smm-compat=on" Suggested-by: Igor Mammedov Signed-off-by: Isaku Yamahata Message-Id: <47254ae0b8c6cc6945422978b6b2af2d213ef891.1613615732.git.isaku.yamahata@intel.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Reviewed-by: Igor Mammedov --- hw/acpi/piix4.c | 2 ++ hw/isa/lpc_ich9.c | 1 + include/hw/acpi/ich9.h | 1 + 3 files changed, 4 insertions(+) diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c index 669be5bbf6..30dd9b2309 100644 --- a/hw/acpi/piix4.c +++ b/hw/acpi/piix4.c @@ -74,6 +74,7 @@ struct PIIX4PMState { qemu_irq irq; qemu_irq smi_irq; int smm_enabled; + bool smm_compat; Notifier machine_ready; Notifier powerdown_notifier; @@ -642,6 +643,7 @@ static Property piix4_pm_properties[] = { use_acpi_root_pci_hotplug, true), DEFINE_PROP_BOOL("memory-hotplug-support", PIIX4PMState, acpi_memory_hotplug.is_enabled, true), + DEFINE_PROP_BOOL("smm-compat", PIIX4PMState, smm_compat, false), DEFINE_PROP_END_OF_LIST(), }; diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c index d3145bf014..3963b73520 100644 --- a/hw/isa/lpc_ich9.c +++ b/hw/isa/lpc_ich9.c @@ -775,6 +775,7 @@ static const VMStateDescription vmstate_ich9_lpc = { static Property ich9_lpc_properties[] = { DEFINE_PROP_BOOL("noreboot", ICH9LPCState, pin_strap.spkr_hi, true), + DEFINE_PROP_BOOL("smm-compat", ICH9LPCState, pm.smm_compat, false), DEFINE_PROP_BIT64("x-smi-broadcast", ICH9LPCState, smi_host_features, ICH9_LPC_SMI_F_BROADCAST_BIT, true), DEFINE_PROP_BIT64("x-smi-cpu-hotplug", ICH9LPCState, smi_host_features, diff --git a/include/hw/acpi/ich9.h b/include/hw/acpi/ich9.h index 54571c77e0..df519e40b5 100644 --- a/include/hw/acpi/ich9.h +++ b/include/hw/acpi/ich9.h @@ -59,6 +59,7 @@ typedef struct ICH9LPCPMRegs { uint8_t disable_s4; uint8_t s4_val; uint8_t smm_enabled; + bool smm_compat; bool enable_tco; TCOIORegs tco_regs; } ICH9LPCPMRegs; From 6be8cf56bc8bda2ed9a070bdb04446191f31acc9 Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Wed, 17 Feb 2021 21:51:12 -0800 Subject: [PATCH 11/17] acpi/core: always set SCI_EN when SMM isn't supported If SMM is not supported, ACPI fixed hardware doesn't support legacy-mode. ACPI-only platform. Where SCI_EN in PM1_CNT register is always set. The bit tells OS legacy mode(SCI_EN cleared) or ACPI mode(SCI_EN set). With the next patch (setting fadt.smi_cmd = 0 when smm isn't enabled), guest Linux tries to switch to ACPI mode, finds smi_cmd = 0, and then fails to initialize acpi subsystem. This patch proactively fixes it. This patch changes guest ABI. To keep compatibility, use "smm-compat" introduced by earlier patch. If the property is true, disable new behavior. ACPI spec 4.8.10.1 PM1 Event Grouping PM1 Eanble Registers > For ACPI-only platforms (where SCI_EN is always set) Reviewed-by: Igor Mammedov Signed-off-by: Isaku Yamahata Message-Id: <500f62081626997e46f96377393d3662211763a8.1613615732.git.isaku.yamahata@intel.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/acpi/core.c | 11 ++++++++++- hw/acpi/ich9.c | 2 +- hw/acpi/piix4.c | 3 ++- hw/core/machine.c | 5 ++++- hw/isa/vt82c686.c | 2 +- include/hw/acpi/acpi.h | 4 +++- 6 files changed, 21 insertions(+), 6 deletions(-) diff --git a/hw/acpi/core.c b/hw/acpi/core.c index 7170bff657..1e004d0078 100644 --- a/hw/acpi/core.c +++ b/hw/acpi/core.c @@ -579,6 +579,10 @@ void acpi_pm1_cnt_update(ACPIREGS *ar, bool sci_enable, bool sci_disable) { /* ACPI specs 3.0, 4.7.2.5 */ + if (ar->pm1.cnt.acpi_only) { + return; + } + if (sci_enable) { ar->pm1.cnt.cnt |= ACPI_BITMASK_SCI_ENABLE; } else if (sci_disable) { @@ -608,11 +612,13 @@ static const MemoryRegionOps acpi_pm_cnt_ops = { }; void acpi_pm1_cnt_init(ACPIREGS *ar, MemoryRegion *parent, - bool disable_s3, bool disable_s4, uint8_t s4_val) + bool disable_s3, bool disable_s4, uint8_t s4_val, + bool acpi_only) { FWCfgState *fw_cfg; ar->pm1.cnt.s4_val = s4_val; + ar->pm1.cnt.acpi_only = acpi_only; ar->wakeup.notify = acpi_notify_wakeup; qemu_register_wakeup_notifier(&ar->wakeup); @@ -638,6 +644,9 @@ void acpi_pm1_cnt_init(ACPIREGS *ar, MemoryRegion *parent, void acpi_pm1_cnt_reset(ACPIREGS *ar) { ar->pm1.cnt.cnt = 0; + if (ar->pm1.cnt.acpi_only) { + ar->pm1.cnt.cnt |= ACPI_BITMASK_SCI_ENABLE; + } } /* ACPI GPE */ diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c index 5ff4e01c36..853447cf9d 100644 --- a/hw/acpi/ich9.c +++ b/hw/acpi/ich9.c @@ -282,7 +282,7 @@ void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm, acpi_pm_tmr_init(&pm->acpi_regs, ich9_pm_update_sci_fn, &pm->io); acpi_pm1_evt_init(&pm->acpi_regs, ich9_pm_update_sci_fn, &pm->io); acpi_pm1_cnt_init(&pm->acpi_regs, &pm->io, pm->disable_s3, pm->disable_s4, - pm->s4_val); + pm->s4_val, !pm->smm_compat && !smm_enabled); acpi_gpe_init(&pm->acpi_regs, ICH9_PMIO_GPE0_LEN); memory_region_init_io(&pm->io_gpe, OBJECT(lpc_pci), &ich9_gpe_ops, pm, diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c index 30dd9b2309..1efc0ded9f 100644 --- a/hw/acpi/piix4.c +++ b/hw/acpi/piix4.c @@ -497,7 +497,8 @@ static void piix4_pm_realize(PCIDevice *dev, Error **errp) acpi_pm_tmr_init(&s->ar, pm_tmr_timer, &s->io); acpi_pm1_evt_init(&s->ar, pm_tmr_timer, &s->io); - acpi_pm1_cnt_init(&s->ar, &s->io, s->disable_s3, s->disable_s4, s->s4_val); + acpi_pm1_cnt_init(&s->ar, &s->io, s->disable_s3, s->disable_s4, s->s4_val, + !s->smm_compat && !s->smm_enabled); acpi_gpe_init(&s->ar, GPE_LEN); s->powerdown_notifier.notify = piix4_pm_powerdown_req; diff --git a/hw/core/machine.c b/hw/core/machine.c index 970046f438..4386f57b5c 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -36,7 +36,10 @@ #include "hw/virtio/virtio.h" #include "hw/virtio/virtio-pci.h" -GlobalProperty hw_compat_5_2[] = {}; +GlobalProperty hw_compat_5_2[] = { + { "ICH9-LPC", "smm-compat", "on"}, + { "PIIX4_PM", "smm-compat", "on"}, +}; const size_t hw_compat_5_2_len = G_N_ELEMENTS(hw_compat_5_2); GlobalProperty hw_compat_5_1[] = { diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c index 5db9b1706c..05d084f698 100644 --- a/hw/isa/vt82c686.c +++ b/hw/isa/vt82c686.c @@ -190,7 +190,7 @@ static void via_pm_realize(PCIDevice *dev, Error **errp) acpi_pm_tmr_init(&s->ar, pm_tmr_timer, &s->io); acpi_pm1_evt_init(&s->ar, pm_tmr_timer, &s->io); - acpi_pm1_cnt_init(&s->ar, &s->io, false, false, 2); + acpi_pm1_cnt_init(&s->ar, &s->io, false, false, 2, false); } typedef struct via_pm_init_info { diff --git a/include/hw/acpi/acpi.h b/include/hw/acpi/acpi.h index 22b0b65bb2..9e8a76f2e2 100644 --- a/include/hw/acpi/acpi.h +++ b/include/hw/acpi/acpi.h @@ -128,6 +128,7 @@ struct ACPIPM1CNT { MemoryRegion io; uint16_t cnt; uint8_t s4_val; + bool acpi_only; }; struct ACPIGPE { @@ -163,7 +164,8 @@ void acpi_pm1_evt_init(ACPIREGS *ar, acpi_update_sci_fn update_sci, /* PM1a_CNT: piix and ich9 don't implement PM1b CNT. */ void acpi_pm1_cnt_init(ACPIREGS *ar, MemoryRegion *parent, - bool disable_s3, bool disable_s4, uint8_t s4_val); + bool disable_s3, bool disable_s4, uint8_t s4_val, + bool acpi_only); void acpi_pm1_cnt_update(ACPIREGS *ar, bool sci_enable, bool sci_disable); void acpi_pm1_cnt_reset(ACPIREGS *ar); From 33b44fdaba54a99e94b604dafb0d5fcaa8a35261 Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Wed, 17 Feb 2021 21:51:13 -0800 Subject: [PATCH 12/17] acpi: set fadt.smi_cmd to zero when SMM is not supported >From table 5.9 SMI_CMD of ACPI spec > This field is reserved and must be zero on system > that does not support System Management mode. When smm is not enabled, set it to zero to comform to the spec. When -machine smm=off is passed, the change to FACP is as follows. @@ -1,46 +1,46 @@ /* * Intel ACPI Component Architecture * AML/ASL+ Disassembler version 20180105 (64-bit version) * Copyright (c) 2000 - 2018 Intel Corporation * - * Disassembly of tests/data/acpi/q35/FACP, Fri Feb 5 16:57:04 2021 + * Disassembly of /tmp/aml-1OQYX0, Fri Feb 5 16:57:04 2021 * * ACPI Data Table [FACP] * * Format: [HexOffset DecimalOffset ByteLength] FieldName : FieldValue */ [000h 0000 4] Signature : "FACP" [Fixed ACPI Description Table (FADT)] [004h 0004 4] Table Length : 000000F4 [008h 0008 1] Revision : 03 -[009h 0009 1] Checksum : 1F +[009h 0009 1] Checksum : D6 [00Ah 0010 6] Oem ID : "BOCHS " [010h 0016 8] Oem Table ID : "BXPCFACP" [018h 0024 4] Oem Revision : 00000001 [01Ch 0028 4] Asl Compiler ID : "BXPC" [020h 0032 4] Asl Compiler Revision : 00000001 [024h 0036 4] FACS Address : 00000000 [028h 0040 4] DSDT Address : 00000000 [02Ch 0044 1] Model : 01 [02Dh 0045 1] PM Profile : 00 [Unspecified] [02Eh 0046 2] SCI Interrupt : 0009 -[030h 0048 4] SMI Command Port : 000000B2 -[034h 0052 1] ACPI Enable Value : 02 -[035h 0053 1] ACPI Disable Value : 03 +[030h 0048 4] SMI Command Port : 00000000 +[034h 0052 1] ACPI Enable Value : 00 +[035h 0053 1] ACPI Disable Value : 00 [036h 0054 1] S4BIOS Command : 00 [037h 0055 1] P-State Control : 00 [038h 0056 4] PM1A Event Block Address : 00000600 [03Ch 0060 4] PM1B Event Block Address : 00000000 [040h 0064 4] PM1A Control Block Address : 00000604 [044h 0068 4] PM1B Control Block Address : 00000000 [048h 0072 4] PM2 Control Block Address : 00000000 [04Ch 0076 4] PM Timer Block Address : 00000608 [050h 0080 4] GPE0 Block Address : 00000620 [054h 0084 4] GPE1 Block Address : 00000000 [058h 0088 1] PM1 Event Block Length : 04 [059h 0089 1] PM1 Control Block Length : 02 [05Ah 0090 1] PM2 Control Block Length : 00 [05Bh 0091 1] PM Timer Block Length : 04 [05Ch 0092 1] GPE0 Block Length : 10 [05Dh 0093 1] GPE1 Block Length : 00 Reviewed-by: Igor Mammedov Signed-off-by: Isaku Yamahata Message-Id: <09ed791ef77fda2b194100669cbc690865c9eb52.1613615732.git.isaku.yamahata@intel.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/i386/acpi-build.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index b9190b924a..49aef4ebd1 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -139,6 +139,14 @@ const struct AcpiGenericAddress x86_nvdimm_acpi_dsmio = { static void init_common_fadt_data(MachineState *ms, Object *o, AcpiFadtData *data) { + X86MachineState *x86ms = X86_MACHINE(ms); + /* + * "ICH9-LPC" or "PIIX4_PM" has "smm-compat" property to keep the old + * behavior for compatibility irrelevant to smm_enabled, which doesn't + * comforms to ACPI spec. + */ + bool smm_enabled = object_property_get_bool(o, "smm-compat", NULL) ? + true : x86_machine_is_smm_enabled(x86ms); uint32_t io = object_property_get_uint(o, ACPI_PM_PROP_PM_IO_BASE, NULL); AmlAddressSpace as = AML_AS_SYSTEM_IO; AcpiFadtData fadt = { @@ -159,12 +167,16 @@ static void init_common_fadt_data(MachineState *ms, Object *o, .rtc_century = RTC_CENTURY, .plvl2_lat = 0xfff /* C2 state not supported */, .plvl3_lat = 0xfff /* C3 state not supported */, - .smi_cmd = ACPI_PORT_SMI_CMD, + .smi_cmd = smm_enabled ? ACPI_PORT_SMI_CMD : 0, .sci_int = object_property_get_uint(o, ACPI_PM_PROP_SCI_INT, NULL), .acpi_enable_cmd = - object_property_get_uint(o, ACPI_PM_PROP_ACPI_ENABLE_CMD, NULL), + smm_enabled ? + object_property_get_uint(o, ACPI_PM_PROP_ACPI_ENABLE_CMD, NULL) : + 0, .acpi_disable_cmd = - object_property_get_uint(o, ACPI_PM_PROP_ACPI_DISABLE_CMD, NULL), + smm_enabled ? + object_property_get_uint(o, ACPI_PM_PROP_ACPI_DISABLE_CMD, NULL) : + 0, .pm1a_evt = { .space_id = as, .bit_width = 4 * 8, .address = io }, .pm1a_cnt = { .space_id = as, .bit_width = 2 * 8, .address = io + 0x04 }, From 0dabb2e802437c2e578dc72bd0bdf3380a25ec96 Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Wed, 17 Feb 2021 21:51:14 -0800 Subject: [PATCH 13/17] acpi: add test case for smm unsupported -machine smm=off Reviewed-by: Igor Mammedov Signed-off-by: Isaku Yamahata Message-Id: <22f774a51255af1608b07b00b257af426adcf4ab.1613615732.git.isaku.yamahata@intel.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- tests/qtest/bios-tables-test.c | 76 ++++++++++++++++++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c index 77053975aa..93d037c29d 100644 --- a/tests/qtest/bios-tables-test.c +++ b/tests/qtest/bios-tables-test.c @@ -973,6 +973,39 @@ static void test_acpi_piix4_tcg_memhp(void) free_test_data(&data); } +static void test_acpi_piix4_tcg_nosmm(void) +{ + test_data data; + + memset(&data, 0, sizeof(data)); + data.machine = MACHINE_PC; + data.variant = ".nosmm"; + test_acpi_one("-machine smm=off", &data); + free_test_data(&data); +} + +static void test_acpi_piix4_tcg_smm_compat(void) +{ + test_data data; + + memset(&data, 0, sizeof(data)); + data.machine = MACHINE_PC; + data.variant = ".smm-compat"; + test_acpi_one("-global PIIX4_PM.smm-compat=on", &data); + free_test_data(&data); +} + +static void test_acpi_piix4_tcg_smm_compat_nosmm(void) +{ + test_data data; + + memset(&data, 0, sizeof(data)); + data.machine = MACHINE_PC; + data.variant = ".smm-compat-nosmm"; + test_acpi_one("-global PIIX4_PM.smm-compat=on -machine smm=off", &data); + free_test_data(&data); +} + static void test_acpi_q35_tcg_numamem(void) { test_data data; @@ -985,6 +1018,39 @@ static void test_acpi_q35_tcg_numamem(void) free_test_data(&data); } +static void test_acpi_q35_tcg_nosmm(void) +{ + test_data data; + + memset(&data, 0, sizeof(data)); + data.machine = MACHINE_Q35; + data.variant = ".nosmm"; + test_acpi_one("-machine smm=off", &data); + free_test_data(&data); +} + +static void test_acpi_q35_tcg_smm_compat(void) +{ + test_data data; + + memset(&data, 0, sizeof(data)); + data.machine = MACHINE_Q35; + data.variant = ".smm-compat"; + test_acpi_one("-global ICH9-LPC.smm-compat=on", &data); + free_test_data(&data); +} + +static void test_acpi_q35_tcg_smm_compat_nosmm(void) +{ + test_data data; + + memset(&data, 0, sizeof(data)); + data.machine = MACHINE_Q35; + data.variant = ".smm-compat-nosmm"; + test_acpi_one("-global ICH9-LPC.smm-compat=on -machine smm=off", &data); + free_test_data(&data); +} + static void test_acpi_piix4_tcg_numamem(void) { test_data data; @@ -1445,6 +1511,16 @@ int main(int argc, char *argv[]) qtest_add_func("acpi/q35/memhp", test_acpi_q35_tcg_memhp); qtest_add_func("acpi/piix4/numamem", test_acpi_piix4_tcg_numamem); qtest_add_func("acpi/q35/numamem", test_acpi_q35_tcg_numamem); + qtest_add_func("acpi/piix4/nosmm", test_acpi_piix4_tcg_nosmm); + qtest_add_func("acpi/piix4/smm-compat", + test_acpi_piix4_tcg_smm_compat); + qtest_add_func("acpi/piix4/smm-compat-nosmm", + test_acpi_piix4_tcg_smm_compat_nosmm); + qtest_add_func("acpi/q35/nosmm", test_acpi_q35_tcg_nosmm); + qtest_add_func("acpi/q35/smm-compat", + test_acpi_q35_tcg_smm_compat); + qtest_add_func("acpi/q35/smm-compat-nosmm", + test_acpi_q35_tcg_smm_compat_nosmm); qtest_add_func("acpi/piix4/dimmpxm", test_acpi_piix4_tcg_dimm_pxm); qtest_add_func("acpi/q35/dimmpxm", test_acpi_q35_tcg_dimm_pxm); qtest_add_func("acpi/piix4/acpihmat", test_acpi_piix4_tcg_acpi_hmat); From e3fb55f06501b7cffd1df2223eeadaa60e25565d Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Wed, 17 Feb 2021 21:51:15 -0800 Subject: [PATCH 14/17] hw/i386: declare ACPI mother board resource for MMCONFIG region Declare PNP0C01 device to reserve MMCONFIG region to conform to the spec better and play nice with guest BIOSes/OSes. According to PCI Firmware Specification[0], MMCONFIG region must be reserved by declaring a motherboard resource. It's optional to reserve the region in memory map by Int 15 E820h or EFIGetMemoryMap. Guest Linux checks if the MMCFG region is reserved by bios memory map or ACPI resource. If it's not reserved, Linux falls back to legacy PCI configuration access. TDVF [1] [2] doesn't reserve MMCONFIG the region in memory map. On the other hand OVMF reserves it in memory map without declaring a motherboard resource. With memory map reservation, linux guest uses MMCONFIG region. However it doesn't comply to PCI Firmware specification. [0] PCI Firmware specification Revision 3.2 4.1.2 MCFG Table Description table 4-2 NOTE 2 If the operating system does not natively comprehend reserving the MMCFG region, The MMCFG region must e reserved by firmware. ... For most systems, the mortheroard resource would appear at the root of the ACPI namespace (under \_SB)... The resource can optionally be returned in Int15 E820h or EFIGetMemoryMap as reserved memory but must always be reported through ACPI as a motherboard resource [1] TDX: Intel Trust Domain Extension https://software.intel.com/content/www/us/en/develop/articles/intel-trust-domain-extensions.html [2] TDX Virtual Firmware https://github.com/tianocore/edk2-staging/tree/TDVF The change to DSDT is as follows. @@ -68,32 +68,47 @@ If ((CDW3 != Local0)) { CDW1 |= 0x10 } CDW3 = Local0 } Else { CDW1 |= 0x04 } Return (Arg3) } } + + Device (DRAC) + { + Name (_HID, "PNP0C01" /* System Board */) // _HID: Hardware ID + Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings + { + DWordMemory (ResourceProducer, PosDecode, MinFixed, MaxFixed, NonCacheable, ReadWrite, + 0x00000000, // Granularity + 0xB0000000, // Range Minimum + 0xBFFFFFFF, // Range Maximum + 0x00000000, // Translation Offset + 0x10000000, // Length + ,, , AddressRangeMemory, TypeStatic) + }) + } } Scope (_SB) { Device (HPET) { Name (_HID, EisaId ("PNP0103") /* HPET System Timer */) // _HID: Hardware ID Name (_UID, Zero) // _UID: Unique ID OperationRegion (HPTM, SystemMemory, 0xFED00000, 0x0400) Field (HPTM, DWordAcc, Lock, Preserve) { VEND, 32, PRD, 32 } Method (_STA, 0, NotSerialized) // _STA: Status Signed-off-by: Isaku Yamahata Message-Id: <6f686b45ce7bc43048c56dbb46e72e1fe51927e6.1613615732.git.isaku.yamahata@intel.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Reviewed-by: Igor Mammedov --- hw/i386/acpi-build.c | 46 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-) diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index 49aef4ebd1..96497475d1 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -1072,6 +1072,46 @@ static void build_q35_pci0_int(Aml *table) aml_append(table, sb_scope); } +static Aml *build_q35_dram_controller(const AcpiMcfgInfo *mcfg) +{ + Aml *dev; + Aml *resource_template; + + /* DRAM controller */ + dev = aml_device("DRAC"); + aml_append(dev, aml_name_decl("_HID", aml_string("PNP0C01"))); + + resource_template = aml_resource_template(); + if (mcfg->base + mcfg->size - 1 >= (1ULL << 32)) { + aml_append(resource_template, + aml_qword_memory(AML_POS_DECODE, + AML_MIN_FIXED, + AML_MAX_FIXED, + AML_NON_CACHEABLE, + AML_READ_WRITE, + 0x0000000000000000, + mcfg->base, + mcfg->base + mcfg->size - 1, + 0x0000000000000000, + mcfg->size)); + } else { + aml_append(resource_template, + aml_dword_memory(AML_POS_DECODE, + AML_MIN_FIXED, + AML_MAX_FIXED, + AML_NON_CACHEABLE, + AML_READ_WRITE, + 0x0000000000000000, + mcfg->base, + mcfg->base + mcfg->size - 1, + 0x0000000000000000, + mcfg->size)); + } + aml_append(dev, aml_name_decl("_CRS", resource_template)); + + return dev; +} + static void build_q35_isa_bridge(Aml *table) { Aml *dev; @@ -1218,6 +1258,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(machine); X86MachineState *x86ms = X86_MACHINE(machine); AcpiMcfgInfo mcfg; + bool mcfg_valid = !!acpi_get_mcfg(&mcfg); uint32_t nr_mem = machine->ram_slots; int root_bus_limit = 0xFF; PCIBus *bus = NULL; @@ -1256,6 +1297,9 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, aml_append(dev, aml_name_decl("_UID", aml_int(0))); aml_append(dev, build_q35_osc_method()); aml_append(sb_scope, dev); + if (mcfg_valid) { + aml_append(sb_scope, build_q35_dram_controller(&mcfg)); + } if (pm->smi_on_cpuhp) { /* reserve SMI block resources, IO ports 0xB2, 0xB3 */ @@ -1386,7 +1430,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, * the PCI0._CRS. Add mmconfig to the set so it will be excluded * too. */ - if (acpi_get_mcfg(&mcfg)) { + if (mcfg_valid) { crs_range_insert(crs_range_set.mem_ranges, mcfg.base, mcfg.base + mcfg.size - 1); } From 51124bbfd2ea31ab4d9f9dd5134b67d308518781 Mon Sep 17 00:00:00 2001 From: Sean Christopherson Date: Wed, 17 Feb 2021 21:51:16 -0800 Subject: [PATCH 15/17] i386: acpi: Don't build HPET ACPI entry if HPET is disabled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Omit HPET AML if the HPET is disabled, QEMU is not emulating it and the guest may get confused by seeing HPET in the ACPI tables without a "physical" device present. The change of DSDT when -no-hpet is as follows. @@ -141,47 +141,6 @@ DefinitionBlock ("", "DSDT", 1, "BOCHS " } } - Scope (_SB) - { - Device (HPET) - { - Name (_HID, EisaId ("PNP0103") /* HPET System Timer */) // _HID: Hardware ID - Name (_UID, Zero) // _UID: Unique ID - OperationRegion (HPTM, SystemMemory, 0xFED00000, 0x0400) - Field (HPTM, DWordAcc, Lock, Preserve) - { - VEND, 32, - PRD, 32 - } - - Method (_STA, 0, NotSerialized) // _STA: Status - { - Local0 = VEND /* \_SB_.HPET.VEND */ - Local1 = PRD /* \_SB_.HPET.PRD_ */ - Local0 >>= 0x10 - If (((Local0 == Zero) || (Local0 == 0xFFFF))) - { - Return (Zero) - } - - If (((Local1 == Zero) || (Local1 > 0x05F5E100))) - { - Return (Zero) - } - - Return (0x0F) - } - - Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings - { - Memory32Fixed (ReadOnly, - 0xFED00000, // Address Base - 0x00000400, // Address Length - ) - }) - } - } - Scope (_SB.PCI0) { Device (ISA) Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Igor Mammedov Signed-off-by: Sean Christopherson Message-Id: <66114dead09232d04891b9e5f5a4081e85cc2c4d.1613615732.git.isaku.yamahata@intel.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/i386/acpi-build.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index 96497475d1..31a5f6f4a5 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -1281,7 +1281,9 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, aml_append(sb_scope, dev); aml_append(dsdt, sb_scope); - build_hpet_aml(dsdt); + if (misc->has_hpet) { + build_hpet_aml(dsdt); + } build_piix4_isa_bridge(dsdt); build_isa_devices_aml(dsdt); if (pm->pcihp_bridge_en || pm->pcihp_root_en) { @@ -1328,7 +1330,9 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, aml_append(dsdt, sb_scope); - build_hpet_aml(dsdt); + if (misc->has_hpet) { + build_hpet_aml(dsdt); + } build_q35_isa_bridge(dsdt); build_isa_devices_aml(dsdt); build_q35_pci0_int(dsdt); From 9a70e043593d913d2c8d5f398b43cabc2f5db382 Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Wed, 17 Feb 2021 21:51:17 -0800 Subject: [PATCH 16/17] acpi: add test case for -no-hpet Reviewed-by: Igor Mammedov Signed-off-by: Isaku Yamahata Message-Id: <5ef9a81e49793afb42ffd19bbf1f44e269c65e93.1613615732.git.isaku.yamahata@intel.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- tests/qtest/bios-tables-test.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c index 93d037c29d..e020c83d2a 100644 --- a/tests/qtest/bios-tables-test.c +++ b/tests/qtest/bios-tables-test.c @@ -1006,6 +1006,17 @@ static void test_acpi_piix4_tcg_smm_compat_nosmm(void) free_test_data(&data); } +static void test_acpi_piix4_tcg_nohpet(void) +{ + test_data data; + + memset(&data, 0, sizeof(data)); + data.machine = MACHINE_PC; + data.variant = ".nohpet"; + test_acpi_one("-no-hpet", &data); + free_test_data(&data); +} + static void test_acpi_q35_tcg_numamem(void) { test_data data; @@ -1051,6 +1062,17 @@ static void test_acpi_q35_tcg_smm_compat_nosmm(void) free_test_data(&data); } +static void test_acpi_q35_tcg_nohpet(void) +{ + test_data data; + + memset(&data, 0, sizeof(data)); + data.machine = MACHINE_Q35; + data.variant = ".nohpet"; + test_acpi_one("-no-hpet", &data); + free_test_data(&data); +} + static void test_acpi_piix4_tcg_numamem(void) { test_data data; @@ -1516,11 +1538,13 @@ int main(int argc, char *argv[]) test_acpi_piix4_tcg_smm_compat); qtest_add_func("acpi/piix4/smm-compat-nosmm", test_acpi_piix4_tcg_smm_compat_nosmm); + qtest_add_func("acpi/piix4/nohpet", test_acpi_piix4_tcg_nohpet); qtest_add_func("acpi/q35/nosmm", test_acpi_q35_tcg_nosmm); qtest_add_func("acpi/q35/smm-compat", test_acpi_q35_tcg_smm_compat); qtest_add_func("acpi/q35/smm-compat-nosmm", test_acpi_q35_tcg_smm_compat_nosmm); + qtest_add_func("acpi/q35/nohpet", test_acpi_q35_tcg_nohpet); qtest_add_func("acpi/piix4/dimmpxm", test_acpi_piix4_tcg_dimm_pxm); qtest_add_func("acpi/q35/dimmpxm", test_acpi_q35_tcg_dimm_pxm); qtest_add_func("acpi/piix4/acpihmat", test_acpi_piix4_tcg_acpi_hmat); From 7b630d937a6c73fb145746fb31e0fb4b08f0cf0e Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Wed, 17 Feb 2021 21:51:18 -0800 Subject: [PATCH 17/17] qtest/acpi/bios-tables-test: update acpi tables update golden master acpi tables and empty bios-tables-test-allowed-diff.h. Signed-off-by: Isaku Yamahata Message-Id: Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- tests/data/acpi/pc/DSDT.nohpet | Bin 0 -> 4923 bytes tests/data/acpi/pc/FACP.nosmm | Bin 0 -> 116 bytes tests/data/acpi/q35/DSDT | Bin 7801 -> 7859 bytes tests/data/acpi/q35/DSDT.acpihmat | Bin 9126 -> 9184 bytes tests/data/acpi/q35/DSDT.bridge | Bin 7819 -> 7877 bytes tests/data/acpi/q35/DSDT.cphp | Bin 8265 -> 8323 bytes tests/data/acpi/q35/DSDT.dimmpxm | Bin 9455 -> 9513 bytes tests/data/acpi/q35/DSDT.ipmibt | Bin 7876 -> 7934 bytes tests/data/acpi/q35/DSDT.memhp | Bin 9160 -> 9218 bytes tests/data/acpi/q35/DSDT.mmio64 | Bin 8932 -> 8990 bytes tests/data/acpi/q35/DSDT.nohpet | Bin 0 -> 7717 bytes tests/data/acpi/q35/DSDT.numamem | Bin 7807 -> 7865 bytes tests/data/acpi/q35/DSDT.tis | Bin 8407 -> 8465 bytes tests/data/acpi/q35/FACP.nosmm | Bin 0 -> 244 bytes tests/qtest/bios-tables-test-allowed-diff.h | 14 -------------- 15 files changed, 14 deletions(-) diff --git a/tests/data/acpi/pc/DSDT.nohpet b/tests/data/acpi/pc/DSDT.nohpet index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..d7d21be070c3b879e558193cbf8ded5a64c15eda 100644 GIT binary patch literal 4923 zcmb7IUvC@75ufE9<>Qi+j?~#wtRyBByJ^xQ<{w*j5TFpbM@qCrisn&CixWUnWKb#y z;DZwZQH7v111O503J5Ct&5I@J=3>dRCmDcnz^+)3kOtYKMT9hc*|+V@6Fuy z8v945@%KjK%WoU3>A>y_{dBkKd~@8`1p@8wI-JvkzFfC*b-TAz5yhldUoV=G?0>kT z)w5<5wEA|@1pPN35QYGD?`F};)?I4^L&O*=_mU#O8(h`P3wIR>B$VB|w~;du?2#%+ zYSprQPOFQm6n2%op6zpDlvr)Pcsbt}TEu`s;kn*B^C zLE!XlC-7z_?5>r6gmt66~}~S44e)boddQ9^t$~nsn0+!uTT}}8=NY}FnPUFt$`B{bQ=ydU*tlkP2P3>(&Pcy zbq=34cY!O=>gy*h1cg+9&GLo`?f&hP7VF1Onp_>JEogrq9Z6XP#ZFp0Fw*3P8fByr z(00B)*yTEe2Lm{612~6G^mp85;Ixk$P1tXqHSA}%?*z=ATg+uO1KlGlP@Q!8pi@m4Dyr*36DU5I2d&-#ZJ!SVw zl)ISl&I7-~eR|A&+I63vaG&;ZpB}n5C*5Or`o-C2#@uII_n8Uz86WoSCtzLB}xfUJDOCZGY1;| zn6#dmW&gADn4+Y{8Ri zns=vlOfgdH#HhZ1(7%5L=2iwyUyCcwm4QJczPI0^Q%PR+jwM963oV**%&ldorYCoW z3**_}oj%N^kl3_++;LKlX&QfY7W4p^(4|MNHs0X+TTHJ}@w-!38- z5vHM@MHEuKLi0srBu8=Y1^Dl`3jm&-_O-!)_BFZ!;9BG%OB)RS^*_(@@1LLl?(!W^ zN3g+Q@K*` z#(f0kOFQl+M;0y}X~%;E%9nPXWLyN&o^a($`zpy;&eGm>6QgA55BbPgzCU|?W8=j89~5v<@85#X!<1dKp25F13p03!n@jLE>j24S*5X%LSC$bPUM bD$c~f!UAOe=l{bMK@Wv1UWi$#Cv$U@CNt=7&sdkGH}E@ j2L%htbIG)eGcfQlGC;tF|NsB*hq47K88%BX#>)Z#Nca{a delta 29 lcmdmN`_qQYCD^R3>k(iG?dShcL#=0sxLH2_gUh diff --git a/tests/data/acpi/q35/DSDT.acpihmat b/tests/data/acpi/q35/DSDT.acpihmat index 2723b690089c9e75869708cb92d3081b6bb5ec65..b3c1dd6bc40210425ac37dba88a650b0ea60ce1c 100644 GIT binary patch delta 88 zcmZ4H{=l8fCDbMK@Wv1UWi$#Cv$U@CNt=7&sdkGH}E@ j2L%htbIG)eGcfQlGC;tF|NsB*hq47K88%BX_9y`WKL-{v delta 29 lcmaFhzRaD=CDk(iG?dShcNai0RW8-2{Hfx diff --git a/tests/data/acpi/q35/DSDT.bridge b/tests/data/acpi/q35/DSDT.bridge index 86711455576518f9ddd1f524d82a447946be32ba..eb5d27d95b2cdeda5f7e1f6b151cfea02e6bd907 100644 GIT binary patch delta 88 zcmeCSJ!;G466_LkRE~jxv3(*}Dw8YU#KM(U(M=XEL5|KG@gANoya9dz2F?bC3>@*! iLBWFZTr%zA3=BMs3=pv4|NsB{p=^OlhRsrpg|Ywj|NsB}P_{rN!)7T)eR%-yPZZJs delta 29 kcmZp6Jn6vY66_M@*! iLBWFZTr%zA3=BMs3=pv4|NsB{p=^OlhRsrpSCs+g?G;b} delta 29 lcmZ4K_1=@qCDv4q?2i3;>)Z3Qzz5 diff --git a/tests/data/acpi/q35/DSDT.ipmibt b/tests/data/acpi/q35/DSDT.ipmibt index 8d3ccc3e75164c2884fbe21659100e12fc70ae38..ce07e9f152def6a22ab29b3bde98b7d1f15a0522 100644 GIT binary patch delta 88 zcmX?N`_GokCDv4q;p+3jms{3IhND diff --git a/tests/data/acpi/q35/DSDT.memhp b/tests/data/acpi/q35/DSDT.memhp index f1c545d94b856fa8d19b8433233d80397cb05714..7acf6243f08cd906aa8a02d3acf1d720b36385ea 100644 GIT binary patch delta 88 zcmX@%-sHjM66_Mfq{6_!cz+^SDw8YU#KM(U(M=XEL5|KG@gANoya9dz2F?bC3>@*! iLBWFZTr%zA3=BMs3=pv4|NsB{p=^OlhRsrp>y!ZYffaB7 delta 29 lcmZqjIN{Fa66_LkLYaYqar#8AR3>k(iG?dShcK>F0sxF532*=a diff --git a/tests/data/acpi/q35/DSDT.mmio64 b/tests/data/acpi/q35/DSDT.mmio64 index 4fb285f2efea00964ea0f5c4172c213f0817b563..77d46369e48efca9a9e5024542c77cd26144beff 100644 GIT binary patch delta 88 zcmaFjI?s*ECDk(iG?dShcKQ}1OSi336B5( diff --git a/tests/data/acpi/q35/DSDT.nohpet b/tests/data/acpi/q35/DSDT.nohpet index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..0b10128e42af0c7b65e010963085bbf690a64065 100644 GIT binary patch literal 7717 zcmb7JTW=f38J*=tS}m92lA(;6*ml-%Vlx=~!YZa@K~B&U_*0$Ics z5W@&yD?sD;p(r9a=%Z{XK;PP@_Obp6eXD`~gTD45ekyyu`F7+Pk^&+=kb7p&nQzYC z&T=LQgN?T@31R)cvgS9!`O2?b{yaXG5Q4Vp*K4LWMdwjasdueRB4u?RXXG}%Dt7V5 zLFH!M`qOUjt6uN^=RIpH65B7lXWLuRXHRk*YSDoyJm2PL>cUpe4WOp0Q zz%BpNvF)zUTS>cH4%))*-fRYr;};eG7+EUmnQa+~(1qs7Ec& zU2dYT7LM+A_OA!+vMqtbxvM&qkfv{Um zzb1-q7r$1tOUV1f`xYGy&;O6SHd^hCR$=Mhqe3Q?>G#F*>QYtkwuSAQD7e(_Bmn9P zBmo+tkWIP$K28qvVniwsCwZULfvO-W(ZA#-q!Qd1ky23;LMlO(NORK}pW}wlanmQD?$b7W+J;Zt^a-f@oG^S&7(ORVpMbhg$MESGJ{{91pzbqc_{YgGiUk) z)O~t}PtWk_nLYt^pLxS)-td_>eFEw}3x>~v;j>`+1k`;_89t{BpHrq!K;382@L4o` z7EPaky3aF)&ohS4Gp0{K-RHF7bK3AZZTbY%eIlH2^{MC3jNx;}^a-f@JZtzoYxq2C z`UKQ{o|DRQ{3JXlm6iBmcaD|*8%W9^=FS?*SwlH%Dgkxnc|&>LP@XrHfV%R6p}b%y zFPKU|U3t+^UNn>!O(meN#A}xJOt>VK>KT8DmFmF_NJ@1E=Zu@gl?2!Zr%W0SbV!43Yqg#cx)3Q&zo0m{_l z00qQoRDd!!MpRP|P^KOSC?G~8xd>3^#whkEsT810RZ#bd0Of9_5&^1FDL?^LDpG&~ zDxU~Y=1L_3RHIUWGF8^300mS&5unVKN(87zr2u8BR8oKfDxU~Y=1L_3RHIUWGF2)m zKmpbHM1V3^DiNR>l>(HhQb_>{sC*(onJbkDP>o6f%2cVO00mS&5unVKN(87zr2u8B zR8oKfDxU~Y=1L_3RHIUWGF2)mKmnCc1SoT*5&^1FDL|Pjl@y?W$|nMpxl)M$)uB?Ty;@`(Utu2dpGH7W%tQ>Bsu6j1p@fHGGq5uh5C z0+gvzNdXF|d?G-ZE0qXPjYu@1ynu}pv;v@1gJ)(0A;FFQh)*~p9oOqN+kkR zqf&q}RVpb!0hLb#D08I}0jg0cK$$9)6rh00CjykYQi%Z7s1%?~l}ZXwK;;tw%3P^L zfNE3^Q29iF0%-vXqz5RV9-x3)fNDwvsHUU< z)sz&Vni2u3DG{KWk^)pyQh;ho1gNG&fNDw#P)$hzswokmni2u3DJeiTB?YLaM1TUR zvx@))!WokXlL8bFr$Iqds$+`)1yaWr0Scs!Ed?l`I<^#`fS7%l?IJ?Op?e_ztRB%H z(&uCJQ|>(e*_9;yOQpZ<^oQPRwb#o?-1Q25`ScaoVje$L+OE-8oxU1&ajLlejeax%qh&2_&$0YbZ!pL8-`jEM<3x$r25p*Do^ck^~ykX%np3(>J_beMOLq1e0udtadh=cXMcsf z^Xv>}kYevn;uPh5t-LSG`vc|u6UzH>`C!1jgBhD|?LN@T2eN!HP(C=Jd=Qtf4wg@y zqI^{=UzO#n1LdnHl&{9+YlG#}rzl_3%GYH1+Ccf*3FT{X`TAh_u~U?*e0`vN z{e<%MxO`)<{P-!#H?;B%S-vq)zHvhN2Fj=CQ8rlKO+1=26 zQ#8B7tIAM!+%~eiq5I&B*&W_rhPvank=+el*VATqc&Qocj@w3dH*}rOnBCz`XQ(@F zGrRN7Ym?f!aHWFTd(70kNla>cYd3gWzu|1YnbK~xtD(9|9o(GArmWw(Ip6r`e)PM& z-_C#h!M%_6KHmA@p6FP9Yk4WYTCR-! zeseXn{PxPPEWi3*iN0!he73L3v{ybIr}wrX2}t#8rQFQ~JSx zc#zq$-)fdh3ndx`uSKMbd4Hq!e&WzM*cHF`OQq7CVq?t55GFd`*}RfcbN6JF4D|ca z?qD;DMWS~&doZ|zEJRMO*ZJPip5>*ZUU4N&woX^b8te)^jO@QyoAyJ0)jq+7?XOmE z6%zeO>_pb>7eV3RwiSunk=2XD){ABLj|V;L*$$d|(avB=X?U;^C*^SF*c$4eh;*^S38ik--0aeERf- zb*TZsEa#&o|ip+C9cuECdPhU%B`u_tU#=zfbSB^hpR~SkFG82|cych`=KMYIP8Q>sW0wEYK1mZ|;EUJyG GwDEs12p4Jq literal 0 HcmV?d00001 diff --git a/tests/data/acpi/q35/DSDT.numamem b/tests/data/acpi/q35/DSDT.numamem index dd9dc9d02501afb50da7b9e77daabeee8968c340..e4c4582e7f76b072ab1123c748b89ea33ea1db87 100644 GIT binary patch delta 88 zcmexwv(uK#CDbMK@Wv1UWi$#Cv$U@CNt=7&sdkGH}E@ j2L%htbIG)eGcfQlGC;tF|NsB*hq47K88%BXrpf{UHWn4+ delta 29 kcmdmK``?DkCDk(iG?dShcNDy2LO#z33>nk diff --git a/tests/data/acpi/q35/FACP.nosmm b/tests/data/acpi/q35/FACP.nosmm index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..6a9aa5f370eb9af6a03dc739d8a159be58fdee01 100644 GIT binary patch literal 244 zcmZ>BbPo8!z`(#<;Nj24S*5X%LSC$X0-f zGcm9T0LA|E|L2FOWMD7?GM2V5Ffej3F#P0!h{7ddihwku0+2v57svwxMxcSn_QAxF TX+{NzJ3wNL4G8yu_%Hwf9PJGO literal 0 HcmV?d00001 diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h index 95592459c5..dfb8523c8b 100644 --- a/tests/qtest/bios-tables-test-allowed-diff.h +++ b/tests/qtest/bios-tables-test-allowed-diff.h @@ -1,15 +1 @@ /* List of comma-separated changed AML files to ignore */ -"tests/data/acpi/pc/FACP.nosmm", -"tests/data/acpi/pc/DSDT.nohpet", -"tests/data/acpi/q35/DSDT", -"tests/data/acpi/q35/DSDT.tis", -"tests/data/acpi/q35/DSDT.bridge", -"tests/data/acpi/q35/DSDT.mmio64", -"tests/data/acpi/q35/DSDT.ipmibt", -"tests/data/acpi/q35/DSDT.cphp", -"tests/data/acpi/q35/DSDT.memhp", -"tests/data/acpi/q35/DSDT.numamem", -"tests/data/acpi/q35/FACP.nosmm", -"tests/data/acpi/q35/DSDT.nohpet", -"tests/data/acpi/q35/DSDT.dimmpxm", -"tests/data/acpi/q35/DSDT.acpihmat",