All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] ACPI related fixes
@ 2021-02-04  8:04 isaku.yamahata
  2021-02-04  8:04 ` [PATCH 1/4] acpi/core: always set SCI_EN when SMM isn't supported isaku.yamahata
                   ` (5 more replies)
  0 siblings, 6 replies; 8+ messages in thread
From: isaku.yamahata @ 2021-02-04  8:04 UTC (permalink / raw)
  To: qemu-devel, imammedo, mst, marcel.apfelbaum; +Cc: Isaku Yamahata

From: Isaku Yamahata <isaku.yamahata@intel.com>

Miscellaneous bug fixes related to ACPI to play nice with guest BIOSes/OSes
by conforming to ACPI spec better.

Isaku Yamahata (3):
  acpi/core: always set SCI_EN when SMM isn't supported
  acpi: set fadt.smi_cmd to zero when SMM is not supported
  hw/i386: declare ACPI mother board resource for MMCONFIG region

Sean Christopherson (1):
  i386: acpi: Don't build HPET ACPI entry if HPET is disabled

 hw/acpi/core.c         |  11 ++-
 hw/acpi/ich9.c         |   2 +-
 hw/acpi/piix4.c        |   3 +-
 hw/i386/acpi-build.c   | 188 +++++++++++++++++++++++++++++++++++++++--
 hw/isa/vt82c686.c      |   2 +-
 include/hw/acpi/acpi.h |   4 +-
 6 files changed, 200 insertions(+), 10 deletions(-)

-- 
2.17.1



^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/4] acpi/core: always set SCI_EN when SMM isn't supported
  2021-02-04  8:04 [PATCH 0/4] ACPI related fixes isaku.yamahata
@ 2021-02-04  8:04 ` isaku.yamahata
  2021-02-04  8:04 ` [PATCH 2/4] acpi: set fadt.smi_cmd to zero when SMM is not supported isaku.yamahata
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: isaku.yamahata @ 2021-02-04  8:04 UTC (permalink / raw)
  To: qemu-devel, imammedo, mst, marcel.apfelbaum; +Cc: Isaku Yamahata

From: Isaku Yamahata <isaku.yamahata@intel.com>

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).

ACPI spec 4.8.10.1 PM1 Event Grouping
PM1 Eanble Registers
> For ACPI-only platforms (where SCI_EN is always set)

Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
---
 hw/acpi/core.c         | 11 ++++++++++-
 hw/acpi/ich9.c         |  2 +-
 hw/acpi/piix4.c        |  3 ++-
 hw/isa/vt82c686.c      |  2 +-
 include/hw/acpi/acpi.h |  4 +++-
 5 files changed, 17 insertions(+), 5 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..1a34d7f621 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, !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 669be5bbf6..0cddf91de5 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -496,7 +496,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_enabled);
     acpi_gpe_init(&s->ar, GPE_LEN);
 
     s->powerdown_notifier.notify = piix4_pm_powerdown_req;
diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index a6f5a0843d..071b64b497 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -240,7 +240,7 @@ static void vt82c686b_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);
 }
 
 static Property via_pm_properties[] = {
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);
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/4] acpi: set fadt.smi_cmd to zero when SMM is not supported
  2021-02-04  8:04 [PATCH 0/4] ACPI related fixes isaku.yamahata
  2021-02-04  8:04 ` [PATCH 1/4] acpi/core: always set SCI_EN when SMM isn't supported isaku.yamahata
@ 2021-02-04  8:04 ` isaku.yamahata
  2021-02-04  8:04 ` [PATCH 3/4] hw/i386: declare ACPI mother board resource for MMCONFIG region isaku.yamahata
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: isaku.yamahata @ 2021-02-04  8:04 UTC (permalink / raw)
  To: qemu-devel, imammedo, mst, marcel.apfelbaum; +Cc: Isaku Yamahata

From: Isaku Yamahata <isaku.yamahata@intel.com>

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.

So when smm is not enabled, set it to zero to comform to the spec.

Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
---
 hw/i386/acpi-build.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index f56d699c7f..005bcc2886 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -139,6 +139,8 @@ const struct AcpiGenericAddress x86_nvdimm_acpi_dsmio = {
 static void init_common_fadt_data(MachineState *ms, Object *o,
                                   AcpiFadtData *data)
 {
+    X86MachineState *x86ms = X86_MACHINE(ms);
+    bool smm_enabled = 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 +161,12 @@ 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 },
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 3/4] hw/i386: declare ACPI mother board resource for MMCONFIG region
  2021-02-04  8:04 [PATCH 0/4] ACPI related fixes isaku.yamahata
  2021-02-04  8:04 ` [PATCH 1/4] acpi/core: always set SCI_EN when SMM isn't supported isaku.yamahata
  2021-02-04  8:04 ` [PATCH 2/4] acpi: set fadt.smi_cmd to zero when SMM is not supported isaku.yamahata
@ 2021-02-04  8:04 ` isaku.yamahata
  2021-02-04  8:04 ` [PATCH 4/4] i386: acpi: Don't build HPET ACPI entry if HPET is disabled isaku.yamahata
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: isaku.yamahata @ 2021-02-04  8:04 UTC (permalink / raw)
  To: qemu-devel, imammedo, mst, marcel.apfelbaum; +Cc: Isaku Yamahata

From: Isaku Yamahata <isaku.yamahata@intel.com>

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, 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.
If guest BIOS doesn't reserve the region in memory map without the
reservation by mother board resource, guest linux abandons to use
MMCFG.

TDVF [0] [1] 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] TDX: Intel Trust Domain Extension
    https://software.intel.com/content/www/us/en/develop/articles/intel-trust-domain-extensions.html
[1] TDX Virtual Firmware
    https://github.com/tianocore/edk2-staging/tree/TDVF

Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
Acked-by: Jiewen Yao <Jiewen.yao@intel.com>
---
 hw/i386/acpi-build.c | 172 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 172 insertions(+)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 005bcc2886..6e38f67120 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1062,6 +1062,177 @@ static void build_q35_pci0_int(Aml *table)
     aml_append(table, sb_scope);
 }
 
+static Aml *build_q35_dram_controller(void)
+{
+    /*
+     * DSDT is created with revision 1 which means 32bit integer.
+     * When the method of _CRS is called to determine MMCONFIG region,
+     * only port io is allowed to access PCI configuration space.
+     * It means qword access isn't allowed.
+     *
+     * Device(DRAC)
+     * {
+     *     Name(_HID, EisaId("PNP0C01"))
+     *     OperationRegion(DRR0, PCI_Config, 0x0000000000000060, 0x8)
+     *     Field(DRR0, DWordAcc, Lock, Preserve)
+     *     {
+     *         PEBL, 4,
+     *         PEBH, 4
+     *     }
+     *     Name(RBUF, ResourceTemplate()
+     *     {
+     *         QWordMemory(ResourceConsumer,
+     *                     PosDecode,
+     *                     MinFixed,
+     *                     MaxFixed,
+     *                     NonCacheable,
+     *                     ReadWrite,
+     *                     0x0000000000000000, // Granularity
+     *                     0x0000000000000000, // Range Minimum
+     *                     0x0000000000000000, // Range Maxium
+     *                     0x0000000000000000, // Translation Offset,
+     *                     0x0000000000000000, // Length,
+     *                     ,,
+     *                     _MCF,
+     *                     AddressRangeMemory,
+     *                     TypeStatic
+     *                     )
+     *     })
+     *     Method(_CRS, 0x0, NotSerialized)
+     *     {
+     *         CreateDWordField(RBUF, DRAC._MCF._MIN, MINL)
+     *         CreateDWordField(RBUF, DRAC._MCF._MIN + 4, MINH)
+     *         CreateDWordField(RBUF, DRAC._MCF._MAX, MAXL)
+     *         CreateDWordField(RBUF, DRAC._MCF._MAX + 4, MAXH)
+     *         CreateQWordField(RBUF, DRAC._MCF._LEN, _LEN)
+     *
+     *         Local0 = PEBL
+     *         Local1 = Local0 & 0x1  // PCIEXBAR PCIEBAREN
+     *         Local2 = Local0 & 0x6  // PCIEXBAR LENGTH
+     *         Local3 = Local0 & ~0x7 // PCIEXBAR base address low 32bit
+     *         Local4 = PEBH          // PCIEXBAR base address high 32bit
+     *         If (Local1 == 1) {
+     *             MINL = Local3
+     *             MINH = Local4
+     *             MAXL = Local3
+     *             MAXH = Local4
+     *
+     *             If (Local2 == 0) {
+     *                 _LEN = 256 * 1024 * 1024
+     *             }
+     *             If (Local2 == 0x2) {
+     *                 _LEN = 128 * 1024 * 1024
+     *             }
+     *             If (Local2 == 0x4) {
+     *                 _LEN = 64 * 1024 * 1024
+     *             }
+     *         }
+     *         return (RBUF)
+     *     }
+     * }
+     */
+
+    Aml *dev;
+    Aml *field;
+    Aml *rbuf;
+    Aml *resource_template;
+    Aml *crs;
+
+    /* DRAM controller */
+    dev = aml_device("DRAC");
+
+    aml_append(dev, aml_name_decl("_HID", aml_string("PNP0C01")));
+    /* 5.1.6 PCIEXBAR: Bus 0:Device 0:Function 0:offset 0x60 */
+    aml_append(dev, aml_operation_region("DRR0", AML_PCI_CONFIG,
+                                         aml_int(0x0000000000000060), 0x8));
+    field = aml_field("DRR0", AML_DWORD_ACC, AML_NOLOCK, AML_PRESERVE);
+    aml_append(field, aml_named_field("PEBL", 32));
+    aml_append(field, aml_named_field("PEBH", 32));
+    aml_append(dev, field);
+
+    resource_template = aml_resource_template();
+    aml_append(resource_template, aml_qword_memory(AML_POS_DECODE,
+                                                   AML_MIN_FIXED,
+                                                   AML_MAX_FIXED,
+                                                   AML_NON_CACHEABLE,
+                                                   AML_READ_WRITE,
+                                                   0x0000000000000000,
+                                                   0x0000000000000000,
+                                                   0x0000000000000000,
+                                                   0x0000000000000000,
+                                                   0x0000000000000000));
+    rbuf = aml_name_decl("RBUF", resource_template);
+    aml_append(dev, rbuf);
+
+    crs = aml_method("_CRS", 0, AML_SERIALIZED);
+    {
+        Aml *rbuf_name;
+        Aml *local0;
+        Aml *local1;
+        Aml *local2;
+        Aml *local3;
+        Aml *local4;
+        Aml *ifc;
+
+        rbuf_name = aml_name("RBUF");
+        aml_append(crs, aml_create_dword_field(rbuf_name,
+                                               aml_int(14), "MINL"));
+        aml_append(crs, aml_create_dword_field(rbuf_name,
+                                               aml_int(14 + 4), "MINH"));
+        aml_append(crs, aml_create_dword_field(rbuf_name,
+                                               aml_int(22), "MAXL"));
+        aml_append(crs, aml_create_dword_field(rbuf_name,
+                                               aml_int(22 + 4), "MAXH"));
+        aml_append(crs, aml_create_qword_field(rbuf_name,
+                                               aml_int(38), "_LEN"));
+
+        local0 = aml_local(0);
+        aml_append(crs, aml_store(aml_name("PEBL"), local0));
+        local1 = aml_local(1);
+        aml_append(crs, aml_and(local0, aml_int(0x1), local1));
+        local2 = aml_local(2);
+        aml_append(crs, aml_and(local0, aml_int(0x6), local2));
+        local3 = aml_local(3);
+        aml_append(crs, aml_and(local0, aml_int((uint32_t)~0x7), local3));
+        local4 = aml_local(4);
+        aml_append(crs, aml_store(aml_name("PEBH"), local4));
+
+        ifc = aml_if(aml_equal(local1, aml_int(0x1)));
+        {
+            Aml *_len;
+            Aml *ifc0;
+            Aml *ifc2;
+            Aml *ifc4;
+
+            aml_append(ifc, aml_store(local3, aml_name("MINL")));
+            aml_append(ifc, aml_store(local4, aml_name("MINH")));
+            aml_append(ifc, aml_store(local3, aml_name("MAXL")));
+            aml_append(ifc, aml_store(local4, aml_name("MAXH")));
+
+            _len = aml_name("_LEN");
+            ifc0 = aml_if(aml_equal(local2, aml_int(0x0)));
+            aml_append(ifc0,
+                       aml_store(aml_int(256 * 1024 * 1024), _len));
+            aml_append(ifc, ifc0);
+
+            ifc2 = aml_if(aml_equal(local2, aml_int(0x2)));
+            aml_append(ifc2,
+                       aml_store(aml_int(128 * 1024 * 1024), _len));
+            aml_append(ifc, ifc2);
+
+            ifc4 = aml_if(aml_equal(local2, aml_int(0x4)));
+            aml_append(ifc4,
+                       aml_store(aml_int(64 * 1024 * 1024), _len));
+            aml_append(ifc, ifc4);
+        }
+        aml_append(crs, ifc);
+        aml_append(crs, aml_return(rbuf_name));
+    }
+    aml_append(dev, crs);
+
+    return dev;
+}
+
 static void build_q35_isa_bridge(Aml *table)
 {
     Aml *dev;
@@ -1246,6 +1417,7 @@ 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);
+        aml_append(sb_scope, build_q35_dram_controller());
 
         if (pm->smi_on_cpuhp) {
             /* reserve SMI block resources, IO ports 0xB2, 0xB3 */
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 4/4] i386: acpi: Don't build HPET ACPI entry if HPET is disabled
  2021-02-04  8:04 [PATCH 0/4] ACPI related fixes isaku.yamahata
                   ` (2 preceding siblings ...)
  2021-02-04  8:04 ` [PATCH 3/4] hw/i386: declare ACPI mother board resource for MMCONFIG region isaku.yamahata
@ 2021-02-04  8:04 ` isaku.yamahata
  2021-02-04  8:56   ` Philippe Mathieu-Daudé
  2021-02-04  8:15 ` [PATCH 0/4] ACPI related fixes no-reply
  2021-02-05 13:48 ` Michael S. Tsirkin
  5 siblings, 1 reply; 8+ messages in thread
From: isaku.yamahata @ 2021-02-04  8:04 UTC (permalink / raw)
  To: qemu-devel, imammedo, mst, marcel.apfelbaum; +Cc: Sean Christopherson

From: Sean Christopherson <sean.j.christopherson@intel.com>

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.

Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
---
 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 6e38f67120..a4fcd14a93 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1401,7 +1401,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) {
@@ -1446,7 +1448,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);
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/4] ACPI related fixes
  2021-02-04  8:04 [PATCH 0/4] ACPI related fixes isaku.yamahata
                   ` (3 preceding siblings ...)
  2021-02-04  8:04 ` [PATCH 4/4] i386: acpi: Don't build HPET ACPI entry if HPET is disabled isaku.yamahata
@ 2021-02-04  8:15 ` no-reply
  2021-02-05 13:48 ` Michael S. Tsirkin
  5 siblings, 0 replies; 8+ messages in thread
From: no-reply @ 2021-02-04  8:15 UTC (permalink / raw)
  To: isaku.yamahata; +Cc: isaku.yamahata, imammedo, qemu-devel, mst

Patchew URL: https://patchew.org/QEMU/cover.1612424814.git.isaku.yamahata@intel.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: cover.1612424814.git.isaku.yamahata@intel.com
Subject: [PATCH 0/4] ACPI related fixes

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]         patchew/cover.1612424814.git.isaku.yamahata@intel.com -> patchew/cover.1612424814.git.isaku.yamahata@intel.com
Switched to a new branch 'test'
8eea4e1 i386: acpi: Don't build HPET ACPI entry if HPET is disabled
4f0ab1b hw/i386: declare ACPI mother board resource for MMCONFIG region
6947812 acpi: set fadt.smi_cmd to zero when SMM is not supported
4ae92fd acpi/core: always set SCI_EN when SMM isn't supported

=== OUTPUT BEGIN ===
1/4 Checking commit 4ae92fd8e8a5 (acpi/core: always set SCI_EN when SMM isn't supported)
2/4 Checking commit 6947812c8e69 (acpi: set fadt.smi_cmd to zero when SMM is not supported)
ERROR: line over 90 characters
#41: FILE: hw/i386/acpi-build.c:167:
+            smm_enabled ? object_property_get_uint(o, ACPI_PM_PROP_ACPI_ENABLE_CMD, NULL) : 0,

ERROR: line over 90 characters
#44: FILE: hw/i386/acpi-build.c:169:
+            smm_enabled ? object_property_get_uint(o, ACPI_PM_PROP_ACPI_DISABLE_CMD, NULL) : 0,

total: 2 errors, 0 warnings, 23 lines checked

Patch 2/4 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

3/4 Checking commit 4f0ab1b9744c (hw/i386: declare ACPI mother board resource for MMCONFIG region)
4/4 Checking commit 8eea4e187ac8 (i386: acpi: Don't build HPET ACPI entry if HPET is disabled)
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/cover.1612424814.git.isaku.yamahata@intel.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 4/4] i386: acpi: Don't build HPET ACPI entry if HPET is disabled
  2021-02-04  8:04 ` [PATCH 4/4] i386: acpi: Don't build HPET ACPI entry if HPET is disabled isaku.yamahata
@ 2021-02-04  8:56   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 8+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-02-04  8:56 UTC (permalink / raw)
  To: isaku.yamahata, qemu-devel, imammedo, mst, marcel.apfelbaum
  Cc: Sean Christopherson

On 2/4/21 9:04 AM, isaku.yamahata@gmail.com wrote:
> From: Sean Christopherson <sean.j.christopherson@intel.com>
> 
> 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.
> 
> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> ---
>  hw/i386/acpi-build.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/4] ACPI related fixes
  2021-02-04  8:04 [PATCH 0/4] ACPI related fixes isaku.yamahata
                   ` (4 preceding siblings ...)
  2021-02-04  8:15 ` [PATCH 0/4] ACPI related fixes no-reply
@ 2021-02-05 13:48 ` Michael S. Tsirkin
  5 siblings, 0 replies; 8+ messages in thread
From: Michael S. Tsirkin @ 2021-02-05 13:48 UTC (permalink / raw)
  To: isaku.yamahata; +Cc: Isaku Yamahata, imammedo, qemu-devel

On Thu, Feb 04, 2021 at 12:04:07AM -0800, isaku.yamahata@gmail.com wrote:
> From: Isaku Yamahata <isaku.yamahata@intel.com>
> 
> Miscellaneous bug fixes related to ACPI to play nice with guest BIOSes/OSes
> by conforming to ACPI spec better.
> 
> Isaku Yamahata (3):
>   acpi/core: always set SCI_EN when SMM isn't supported
>   acpi: set fadt.smi_cmd to zero when SMM is not supported
>   hw/i386: declare ACPI mother board resource for MMCONFIG region

These all look good, but break make check:

acpi-test: Warning! DSDT binary file mismatch. Actual [aml:/tmp/aml-YLJ7X0], Expected [aml:tests/data/acpi/q35/DSDT].
See source file tests/qtest/bios-tables-test.c for instructions on how to update expected files.
acpi-test: Warning! DSDT mismatch. Actual [asl:/tmp/asl-2KJ7X0.dsl, aml:/tmp/aml-YLJ7X0], Expected [asl:/tmp/asl-U2R7X0.dsl, aml:tests/data/acpi/q35/DSDT].
**
ERROR:../qemu/tests/qtest/bios-tables-test.c:509:test_acpi_asl: assertion failed: (all_tables_match)
ERROR qtest-i386/bios-tables-test - Bail out! ERROR:../qemu/tests/qtest/bios-tables-test.c:509:test_acpi_asl: assertion failed: (all_tables_match)
make: *** [Makefile.mtest:1713: run-test-212] Error 1

Probably because you didn't update the expected files.
Pls follow the process at the top of tests/qtest/bios-tables-test.c

and make sure all tests pass.


> Sean Christopherson (1):
>   i386: acpi: Don't build HPET ACPI entry if HPET is disabled
> 
>  hw/acpi/core.c         |  11 ++-
>  hw/acpi/ich9.c         |   2 +-
>  hw/acpi/piix4.c        |   3 +-
>  hw/i386/acpi-build.c   | 188 +++++++++++++++++++++++++++++++++++++++--
>  hw/isa/vt82c686.c      |   2 +-
>  include/hw/acpi/acpi.h |   4 +-
>  6 files changed, 200 insertions(+), 10 deletions(-)
> 
> -- 
> 2.17.1



^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-02-05 13:49 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-04  8:04 [PATCH 0/4] ACPI related fixes isaku.yamahata
2021-02-04  8:04 ` [PATCH 1/4] acpi/core: always set SCI_EN when SMM isn't supported isaku.yamahata
2021-02-04  8:04 ` [PATCH 2/4] acpi: set fadt.smi_cmd to zero when SMM is not supported isaku.yamahata
2021-02-04  8:04 ` [PATCH 3/4] hw/i386: declare ACPI mother board resource for MMCONFIG region isaku.yamahata
2021-02-04  8:04 ` [PATCH 4/4] i386: acpi: Don't build HPET ACPI entry if HPET is disabled isaku.yamahata
2021-02-04  8:56   ` Philippe Mathieu-Daudé
2021-02-04  8:15 ` [PATCH 0/4] ACPI related fixes no-reply
2021-02-05 13:48 ` Michael S. Tsirkin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.