* [PATCH v2 1/9] checkpatch: don't emit warning on newly created acpi data files
2021-02-08 21:57 [PATCH v2 0/9] ACPI related fixes isaku.yamahata
@ 2021-02-08 21:57 ` isaku.yamahata
2021-02-08 21:57 ` [PATCH v2 2/9] qtest: update tests/qtest/bios-tables-test-allowed-diff.h isaku.yamahata
` (7 subsequent siblings)
8 siblings, 0 replies; 25+ messages in thread
From: isaku.yamahata @ 2021-02-08 21:57 UTC (permalink / raw)
To: qemu-devel, imammedo, mst, marcel.apfelbaum, philmd; +Cc: Isaku Yamahata
From: Isaku Yamahata <isaku.yamahata@intel.com>
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 <isaku.yamahata@intel.com>
---
scripts/checkpatch.pl | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index e47ad878d8..40c9cc7def 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);
}
--
2.17.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v2 2/9] qtest: update tests/qtest/bios-tables-test-allowed-diff.h
2021-02-08 21:57 [PATCH v2 0/9] ACPI related fixes isaku.yamahata
2021-02-08 21:57 ` [PATCH v2 1/9] checkpatch: don't emit warning on newly created acpi data files isaku.yamahata
@ 2021-02-08 21:57 ` isaku.yamahata
2021-02-09 12:46 ` Igor Mammedov
2021-02-08 21:57 ` [PATCH v2 3/9] acpi/core: always set SCI_EN when SMM isn't supported isaku.yamahata
` (6 subsequent siblings)
8 siblings, 1 reply; 25+ messages in thread
From: isaku.yamahata @ 2021-02-08 21:57 UTC (permalink / raw)
To: qemu-devel, imammedo, mst, marcel.apfelbaum, philmd; +Cc: Isaku Yamahata
From: Isaku Yamahata <isaku.yamahata@intel.com>
The following tests will modify acpi tables.
prepare qtests to allow acpi table change.
add new tables for new tests.
- tests/data/acpi/q35/DSDT.nosmm
- tests/data/acpi/q35/FACP.nosmm
- tests/data/acpi/q35/DSDT.nohpet
Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
---
tests/data/acpi/q35/DSDT.nohpet | 0
tests/data/acpi/q35/DSDT.nosmm | 0
tests/data/acpi/q35/FACP.nosmm | 0
tests/qtest/bios-tables-test-allowed-diff.h | 13 +++++++++++++
4 files changed, 13 insertions(+)
create mode 100644 tests/data/acpi/q35/DSDT.nohpet
create mode 100644 tests/data/acpi/q35/DSDT.nosmm
create mode 100644 tests/data/acpi/q35/FACP.nosmm
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/DSDT.nosmm b/tests/data/acpi/q35/DSDT.nosmm
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..b79ac495c2 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1 +1,14 @@
/* List of comma-separated changed AML files to ignore */
+"tests/data/acpi/q35/DSDT",
+"tests/data/acpi/q35/DSDT.tis",
+"tests/data/acpi/q35/DSDT.bridge",
+"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/DSDT.dimmpxm",
+"tests/data/acpi/q35/DSDT.acpihmat",
+"tests/data/acpi/q35/DSDT.mmio64",
+"tests/data/acpi/q35/DSDT.nosmm",
+"tests/data/acpi/q35/FACP.nosmm",
+"tests/data/acpi/q35/DSDT.nohpet",
--
2.17.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH v2 2/9] qtest: update tests/qtest/bios-tables-test-allowed-diff.h
2021-02-08 21:57 ` [PATCH v2 2/9] qtest: update tests/qtest/bios-tables-test-allowed-diff.h isaku.yamahata
@ 2021-02-09 12:46 ` Igor Mammedov
0 siblings, 0 replies; 25+ messages in thread
From: Igor Mammedov @ 2021-02-09 12:46 UTC (permalink / raw)
To: isaku.yamahata; +Cc: Isaku Yamahata, philmd, qemu-devel, mst
On Mon, 8 Feb 2021 13:57:21 -0800
isaku.yamahata@gmail.com wrote:
> From: Isaku Yamahata <isaku.yamahata@intel.com>
>
> The following tests will modify acpi tables.
> prepare qtests to allow acpi table change.
> add new tables for new tests.
> - tests/data/acpi/q35/DSDT.nosmm
> - tests/data/acpi/q35/FACP.nosmm
> - tests/data/acpi/q35/DSDT.nohpet
>
> Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
Acked-by: Igor Mammedov <imammedo@redhat.com>
> ---
> tests/data/acpi/q35/DSDT.nohpet | 0
> tests/data/acpi/q35/DSDT.nosmm | 0
> tests/data/acpi/q35/FACP.nosmm | 0
> tests/qtest/bios-tables-test-allowed-diff.h | 13 +++++++++++++
> 4 files changed, 13 insertions(+)
> create mode 100644 tests/data/acpi/q35/DSDT.nohpet
> create mode 100644 tests/data/acpi/q35/DSDT.nosmm
> create mode 100644 tests/data/acpi/q35/FACP.nosmm
>
> 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/DSDT.nosmm b/tests/data/acpi/q35/DSDT.nosmm
> 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..b79ac495c2 100644
> --- a/tests/qtest/bios-tables-test-allowed-diff.h
> +++ b/tests/qtest/bios-tables-test-allowed-diff.h
> @@ -1 +1,14 @@
> /* List of comma-separated changed AML files to ignore */
> +"tests/data/acpi/q35/DSDT",
> +"tests/data/acpi/q35/DSDT.tis",
> +"tests/data/acpi/q35/DSDT.bridge",
> +"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/DSDT.dimmpxm",
> +"tests/data/acpi/q35/DSDT.acpihmat",
> +"tests/data/acpi/q35/DSDT.mmio64",
> +"tests/data/acpi/q35/DSDT.nosmm",
> +"tests/data/acpi/q35/FACP.nosmm",
> +"tests/data/acpi/q35/DSDT.nohpet",
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v2 3/9] acpi/core: always set SCI_EN when SMM isn't supported
2021-02-08 21:57 [PATCH v2 0/9] ACPI related fixes isaku.yamahata
2021-02-08 21:57 ` [PATCH v2 1/9] checkpatch: don't emit warning on newly created acpi data files isaku.yamahata
2021-02-08 21:57 ` [PATCH v2 2/9] qtest: update tests/qtest/bios-tables-test-allowed-diff.h isaku.yamahata
@ 2021-02-08 21:57 ` isaku.yamahata
2021-02-09 15:05 ` Igor Mammedov
2021-02-08 21:57 ` [PATCH v2 4/9] acpi: set fadt.smi_cmd to zero when SMM is not supported isaku.yamahata
` (5 subsequent siblings)
8 siblings, 1 reply; 25+ messages in thread
From: isaku.yamahata @ 2021-02-08 21:57 UTC (permalink / raw)
To: qemu-devel, imammedo, mst, marcel.apfelbaum, philmd; +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] 25+ messages in thread* Re: [PATCH v2 3/9] acpi/core: always set SCI_EN when SMM isn't supported
2021-02-08 21:57 ` [PATCH v2 3/9] acpi/core: always set SCI_EN when SMM isn't supported isaku.yamahata
@ 2021-02-09 15:05 ` Igor Mammedov
2021-02-09 19:23 ` Isaku Yamahata
0 siblings, 1 reply; 25+ messages in thread
From: Igor Mammedov @ 2021-02-09 15:05 UTC (permalink / raw)
To: isaku.yamahata; +Cc: Isaku Yamahata, philmd, qemu-devel, mst
On Mon, 8 Feb 2021 13:57:22 -0800
isaku.yamahata@gmail.com wrote:
> 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).
does it break some specific software?
> 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>
it changes guest ABI for old machine types but it seems to me that
it's harmless (in typical use-cases backward and forward migrated
guest should work fine).
The only thing that is broken is transitioning to legacy mode
when guest was started on old QEMU and then migrated to the new one
where disable op will be NOP and qemu always stays in ACPI mode
(so guest will hang while it waits for transition to happen).
Can you test this scenario with various guest OSes (old/new/MS Windows)
to check if it won't break them.
if we are to be conservative, we need to disable this compliance fix
on old machine types.
other than that patch looks good to me.
> ---
> 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);
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH v2 3/9] acpi/core: always set SCI_EN when SMM isn't supported
2021-02-09 15:05 ` Igor Mammedov
@ 2021-02-09 19:23 ` Isaku Yamahata
2021-02-10 13:17 ` Igor Mammedov
0 siblings, 1 reply; 25+ messages in thread
From: Isaku Yamahata @ 2021-02-09 19:23 UTC (permalink / raw)
To: Igor Mammedov; +Cc: Isaku Yamahata, philmd, qemu-devel, isaku.yamahata, mst
On Tue, Feb 09, 2021 at 04:05:14PM +0100,
Igor Mammedov <imammedo@redhat.com> wrote:
> On Mon, 8 Feb 2021 13:57:22 -0800
> isaku.yamahata@gmail.com wrote:
>
> > 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).
>
> does it break some specific software?
With the next patch (setting fadt.smi_cmd = 0 when smm isn't supported),
guest Linux tries to switch to ACPI mode, finds smi_cmd = 0, and then
fails to initialize acpi subsystem.
will update the commit message in next respin.
> > 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>
> it changes guest ABI for old machine types but it seems to me that
> it's harmless (in typical use-cases backward and forward migrated
> guest should work fine).
>
> The only thing that is broken is transitioning to legacy mode
> when guest was started on old QEMU and then migrated to the new one
> where disable op will be NOP and qemu always stays in ACPI mode
> (so guest will hang while it waits for transition to happen).
The patch affects guests only when SMM isn't supported.
Concretely
- user explicitly specified to disable smm by -machine smm=off
or
- underlying kvm doesn't have KVM_CAP_X86_SMM (smm=auto: default)
Please refer to x86_machine_is_smm_enabled().
Also Libvirt checks if guest bios requires SMI and enables smm even
when user disabling SMM. qemuFirmwareEnableFeatures()
If smm is disabled and legacy-mode is enabled without this patch,
SMI won't be injected to guest anyway. Thus guest breaks already.
> Can you test this scenario with various guest OSes (old/new/MS Windows)
> to check if it won't break them.
Unless -machine smm=off is explicitly passed, this patch won't break
guests. And such case is rare as I described above.
My motivation for this patch series is preparation for TDX which disallows
SMM mode and SMI injection.
> if we are to be conservative, we need to disable this compliance fix
> on old machine types.
I'm fine with adding one more knob to be on safe side.
-machine smm=off is such knob, though.
Thanks,
--
Isaku Yamahata <isaku.yamahata@gmail.com>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 3/9] acpi/core: always set SCI_EN when SMM isn't supported
2021-02-09 19:23 ` Isaku Yamahata
@ 2021-02-10 13:17 ` Igor Mammedov
0 siblings, 0 replies; 25+ messages in thread
From: Igor Mammedov @ 2021-02-10 13:17 UTC (permalink / raw)
To: Isaku Yamahata; +Cc: Isaku Yamahata, philmd, qemu-devel, isaku.yamahata, mst
On Tue, 9 Feb 2021 11:23:05 -0800
Isaku Yamahata <yamahata.qemudev@gmail.com> wrote:
> On Tue, Feb 09, 2021 at 04:05:14PM +0100,
> Igor Mammedov <imammedo@redhat.com> wrote:
>
> > On Mon, 8 Feb 2021 13:57:22 -0800
> > isaku.yamahata@gmail.com wrote:
> >
> > > 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).
> >
> > does it break some specific software?
>
> With the next patch (setting fadt.smi_cmd = 0 when smm isn't supported),
> guest Linux tries to switch to ACPI mode, finds smi_cmd = 0, and then
> fails to initialize acpi subsystem.
> will update the commit message in next respin.
>
>
> > > 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>
> > it changes guest ABI for old machine types but it seems to me that
> > it's harmless (in typical use-cases backward and forward migrated
> > guest should work fine).
> >
> > The only thing that is broken is transitioning to legacy mode
> > when guest was started on old QEMU and then migrated to the new one
> > where disable op will be NOP and qemu always stays in ACPI mode
> > (so guest will hang while it waits for transition to happen).
>
> The patch affects guests only when SMM isn't supported.
> Concretely
> - user explicitly specified to disable smm by -machine smm=off
> or
> - underlying kvm doesn't have KVM_CAP_X86_SMM (smm=auto: default)
> Please refer to x86_machine_is_smm_enabled().
> Also Libvirt checks if guest bios requires SMI and enables smm even
> when user disabling SMM. qemuFirmwareEnableFeatures()
> If smm is disabled and legacy-mode is enabled without this patch,
> SMI won't be injected to guest anyway. Thus guest breaks already.
can you point to code in QEMU that prevents SMI being triggered?
(what see is QEMU faking SMM being configured when smm_enable=false,
and seabios skipping its smm code based on that, other guest code
may behave differently though).
But that's beside point, guest started on old QEMU may believe that
it runs on Legacy/ACPI platform even with (smm=off), and hence can try
to enable/disable ACPI mode explicitly. Combined with migration to QEMU
with this patches it might turn into problem (which is hard to
troubleshoot in the wild).
> > Can you test this scenario with various guest OSes (old/new/MS Windows)
> > to check if it won't break them.
>
> Unless -machine smm=off is explicitly passed, this patch won't break
> guests. And such case is rare as I described above.
yes, it's a corner case but it doesn't guarantee that someone isn't using
it it in the wild.
> My motivation for this patch series is preparation for TDX which disallows
> SMM mode and SMI injection.
>
>
> > if we are to be conservative, we need to disable this compliance fix
> > on old machine types.
>
> I'm fine with adding one more knob to be on safe side.
> -machine smm=off is such knob, though.
you can use 2ebc21216f as an example of compat knob that changes behavior
depending on machine version. (i.e. to keep old behavior in piix4/ich9 on
exiting machine versions)
> Thanks,
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v2 4/9] acpi: set fadt.smi_cmd to zero when SMM is not supported
2021-02-08 21:57 [PATCH v2 0/9] ACPI related fixes isaku.yamahata
` (2 preceding siblings ...)
2021-02-08 21:57 ` [PATCH v2 3/9] acpi/core: always set SCI_EN when SMM isn't supported isaku.yamahata
@ 2021-02-08 21:57 ` isaku.yamahata
2021-02-09 15:14 ` Igor Mammedov
2021-02-08 21:57 ` [PATCH v2 5/9] acpi: add test case for smm unsupported -machine smm=off isaku.yamahata
` (4 subsequent siblings)
8 siblings, 1 reply; 25+ messages in thread
From: isaku.yamahata @ 2021-02-08 21:57 UTC (permalink / raw)
To: qemu-devel, imammedo, mst, marcel.apfelbaum, philmd; +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.
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
Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
---
hw/i386/acpi-build.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index f56d699c7f..c2f11d95d8 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,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 },
--
2.17.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH v2 4/9] acpi: set fadt.smi_cmd to zero when SMM is not supported
2021-02-08 21:57 ` [PATCH v2 4/9] acpi: set fadt.smi_cmd to zero when SMM is not supported isaku.yamahata
@ 2021-02-09 15:14 ` Igor Mammedov
0 siblings, 0 replies; 25+ messages in thread
From: Igor Mammedov @ 2021-02-09 15:14 UTC (permalink / raw)
To: isaku.yamahata; +Cc: Isaku Yamahata, philmd, qemu-devel, mst
On Mon, 8 Feb 2021 13:57:23 -0800
isaku.yamahata@gmail.com wrote:
> 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.
>
> 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
>
> Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
when migrated to old QEMU, and VM after that is reset it
regenerate ACPI tables for old QEMU, so guest should be
able to turn on ACPI.
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> ---
> hw/i386/acpi-build.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index f56d699c7f..c2f11d95d8 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,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 },
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v2 5/9] acpi: add test case for smm unsupported -machine smm=off
2021-02-08 21:57 [PATCH v2 0/9] ACPI related fixes isaku.yamahata
` (3 preceding siblings ...)
2021-02-08 21:57 ` [PATCH v2 4/9] acpi: set fadt.smi_cmd to zero when SMM is not supported isaku.yamahata
@ 2021-02-08 21:57 ` isaku.yamahata
2021-02-09 15:25 ` Igor Mammedov
2021-02-08 21:57 ` [PATCH v2 6/9] hw/i386: declare ACPI mother board resource for MMCONFIG region isaku.yamahata
` (3 subsequent siblings)
8 siblings, 1 reply; 25+ messages in thread
From: isaku.yamahata @ 2021-02-08 21:57 UTC (permalink / raw)
To: qemu-devel, imammedo, mst, marcel.apfelbaum, philmd; +Cc: Isaku Yamahata
From: Isaku Yamahata <isaku.yamahata@intel.com>
Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
---
tests/qtest/bios-tables-test.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
index 669202fc95..096d15db68 100644
--- a/tests/qtest/bios-tables-test.c
+++ b/tests/qtest/bios-tables-test.c
@@ -969,6 +969,17 @@ 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_piix4_tcg_numamem(void)
{
test_data data;
@@ -1325,6 +1336,7 @@ 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/q35/nosmm", test_acpi_q35_tcg_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);
--
2.17.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH v2 5/9] acpi: add test case for smm unsupported -machine smm=off
2021-02-08 21:57 ` [PATCH v2 5/9] acpi: add test case for smm unsupported -machine smm=off isaku.yamahata
@ 2021-02-09 15:25 ` Igor Mammedov
0 siblings, 0 replies; 25+ messages in thread
From: Igor Mammedov @ 2021-02-09 15:25 UTC (permalink / raw)
To: isaku.yamahata; +Cc: Isaku Yamahata, philmd, qemu-devel, mst
On Mon, 8 Feb 2021 13:57:24 -0800
isaku.yamahata@gmail.com wrote:
> From: Isaku Yamahata <isaku.yamahata@intel.com>
>
> Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> ---
> tests/qtest/bios-tables-test.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
> index 669202fc95..096d15db68 100644
> --- a/tests/qtest/bios-tables-test.c
> +++ b/tests/qtest/bios-tables-test.c
> @@ -969,6 +969,17 @@ 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_piix4_tcg_numamem(void)
> {
> test_data data;
> @@ -1325,6 +1336,7 @@ 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/q35/nosmm", test_acpi_q35_tcg_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);
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v2 6/9] hw/i386: declare ACPI mother board resource for MMCONFIG region
2021-02-08 21:57 [PATCH v2 0/9] ACPI related fixes isaku.yamahata
` (4 preceding siblings ...)
2021-02-08 21:57 ` [PATCH v2 5/9] acpi: add test case for smm unsupported -machine smm=off isaku.yamahata
@ 2021-02-08 21:57 ` isaku.yamahata
2021-02-09 15:52 ` Igor Mammedov
2021-02-08 21:57 ` [PATCH v2 7/9] i386: acpi: Don't build HPET ACPI entry if HPET is disabled isaku.yamahata
` (2 subsequent siblings)
8 siblings, 1 reply; 25+ messages in thread
From: isaku.yamahata @ 2021-02-08 21:57 UTC (permalink / raw)
To: qemu-devel, imammedo, mst, marcel.apfelbaum, philmd; +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
The change to DSDT is as follows.
@@ -68,32 +68,90 @@
If ((CDW3 != Local0))
{
CDW1 |= 0x10
}
CDW3 = Local0
}
Else
{
CDW1 |= 0x04
}
Return (Arg3)
}
}
+
+ Device (DRAC)
+ {
+ Name (_HID, "PNP0C01" /* System Board */) // _HID: Hardware ID
+ OperationRegion (DRR0, PCI_Config, 0x60, 0x08)
+ Field (DRR0, DWordAcc, NoLock, Preserve)
+ {
+ PEBL, 32,
+ PEBH, 32
+ }
+
+ Name (RBUF, ResourceTemplate ()
+ {
+ QWordMemory (ResourceProducer, PosDecode, MinFixed, MaxFixed, NonCacheable, ReadWrite,
+ 0x0000000000000000, // Granularity
+ 0x0000000000000000, // Range Minimum
+ 0x0000000000000000, // Range Maximum
+ 0x0000000000000000, // Translation Offset
+ 0x0000000000000000, // Length
+ ,, _Y00, AddressRangeMemory, TypeStatic)
+ })
+ Method (_CRS, 0, Serialized) // _CRS: Current Resource Settings
+ {
+ CreateDWordField (RBUF, \_SB.DRAC._Y00._MIN, MINL) // _MIN: Minimum Base Address
+ CreateDWordField (RBUF, 0x12, MINH)
+ CreateDWordField (RBUF, \_SB.DRAC._Y00._MAX, MAXL) // _MAX: Maximum Base Address
+ CreateDWordField (RBUF, 0x1A, MAXH)
+ CreateQWordField (RBUF, \_SB.DRAC._Y00._LEN, _LEN) // _LEN: Length
+ Local0 = PEBL /* \_SB_.DRAC.PEBL */
+ Local1 = (Local0 & One)
+ Local2 = (Local0 & 0x06)
+ Local3 = (Local0 & 0xFFFFFFF8)
+ Local4 = PEBH /* \_SB_.DRAC.PEBH */
+ If ((Local1 == One))
+ {
+ MINL = Local3
+ MINH = Local4
+ MAXL = Local3
+ MAXH = Local4
+ If ((Local2 == Zero))
+ {
+ _LEN = 0x10000000
+ }
+
+ If ((Local2 == 0x02))
+ {
+ _LEN = 0x08000000
+ }
+
+ If ((Local2 == 0x04))
+ {
+ _LEN = 0x04000000
+ }
+ }
+
+ Return (RBUF) /* \_SB_.DRAC.RBUF */
+ }
+ }
}
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 <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 c2f11d95d8..bcb1f65c1d 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1066,6 +1066,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;
@@ -1250,6 +1421,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] 25+ messages in thread* Re: [PATCH v2 6/9] hw/i386: declare ACPI mother board resource for MMCONFIG region
2021-02-08 21:57 ` [PATCH v2 6/9] hw/i386: declare ACPI mother board resource for MMCONFIG region isaku.yamahata
@ 2021-02-09 15:52 ` Igor Mammedov
2021-02-09 20:02 ` Isaku Yamahata
2021-02-10 8:28 ` Michael S. Tsirkin
0 siblings, 2 replies; 25+ messages in thread
From: Igor Mammedov @ 2021-02-09 15:52 UTC (permalink / raw)
To: isaku.yamahata; +Cc: Isaku Yamahata, philmd, qemu-devel, mst
On Mon, 8 Feb 2021 13:57:25 -0800
isaku.yamahata@gmail.com wrote:
> 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.
could you point to concrete spec version/chapter where it stated.
(should be part of commit message)
> 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.
can parse this, can you rephrase and avoid double negation, please?
> 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
>
> The change to DSDT is as follows.
> @@ -68,32 +68,90 @@
>
> If ((CDW3 != Local0))
> {
> CDW1 |= 0x10
> }
>
> CDW3 = Local0
> }
> Else
> {
> CDW1 |= 0x04
> }
>
> Return (Arg3)
> }
> }
> +
> + Device (DRAC)
> + {
> + Name (_HID, "PNP0C01" /* System Board */) // _HID: Hardware ID
> + OperationRegion (DRR0, PCI_Config, 0x60, 0x08)
> + Field (DRR0, DWordAcc, NoLock, Preserve)
> + {
> + PEBL, 32,
> + PEBH, 32
> + }
> +
> + Name (RBUF, ResourceTemplate ()
> + {
> + QWordMemory (ResourceProducer, PosDecode, MinFixed, MaxFixed, NonCacheable, ReadWrite,
> + 0x0000000000000000, // Granularity
> + 0x0000000000000000, // Range Minimum
> + 0x0000000000000000, // Range Maximum
> + 0x0000000000000000, // Translation Offset
> + 0x0000000000000000, // Length
> + ,, _Y00, AddressRangeMemory, TypeStatic)
> + })
> + Method (_CRS, 0, Serialized) // _CRS: Current Resource Settings
> + {
> + CreateDWordField (RBUF, \_SB.DRAC._Y00._MIN, MINL) // _MIN: Minimum Base Address
> + CreateDWordField (RBUF, 0x12, MINH)
> + CreateDWordField (RBUF, \_SB.DRAC._Y00._MAX, MAXL) // _MAX: Maximum Base Address
> + CreateDWordField (RBUF, 0x1A, MAXH)
> + CreateQWordField (RBUF, \_SB.DRAC._Y00._LEN, _LEN) // _LEN: Length
> + Local0 = PEBL /* \_SB_.DRAC.PEBL */
> + Local1 = (Local0 & One)
> + Local2 = (Local0 & 0x06)
> + Local3 = (Local0 & 0xFFFFFFF8)
> + Local4 = PEBH /* \_SB_.DRAC.PEBH */
> + If ((Local1 == One))
> + {
> + MINL = Local3
> + MINH = Local4
> + MAXL = Local3
> + MAXH = Local4
> + If ((Local2 == Zero))
> + {
> + _LEN = 0x10000000
> + }
> +
> + If ((Local2 == 0x02))
> + {
> + _LEN = 0x08000000
> + }
> +
> + If ((Local2 == 0x04))
> + {
> + _LEN = 0x04000000
> + }
> + }
> +
> + Return (RBUF) /* \_SB_.DRAC.RBUF */
> + }
> + }
> }
>
> 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 <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 c2f11d95d8..bcb1f65c1d 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -1066,6 +1066,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.
version 1, is kept for Windows XP sake which is EOLed long ago,
and it even will boot with v2 as long as AML doesn't contain non v1
terms or those are hidden behind dynamic method which are not evaluated.
Perhaps we should bump dsdt revision to v2.
And anyways with this patch XP probably will trip over, if it tries to parse
64bit CRS, but we probably don't care about XP on q35.
> + * 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
> + * }
why are you trying to fetch it dynamically?
what prevents you from getting MMCONFIG address in QEMU when building
ACPI tables and encode _CRS statically at that time?
> + * 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;
> @@ -1250,6 +1421,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 */
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH v2 6/9] hw/i386: declare ACPI mother board resource for MMCONFIG region
2021-02-09 15:52 ` Igor Mammedov
@ 2021-02-09 20:02 ` Isaku Yamahata
2021-02-10 8:31 ` Michael S. Tsirkin
2021-02-10 13:37 ` Igor Mammedov
2021-02-10 8:28 ` Michael S. Tsirkin
1 sibling, 2 replies; 25+ messages in thread
From: Isaku Yamahata @ 2021-02-09 20:02 UTC (permalink / raw)
To: Igor Mammedov; +Cc: Isaku Yamahata, philmd, qemu-devel, isaku.yamahata, mst
On Tue, Feb 09, 2021 at 04:52:41PM +0100,
Igor Mammedov <imammedo@redhat.com> wrote:
> On Mon, 8 Feb 2021 13:57:25 -0800
> isaku.yamahata@gmail.com wrote:
>
> > 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.
> could you point to concrete spec version/chapter where it stated.
> (should be part of commit message)
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
Will include it in next respin.
>
> > 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.
> can parse this, can you rephrase and avoid double negation, please?
How about this?
Guest Linux checks if the MMCFG region is reserved by bios memory map or
ACPI resource. It failed, it falls back to legacy PCI configuraiton access.
> > + * 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
> > + * }
>
> why are you trying to fetch it dynamically?
> what prevents you from getting MMCONFIG address in QEMU when building
> ACPI tables and encode _CRS statically at that time?
My motivation is to prepare for TDX where ACPI tables will be part of
measurement. I wanted ACPI tables to remain same irrelevant of chipset
configuration which guest can change.
Thanks,
--
Isaku Yamahata <isaku.yamahata@gmail.com>
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH v2 6/9] hw/i386: declare ACPI mother board resource for MMCONFIG region
2021-02-09 20:02 ` Isaku Yamahata
@ 2021-02-10 8:31 ` Michael S. Tsirkin
2021-02-10 22:04 ` Isaku Yamahata
2021-02-10 13:37 ` Igor Mammedov
1 sibling, 1 reply; 25+ messages in thread
From: Michael S. Tsirkin @ 2021-02-10 8:31 UTC (permalink / raw)
To: Isaku Yamahata
Cc: Isaku Yamahata, Igor Mammedov, philmd, qemu-devel, isaku.yamahata
On Tue, Feb 09, 2021 at 12:02:58PM -0800, Isaku Yamahata wrote:
> > > + * 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
> > > + * }
> >
> > why are you trying to fetch it dynamically?
> > what prevents you from getting MMCONFIG address in QEMU when building
> > ACPI tables and encode _CRS statically at that time?
>
> My motivation is to prepare for TDX where ACPI tables will be part of
> measurement. I wanted ACPI tables to remain same irrelevant of chipset
> configuration which guest can change.
I mean we are encoding lots of things like PCI description which is
guest controllable. Is there reason to think mmconfig specifically will
change after measurement?
--
MST
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH v2 6/9] hw/i386: declare ACPI mother board resource for MMCONFIG region
2021-02-10 8:31 ` Michael S. Tsirkin
@ 2021-02-10 22:04 ` Isaku Yamahata
0 siblings, 0 replies; 25+ messages in thread
From: Isaku Yamahata @ 2021-02-10 22:04 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Isaku Yamahata, qemu-devel, Isaku Yamahata, Igor Mammedov, philmd,
isaku.yamahata
On Wed, Feb 10, 2021 at 03:31:57AM -0500,
"Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Tue, Feb 09, 2021 at 12:02:58PM -0800, Isaku Yamahata wrote:
> > > > + * 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
> > > > + * }
> > >
> > > why are you trying to fetch it dynamically?
> > > what prevents you from getting MMCONFIG address in QEMU when building
> > > ACPI tables and encode _CRS statically at that time?
> >
> > My motivation is to prepare for TDX where ACPI tables will be part of
> > measurement. I wanted ACPI tables to remain same irrelevant of chipset
> > configuration which guest can change.
>
> I mean we are encoding lots of things like PCI description which is
> guest controllable. Is there reason to think mmconfig specifically will
> change after measurement?
No in fact.
I'll switch to use MMCONFIG address in qemu.
--
Isaku Yamahata <isaku.yamahata@gmail.com>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 6/9] hw/i386: declare ACPI mother board resource for MMCONFIG region
2021-02-09 20:02 ` Isaku Yamahata
2021-02-10 8:31 ` Michael S. Tsirkin
@ 2021-02-10 13:37 ` Igor Mammedov
2021-02-10 22:03 ` Isaku Yamahata
1 sibling, 1 reply; 25+ messages in thread
From: Igor Mammedov @ 2021-02-10 13:37 UTC (permalink / raw)
To: Isaku Yamahata; +Cc: Isaku Yamahata, philmd, qemu-devel, isaku.yamahata, mst
On Tue, 9 Feb 2021 12:02:58 -0800
Isaku Yamahata <yamahata.qemudev@gmail.com> wrote:
> On Tue, Feb 09, 2021 at 04:52:41PM +0100,
> Igor Mammedov <imammedo@redhat.com> wrote:
>
> > On Mon, 8 Feb 2021 13:57:25 -0800
> > isaku.yamahata@gmail.com wrote:
> >
> > > 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.
> > could you point to concrete spec version/chapter where it stated.
> > (should be part of commit message)
>
> 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
>
> Will include it in next respin.
>
> >
> > > 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.
> > can parse this, can you rephrase and avoid double negation, please?
>
> How about this?
> Guest Linux checks if the MMCFG region is reserved by bios memory map or
> ACPI resource.
> It failed, it falls back to legacy PCI configuraiton access.
clarify what/how failed, pls.
> > > + * 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
> > > + * }
> >
> > why are you trying to fetch it dynamically?
> > what prevents you from getting MMCONFIG address in QEMU when building
> > ACPI tables and encode _CRS statically at that time?
>
> My motivation is to prepare for TDX where ACPI tables will be part of
> measurement. I wanted ACPI tables to remain same irrelevant of chipset
> configuration which guest can change.
ACPI tables are supposed to be read from QEMU after firmware configured
PCI subsystem, including MMCONFIG.
If configuration is changed after that MCFG table won't be correct anymore.
Given MCFG is statically generated, I see no reason to fetch the same info
dynamically from DSDT.
PS:
goal of having fixed ACPI tables is hard to achieve in QEMU,
it might be possible within single QEMU version for a concrete CLI configuration,
but any deviation from that may trigger ACPI tables change.
> Thanks,
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH v2 6/9] hw/i386: declare ACPI mother board resource for MMCONFIG region
2021-02-10 13:37 ` Igor Mammedov
@ 2021-02-10 22:03 ` Isaku Yamahata
0 siblings, 0 replies; 25+ messages in thread
From: Isaku Yamahata @ 2021-02-10 22:03 UTC (permalink / raw)
To: Igor Mammedov
Cc: Isaku Yamahata, mst, qemu-devel, Isaku Yamahata, philmd,
isaku.yamahata
On Wed, Feb 10, 2021 at 02:37:31PM +0100,
Igor Mammedov <imammedo@redhat.com> wrote:
> On Tue, 9 Feb 2021 12:02:58 -0800
> Isaku Yamahata <yamahata.qemudev@gmail.com> wrote:
>
> > On Tue, Feb 09, 2021 at 04:52:41PM +0100,
> > Igor Mammedov <imammedo@redhat.com> wrote:
> >
> > > On Mon, 8 Feb 2021 13:57:25 -0800
> > > isaku.yamahata@gmail.com wrote:
> > >
> > > > 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.
> > > could you point to concrete spec version/chapter where it stated.
> > > (should be part of commit message)
> >
> > 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
> >
> > Will include it in next respin.
> >
> > >
> > > > 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.
> > > can parse this, can you rephrase and avoid double negation, please?
> >
> > How about this?
> > Guest Linux checks if the MMCFG region is reserved by bios memory map or
> > ACPI resource.
>
> > It failed, it falls back to legacy PCI configuraiton access.
> clarify what/how failed, pls.
It should be "If it's not reserved".
> > > > + * 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
> > > > + * }
> > >
> > > why are you trying to fetch it dynamically?
> > > what prevents you from getting MMCONFIG address in QEMU when building
> > > ACPI tables and encode _CRS statically at that time?
> >
> > My motivation is to prepare for TDX where ACPI tables will be part of
> > measurement. I wanted ACPI tables to remain same irrelevant of chipset
> > configuration which guest can change.
> ACPI tables are supposed to be read from QEMU after firmware configured
> PCI subsystem, including MMCONFIG.
> If configuration is changed after that MCFG table won't be correct anymore.
> Given MCFG is statically generated, I see no reason to fetch the same info
> dynamically from DSDT.
Ok, given that other part of ACPI table generation code,
I'll switch to use MMCONFIG address in qemu.
> PS:
> goal of having fixed ACPI tables is hard to achieve in QEMU,
> it might be possible within single QEMU version for a concrete CLI configuration,
> but any deviation from that may trigger ACPI tables change.
>
> > Thanks,
>
Thanks,
--
Isaku Yamahata <isaku.yamahata@gmail.com>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 6/9] hw/i386: declare ACPI mother board resource for MMCONFIG region
2021-02-09 15:52 ` Igor Mammedov
2021-02-09 20:02 ` Isaku Yamahata
@ 2021-02-10 8:28 ` Michael S. Tsirkin
1 sibling, 0 replies; 25+ messages in thread
From: Michael S. Tsirkin @ 2021-02-10 8:28 UTC (permalink / raw)
To: Igor Mammedov; +Cc: Isaku Yamahata, philmd, qemu-devel, isaku.yamahata
On Tue, Feb 09, 2021 at 04:52:41PM +0100, Igor Mammedov wrote:
> > ---
> > 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 c2f11d95d8..bcb1f65c1d 100644
> > --- a/hw/i386/acpi-build.c
> > +++ b/hw/i386/acpi-build.c
> > @@ -1066,6 +1066,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.
> version 1, is kept for Windows XP sake which is EOLed long ago,
> and it even will boot with v2 as long as AML doesn't contain non v1
> terms or those are hidden behind dynamic method which are not evaluated.
right, this are doing lots of dynamic stuff, generally it is easy to hide
things there. E.g.
if (high != 0) return qword else return dword
> Perhaps we should bump dsdt revision to v2.
I agree that bumping up to v2 is likely safe ...
> And anyways with this patch XP probably will trip over, if it tries to parse
> 64bit CRS, but we probably don't care about XP on q35.
Yea I'm not sure why are we using qword here.
--
MST
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v2 7/9] i386: acpi: Don't build HPET ACPI entry if HPET is disabled
2021-02-08 21:57 [PATCH v2 0/9] ACPI related fixes isaku.yamahata
` (5 preceding siblings ...)
2021-02-08 21:57 ` [PATCH v2 6/9] hw/i386: declare ACPI mother board resource for MMCONFIG region isaku.yamahata
@ 2021-02-08 21:57 ` isaku.yamahata
2021-02-09 15:54 ` Igor Mammedov
2021-02-08 21:57 ` [PATCH v2 8/9] acpi: add test case for -no-hpet isaku.yamahata
2021-02-08 21:57 ` [PATCH v2 9/9] qtest/acpi/bios-tables-test: update acpi tables isaku.yamahata
8 siblings, 1 reply; 25+ messages in thread
From: isaku.yamahata @ 2021-02-08 21:57 UTC (permalink / raw)
To: qemu-devel, imammedo, mst, marcel.apfelbaum, philmd; +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.
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é <philmd@redhat.com>
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 bcb1f65c1d..73ec0b6d32 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1405,7 +1405,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) {
@@ -1450,7 +1452,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] 25+ messages in thread* Re: [PATCH v2 7/9] i386: acpi: Don't build HPET ACPI entry if HPET is disabled
2021-02-08 21:57 ` [PATCH v2 7/9] i386: acpi: Don't build HPET ACPI entry if HPET is disabled isaku.yamahata
@ 2021-02-09 15:54 ` Igor Mammedov
0 siblings, 0 replies; 25+ messages in thread
From: Igor Mammedov @ 2021-02-09 15:54 UTC (permalink / raw)
To: isaku.yamahata; +Cc: Sean Christopherson, philmd, qemu-devel, mst
On Mon, 8 Feb 2021 13:57:26 -0800
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.
>
> 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é <philmd@redhat.com>
> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.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 bcb1f65c1d..73ec0b6d32 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -1405,7 +1405,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) {
> @@ -1450,7 +1452,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);
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v2 8/9] acpi: add test case for -no-hpet
2021-02-08 21:57 [PATCH v2 0/9] ACPI related fixes isaku.yamahata
` (6 preceding siblings ...)
2021-02-08 21:57 ` [PATCH v2 7/9] i386: acpi: Don't build HPET ACPI entry if HPET is disabled isaku.yamahata
@ 2021-02-08 21:57 ` isaku.yamahata
2021-02-09 15:59 ` Igor Mammedov
2021-02-08 21:57 ` [PATCH v2 9/9] qtest/acpi/bios-tables-test: update acpi tables isaku.yamahata
8 siblings, 1 reply; 25+ messages in thread
From: isaku.yamahata @ 2021-02-08 21:57 UTC (permalink / raw)
To: qemu-devel, imammedo, mst, marcel.apfelbaum, philmd; +Cc: Isaku Yamahata
From: Isaku Yamahata <isaku.yamahata@intel.com>
Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
---
tests/qtest/bios-tables-test.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
index 096d15db68..72c8765baf 100644
--- a/tests/qtest/bios-tables-test.c
+++ b/tests/qtest/bios-tables-test.c
@@ -980,6 +980,17 @@ static void test_acpi_q35_tcg_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;
@@ -1337,6 +1348,7 @@ int main(int argc, char *argv[])
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/q35/nosmm", test_acpi_q35_tcg_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);
--
2.17.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH v2 8/9] acpi: add test case for -no-hpet
2021-02-08 21:57 ` [PATCH v2 8/9] acpi: add test case for -no-hpet isaku.yamahata
@ 2021-02-09 15:59 ` Igor Mammedov
0 siblings, 0 replies; 25+ messages in thread
From: Igor Mammedov @ 2021-02-09 15:59 UTC (permalink / raw)
To: isaku.yamahata; +Cc: Isaku Yamahata, philmd, qemu-devel, mst
On Mon, 8 Feb 2021 13:57:27 -0800
isaku.yamahata@gmail.com wrote:
> From: Isaku Yamahata <isaku.yamahata@intel.com>
>
> Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> ---
> tests/qtest/bios-tables-test.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
> index 096d15db68..72c8765baf 100644
> --- a/tests/qtest/bios-tables-test.c
> +++ b/tests/qtest/bios-tables-test.c
> @@ -980,6 +980,17 @@ static void test_acpi_q35_tcg_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;
> @@ -1337,6 +1348,7 @@ int main(int argc, char *argv[])
> 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/q35/nosmm", test_acpi_q35_tcg_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);
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v2 9/9] qtest/acpi/bios-tables-test: update acpi tables
2021-02-08 21:57 [PATCH v2 0/9] ACPI related fixes isaku.yamahata
` (7 preceding siblings ...)
2021-02-08 21:57 ` [PATCH v2 8/9] acpi: add test case for -no-hpet isaku.yamahata
@ 2021-02-08 21:57 ` isaku.yamahata
8 siblings, 0 replies; 25+ messages in thread
From: isaku.yamahata @ 2021-02-08 21:57 UTC (permalink / raw)
To: qemu-devel, imammedo, mst, marcel.apfelbaum, philmd; +Cc: Isaku Yamahata
From: Isaku Yamahata <isaku.yamahata@intel.com>
update golden master acpi tables and empty
bios-tables-test-allowed-diff.h.
Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
---
tests/data/acpi/q35/DSDT | Bin 7801 -> 8083 bytes
tests/data/acpi/q35/DSDT.acpihmat | Bin 9126 -> 9408 bytes
tests/data/acpi/q35/DSDT.bridge | Bin 7819 -> 8101 bytes
tests/data/acpi/q35/DSDT.cphp | Bin 8265 -> 8547 bytes
tests/data/acpi/q35/DSDT.dimmpxm | Bin 9455 -> 9737 bytes
tests/data/acpi/q35/DSDT.ipmibt | Bin 7876 -> 8158 bytes
tests/data/acpi/q35/DSDT.memhp | Bin 9160 -> 9442 bytes
tests/data/acpi/q35/DSDT.mmio64 | Bin 8932 -> 9214 bytes
tests/data/acpi/q35/DSDT.nohpet | Bin 0 -> 7941 bytes
tests/data/acpi/q35/DSDT.nosmm | Bin 0 -> 8083 bytes
tests/data/acpi/q35/DSDT.numamem | Bin 7807 -> 8089 bytes
tests/data/acpi/q35/DSDT.tis | Bin 8407 -> 8689 bytes
tests/data/acpi/q35/FACP.nosmm | Bin 0 -> 244 bytes
tests/qtest/bios-tables-test-allowed-diff.h | 13 -------------
14 files changed, 13 deletions(-)
diff --git a/tests/data/acpi/q35/DSDT b/tests/data/acpi/q35/DSDT
index d25cd7072932886d6967f4023faac1e1fa6e836c..00c60fca65ec37200fc0a17db2d6eb895a1553d2 100644
GIT binary patch
delta 314
zcmexqGufWYCD<iovOEI=<C%$EsZ6di6AM??M>ly0x&%2obHsaiy6^`01sFIR7&1gR
zxC8|mFmWYtaYQ!?fY{6du1-D*K*B?TBgiS#P0*Ojpi7&9frpWSNKnZj;>#8992Csa
z1u}q(&)3t>ryjx<0<t|IOfg@_2p<Sj3dr>62P;;K_i^<r0J$NdI)O2<I)RHVsXBq@
z$N&HTlR?5BDGPj9CMPl$B!f&XNCBA#WCM)?vVo>7;G3MpP{0FpssPBO1^knfxR`)^
Y83qOpxBv@S0Skz~1Z3-GDaLqN09%VpH2?qr
delta 29
lcmbPi|I>!cCD<jTQjURv@!LeMR3>k(iG?dSn=r-80sxN%2{r%#
diff --git a/tests/data/acpi/q35/DSDT.acpihmat b/tests/data/acpi/q35/DSDT.acpihmat
index 722e06af83abcde203a2b96a8ec81fd3bab9fc98..47547fa043d102578f6613cb7f19009a1ee5a868 100644
GIT binary patch
delta 314
zcmZ4He!!E<CD<k8fC>WxW9LM!R3=xMiG?fcqnkVgU4k5)IpRG$U3dfh0t}oD3>l&u
zT!MlOn79(SIHDT`Ky2m!S0^6@AmO3F5#$u=CTPrM(5215z{AKuB&cK%@#Ttl4hrVz
z0vW)?=j-X`Qx9Pa0ofi9rkJl|gb#!%1!Q{kgB7dA`?&fQfZUK!oxqq_oxsJGRGq-{
z<NyEv$sl2mlm$L4lM@*Wl0l{xq=3u=vVlec*+5ek@J&u)DBuA)RRCnt0{+QKTueZ|
Y3<CoPT!00vfCa=~0<v|p6l0GP05g_MMF0Q*
delta 29
lcmX@$xy+r*CD<ionKA<d<K~H6sZ8En6AM>vHeu>f0sxKZ2}S?_
diff --git a/tests/data/acpi/q35/DSDT.bridge b/tests/data/acpi/q35/DSDT.bridge
index 06bac139d668ddfc7914e258b471a303c9dbd192..ab0d94da7d3dce23d18d5868760f5fd5ac6f7958 100644
GIT binary patch
delta 314
zcmeCSU24ze66_MPRGxu>F?AwWDwC_s#KM*J(M=wLE<ujY9Pu8WF1!JL0S3+nh78dS
zE<r&COk4?E9MO#eAU1P=tCNocknm982yzN_6Ex;B=+b6j;9+DS5>zsX_;STN2L*F<
zfehf{^Y!%esfVzIfNT#4Q_R;f!Uw{X0x~`N!HU)5eO&ztKyFB=PGC%|PT*oos!rhf
z@&Et-WRS2&$^svj$%%{w$skh;Qb6Vb*+8R!Y@jI%_$DVY6z~9@DgZKR0srJAE+!yf
YhJk?tF2Dj-zyjhg0ol4)im^}@0EnAQmjD0&
delta 29
kcmZ2#-)+m~66_MvEyuvX*guggmC2iHV&TfoCQOC00D%$+m;e9(
diff --git a/tests/data/acpi/q35/DSDT.cphp b/tests/data/acpi/q35/DSDT.cphp
index 2b933ac482e6883efccbd7d6c96089602f2c0b4d..19f75894abfc33762dc4e2b973f17bb9e989e82e 100644
GIT binary patch
delta 314
zcmX@<@YspVCD<h-S&@N((O@E1DwC_s#KM*J(M=wLE<ujY9Pu8WF1!JL0S3+nh78dS
zE<r&COk4?E9MO#eAU1P=tCNocknm982yzN_6Ex;B=+b6j;9+DS5>zsX_;STN2L*F<
zfehf{^Y!%esfVzIfNT#4Q_R;f!Uw{X0x~`N!HU)5eO&ztKyFB=PGC%|PT*oos!rhf
z@&Et-WRS2&$^svj$%%{w$skh;Qb6Vb*+8R!Y@jI%_$DVY6z~9@DgZKR0srJAE+!yf
YhJk?tF2Dj-zyjhg0ol4)icw!40Gy{wWB>pF
delta 29
kcmaFtbkc##CD<jzQ-OhjF=`@LDw8+Y#KM)EO_=oM0fwarWdHyG
diff --git a/tests/data/acpi/q35/DSDT.dimmpxm b/tests/data/acpi/q35/DSDT.dimmpxm
index bd8f8305b028ef20f9b6d1a0c69ac428d027e3d1..d7344a4e1328318510fe26508a608a985b5d44eb 100644
GIT binary patch
delta 314
zcmaFw+3CaO66_Mfsm8#-_<JH(DwC_s#KM*J(M=wLE<ujY9Pu8WF1!JL0S3+nh78dS
zE<r&COk4?E9MO#eAU1P=tCNocknm982yzN_6Ex;B=+b6j;9+DS5>zsX_;STN2L*F<
zfehf{^Y!%esfVzIfNT#4Q_R;f!Uw{X0x~`N!HU)5eO&ztKyFB=PGC%|PT*oos!rhf
z@&Et-WRS2&$^svj$%%{w$skh;Qb6Vb*+8R!Y@jI%_$DVY6z~9@DgZKR0srJAE+!yf
YhJk?tF2Dj-zyjhg0ol4)it(y40RE&*V*mgE
delta 29
lcmeD5dGE>P66_N4UWI{yQEehuDw8+Y#KM)EO_;7K0|1Tx31k2O
diff --git a/tests/data/acpi/q35/DSDT.ipmibt b/tests/data/acpi/q35/DSDT.ipmibt
index a8f868e23c25688ab1c0371016c071f23e9d732f..57c32c31a3fd40f5df01f743f062a5dcaa9b8231 100644
GIT binary patch
delta 314
zcmX?Nd(WQBCD<k8o;(8sBf~_lR3=xMiG?fcqnkVgU4k5)IpRG$U3dfh0t}oD3>l&u
zT!MlOn79(SIHDT`Ky2m!S0^6@AmO3F5#$u=CTPrM(5215z{AKuB&cK%@#Ttl4hrVz
z0vW)?=j-X`Qx9Pa0ofi9rkJl|gb#!%1!Q{kgB7dA`?&fQfZUK!oxqq_oxsJGRGq-{
z<NyEv$sl2mlm$L4lM@*Wl0l{xq=3u=vVlec*+5ek@J&u)DBuA)RRCnt0{+QKTueZ|
Y3<CoPT!00vfCa=~0<v|p6yqvc0O9;h761SM
delta 29
lcmca-f5eu{CD<k8h#Uh0qt-;OR3>k(iG?dSn=q}C1pthN2^RnW
diff --git a/tests/data/acpi/q35/DSDT.memhp b/tests/data/acpi/q35/DSDT.memhp
index 9a802e4c67022386442976d5cb997ea3fc57b58f..ae63519a139cf287cd6a1484c8298d17ee8c70dd 100644
GIT binary patch
delta 314
zcmX@%{>YQdCD<k8kqQF?<GzVpsZ6di6AM??M>ly0x&%2obHsaiy6^`01sFIR7&1gR
zxC8|mFmWYtaYQ!?fY{6du1-D*K*B?TBgiS#P0*Ojpi7&9frpWSNKnZj;>#8992Csa
z1u}q(&)3t>ryjx<0<t|IOfg@_2p<Sj3dr>62P;;K_i^<r0J$NdI)O2<I)RHVsXBq@
z$N&HTlR?5BDGPj9CMPl$B!f&XNCBA#WCM)?vVo>7;G3MpP{0FpssPBO1^knfxR`)^
Y83qOpxBv@S0Skz~1Z3-GDaLh50PUPj0ssI2
delta 29
lcmaFldBUB`CD<k8gfasI<BN%0sZ8En6AM>vHep()1OS^~3IhND
diff --git a/tests/data/acpi/q35/DSDT.mmio64 b/tests/data/acpi/q35/DSDT.mmio64
index 948c2dc7264c31932b490ca00691a7c4d9aefdb0..689ff7d5a75bb3a1f98b81e6c0e5748528e3720e 100644
GIT binary patch
delta 314
zcmaFj`p=!qCD<k8pE3gjWAQ|;R3=xMiG?fcqnkVgU4k5)IpRG$U3dfh0t}oD3>l&u
zT!MlOn79(SIHDT`Ky2m!S0^6@AmO3F5#$u=CTPrM(5215z{AKuB&cK%@#Ttl4hrVz
z0vW)?=j-X`Qx9Pa0ofi9rkJl|gb#!%1!Q{kgB7dA`?&fQfZUK!oxqq_oxsJGRGq-{
z<NyEv$sl2mlm$L4lM@*Wl0l{xq=3u=vVlec*+5ek@J&u)DBuA)RRCnt0{+QKTueZ|
Y3<CoPT!00vfCa=~0<v|p6yqsH0QIv?ApigX
delta 29
lcmez8{=}8bCD<k8i4p??<J^f{sZ8En6AM>vHeouY2mqe~3L*di
diff --git a/tests/data/acpi/q35/DSDT.nohpet b/tests/data/acpi/q35/DSDT.nohpet
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..73a7826f92a3ac99855c1d1ff1e9d72104e13e3a 100644
GIT binary patch
literal 7941
zcmb7JTW=f38J*=#t0gHeDO$4Siv%}KuZSthX@eFuN-nPvDN&?UC!hdQqSDH7fh=MR
zh+zb<6`*nS&=w&a^pQ3cpl^NaYyA`YRs;P9eeFZ^RQ7z|aO4@10&G5zb7s$(Z_eJ%
zawp%+Z-p0yu%6~NymE6X_d(suqt6mTP@8_;N@z#49_4eTwiOM8tk&ZwxAB#;(?8AU
zwoBGu4m#g-I`_ZqSi8R1d+t8n+x5SA(%BP4I^6RehZ=6-O1qhBw~jog?o~2&yIje~
zvj22!yY0zV&~9h*4Pm#pD|wNwPPI3hUg-F)@bb5p`|IJ}L2}k{TeW|$ez$n#*WcW}
zpMCMGfBp9Ot`h=b1-~|aSAF{GJnT4&oxb~Z?~e63$$8syfA;GhZ<c*7ZSgzQ^DHL2
zXsG3ryRD;gGT+F0Jjr-{y`J$h%d{IBjZDDlICkr?OYLC2kgG1?tGr}4a@)n^s*^eX
zi4ZZ`b+7e#AksCb?KPVAF%96%)GF&$*-W8Xmd&|Z_Jd*X0-EJNG576uwXp8f6yVp|
zh3ClL!%pmUFc@^HyS(48hI+e}-=#tQeHssK423pE>Cg1`gdf8^T^gyM=<PWH#`YNw
zF%A(vp`z~F@6b`CBiK4xs5G-}K4=(Z5&NfJX>8K|iMeZu8$m@RR_o~IS}qT=JnBIl
z>*iM0N#Ni&n#qgKg_6|~9bs7|YhUc+_j*7)5Ur!PY8%^Us>H0JvYqgC;Sy`5THjh$
zV~6A!WEou7iI=R;f}aI#`}!judBxkPooT{g<E)#YISrl$g?-&+&G#Ey-q~!Ts6&(I
zp?%>C%f2qG2g0sbyrM|Q+W58n148Z}KeT9XxZZ#G+^;vb>Z!GRk5bW4bTAOr^|gYK
zZ42$9NX4k%2>_H72m+KvDjtdr1~|PWZ-__*;*8wJI#3lPCB|oNfR*455h)clAXo`1
zD*;J~*)TWAN;QL^Af=)Pr4m$D0+Lcy39(YmAjC>V4N0Y%JfQ9~Z1@bbQc=TFsU{Do
z`-~VqBdk=^i0Knh_c>+woHBe)nLYt^pSI!CHhkKqPe9%0wBd8w@HuVz1k`;xhEK=v
z>6ktNb)QkgXVmZ+HGKl=K5?<jW6$SB%<vgAeFEw}<A%?;;WKXf1k`=b7(QnVpEIUU
zK;37;@R=}tCQP4zx=+{e=^8#=(<h+rGimrt8a|VzPe9#g%J7*oe5OpFfV$6F!{@Bw
zbJp|;sQXMCKGTNJwCNL2_j$qadBN~`!So5J`<yd;&KW-EOrL<d&w0b=yy0`+^a-f@
zylD8mX!yKn`UKQ{USef-cqhEX%G~f~cS$O}SCEwXA$P%0E*Q!MQwgXmFB{6shVrth
z1k{yR4CNI=dBs!$>dLEz@~WY{YAOMB<uz8Sd;B$4s(bu3sZ=*^KvJqC!2>9e+P8~F
z%|){&psZ;lK(ktamYNlm<}kI;M&V(A0-&q}K>$@00V;WX3$p~N)ch<ImDIBU1yt4$
zppvTsRHIUWO6pmF0%A5QKqYsGs3t~LiAhSe$0$GnF(c+8KqYsGh8|f-0V=5q>OK*m
za#&U(Ks71_D4<G33Q$1h69Fo@tVDonR0>c@<(d?rfXXKVRB~B~0M)1zppwc;3Q$1h
z69Fo@tVDonR0>c@WhDhDpgNuiP|0N_0#u_?fJ!PWDL?_0PXwssvJwHRQ7J$rm6a5r
zfXXKVRB~B~0M)1zppwc;3Q$1h69Fo@tVDonR0>c@WhDhDpz?_Tm0VULKs71_sHC!z
z0u)gBM1V>zD-oa?l>$^!SxEs3sC*(oC6|>5P>o6fDygib00mS&5ulRGN(87zr2v&w
zR#JchDxU~Y$z>%1RHIUWN-8TUKmnCc1gPY)5&^1FDL^Hal@y?W$|nL;a#@K0)u<Gp
zlFCX7P(bAq0V=tyM1X2k3Q$R9B?Ty;@`(VITvj4LH7W(Dq_UC%6j1p@fJ!ba5uh5C
z0#s62NdXF|d?G+4mz4-mjY<J3sjQ>`1ynu}ppwf<1gJ)(0F_i$Qh)*~p9oMOb(2Sc
z0_gz?s0S#Z7ND9E0jen}Ks6-=sHQ}KYDxsCrlbJXloX(v5&^0y5uloq0#s8{fNDwv
zsHQ}KYDx-FO-TW&DG{JR>gXasfpEllXHtLy;xH&kO0{nhpg?NhB0z!EzNG*KRQr|!
z6cBG$7TSnV(U0}`&*};NA$=C1pKR;#FKz_sUn>1=r$6*oy|I}^;%?^X%cHNnEtc?8
zpmveIO7vB>)3cS$=9bl;e&{?p5GT&`i4#rVtZaIXX%HHj<ct&jDk7%&^~eKpOnpum
z-xdGifs+u_!Xz4aw@{s;rvmk#K86<s^j65mw42E}XZEWZF&#v@3EaO0?!@?eo=(s!
z1#v=Cz-+SVWIJDJiqEO-JmQ`N{b47D*I+!~ZZ?|M@xgEoRG#{W)pH}&@pj;~tLL=p
zIj)|={PgO%^yKQf)=`bT%a>1hLpJp81m2*$r<M1(yf;$bJEy!iET11S@8FHiYrD^D
z<?~!VKT<w_PWk+>d||YF=ncvjwDJWmUl=K0IH!DJSiU$~KKusdi(2_2moJW#FP>Ar
zI4oZpEgyM<@+GZ&iOZKp%9qY5UmBJ#kCvZ$gYspqe3{FaN6MGaDPKnU5Z%f~%e%6?
zx_RKS0++jd`DXT|BYer@hpy}4aHE&8lQf6Jv&PtPvTx#WW7lQM91ag4W5db5iNlRu
zXL!d+*O@Ygr;@SZWZ%T$#;%*RIUF8U#)gx96NekS4lbC(;rV53IN3LGxUutk&KwR8
zHDklczKO$)ou~8WaCp)g8&39_!?~BWm)ePDEr-<!tVG0uc&Y7f+>*-{WoPHrEA4ut
z&{SusgNqZ{6!+@eOXW}Q`@cW@-O{HY-TUP5)9y$2M9cE()wSW-8h4zM^)mJ{X2tN@
z8kvN!jJ}LHvDV`s63B`ruU=TUyvlmh@*1_@SYF}%41E<X5%j!T9xD$iVg|itp}GWm
zKD)Jqcg>acitP*P#96<(!+tO!Imvj|t5-6a<qS<juC;{INpGw8VW98y4#Xe4OeS+D
zU5<E|!gT9<J2yh=-8~s41A~EoFxrh`zUcH9dZQc2g6||ctsjhSsji)L(zP(zdb2{-
z=ujA8;`r&}oY(Z$?Q?9{-g@D7DlqUx*SGec=Tp6X%NP5;)$zsd^J?tRy^i&?i>{8}
z9WBX*M>}y)`twgmdvRF0-}a|_yUqY*zV#<<@4q#+=jgcm-6u51U2(XJSv~uC|J^Yf
zpBXrOT_@;|?HFu??yyU1afOgvjH3bfM3)XZymed4lg*8dh4~!rP2SD5X>X&s6+34)
zPd;15#>dX{?@ZV?z7rVjCF;kco!HG2XV?1TJvygx{o{kn1Lw7ULT5LwfLeAd8Fg%d
zw+-|hnx4Tr<5rZ+h=V&Hr!V8;Db)+@LS{t_uXwtU=$vxXw1uzx;U$&%nQ*g7dAZ3|
zO<ECpKkEGbNn8YIX-$X$ePLbeG<F)ZZ1h{WvEcI^HlN1Ma2yLkLj3nj+@R!kdbS-5
z=-HM&0l{1N$r;ebBH!P5gAji;w!9b&{_#J#<;5*tjO1!VE?b6n0Qbg+FfFBdAy|~F
zv%K(`g++OPqkiqiCYRTuGKTrawiH>4SXhs9f3yQhbG4m{ge*)L?VIhC9dZH=-kt%y
zOIxVA`^#6mu66WI@wO)eC%##zZR7Z@5F5*cdKEY9hUxa{7DGF{rglp%Mn;>Nj9iR_
biyt>L?@552Yzc&5b2%?gxUr%dtJL^E0NhP#
literal 0
HcmV?d00001
diff --git a/tests/data/acpi/q35/DSDT.nosmm b/tests/data/acpi/q35/DSDT.nosmm
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..00c60fca65ec37200fc0a17db2d6eb895a1553d2 100644
GIT binary patch
literal 8083
zcmb7JOK%(38NI`YG#ZYiAw|o!{1V(WeIlkLCk<NEC^^GdM9LIN#|dbF)FZ7N7sw#C
zfVf5gTLBVBi?#{tpiA0NfbP2Mw*CQKcvk~mb=z%G#4EGs`!271FG&G57v!8f=iKj}
zJC8HG6EuQm_nZ*cKXRA-Qe!dqUd<2CW(gr^n|{4=?3!rZ4RXb{m59Ww*1ZI8<11&U
ze;VYj6s<q+biV0yZhg_QHbSxa$a}E45q@^RvnhzQyBRtTZFu?9?MANM+Vh>7U(VR=
zQaNz5PaNBB`*JI4x3fWA*zGIjK%^^^?aK{6wtqwT!PTYSTD-fHoO8TZ^<OLBojd)j
zZ?4|TKK|uDf4zUhiGi?;UmL$GA$@i3bewaYp7(Y4y7ei^dEZiR?&~h^mTeyG2|LvC
z(3MTp)zZO@)?O(Y)U!Sha=Nxw%lMfknudBk6LC6@-MZ(|cC?nyRTlA8TD0rAD~05W
zliB}?5H3yKOMPyObk%A5^@hDq9XK=9@>)f1rm$I(oAcG|dxO>mY?l7O+}GQc{8~su
zfM06^o+mc%blhkCet(OaOWVClth-@_ThwW|P5ogTU11x&^k%x7B6KlMk9z9Ob~l{}
zWBU-t7{>@7P*JaH%cGdLOSODc9YqVrj*eREZmHI+3iSEpc|yc6O@!y(&94QEwR+HA
zOuyA`G_xY4mQF}b8^7x~UZ>}N)$NFGC(-X8h~CUs-3{@l4Ojg8fAIrx;LPG2sW*~I
zXXf)Mn${=JMNEj>!h8J<8V#LKt-XbEBirUv2<6XVD7A8ZosO34t<GMEDk8C3dly%8
z0m$+kjAHZ{l`3<>Iaahfq9ZJ;Xl;va{GN}XT5IpM>g6kks@XY1Wjpcn!XwslrPf?h
zeaGbaY#Ch7nJ!wNL_dkz_W8Tq^Rj=rdZ>wmjoBQdc@}*Z74~_LHQ%o{{ln2DP>1G-
zyADMtEc?8$ZVS6s_6s8Iw()C)JA}O7zi-iOc>aI*O03nJwbbg(yQxGB(=RG(t9c=}
zEo>J=%B6NE0#H;S3Q!WM>6qK^<7Ah-0U{NMGjbp6Kvj^G=%2X}R)RY~q*T<1U?r%m
z1SBO!!`vt<)d-@3l!_XaN>Et|NJ>=&CywNDKQUG+YD_BC-~n}?al>bvm5LgdN;P;u
z-RFeibApwMI$`<*)O}7GJ|_*IlcrBV-KTB%v<;uO=@U@*iAzkI=_$kKl<5;t_vsiu
z9mA(%`UKQ{CJdhm!)L<u38?$T#jYJs*YN3@J^^)~(}vG!!{@Z=6Hxa#WB8mge9o9Y
z0d=3VhR<2U=d9@yQ1|H>K0U*yXZi%xeI^Z`NyBH-^a-f@Oc_2?hR>Ag6Hxa#XZV~m
ze9oCZ0d=2g!)My?nKpd_>OPMdK93nbkC{FJb)WNw&w0b=yy+8A_j%m#dED@M-1G^k
z`#fRzJYo1eVfqBreV$}xc5o*=$;#Z|W_MC5{TGmw!GOD9C>IRnf~f@5m8T5lDMNY6
zR08VC(}wc2p*(FW0d?gWLwUwfo-vhxy7DY5Rmz-YrMkzTl}dHv1|+3Q5<CtAskuF8
z)I4X_1e7&x1ZY+Z&|;&E(j2B1wo!Nxpa3W<K@>n0MSw~kN0KE-rN(EWsH7eSD4?>2
z0F_)7pc<6|R8kKE6cD3P0V=rzL^bsQmDIxk1;mJ$ivX3}0UCH@Dg~&dDyaKJfXZ%J
zi2&886rg}A6)8Xgl}`ky<gyY0s!=IGC6#MZfC4I?2vEsoB?45VQh-V-D=9z$l}`ky
z<gyY0s!=IGC6$#Fpn&RlB0wdVl?YIcN&zaVtfT-1R6Y@)lFLd2s79p#l~h(zfC4I?
z2vEsoB?45VQh-V-D=9z$l}`ky<gyY0s!=IGC6$#Fpn%FJ0#tHYi2&886rhsIN(xXw
z<r4uaxvWHhYE%kPNo6GkD4_C*0F_);B0x1N1*oL5k^&S^`9y$9E-MkB8kGW6Qdvm>
z3aES{KqZ%z2vChm0V=7iqyPm}J`tdj%Sr^OMx_9iR8~@e0xF*fP|0N_0#u_?fJ!PW
zDL?_0PXwssvJwHRQ7J$rm6a5rfXXKVRB~B~0M)1zppwc;3Q$1h69Fo@tVDonR0>c@
zWhDhDpz?_Tm0VULKs71_sHC!z0u)gBM1V>zD-oa?l>$^!SxEs3sC*(ofz(YN0ScrC
zD4-sofLefRN(88;qyW{F6rh?C0jenxpqi2bR8vxbYDxsCrbK{hN(xX-Ndc-U5uln9
z0jen}Ks6-=sHQ}K0;!{m00qJk<H@7|1;k-ckd$g}5uiY7ZV{kBYHle&0oB}6fC6Hz
zXQ7P<6+O4he^w9Z59z}R`pLHL{rp0d{-x62cKSna)#~e6B<^~SzI^%$Y=Omv^*n7C
z=&MLyB|AM=UT-w5-qaoE?haNFrVgA$`eJ$AuTOzc&m?D@#FrCdiWh5ci+yTy;`lCy
zcXpgvL0cF^9cv4fNqQ<!|LI+<N6>1AjbS&E^UmCtGh!-=bQ8FL3*3qE_dGqz3mpeE
z1dJw|PPT(`Lwrix&RuSqr9bQf48P9!c)L+=So=GJF;IDGA5_l`RmbYUOIOcn)pJ}u
zhw<svbLsKbbFIB9d6!FJSg#v+cOtJ)-q*_eT;3ll?;la#ACwP<%sW`Id1?27RzBeJ
z!BF|&i1NXpe15om>=nxAweoo`pC2lpKcakoP`)r+KK=^j3tIUCmoE&JFC0<6FeqOf
zE<f=K<%?SRB9|`?l`kGqzBnjf8ZJNi3gt^$`4X2e4V5n)QND!oF}jrvm-l3Ob@RYu
z1ul2F_-3}HC-{=b4_()T?uIX8Cuw$vXN{5WWZT&8My|`0*&QB0M!J)2W4jx<&alQw
z*O@Ygr;?HGWZT&8My{K**&QBLM!J)2W4jx<4vv}K;rV5xJJ~k2yOHyH-s}z!H6z{0
zwz1ufoTtal?(n2D(w%HGyYo(Ii`uh|Y7Xy8@Lt4?Vo}?>d`T|9mz--amb7d2d_$e3
z4lYh))3jf^vRL})R`}c9-z<Lo!Of3$Ki>M_rf6Ayt+F~eTc;hTXgzhGx|WM&Yh)6_
zGV#=P+}6Dx63B{0zm{LK{PJ4E^6S-iEkFNGhQ11xi28mtz<ZDsF{6GXUs(h_$Tk<T
z)?8jI+o7OFob@Z$*bfFI2RWVfYvoL4DMQ1MZ+_zGq~9#OAL%*W9r3xJ$z-mlOA|hZ
zFxC3rwF@z|c27phK))aE3^$`#C_24`?(hz>5IVD+)(=MZR8|i<>1v#8y;>n_xGVH9
zw*Pct-f#G8_7OI0e=UDC73qg!E3~#B1*z_~6^iZ9>V#tBQN{gZw_`onLQ^N)8ZOC(
zhZ}KFddD9Ox8ksDecPMrZa96E`8FK4zxUe6%+YZVx9-yzH^lA+M)mM#y*Ec_d}iSA
zZ8_0!WMZ%ry4@{$GnkLbSA*2yrr4rG4sTnnrSaxQ`oeg2H^)zNb;`foXu3yC^Z2u6
zq<>6aczw*=gid6*m8c&MH)5K{j;{6D+jLIj`o{;C2hMBzfX;4Q0o80XnQ&}@)dqSF
zP0!%1XEQ-&#Lo2()2DFp6l?i*KC>(aS3F%vbWV9`+QZlV;F8L`CA@4>UT*RoE4>kV
zC*l0<{<MhTe?C5a`oepyXYMoCve9ng#)8jx*!|2s#M5{YB*cHezzs_7r)S%KpPp^$
z6A`?JpPT`0ymI_2?-1fI#-7Lh{@<U=J&!N(t5L2t;Id_C`*3f32-B-HKL(5PT`|9g
z&BCI*zfrq(W0SAf5;BJQ#<n=IIAP&^ocE(^kTh1>sfn0{0b~1OJ7ve5h=bKLpf_m`
zRrh}JV(MCZZxpWjGH|BX^VKUje#^whYeKz>7x&_H`}7L2Yy3{_l6)DNXk;?-Wh7ku
axRH5V0!*?c5Q2@RKpgPKvf5aojsFAO`F3gm
literal 0
HcmV?d00001
diff --git a/tests/data/acpi/q35/DSDT.numamem b/tests/data/acpi/q35/DSDT.numamem
index 44ec1b0af400da6d298284aa959aa38add7e6dd5..d1f45e46a95a98f720a5d8774c3e7a4cedb6adc5 100644
GIT binary patch
delta 314
zcmexwGt-{SCD<ioraS`!W7$NmR3=xMiG?fcqnkVgU4k5)IpRG$U3dfh0t}oD3>l&u
zT!MlOn79(SIHDT`Ky2m!S0^6@AmO3F5#$u=CTPrM(5215z{AKuB&cK%@#Ttl4hrVz
z0vW)?=j-X`Qx9Pa0ofi9rkJl|gb#!%1!Q{kgB7dA`?&fQfZUK!oxqq_oxsJGRGq-{
z<NyEv$sl2mlm$L4lM@*Wl0l{xq=3u=vVlec*+5ek@J&u)DBuA)RRCnt0{+QKTueZ|
Y3<CoPT!00vfCa=~0<v|p6l1C^01Tl^_W%F@
delta 29
lcmbPf|KEnoCD<jTUXFo*alu5cR3>k(iG?dSn=qxy0sxE82>1X1
diff --git a/tests/data/acpi/q35/DSDT.tis b/tests/data/acpi/q35/DSDT.tis
index 30da3ec27958881801dacc954a343321ba26a2ae..eb66aff05a8a88aab07eda12a18e4bb4e3f11e5f 100644
GIT binary patch
delta 314
zcmcca_|cimCD<k8qap(X<GhJnsZ6di6AM??M>ly0x&%2obHsaiy6^`01sFIR7&1gR
zxC8|mFmWYtaYQ!?fY{6du1-D*K*B?TBgiS#P0*Ojpi7&9frpWSNKnZj;>#8992Csa
z1u}q(&)3t>ryjx<0<t|IOfg@_2p<Sj3dr>62P;;K_i^<r0J$NdI)O2<I)RHVsXBq@
z$N&HTlR?5BDGPj9CMPl$B!f&XNCBA#WCM)?vVo>7;G3MpP{0FpssPBO1^knfxR`)^
Y83qOpxBv@S0Skz~1Z3-GDaO6>0PK`a3;+NC
delta 29
lcmez9eBF`DCD<k8x&i|O<B5q}sZ8En6AM>vHeuQ;4*;Bz3Jm}N
diff --git a/tests/data/acpi/q35/FACP.nosmm b/tests/data/acpi/q35/FACP.nosmm
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..891450367cbd1aca397d296831ebeb5cc0788f0f 100644
GIT binary patch
literal 244
zcmZ>BbPo8!z`($K&B@={BUr&HBEZ=VD8>jB1F=Cg3@|cq!k7#UY!D_3lm_uQfNTYr
zI1>Yl08s2d|9^gnN(KfaAY*9@0|O%m1H(@qh$vhVrU*zgDgX&2bAdcCU<4Y7Y#&S<
TnPyagxC11{*nn^khz|n*0A~&X
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 b79ac495c2..dfb8523c8b 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1,14 +1 @@
/* List of comma-separated changed AML files to ignore */
-"tests/data/acpi/q35/DSDT",
-"tests/data/acpi/q35/DSDT.tis",
-"tests/data/acpi/q35/DSDT.bridge",
-"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/DSDT.dimmpxm",
-"tests/data/acpi/q35/DSDT.acpihmat",
-"tests/data/acpi/q35/DSDT.mmio64",
-"tests/data/acpi/q35/DSDT.nosmm",
-"tests/data/acpi/q35/FACP.nosmm",
-"tests/data/acpi/q35/DSDT.nohpet",
--
2.17.1
^ permalink raw reply related [flat|nested] 25+ messages in thread