11:03 a.m.
Signed-off-by: Gleb Natapov <gleb(a)redhat.com>
---
tools/acpi_extract.py | 6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/tools/acpi_extract.py b/tools/acpi_extract.py
index 5f613e4..8038269 100755
--- a/tools/acpi_extract.py
+++ b/tools/acpi_extract.py
@@ -121,7 +121,11 @@ def aml_name_string(offset):
if (aml[offset] != 0x08):
die( "Name offset 0x%x: expected 0x08 actual 0x%x" %
(offset, aml[offset]));
- return offset + 1;
+ offset += 1
+ # Block Name Modifier. Skip it.
+ if (aml[offset] == 0x5c or aml[offset] == 0x5e):
+ offset += 1
+ return offset;
# Given data offset, find dword const offset
def aml_data_dword_const(offset):
--
1.7.7.3
11:03 a.m.
New subject: [PATCH 2/3] Add ACPI_EXTRACT_PKG_START macro parsing
It allows to extract the beginning of a Package object content.
Signed-off-by: Gleb Natapov <gleb(a)redhat.com>
---
tools/acpi_extract.py | 12 ++++++++++++
1 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/tools/acpi_extract.py b/tools/acpi_extract.py
index 8038269..167a322 100755
--- a/tools/acpi_extract.py
+++ b/tools/acpi_extract.py
@@ -29,6 +29,7 @@
# ACPI_EXTRACT_PROCESSOR_START - start of Processor() block
# ACPI_EXTRACT_PROCESSOR_STRING - extract a NameString from Processor()
# ACPI_EXTRACT_PROCESSOR_END - offset at last byte of Processor() + 1
+# ACPI_EXTRACT_PKG_START - start of Package block
#
# ACPI_EXTRACT_ALL_CODE - create an array storing the generated AML bytecode
#
@@ -185,6 +186,15 @@ def aml_processor_end(offset):
pkglen = aml_pkglen(offset)
return offset + pkglen
+def aml_package_start(offset):
+ offset = aml_name_string(offset) + 4
+ # 0x12 PkgLength NumElements PackageElementList
+ if (aml[offset] != 0x12):
+ die( "Name offset 0x%x: expected 0x12 actual 0x%x" %
+ (offset, aml[offset]));
+ offset += 1
+ return offset + aml_pkglen_bytes(offset) + 1
+
lineno = 0
for line in fileinput.input():
# Strip trailing newline
@@ -267,6 +277,8 @@ for i in range(len(asl)):
offset = aml_processor_string(offset)
elif (directive == "ACPI_EXTRACT_PROCESSOR_END"):
offset = aml_processor_end(offset)
+ elif (directive == "ACPI_EXTRACT_PKG_START"):
+ offset = aml_package_start(offset)
else:
die("Unsupported directive %s" % directive)
--
1.7.7.3
11:03 a.m.
New subject: [PATCH 3/3] Get system state configuration from QEMU and patch DSDT with it.
QEMU may want to disable guest's S3/S4 support and it wants to distinguish
between regular powerdown and S4 powerdown. To support that new fw_cfg
option was added that passes supported system states and what value should
guest use to enter each state. States are passed in 6 byte array. Each
byte represents one system state. If byte at offset X has its MSB set
it means that system state X is supported and to enter it guest should
use the value from lowest 7 bits. Patch also detects old QEMU and uses
values that work in backwards compatible way there.
Signed-off-by: Gleb Natapov <gleb(a)redhat.com>
---
src/acpi-dsdt.dsl | 32 ---------
src/acpi-dsdt.hex | 42 +-----------
src/acpi.c | 15 ++++
src/ssdt-pcihp.dsl | 36 ++++++++++
src/ssdt-pcihp.hex | 185 +++++++++++++++++++++++++++++++++-------------------
5 files changed, 172 insertions(+), 138 deletions(-)
diff --git a/src/acpi-dsdt.dsl b/src/acpi-dsdt.dsl
index 4bdc268..37899fc 100644
--- a/src/acpi-dsdt.dsl
+++ b/src/acpi-dsdt.dsl
@@ -604,38 +604,6 @@ DefinitionBlock (
}
}
-
-/****************************************************************
- * Suspend
- ****************************************************************/
-
- /*
- * S3 (suspend-to-ram), S4 (suspend-to-disk) and S5 (power-off) type codes:
- * must match piix4 emulation.
- */
- Name (\_S3, Package (0x04)
- {
- 0x01, /* PM1a_CNT.SLP_TYP */
- 0x01, /* PM1b_CNT.SLP_TYP */
- Zero, /* reserved */
- Zero /* reserved */
- })
- Name (\_S4, Package (0x04)
- {
- Zero, /* PM1a_CNT.SLP_TYP */
- Zero, /* PM1b_CNT.SLP_TYP */
- Zero, /* reserved */
- Zero /* reserved */
- })
- Name (\_S5, Package (0x04)
- {
- Zero, /* PM1a_CNT.SLP_TYP */
- Zero, /* PM1b_CNT.SLP_TYP */
- Zero, /* reserved */
- Zero /* reserved */
- })
-
-
/****************************************************************
* CPU hotplug
****************************************************************/
diff --git a/src/acpi-dsdt.hex b/src/acpi-dsdt.hex
index a4af597..8678fbf 100644
--- a/src/acpi-dsdt.hex
+++ b/src/acpi-dsdt.hex
@@ -3,12 +3,12 @@ static unsigned char AmlCode[] = {
0x53,
0x44,
0x54,
-0x21,
-0x11,
+0xfd,
+0x10,
0x0,
0x0,
0x1,
-0xe8,
+0x4a,
0x42,
0x58,
0x50,
@@ -3925,42 +3925,6 @@ static unsigned char AmlCode[] = {
0x52,
0x51,
0x30,
-0x8,
-0x5f,
-0x53,
-0x33,
-0x5f,
-0x12,
-0x6,
-0x4,
-0x1,
-0x1,
-0x0,
-0x0,
-0x8,
-0x5f,
-0x53,
-0x34,
-0x5f,
-0x12,
-0x6,
-0x4,
-0x0,
-0x0,
-0x0,
-0x0,
-0x8,
-0x5f,
-0x53,
-0x35,
-0x5f,
-0x12,
-0x6,
-0x4,
-0x0,
-0x0,
-0x0,
-0x0,
0x10,
0x49,
0xe,
diff --git a/src/acpi.c b/src/acpi.c
index 30888b9..06ffe0a 100644
--- a/src/acpi.c
+++ b/src/acpi.c
@@ -492,6 +492,8 @@ extern void link_time_assertion(void);
static void* build_pcihp(void)
{
+ char *sys_states;
+ int sys_state_size;
u32 rmvc_pcrm;
int i;
@@ -523,6 +525,19 @@ static void* build_pcihp(void)
}
}
+ sys_states = romfile_loadfile("etc/system-states", &sys_state_size);
+ if (!sys_states || sys_state_size != 6)
+ sys_states = (char[]){128, 0, 0, 129, 128, 128};
+
+ if (!(sys_states[3] & 128))
+ ssdt[acpi_s3_name[0]] = 'X';
+ if (!(sys_states[4] & 128))
+ ssdt[acpi_s4_name[0]] = 'X';
+ else
+ ssdt[acpi_s4_pkg[0] + 1] = ssdt[acpi_s4_pkg[0] + 3] = sys_states[4] & 127;
+ ((struct acpi_table_header*)ssdt)->checksum = 0;
+ ((struct acpi_table_header*)ssdt)->checksum -= checksum(ssdt,
sizeof(ssdp_pcihp_aml));
+
return ssdt;
}
diff --git a/src/ssdt-pcihp.dsl b/src/ssdt-pcihp.dsl
index 4b435b8..12555e2 100644
--- a/src/ssdt-pcihp.dsl
+++ b/src/ssdt-pcihp.dsl
@@ -95,4 +95,40 @@ DefinitionBlock ("ssdt-pcihp.aml", "SSDT", 0x01,
"BXPC", "BXSSDTPCIHP", 0x1)
gen_pci_hotplug(1f)
}
}
+
+ Scope(\) {
+/****************************************************************
+ * Suspend
+ ****************************************************************/
+
+ /*
+ * S3 (suspend-to-ram), S4 (suspend-to-disk) and S5 (power-off) type codes:
+ * must match piix4 emulation.
+ */
+
+ ACPI_EXTRACT_NAME_STRING acpi_s3_name
+ Name (_S3, Package (0x04)
+ {
+ One, /* PM1a_CNT.SLP_TYP */
+ One, /* PM1b_CNT.SLP_TYP */
+ Zero, /* reserved */
+ Zero /* reserved */
+ })
+ ACPI_EXTRACT_NAME_STRING acpi_s4_name
+ ACPI_EXTRACT_PKG_START acpi_s4_pkg
+ Name (_S4, Package (0x04)
+ {
+ 0x2, /* PM1a_CNT.SLP_TYP */
+ 0x2, /* PM1b_CNT.SLP_TYP */
+ Zero, /* reserved */
+ Zero /* reserved */
+ })
+ Name (_S5, Package (0x04)
+ {
+ Zero, /* PM1a_CNT.SLP_TYP */
+ Zero, /* PM1b_CNT.SLP_TYP */
+ Zero, /* reserved */
+ Zero /* reserved */
+ })
+ }
}
diff --git a/src/ssdt-pcihp.hex b/src/ssdt-pcihp.hex
index b15ad5a..f3bb3c6 100644
--- a/src/ssdt-pcihp.hex
+++ b/src/ssdt-pcihp.hex
@@ -1,80 +1,20 @@
-static unsigned short aml_adr_dword[] = {
-0x3e,
-0x62,
-0x88,
-0xae,
-0xd4,
-0xfa,
-0x120,
-0x146,
-0x16c,
-0x192,
-0x1b8,
-0x1de,
-0x204,
-0x22a,
-0x250,
-0x276,
-0x29c,
-0x2c2,
-0x2e8,
-0x30e,
-0x334,
-0x35a,
-0x380,
-0x3a6,
-0x3cc,
-0x3f2,
-0x418,
-0x43e,
-0x464,
-0x48a,
-0x4b0
+static unsigned short acpi_s4_pkg[] = {
+0x65c
};
-static unsigned short aml_ej0_name[] = {
-0x44,
-0x68,
-0x8e,
-0xb4,
-0xda,
-0x100,
-0x126,
-0x14c,
-0x172,
-0x198,
-0x1be,
-0x1e4,
-0x20a,
-0x230,
-0x256,
-0x27c,
-0x2a2,
-0x2c8,
-0x2ee,
-0x314,
-0x33a,
-0x360,
-0x386,
-0x3ac,
-0x3d2,
-0x3f8,
-0x41e,
-0x444,
-0x46a,
-0x490,
-0x4b6
+static unsigned short acpi_s3_name[] = {
+0x649
};
static unsigned char ssdp_pcihp_aml[] = {
0x53,
0x53,
0x44,
0x54,
-0x44,
+0x6e,
0x6,
0x0,
0x0,
0x1,
-0x94,
+0x7e,
0x42,
0x58,
0x50,
@@ -1668,5 +1608,116 @@ static unsigned char ssdp_pcihp_aml[] = {
0x31,
0x46,
0x5f,
-0x69
+0x69,
+0x10,
+0x29,
+0x5c,
+0x0,
+0x8,
+0x5f,
+0x53,
+0x33,
+0x5f,
+0x12,
+0x6,
+0x4,
+0x1,
+0x1,
+0x0,
+0x0,
+0x8,
+0x5f,
+0x53,
+0x34,
+0x5f,
+0x12,
+0x8,
+0x4,
+0xa,
+0x2,
+0xa,
+0x2,
+0x0,
+0x0,
+0x8,
+0x5f,
+0x53,
+0x35,
+0x5f,
+0x12,
+0x6,
+0x4,
+0x0,
+0x0,
+0x0,
+0x0
+};
+static unsigned short aml_adr_dword[] = {
+0x3e,
+0x62,
+0x88,
+0xae,
+0xd4,
+0xfa,
+0x120,
+0x146,
+0x16c,
+0x192,
+0x1b8,
+0x1de,
+0x204,
+0x22a,
+0x250,
+0x276,
+0x29c,
+0x2c2,
+0x2e8,
+0x30e,
+0x334,
+0x35a,
+0x380,
+0x3a6,
+0x3cc,
+0x3f2,
+0x418,
+0x43e,
+0x464,
+0x48a,
+0x4b0
+};
+static unsigned short acpi_s4_name[] = {
+0x655
+};
+static unsigned short aml_ej0_name[] = {
+0x44,
+0x68,
+0x8e,
+0xb4,
+0xda,
+0x100,
+0x126,
+0x14c,
+0x172,
+0x198,
+0x1be,
+0x1e4,
+0x20a,
+0x230,
+0x256,
+0x27c,
+0x2a2,
+0x2c8,
+0x2ee,
+0x314,
+0x33a,
+0x360,
+0x386,
+0x3ac,
+0x3d2,
+0x3f8,
+0x41e,
+0x444,
+0x46a,
+0x490,
+0x4b6
};
--
1.7.7.3
1:44 p.m.
New subject: [Qemu-devel] [PATCH 3/3] Get system state configuration from QEMU and patch DSDT with it.
On 05/20/2012 12:03 PM, Gleb Natapov wrote:
QEMU may want to disable guest's S3/S4 support and
it wants to distinguish
between regular powerdown and S4 powerdown. To support that new fw_cfg
option was added that passes supported system states and what value should
guest use to enter each state. States are passed in 6 byte array. Each
byte represents one system state. If byte at offset X has its MSB set
it means that system state X is supported and to enter it guest should
use the value from lowest 7 bits. Patch also detects old QEMU and uses
values that work in backwards compatible way there.
Do we actually have to patch the DSDT? Or can _S3 etc be made into
functions instead? (and talk to the bios, or even to fwcfg directly?)
--
error compiling committee.c: too many arguments to function
2:15 p.m.
New subject: [Qemu-devel] [PATCH 3/3] Get system state configuration from QEMU and patch DSDT with it.
On Sun, May 20, 2012 at 02:44:51PM +0300, Avi Kivity wrote:
On 05/20/2012 12:03 PM, Gleb Natapov wrote:
We better not talk to fwcfg after OSPM is started since this is firmware
confing interface. Regardless, presence of _S3 name or method is all
that needed for OS enabling S3 option. If _S3 is defined as a method it
has to return Package() otherwise iasl refuses to compile it.
--
Gleb.
QEMU may want to disable guest's S3/S4
support and it wants to distinguish
between regular powerdown and S4 powerdown. To support that new fw_cfg
option was added that passes supported system states and what value should
guest use to enter each state. States are passed in 6 byte array. Each
byte represents one system state. If byte at offset X has its MSB set
it means that system state X is supported and to enter it guest should
use the value from lowest 7 bits. Patch also detects old QEMU and uses
values that work in backwards compatible way there.
Do we actually have to patch the DSDT? Or can _S3 etc be made into
functions instead? (and talk to the bios, or even to fwcfg directly?)
2:30 p.m.
New subject: [Qemu-devel] [PATCH 3/3] Get system state configuration from QEMU and patch DSDT with it.
On 05/20/2012 03:15 PM, Gleb Natapov wrote:
On Sun, May 20, 2012 at 02:44:51PM +0300, Avi Kivity
wrote:
Why not? The OS isn't going to talk to it, so we can have a driver in ACPI.
On 05/20/2012 12:03 PM, Gleb Natapov wrote:
We better not talk to fwcfg after OSPM is started since this is firmware
confing interface. QEMU may want to disable guest's S3/S4
support and it wants to distinguish
between regular powerdown and S4 powerdown. To support that new fw_cfg
option was added that passes supported system states and what value should
guest use to enter each state. States are passed in 6 byte array. Each
byte represents one system state. If byte at offset X has its MSB set
it means that system state X is supported and to enter it guest should
use the value from lowest 7 bits. Patch also detects old QEMU and uses
values that work in backwards compatible way there.
Do we actually have to patch the DSDT? Or can _S3 etc be made into
functions instead? (and talk to the bios, or even to fwcfg directly?)
Regardless, presence of _S3 name or method is all
that needed for OS enabling S3 option. If _S3 is defined as a method it
has to return Package() otherwise iasl refuses to compile it.
Can't we Return (Package (...) { ... }) or equivalent?
--
error compiling committee.c: too many arguments to function
2:36 p.m.
New subject: [Qemu-devel] [PATCH 3/3] Get system state configuration from QEMU and patch DSDT with it.
On Sun, May 20, 2012 at 03:30:50PM +0300, Avi Kivity wrote:
We can, how does it help?
--
Gleb.
On 05/20/2012 03:15 PM, Gleb Natapov wrote:
The OS is going to talk to it since the OS is the one who interprets
AML. We may want to disable fwcfg after OS bootup at all in the feature.
Who knows what kind of sensitive information we may want to pass by it
in the feature? May be something TPM related? And I do not see any advantage
of using fwcfg from AML.
On Sun, May 20, 2012 at 02:44:51PM +0300, Avi
Kivity wrote:
Why not? The OS isn't going to talk to it, so we can have a driver in ACPI.
On 05/20/2012 12:03 PM, Gleb Natapov wrote:
We better not talk to fwcfg after OSPM is started since this is firmware
confing interface. QEMU may want to disable guest's S3/S4
support and it wants to distinguish
between regular powerdown and S4 powerdown. To support that new fw_cfg
option was added that passes supported system states and what value should
guest use to enter each state. States are passed in 6 byte array. Each
byte represents one system state. If byte at offset X has its MSB set
it means that system state X is supported and to enter it guest should
use the value from lowest 7 bits. Patch also detects old QEMU and uses
values that work in backwards compatible way there.
Do we actually have to patch the DSDT? Or can _S3 etc be made into
functions instead? (and talk to the bios, or even to fwcfg directly?)
Regardless,
presence of _S3 name or method is all
that needed for OS enabling S3 option. If _S3 is defined as a method it
has to return Package() otherwise iasl refuses to compile it.
Can't we Return (Package (...) { ... }) or equivalent?
2:47 p.m.
New subject: [Qemu-devel] [PATCH 3/3] Get system state configuration from QEMU and patch DSDT with it.
On 05/20/2012 03:36 PM, Gleb Natapov wrote:
We can, how does it help?
The contents of the package can be determined at runtime.
--
error compiling committee.c: too many arguments to function
On Sun, May 20, 2012 at 03:30:50PM +0300, Avi Kivity
wrote:
I meant, not directly. So the driver in ACPI has exclusive access.
On 05/20/2012 03:15 PM, Gleb Natapov wrote:
The OS is going to talk to it since the OS is the one who interprets
AML. On Sun, May 20, 2012 at 02:44:51PM +0300, Avi
Kivity wrote:
Why not? The OS isn't going to talk to it, so we can have a driver in ACPI.
On 05/20/2012 12:03 PM, Gleb Natapov wrote:
> QEMU may want to disable guest's S3/S4 support and it wants to distinguish
> between regular powerdown and S4 powerdown. To support that new fw_cfg
> option was added that passes supported system states and what value should
> guest use to enter each state. States are passed in 6 byte array. Each
> byte represents one system state. If byte at offset X has its MSB set
> it means that system state X is supported and to enter it guest should
> use the value from lowest 7 bits. Patch also detects old QEMU and uses
> values that work in backwards compatible way there.
>
Do we actually have to patch the DSDT? Or can _S3 etc be made into
functions instead? (and talk to the bios, or even to fwcfg directly?)
We better not talk to fwcfg after OSPM is started since this is firmware
confing interface. We may want to disable fwcfg after OS bootup at all in
the feature.
Who knows what kind of sensitive information we may want to pass by it
in the feature? May be something TPM related?
fwcfg is for passing information to the guest. If you want to hide
something from the guest, just don't put it in fwcfg.
And I do not see any advantage
of using fwcfg from AML.
It's an alternative to patching AML. Sure it takes some effort to write
the driver, but afterwards we can modify the guest behaviour more
easily. One possible client is -M old, so you can revert to previous
behaviour depending on fwcfg data.
(we don't need a driver in AML to avoid patching, we can have AML talk
to the bios and the bios drive fwcfg; but I think we'll find uses for a
driver).
Regardless, presence of _S3 name or method is all
that needed for OS enabling S3 option. If _S3 is defined as a method it
has to return Package() otherwise iasl refuses to compile it.
Can't we Return (Package (...) { ... }) or equivalent?
2:59 p.m.
New subject: [Qemu-devel] [PATCH 3/3] Get system state configuration from QEMU and patch DSDT with it.
On Sun, May 20, 2012 at 03:47:02PM +0300, Avi Kivity wrote:
Where to put it if we want to pass it to a firmware, but not an OS.
That was the point of fwcfg. If you want to pass something to a guest OS
use virtio-serial.
-M old is easy to support with the current
patch. You just set new
properties to compatibility values. The code is written with this in
mind. And this is not an alternative to patching AML as I am trying to
explain to you below. You can eliminate patching of s4 value, but that's
it, you still need to patch out _S3/_S4 names.
We can, how does it help?
The contents of the package can be determined at runtime.
And? _S3 name should not exists at all in order to disable S3, not return
something different.
--
Gleb.
On 05/20/2012 03:36 PM, Gleb Natapov wrote:
What's the difference?
On Sun, May 20, 2012 at 03:30:50PM +0300, Avi
Kivity wrote:
I meant, not directly. So the driver in ACPI has exclusive access.
On 05/20/2012 03:15 PM, Gleb Natapov wrote:
The OS is going to talk to it since the OS is the one who interprets
AML. On Sun, May 20, 2012 at 02:44:51PM +0300, Avi
Kivity wrote:
> On 05/20/2012 12:03 PM, Gleb Natapov wrote:
> > QEMU may want to disable guest's S3/S4 support and it wants to distinguish
> > between regular powerdown and S4 powerdown. To support that new fw_cfg
> > option was added that passes supported system states and what value should
> > guest use to enter each state. States are passed in 6 byte array. Each
> > byte represents one system state. If byte at offset X has its MSB set
> > it means that system state X is supported and to enter it guest should
> > use the value from lowest 7 bits. Patch also detects old QEMU and uses
> > values that work in backwards compatible way there.
> >
>
>
> Do we actually have to patch the DSDT? Or can _S3 etc be made into
> functions instead? (and talk to the bios, or even to fwcfg directly?)
>
We better not talk to fwcfg after OSPM is started since this is firmware
confing interface.
Why not? The OS isn't going to talk to it, so we can have a driver in ACPI.
We may want to
disable fwcfg after OS bootup at all in the feature.
Who knows what kind of sensitive information we may want to pass by it
in the feature? May be something TPM related?
fwcfg is for passing information to the guest. If you want to hide
something from the guest, just don't put it in fwcfg.
And I do not
see any advantage
of using fwcfg from AML.
It's an alternative to patching AML. Sure it takes some effort to write
the driver, but afterwards we can modify the guest behaviour more
easily. One possible client is -M old, so you can revert to previous
behaviour depending on fwcfg data.
(we don't need a driver in AML to avoid patching, we can have AML talk
to the bios and the bios drive fwcfg; but I think we'll find uses for a
driver).
I am not sure what you mean. AML can't talk to the bios. It can read
values that bios put somewhere. I do not see advantage of this method
and it requires patching still.
Regardless, presence of _S3 name or method is all
that needed for OS enabling S3 option. If _S3 is defined as a method it
has to return Package() otherwise iasl refuses to compile it.
Can't we Return (Package (...) { ... }) or equivalent?
3:39 p.m.
New subject: [Qemu-devel] [PATCH 3/3] Get system state configuration from QEMU and patch DSDT with it.
On 05/20/2012 03:59 PM, Gleb Natapov wrote:
I meant, not directly. So the driver in ACPI has exclusive access.
What's the difference?
ACPI is firmware, not OS.
Where to put it if we want to pass it to a firmware, but not an OS.
That was the point of fwcfg. If you want to pass something to a guest OS
use virtio-serial.
See above.
-M old is easy to support with the current
patch. You just set new
properties to compatibility values. The code is written with this in
mind. And this is not an alternative to patching AML as I am trying to
explain to you below. You can eliminate patching of s4 value, but that's
it, you still need to patch out _S3/_S4 names.
What about
If (Fcfg(...)) {
Method()...
}
?
(i.e.. define the method conditionally at runtime)
That's what I meant - communicate through memory.
The contents of the package can be determined at runtime.
And? _S3 name should not exists at all in order to disable S3, not return
something different.
See above.
--
error compiling committee.c: too many arguments to function
> >
> > Do we actually have to patch the DSDT? Or can _S3 etc be made into
> > functions instead? (and talk to the bios, or even to fwcfg directly?)
> >
> We better not talk to fwcfg after OSPM is started since this is firmware
> confing interface.
Why not? The OS isn't going to talk to it, so we can have a driver in ACPI.
The OS is going to talk to it since the OS is the one who interprets
AML. We may want to disable fwcfg after OS bootup at all in
the feature.
Who knows what kind of sensitive information we may want to pass by it
in the feature? May be something TPM related?
fwcfg is for passing information to the guest. If you want to hide
something from the guest, just don't put it in fwcfg.
And I do not see any advantage
of using fwcfg from AML.
It's an alternative to patching AML. Sure it takes some effort to write
the driver, but afterwards we can modify the guest behaviour more
easily. One possible client is -M old, so you can revert to previous
behaviour depending on fwcfg data.
(we don't need a driver in AML to avoid patching, we can have AML talk
to the bios and the bios drive fwcfg; but I think we'll find uses for a
driver).
I am not sure what you mean. AML can't talk to the bios. It can read
values that bios put somewhere. I do not see advantage of this method
and it requires patching still.
For the existence of the names? Yes, if we can't avoid it it's a
problem. But if we can avoid patching, we should.
> Regardless, presence of _S3 name or method
is all
> that needed for OS enabling S3 option. If _S3 is defined as a method it
> has to return Package() otherwise iasl refuses to compile it.
Can't we Return (Package (...) { ... }) or equivalent?
We can, how does it help?
3:57 p.m.
New subject: [Qemu-devel] [PATCH 3/3] Get system state configuration from QEMU and patch DSDT with it.
On Sun, May 20, 2012 at 04:39:01PM +0300, Avi Kivity wrote:
Where to put it if we want to pass it to a firmware, but not an OS.
That was the point of fwcfg. If you want to pass something to a guest OS
use virtio-serial.
See above.
-M old is easy to support with the current
patch. You just set new
properties to compatibility values. The code is written with this in
mind. And this is not an alternative to patching AML as I am trying to
explain to you below. You can eliminate patching of s4 value, but that's
it, you still need to patch out _S3/_S4 names.
What about
If (Fcfg(...)) {
Method()...
}
? syntax error, unexpected PARSEOP_IF
If we can, we should, but we can't as far as I see. The patching was here long
before
this patch.
And? _S3 name should not exists at all in order to disable S3, not return
something different.
See above.
Does not work for me, can you send me a patch that works?
--
Gleb.
On 05/20/2012 03:59 PM, Gleb Natapov wrote:
What's the difference?
ACPI is firmware, not OS.
AML is a data provided by firmware. AML's runtime is
different from firmware's.
> > >
> > > Do we actually have to patch the DSDT? Or can _S3 etc be made into
> > > functions instead? (and talk to the bios, or even to fwcfg directly?)
> > >
> > We better not talk to fwcfg after OSPM is started since this is firmware
> > confing interface.
>
> Why not? The OS isn't going to talk to it, so we can have a driver in ACPI.
>
The OS is going to talk to it since the OS is the one who interprets
AML.
I meant, not directly. So the driver in ACPI has exclusive access.
We may want to disable fwcfg after OS bootup at all in
the feature.
Who knows what kind of sensitive information we may want to pass by it
in the feature? May be something TPM related?
fwcfg is for passing information to the guest. If you want to hide
something from the guest, just don't put it in fwcfg.
And I do not see any advantage
of using fwcfg from AML.
It's an alternative to patching AML. Sure it takes some effort to write
the driver, but afterwards we can modify the guest behaviour more
easily. One possible client is -M old, so you can revert to previous
behaviour depending on fwcfg data.
(i.e.. define the method conditionally at runtime)
That's what I meant - communicate through memory.
What's the benefit? The patching is still needed. You need to pass
address of OperationRegion() to AML. You can do it either by patching or
by creating OperationRegion() code dynamically.
(we don't need a driver in AML to avoid patching, we can have AML talk
to the bios and the bios drive fwcfg; but I think we'll find uses for a
driver).
I am not sure what you mean. AML can't talk to the bios. It can read
values that bios put somewhere. I do not see
advantage of this method
and it requires patching still.
For the existence of the names? Yes, if we can't avoid it it's a
problem. But if we can avoid patching, we should.
> > Regardless, presence of _S3 name or method is all
> > that needed for OS enabling S3 option. If _S3 is defined as a method it
> > has to return Package() otherwise iasl refuses to compile it.
>
> Can't we Return (Package (...) { ... }) or equivalent?
>
We can, how does it help?
The contents of the package can be determined at runtime.
4:34 p.m.
New subject: [Qemu-devel] [PATCH 3/3] Get system state configuration from QEMU and patch DSDT with it.
On 05/20/2012 04:57 PM, Gleb Natapov wrote:
What about
If (Fcfg(...)) {
Method()...
}
? syntax error, unexpected PARSEOP_IF
Unfortunately the ACPI spec forbids this construct, so either patching
or double complication is necessary.
Or it can be a fixed address in low memory, or a scratch register in
hardware.
If we can, we should, but we can't as far as I see. The patching was here
long before
this patch.
I agree we probably can't.
--
error compiling committee.c: too many arguments to function
On Sun, May 20, 2012 at 04:39:01PM +0300, Avi Kivity
wrote:
It's still firmware.
On 05/20/2012 03:59 PM, Gleb Natapov wrote:
ACPI is firmware, not OS.
AML is a data provided by firmware. AML's runtime is
different from firmware's. > >
> >
> > > > Do we actually have to patch the DSDT? Or can _S3 etc be made into
> > > > functions instead? (and talk to the bios, or even to fwcfg
directly?)
> > > >
> > > We better not talk to fwcfg after OSPM is started since this is firmware
> > > confing interface.
> >
> > Why not? The OS isn't going to talk to it, so we can have a driver in
ACPI.
> >
> The OS is going to talk to it since the OS is the one who interprets
> AML.
I meant, not directly. So the driver in ACPI has exclusive access.
What's the difference?
It's an alternative to patching AML. Sure it takes some effort to write
the driver, but afterwards we can modify the guest behaviour more
easily. One possible client is -M old, so you can revert to previous
behaviour depending on fwcfg data.
-M old is easy to support with the current
patch. You just set new
properties to compatibility values. The code is written with this in
mind. And this is not an alternative to patching AML as I am trying to
explain to you below. You can eliminate patching of s4 value, but that's
it, you still need to patch out _S3/_S4 names.
(i.e.. define the method conditionally at runtime)
That's what I meant - communicate through memory.
What's the benefit? The patching is still needed. You need to pass
address of OperationRegion() to AML. You can do it either by patching or
by creating OperationRegion() code dynamically.
(we don't need a driver in AML to avoid patching, we can have AML talk
to the bios and the bios drive fwcfg; but I think we'll find uses for a
driver).
I am not sure what you mean. AML can't talk to the bios. It can read
values that bios put somewhere. I do not
see advantage of this method
and it requires patching still.
For the existence of the names? Yes, if we can't avoid it it's a
problem. But if we can avoid patching, we should.
4:43 p.m.
New subject: [Qemu-devel] [PATCH 3/3] Get system state configuration from QEMU and patch DSDT with it.
On Sun, May 20, 2012 at 05:34:56PM +0300, Avi Kivity wrote:
syntax error, unexpected PARSEOP_IF
Unfortunately the ACPI spec forbids this construct, so either patching
or double complication is necessary.
It's not double if we will take all possible combinations into account.
Or it can be a fixed address in low memory, or a scratch register in
hardware.
Both will work (fixed addresses are better be avoided and who needs
another PV device), but I do not see how either of them is better then
patching. What is your concern?
If we can, we should, but we can't as far as I see. The patching was here
long before
this patch.
I agree we probably can't.
--
error compiling committee.c: too many arguments to function
--
Gleb.
On 05/20/2012 04:57 PM, Gleb Natapov wrote:
We have to agree to disagree here :) It's just a data for OS to use as
far as I am concern.
On Sun, May 20, 2012 at 04:39:01PM +0300, Avi
Kivity wrote:
It's still firmware.
On 05/20/2012 03:59 PM, Gleb Natapov wrote:
AML is a data provided by firmware. AML's runtime is
different from firmware's. > > > > >
> > > > > Do we actually have to patch the DSDT? Or can _S3 etc be made
into
> > > > > functions instead? (and talk to the bios, or even to fwcfg
directly?)
> > > > >
> > > > We better not talk to fwcfg after OSPM is started since this is
firmware
> > > > confing interface.
> > >
> > > Why not? The OS isn't going to talk to it, so we can have a driver in
ACPI.
> > >
> > The OS is going to talk to it since the OS is the one who interprets
> > AML.
>
> I meant, not directly. So the driver in ACPI has exclusive access.
>
What's the difference?
ACPI is firmware, not OS. >
> It's an alternative to patching AML. Sure it takes some effort to write
> the driver, but afterwards we can modify the guest behaviour more
> easily. One possible client is -M old, so you can revert to previous
> behaviour depending on fwcfg data.
-M old is easy to support with the current patch. You just set new
properties to compatibility values. The code is written with this in
mind. And this is not an alternative to patching AML as I am trying to
explain to you below. You can eliminate patching of s4 value, but that's
it, you still need to patch out _S3/_S4 names.
What about
If (Fcfg(...)) {
Method()...
}
?
(i.e.. define the method conditionally at runtime)
What's the benefit? The patching is still needed. You need to pass
address of OperationRegion() to AML. You can do it either by patching or
by creating OperationRegion() code dynamically.
>
> (we don't need a driver in AML to avoid patching, we can have AML talk
> to the bios and the bios drive fwcfg; but I think we'll find uses for a
> driver).
I am not sure what you mean. AML can't talk to the bios. It can read
values that bios put somewhere.
That's what I meant - communicate through memory.
I do not
see advantage of this method
and it requires patching still.
For the existence of the names? Yes, if we can't avoid it it's a
problem. But if we can avoid patching, we should.
4:46 p.m.
New subject: [Qemu-devel] [PATCH 3/3] Get system state configuration from QEMU and patch DSDT with it.
On 05/20/2012 05:43 PM, Gleb Natapov wrote:
Patching is harder to maintain. Unfortunately it's unavoidable.
--
error compiling committee.c: too many arguments to function
Or it can be a fixed address in low memory, or a scratch register in
hardware.
Both will work (fixed addresses are better be avoided and who needs
another PV device), but I do not see how either of them is better then
patching. What is your concern?
5:15 p.m.
New subject: [Qemu-devel] [PATCH 3/3] Get system state configuration from QEMU and patch DSDT with it.
On Sun, May 20, 2012 at 05:46:46PM +0300, Avi Kivity wrote:
On 05/20/2012 05:43 PM, Gleb Natapov wrote:
Patching is harder to maintain. Unfortunately it's unavoidable.
Here we in agreement, and I was against patching till it was unavoidable,
but than pci hotplug started using it, and afterwards processor
definitions, so no point in avoiding it now by using inferior methods.
--
Gleb.
Or it can be a fixed address in low memory, or a scratch register in
hardware.
Both will work (fixed addresses are better be avoided and who needs
another PV device), but I do not see how either of them is better then
patching. What is your concern?
6:16 p.m.
New subject: [Qemu-devel] [PATCH 3/3] Get system state configuration from QEMU and patch DSDT with it.
On Sun, May 20, 2012 at 06:15:44PM +0300, Gleb Natapov wrote:
On Sun, May 20, 2012 at 05:46:46PM +0300, Avi Kivity
wrote:
I agree as well.
What's the background to needing to have dynamic S3/S4 definitions?
(Why will some qemu instances be able to sleep and not others?)
-Kevin
On 05/20/2012 05:43 PM, Gleb Natapov wrote:
Patching is harder to maintain. Unfortunately it's unavoidable.
Here we in agreement, and I was against patching till it was unavoidable,
but than pci hotplug started using it, and afterwards processor
definitions, so no point in avoiding it now by using inferior methods.
Or it can be a fixed address in low memory, or a scratch register in
hardware.
Both will work (fixed addresses are better be avoided and who needs
another PV device), but I do not see how either of them is better then
patching. What is your concern?
6:25 p.m.
New subject: [Qemu-devel] [PATCH 3/3] Get system state configuration from QEMU and patch DSDT with it.
On 05/20/2012 07:16 PM, Kevin O'Connor wrote:
Backwards compatibility. qemu has a -M machine-type option that expose
an old qemu's guest-visible attributes. If an old qemu didn't support
S3, then -M old shouldn't either.
--
error compiling committee.c: too many arguments to function
Here we in
agreement, and I was against patching till it was unavoidable,
but than pci hotplug started using it, and afterwards processor
definitions, so no point in avoiding it now by using inferior methods.
I agree as well.
What's the background to needing to have dynamic S3/S4 definitions?
(Why will some qemu instances be able to sleep and not others?)
6:39 p.m.
New subject: [Qemu-devel] [PATCH 3/3] Get system state configuration from QEMU and patch DSDT with it.
On Sun, May 20, 2012 at 07:25:40PM +0300, Avi Kivity wrote:
On 05/20/2012 07:16 PM, Kevin O'Connor wrote:
Backwards compatibility. qemu has a -M machine-type option that expose
an old qemu's guest-visible attributes. If an old qemu didn't support
S3, then -M old shouldn't either.
The DSDT has claimed S3, S4, and S5 support since SeaBIOS has
supported ACPI. What's the background to the requirement to stop
claiming support for it?
-Kevin
Here we
in agreement, and I was against patching till it was unavoidable,
but than pci hotplug started using it, and afterwards processor
definitions, so no point in avoiding it now by using inferior methods.
I agree as well.
What's the background to needing to have dynamic S3/S4 definitions?
(Why will some qemu instances be able to sleep and not others?)
6:49 p.m.
New subject: [Qemu-devel] [PATCH 3/3] Get system state configuration from QEMU and patch DSDT with it.
On Sun, May 20, 2012 at 12:39:13PM -0400, Kevin O'Connor wrote:
On Sun, May 20, 2012 at 07:25:40PM +0300, Avi Kivity
wrote:
Not all guests have working S3/S4 implementation. We want management to
be able to disable S3/S4 for such guests. S4 value changes since we want
to distinguish between S4 and S5 in qemu.
--
Gleb.
On 05/20/2012 07:16 PM, Kevin O'Connor
wrote:
Backwards compatibility. qemu has a -M machine-type option that expose
an old qemu's guest-visible attributes. If an old qemu didn't support
S3, then -M old shouldn't either.
The DSDT has claimed S3, S4, and S5 support since SeaBIOS has
supported ACPI. What's the background to the requirement to stop
claiming support for it?
Here we
in agreement, and I was against patching till it was unavoidable,
but than pci hotplug started using it, and afterwards processor
definitions, so no point in avoiding it now by using inferior methods.
I agree as well.
What's the background to needing to have dynamic S3/S4 definitions?
(Why will some qemu instances be able to sleep and not others?)
6:11 p.m.
New subject: [Qemu-devel] [PATCH 3/3] Get system state configuration from QEMU and patch DSDT with it.
On Sun, May 20, 2012 at 04:39:01PM +0300, Avi Kivity wrote:
What about
If (Fcfg(...)) {
Method()...
}
?
(i.e.. define the method conditionally at runtime)
As Gleb points out, this wont work. AML defines a static
device/method/variable tree heirarchy. Only the return values of
methods is programmable. This completely confused me when I first
started looking at AML.
ACPI is truly a bizarre spec.
-Kevin
12:32 p.m.
New subject: [PATCH 1/3] Fix aml_name_string() to recognize block name modifiers.
On Sun, May 20, 2012 at 12:03:38PM +0300, Gleb Natapov wrote:
Signed-off-by: Gleb Natapov <gleb(a)redhat.com>
---
tools/acpi_extract.py | 6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/tools/acpi_extract.py b/tools/acpi_extract.py
index 5f613e4..8038269 100755
--- a/tools/acpi_extract.py
+++ b/tools/acpi_extract.py
@@ -121,7 +121,11 @@ def aml_name_string(offset):
if (aml[offset] != 0x08):
die( "Name offset 0x%x: expected 0x08 actual 0x%x" %
(offset, aml[offset]));
- return offset + 1;
+ offset += 1
+ # Block Name Modifier. Skip it.
+ if (aml[offset] == 0x5c or aml[offset] == 0x5e):
You don't need parenthesis around the whole conditional.
+ offset += 1
+ return offset;
# Given data offset, find dword const offset
def aml_data_dword_const(offset):
--
1.7.7.3
_______________________________________________
SeaBIOS mailing list
SeaBIOS(a)seabios.org
http://www.seabios.org/mailman/listinfo/seabios
1:13 p.m.
New subject: [PATCH 1/3] Fix aml_name_string() to recognize block name modifiers.
On Sun, May 20, 2012 at 01:32:18PM +0300, Alon Levy wrote:
On Sun, May 20, 2012 at 12:03:38PM +0300, Gleb Natapov
wrote:
Rest of the code has
it. Better to keep same style :)
Signed-off-by: Gleb Natapov <gleb(a)redhat.com>
---
tools/acpi_extract.py | 6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/tools/acpi_extract.py b/tools/acpi_extract.py
index 5f613e4..8038269 100755
--- a/tools/acpi_extract.py
+++ b/tools/acpi_extract.py
@@ -121,7 +121,11 @@ def aml_name_string(offset):
if (aml[offset] != 0x08):
die( "Name offset 0x%x: expected 0x08 actual 0x%x" %
(offset, aml[offset]));
- return offset + 1;
+ offset += 1
+ # Block Name Modifier. Skip it.
+ if (aml[offset] == 0x5c or aml[offset] == 0x5e):
You don't need parenthesis around the whole conditional.
> + offset += 1
> + return offset;
>
> # Given data offset, find dword const offset
> def aml_data_dword_const(offset):
> --
> 1.7.7.3
>
>
> _______________________________________________
> SeaBIOS mailing list
> SeaBIOS(a)seabios.org
> http://www.seabios.org/mailman/listinfo/seabios
--
Gleb.
3:23 a.m.
New subject: [Qemu-devel] [PATCH 1/3] Fix aml_name_string() to recognize block name modifiers.
On Sun, May 20, 2012 at 12:03:38PM +0300, Gleb Natapov wrote:
Signed-off-by: Gleb Natapov <gleb(a)redhat.com>
The patch series looks okay to me. Let me know when the corresponding
qemu patches are committed.
-Kevin
3:08 p.m.
New subject: [Qemu-devel] [PATCH 1/3] Fix aml_name_string() to recognize block name modifiers.
On Tue, May 22, 2012 at 09:23:03PM -0400, Kevin O'Connor wrote:
On Sun, May 20, 2012 at 12:03:38PM +0300, Gleb Natapov
wrote:
It is committed now: 459ae5ea5ad682c2b3220beb244d4102c1a4e332
--
Gleb.
Signed-off-by: Gleb Natapov <gleb(a)redhat.com>
The patch series looks okay to me. Let me know when the corresponding
qemu patches are committed.
3:06 a.m.
New subject: [Qemu-devel] [PATCH 1/3] Fix aml_name_string() to recognize block name modifiers.
On Wed, Jun 20, 2012 at 04:08:41PM +0300, Gleb Natapov wrote:
On Tue, May 22, 2012 at 09:23:03PM -0400, Kevin
O'Connor wrote:
Thanks. I committed your seabios patches.
-Kevin
On Sun, May 20, 2012 at 12:03:38PM +0300, Gleb
Natapov wrote:
It is committed now: 459ae5ea5ad682c2b3220beb244d4102c1a4e332
Signed-off-by: Gleb Natapov <gleb(a)redhat.com>
The patch series looks okay to me. Let me know when the corresponding
qemu patches are committed.
2772
days inactive
2805
days old
24 comments
4 participants
participants (4)
-
Alon Levy -
Avi Kivity -
Gleb Natapov -
Kevin O'Connor