* [PATCH v3 01/10] checkpatch: don't emit warning on newly created acpi data files
2021-02-11 6:46 [PATCH v3 00/10] ACPI related fixes to comform the spec better Isaku Yamahata
@ 2021-02-11 6:46 ` Isaku Yamahata
2021-02-11 6:46 ` [PATCH v3 02/10] qtest: update tests/qtest/bios-tables-test-allowed-diff.h Isaku Yamahata
` (8 subsequent siblings)
9 siblings, 0 replies; 21+ messages in thread
From: Isaku Yamahata @ 2021-02-11 6:46 UTC (permalink / raw)
To: qemu-devel, imammedo, mst, marcel.apfelbaum
Cc: isaku.yamahata, isaku.yamahata
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] 21+ messages in thread* [PATCH v3 02/10] qtest: update tests/qtest/bios-tables-test-allowed-diff.h
2021-02-11 6:46 [PATCH v3 00/10] ACPI related fixes to comform the spec better Isaku Yamahata
2021-02-11 6:46 ` [PATCH v3 01/10] checkpatch: don't emit warning on newly created acpi data files Isaku Yamahata
@ 2021-02-11 6:46 ` Isaku Yamahata
2021-02-11 6:46 ` [PATCH v3 03/10] i386: add properoty, x-smm-compat-5, to keep compatibility of SMM Isaku Yamahata
` (7 subsequent siblings)
9 siblings, 0 replies; 21+ messages in thread
From: Isaku Yamahata @ 2021-02-11 6:46 UTC (permalink / raw)
To: qemu-devel, imammedo, mst, marcel.apfelbaum
Cc: isaku.yamahata, isaku.yamahata
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
Acked-by: Igor Mammedov <imammedo@redhat.com>
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] 21+ messages in thread* [PATCH v3 03/10] i386: add properoty, x-smm-compat-5, to keep compatibility of SMM
2021-02-11 6:46 [PATCH v3 00/10] ACPI related fixes to comform the spec better Isaku Yamahata
2021-02-11 6:46 ` [PATCH v3 01/10] checkpatch: don't emit warning on newly created acpi data files Isaku Yamahata
2021-02-11 6:46 ` [PATCH v3 02/10] qtest: update tests/qtest/bios-tables-test-allowed-diff.h Isaku Yamahata
@ 2021-02-11 6:46 ` Isaku Yamahata
2021-02-12 14:54 ` Igor Mammedov
2021-02-11 6:46 ` [PATCH v3 04/10] acpi/core: always set SCI_EN when SMM isn't supported Isaku Yamahata
` (6 subsequent siblings)
9 siblings, 1 reply; 21+ messages in thread
From: Isaku Yamahata @ 2021-02-11 6:46 UTC (permalink / raw)
To: qemu-devel, imammedo, mst, marcel.apfelbaum
Cc: isaku.yamahata, isaku.yamahata
The following patch will introduce incompatible behavior of SMM.
Introduce a property to keep the old behavior for compatibility.
To enable smm compat, use "-machine x-smm-compat-5=on"
Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
---
hw/i386/pc_piix.c | 10 ++++++----
hw/i386/pc_q35.c | 1 +
hw/i386/x86.c | 18 ++++++++++++++++++
include/hw/i386/x86.h | 2 ++
4 files changed, 27 insertions(+), 4 deletions(-)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 6188c3e97e..87269e170e 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -441,6 +441,7 @@ DEFINE_I440FX_MACHINE(v6_0, "pc-i440fx-6.0", NULL,
static void pc_i440fx_5_2_machine_options(MachineClass *m)
{
pc_i440fx_6_0_machine_options(m);
+ m->default_machine_opts = "firmware=bios-256k.bin,x-smm-compat-5=on";
m->alias = NULL;
m->is_default = false;
compat_props_add(m->compat_props, hw_compat_5_2, hw_compat_5_2_len);
@@ -664,7 +665,8 @@ static void pc_i440fx_2_2_machine_options(MachineClass *m)
pc_i440fx_2_3_machine_options(m);
m->hw_version = "2.2.0";
- m->default_machine_opts = "firmware=bios-256k.bin,suppress-vmdesc=on";
+ m->default_machine_opts = "firmware=bios-256k.bin,suppress-vmdesc=on"
+ ",x-smm-compat-5=on";
compat_props_add(m->compat_props, hw_compat_2_2, hw_compat_2_2_len);
compat_props_add(m->compat_props, pc_compat_2_2, pc_compat_2_2_len);
pcmc->rsdp_in_ram = false;
@@ -727,7 +729,7 @@ static void pc_i440fx_1_7_machine_options(MachineClass *m)
pc_i440fx_2_0_machine_options(m);
m->hw_version = "1.7.0";
- m->default_machine_opts = NULL;
+ m->default_machine_opts = "x-smm-compat-5=on";
m->option_rom_has_mr = true;
compat_props_add(m->compat_props, pc_compat_1_7, pc_compat_1_7_len);
pcmc->smbios_defaults = false;
@@ -999,7 +1001,7 @@ static void xenfv_4_2_machine_options(MachineClass *m)
pc_i440fx_4_2_machine_options(m);
m->desc = "Xen Fully-virtualized PC";
m->max_cpus = HVM_MAX_VCPUS;
- m->default_machine_opts = "accel=xen,suppress-vmdesc=on";
+ m->default_machine_opts = "accel=xen,suppress-vmdesc=on,x-smm-compat-5=on";
}
DEFINE_PC_MACHINE(xenfv_4_2, "xenfv-4.2", pc_xen_hvm_init,
@@ -1011,7 +1013,7 @@ static void xenfv_3_1_machine_options(MachineClass *m)
m->desc = "Xen Fully-virtualized PC";
m->alias = "xenfv";
m->max_cpus = HVM_MAX_VCPUS;
- m->default_machine_opts = "accel=xen,suppress-vmdesc=on";
+ m->default_machine_opts = "accel=xen,suppress-vmdesc=on,x-smm-compat-5=on";
}
DEFINE_PC_MACHINE(xenfv, "xenfv-3.1", pc_xen_hvm_init,
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 0a212443aa..14974b7255 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -358,6 +358,7 @@ DEFINE_Q35_MACHINE(v6_0, "pc-q35-6.0", NULL,
static void pc_q35_5_2_machine_options(MachineClass *m)
{
pc_q35_6_0_machine_options(m);
+ m->default_machine_opts = "firmware=bios-256k.bin,x-smm-compat-5=on";
m->alias = NULL;
compat_props_add(m->compat_props, hw_compat_5_2, hw_compat_5_2_len);
compat_props_add(m->compat_props, pc_compat_5_2, pc_compat_5_2_len);
diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index 6329f90ef9..00eb2253d3 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -1174,6 +1174,18 @@ static void x86_machine_set_smm(Object *obj, Visitor *v, const char *name,
visit_type_OnOffAuto(v, name, &x86ms->smm, errp);
}
+static bool x86_machine_get_smm_compat_5(Object *obj, Error **errp)
+{
+ X86MachineState *x86ms = X86_MACHINE(obj);
+ return x86ms->smm_compat_5;
+}
+
+static void x86_machine_set_smm_compat_5(Object *obj, bool value, Error **errp)
+{
+ X86MachineState *x86ms = X86_MACHINE(obj);
+ x86ms->smm_compat_5 = value;
+}
+
bool x86_machine_is_acpi_enabled(const X86MachineState *x86ms)
{
if (x86ms->acpi == ON_OFF_AUTO_OFF) {
@@ -1204,6 +1216,7 @@ static void x86_machine_initfn(Object *obj)
X86MachineState *x86ms = X86_MACHINE(obj);
x86ms->smm = ON_OFF_AUTO_AUTO;
+ x86ms->smm_compat_5 = false;
x86ms->acpi = ON_OFF_AUTO_AUTO;
x86ms->smp_dies = 1;
x86ms->pci_irq_mask = ACPI_BUILD_PCI_IRQS;
@@ -1228,6 +1241,11 @@ static void x86_machine_class_init(ObjectClass *oc, void *data)
object_class_property_set_description(oc, X86_MACHINE_SMM,
"Enable SMM");
+ object_class_property_add_bool(oc, X86_MACHINE_SMM_COMPAT_5,
+ x86_machine_get_smm_compat_5, x86_machine_set_smm_compat_5);
+ object_class_property_set_description(oc, X86_MACHINE_SMM_COMPAT_5,
+ "Enable SMM compatible behavior");
+
object_class_property_add(oc, X86_MACHINE_ACPI, "OnOffAuto",
x86_machine_get_acpi, x86_machine_set_acpi,
NULL, NULL);
diff --git a/include/hw/i386/x86.h b/include/hw/i386/x86.h
index 56080bd1fb..3dbe19a335 100644
--- a/include/hw/i386/x86.h
+++ b/include/hw/i386/x86.h
@@ -65,6 +65,7 @@ struct X86MachineState {
unsigned smp_dies;
OnOffAuto smm;
+ bool smm_compat_5;
OnOffAuto acpi;
/*
@@ -75,6 +76,7 @@ struct X86MachineState {
};
#define X86_MACHINE_SMM "smm"
+#define X86_MACHINE_SMM_COMPAT_5 "x-smm-compat-5"
#define X86_MACHINE_ACPI "acpi"
#define TYPE_X86_MACHINE MACHINE_TYPE_NAME("x86")
--
2.17.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v3 03/10] i386: add properoty, x-smm-compat-5, to keep compatibility of SMM
2021-02-11 6:46 ` [PATCH v3 03/10] i386: add properoty, x-smm-compat-5, to keep compatibility of SMM Isaku Yamahata
@ 2021-02-12 14:54 ` Igor Mammedov
0 siblings, 0 replies; 21+ messages in thread
From: Igor Mammedov @ 2021-02-12 14:54 UTC (permalink / raw)
To: Isaku Yamahata; +Cc: isaku.yamahata, qemu-devel, mst
On Wed, 10 Feb 2021 22:46:39 -0800
Isaku Yamahata <isaku.yamahata@intel.com> wrote:
> The following patch will introduce incompatible behavior of SMM.
> Introduce a property to keep the old behavior for compatibility.
> To enable smm compat, use "-machine x-smm-compat-5=on"
just curious, why it has "-5" suffix
also I'd drop x- prefix.
> Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
> ---
> hw/i386/pc_piix.c | 10 ++++++----
> hw/i386/pc_q35.c | 1 +
> hw/i386/x86.c | 18 ++++++++++++++++++
> include/hw/i386/x86.h | 2 ++
> 4 files changed, 27 insertions(+), 4 deletions(-)
>
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 6188c3e97e..87269e170e 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -441,6 +441,7 @@ DEFINE_I440FX_MACHINE(v6_0, "pc-i440fx-6.0", NULL,
> static void pc_i440fx_5_2_machine_options(MachineClass *m)
> {
> pc_i440fx_6_0_machine_options(m);
> + m->default_machine_opts = "firmware=bios-256k.bin,x-smm-compat-5=on";
did I point to a wrong example :/
smm machinery is part of ich9/piix4 devices, so it would be simpler
by adding 'smm-compat' property to them
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 970046f438..bcb0333ddf 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -36,7 +36,9 @@
#include "hw/virtio/virtio.h"
#include "hw/virtio/virtio-pci.h"
-GlobalProperty hw_compat_5_2[] = {};
+GlobalProperty hw_compat_5_2[] = {
it turns on compat mode on 5.2 and older machines types
+ { "ICH9-LPC", "smm-compat", "on"},
+};
const size_t hw_compat_5_2_len = G_N_ELEMENTS(hw_compat_5_2);
GlobalProperty hw_compat_5_1[] = {
diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
index d3145bf014..13003b2ac2 100644
--- a/hw/isa/lpc_ich9.c
+++ b/hw/isa/lpc_ich9.c
@@ -774,6 +774,7 @@ static const VMStateDescription vmstate_ich9_lpc = {
};
static Property ich9_lpc_properties[] = {
default would be used on new and later machine types
+ DEFINE_PROP_BOOL("smm-compat", ICH9LPCState, pm.smm_compat , false),
DEFINE_PROP_BOOL("noreboot", ICH9LPCState, pin_strap.spkr_hi, true),
DEFINE_PROP_BIT64("x-smi-broadcast", ICH9LPCState, smi_host_features,
ICH9_LPC_SMI_F_BROADCAST_BIT, true),
then do the same for piix4 pm device
> m->alias = NULL;
> m->is_default = false;
> compat_props_add(m->compat_props, hw_compat_5_2, hw_compat_5_2_len);
> @@ -664,7 +665,8 @@ static void pc_i440fx_2_2_machine_options(MachineClass *m)
>
> pc_i440fx_2_3_machine_options(m);
> m->hw_version = "2.2.0";
> - m->default_machine_opts = "firmware=bios-256k.bin,suppress-vmdesc=on";
> + m->default_machine_opts = "firmware=bios-256k.bin,suppress-vmdesc=on"
> + ",x-smm-compat-5=on";
> compat_props_add(m->compat_props, hw_compat_2_2, hw_compat_2_2_len);
> compat_props_add(m->compat_props, pc_compat_2_2, pc_compat_2_2_len);
> pcmc->rsdp_in_ram = false;
> @@ -727,7 +729,7 @@ static void pc_i440fx_1_7_machine_options(MachineClass *m)
>
> pc_i440fx_2_0_machine_options(m);
> m->hw_version = "1.7.0";
> - m->default_machine_opts = NULL;
> + m->default_machine_opts = "x-smm-compat-5=on";
> m->option_rom_has_mr = true;
> compat_props_add(m->compat_props, pc_compat_1_7, pc_compat_1_7_len);
> pcmc->smbios_defaults = false;
> @@ -999,7 +1001,7 @@ static void xenfv_4_2_machine_options(MachineClass *m)
> pc_i440fx_4_2_machine_options(m);
> m->desc = "Xen Fully-virtualized PC";
> m->max_cpus = HVM_MAX_VCPUS;
> - m->default_machine_opts = "accel=xen,suppress-vmdesc=on";
> + m->default_machine_opts = "accel=xen,suppress-vmdesc=on,x-smm-compat-5=on";
> }
>
> DEFINE_PC_MACHINE(xenfv_4_2, "xenfv-4.2", pc_xen_hvm_init,
> @@ -1011,7 +1013,7 @@ static void xenfv_3_1_machine_options(MachineClass *m)
> m->desc = "Xen Fully-virtualized PC";
> m->alias = "xenfv";
> m->max_cpus = HVM_MAX_VCPUS;
> - m->default_machine_opts = "accel=xen,suppress-vmdesc=on";
> + m->default_machine_opts = "accel=xen,suppress-vmdesc=on,x-smm-compat-5=on";
> }
>
> DEFINE_PC_MACHINE(xenfv, "xenfv-3.1", pc_xen_hvm_init,
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index 0a212443aa..14974b7255 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -358,6 +358,7 @@ DEFINE_Q35_MACHINE(v6_0, "pc-q35-6.0", NULL,
> static void pc_q35_5_2_machine_options(MachineClass *m)
> {
> pc_q35_6_0_machine_options(m);
> + m->default_machine_opts = "firmware=bios-256k.bin,x-smm-compat-5=on";
> m->alias = NULL;
> compat_props_add(m->compat_props, hw_compat_5_2, hw_compat_5_2_len);
> compat_props_add(m->compat_props, pc_compat_5_2, pc_compat_5_2_len);
> diff --git a/hw/i386/x86.c b/hw/i386/x86.c
> index 6329f90ef9..00eb2253d3 100644
> --- a/hw/i386/x86.c
> +++ b/hw/i386/x86.c
> @@ -1174,6 +1174,18 @@ static void x86_machine_set_smm(Object *obj, Visitor *v, const char *name,
> visit_type_OnOffAuto(v, name, &x86ms->smm, errp);
> }
>
> +static bool x86_machine_get_smm_compat_5(Object *obj, Error **errp)
> +{
> + X86MachineState *x86ms = X86_MACHINE(obj);
> + return x86ms->smm_compat_5;
> +}
> +
> +static void x86_machine_set_smm_compat_5(Object *obj, bool value, Error **errp)
> +{
> + X86MachineState *x86ms = X86_MACHINE(obj);
> + x86ms->smm_compat_5 = value;
> +}
> +
> bool x86_machine_is_acpi_enabled(const X86MachineState *x86ms)
> {
> if (x86ms->acpi == ON_OFF_AUTO_OFF) {
> @@ -1204,6 +1216,7 @@ static void x86_machine_initfn(Object *obj)
> X86MachineState *x86ms = X86_MACHINE(obj);
>
> x86ms->smm = ON_OFF_AUTO_AUTO;
> + x86ms->smm_compat_5 = false;
> x86ms->acpi = ON_OFF_AUTO_AUTO;
> x86ms->smp_dies = 1;
> x86ms->pci_irq_mask = ACPI_BUILD_PCI_IRQS;
> @@ -1228,6 +1241,11 @@ static void x86_machine_class_init(ObjectClass *oc, void *data)
> object_class_property_set_description(oc, X86_MACHINE_SMM,
> "Enable SMM");
>
> + object_class_property_add_bool(oc, X86_MACHINE_SMM_COMPAT_5,
> + x86_machine_get_smm_compat_5, x86_machine_set_smm_compat_5);
> + object_class_property_set_description(oc, X86_MACHINE_SMM_COMPAT_5,
> + "Enable SMM compatible behavior");
> +
> object_class_property_add(oc, X86_MACHINE_ACPI, "OnOffAuto",
> x86_machine_get_acpi, x86_machine_set_acpi,
> NULL, NULL);
> diff --git a/include/hw/i386/x86.h b/include/hw/i386/x86.h
> index 56080bd1fb..3dbe19a335 100644
> --- a/include/hw/i386/x86.h
> +++ b/include/hw/i386/x86.h
> @@ -65,6 +65,7 @@ struct X86MachineState {
> unsigned smp_dies;
>
> OnOffAuto smm;
> + bool smm_compat_5;
> OnOffAuto acpi;
>
> /*
> @@ -75,6 +76,7 @@ struct X86MachineState {
> };
>
> #define X86_MACHINE_SMM "smm"
> +#define X86_MACHINE_SMM_COMPAT_5 "x-smm-compat-5"
> #define X86_MACHINE_ACPI "acpi"
>
> #define TYPE_X86_MACHINE MACHINE_TYPE_NAME("x86")
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v3 04/10] acpi/core: always set SCI_EN when SMM isn't supported
2021-02-11 6:46 [PATCH v3 00/10] ACPI related fixes to comform the spec better Isaku Yamahata
` (2 preceding siblings ...)
2021-02-11 6:46 ` [PATCH v3 03/10] i386: add properoty, x-smm-compat-5, to keep compatibility of SMM Isaku Yamahata
@ 2021-02-11 6:46 ` Isaku Yamahata
2021-02-12 15:09 ` Igor Mammedov
2021-02-11 6:46 ` [PATCH v3 05/10] acpi: set fadt.smi_cmd to zero when SMM is not supported Isaku Yamahata
` (5 subsequent siblings)
9 siblings, 1 reply; 21+ messages in thread
From: Isaku Yamahata @ 2021-02-11 6:46 UTC (permalink / raw)
To: qemu-devel, imammedo, mst, marcel.apfelbaum
Cc: isaku.yamahata, isaku.yamahata
If SMM is not supported, ACPI fixed hardware doesn't support
legacy-mode. ACPI-only platform. Where SCI_EN in PM1_CNT register is
always set.
The bit tells OS legacy mode(SCI_EN cleared) or ACPI mode(SCI_EN set).
With the next patch (setting fadt.smi_cmd = 0 when smm isn't enabled),
guest Linux tries to switch to ACPI mode, finds smi_cmd = 0, and then
fails to initialize acpi subsystem. This patch proactively fixes it.
This patch changes guest ABI. To keep compatibility, use
"x-smm-compat-5" introduced by earlier patch.
If the property is true, disable new behavior.
ACPI spec 4.8.10.1 PM1 Event Grouping
PM1 Eanble Registers
> For ACPI-only platforms (where SCI_EN is always set)
Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
---
hw/acpi/core.c | 20 +++++++++++++++++++-
hw/acpi/ich9.c | 2 +-
hw/acpi/piix4.c | 3 ++-
hw/isa/vt82c686.c | 2 +-
include/hw/acpi/acpi.h | 4 +++-
5 files changed, 26 insertions(+), 5 deletions(-)
diff --git a/hw/acpi/core.c b/hw/acpi/core.c
index 7170bff657..03a6e949e8 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,22 @@ 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;
+ /*
+ * Until v5, pm1 cnt allows to change mode (legacy vs acpi)
+ * even SMM isn't enabled.
+ * Keep the old behavior for compatibility.
+ */
+ if (object_property_get_bool(qdev_get_machine(), "x-smm-compat-5", NULL)) {
+ acpi_only = false;
+ }
+
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 +653,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] 21+ messages in thread* Re: [PATCH v3 04/10] acpi/core: always set SCI_EN when SMM isn't supported
2021-02-11 6:46 ` [PATCH v3 04/10] acpi/core: always set SCI_EN when SMM isn't supported Isaku Yamahata
@ 2021-02-12 15:09 ` Igor Mammedov
0 siblings, 0 replies; 21+ messages in thread
From: Igor Mammedov @ 2021-02-12 15:09 UTC (permalink / raw)
To: Isaku Yamahata; +Cc: isaku.yamahata, qemu-devel, mst
On Wed, 10 Feb 2021 22:46:40 -0800
Isaku Yamahata <isaku.yamahata@intel.com> wrote:
> If SMM is not supported, ACPI fixed hardware doesn't support
> legacy-mode. ACPI-only platform. Where SCI_EN in PM1_CNT register is
> always set.
> The bit tells OS legacy mode(SCI_EN cleared) or ACPI mode(SCI_EN set).
>
> With the next patch (setting fadt.smi_cmd = 0 when smm isn't enabled),
> guest Linux tries to switch to ACPI mode, finds smi_cmd = 0, and then
> fails to initialize acpi subsystem. This patch proactively fixes it.
>
> This patch changes guest ABI. To keep compatibility, use
> "x-smm-compat-5" introduced by earlier patch.
> If the property is true, disable new behavior.
>
> ACPI spec 4.8.10.1 PM1 Event Grouping
> PM1 Eanble Registers
> > For ACPI-only platforms (where SCI_EN is always set)
>
> Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
> ---
> hw/acpi/core.c | 20 +++++++++++++++++++-
> hw/acpi/ich9.c | 2 +-
> hw/acpi/piix4.c | 3 ++-
> hw/isa/vt82c686.c | 2 +-
> include/hw/acpi/acpi.h | 4 +++-
> 5 files changed, 26 insertions(+), 5 deletions(-)
>
> diff --git a/hw/acpi/core.c b/hw/acpi/core.c
> index 7170bff657..03a6e949e8 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,22 @@ 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;
>
> + /*
> + * Until v5, pm1 cnt allows to change mode (legacy vs acpi)
> + * even SMM isn't enabled.
> + * Keep the old behavior for compatibility.
> + */
> + if (object_property_get_bool(qdev_get_machine(), "x-smm-compat-5", NULL)) {
> + acpi_only = false;
> + }
> +
> 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 +653,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);
with device compat prop as suggested on 3/10 it will look like
s/!smm_enabled/!pm->smm_compat && !smm_enabled/
>
> acpi_gpe_init(&pm->acpi_regs, ICH9_PMIO_GPE0_LEN);
> memory_region_init_io(&pm->io_gpe, OBJECT(lpc_pci), &ich9_gpe_ops, pm,
> diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
> index 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] 21+ messages in thread
* [PATCH v3 05/10] acpi: set fadt.smi_cmd to zero when SMM is not supported
2021-02-11 6:46 [PATCH v3 00/10] ACPI related fixes to comform the spec better Isaku Yamahata
` (3 preceding siblings ...)
2021-02-11 6:46 ` [PATCH v3 04/10] acpi/core: always set SCI_EN when SMM isn't supported Isaku Yamahata
@ 2021-02-11 6:46 ` Isaku Yamahata
2021-02-12 15:15 ` Igor Mammedov
2021-02-11 6:46 ` [PATCH v3 06/10] acpi: add test case for smm unsupported -machine smm=off Isaku Yamahata
` (4 subsequent siblings)
9 siblings, 1 reply; 21+ messages in thread
From: Isaku Yamahata @ 2021-02-11 6:46 UTC (permalink / raw)
To: qemu-devel, imammedo, mst, marcel.apfelbaum
Cc: isaku.yamahata, isaku.yamahata
From table 5.9 SMI_CMD of ACPI spec
> This field is reserved and must be zero on system
> that does not support System Management mode.
When smm is not enabled, set it to zero to comform to the spec.
When -machine smm=off is passed, the change to FACP is as follows.
@@ -1,46 +1,46 @@
/*
* Intel ACPI Component Architecture
* AML/ASL+ Disassembler version 20180105 (64-bit version)
* Copyright (c) 2000 - 2018 Intel Corporation
*
- * Disassembly of tests/data/acpi/q35/FACP, Fri Feb 5 16:57:04 2021
+ * Disassembly of /tmp/aml-1OQYX0, Fri Feb 5 16:57:04 2021
*
* ACPI Data Table [FACP]
*
* Format: [HexOffset DecimalOffset ByteLength] FieldName : FieldValue
*/
[000h 0000 4] Signature : "FACP" [Fixed ACPI Description Table (FADT)]
[004h 0004 4] Table Length : 000000F4
[008h 0008 1] Revision : 03
-[009h 0009 1] Checksum : 1F
+[009h 0009 1] Checksum : D6
[00Ah 0010 6] Oem ID : "BOCHS "
[010h 0016 8] Oem Table ID : "BXPCFACP"
[018h 0024 4] Oem Revision : 00000001
[01Ch 0028 4] Asl Compiler ID : "BXPC"
[020h 0032 4] Asl Compiler Revision : 00000001
[024h 0036 4] FACS Address : 00000000
[028h 0040 4] DSDT Address : 00000000
[02Ch 0044 1] Model : 01
[02Dh 0045 1] PM Profile : 00 [Unspecified]
[02Eh 0046 2] SCI Interrupt : 0009
-[030h 0048 4] SMI Command Port : 000000B2
-[034h 0052 1] ACPI Enable Value : 02
-[035h 0053 1] ACPI Disable Value : 03
+[030h 0048 4] SMI Command Port : 00000000
+[034h 0052 1] ACPI Enable Value : 00
+[035h 0053 1] ACPI Disable Value : 00
[036h 0054 1] S4BIOS Command : 00
[037h 0055 1] P-State Control : 00
[038h 0056 4] PM1A Event Block Address : 00000600
[03Ch 0060 4] PM1B Event Block Address : 00000000
[040h 0064 4] PM1A Control Block Address : 00000604
[044h 0068 4] PM1B Control Block Address : 00000000
[048h 0072 4] PM2 Control Block Address : 00000000
[04Ch 0076 4] PM Timer Block Address : 00000608
[050h 0080 4] GPE0 Block Address : 00000620
[054h 0084 4] GPE1 Block Address : 00000000
[058h 0088 1] PM1 Event Block Length : 04
[059h 0089 1] PM1 Control Block Length : 02
[05Ah 0090 1] PM2 Control Block Length : 00
[05Bh 0091 1] PM Timer Block Length : 04
[05Ch 0092 1] GPE0 Block Length : 10
[05Dh 0093 1] GPE1 Block Length : 00
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
---
hw/i386/acpi-build.c | 18 +++++++++++++++---
1 file changed, 15 insertions(+), 3 deletions(-)
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index f56d699c7f..00cc119362 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -139,6 +139,14 @@ const struct AcpiGenericAddress x86_nvdimm_acpi_dsmio = {
static void init_common_fadt_data(MachineState *ms, Object *o,
AcpiFadtData *data)
{
+ X86MachineState *x86ms = X86_MACHINE(ms);
+ /*
+ * Until v5, smi_cmd/acpi_enable_cmd/acpi_disable_cmd were always set
+ * irrelevant to smm_enabled, which doesn't comforms to ACPI spec.
+ * Keep guest ABI compatibility when smm_compat_5 is true.
+ */
+ bool smm_enabled = x86ms->smm_compat_5 ?
+ true : x86_machine_is_smm_enabled(x86ms);
uint32_t io = object_property_get_uint(o, ACPI_PM_PROP_PM_IO_BASE, NULL);
AmlAddressSpace as = AML_AS_SYSTEM_IO;
AcpiFadtData fadt = {
@@ -159,12 +167,16 @@ static void init_common_fadt_data(MachineState *ms, Object *o,
.rtc_century = RTC_CENTURY,
.plvl2_lat = 0xfff /* C2 state not supported */,
.plvl3_lat = 0xfff /* C3 state not supported */,
- .smi_cmd = ACPI_PORT_SMI_CMD,
+ .smi_cmd = smm_enabled ? ACPI_PORT_SMI_CMD : 0,
.sci_int = object_property_get_uint(o, ACPI_PM_PROP_SCI_INT, NULL),
.acpi_enable_cmd =
- object_property_get_uint(o, ACPI_PM_PROP_ACPI_ENABLE_CMD, NULL),
+ smm_enabled ?
+ object_property_get_uint(o, ACPI_PM_PROP_ACPI_ENABLE_CMD, NULL) :
+ 0,
.acpi_disable_cmd =
- object_property_get_uint(o, ACPI_PM_PROP_ACPI_DISABLE_CMD, NULL),
+ smm_enabled ?
+ object_property_get_uint(o, ACPI_PM_PROP_ACPI_DISABLE_CMD, NULL) :
+ 0,
.pm1a_evt = { .space_id = as, .bit_width = 4 * 8, .address = io },
.pm1a_cnt = { .space_id = as, .bit_width = 2 * 8,
.address = io + 0x04 },
--
2.17.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v3 05/10] acpi: set fadt.smi_cmd to zero when SMM is not supported
2021-02-11 6:46 ` [PATCH v3 05/10] acpi: set fadt.smi_cmd to zero when SMM is not supported Isaku Yamahata
@ 2021-02-12 15:15 ` Igor Mammedov
0 siblings, 0 replies; 21+ messages in thread
From: Igor Mammedov @ 2021-02-12 15:15 UTC (permalink / raw)
To: Isaku Yamahata; +Cc: isaku.yamahata, qemu-devel, mst
On Wed, 10 Feb 2021 22:46:41 -0800
Isaku Yamahata <isaku.yamahata@intel.com> wrote:
> From table 5.9 SMI_CMD of ACPI spec
> > This field is reserved and must be zero on system
> > that does not support System Management mode.
>
> When smm is not enabled, set it to zero to comform to the spec.
> When -machine smm=off is passed, the change to FACP is as follows.
>
> @@ -1,46 +1,46 @@
> /*
> * Intel ACPI Component Architecture
> * AML/ASL+ Disassembler version 20180105 (64-bit version)
> * Copyright (c) 2000 - 2018 Intel Corporation
> *
> - * Disassembly of tests/data/acpi/q35/FACP, Fri Feb 5 16:57:04 2021
> + * Disassembly of /tmp/aml-1OQYX0, Fri Feb 5 16:57:04 2021
> *
> * ACPI Data Table [FACP]
> *
> * Format: [HexOffset DecimalOffset ByteLength] FieldName : FieldValue
> */
>
> [000h 0000 4] Signature : "FACP" [Fixed ACPI Description Table (FADT)]
> [004h 0004 4] Table Length : 000000F4
> [008h 0008 1] Revision : 03
> -[009h 0009 1] Checksum : 1F
> +[009h 0009 1] Checksum : D6
> [00Ah 0010 6] Oem ID : "BOCHS "
> [010h 0016 8] Oem Table ID : "BXPCFACP"
> [018h 0024 4] Oem Revision : 00000001
> [01Ch 0028 4] Asl Compiler ID : "BXPC"
> [020h 0032 4] Asl Compiler Revision : 00000001
>
> [024h 0036 4] FACS Address : 00000000
> [028h 0040 4] DSDT Address : 00000000
> [02Ch 0044 1] Model : 01
> [02Dh 0045 1] PM Profile : 00 [Unspecified]
> [02Eh 0046 2] SCI Interrupt : 0009
> -[030h 0048 4] SMI Command Port : 000000B2
> -[034h 0052 1] ACPI Enable Value : 02
> -[035h 0053 1] ACPI Disable Value : 03
> +[030h 0048 4] SMI Command Port : 00000000
> +[034h 0052 1] ACPI Enable Value : 00
> +[035h 0053 1] ACPI Disable Value : 00
> [036h 0054 1] S4BIOS Command : 00
> [037h 0055 1] P-State Control : 00
> [038h 0056 4] PM1A Event Block Address : 00000600
> [03Ch 0060 4] PM1B Event Block Address : 00000000
> [040h 0064 4] PM1A Control Block Address : 00000604
> [044h 0068 4] PM1B Control Block Address : 00000000
> [048h 0072 4] PM2 Control Block Address : 00000000
> [04Ch 0076 4] PM Timer Block Address : 00000608
> [050h 0080 4] GPE0 Block Address : 00000620
> [054h 0084 4] GPE1 Block Address : 00000000
> [058h 0088 1] PM1 Event Block Length : 04
> [059h 0089 1] PM1 Control Block Length : 02
> [05Ah 0090 1] PM2 Control Block Length : 00
> [05Bh 0091 1] PM Timer Block Length : 04
> [05Ch 0092 1] GPE0 Block Length : 10
> [05Dh 0093 1] GPE1 Block Length : 00
>
> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
> ---
> hw/i386/acpi-build.c | 18 +++++++++++++++---
> 1 file changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index f56d699c7f..00cc119362 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -139,6 +139,14 @@ const struct AcpiGenericAddress x86_nvdimm_acpi_dsmio = {
> static void init_common_fadt_data(MachineState *ms, Object *o,
> AcpiFadtData *data)
> {
> + X86MachineState *x86ms = X86_MACHINE(ms);
> + /*
> + * Until v5, smi_cmd/acpi_enable_cmd/acpi_disable_cmd were always set
> + * irrelevant to smm_enabled, which doesn't comforms to ACPI spec.
> + * Keep guest ABI compatibility when smm_compat_5 is true.
> + */
> + bool smm_enabled = x86ms->smm_compat_5 ?
> + true : x86_machine_is_smm_enabled(x86ms);
> uint32_t io = object_property_get_uint(o, ACPI_PM_PROP_PM_IO_BASE, NULL);
> AmlAddressSpace as = AML_AS_SYSTEM_IO;
> AcpiFadtData fadt = {
> @@ -159,12 +167,16 @@ static void init_common_fadt_data(MachineState *ms, Object *o,
> .rtc_century = RTC_CENTURY,
> .plvl2_lat = 0xfff /* C2 state not supported */,
> .plvl3_lat = 0xfff /* C3 state not supported */,
> - .smi_cmd = ACPI_PORT_SMI_CMD,
> + .smi_cmd = smm_enabled ? ACPI_PORT_SMI_CMD : 0,
I'd fetch smm-compat property from 'o' object (assuming per device compat property)
> .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,
for both ACPI_PM_PROP_ACPI_[EN|DIS]ABLE_CMD can return 0 when ich9/piix4 in acpi_only mode
> .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] 21+ messages in thread
* [PATCH v3 06/10] acpi: add test case for smm unsupported -machine smm=off
2021-02-11 6:46 [PATCH v3 00/10] ACPI related fixes to comform the spec better Isaku Yamahata
` (4 preceding siblings ...)
2021-02-11 6:46 ` [PATCH v3 05/10] acpi: set fadt.smi_cmd to zero when SMM is not supported Isaku Yamahata
@ 2021-02-11 6:46 ` Isaku Yamahata
2021-02-11 6:46 ` [PATCH v3 07/10] hw/i386: declare ACPI mother board resource for MMCONFIG region Isaku Yamahata
` (3 subsequent siblings)
9 siblings, 0 replies; 21+ messages in thread
From: Isaku Yamahata @ 2021-02-11 6:46 UTC (permalink / raw)
To: qemu-devel, imammedo, mst, marcel.apfelbaum
Cc: isaku.yamahata, isaku.yamahata
Reviewed-by: Igor Mammedov <imammedo@redhat.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] 21+ messages in thread* [PATCH v3 07/10] hw/i386: declare ACPI mother board resource for MMCONFIG region
2021-02-11 6:46 [PATCH v3 00/10] ACPI related fixes to comform the spec better Isaku Yamahata
` (5 preceding siblings ...)
2021-02-11 6:46 ` [PATCH v3 06/10] acpi: add test case for smm unsupported -machine smm=off Isaku Yamahata
@ 2021-02-11 6:46 ` Isaku Yamahata
2021-02-12 15:40 ` Igor Mammedov
2021-02-11 6:46 ` [PATCH v3 08/10] i386: acpi: Don't build HPET ACPI entry if HPET is disabled Isaku Yamahata
` (2 subsequent siblings)
9 siblings, 1 reply; 21+ messages in thread
From: Isaku Yamahata @ 2021-02-11 6:46 UTC (permalink / raw)
To: qemu-devel, imammedo, mst, marcel.apfelbaum
Cc: isaku.yamahata, isaku.yamahata
Declare PNP0C01 device to reserve MMCONFIG region to conform to the
spec better and play nice with guest BIOSes/OSes.
According to PCI Firmware Specification[0], MMCONFIG region must be
reserved by declaring a motherboard resource. It's optional to reserve
the region in memory map by Int 15 E820h or EFIGetMemoryMap.
Guest Linux checks if the MMCFG region is reserved by bios memory map
or ACPI resource. If it's not reserved, Linux falls back to legacy PCI
configuration access.
TDVF [1] [2] doesn't reserve MMCONFIG the region in memory map.
On the other hand OVMF reserves it in memory map without declaring a
motherboard resource. With memory map reservation, linux guest uses
MMCONFIG region. However it doesn't comply to PCI Firmware
specification.
[0] PCI Firmware specification Revision 3.2
4.1.2 MCFG Table Description table 4-2 NOTE 2
If the operating system does not natively comprehend reserving the
MMCFG region, The MMCFG region must e reserved by firmware. ...
For most systems, the mortheroard resource would appear at the root
of the ACPI namespace (under \_SB)...
The resource can optionally be returned in Int15 E820h or
EFIGetMemoryMap as reserved memory but must always be reported
through ACPI as a motherboard resource
[1] TDX: Intel Trust Domain Extension
https://software.intel.com/content/www/us/en/develop/articles/intel-trust-domain-extensions.html
[2] TDX Virtual Firmware
https://github.com/tianocore/edk2-staging/tree/TDVF
The change to DSDT is as follows.
@@ -68,32 +68,51 @@
If ((CDW3 != Local0))
{
CDW1 |= 0x10
}
CDW3 = Local0
}
Else
{
CDW1 |= 0x04
}
Return (Arg3)
}
}
+
+ Device (DRAC)
+ {
+ Name (_HID, "PNP0C01" /* System Board */) // _HID: Hardware ID
+ Name (RBUF, ResourceTemplate ()
+ {
+ QWordMemory (ResourceProducer, PosDecode, MinFixed, MaxFixed, NonCacheable, ReadWrite,
+ 0x0000000000000000, // Granularity
+ 0x00000000B0000000, // Range Minimum
+ 0x00000000BFFFFFFF, // Range Maximum
+ 0x0000000000000000, // Translation Offset
+ 0x0000000010000000, // Length
+ ,, , AddressRangeMemory, TypeStatic)
+ })
+ Method (_CRS, 0, Serialized) // _CRS: Current Resource Settings
+ {
+ 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>
---
hw/i386/acpi-build.c | 46 +++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 45 insertions(+), 1 deletion(-)
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 00cc119362..e369908b1a 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1072,6 +1072,46 @@ static void build_q35_pci0_int(Aml *table)
aml_append(table, sb_scope);
}
+static Aml *build_q35_dram_controller(void)
+{
+ AcpiMcfgInfo mcfg;
+ Aml *dev;
+ Aml *rbuf;
+ Aml *resource_template;
+ Aml *rbuf_name;
+ Aml *crs;
+
+ if (!acpi_get_mcfg(&mcfg)) {
+ return NULL;
+ }
+
+ /* DRAM controller */
+ dev = aml_device("DRAC");
+ aml_append(dev, aml_name_decl("_HID", aml_string("PNP0C01")));
+
+ 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,
+ mcfg.base,
+ mcfg.base + mcfg.size - 1,
+ 0x0000000000000000,
+ mcfg.size));
+ rbuf = aml_name_decl("RBUF", resource_template);
+ aml_append(dev, rbuf);
+
+ crs = aml_method("_CRS", 0, AML_SERIALIZED);
+ rbuf_name = aml_name("RBUF");
+ aml_append(crs, aml_return(rbuf_name));
+ aml_append(dev, crs);
+
+ return dev;
+}
+
static void build_q35_isa_bridge(Aml *table)
{
Aml *dev;
@@ -1212,7 +1252,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
Range *pci_hole, Range *pci_hole64, MachineState *machine)
{
CrsRangeEntry *entry;
- Aml *dsdt, *sb_scope, *scope, *dev, *method, *field, *pkg, *crs;
+ Aml *dsdt, *sb_scope, *scope, *dev, *method, *field, *pkg, *crs, *drac;
CrsRangeSet crs_range_set;
PCMachineState *pcms = PC_MACHINE(machine);
PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(machine);
@@ -1256,6 +1296,10 @@ 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);
+ drac = build_q35_dram_controller();
+ if (drac) {
+ aml_append(sb_scope, drac);
+ }
if (pm->smi_on_cpuhp) {
/* reserve SMI block resources, IO ports 0xB2, 0xB3 */
--
2.17.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v3 07/10] hw/i386: declare ACPI mother board resource for MMCONFIG region
2021-02-11 6:46 ` [PATCH v3 07/10] hw/i386: declare ACPI mother board resource for MMCONFIG region Isaku Yamahata
@ 2021-02-12 15:40 ` Igor Mammedov
2021-02-12 20:51 ` Isaku Yamahata
0 siblings, 1 reply; 21+ messages in thread
From: Igor Mammedov @ 2021-02-12 15:40 UTC (permalink / raw)
To: Isaku Yamahata; +Cc: isaku.yamahata, qemu-devel, mst
On Wed, 10 Feb 2021 22:46:43 -0800
Isaku Yamahata <isaku.yamahata@intel.com> wrote:
> Declare PNP0C01 device to reserve MMCONFIG region to conform to the
> spec better and play nice with guest BIOSes/OSes.
>
> According to PCI Firmware Specification[0], MMCONFIG region must be
> reserved by declaring a motherboard resource. It's optional to reserve
> the region in memory map by Int 15 E820h or EFIGetMemoryMap.
> Guest Linux checks if the MMCFG region is reserved by bios memory map
> or ACPI resource. If it's not reserved, Linux falls back to legacy PCI
> configuration access.
>
> TDVF [1] [2] doesn't reserve MMCONFIG the region in memory map.
> On the other hand OVMF reserves it in memory map without declaring a
> motherboard resource. With memory map reservation, linux guest uses
> MMCONFIG region. However it doesn't comply to PCI Firmware
> specification.
>
> [0] PCI Firmware specification Revision 3.2
> 4.1.2 MCFG Table Description table 4-2 NOTE 2
> If the operating system does not natively comprehend reserving the
> MMCFG region, The MMCFG region must e reserved by firmware. ...
> For most systems, the mortheroard resource would appear at the root
> of the ACPI namespace (under \_SB)...
> The resource can optionally be returned in Int15 E820h or
> EFIGetMemoryMap as reserved memory but must always be reported
> through ACPI as a motherboard resource
>
> [1] TDX: Intel Trust Domain Extension
> https://software.intel.com/content/www/us/en/develop/articles/intel-trust-domain-extensions.html
> [2] TDX Virtual Firmware
> https://github.com/tianocore/edk2-staging/tree/TDVF
>
> The change to DSDT is as follows.
> @@ -68,32 +68,51 @@
>
> If ((CDW3 != Local0))
> {
> CDW1 |= 0x10
> }
>
> CDW3 = Local0
> }
> Else
> {
> CDW1 |= 0x04
> }
>
> Return (Arg3)
> }
> }
> +
> + Device (DRAC)
> + {
> + Name (_HID, "PNP0C01" /* System Board */) // _HID: Hardware ID
> + Name (RBUF, ResourceTemplate ()
> + {
> + QWordMemory (ResourceProducer, PosDecode, MinFixed, MaxFixed, NonCacheable, ReadWrite,
> + 0x0000000000000000, // Granularity
> + 0x00000000B0000000, // Range Minimum
> + 0x00000000BFFFFFFF, // Range Maximum
> + 0x0000000000000000, // Translation Offset
> + 0x0000000010000000, // Length
> + ,, , AddressRangeMemory, TypeStatic)
> + })
> + Method (_CRS, 0, Serialized) // _CRS: Current Resource Settings
> + {
> + 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>
> ---
> hw/i386/acpi-build.c | 46 +++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 45 insertions(+), 1 deletion(-)
>
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 00cc119362..e369908b1a 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -1072,6 +1072,46 @@ static void build_q35_pci0_int(Aml *table)
> aml_append(table, sb_scope);
> }
>
> +static Aml *build_q35_dram_controller(void)
> +{
> + AcpiMcfgInfo mcfg;
build_dsdt() already calls acpi_get_mcfg(),
I suggest to cache it there and pass to build_q35_dram_controller
as an argument.
> + Aml *dev;
> + Aml *rbuf;
> + Aml *resource_template;
> + Aml *rbuf_name;
> + Aml *crs;
> +
> + if (!acpi_get_mcfg(&mcfg)) {
> + return NULL;
> + }
> +
> + /* DRAM controller */
> + dev = aml_device("DRAC");
> + aml_append(dev, aml_name_decl("_HID", aml_string("PNP0C01")));
> +
> + 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,
> + mcfg.base,
> + mcfg.base + mcfg.size - 1,
s/mcfg.base + mcfg.size - 1/mcfg.base/
> + 0x0000000000000000,
> + mcfg.size));
> + rbuf = aml_name_decl("RBUF", resource_template);
> + aml_append(dev, rbuf);
> +
> + crs = aml_method("_CRS", 0, AML_SERIALIZED);
> + rbuf_name = aml_name("RBUF");
> + aml_append(crs, aml_return(rbuf_name));
> + aml_append(dev, crs);
> +
> + return dev;
> +}
> +
> static void build_q35_isa_bridge(Aml *table)
> {
> Aml *dev;
> @@ -1212,7 +1252,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> Range *pci_hole, Range *pci_hole64, MachineState *machine)
> {
> CrsRangeEntry *entry;
> - Aml *dsdt, *sb_scope, *scope, *dev, *method, *field, *pkg, *crs;
> + Aml *dsdt, *sb_scope, *scope, *dev, *method, *field, *pkg, *crs, *drac;
maybe limit drac to
if (misc->is_piix4) {
... } else {
Aml *drac
scope
> CrsRangeSet crs_range_set;
> PCMachineState *pcms = PC_MACHINE(machine);
> PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(machine);
> @@ -1256,6 +1296,10 @@ 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);
> + drac = build_q35_dram_controller();
> + if (drac) {
> + aml_append(sb_scope, drac);
> + }
mmcfg isn't optional for q35, is it?
I'd simplify to:
aml_append(sb_scope, build_q35_dram_controller(mmcfg_info));
or put all of it under condition if it's optional
if(foo) aml_append(sb_scope, build_q35_dram_controller(mmcfg_info))
> if (pm->smi_on_cpuhp) {
> /* reserve SMI block resources, IO ports 0xB2, 0xB3 */
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v3 07/10] hw/i386: declare ACPI mother board resource for MMCONFIG region
2021-02-12 15:40 ` Igor Mammedov
@ 2021-02-12 20:51 ` Isaku Yamahata
2021-02-15 12:48 ` Igor Mammedov
0 siblings, 1 reply; 21+ messages in thread
From: Isaku Yamahata @ 2021-02-12 20:51 UTC (permalink / raw)
To: Igor Mammedov; +Cc: Isaku Yamahata, qemu-devel, isaku.yamahata, mst
On Fri, Feb 12, 2021 at 04:40:38PM +0100,
Igor Mammedov <imammedo@redhat.com> wrote:
> On Wed, 10 Feb 2021 22:46:43 -0800
> Isaku Yamahata <isaku.yamahata@intel.com> wrote:
>
> > Declare PNP0C01 device to reserve MMCONFIG region to conform to the
> > spec better and play nice with guest BIOSes/OSes.
> >
> > According to PCI Firmware Specification[0], MMCONFIG region must be
> > reserved by declaring a motherboard resource. It's optional to reserve
> > the region in memory map by Int 15 E820h or EFIGetMemoryMap.
> > Guest Linux checks if the MMCFG region is reserved by bios memory map
> > or ACPI resource. If it's not reserved, Linux falls back to legacy PCI
> > configuration access.
> >
> > TDVF [1] [2] doesn't reserve MMCONFIG the region in memory map.
> > On the other hand OVMF reserves it in memory map without declaring a
> > motherboard resource. With memory map reservation, linux guest uses
> > MMCONFIG region. However it doesn't comply to PCI Firmware
> > specification.
> >
> > [0] PCI Firmware specification Revision 3.2
> > 4.1.2 MCFG Table Description table 4-2 NOTE 2
> > If the operating system does not natively comprehend reserving the
> > MMCFG region, The MMCFG region must e reserved by firmware. ...
> > For most systems, the mortheroard resource would appear at the root
> > of the ACPI namespace (under \_SB)...
> > The resource can optionally be returned in Int15 E820h or
> > EFIGetMemoryMap as reserved memory but must always be reported
> > through ACPI as a motherboard resource
> >
> > [1] TDX: Intel Trust Domain Extension
> > https://software.intel.com/content/www/us/en/develop/articles/intel-trust-domain-extensions.html
> > [2] TDX Virtual Firmware
> > https://github.com/tianocore/edk2-staging/tree/TDVF
> >
> > The change to DSDT is as follows.
> > @@ -68,32 +68,51 @@
> >
> > If ((CDW3 != Local0))
> > {
> > CDW1 |= 0x10
> > }
> >
> > CDW3 = Local0
> > }
> > Else
> > {
> > CDW1 |= 0x04
> > }
> >
> > Return (Arg3)
> > }
> > }
> > +
> > + Device (DRAC)
> > + {
> > + Name (_HID, "PNP0C01" /* System Board */) // _HID: Hardware ID
> > + Name (RBUF, ResourceTemplate ()
> > + {
> > + QWordMemory (ResourceProducer, PosDecode, MinFixed, MaxFixed, NonCacheable, ReadWrite,
> > + 0x0000000000000000, // Granularity
> > + 0x00000000B0000000, // Range Minimum
> > + 0x00000000BFFFFFFF, // Range Maximum
> > + 0x0000000000000000, // Translation Offset
> > + 0x0000000010000000, // Length
> > + ,, , AddressRangeMemory, TypeStatic)
> > + })
> > + Method (_CRS, 0, Serialized) // _CRS: Current Resource Settings
> > + {
> > + 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>
> > ---
> > hw/i386/acpi-build.c | 46 +++++++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 45 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > index 00cc119362..e369908b1a 100644
> > --- a/hw/i386/acpi-build.c
> > +++ b/hw/i386/acpi-build.c
> > @@ -1072,6 +1072,46 @@ static void build_q35_pci0_int(Aml *table)
> > aml_append(table, sb_scope);
> > }
> >
> > +static Aml *build_q35_dram_controller(void)
> > +{
> > + AcpiMcfgInfo mcfg;
> build_dsdt() already calls acpi_get_mcfg(),
> I suggest to cache it there and pass to build_q35_dram_controller
> as an argument.
Sure.
> > + Aml *dev;
> > + Aml *rbuf;
> > + Aml *resource_template;
> > + Aml *rbuf_name;
> > + Aml *crs;
> > +
> > + if (!acpi_get_mcfg(&mcfg)) {
> > + return NULL;
> > + }
> > +
> > + /* DRAM controller */
> > + dev = aml_device("DRAC");
> > + aml_append(dev, aml_name_decl("_HID", aml_string("PNP0C01")));
> > +
> > + 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,
> > + mcfg.base,
> > + mcfg.base + mcfg.size - 1,
> s/mcfg.base + mcfg.size - 1/mcfg.base/
AddressMaximum needs to be the highest address of the region.
Not base address. By disassemble/assembl it, iasl complains as follows.
Also there are similar examples in acpi-build.c.
iasl drm-1.dsl
Intel ACPI Component Architecture
ASL+ Optimizing Compiler/Disassembler version 20190509
Copyright (c) 2000 - 2019 Intel Corporation
drm-1.dsl 23: 0x10000000, // Length = 256M
Error 6049 - ^ Length is larger than Min/Max window
ASL Input: drm-1.dsl - 1000 bytes 6 keywords 37 source lines
Compilation failed. 1 Errors, 0 Warnings, 0 Remarks
No AML files were generated due to compiler error(s)
> > + 0x0000000000000000,
> > + mcfg.size));
> > + rbuf = aml_name_decl("RBUF", resource_template);
> > + aml_append(dev, rbuf);
> > +
> > + crs = aml_method("_CRS", 0, AML_SERIALIZED);
> > + rbuf_name = aml_name("RBUF");
> > + aml_append(crs, aml_return(rbuf_name));
> > + aml_append(dev, crs);
> > +
> > + return dev;
> > +}
> > +
> > static void build_q35_isa_bridge(Aml *table)
> > {
> > Aml *dev;
> > @@ -1212,7 +1252,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> > Range *pci_hole, Range *pci_hole64, MachineState *machine)
> > {
> > CrsRangeEntry *entry;
> > - Aml *dsdt, *sb_scope, *scope, *dev, *method, *field, *pkg, *crs;
> > + Aml *dsdt, *sb_scope, *scope, *dev, *method, *field, *pkg, *crs, *drac;
> maybe limit drac to
> if (misc->is_piix4) {
> ... } else {
> Aml *drac
> scope
ok.
> > CrsRangeSet crs_range_set;
> > PCMachineState *pcms = PC_MACHINE(machine);
> > PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(machine);
> > @@ -1256,6 +1296,10 @@ 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);
> > + drac = build_q35_dram_controller();
> > + if (drac) {
> > + aml_append(sb_scope, drac);
> > + }
> mmcfg isn't optional for q35, is it?
> I'd simplify to:
> aml_append(sb_scope, build_q35_dram_controller(mmcfg_info));
> or put all of it under condition if it's optional
> if(foo) aml_append(sb_scope, build_q35_dram_controller(mmcfg_info))
It's optional. If MCFG isn't setup after reset, PCIE_BASE_ADDR_UNMAPPED
is returned.
--
Isaku Yamahata <isaku.yamahata@gmail.com>
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v3 07/10] hw/i386: declare ACPI mother board resource for MMCONFIG region
2021-02-12 20:51 ` Isaku Yamahata
@ 2021-02-15 12:48 ` Igor Mammedov
2021-02-16 9:43 ` Isaku Yamahata
0 siblings, 1 reply; 21+ messages in thread
From: Igor Mammedov @ 2021-02-15 12:48 UTC (permalink / raw)
To: Isaku Yamahata; +Cc: Isaku Yamahata, qemu-devel, mst
On Fri, 12 Feb 2021 12:51:57 -0800
Isaku Yamahata <isaku.yamahata@gmail.com> wrote:
> On Fri, Feb 12, 2021 at 04:40:38PM +0100,
> Igor Mammedov <imammedo@redhat.com> wrote:
>
> > On Wed, 10 Feb 2021 22:46:43 -0800
> > Isaku Yamahata <isaku.yamahata@intel.com> wrote:
> >
> > > Declare PNP0C01 device to reserve MMCONFIG region to conform to the
> > > spec better and play nice with guest BIOSes/OSes.
> > >
> > > According to PCI Firmware Specification[0], MMCONFIG region must be
> > > reserved by declaring a motherboard resource. It's optional to reserve
> > > the region in memory map by Int 15 E820h or EFIGetMemoryMap.
> > > Guest Linux checks if the MMCFG region is reserved by bios memory map
> > > or ACPI resource. If it's not reserved, Linux falls back to legacy PCI
> > > configuration access.
> > >
> > > TDVF [1] [2] doesn't reserve MMCONFIG the region in memory map.
> > > On the other hand OVMF reserves it in memory map without declaring a
> > > motherboard resource. With memory map reservation, linux guest uses
> > > MMCONFIG region. However it doesn't comply to PCI Firmware
> > > specification.
> > >
> > > [0] PCI Firmware specification Revision 3.2
> > > 4.1.2 MCFG Table Description table 4-2 NOTE 2
> > > If the operating system does not natively comprehend reserving the
> > > MMCFG region, The MMCFG region must e reserved by firmware. ...
> > > For most systems, the mortheroard resource would appear at the root
> > > of the ACPI namespace (under \_SB)...
> > > The resource can optionally be returned in Int15 E820h or
> > > EFIGetMemoryMap as reserved memory but must always be reported
> > > through ACPI as a motherboard resource
> > >
> > > [1] TDX: Intel Trust Domain Extension
> > > https://software.intel.com/content/www/us/en/develop/articles/intel-trust-domain-extensions.html
> > > [2] TDX Virtual Firmware
> > > https://github.com/tianocore/edk2-staging/tree/TDVF
> > >
> > > The change to DSDT is as follows.
> > > @@ -68,32 +68,51 @@
> > >
> > > If ((CDW3 != Local0))
> > > {
> > > CDW1 |= 0x10
> > > }
> > >
> > > CDW3 = Local0
> > > }
> > > Else
> > > {
> > > CDW1 |= 0x04
> > > }
> > >
> > > Return (Arg3)
> > > }
> > > }
> > > +
> > > + Device (DRAC)
> > > + {
> > > + Name (_HID, "PNP0C01" /* System Board */) // _HID: Hardware ID
> > > + Name (RBUF, ResourceTemplate ()
> > > + {
> > > + QWordMemory (ResourceProducer, PosDecode, MinFixed, MaxFixed, NonCacheable, ReadWrite,
> > > + 0x0000000000000000, // Granularity
> > > + 0x00000000B0000000, // Range Minimum
> > > + 0x00000000BFFFFFFF, // Range Maximum
> > > + 0x0000000000000000, // Translation Offset
> > > + 0x0000000010000000, // Length
> > > + ,, , AddressRangeMemory, TypeStatic)
> > > + })
> > > + Method (_CRS, 0, Serialized) // _CRS: Current Resource Settings
> > > + {
> > > + 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>
> > > ---
> > > hw/i386/acpi-build.c | 46 +++++++++++++++++++++++++++++++++++++++++++-
> > > 1 file changed, 45 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > > index 00cc119362..e369908b1a 100644
> > > --- a/hw/i386/acpi-build.c
> > > +++ b/hw/i386/acpi-build.c
> > > @@ -1072,6 +1072,46 @@ static void build_q35_pci0_int(Aml *table)
> > > aml_append(table, sb_scope);
> > > }
> > >
> > > +static Aml *build_q35_dram_controller(void)
> > > +{
> > > + AcpiMcfgInfo mcfg;
> > build_dsdt() already calls acpi_get_mcfg(),
> > I suggest to cache it there and pass to build_q35_dram_controller
> > as an argument.
>
> Sure.
>
>
> > > + Aml *dev;
> > > + Aml *rbuf;
> > > + Aml *resource_template;
> > > + Aml *rbuf_name;
> > > + Aml *crs;
> > > +
> > > + if (!acpi_get_mcfg(&mcfg)) {
> > > + return NULL;
> > > + }
> > > +
> > > + /* DRAM controller */
> > > + dev = aml_device("DRAC");
> > > + aml_append(dev, aml_name_decl("_HID", aml_string("PNP0C01")));
> > > +
> > > + 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,
> > > + mcfg.base,
> > > + mcfg.base + mcfg.size - 1,
> > s/mcfg.base + mcfg.size - 1/mcfg.base/
>
> AddressMaximum needs to be the highest address of the region.
> Not base address. By disassemble/assembl it, iasl complains as follows.
> Also there are similar examples in acpi-build.c.
I mostly clean up all places to use the same base address for min/max,
but probably something were left behind.
spec says:
acpi 6.3: 19.6.110 QWordMemory
AddressMaximum evaluates to a 64-bit integer that specifies the highest possible base address of the
Memory range. The value must have ‘0’ in all bits where the corresponding bit in AddressGranularity is
‘1’. For bridge devices which translate addresses, this is the address on the secondary bus. The 64-bit
field DescriptorName ._MAX is automatically created to refer to this portion of the resource descriptor.
> iasl drm-1.dsl
>
> Intel ACPI Component Architecture
> ASL+ Optimizing Compiler/Disassembler version 20190509
> Copyright (c) 2000 - 2019 Intel Corporation
>
> drm-1.dsl 23: 0x10000000, // Length = 256M
> Error 6049 - ^ Length is larger than Min/Max window
>
> ASL Input: drm-1.dsl - 1000 bytes 6 keywords 37 source lines
>
> Compilation failed. 1 Errors, 0 Warnings, 0 Remarks
> No AML files were generated due to compiler error(s)
>
>
> > > + 0x0000000000000000,
> > > + mcfg.size));
> > > + rbuf = aml_name_decl("RBUF", resource_template);
> > > + aml_append(dev, rbuf);
> > > +
> > > + crs = aml_method("_CRS", 0, AML_SERIALIZED);
> > > + rbuf_name = aml_name("RBUF");
> > > + aml_append(crs, aml_return(rbuf_name));
> > > + aml_append(dev, crs);
> > > +
> > > + return dev;
> > > +}
> > > +
> > > static void build_q35_isa_bridge(Aml *table)
> > > {
> > > Aml *dev;
> > > @@ -1212,7 +1252,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> > > Range *pci_hole, Range *pci_hole64, MachineState *machine)
> > > {
> > > CrsRangeEntry *entry;
> > > - Aml *dsdt, *sb_scope, *scope, *dev, *method, *field, *pkg, *crs;
> > > + Aml *dsdt, *sb_scope, *scope, *dev, *method, *field, *pkg, *crs, *drac;
> > maybe limit drac to
> > if (misc->is_piix4) {
> > ... } else {
> > Aml *drac
> > scope
>
> ok.
>
>
> > > CrsRangeSet crs_range_set;
> > > PCMachineState *pcms = PC_MACHINE(machine);
> > > PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(machine);
> > > @@ -1256,6 +1296,10 @@ 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);
> > > + drac = build_q35_dram_controller();
> > > + if (drac) {
> > > + aml_append(sb_scope, drac);
> > > + }
> > mmcfg isn't optional for q35, is it?
> > I'd simplify to:
> > aml_append(sb_scope, build_q35_dram_controller(mmcfg_info));
> > or put all of it under condition if it's optional
> > if(foo) aml_append(sb_scope, build_q35_dram_controller(mmcfg_info))
>
> It's optional. If MCFG isn't setup after reset, PCIE_BASE_ADDR_UNMAPPED
> is returned.
ok
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v3 07/10] hw/i386: declare ACPI mother board resource for MMCONFIG region
2021-02-15 12:48 ` Igor Mammedov
@ 2021-02-16 9:43 ` Isaku Yamahata
2021-02-16 13:45 ` Michael S. Tsirkin
0 siblings, 1 reply; 21+ messages in thread
From: Isaku Yamahata @ 2021-02-16 9:43 UTC (permalink / raw)
To: Igor Mammedov; +Cc: Isaku Yamahata, qemu-devel, Isaku Yamahata, mst
On Mon, Feb 15, 2021 at 01:48:32PM +0100,
Igor Mammedov <imammedo@redhat.com> wrote:
> On Fri, 12 Feb 2021 12:51:57 -0800
> Isaku Yamahata <isaku.yamahata@gmail.com> wrote:
>
> > On Fri, Feb 12, 2021 at 04:40:38PM +0100,
> > Igor Mammedov <imammedo@redhat.com> wrote:
> >
> > > On Wed, 10 Feb 2021 22:46:43 -0800
> > > Isaku Yamahata <isaku.yamahata@intel.com> wrote:
> > >
> > > > + Aml *dev;
> > > > + Aml *rbuf;
> > > > + Aml *resource_template;
> > > > + Aml *rbuf_name;
> > > > + Aml *crs;
> > > > +
> > > > + if (!acpi_get_mcfg(&mcfg)) {
> > > > + return NULL;
> > > > + }
> > > > +
> > > > + /* DRAM controller */
> > > > + dev = aml_device("DRAC");
> > > > + aml_append(dev, aml_name_decl("_HID", aml_string("PNP0C01")));
> > > > +
> > > > + 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,
> > > > + mcfg.base,
> > > > + mcfg.base + mcfg.size - 1,
> > > s/mcfg.base + mcfg.size - 1/mcfg.base/
> >
> > AddressMaximum needs to be the highest address of the region.
> > Not base address. By disassemble/assembl it, iasl complains as follows.
> > Also there are similar examples in acpi-build.c.
> I mostly clean up all places to use the same base address for min/max,
> but probably something were left behind.
>
> spec says:
>
> acpi 6.3: 19.6.110 QWordMemory
>
> AddressMaximum evaluates to a 64-bit integer that specifies the highest possible base address of the
> Memory range. The value must have ‘0’ in all bits where the corresponding bit in AddressGranularity is
> ‘1’. For bridge devices which translate addresses, this is the address on the secondary bus. The 64-bit
> field DescriptorName ._MAX is automatically created to refer to this portion of the resource descriptor.
Ok, Linux guest is happy with min=max.
I conlude that it's iasl issue.
Thanks,
--
Isaku Yamahata <isaku.yamahata@gmail.com>
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v3 07/10] hw/i386: declare ACPI mother board resource for MMCONFIG region
2021-02-16 9:43 ` Isaku Yamahata
@ 2021-02-16 13:45 ` Michael S. Tsirkin
2021-02-16 18:13 ` Isaku Yamahata
0 siblings, 1 reply; 21+ messages in thread
From: Michael S. Tsirkin @ 2021-02-16 13:45 UTC (permalink / raw)
To: Isaku Yamahata; +Cc: Isaku Yamahata, Igor Mammedov, qemu-devel
On Tue, Feb 16, 2021 at 01:43:01AM -0800, Isaku Yamahata wrote:
> On Mon, Feb 15, 2021 at 01:48:32PM +0100,
> Igor Mammedov <imammedo@redhat.com> wrote:
>
> > On Fri, 12 Feb 2021 12:51:57 -0800
> > Isaku Yamahata <isaku.yamahata@gmail.com> wrote:
> >
> > > On Fri, Feb 12, 2021 at 04:40:38PM +0100,
> > > Igor Mammedov <imammedo@redhat.com> wrote:
> > >
> > > > On Wed, 10 Feb 2021 22:46:43 -0800
> > > > Isaku Yamahata <isaku.yamahata@intel.com> wrote:
> > > >
> > > > > + Aml *dev;
> > > > > + Aml *rbuf;
> > > > > + Aml *resource_template;
> > > > > + Aml *rbuf_name;
> > > > > + Aml *crs;
> > > > > +
> > > > > + if (!acpi_get_mcfg(&mcfg)) {
> > > > > + return NULL;
> > > > > + }
> > > > > +
> > > > > + /* DRAM controller */
> > > > > + dev = aml_device("DRAC");
> > > > > + aml_append(dev, aml_name_decl("_HID", aml_string("PNP0C01")));
> > > > > +
> > > > > + 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,
> > > > > + mcfg.base,
> > > > > + mcfg.base + mcfg.size - 1,
> > > > s/mcfg.base + mcfg.size - 1/mcfg.base/
> > >
> > > AddressMaximum needs to be the highest address of the region.
> > > Not base address. By disassemble/assembl it, iasl complains as follows.
> > > Also there are similar examples in acpi-build.c.
> > I mostly clean up all places to use the same base address for min/max,
> > but probably something were left behind.
> >
> > spec says:
> >
> > acpi 6.3: 19.6.110 QWordMemory
> >
> > AddressMaximum evaluates to a 64-bit integer that specifies the highest possible base address of the
> > Memory range. The value must have ‘0’ in all bits where the corresponding bit in AddressGranularity is
> > ‘1’. For bridge devices which translate addresses, this is the address on the secondary bus. The 64-bit
> > field DescriptorName ._MAX is automatically created to refer to this portion of the resource descriptor.
>
> Ok, Linux guest is happy with min=max.
> I conlude that it's iasl issue.
>
> Thanks,
OK but what about all the other places in the code that seem to use this
field differently?
--
MST
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v3 07/10] hw/i386: declare ACPI mother board resource for MMCONFIG region
2021-02-16 13:45 ` Michael S. Tsirkin
@ 2021-02-16 18:13 ` Isaku Yamahata
2021-02-16 22:04 ` Igor Mammedov
0 siblings, 1 reply; 21+ messages in thread
From: Isaku Yamahata @ 2021-02-16 18:13 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Isaku Yamahata, Igor Mammedov, qemu-devel, Isaku Yamahata
On Tue, Feb 16, 2021 at 08:45:48AM -0500,
"Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Tue, Feb 16, 2021 at 01:43:01AM -0800, Isaku Yamahata wrote:
> > On Mon, Feb 15, 2021 at 01:48:32PM +0100,
> > Igor Mammedov <imammedo@redhat.com> wrote:
> >
> > > On Fri, 12 Feb 2021 12:51:57 -0800
> > > Isaku Yamahata <isaku.yamahata@gmail.com> wrote:
> > >
> > > > On Fri, Feb 12, 2021 at 04:40:38PM +0100,
> > > > Igor Mammedov <imammedo@redhat.com> wrote:
> > > >
> > > > > On Wed, 10 Feb 2021 22:46:43 -0800
> > > > > Isaku Yamahata <isaku.yamahata@intel.com> wrote:
> > > > >
> > > > > > + Aml *dev;
> > > > > > + Aml *rbuf;
> > > > > > + Aml *resource_template;
> > > > > > + Aml *rbuf_name;
> > > > > > + Aml *crs;
> > > > > > +
> > > > > > + if (!acpi_get_mcfg(&mcfg)) {
> > > > > > + return NULL;
> > > > > > + }
> > > > > > +
> > > > > > + /* DRAM controller */
> > > > > > + dev = aml_device("DRAC");
> > > > > > + aml_append(dev, aml_name_decl("_HID", aml_string("PNP0C01")));
> > > > > > +
> > > > > > + 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,
> > > > > > + mcfg.base,
> > > > > > + mcfg.base + mcfg.size - 1,
> > > > > s/mcfg.base + mcfg.size - 1/mcfg.base/
> > > >
> > > > AddressMaximum needs to be the highest address of the region.
> > > > Not base address. By disassemble/assembl it, iasl complains as follows.
> > > > Also there are similar examples in acpi-build.c.
> > > I mostly clean up all places to use the same base address for min/max,
> > > but probably something were left behind.
> > >
> > > spec says:
> > >
> > > acpi 6.3: 19.6.110 QWordMemory
> > >
> > > AddressMaximum evaluates to a 64-bit integer that specifies the highest possible base address of the
> > > Memory range. The value must have ‘0’ in all bits where the corresponding bit in AddressGranularity is
> > > ‘1’. For bridge devices which translate addresses, this is the address on the secondary bus. The 64-bit
> > > field DescriptorName ._MAX is automatically created to refer to this portion of the resource descriptor.
> >
> > Ok, Linux guest is happy with min=max.
> > I conlude that it's iasl issue.
> >
> > Thanks,
>
> OK but what about all the other places in the code that seem to use this
> field differently?
Igor, what do you think?
The followings are the summary of the situation.
- acpi 6.4: 19.6.110 QWordMemory
_MAX: maximum of base address.
- acpi 6.4: 6.4.3.5 Address Space Resource Descriptors
table 6.44
If _LEN > 0 and _MIF = 1 and _MAF = 1, then _LEN = _MAX - _MIN + 1
This doesn't match with the above description
- iasl
If _LEN > 0 and _MIF = 1 and _MAF = 1,
it emits warning on _LEN != _MAX - _MIN + 1
- Linux Guest MMCONFIG check
check_mcfg_resoure() uses only _MIN. doesn't use _MAX.
_MAX value doesn't matter
- Linux acpi code
acpi_decode_space() uses _MAX to calulate range [start, end], not use _LEN.
i.e. It assumes _LEN = _MAX - _MIN + 1 if _LEN > 0, _MIF = 1, _MAF = 1
- qemu code sets for [qd]word_memory_resource (except this patch)
If _LEN > 0 and _MIF = 1 and _MAF = 1, then set _LEN = _MAX - _MIN + 1
--
Isaku Yamahata <isaku.yamahata@gmail.com>
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v3 07/10] hw/i386: declare ACPI mother board resource for MMCONFIG region
2021-02-16 18:13 ` Isaku Yamahata
@ 2021-02-16 22:04 ` Igor Mammedov
0 siblings, 0 replies; 21+ messages in thread
From: Igor Mammedov @ 2021-02-16 22:04 UTC (permalink / raw)
To: Isaku Yamahata; +Cc: Isaku Yamahata, qemu-devel, Michael S. Tsirkin
On Tue, 16 Feb 2021 10:13:25 -0800
Isaku Yamahata <isaku.yamahata@gmail.com> wrote:
> On Tue, Feb 16, 2021 at 08:45:48AM -0500,
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>
> > On Tue, Feb 16, 2021 at 01:43:01AM -0800, Isaku Yamahata wrote:
> > > On Mon, Feb 15, 2021 at 01:48:32PM +0100,
> > > Igor Mammedov <imammedo@redhat.com> wrote:
> > >
> > > > On Fri, 12 Feb 2021 12:51:57 -0800
> > > > Isaku Yamahata <isaku.yamahata@gmail.com> wrote:
> > > >
> > > > > On Fri, Feb 12, 2021 at 04:40:38PM +0100,
> > > > > Igor Mammedov <imammedo@redhat.com> wrote:
> > > > >
> > > > > > On Wed, 10 Feb 2021 22:46:43 -0800
> > > > > > Isaku Yamahata <isaku.yamahata@intel.com> wrote:
> > > > > >
> > > > > > > + Aml *dev;
> > > > > > > + Aml *rbuf;
> > > > > > > + Aml *resource_template;
> > > > > > > + Aml *rbuf_name;
> > > > > > > + Aml *crs;
> > > > > > > +
> > > > > > > + if (!acpi_get_mcfg(&mcfg)) {
> > > > > > > + return NULL;
> > > > > > > + }
> > > > > > > +
> > > > > > > + /* DRAM controller */
> > > > > > > + dev = aml_device("DRAC");
> > > > > > > + aml_append(dev, aml_name_decl("_HID", aml_string("PNP0C01")));
> > > > > > > +
> > > > > > > + 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,
> > > > > > > + mcfg.base,
> > > > > > > + mcfg.base + mcfg.size - 1,
> > > > > > s/mcfg.base + mcfg.size - 1/mcfg.base/
> > > > >
> > > > > AddressMaximum needs to be the highest address of the region.
> > > > > Not base address. By disassemble/assembl it, iasl complains as follows.
> > > > > Also there are similar examples in acpi-build.c.
> > > > I mostly clean up all places to use the same base address for min/max,
> > > > but probably something were left behind.
> > > >
> > > > spec says:
> > > >
> > > > acpi 6.3: 19.6.110 QWordMemory
> > > >
> > > > AddressMaximum evaluates to a 64-bit integer that specifies the highest possible base address of the
> > > > Memory range. The value must have ‘0’ in all bits where the corresponding bit in AddressGranularity is
> > > > ‘1’. For bridge devices which translate addresses, this is the address on the secondary bus. The 64-bit
> > > > field DescriptorName ._MAX is automatically created to refer to this portion of the resource descriptor.
> > >
> > > Ok, Linux guest is happy with min=max.
> > > I conlude that it's iasl issue.
> > >
> > > Thanks,
> >
> > OK but what about all the other places in the code that seem to use this
> > field differently?
>
> Igor, what do you think?
> The followings are the summary of the situation.
>
> - acpi 6.4: 19.6.110 QWordMemory
> _MAX: maximum of base address.
>
> - acpi 6.4: 6.4.3.5 Address Space Resource Descriptors
> table 6.44
> If _LEN > 0 and _MIF = 1 and _MAF = 1, then _LEN = _MAX - _MIN + 1
> This doesn't match with the above description
I'd say it 19.6.110 doesn't describes whole possibilities of resource,
but only subset in its current phrasing.
> - iasl
> If _LEN > 0 and _MIF = 1 and _MAF = 1,
> it emits warning on _LEN != _MAX - _MIN + 1
so IASL is right and follows spec.
Anyways, My apologies for misleading and your patch is correct.
I need to add similar checks to QEMU code plus comments to avoid confusion
in future.
> - Linux Guest MMCONFIG check
> check_mcfg_resoure() uses only _MIN. doesn't use _MAX.
> _MAX value doesn't matter
>
> - Linux acpi code
> acpi_decode_space() uses _MAX to calulate range [start, end], not use _LEN.
> i.e. It assumes _LEN = _MAX - _MIN + 1 if _LEN > 0, _MIF = 1, _MAF = 1
>
> - qemu code sets for [qd]word_memory_resource (except this patch)
> If _LEN > 0 and _MIF = 1 and _MAF = 1, then set _LEN = _MAX - _MIN + 1
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v3 08/10] i386: acpi: Don't build HPET ACPI entry if HPET is disabled
2021-02-11 6:46 [PATCH v3 00/10] ACPI related fixes to comform the spec better Isaku Yamahata
` (6 preceding siblings ...)
2021-02-11 6:46 ` [PATCH v3 07/10] hw/i386: declare ACPI mother board resource for MMCONFIG region Isaku Yamahata
@ 2021-02-11 6:46 ` Isaku Yamahata
2021-02-11 6:46 ` [PATCH v3 09/10] acpi: add test case for -no-hpet Isaku Yamahata
2021-02-11 6:46 ` [PATCH v3 10/10] qtest/acpi/bios-tables-test: update acpi tables Isaku Yamahata
9 siblings, 0 replies; 21+ messages in thread
From: Isaku Yamahata @ 2021-02-11 6:46 UTC (permalink / raw)
To: qemu-devel, imammedo, mst, marcel.apfelbaum
Cc: isaku.yamahata, Sean Christopherson, isaku.yamahata
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>
Reviewed-by: Igor Mammedov <imammedo@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 e369908b1a..bfdadd7907 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1280,7 +1280,9 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
aml_append(sb_scope, dev);
aml_append(dsdt, sb_scope);
- build_hpet_aml(dsdt);
+ if (misc->has_hpet) {
+ build_hpet_aml(dsdt);
+ }
build_piix4_isa_bridge(dsdt);
build_isa_devices_aml(dsdt);
if (pm->pcihp_bridge_en || pm->pcihp_root_en) {
@@ -1328,7 +1330,9 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
aml_append(dsdt, sb_scope);
- build_hpet_aml(dsdt);
+ if (misc->has_hpet) {
+ build_hpet_aml(dsdt);
+ }
build_q35_isa_bridge(dsdt);
build_isa_devices_aml(dsdt);
build_q35_pci0_int(dsdt);
--
2.17.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v3 09/10] acpi: add test case for -no-hpet
2021-02-11 6:46 [PATCH v3 00/10] ACPI related fixes to comform the spec better Isaku Yamahata
` (7 preceding siblings ...)
2021-02-11 6:46 ` [PATCH v3 08/10] i386: acpi: Don't build HPET ACPI entry if HPET is disabled Isaku Yamahata
@ 2021-02-11 6:46 ` Isaku Yamahata
2021-02-11 6:46 ` [PATCH v3 10/10] qtest/acpi/bios-tables-test: update acpi tables Isaku Yamahata
9 siblings, 0 replies; 21+ messages in thread
From: Isaku Yamahata @ 2021-02-11 6:46 UTC (permalink / raw)
To: qemu-devel, imammedo, mst, marcel.apfelbaum
Cc: isaku.yamahata, isaku.yamahata
Reviewed-by: Igor Mammedov <imammedo@redhat.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] 21+ messages in thread* [PATCH v3 10/10] qtest/acpi/bios-tables-test: update acpi tables
2021-02-11 6:46 [PATCH v3 00/10] ACPI related fixes to comform the spec better Isaku Yamahata
` (8 preceding siblings ...)
2021-02-11 6:46 ` [PATCH v3 09/10] acpi: add test case for -no-hpet Isaku Yamahata
@ 2021-02-11 6:46 ` Isaku Yamahata
9 siblings, 0 replies; 21+ messages in thread
From: Isaku Yamahata @ 2021-02-11 6:46 UTC (permalink / raw)
To: qemu-devel, imammedo, mst, marcel.apfelbaum
Cc: isaku.yamahata, isaku.yamahata
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 -> 7892 bytes
tests/data/acpi/q35/DSDT.acpihmat | Bin 9126 -> 9217 bytes
tests/data/acpi/q35/DSDT.bridge | Bin 7819 -> 7910 bytes
tests/data/acpi/q35/DSDT.cphp | Bin 8265 -> 8356 bytes
tests/data/acpi/q35/DSDT.dimmpxm | Bin 9455 -> 9546 bytes
tests/data/acpi/q35/DSDT.ipmibt | Bin 7876 -> 7967 bytes
tests/data/acpi/q35/DSDT.memhp | Bin 9160 -> 9251 bytes
tests/data/acpi/q35/DSDT.mmio64 | Bin 8932 -> 9023 bytes
tests/data/acpi/q35/DSDT.nohpet | Bin 0 -> 7750 bytes
tests/data/acpi/q35/DSDT.nosmm | Bin 0 -> 7892 bytes
tests/data/acpi/q35/DSDT.numamem | Bin 7807 -> 7898 bytes
tests/data/acpi/q35/DSDT.tis | Bin 8407 -> 8498 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..6d3097f30420049230a42706208f6013e22dce2f 100644
GIT binary patch
delta 121
zcmexqbH$d+CD<k8iW~z2<J^f{sZ4GH6AM>HM>l!0x&%2obHsaiy6^`01sFIR7&34K
zIfc3j8gm(RX)`eJFft&(1`zZA|Nr|DLSU8vh^%A~;f{9>3g%b>(!W`XF<uq`0HztW
delta 29
lcmca&`_qQYCD<jTQjURv@!LeMR3>k(iG?dSmomo70sxmQ3AF$K
diff --git a/tests/data/acpi/q35/DSDT.acpihmat b/tests/data/acpi/q35/DSDT.acpihmat
index 722e06af83abcde203a2b96a8ec81fd3bab9fc98..7112fac785602bc559888d2e94447e4b4ce1c171 100644
GIT binary patch
delta 121
zcmZ4H-sr*Q66_MfsKUU&7&(zEmB~$DV&Tf@=q68Ammo)Hj(87G7v2EB00U<OLk5l@
zr%*RRV=jX(Z3YG&Mg|1f0Al|C|9?M12+R@yk(CT0-0{vq!5m9K`Zr54_9y`WM?M%v
delta 29
lcmZqlSmw^<66_MPOqqd!aq~p3R3>k(iG?dSmooM!0RV@p2}S?_
diff --git a/tests/data/acpi/q35/DSDT.bridge b/tests/data/acpi/q35/DSDT.bridge
index 06bac139d668ddfc7914e258b471a303c9dbd192..d3bbfcaa504f9c877de2a725e8e488023b6a75bb 100644
GIT binary patch
delta 121
zcmeCSeP+w$66_N4Opbwp(QG1DDwCVQ#KM))(M_JLE<ujY9Pu8WF1!JL0S3+nh724*
zPN8ms##{zn+6)Xlj0^~{0mS_O|Nnl35SS$ZA}bk0xZ|CJf;pCe^lz48ER+QRvSt|<
delta 29
kcmaE6+ilC`66_MvEyuvX*guggmC2iHV&TforHqBL0EsvW7XSbN
diff --git a/tests/data/acpi/q35/DSDT.cphp b/tests/data/acpi/q35/DSDT.cphp
index 2b933ac482e6883efccbd7d6c96089602f2c0b4d..8cf1fbe3cb04f02c9a12a835e9235303ab2169f3 100644
GIT binary patch
delta 121
zcmX@<u*8weCD<ioi2?%yBjZG_R3<lpiG?epqnkWgU4k5)IpRG$U3dfh0t}oD3>i3r
zoI>3MjkyfEv>6z97#R>?1Bm(m|Ns36Auvk-L{>70aK}3b1#>I`>EA5Hs4ouyftwg*
delta 29
kcmZ4Dc+!E(CD<jzQ-OhjF=`@LDw8+Y#KM)EOBwa$0f1)+WdHyG
diff --git a/tests/data/acpi/q35/DSDT.dimmpxm b/tests/data/acpi/q35/DSDT.dimmpxm
index bd8f8305b028ef20f9b6d1a0c69ac428d027e3d1..78a018140ff41e84b6ea93683cd0444f216ecded 100644
GIT binary patch
delta 121
zcmaFwdCH5+CD<jzOO=6v@ytZ7R3<lpiG?epqnkWgU4k5)IpRG$U3dfh0t}oD3>i3r
zoI>3MjkyfEv>6z97#R>?1Bm(m|Ns36Auvk-L{>70aK}3b1#>I`>EA5HcvTqy-i;aK
delta 29
lcmX@*_1=@qCD<k8y$S;ZquNBSR3>k(iG?dSmoi>e1^|{N3F80&
diff --git a/tests/data/acpi/q35/DSDT.ipmibt b/tests/data/acpi/q35/DSDT.ipmibt
index a8f868e23c25688ab1c0371016c071f23e9d732f..f27e355455b207e6336e14f33bb8a57c31f91604 100644
GIT binary patch
delta 121
zcmX?NJKv7WCD<iIUY>!0@xnx|R3<lpiG?epqnkWgU4k5)IpRG$U3dfh0t}oD3>i3r
zoI>3MjkyfEv>6z97#R>?1Bm(m|Ns36Auvk-L{>70aK}3b1#>I`>EA5HxJniPmirkO
delta 29
lcmbPlcf^*<CD<k8h#Uh0qt-;OR3>k(iG?dSmol!B1ptOe2^RnW
diff --git a/tests/data/acpi/q35/DSDT.memhp b/tests/data/acpi/q35/DSDT.memhp
index 9a802e4c67022386442976d5cb997ea3fc57b58f..97fe90178ad4ff250bea8e815706e26cc7cc2ac6 100644
GIT binary patch
delta 121
zcmX@%zSx7yCD<iIS%ratv2P+*DwCVQ#KM))(M_JLE<ujY9Pu8WF1!JL0S3+nh724*
zPN8ms##{zn+6)Xlj0^~{0mS_O|Nnl35SS$ZA}bk0xZ|CJf;pCe^lz48T&Dy8jA<DI
delta 29
lcmZ4Nal)O;CD<k8gfasI<BN%0sZ8En6AM>vE@fP&1OSyG3IhND
diff --git a/tests/data/acpi/q35/DSDT.mmio64 b/tests/data/acpi/q35/DSDT.mmio64
index 948c2dc7264c31932b490ca00691a7c4d9aefdb0..c744f209d4c8c3dd7ad31b2c9d800c22c2cba37e 100644
GIT binary patch
delta 121
zcmaFjy5Eh<CD<jzUYUV`(Rm_QDwCVQ#KM))(M_JLE<ujY9Pu8WF1!JL0S3+nh724*
zPN8ms##{zn+6)Xlj0^~{0mS_O|Nnl35SS$ZA}bk0xZ|CJf;pCe^lz48Jf#Q#k$f2<
delta 29
lcmdn*_QaLTCD<k8i4p??<J^f{sZ8En6AM>vE@eEW2mqMG3L*di
diff --git a/tests/data/acpi/q35/DSDT.nohpet b/tests/data/acpi/q35/DSDT.nohpet
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..b19f8c6bee66c187a707eb9b1b94cd7e1a7265da 100644
GIT binary patch
literal 7750
zcmb7JTW=f38J*=tS}m8-lA<MBzDRJ>dl6HT(*`YSl-%Vlx=~!IZa@K~Bqx>Q0$Ics
z5W@&yD?sD;p(r9a=%Z{XK;GJ?zSch>Z!yq+(APf1Pi4>d&5k@nQb5E9a?k8J^UayP
zoaJl~1{>8YLRjyY*Zg`oSN=oO571@_A!wU^y+&$Fv>yiLTF1&HQdav>hPUxmwhO-q
z%9}OoZ+qP@yWM-Ab*=44>^%3L?rcY&KJM-aBJJ-)jzb$>)$4@iPW!-jntr2bcj}G6
zEq&wIcE^`nNxM@DTEgyZHUd#t9__4!e(K2`;RiS8k5<!%d$~!+Yp?u!@w?eezx(p$
zz0!-{e*6BDJ5CCO1^n9hU5x0f`=IO0c8|O-4sTnZkev6;A5DI7$h&2mM|+|!wLEiW
z6LmFze5ZZTNC&Nw&x6c1SDQt@I4|s0d9#{Zbc#=YE`&?7qdxzjjb5+!FVztLdh~l=
z6fzyZ723z7_kQ#yO#@9q`(UOKmOAX(6=D|i+-$Vg6HeE0y`_n3Nkt@9`{4RgIRIHs
zTN0BMZj_t}%w8+Zop;XFtgh$^%c@zo#4Y?@O`vl7;Ek2F%@ftcq@l8%^i|;zYhk&$
zF|Ybg$wOcnT+hkYtdEl)CvE%cL+*LOUt2lRq`}5v8=`rYe3caTRgX14Y;E`_qsgES
z4W7G>L?kTxs<7@0yV>xoqTqJ$Yejp6yuZ9_(QJ7BfB58Swl<pirMnOFnN+6N6U(bh
z6(P4RY*$6zrFJI)P*WfYP#5`Z%I)=V@FXurqylkV?qeOO3X&52GdICXaAQPDMNJ4+
zg33xjQergBO|nvrASp<xs7a{=m6d>`R8>-}R3k{SQc+V<sRj?I`%D`?)2vj~v{b6W
z1L{6U44)&cRMZjEC!p?g)bKfK_#8ET0_r|(!>4Wdv`wFYy3aAg=a}Jh%=8JU`*aMS
zj^WcWeFEw}GltKM;WK0U1k`=vVwcCB&kNV^>6$(Pb)Q+oXV&nUHGKl=KF1B8<A%?1
z(<h+rbHeaBVfdUdeFEw}ar@JJdWKKW^a-f@%o#p&hR>Yo6Hxb=H+<#|pLx?Kpzd?h
z@HuJtoHTs`>OKpG&w}BzVEP2qeV#LXo-=%&GkpT;KBo+yQ-;qe(<h+rbK3AZZTOrv
zeFEw}&l^6^8$QpQJ^^)~7g$+}?}QgvS&naZ7o^gE4M`cq+!;eTV<=}#C7`anXecil
z%8RBFP*+|ul$Q+UB~uBgD=!<$%ZBo@sRY!Ocn;I9l`E`N_xLMPsczhWq*O^VYt)=I
zYR;N90cA}a0b0@mv=%lHFC3;8woy0^Pym#bAPJy~B0wdNZ()`ol^UOgqLO+NpnxjA
z5TKH)0#u_?fJ*8~fC6GPDnKPSMpRP|P)R)rP(X}`xd>3njZy58sT81+s-W%@0V=y?
zB?45VQh)-gRHOg}R6Y@)lFLd2s79p#l~k@t0Sc&mB0wdVl?YIcN&zaVtfT-1R6Y@)
zlFLd2s79p#l~h(zfC8%Hi2#*cRw6((Dg~&dvXTN6Q29iFN-irApc<6|R8m<<0Sc&m
zB0wdVl?YIcN&zaVtfT-1R6Y@)lFLd2s79p#l~h(zfC4I?2vEsoB?45VQh-V-D=9z$
zl}`ky<gyY0s!=IGC6$#Fpn%FJ0#tHYi2&886rhsIN(xXw<r4uaxvWHhYE%kPNo6Gk
zD4_C*0F_);B0x1N1*oL5k^&S^`9y$9E-MkB8kGW6Qdvm>3aES{KqZ%z2vChm0V=7i
zqyPm}J`tdj%Sr^OMx_9iR8~@e0xF*fP|0N_0#u_?fJ!PWDL?_0PXwssvJwHRQ7J$r
zm6a5rfXXKVRB~B~0M)1zppwc;3Q$1h69Fo@tVDonR0>c@WhDhDpz?_T1=0c(NDojz
zJwO4q0M(QTP)$hzswpWzH6;R6QzAe$B?YLaqyW{F2vALl0M(Qfpqi2bR8t~AH6;R6
zQ&NCxN(xX-i2wysM;8GKgd@h2NdXFo!=NB3)!ZULfz;e0K!McUQh)-gxupOF#PY;U
z2N5cc+(Z7edQ5*vpN-H@sr~5J*OK%vmHxKVA9}0VS}!4S*UR+f(^p`NIs8;;yGma*
z`l{Q7$;NuPVI7S<a31c7V`uEx$rP?P*8SEP2(4mn+{t`CBF1>};l6l6ZB82Bjp)Ii
zGa+aTgJ@xGVR;nK2qpEO-o-)zEvwiVc9@%TCO;n+V@af&!2Mg`PE5-b6SSltj%f%O
zO{tLU1dUL9Lfg(mZkeDz?8NX}jHf$cE3}^M#bcoI)E-wa_f^O0!1u0R)~c7edKu%>
ztCtJItC!mcE970S-e8F-_U<IkP~O+d`&{1dEAO9D-jB-%edZmk*nDsIfmS}?@<Ctu
z;FR(~T)xs@K6Qrj6|H=Q%UAl!S57HkiOW~}%csv!zN(e4a`|ds`RXa<t8w{SfBBI!
zl&@*!Yh1q8SH5;i`C44Q-d}$74CU)u`8t=c_m!`oQofGzDY}*Qm-l3Ob@QNQNi4JR
z;+xr47~xAEKXhHk-SuC_PR{HO&l&^W$+n^04P2LbvpYP140I>khITh_oneiWt}|r}
zPbCB0$+n^04O}+`vpYPh40I>khITh_9h@_}!}H5Pcd~70cLV43l-V5~Y6iNKZ9}^o
zI8Ud|?(n2D(4A~EyYnt;i`o<6N*T-hSju;kSk(5`Zpd~2y0i6qNxRvqgz7AHaB(7=
zvVL=OuKwY@=ui89ocrj#yC3d<wENy&(YE~N@=|=ZW*w(yy>wr?mWyR;WD>$M^U`(P
z_M@K=D2X}0Sy{FG#%gH!t(D(fe&wAaeN`=y^!=5<Za>NsGwFww<vGxU(#9Otnj5PP
zI}+50vwm@l{a`?HklB*oY!r+0MH+^DS&=T}{Eh0ni6iH5PyE?07K^tF^${OK7;FDv
z>sm^!-IGx=(CbBe{mm#AiSE(NVgC-Y5IGaw_KyelEH52*3oB`|b+$rQe^=;XX#a)k
zlpp%5_9-@Of3<QmpXfzmH?nR$5Auh%tVrC7tZpQ>pD(+AJ?vUfchS_1cKb`R;r>P(
zl%wgV{jE4GyI&oR9d0{4l=&(ew*TmjftjP@9_>D+F>Z_fZH(&Kua4dtpz)c3!?)`s
zqk)ORPU!Y`>19?WCEsRIhdW}I4mrH-w&#bN8|Vw;+20vH%`0R6TDak!GR?!!mVy2;
zdC~15a}zm<{#K%X(BFt@9y+@Ar*G3ajq4vDTpl>D?PEH-aRscDHgXxq7Fcbd=g`79
zUes-5$c)&#{XyX(E}mMm(y0^|M0~~5g+%9+SD-z7-N%<y@t49Y<>ciiUklP3p?5OQ
zKObjB0{`>z>CqS7YrS$`xt5J~3pW;gzQgWU?g`G~MUW8x{R}rKxu2eGdp&x#rB6ce
z9)5BLwDG#`@4Q2ZuZ=w~@E-4fa?gt!{Hl+u#ay-wZ4d5^4`F(h=BHp$zTV_Fhb35)
z_cv<SZfx@PT1Lh&-`M6x=0+^MkMn-I1xdKr$&aKg3>e$jJ9#_hBpj@s0lh_gsJi!?
z*HhO%c(Z!bmw}UAudHn1_$?3{uL<=kUfN63?bEBKw)mae4f!%M6Bdi|Wh7kuEG)h)
V0VdfJ2*GeZ5XZc+pf(n1<NtS=8)^Um
literal 0
HcmV?d00001
diff --git a/tests/data/acpi/q35/DSDT.nosmm b/tests/data/acpi/q35/DSDT.nosmm
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..6d3097f30420049230a42706208f6013e22dce2f 100644
GIT binary patch
literal 7892
zcmb7J&2JmW9i8Qev|6sDB}GfNY)68deitz%Icd<MM#)`%MWRHJRNR09NJ&mB#|5&8
zEg-HDKvsan@u6s<I>@B}Z74u*?Wx!LPw=5Y4fND&uSF4`%)Z}mIPwfh0TB=6zS;NY
zH*faiEoXyz(D=tILRf#wt@)+;eC}6OKR}x$grIHu^~$kL(cBMm#g>(b#H{AM1aIRj
zXQzJ><Ti@dSG(;m+wI$*wymvDY(MrMZf}Jj-*0aVBJFL5jzb$>!E4oXt>%I6RQ+<s
zZk5V`n|<oocFUJrQM;85YQk=9lmn4o8EviA{n&$>!Vj)3bXMbsyU8iXYgYcX^v&Fb
zUw(P*cJ|3H{`u<%H=P&=i}<ziyA;w_`)=ErYj?aa4sTc=k(~D}bf&&I<lVB(qdj4p
zS{}KwiMm=iy4gG^$Aend=RuBFSF0I6vmoqRZljP~axxEoEQCw5qdtG7jc&L5ch%7Q
z_2~DGNGDo;t!^KY-mBPh8uE6jns2CSuwb5SH}^}`Mn$+nJbjT6vF1IG@YLJ+)nL9_
z3tIE(H(K>ZR)o~j4ykGDcWuXOcihhp+v2dD=ys1pXX5k2E%C=KSN!{b@gs5MOk$F2
z^<>hS_+*U6^z?;@(a5~-y+)&<1JXR0E!VRxJ_aa1hoMx<we^V8c3f|H@^VxWiPb!~
zvYZP*mIpD4(PLDq%vtA5(Q1pfu&knWU0lcSr3k7u4_>XTZ5*p6rwo<t#4ibtSc@yw
z#)9fQCQk^<;Cjw@(fTm@Vbr!S?Q_qI{#xZ&69*f!IYje3`aCM^OCD>!TWk2oqe-9+
z%@KDUicnbgC1KqWcD3vmMA~iP*9vzDdB1<hqS^5L|M00?tu?Bt<y-ryL=4j}R#unu
zLT+2wE{K#%?M?)ss6Z5;BvRusx7)?3BzZkVDi9~+KGuP%ASuy5b0e$-w}(ins1d<R
zP+18`N{oiNQC6xEL<K1oH7b>$vJ#M#stQgV$>n}xtW?yPRI0%P>OSL!&p0a;H7=EE
z@PN9{5yR&QD;0Ib^a-f@95sB78a_u&pMbhg+wf@{K5f${pzafwm^Ra6hR-q6C!p@r
zF?>3PPsj8LsQXM9J`;w|gy|Dd_lb*LJD#rL(=~kp>ORK}pW}wlanmQD?sLNMIbrym
zFnt2*J|_*IlZMYp(<h+r6E|bcr)T)|OrL<d&!piqY4}W<J^^)~DZ^*V@R>4w0_r}e
z44+em&neR<pzbqm_)Hr<)22^A-RBv@=NZH28Pg}A?sMAkIc@lyHhlu>K4%P{GltI@
z(<h+r^Q__Xtl{&l=@U@*d5)FY-ktCqD|5Y@-8rfBUqVs_J?^ZboHdlQrV>zBo;Q@|
z4dr=L38*VC7|IKV@`9-Z)Rh+v<wZkz(NqHJN<0;6*UBrbRQLE-q*C3u0ZFNnWX`BL
zXVjcCYXZueHUc!O1!%EeJ`PZ9qi`=k0Z>+gD1a)80F^wBBuiM5s1h?GsS1)(3sA{b
z0jg0cKqd7!Kmjot6`+#aLsU}_P)R)wP(Tcwxd>3n?IBXC$)y05R8~@e0xF*fP|0N_
z0#u_?fJ!PWDL?_0PXwssvJwHRQ7J$rm6a5rfXXKVRB~B~0M)1zppwc;3Q$1h69Fo@
ztVDonR0>c@WhDhDpz?_Tm0VULKs71_sHC!z0u)gBM1V>zD-oa?l>$^!SxEs3sC*(o
zC6|>5P>o6fDygib00mS&5ulRGN(87zr2v&wR#JchDxU~Y$z>%1RHIUWN-8TUKmnCc
z1gPY)5&^1FDL^Hal@y?W$|nL;a#@K0)u<GplFCX7P(bAq0V=tyM1X2k3Q$R9B?Ty;
z@`(VITvj4LH7W(Dq_UC%6j1p@fJ!ba5uh5C0#s62NdXF|d?G+4mz4-mjY<J3sjQ>`
z1ynu}ppwf<1gJ)(0F_i$Qh)*~p9oONWhDYsqf&rMDk~{K0hLb#sN}K|0jg0cKqZxx
z6rh00CjwM*S&0DEs1%@*%1R1QK;;twD!HsgfNE3<P)TJa1t_5Mi2w!C0u)FOP(VFE
z0kr_tln78wNdc-UDL^$P0#s8XKs6-=sHUU<)szTOO^E>2loX(vk^)pyB0x1I0#s8{
zfNDw#P)&&d1yV;B0Sbg8#*;|_3W&p?ASu<{B0z!E+#*1M)Z9{l0;;*C00qQi$!rS|
zDmv~V|5-hvKctUF=qKB}_p{4U`j<+7+vyLzRjsXOk+|zQ`ts>3umu(u*7LMopsyl*
zmF)CXdA;7SI%9X8{avggj2$_N^p*0uUmF9VmPt-HiO)vF7%zp~5f7-%iQ~H*-raR3
z1#MvvHLNYHjN%y~tNzowSdXC94jaR+C#RjM&nCoJ6zL{#{}#9t<L`NTk{3FTXb2ch
zHl1t*<+}KYww--$nWR7L0t~;#c&1ga)vX7+y)jUEYVTFg^;O5}z_+fR)2ipVdJf~$
ztLM_gtLK^r74k0ElCa>_^X^1WQQp_e`&{1dEAO9B-tUzU`pi36vH8~S1Fd|(<%7QR
z!3pJqUio}~`PeDS=e6>AE}!o!pFg2|zE{4`Up{_{@&&DYfy)>A$`?*3U+9%D_Lm<y
zMfsvuzR2Z^edUWMlrQ$mm-@?(o}zq7D_`RBrM~i|6UvuRK1R2){_>tIuWlZ+NQz}P
zUVJm#(j$Dy<A<*6UU&VMv6D2r!?VUfcd~70cLUdD%IppgAOqdWwxQh(TxVG0r0Yx>
z!&AvXcd~70cLUc=+UyRGDg)igwxQh(TnA^&?(qCF(4A}>+TFl;J#BV}hnj)zWZTg0
z2F}wNvpYQL40I>k%<jDN+M@Pky^_Pr2E15sqgd4T)~?Fs_mZ>ua!I>d%h%Oe>fqu;
zHjVq$jrr31x5MA={bv4ycW=GF_rcD)w?xzOt1HXBvvu5Yiq<ptnQOUNwnio)EECUM
z$8FyG0fDTT_pA9;%P+6iEx%TI+w${oW$3G5iKy>a0=s!HMa-yQ&#%mb9%LKySZgk?
zmhDhbBhLDzP4<HU$w7{1{c1UrS;){Z<g1f-I_WnG??gJz;jZ|^&tx(;(xnj}Ll|p*
zXY+DQt=*GRGSKaYyZy~57K(Of_OO2kSqPoUcJuoKdsdc@+UZK1Y@M!<)!!9*7}|fj
zFzwg<Rr>@Rw!fOcmWp&ku@hR?9|x(!>sBbPhgLfjTaQ=VKODBLhdXF$hdccx*>Ha&
z4oYX{VSg(Q%g)!GvBNE=i!xt_!}fPx9hf;f?%~dT8snDO+rp?G{j~G?0FBQK9KIbV
z8V*bhc0#weL$B8IG5KzdI@}gJbjaatr@1iP+(2I#&))X%X|9a<YxRbE!ZZ&*TL${a
z<b^ke%uVP-`df+mUVkH|dFbexAHPZGG_HSqaCzXowvXuS#uZS>Hj)X)7Fcbd=g{;7
zUK=(NWJc`XcrSe(7f-R8Z{;(KqIbp9g+%9+m!>^@-S;l3%p1bXCgtTOUsBQ=p|=vw
z-|ml#2>$2e)1@!G*Lv<gcP$(37H%x~e1qN3-D5nC7ePY&_fy=U<bHa#?RM$emOc@|
zd-%y2(8kNdzwizr{%q`dg13q<<en#2`Nbnw>v7pKv|YG2K7{F2njeEj`LdPYV`gDd
z-ruNQyRpgFYY7>{d}Es*nIEz6KF<5$CM5NxR%#?>VZhkF(n{GeC*t7s6VU6lhpKx&
ze>ru{gVzezd>J_7>-owCj^84&@tRPt;>Eo<-9EiSY?I%qU6n5*6ZK3+zKn#6AJsE&
WN`OhW1VXUB5Qrn*SX3KJwDErfCPZog
literal 0
HcmV?d00001
diff --git a/tests/data/acpi/q35/DSDT.numamem b/tests/data/acpi/q35/DSDT.numamem
index 44ec1b0af400da6d298284aa959aa38add7e6dd5..99c34bd71abf680261dd5047f63b91641d0ba02d 100644
GIT binary patch
delta 121
zcmexwbIX>?CD<k8mK*~Eqx(dzR3<lpiG?epqnkWgU4k5)IpRG$U3dfh0t}oD3>i3r
zoI>3MjkyfEv>6z97#R>?1Bm(m|Ns36Auvk-L{>70aK}3b1#>I`>EA5Hm?{eZ<nbAI
delta 29
lcmca*``?DkCD<jTUXFo*alu5cR3>k(iG?dSmolcx0sxcs33mVh
diff --git a/tests/data/acpi/q35/DSDT.tis b/tests/data/acpi/q35/DSDT.tis
index 30da3ec27958881801dacc954a343321ba26a2ae..f2461ecd90da760852806aadf2fe9e49940707c9 100644
GIT binary patch
delta 121
zcmccaxXFpjCD<jzNRfeoF>fMQDwCVQ#KM))(M_JLE<ujY9Pu8WF1!JL0S3+nh724*
zPN8ms##{zn+6)Xlj0^~{0mS_O|Nnl35SS$ZA}bk0xZ|CJf;pCe^lz48+$#?NjN=&%
delta 29
lcmdnwbls85CD<k8x&i|O<B5q}sZ8En6AM>vE@j*+4*-@^3Jm}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] 21+ messages in thread