Define and implement non-standard feature control for SMI handling via the
APM_STS register, such that it remains compatible with SeaBIOS's current
access pattern, and enables OVMF to negotiate features.
Cc: "Kevin O'Connor" <ke...@koconnor.net>
Cc: "Michael S. Tsirkin" <m...@redhat.com>
Cc: Gerd Hoffmann <kra...@redhat.com>
Cc: Paolo Bonzini <pbonz...@redhat.com>
Signed-off-by: Laszlo Ersek <ler...@redhat.com>
---
Notes:
v3:
- feature negotiation is new in v3 [Paolo, Michael]
docs/specs/q35-apm-sts.txt | 71 ++++++++++++++++++++++++++++++++++++++++++++++
include/hw/i386/ich9.h | 8 ++++++
hw/isa/lpc_ich9.c | 54 ++++++++++++++++++++++++++++++++++-
3 files changed, 132 insertions(+), 1 deletion(-)
create mode 100644 docs/specs/q35-apm-sts.txt
diff --git a/docs/specs/q35-apm-sts.txt b/docs/specs/q35-apm-sts.txt
new file mode 100644
index 000000000000..cdffb6834380
--- /dev/null
+++ b/docs/specs/q35-apm-sts.txt
@@ -0,0 +1,71 @@
+According to the ICH9 specification, the APM_STS (Advanced Power Management
+Status Port) Register, at IO Port 0xB3, is a scratchpad register for passing
+data between the software agent that raises the synchronous SMI, and the SMI
+handler routine in the firmware. It is not coupled to any hardware
+functionality. The register is also known as SMI Data Port.
+
+While the ACPI FADT exposes the SMI Command Port (APM_CNT) to the OS, the Data
+Port is not exposed.
+
+SeaBIOS uses the Data Port values 0x00 and 0x01 in its SMI handler. SeaBIOS
+also relies on the original behavior of QEMU, where writing to the Command
+Port (APM_CNT) would raise the SMI only on the processor writing the port.
+
+OVMF needs dynamically detectable and settable features for SMI behavior. We
+can use the remaining bits in APM_STS for this, without disturbing SeaBIOS.
+
+The following describes the (non-standard) bit definitions in APM_STS.
+
+ 7 6 5 4 3 2 1 0
+ +---+---+---+---+---+---+---+---+
+ | | | | | | | | |
+ +---+---+---+---+---+---+---+---+
+ ^ ^ ^ ^ ^ ^ ^ ^
+ | | | | | | | Transparent bit. Whatever bit the firmware
+ | | | | | | | writes in this position has no effect on
+ | | | | | | | hardware, and can be read back identically.
+ | | | | | | |
+ | | | | | | Feature negotiation bit.
+ | | | | | |
+ Feature bits. All reserved at the moment.
+
+Feature negotiation
+-------------------
+
+Firmware that is enlightened about this feature shall write 1 to the feature
+negotiation bit first (clearing all other bits), then read back the APM_STS
+register. If the feature negotiation bit is set in the result, then QEMU lacks
+the feature negotiation feature, and APM_STS is entirely transparent. Otherwise
+(i.e., the feature negotiation bit is clear in the result), the more
+significant bits (the feature bits) expose the features supported by QEMU. At
+the moment, no features are defined, and all feature bits read as zero.
+
+Once firmware confirms feature negotiation is available, it shall set (select)
+a subset of the advertised feature bits, and clear the feature negotiation bit,
+in a single write to the APM_STS register. In order to verify the result, the
+firmware shall read back the APM_STS register. If the feature negotiation bit
+is clear in the result, then the feature selection was successful. Otherwise,
+the requested features either weren't a subset of the available features, or
+constituted an inconsistent group of features (breaking inter-feature
+dependencies, for example). Regardless of the feature negotiation bit in the
+read back value, the higher order bits (i.e., the individual feature bits) are
+always zero in that value.
+
+SeaBIOS compatibility
+---------------------
+
+Writing 0x00 and 0x01 to APM_STS translates to "clear all features", with the
+following consequences:
+
+- The SMI behavior remains the original QEMU behavior, regardless of any future
+ features / feature bits.
+
+- Reading back APM_STS returns the value previously written. This is because
+
+ - the least significant bit is transparent,
+
+ - clearing all features is a request that always succeeds, hence the feature
+ negotiation bit will read as zero,
+
+ - the feature bits always read as zero after writing zero to the feature
+ negotiation bit.
diff --git a/include/hw/i386/ich9.h b/include/hw/i386/ich9.h
index 5fd7e97d2347..8304396a487f 100644
--- a/include/hw/i386/ich9.h
+++ b/include/hw/i386/ich9.h
@@ -70,6 +70,9 @@ typedef struct ICH9LPCState {
Notifier machine_ready;
qemu_irq gsi[GSI_NUM_PINS];
+
+ /* SMI features negotiated via APM_STS */
+ uint8_t smi_features;
} ICH9LPCState;
Object *ich9_lpc_find(void);
@@ -208,6 +211,11 @@ Object *ich9_lpc_find(void);
#define ICH9_APM_ACPI_ENABLE 0x2
#define ICH9_APM_ACPI_DISABLE 0x3
+/* non-standard bits for the APM_STS register */
+#define ICH9_APM_STS_TRANSPARENT_MASK 0x01
+#define ICH9_APM_STS_GET_SET_FEATURES 0x02
+#define ICH9_APM_STS_KNOWN_FEATURES 0x00
+#define ICH9_APM_STS_FEATURE_MASK 0xfc
/* D31:F3 SMBus controller */
#define TYPE_ICH9_SMB_DEVICE "ICH9 SMB"
diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
index e3556c9f9eae..a50c4a15b6d1 100644
--- a/hw/isa/lpc_ich9.c
+++ b/hw/isa/lpc_ich9.c
@@ -390,6 +390,37 @@ static void ich9_apm_ctrl_changed(uint32_t val, void *arg)
}
}
+static void ich9_apm_status_changed(uint32_t val, void *arg)
+{
+ ICH9LPCState *lpc = arg;
+ uint32_t new_val = 0;
+
+ /* copy the transparent bits */
+ new_val |= val & ICH9_APM_STS_TRANSPARENT_MASK;
+
+ if (val & ICH9_APM_STS_GET_SET_FEATURES) {
+ /* The guest is querying features. Clearing
+ * ICH9_APM_STS_GET_SET_FEATURES means success.
+ */
+
+ /* provide known features */
+ new_val |= ICH9_APM_STS_KNOWN_FEATURES;
+ } else {
+ /* The guest is setting features. */
+ if (val & ICH9_APM_STS_FEATURE_MASK &
+ ~(uint32_t)ICH9_APM_STS_KNOWN_FEATURES) {
+ /* Unknown feature requested, report error. */
+ new_val |= ICH9_APM_STS_GET_SET_FEATURES;
+ } else {
+ /* Set requested features. Clearing ICH9_APM_STS_GET_SET_FEATURES
+ * means success.*/
+ lpc->smi_features = val & ICH9_APM_STS_KNOWN_FEATURES;
+ }
+ }
+
+ lpc->apm.apms = new_val;
+}
+
/* config:PMBASE */
static void
ich9_lpc_pmbase_sci_update(ICH9LPCState *lpc)
@@ -507,6 +538,7 @@ static void ich9_lpc_reset(DeviceState *qdev)
lpc->sci_level = 0;
lpc->rst_cnt = 0;
+ lpc->smi_features = 0;
}
/* root complex register block is mapped into memory space */
@@ -634,7 +666,8 @@ static void ich9_lpc_realize(PCIDevice *d, Error **errp)
lpc->isa_bus = isa_bus;
ich9_cc_init(lpc);
- apm_init(d, &lpc->apm, ich9_apm_ctrl_changed, NULL, lpc);
+ apm_init(d, &lpc->apm, ich9_apm_ctrl_changed, ich9_apm_status_changed,
+ lpc);
lpc->machine_ready.notify = ich9_lpc_machine_ready;
qemu_add_machine_init_done_notifier(&lpc->machine_ready);
@@ -668,6 +701,24 @@ static const VMStateDescription vmstate_ich9_rst_cnt = {
}
};
+static bool ich9_smi_features_needed(void *opaque)
+{
+ ICH9LPCState *lpc = opaque;
+
+ return (lpc->smi_features != 0);
+}
+
+static const VMStateDescription vmstate_ich9_smi_features = {
+ .name = "ICH9LPC/smi_features",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .needed = ich9_smi_features_needed,
+ .fields = (VMStateField[]) {
+ VMSTATE_UINT8(smi_features, ICH9LPCState),
+ VMSTATE_END_OF_LIST()
+ }
+};
+
static const VMStateDescription vmstate_ich9_lpc = {
.name = "ICH9LPC",
.version_id = 1,
@@ -683,6 +734,7 @@ static const VMStateDescription vmstate_ich9_lpc = {
},
.subsections = (const VMStateDescription*[]) {
&vmstate_ich9_rst_cnt,
+ &vmstate_ich9_smi_features,
NULL
}
};
--
2.9.2
