Hi Laszlo,
On 1/24/20 12:40 PM, Laszlo Ersek wrote:
On 10/24/19 17:12, Philippe Mathieu-Daudé wrote:
On 10/24/19 12:29 PM, Laszlo Ersek wrote:
On 10/23/19 14:05, Philippe Mathieu-Daudé wrote:
Hi Laszlo,
On 10/23/19 12:15 AM, Laszlo Ersek wrote:
In v1.5.0, QEMU's "pc" (i440fx) board gained a "CPU present bitmap"
register block. In v2.0.0, this was extended to the "q35" board.
In v2.7.0, a new (read/write) register interface was laid over the "CPU
present bitmap", with an option for the guest to switch the register
block
to the new (a.k.a. modern) interface.
This historical information is helpful to understand when these QEMU
models started to diverge from the original chipset datasheet.
Both interfaces are documented in "docs/specs/acpi_cpu_hotplug.txt" in
the
QEMU tree.
Add macros for a minimal subset of the modern interface, just so we can
count the possible CPUs (as opposed to boot CPUs) in a later patch in
this
series.
Cc: Ard Biesheuvel <ard.biesheu...@linaro.org>
Cc: Igor Mammedov <imamm...@redhat.com>
Cc: Jordan Justen <jordan.l.jus...@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1515
Signed-off-by: Laszlo Ersek <ler...@redhat.com>
---
Notes:
v2:
- use QEMU's existent CPU hotplug register block, rather than
a new
named file in fw_cfg [Igor]
OvmfPkg/Include/IndustryStandard/I440FxPiix4.h | 5 +++
OvmfPkg/Include/IndustryStandard/Q35MchIch9.h | 2 +
OvmfPkg/Include/IndustryStandard/QemuCpuHotplug.h | 43
++++++++++++++++++++
3 files changed, 50 insertions(+)
diff --git a/OvmfPkg/Include/IndustryStandard/I440FxPiix4.h
b/OvmfPkg/Include/IndustryStandard/I440FxPiix4.h
index e7d7fde14c65..3973ff0a95b4 100644
--- a/OvmfPkg/Include/IndustryStandard/I440FxPiix4.h
+++ b/OvmfPkg/Include/IndustryStandard/I440FxPiix4.h
@@ -44,6 +44,11 @@
BIT10 | BIT9 | BIT8 | BIT7 |
BIT6)
#define PIIX4_PMREGMISC 0x80
#define PIIX4_PMREGMISC_PMIOSE BIT0
+//
+// IO ports
+//
+#define PIIX4_CPU_HOTPLUG_BASE 0xAF00
OK
+
#endif
diff --git a/OvmfPkg/Include/IndustryStandard/Q35MchIch9.h
b/OvmfPkg/Include/IndustryStandard/Q35MchIch9.h
index 391cb4622226..2ac16f19c62e 100644
--- a/OvmfPkg/Include/IndustryStandard/Q35MchIch9.h
+++ b/OvmfPkg/Include/IndustryStandard/Q35MchIch9.h
@@ -104,10 +104,12 @@
// IO ports
//
#define ICH9_APM_CNT 0xB2
#define ICH9_APM_STS 0xB3
+#define ICH9_CPU_HOTPLUG_BASE 0x0CD8
OK
+
//
// IO ports relative to PMBASE
//
#define ICH9_PMBASE_OFS_SMI_EN 0x30
#define ICH9_SMI_EN_APMC_EN BIT5
diff --git a/OvmfPkg/Include/IndustryStandard/QemuCpuHotplug.h
b/OvmfPkg/Include/IndustryStandard/QemuCpuHotplug.h
new file mode 100644
index 000000000000..cf0745610f2c
--- /dev/null
+++ b/OvmfPkg/Include/IndustryStandard/QemuCpuHotplug.h
@@ -0,0 +1,43 @@
+/** @file
+ Macros for accessing QEMU's CPU hotplug register block.
+
+ Copyright (C) 2019, Red Hat, Inc.
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+ @par Specification Reference:
+
+ - "docs/specs/acpi_cpu_hotplug.txt" in the QEMU source tree.
+
+ The original (now "legacy") CPU hotplug interface appeared in
QEMU v1.5.0.
+ The new ("modern") hotplug interface appeared in QEMU v2.7.0.
+
+ The macros in this header file map to the minimal subset of the
modern
+ interface that OVMF needs.
+**/
+
+#ifndef QEMU_CPU_HOTPLUG_H_
+#define QEMU_CPU_HOTPLUG_H_
+
+#include <Base.h>
+
+//
+// Each register offset is:
+// - relative to the board-dependent IO base address of the register
block,
+// - named QEMU_CPUHP_(R|W|RW)_*, according to the possible access
modes of the
+// register,
+// - followed by distinguished bitmasks or values in the register.
+//
+#define QEMU_CPUHP_R_CMD_DATA2 0x0
I don't understand this one. Read register 0x0 is marked "reserved"
in the spec, and CMD_DATA are registers [0x8 .. 0xb].
The documentation (spec) is being cleaned up, and also extended. Please
see the following QEMU patch set (warning: long discussion):
[qemu-devel] [RFC 0/3] acpi: cphp: add CPHP_GET_CPU_ID_CMD command to
cpu hotplug MMIO interface
http://mid.mail-archive.com/20191009132252.17860-1-imammedo@redhat.com
In that qemu RFC series, the first two patches are of primary interest
here, for this edk2 / OvmfPkg patch set. The first two patches will not
extend the modern interface, they will just document it better (clean up
the documentation for the existent behavior).
OK I read it, but it is still RFC, so this patch shouldn't get merged
until the QEMU counterpart get merged, right?
If you look at the next patch in this series, you'll see that the
OvmfPkg/PlatformPei code uses QEMU_CPUHP_R_CMD_DATA2 considering *both*
situations, that is,
- when the offset range [0..3] is marked as reserved for reading,
- and when it is specified as Command Data 2 (by the last patch in the
QEMU RFC series).
Therefore, we could call this register
QEMU_CPUHP_R_RESERVED_0
for now, but very soon we'd have to rename it to the above-proposed
QEMU_CPUHP_R_CMD_DATA2
anyway. Again, the code considers both cases.
(In fact, the code in the next patch considers *four* cases:
- hotplug register block absent,
- hotplug register block present, but only legacy mode is supported,
- hotplug register block present, modern mode available, but offset 0 is
still reserved for reading,
- hotplug register block present, modern mode available, offset 0
defined as Cmd Data 2.)
Understood.
This patch is fine then.
Since the QEMU spec is RFC and not merged, I'm worried it might
change. I'd rather review this patch again once the QEMU spec
update is accepted/merged.
Igor's QEMU work is now upstream: 3e08b2b9cb64..3a61c8db9d25
Can you please re-check this patch? No changes should be necessary.
In particular,
- QEMU commit e6d0c3ce6895 ("acpi: cpuhp: introduce 'Command data 2'
field", 2020-01-22) covers the definition of QEMU_CPUHP_R_CMD_DATA2, in
this patch,
- QEMU commit ae340aa3d256 ("acpi: cpuhp: spec: add typical usecases",
2020-01-22), section "detect and enable modern CPU hotplug interface",
matches the QEMU_CPUHP_R_CMD_DATA2 usage in the next patch of this series.
Re-checked correctly. Thanks for waiting/tracking it get merged.
My Reviewed-by stands.
Reviewed-by: Philippe Mathieu-Daude <phi...@redhat.com>
+
+#define QEMU_CPUHP_R_CPU_STAT 0x4
+#define QEMU_CPUHP_STAT_ENABLED BIT0
OK
+
+#define QEMU_CPUHP_RW_CMD_DATA 0x8
Here the spec says "DWORD access" but the implementation use
"BYTE access" (see AcpiCpuHotplug_ops in hw/acpi/cpu_hotplug.c).
I understand this as "there are 4 consecutive BYTE register).
You are looking at the legacy mode of the register block (documented
near the top of the "docs/specs/acpi_cpu_hotplug.txt" file).
The operations for modern mode are in "hw/acpi/cpu.c", object
"cpu_hotplug_ops".
Not this patch problem although.
+
+#define QEMU_CPUHP_W_CPU_SEL 0x0
Another "DWORD access". OK.
+
+#define QEMU_CPUHP_W_CMD 0x5
+#define QEMU_CPUHP_CMD_GET_PENDING 0x0
'PENDING' is more meaningful than "CPU device with
inserting/removing events". OK
Thanks!
Laszlo
+
+#endif // QEMU_CPU_HOTPLUG_H_
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#53525): https://edk2.groups.io/g/devel/message/53525
Mute This Topic: https://groups.io/mt/36465006/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
