diff mbox

[v3,for-2.9,2/3] hw/isa/lpc_ich9: add SMI feature negotiation via APM_STS

Message ID 20161118103659.10448-3-lersek@redhat.com
State New
Headers show

Commit Message

Laszlo Ersek Nov. 18, 2016, 10:36 a.m. UTC
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" <kevin@koconnor.net>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Laszlo Ersek <lersek@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

-- 
2.9.2
diff mbox

Patch

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
     }
 };