* [Qemu-devel] [PATCH-for-4.2 v10 00/11] ARM virt: ACPI memory hotplug support
@ 2019-09-04 8:56 Shameer Kolothum
2019-09-04 8:56 ` [Qemu-devel] [PATCH-for-4.2 v10 01/11] hw/acpi: Make ACPI IO address space configurable Shameer Kolothum
` (10 more replies)
0 siblings, 11 replies; 29+ messages in thread
From: Shameer Kolothum @ 2019-09-04 8:56 UTC (permalink / raw)
To: qemu-devel, qemu-arm, eric.auger, imammedo
Cc: peter.maydell, sameo, ard.biesheuvel, linuxarm, xuwei5,
shannon.zhaosl, sebastien.boeuf, lersek
This series is an attempt to provide device memory hotplug support
on ARM virt platform. This is based on Eric's recent works here[1]
and carries some of the pc-dimm related patches dropped from his
series.
The kernel support for arm64 memory hot add was added recently by
Robin and hence the guest kernel should be => 5.0-rc1.
NVDIM support is not included currently as we still have an unresolved
issue while hot adding NVDIMM[2]. However NVDIMM cold plug patches
can be included, but not done for now, for keeping it simple.
This makes use of GED device to sent hotplug ACPI events to the
Guest. GED code is based on Nemu. Thanks to the efforts of Samuel and
Sebastien to add the hardware-reduced support to Nemu using GED
device[3]. (Please shout if I got the author/signed-off wrong for
those patches or missed any names).
This is sanity tested on a HiSilicon ARM64 platform and appreciate
any further testing.
Note:
Attempted adding dimm_pxm test case to bios-tables-test for arm/virt.
But noticed the issue decribed here[5]. This is under investigation
now.
Thanks,
Shameer
[1] https://patchwork.kernel.org/cover/10837565/
[2] https://patchwork.kernel.org/cover/10783589/
[3] https://github.com/intel/nemu/blob/topic/virt-x86/hw/acpi/ged.c
[4] http://lists.infradead.org/pipermail/linux-arm-kernel/2019-May/651763.html
[5] https://www.mail-archive.com/qemu-devel@nongnu.org/msg632651.html
v9 --> v10
-Fix for "make check" failure on x86_64(Patch #1).
-Minor updates based on Eric's comments.
-Dropped patch "hw/arm/virt: Add 4.2 machine type" as this is already
in master now.
-Added R-by tags by Eric.
v8 --> v9
-Changes related to GED being a TYPE_SYS_BUS_DEVICE now.
-Re-arranged patches 8 and 9.
-Added GED ABI documentation(patch #10).
-Added numamem and memhp tests to arm/virt(#11 and #12)
-Dropped few R-by tags as code has changed a bit.
-Please see Individual patch history for details.
v7 --> v8
-Addressed comments from Igor.Please see individual patches.
-Updated bios-tables-test-allowed-diff.h to avoid "make check"
failure (patch #6) and dropped patch #10
-Added Igor's R-by to patches 4 & 5.
-Dropped Erics's R-by from patch #9 for now.
v6 --> v7
- Added 4.2 machine support and restricted GED creation for < 4.2
This is to address the migration test fail reported by Eric.
- Included "tests: Update DSDT ACPI table.." patch(#10) from Eric
to fix the "make check" bios-tables-test failure.
v5 --> v6
-Addressed comments from Eric.
-Added R-by from Eric and Igor.
v4 --> v5
-Removed gsi/ged-irq routing in virt.
-Added Migration support.
-Dropped support for DT coldplug case based on the discussions
here[4]
-Added system_powerdown support through GED.
v3 --> v4
Addressed comments from Igor and Eric,
-Renamed "virt-acpi" to "acpi-ged".
-Changed ged device parent to TYPE_DEVICE.
-Introduced DT memory node property "hotpluggable" to resolve device
memory being treated as early boot memory issue(patch #7).
-Combined patches #3 and #9 from v3 into #3.
v2 --> v3
Addressed comments from Igor and Eric,
-Made virt acpi device platform independent and moved
to hw/acpi/generic_event_device.c
-Moved ged specific code into hw/acpi/generic_event_device.c
-Introduced an opt-in feature "fdt" to resolve device-memory being
treated as early boot memory.
-Dropped patch #1 from v2.
RFC --> v2
-Use GED device instead of GPIO for ACPI hotplug events.
-Removed NVDIMM support for now.
-Includes dropped patches from Eric's v9 series.
Eric Auger (1):
hw/arm/virt: Add memory hotplug framework
Samuel Ortiz (2):
hw/acpi: Do not create memory hotplug method when handler is not
defined
hw/acpi: Add ACPI Generic Event Device Support
Shameer Kolothum (8):
hw/acpi: Make ACPI IO address space configurable
hw/arm/virt: Enable device memory cold/hot plug with ACPI boot
hw/arm/virt-acpi-build: Add PC-DIMM in SRAT
hw/arm: Factor out powerdown notifier from GPIO
hw/arm: Use GED for system_powerdown event
docs/specs: Add ACPI GED documentation
tests: add dummy ACPI tables for arm/virt board
tests: Add bios tests to arm/virt
docs/specs/acpi_hw_reduced_hotplug.txt | 60 +++++
hw/acpi/Kconfig | 4 +
hw/acpi/Makefile.objs | 1 +
hw/acpi/generic_event_device.c | 325 +++++++++++++++++++++++++
hw/acpi/memory_hotplug.c | 43 ++--
hw/arm/Kconfig | 4 +
hw/arm/virt-acpi-build.c | 31 ++-
hw/arm/virt.c | 127 +++++++++-
hw/i386/acpi-build.c | 7 +-
hw/i386/pc.c | 3 +
include/hw/acpi/acpi_dev_interface.h | 1 +
include/hw/acpi/generic_event_device.h | 103 ++++++++
include/hw/acpi/memory_hotplug.h | 9 +-
include/hw/arm/virt.h | 5 +
include/hw/i386/pc.h | 3 +
tests/bios-tables-test-allowed-diff.h | 2 +
tests/bios-tables-test.c | 49 ++++
tests/data/acpi/virt/SLIT | Bin 0 -> 48 bytes
tests/data/acpi/virt/SRAT | Bin 0 -> 224 bytes
19 files changed, 736 insertions(+), 41 deletions(-)
create mode 100644 docs/specs/acpi_hw_reduced_hotplug.txt
create mode 100644 hw/acpi/generic_event_device.c
create mode 100644 include/hw/acpi/generic_event_device.h
create mode 100644 tests/data/acpi/virt/SLIT
create mode 100644 tests/data/acpi/virt/SRAT
--
2.17.1
^ permalink raw reply [flat|nested] 29+ messages in thread
* [Qemu-devel] [PATCH-for-4.2 v10 01/11] hw/acpi: Make ACPI IO address space configurable
2019-09-04 8:56 [Qemu-devel] [PATCH-for-4.2 v10 00/11] ARM virt: ACPI memory hotplug support Shameer Kolothum
@ 2019-09-04 8:56 ` Shameer Kolothum
2019-09-04 11:27 ` Igor Mammedov
2019-09-04 8:56 ` [Qemu-devel] [PATCH-for-4.2 v10 02/11] hw/acpi: Do not create memory hotplug method when handler is not defined Shameer Kolothum
` (9 subsequent siblings)
10 siblings, 1 reply; 29+ messages in thread
From: Shameer Kolothum @ 2019-09-04 8:56 UTC (permalink / raw)
To: qemu-devel, qemu-arm, eric.auger, imammedo
Cc: peter.maydell, sameo, ard.biesheuvel, linuxarm, xuwei5,
shannon.zhaosl, sebastien.boeuf, lersek
This is in preparation for adding support for ARM64 platforms
where it doesn't use port mapped IO for ACPI IO space. We are
making changes so that MMIO region can be accommodated
and board can pass the base address into the aml build function.
Also move few MEMORY_* definitions to header so that other memory
hotplug event signalling mechanisms (eg. Generic Event Device on
HW-reduced acpi platforms) can use the same from their respective
event handler code.
Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
---
v9 --> v10
- Fixed "make check" failure on x86_64.
v8 --> v9
-base address is an input into build_memory_hotplug_aml()
-Removed R-by tags from Igor and Eric for now.
---
hw/acpi/memory_hotplug.c | 33 ++++++++++++++------------------
hw/i386/acpi-build.c | 7 ++++++-
hw/i386/pc.c | 3 +++
include/hw/acpi/memory_hotplug.h | 9 +++++++--
include/hw/i386/pc.h | 3 +++
5 files changed, 33 insertions(+), 22 deletions(-)
diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
index 9483d66e86..9b0b150f4f 100644
--- a/hw/acpi/memory_hotplug.c
+++ b/hw/acpi/memory_hotplug.c
@@ -30,12 +30,7 @@
#define MEMORY_SLOT_PROXIMITY_METHOD "MPXM"
#define MEMORY_SLOT_EJECT_METHOD "MEJ0"
#define MEMORY_SLOT_NOTIFY_METHOD "MTFY"
-#define MEMORY_SLOT_SCAN_METHOD "MSCN"
#define MEMORY_HOTPLUG_DEVICE "MHPD"
-#define MEMORY_HOTPLUG_IO_LEN 24
-#define MEMORY_DEVICES_CONTAINER "\\_SB.MHPC"
-
-static uint16_t memhp_io_base;
static ACPIOSTInfo *acpi_memory_device_status(int slot, MemStatus *mdev)
{
@@ -210,7 +205,7 @@ static const MemoryRegionOps acpi_memory_hotplug_ops = {
};
void acpi_memory_hotplug_init(MemoryRegion *as, Object *owner,
- MemHotplugState *state, uint16_t io_base)
+ MemHotplugState *state, hwaddr io_base)
{
MachineState *machine = MACHINE(qdev_get_machine());
@@ -219,12 +214,10 @@ void acpi_memory_hotplug_init(MemoryRegion *as, Object *owner,
return;
}
- assert(!memhp_io_base);
- memhp_io_base = io_base;
state->devs = g_malloc0(sizeof(*state->devs) * state->dev_count);
memory_region_init_io(&state->io, owner, &acpi_memory_hotplug_ops, state,
"acpi-mem-hotplug", MEMORY_HOTPLUG_IO_LEN);
- memory_region_add_subregion(as, memhp_io_base, &state->io);
+ memory_region_add_subregion(as, io_base, &state->io);
}
/**
@@ -343,7 +336,8 @@ const VMStateDescription vmstate_memory_hotplug = {
void build_memory_hotplug_aml(Aml *table, uint32_t nr_mem,
const char *res_root,
- const char *event_handler_method)
+ const char *event_handler_method,
+ AmlRegionSpace rs, hwaddr memhp_io_base)
{
int i;
Aml *ifctx;
@@ -352,10 +346,6 @@ void build_memory_hotplug_aml(Aml *table, uint32_t nr_mem,
Aml *mem_ctrl_dev;
char *mhp_res_path;
- if (!memhp_io_base) {
- return;
- }
-
mhp_res_path = g_strdup_printf("%s." MEMORY_HOTPLUG_DEVICE, res_root);
mem_ctrl_dev = aml_device("%s", mhp_res_path);
{
@@ -366,14 +356,19 @@ void build_memory_hotplug_aml(Aml *table, uint32_t nr_mem,
aml_name_decl("_UID", aml_string("Memory hotplug resources")));
crs = aml_resource_template();
- aml_append(crs,
- aml_io(AML_DECODE16, memhp_io_base, memhp_io_base, 0,
- MEMORY_HOTPLUG_IO_LEN)
- );
+ if (rs == AML_SYSTEM_IO) {
+ aml_append(crs,
+ aml_io(AML_DECODE16, memhp_io_base, memhp_io_base, 0,
+ MEMORY_HOTPLUG_IO_LEN)
+ );
+ } else {
+ aml_append(crs, aml_memory32_fixed(memhp_io_base,
+ MEMORY_HOTPLUG_IO_LEN, AML_READ_WRITE));
+ }
aml_append(mem_ctrl_dev, aml_name_decl("_CRS", crs));
aml_append(mem_ctrl_dev, aml_operation_region(
- MEMORY_HOTPLUG_IO_REGION, AML_SYSTEM_IO,
+ MEMORY_HOTPLUG_IO_REGION, rs,
aml_int(memhp_io_base), MEMORY_HOTPLUG_IO_LEN)
);
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 034e413fd0..0d97cc701d 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1873,7 +1873,12 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
build_cpus_aml(dsdt, machine, opts, pm->cpu_hp_io_base,
"\\_SB.PCI0", "\\_GPE._E02");
}
- build_memory_hotplug_aml(dsdt, nr_mem, "\\_SB.PCI0", "\\_GPE._E03");
+
+ if (pcms->memhp_io_base && nr_mem) {
+ build_memory_hotplug_aml(dsdt, nr_mem, "\\_SB.PCI0",
+ "\\_GPE._E03", AML_SYSTEM_IO,
+ pcms->memhp_io_base);
+ }
scope = aml_scope("_GPE");
{
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index c14ed86439..b38cd07231 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1951,6 +1951,9 @@ void pc_memory_init(PCMachineState *pcms,
/* Init default IOAPIC address space */
pcms->ioapic_as = &address_space_memory;
+
+ /* Init ACPI memory hotplug IO base address */
+ pcms->memhp_io_base = ACPI_MEMORY_HOTPLUG_BASE;
}
/*
diff --git a/include/hw/acpi/memory_hotplug.h b/include/hw/acpi/memory_hotplug.h
index 77c65765d6..dfe9cf3fde 100644
--- a/include/hw/acpi/memory_hotplug.h
+++ b/include/hw/acpi/memory_hotplug.h
@@ -5,6 +5,10 @@
#include "hw/acpi/acpi.h"
#include "hw/acpi/aml-build.h"
+#define MEMORY_SLOT_SCAN_METHOD "MSCN"
+#define MEMORY_DEVICES_CONTAINER "\\_SB.MHPC"
+#define MEMORY_HOTPLUG_IO_LEN 24
+
/**
* MemStatus:
* @is_removing: the memory device in slot has been requested to be ejected.
@@ -29,7 +33,7 @@ typedef struct MemHotplugState {
} MemHotplugState;
void acpi_memory_hotplug_init(MemoryRegion *as, Object *owner,
- MemHotplugState *state, uint16_t io_base);
+ MemHotplugState *state, hwaddr io_base);
void acpi_memory_plug_cb(HotplugHandler *hotplug_dev, MemHotplugState *mem_st,
DeviceState *dev, Error **errp);
@@ -48,5 +52,6 @@ void acpi_memory_ospm_status(MemHotplugState *mem_st, ACPIOSTInfoList ***list);
void build_memory_hotplug_aml(Aml *table, uint32_t nr_mem,
const char *res_root,
- const char *event_handler_method);
+ const char *event_handler_method,
+ AmlRegionSpace rs, hwaddr memhp_io_base);
#endif
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 19a837889d..ac09e42030 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -69,6 +69,9 @@ struct PCMachineState {
/* Address space used by IOAPIC device. All IOAPIC interrupts
* will be translated to MSI messages in the address space. */
AddressSpace *ioapic_as;
+
+ /* ACPI Memory hotplug IO base address */
+ hwaddr memhp_io_base;
};
#define PC_MACHINE_ACPI_DEVICE_PROP "acpi-device"
--
2.17.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Qemu-devel] [PATCH-for-4.2 v10 02/11] hw/acpi: Do not create memory hotplug method when handler is not defined
2019-09-04 8:56 [Qemu-devel] [PATCH-for-4.2 v10 00/11] ARM virt: ACPI memory hotplug support Shameer Kolothum
2019-09-04 8:56 ` [Qemu-devel] [PATCH-for-4.2 v10 01/11] hw/acpi: Make ACPI IO address space configurable Shameer Kolothum
@ 2019-09-04 8:56 ` Shameer Kolothum
2019-09-04 8:56 ` [Qemu-devel] [PATCH-for-4.2 v10 03/11] hw/acpi: Add ACPI Generic Event Device Support Shameer Kolothum
` (8 subsequent siblings)
10 siblings, 0 replies; 29+ messages in thread
From: Shameer Kolothum @ 2019-09-04 8:56 UTC (permalink / raw)
To: qemu-devel, qemu-arm, eric.auger, imammedo
Cc: peter.maydell, sameo, ard.biesheuvel, linuxarm, xuwei5,
shannon.zhaosl, sebastien.boeuf, lersek
From: Samuel Ortiz <sameo@linux.intel.com>
With Hardware-reduced ACPI, the GED device will manage ACPI
hotplug entirely. As a consequence, make the memory specific
events AML generation optional. The code will only be added
when the method name is not NULL.
Signed-off-by: Samuel Ortiz <sameo@linux.intel.com>
Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
---
hw/acpi/memory_hotplug.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
index 9b0b150f4f..8d2e82240f 100644
--- a/hw/acpi/memory_hotplug.c
+++ b/hw/acpi/memory_hotplug.c
@@ -712,10 +712,12 @@ void build_memory_hotplug_aml(Aml *table, uint32_t nr_mem,
}
aml_append(table, dev_container);
- method = aml_method(event_handler_method, 0, AML_NOTSERIALIZED);
- aml_append(method,
- aml_call0(MEMORY_DEVICES_CONTAINER "." MEMORY_SLOT_SCAN_METHOD));
- aml_append(table, method);
+ if (event_handler_method) {
+ method = aml_method(event_handler_method, 0, AML_NOTSERIALIZED);
+ aml_append(method, aml_call0(MEMORY_DEVICES_CONTAINER "."
+ MEMORY_SLOT_SCAN_METHOD));
+ aml_append(table, method);
+ }
g_free(mhp_res_path);
}
--
2.17.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Qemu-devel] [PATCH-for-4.2 v10 03/11] hw/acpi: Add ACPI Generic Event Device Support
2019-09-04 8:56 [Qemu-devel] [PATCH-for-4.2 v10 00/11] ARM virt: ACPI memory hotplug support Shameer Kolothum
2019-09-04 8:56 ` [Qemu-devel] [PATCH-for-4.2 v10 01/11] hw/acpi: Make ACPI IO address space configurable Shameer Kolothum
2019-09-04 8:56 ` [Qemu-devel] [PATCH-for-4.2 v10 02/11] hw/acpi: Do not create memory hotplug method when handler is not defined Shameer Kolothum
@ 2019-09-04 8:56 ` Shameer Kolothum
2019-09-11 12:34 ` Igor Mammedov
2019-09-04 8:56 ` [Qemu-devel] [PATCH-for-4.2 v10 04/11] hw/arm/virt: Add memory hotplug framework Shameer Kolothum
` (7 subsequent siblings)
10 siblings, 1 reply; 29+ messages in thread
From: Shameer Kolothum @ 2019-09-04 8:56 UTC (permalink / raw)
To: qemu-devel, qemu-arm, eric.auger, imammedo
Cc: peter.maydell, sameo, ard.biesheuvel, linuxarm, xuwei5,
shannon.zhaosl, sebastien.boeuf, lersek
From: Samuel Ortiz <sameo@linux.intel.com>
The ACPI Generic Event Device (GED) is a hardware-reduced specific
device[ACPI v6.1 Section 5.6.9] that handles all platform events,
including the hotplug ones. This patch generates the AML code that
defines GEDs.
Platforms need to specify their own GED Event bitmap to describe
what kind of events they want to support through GED. Also this
uses a a single interrupt for the GED device, relying on IO
memory region to communicate the type of device affected by the
interrupt. This way, we can support up to 32 events with a unique
interrupt.
This supports only memory hotplug for now.
Signed-off-by: Samuel Ortiz <sameo@linux.intel.com>
Signed-off-by: Sebastien Boeuf <sebastien.boeuf@intel.com>
Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
---
v9 --> v10
-Added few required headers.
-Added Eric's R-by
v8 --> v9
-Changes related to GED being a TYPE_SYS_BUS_DEVICE now.
-Removed Eric's R-by tag for now.
v7 --> v8.
-Removed qemu_mutex_lock() across the ged state selector access.
-Rephrased comments section in acpi_ged_send_event().
-Moved acpi_ged_event() code into acpi_ged_send_event().
-Added check for memhp_base and ged_base in realize().
---
hw/acpi/Kconfig | 4 +
hw/acpi/Makefile.objs | 1 +
hw/acpi/generic_event_device.c | 317 +++++++++++++++++++++++++
include/hw/acpi/generic_event_device.h | 100 ++++++++
4 files changed, 422 insertions(+)
create mode 100644 hw/acpi/generic_event_device.c
create mode 100644 include/hw/acpi/generic_event_device.h
diff --git a/hw/acpi/Kconfig b/hw/acpi/Kconfig
index 7c59cf900b..12e3f1e86e 100644
--- a/hw/acpi/Kconfig
+++ b/hw/acpi/Kconfig
@@ -31,3 +31,7 @@ config ACPI_VMGENID
bool
default y
depends on PC
+
+config ACPI_HW_REDUCED
+ bool
+ depends on ACPI
diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs
index 9bb2101e3b..655a9c1973 100644
--- a/hw/acpi/Makefile.objs
+++ b/hw/acpi/Makefile.objs
@@ -6,6 +6,7 @@ common-obj-$(CONFIG_ACPI_MEMORY_HOTPLUG) += memory_hotplug.o
common-obj-$(CONFIG_ACPI_CPU_HOTPLUG) += cpu.o
common-obj-$(CONFIG_ACPI_NVDIMM) += nvdimm.o
common-obj-$(CONFIG_ACPI_VMGENID) += vmgenid.o
+common-obj-$(CONFIG_ACPI_HW_REDUCED) += generic_event_device.o
common-obj-$(call lnot,$(CONFIG_ACPI_X86)) += acpi-stub.o
common-obj-y += acpi_interface.o
diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
new file mode 100644
index 0000000000..f81d966058
--- /dev/null
+++ b/hw/acpi/generic_event_device.c
@@ -0,0 +1,317 @@
+/*
+ *
+ * Copyright (c) 2018 Intel Corporation
+ * Copyright (c) 2019 Huawei Technologies R & D (UK) Ltd
+ * Written by Samuel Ortiz, Shameer Kolothum
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "exec/address-spaces.h"
+#include "hw/acpi/acpi.h"
+#include "hw/acpi/generic_event_device.h"
+#include "hw/irq.h"
+#include "hw/mem/pc-dimm.h"
+#include "hw/qdev-properties.h"
+#include "migration/vmstate.h"
+#include "qemu/error-report.h"
+
+static const uint32_t ged_supported_events[] = {
+ ACPI_GED_MEM_HOTPLUG_EVT,
+};
+
+/*
+ * The ACPI Generic Event Device (GED) is a hardware-reduced specific
+ * device[ACPI v6.1 Section 5.6.9] that handles all platform events,
+ * including the hotplug ones. Platforms need to specify their own
+ * GED Event bitmap to describe what kind of events they want to support
+ * through GED. This routine uses a single interrupt for the GED device,
+ * relying on IO memory region to communicate the type of device
+ * affected by the interrupt. This way, we can support up to 32 events
+ * with a unique interrupt.
+ */
+void build_ged_aml(Aml *table, const char *name, HotplugHandler *hotplug_dev,
+ uint32_t ged_irq, AmlRegionSpace rs, hwaddr ged_base)
+{
+ AcpiGedState *s = ACPI_GED(hotplug_dev);
+ Aml *crs = aml_resource_template();
+ Aml *evt, *field;
+ Aml *dev = aml_device("%s", name);
+ Aml *evt_sel = aml_local(0);
+ Aml *esel = aml_name(AML_GED_EVT_SEL);
+
+ /* _CRS interrupt */
+ aml_append(crs, aml_interrupt(AML_CONSUMER, AML_EDGE, AML_ACTIVE_HIGH,
+ AML_EXCLUSIVE, &ged_irq, 1));
+
+ aml_append(dev, aml_name_decl("_HID", aml_string("ACPI0013")));
+ aml_append(dev, aml_name_decl("_UID", aml_string(GED_DEVICE)));
+ aml_append(dev, aml_name_decl("_CRS", crs));
+
+ /* Append IO region */
+ aml_append(dev, aml_operation_region(AML_GED_EVT_REG, rs,
+ aml_int(ged_base + ACPI_GED_EVT_SEL_OFFSET),
+ ACPI_GED_EVT_SEL_LEN));
+ field = aml_field(AML_GED_EVT_REG, AML_DWORD_ACC, AML_NOLOCK,
+ AML_WRITE_AS_ZEROS);
+ aml_append(field, aml_named_field(AML_GED_EVT_SEL,
+ ACPI_GED_EVT_SEL_LEN * BITS_PER_BYTE));
+ aml_append(dev, field);
+
+ /*
+ * For each GED event we:
+ * - Add a conditional block for each event, inside a loop.
+ * - Call a method for each supported GED event type.
+ *
+ * The resulting ASL code looks like:
+ *
+ * Local0 = ESEL
+ * If ((Local0 & One) == One)
+ * {
+ * MethodEvent0()
+ * }
+ *
+ * If ((Local0 & 0x2) == 0x2)
+ * {
+ * MethodEvent1()
+ * }
+ * ...
+ */
+ evt = aml_method("_EVT", 1, AML_SERIALIZED);
+ {
+ Aml *if_ctx;
+ uint32_t i;
+ uint32_t ged_events = ctpop32(s->ged_event_bitmap);
+
+ /* Local0 = ESEL */
+ aml_append(evt, aml_store(esel, evt_sel));
+
+ for (i = 0; i < ARRAY_SIZE(ged_supported_events) && ged_events; i++) {
+ uint32_t event = s->ged_event_bitmap & ged_supported_events[i];
+
+ if (!event) {
+ continue;
+ }
+
+ if_ctx = aml_if(aml_equal(aml_and(evt_sel, aml_int(event), NULL),
+ aml_int(event)));
+ switch (event) {
+ case ACPI_GED_MEM_HOTPLUG_EVT:
+ aml_append(if_ctx, aml_call0(MEMORY_DEVICES_CONTAINER "."
+ MEMORY_SLOT_SCAN_METHOD));
+ break;
+ default:
+ /*
+ * Please make sure all the events in ged_supported_events[]
+ * are handled above.
+ */
+ g_assert_not_reached();
+ }
+
+ aml_append(evt, if_ctx);
+ ged_events--;
+ }
+
+ if (ged_events) {
+ error_report("Unsupported events specified");
+ abort();
+ }
+ }
+
+ /* Append _EVT method */
+ aml_append(dev, evt);
+
+ aml_append(table, dev);
+}
+
+/* Memory read by the GED _EVT AML dynamic method */
+static uint64_t ged_read(void *opaque, hwaddr addr, unsigned size)
+{
+ uint64_t val = 0;
+ GEDState *ged_st = opaque;
+
+ switch (addr) {
+ case ACPI_GED_EVT_SEL_OFFSET:
+ /* Read the selector value and reset it */
+ val = ged_st->sel;
+ ged_st->sel = 0;
+ break;
+ default:
+ break;
+ }
+
+ return val;
+}
+
+/* Nothing is expected to be written to the GED memory region */
+static void ged_write(void *opaque, hwaddr addr, uint64_t data,
+ unsigned int size)
+{
+}
+
+static const MemoryRegionOps ged_ops = {
+ .read = ged_read,
+ .write = ged_write,
+ .endianness = DEVICE_LITTLE_ENDIAN,
+ .valid = {
+ .min_access_size = 4,
+ .max_access_size = 4,
+ },
+};
+
+static void acpi_ged_device_plug_cb(HotplugHandler *hotplug_dev,
+ DeviceState *dev, Error **errp)
+{
+ AcpiGedState *s = ACPI_GED(hotplug_dev);
+
+ if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
+ if (s->memhp_state.is_enabled) {
+ acpi_memory_plug_cb(hotplug_dev, &s->memhp_state, dev, errp);
+ } else {
+ error_setg(errp,
+ "memory hotplug is not enabled: %s.memory-hotplug-support "
+ "is not set", object_get_typename(OBJECT(s)));
+ }
+ } else {
+ error_setg(errp, "virt: device plug request for unsupported device"
+ " type: %s", object_get_typename(OBJECT(dev)));
+ }
+}
+
+static void acpi_ged_send_event(AcpiDeviceIf *adev, AcpiEventStatusBits ev)
+{
+ AcpiGedState *s = ACPI_GED(adev);
+ GEDState *ged_st = &s->ged_state;
+ uint32_t sel;
+
+ if (ev & ACPI_MEMORY_HOTPLUG_STATUS) {
+ sel = ACPI_GED_MEM_HOTPLUG_EVT;
+ } else {
+ /* Unknown event. Return without generating interrupt. */
+ warn_report("GED: Unsupported event %d. No irq injected", ev);
+ return;
+ }
+
+ /*
+ * Set the GED selector field to communicate the event type.
+ * This will be read by GED aml code to select the appropriate
+ * event method.
+ */
+ ged_st->sel |= sel;
+
+ /* Trigger the event by sending an interrupt to the guest. */
+ qemu_irq_pulse(s->irq);
+}
+
+static void acpi_ged_device_realize(DeviceState *dev, Error **errp)
+{
+ AcpiGedState *s = ACPI_GED(dev);
+ GEDState *ged_st = &s->ged_state;
+
+ memory_region_init_io(&ged_st->io, OBJECT(dev), &ged_ops, ged_st,
+ TYPE_ACPI_GED, ACPI_GED_EVT_SEL_LEN);
+ sysbus_init_mmio(SYS_BUS_DEVICE(dev), &ged_st->io);
+
+ sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq);
+
+ if (s->memhp_state.is_enabled) {
+ /*
+ * GED handles memory hotplug event and acpi-mem-hotplug
+ * memory region gets initialized here. Create an exclusive
+ * container for memory hotplug IO and expose it as GED sysbus
+ * MMIO so that boards can map it separately.
+ */
+ memory_region_init(&s->container_memhp, OBJECT(dev), "container",
+ MEMORY_HOTPLUG_IO_LEN);
+ sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->container_memhp);
+ acpi_memory_hotplug_init(&s->container_memhp, OBJECT(dev),
+ &s->memhp_state, 0);
+ }
+}
+
+static Property acpi_ged_properties[] = {
+ DEFINE_PROP_BOOL("memory-hotplug-support", AcpiGedState,
+ memhp_state.is_enabled, true),
+ DEFINE_PROP_UINT32("ged-event", AcpiGedState, ged_event_bitmap, 0),
+ DEFINE_PROP_END_OF_LIST(),
+};
+
+static bool vmstate_test_use_memhp(void *opaque)
+{
+ AcpiGedState *s = opaque;
+ return s->memhp_state.is_enabled;
+}
+
+static const VMStateDescription vmstate_memhp_state = {
+ .name = "acpi-ged/memhp",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .needed = vmstate_test_use_memhp,
+ .fields = (VMStateField[]) {
+ VMSTATE_MEMORY_HOTPLUG(memhp_state, AcpiGedState),
+ VMSTATE_END_OF_LIST()
+ }
+};
+
+static const VMStateDescription vmstate_ged_state = {
+ .name = "acpi-ged-state",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .fields = (VMStateField[]) {
+ VMSTATE_UINT32(sel, GEDState),
+ VMSTATE_END_OF_LIST()
+ }
+};
+
+static const VMStateDescription vmstate_acpi_ged = {
+ .name = "acpi-ged",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .fields = (VMStateField[]) {
+ VMSTATE_STRUCT(ged_state, AcpiGedState, 1, vmstate_ged_state, GEDState),
+ VMSTATE_END_OF_LIST(),
+ },
+ .subsections = (const VMStateDescription * []) {
+ &vmstate_memhp_state,
+ NULL
+ }
+};
+
+static void acpi_ged_class_init(ObjectClass *class, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(class);
+ HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(class);
+ AcpiDeviceIfClass *adevc = ACPI_DEVICE_IF_CLASS(class);
+
+ dc->desc = "ACPI Generic Event Device";
+ dc->props = acpi_ged_properties;
+ dc->realize = acpi_ged_device_realize;
+ dc->vmsd = &vmstate_acpi_ged;
+
+ hc->plug = acpi_ged_device_plug_cb;
+
+ adevc->send_event = acpi_ged_send_event;
+}
+
+static const TypeInfo acpi_ged_info = {
+ .name = TYPE_ACPI_GED,
+ .parent = TYPE_SYS_BUS_DEVICE,
+ .instance_size = sizeof(AcpiGedState),
+ .class_init = acpi_ged_class_init,
+ .interfaces = (InterfaceInfo[]) {
+ { TYPE_HOTPLUG_HANDLER },
+ { TYPE_ACPI_DEVICE_IF },
+ { }
+ }
+};
+
+static void acpi_ged_register_types(void)
+{
+ type_register_static(&acpi_ged_info);
+}
+
+type_init(acpi_ged_register_types)
diff --git a/include/hw/acpi/generic_event_device.h b/include/hw/acpi/generic_event_device.h
new file mode 100644
index 0000000000..2049e8d873
--- /dev/null
+++ b/include/hw/acpi/generic_event_device.h
@@ -0,0 +1,100 @@
+/*
+ *
+ * Copyright (c) 2018 Intel Corporation
+ * Copyright (c) 2019 Huawei Technologies R & D (UK) Ltd
+ * Written by Samuel Ortiz, Shameer Kolothum
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * The ACPI Generic Event Device (GED) is a hardware-reduced specific
+ * device[ACPI v6.1 Section 5.6.9] that handles all platform events,
+ * including the hotplug ones. Generic Event Device allows platforms
+ * to handle interrupts in ACPI ASL statements. It follows a very
+ * similar approach like the _EVT method from GPIO events. All
+ * interrupts are listed in _CRS and the handler is written in _EVT
+ * method. Here, we use a single interrupt for the GED device, relying
+ * on IO memory region to communicate the type of device affected by
+ * the interrupt. This way, we can support up to 32 events with a
+ * unique interrupt.
+ *
+ * Here is an example.
+ *
+ * Device (\_SB.GED)
+ * {
+ * Name (_HID, "ACPI0013")
+ * Name (_UID, Zero)
+ * Name (_CRS, ResourceTemplate ()
+ * {
+ * Interrupt (ResourceConsumer, Edge, ActiveHigh, Exclusive, ,, )
+ * {
+ * 0x00000029,
+ * }
+ * })
+ * OperationRegion (EREG, SystemMemory, 0x09080000, 0x04)
+ * Field (EREG, DWordAcc, NoLock, WriteAsZeros)
+ * {
+ * ESEL, 32
+ * }
+ *
+ * Method (_EVT, 1, Serialized) // _EVT: Event
+ * {
+ * Local0 = ESEL // ESEL = IO memory region which specifies the
+ * // device type.
+ * If (((Local0 & One) == One))
+ * {
+ * MethodEvent1()
+ * }
+ * If ((Local0 & 0x2) == 0x2)
+ * {
+ * MethodEvent2()
+ * }
+ * ...
+ * }
+ * }
+ *
+ */
+
+#ifndef HW_ACPI_GED_H
+#define HW_ACPI_GED_H
+
+#include "hw/sysbus.h"
+#include "hw/acpi/memory_hotplug.h"
+
+#define TYPE_ACPI_GED "acpi-ged"
+#define ACPI_GED(obj) \
+ OBJECT_CHECK(AcpiGedState, (obj), TYPE_ACPI_GED)
+
+#define ACPI_GED_EVT_SEL_OFFSET 0x0
+#define ACPI_GED_EVT_SEL_LEN 0x4
+
+#define GED_DEVICE "GED"
+#define AML_GED_EVT_REG "EREG"
+#define AML_GED_EVT_SEL "ESEL"
+
+/*
+ * Platforms need to specify the GED event bitmap
+ * to describe what kind of events they want to support
+ * through GED.
+ */
+#define ACPI_GED_MEM_HOTPLUG_EVT 0x1
+
+typedef struct GEDState {
+ MemoryRegion io;
+ uint32_t sel;
+} GEDState;
+
+typedef struct AcpiGedState {
+ SysBusDevice parent_obj;
+ MemHotplugState memhp_state;
+ MemoryRegion container_memhp;
+ GEDState ged_state;
+ uint32_t ged_event_bitmap;
+ qemu_irq irq;
+} AcpiGedState;
+
+void build_ged_aml(Aml *table, const char* name, HotplugHandler *hotplug_dev,
+ uint32_t ged_irq, AmlRegionSpace rs, hwaddr ged_base);
+
+#endif
--
2.17.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Qemu-devel] [PATCH-for-4.2 v10 04/11] hw/arm/virt: Add memory hotplug framework
2019-09-04 8:56 [Qemu-devel] [PATCH-for-4.2 v10 00/11] ARM virt: ACPI memory hotplug support Shameer Kolothum
` (2 preceding siblings ...)
2019-09-04 8:56 ` [Qemu-devel] [PATCH-for-4.2 v10 03/11] hw/acpi: Add ACPI Generic Event Device Support Shameer Kolothum
@ 2019-09-04 8:56 ` Shameer Kolothum
2019-09-04 8:56 ` [Qemu-devel] [PATCH-for-4.2 v10 05/11] hw/arm/virt: Enable device memory cold/hot plug with ACPI boot Shameer Kolothum
` (6 subsequent siblings)
10 siblings, 0 replies; 29+ messages in thread
From: Shameer Kolothum @ 2019-09-04 8:56 UTC (permalink / raw)
To: qemu-devel, qemu-arm, eric.auger, imammedo
Cc: peter.maydell, sameo, ard.biesheuvel, linuxarm, xuwei5,
shannon.zhaosl, sebastien.boeuf, lersek
From: Eric Auger <eric.auger@redhat.com>
This patch adds the memory hot-plug/hot-unplug infrastructure
in machvirt. The device memory is not yet exposed to the Guest
either through DT or ACPI and hence both cold/hot plug of memory
is explicitly disabled for now.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Kwangwoo Lee <kwangwoo.lee@sk.com>
Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
---
v8 --> v9
-Added error propagation.
---
hw/arm/Kconfig | 2 ++
hw/arm/virt.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 54 insertions(+), 1 deletion(-)
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 76a2a6bcbf..39d285ad3d 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -20,6 +20,8 @@ config ARM_VIRT
select SMBIOS
select VIRTIO_MMIO
select ACPI_PCI
+ select MEM_DEVICE
+ select DIMM
config CHEETAH
bool
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 0d1629ccb3..7c492c31a0 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -68,6 +68,8 @@
#include "hw/arm/smmuv3.h"
#include "hw/acpi/acpi.h"
#include "target/arm/internals.h"
+#include "hw/mem/pc-dimm.h"
+#include "hw/mem/nvdimm.h"
#define DEFINE_VIRT_MACHINE_LATEST(major, minor, latest) \
static void virt_##major##_##minor##_class_init(ObjectClass *oc, \
@@ -1875,6 +1877,42 @@ static const CPUArchIdList *virt_possible_cpu_arch_ids(MachineState *ms)
return ms->possible_cpus;
}
+static void virt_memory_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
+ Error **errp)
+{
+
+ /*
+ * The device memory is not yet exposed to the Guest either through
+ * DT or ACPI and hence both cold/hot plug of memory is explicitly
+ * disabled for now.
+ */
+ if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
+ error_setg(errp, "memory cold/hot plug is not yet supported");
+ return;
+ }
+
+ pc_dimm_pre_plug(PC_DIMM(dev), MACHINE(hotplug_dev), NULL, errp);
+}
+
+static void virt_memory_plug(HotplugHandler *hotplug_dev,
+ DeviceState *dev, Error **errp)
+{
+ VirtMachineState *vms = VIRT_MACHINE(hotplug_dev);
+ Error *local_err = NULL;
+
+ pc_dimm_plug(PC_DIMM(dev), MACHINE(vms), &local_err);
+
+ error_propagate(errp, local_err);
+}
+
+static void virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
+ DeviceState *dev, Error **errp)
+{
+ if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
+ virt_memory_pre_plug(hotplug_dev, dev, errp);
+ }
+}
+
static void virt_machine_device_plug_cb(HotplugHandler *hotplug_dev,
DeviceState *dev, Error **errp)
{
@@ -1886,12 +1924,23 @@ static void virt_machine_device_plug_cb(HotplugHandler *hotplug_dev,
SYS_BUS_DEVICE(dev));
}
}
+ if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
+ virt_memory_plug(hotplug_dev, dev, errp);
+ }
+}
+
+static void virt_machine_device_unplug_request_cb(HotplugHandler *hotplug_dev,
+ DeviceState *dev, Error **errp)
+{
+ error_setg(errp, "device unplug request for unsupported device"
+ " type: %s", object_get_typename(OBJECT(dev)));
}
static HotplugHandler *virt_machine_get_hotplug_handler(MachineState *machine,
DeviceState *dev)
{
- if (object_dynamic_cast(OBJECT(dev), TYPE_SYS_BUS_DEVICE)) {
+ if (object_dynamic_cast(OBJECT(dev), TYPE_SYS_BUS_DEVICE) ||
+ (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM))) {
return HOTPLUG_HANDLER(machine);
}
@@ -1955,7 +2004,9 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
mc->kvm_type = virt_kvm_type;
assert(!mc->get_hotplug_handler);
mc->get_hotplug_handler = virt_machine_get_hotplug_handler;
+ hc->pre_plug = virt_machine_device_pre_plug_cb;
hc->plug = virt_machine_device_plug_cb;
+ hc->unplug_request = virt_machine_device_unplug_request_cb;
mc->numa_mem_supported = true;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Qemu-devel] [PATCH-for-4.2 v10 05/11] hw/arm/virt: Enable device memory cold/hot plug with ACPI boot
2019-09-04 8:56 [Qemu-devel] [PATCH-for-4.2 v10 00/11] ARM virt: ACPI memory hotplug support Shameer Kolothum
` (3 preceding siblings ...)
2019-09-04 8:56 ` [Qemu-devel] [PATCH-for-4.2 v10 04/11] hw/arm/virt: Add memory hotplug framework Shameer Kolothum
@ 2019-09-04 8:56 ` Shameer Kolothum
2019-09-11 12:48 ` Igor Mammedov
2019-09-11 13:07 ` Igor Mammedov
2019-09-04 8:56 ` [Qemu-devel] [PATCH-for-4.2 v10 06/11] hw/arm/virt-acpi-build: Add PC-DIMM in SRAT Shameer Kolothum
` (5 subsequent siblings)
10 siblings, 2 replies; 29+ messages in thread
From: Shameer Kolothum @ 2019-09-04 8:56 UTC (permalink / raw)
To: qemu-devel, qemu-arm, eric.auger, imammedo
Cc: peter.maydell, sameo, ard.biesheuvel, linuxarm, xuwei5,
shannon.zhaosl, sebastien.boeuf, lersek
This initializes the GED device with base memory and irq, configures
ged memory hotplug event and builds the corresponding aml code. With
this, both hot and cold plug of device memory is enabled now for Guest
with ACPI boot. Memory cold plug support with Guest DT boot is not yet
supported.
As DSDT table gets changed by this, update bios-tables-test-allowed-diff.h
to avoid "make check" failure.
Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
---
v9 --> v10
-updated commit log with bios-tables-test-allowed-diff.h info.
-Removed object_property_add_child() from create_acpi_ged().
v8 --> v9
-Changes related to GED being a TYPE_SYS_BUS_DEVICE now.
-Error propagation to _plug() handler.
-Removed R-by by Eric for now.
v7 --> v8
-Changed no_acpi_dev to no_ged.
-Fixed 'dev' reference leak by object_new().
-Updated bios-tables-test-allowed-diff.h to avoid "make check"
failure.
---
hw/arm/Kconfig | 2 +
hw/arm/virt-acpi-build.c | 16 +++++++
hw/arm/virt.c | 60 +++++++++++++++++++++++----
include/hw/arm/virt.h | 4 ++
tests/bios-tables-test-allowed-diff.h | 1 +
5 files changed, 76 insertions(+), 7 deletions(-)
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 39d285ad3d..c6e7782580 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -22,6 +22,8 @@ config ARM_VIRT
select ACPI_PCI
select MEM_DEVICE
select DIMM
+ select ACPI_MEMORY_HOTPLUG
+ select ACPI_HW_REDUCED
config CHEETAH
bool
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index a8b2d97fe9..5063f28812 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -39,6 +39,8 @@
#include "hw/acpi/aml-build.h"
#include "hw/acpi/utils.h"
#include "hw/acpi/pci.h"
+#include "hw/acpi/memory_hotplug.h"
+#include "hw/acpi/generic_event_device.h"
#include "hw/pci/pcie_host.h"
#include "hw/pci/pci.h"
#include "hw/arm/virt.h"
@@ -706,6 +708,7 @@ static void
build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
{
Aml *scope, *dsdt;
+ MachineState *ms = MACHINE(vms);
const MemMapEntry *memmap = vms->memmap;
const int *irqmap = vms->irqmap;
@@ -730,6 +733,19 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
vms->highmem, vms->highmem_ecam);
acpi_dsdt_add_gpio(scope, &memmap[VIRT_GPIO],
(irqmap[VIRT_GPIO] + ARM_SPI_BASE));
+ if (vms->acpi_dev) {
+ build_ged_aml(scope, "\\_SB."GED_DEVICE,
+ HOTPLUG_HANDLER(vms->acpi_dev),
+ irqmap[VIRT_ACPI_GED] + ARM_SPI_BASE, AML_SYSTEM_MEMORY,
+ memmap[VIRT_ACPI_GED].base);
+ }
+
+ if (vms->acpi_dev && ms->ram_slots) {
+ build_memory_hotplug_aml(scope, ms->ram_slots, "\\_SB", NULL,
+ AML_SYSTEM_MEMORY,
+ memmap[VIRT_PCDIMM_ACPI].base);
+ }
+
acpi_dsdt_add_power_button(scope);
aml_append(dsdt, scope);
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 7c492c31a0..b99c12ad9c 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -70,6 +70,7 @@
#include "target/arm/internals.h"
#include "hw/mem/pc-dimm.h"
#include "hw/mem/nvdimm.h"
+#include "hw/acpi/generic_event_device.h"
#define DEFINE_VIRT_MACHINE_LATEST(major, minor, latest) \
static void virt_##major##_##minor##_class_init(ObjectClass *oc, \
@@ -140,6 +141,8 @@ static const MemMapEntry base_memmap[] = {
[VIRT_GPIO] = { 0x09030000, 0x00001000 },
[VIRT_SECURE_UART] = { 0x09040000, 0x00001000 },
[VIRT_SMMU] = { 0x09050000, 0x00020000 },
+ [VIRT_PCDIMM_ACPI] = { 0x09070000, MEMORY_HOTPLUG_IO_LEN },
+ [VIRT_ACPI_GED] = { 0x09080000, ACPI_GED_EVT_SEL_LEN },
[VIRT_MMIO] = { 0x0a000000, 0x00000200 },
/* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */
[VIRT_PLATFORM_BUS] = { 0x0c000000, 0x02000000 },
@@ -175,6 +178,7 @@ static const int a15irqmap[] = {
[VIRT_PCIE] = 3, /* ... to 6 */
[VIRT_GPIO] = 7,
[VIRT_SECURE_UART] = 8,
+ [VIRT_ACPI_GED] = 9,
[VIRT_MMIO] = 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */
[VIRT_GIC_V2M] = 48, /* ...to 48 + NUM_GICV2M_SPIS - 1 */
[VIRT_SMMU] = 74, /* ...to 74 + NUM_SMMU_IRQS - 1 */
@@ -524,6 +528,24 @@ static void fdt_add_pmu_nodes(const VirtMachineState *vms)
}
}
+static inline DeviceState *create_acpi_ged(VirtMachineState *vms, qemu_irq *pic)
+{
+ DeviceState *dev;
+ int irq = vms->irqmap[VIRT_ACPI_GED];
+ uint32_t event = ACPI_GED_MEM_HOTPLUG_EVT;
+
+ dev = qdev_create(NULL, TYPE_ACPI_GED);
+ qdev_prop_set_uint32(dev, "ged-event", event);
+ qdev_init_nofail(dev);
+
+ sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, vms->memmap[VIRT_ACPI_GED].base);
+ sysbus_mmio_map(SYS_BUS_DEVICE(dev), 1, vms->memmap[VIRT_PCDIMM_ACPI].base);
+
+ sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[irq]);
+
+ return dev;
+}
+
static void create_its(VirtMachineState *vms, DeviceState *gicdev)
{
const char *itsclass = its_class_name();
@@ -1487,6 +1509,7 @@ static void machvirt_init(MachineState *machine)
MemoryRegion *ram = g_new(MemoryRegion, 1);
bool firmware_loaded;
bool aarch64 = true;
+ bool has_ged = !vmc->no_ged;
unsigned int smp_cpus = machine->smp.cpus;
unsigned int max_cpus = machine->smp.max_cpus;
@@ -1701,6 +1724,10 @@ static void machvirt_init(MachineState *machine)
create_gpio(vms, pic);
+ if (has_ged && aarch64 && firmware_loaded && acpi_enabled) {
+ vms->acpi_dev = create_acpi_ged(vms, pic);
+ }
+
/* Create mmio transports, so the user can create virtio backends
* (which will be automatically plugged in to the transports). If
* no backend is created the transport will just sit harmlessly idle.
@@ -1880,14 +1907,23 @@ static const CPUArchIdList *virt_possible_cpu_arch_ids(MachineState *ms)
static void virt_memory_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
Error **errp)
{
+ VirtMachineState *vms = VIRT_MACHINE(hotplug_dev);
+ const bool is_nvdimm = object_dynamic_cast(OBJECT(dev), TYPE_NVDIMM);
+ Error *local_err = NULL;
- /*
- * The device memory is not yet exposed to the Guest either through
- * DT or ACPI and hence both cold/hot plug of memory is explicitly
- * disabled for now.
- */
- if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
- error_setg(errp, "memory cold/hot plug is not yet supported");
+ if (is_nvdimm) {
+ error_setg(errp, "nvdimm is not yet supported");
+ return;
+ }
+
+ if (!vms->acpi_dev) {
+ error_setg(errp, "memory hotplug is not enabled: missing acpi device");
+ return;
+ }
+
+ hotplug_handler_pre_plug(HOTPLUG_HANDLER(vms->acpi_dev), dev, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
return;
}
@@ -1897,11 +1933,18 @@ static void virt_memory_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
static void virt_memory_plug(HotplugHandler *hotplug_dev,
DeviceState *dev, Error **errp)
{
+ HotplugHandlerClass *hhc;
VirtMachineState *vms = VIRT_MACHINE(hotplug_dev);
Error *local_err = NULL;
pc_dimm_plug(PC_DIMM(dev), MACHINE(vms), &local_err);
+ if (local_err) {
+ goto out;
+ }
+ hhc = HOTPLUG_HANDLER_GET_CLASS(vms->acpi_dev);
+ hhc->plug(HOTPLUG_HANDLER(vms->acpi_dev), dev, &local_err);
+out:
error_propagate(errp, local_err);
}
@@ -2108,8 +2151,11 @@ DEFINE_VIRT_MACHINE_AS_LATEST(4, 2)
static void virt_machine_4_1_options(MachineClass *mc)
{
+ VirtMachineClass *vmc = VIRT_MACHINE_CLASS(OBJECT_CLASS(mc));
+
virt_machine_4_2_options(mc);
compat_props_add(mc->compat_props, hw_compat_4_1, hw_compat_4_1_len);
+ vmc->no_ged = true;
}
DEFINE_VIRT_MACHINE(4, 1)
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index a72094204e..577ee49b4b 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -77,6 +77,8 @@ enum {
VIRT_GPIO,
VIRT_SECURE_UART,
VIRT_SECURE_MEM,
+ VIRT_PCDIMM_ACPI,
+ VIRT_ACPI_GED,
VIRT_LOWMEMMAP_LAST,
};
@@ -106,6 +108,7 @@ typedef struct {
bool claim_edge_triggered_timers;
bool smbios_old_sys_ver;
bool no_highmem_ecam;
+ bool no_ged; /* Machines < 4.2 has no support for ACPI GED device */
} VirtMachineClass;
typedef struct {
@@ -133,6 +136,7 @@ typedef struct {
uint32_t iommu_phandle;
int psci_conduit;
hwaddr highest_gpa;
+ DeviceState *acpi_dev;
} VirtMachineState;
#define VIRT_ECAM_ID(high) (high ? VIRT_HIGH_PCIE_ECAM : VIRT_PCIE_ECAM)
diff --git a/tests/bios-tables-test-allowed-diff.h b/tests/bios-tables-test-allowed-diff.h
index dfb8523c8b..7b4adbc822 100644
--- a/tests/bios-tables-test-allowed-diff.h
+++ b/tests/bios-tables-test-allowed-diff.h
@@ -1 +1,2 @@
/* List of comma-separated changed AML files to ignore */
+"tests/data/acpi/virt/DSDT",
--
2.17.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Qemu-devel] [PATCH-for-4.2 v10 06/11] hw/arm/virt-acpi-build: Add PC-DIMM in SRAT
2019-09-04 8:56 [Qemu-devel] [PATCH-for-4.2 v10 00/11] ARM virt: ACPI memory hotplug support Shameer Kolothum
` (4 preceding siblings ...)
2019-09-04 8:56 ` [Qemu-devel] [PATCH-for-4.2 v10 05/11] hw/arm/virt: Enable device memory cold/hot plug with ACPI boot Shameer Kolothum
@ 2019-09-04 8:56 ` Shameer Kolothum
2019-09-11 13:14 ` Igor Mammedov
2019-09-04 8:56 ` [Qemu-devel] [PATCH-for-4.2 v10 07/11] hw/arm: Factor out powerdown notifier from GPIO Shameer Kolothum
` (4 subsequent siblings)
10 siblings, 1 reply; 29+ messages in thread
From: Shameer Kolothum @ 2019-09-04 8:56 UTC (permalink / raw)
To: qemu-devel, qemu-arm, eric.auger, imammedo
Cc: peter.maydell, sameo, ard.biesheuvel, linuxarm, xuwei5,
shannon.zhaosl, sebastien.boeuf, lersek
Generate Memory Affinity Structures for PC-DIMM ranges.
Also, Linux and Windows need ACPI SRAT table to make memory hotplug
work properly, however currently QEMU doesn't create SRAT table if
numa options aren't present on CLI. Hence add support(>=4.2) to
create numa node automatically (auto_enable_numa_with_memhp) when
QEMU is started with memory hotplug enabled but without '-numa'
options on CLI.
Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
Signed-off-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
---
v8 --> v9
- Added auto_enable_numa_with_memhp support.
---
hw/arm/virt-acpi-build.c | 9 +++++++++
hw/arm/virt.c | 2 ++
2 files changed, 11 insertions(+)
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 5063f28812..a4872a0b00 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -519,6 +519,7 @@ build_srat(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
int i, srat_start;
uint64_t mem_base;
MachineClass *mc = MACHINE_GET_CLASS(vms);
+ MachineState *ms = MACHINE(vms);
const CPUArchIdList *cpu_list = mc->possible_cpu_arch_ids(MACHINE(vms));
srat_start = table_data->len;
@@ -544,6 +545,14 @@ build_srat(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
}
}
+ if (ms->device_memory) {
+ numamem = acpi_data_push(table_data, sizeof *numamem);
+ build_srat_memory(numamem, ms->device_memory->base,
+ memory_region_size(&ms->device_memory->mr),
+ nb_numa_nodes - 1,
+ MEM_AFFINITY_HOTPLUGGABLE | MEM_AFFINITY_ENABLED);
+ }
+
build_header(linker, table_data, (void *)(table_data->data + srat_start),
"SRAT", table_data->len - srat_start, 3, NULL, NULL);
}
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 29bc2837ce..95accc28f1 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2053,6 +2053,7 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
hc->plug = virt_machine_device_plug_cb;
hc->unplug_request = virt_machine_device_unplug_request_cb;
mc->numa_mem_supported = true;
+ mc->auto_enable_numa_with_memhp = true;
}
static void virt_instance_init(Object *obj)
@@ -2158,6 +2159,7 @@ static void virt_machine_4_1_options(MachineClass *mc)
virt_machine_4_2_options(mc);
compat_props_add(mc->compat_props, hw_compat_4_1, hw_compat_4_1_len);
vmc->no_ged = true;
+ mc->auto_enable_numa_with_memhp = false;
}
DEFINE_VIRT_MACHINE(4, 1)
--
2.17.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Qemu-devel] [PATCH-for-4.2 v10 07/11] hw/arm: Factor out powerdown notifier from GPIO
2019-09-04 8:56 [Qemu-devel] [PATCH-for-4.2 v10 00/11] ARM virt: ACPI memory hotplug support Shameer Kolothum
` (5 preceding siblings ...)
2019-09-04 8:56 ` [Qemu-devel] [PATCH-for-4.2 v10 06/11] hw/arm/virt-acpi-build: Add PC-DIMM in SRAT Shameer Kolothum
@ 2019-09-04 8:56 ` Shameer Kolothum
2019-09-11 13:08 ` Igor Mammedov
2019-09-04 8:56 ` [Qemu-devel] [PATCH-for-4.2 v10 08/11] hw/arm: Use GED for system_powerdown event Shameer Kolothum
` (3 subsequent siblings)
10 siblings, 1 reply; 29+ messages in thread
From: Shameer Kolothum @ 2019-09-04 8:56 UTC (permalink / raw)
To: qemu-devel, qemu-arm, eric.auger, imammedo
Cc: peter.maydell, sameo, ard.biesheuvel, linuxarm, xuwei5,
shannon.zhaosl, sebastien.boeuf, lersek
This is in preparation of using GED device for
system_powerdown event. Make the powerdown notifier
registration independent of create_gpio() fn.
Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
---
hw/arm/virt.c | 12 ++++--------
include/hw/arm/virt.h | 1 +
2 files changed, 5 insertions(+), 8 deletions(-)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 95accc28f1..89ee37d7de 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -791,10 +791,6 @@ static void virt_powerdown_req(Notifier *n, void *opaque)
qemu_set_irq(qdev_get_gpio_in(gpio_key_dev, 0), 1);
}
-static Notifier virt_system_powerdown_notifier = {
- .notify = virt_powerdown_req
-};
-
static void create_gpio(const VirtMachineState *vms, qemu_irq *pic)
{
char *nodename;
@@ -835,10 +831,6 @@ static void create_gpio(const VirtMachineState *vms, qemu_irq *pic)
KEY_POWER);
qemu_fdt_setprop_cells(vms->fdt, "/gpio-keys/poweroff",
"gpios", phandle, 3, 0);
-
- /* connect powerdown request */
- qemu_register_powerdown_notifier(&virt_system_powerdown_notifier);
-
g_free(nodename);
}
@@ -1730,6 +1722,10 @@ static void machvirt_init(MachineState *machine)
vms->acpi_dev = create_acpi_ged(vms, pic);
}
+ /* connect powerdown request */
+ vms->powerdown_notifier.notify = virt_powerdown_req;
+ qemu_register_powerdown_notifier(&vms->powerdown_notifier);
+
/* Create mmio transports, so the user can create virtio backends
* (which will be automatically plugged in to the transports). If
* no backend is created the transport will just sit harmlessly idle.
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index 577ee49b4b..0b41083e9d 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -137,6 +137,7 @@ typedef struct {
int psci_conduit;
hwaddr highest_gpa;
DeviceState *acpi_dev;
+ Notifier powerdown_notifier;
} VirtMachineState;
#define VIRT_ECAM_ID(high) (high ? VIRT_HIGH_PCIE_ECAM : VIRT_PCIE_ECAM)
--
2.17.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Qemu-devel] [PATCH-for-4.2 v10 08/11] hw/arm: Use GED for system_powerdown event
2019-09-04 8:56 [Qemu-devel] [PATCH-for-4.2 v10 00/11] ARM virt: ACPI memory hotplug support Shameer Kolothum
` (6 preceding siblings ...)
2019-09-04 8:56 ` [Qemu-devel] [PATCH-for-4.2 v10 07/11] hw/arm: Factor out powerdown notifier from GPIO Shameer Kolothum
@ 2019-09-04 8:56 ` Shameer Kolothum
2019-09-11 13:20 ` Igor Mammedov
2019-09-04 8:56 ` [Qemu-devel] [PATCH-for-4.2 v10 09/11] docs/specs: Add ACPI GED documentation Shameer Kolothum
` (2 subsequent siblings)
10 siblings, 1 reply; 29+ messages in thread
From: Shameer Kolothum @ 2019-09-04 8:56 UTC (permalink / raw)
To: qemu-devel, qemu-arm, eric.auger, imammedo
Cc: peter.maydell, sameo, ard.biesheuvel, linuxarm, xuwei5,
shannon.zhaosl, sebastien.boeuf, lersek
For machines 4.2 or higher with ACPI boot use GED for system_powerdown
event instead of GPIO. Guest boot with DT still uses GPIO.
Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
---
v9 --> v10
-Eric's R-by
v8 --> v9
-Re-arranged patches 8 & 9 from v8 based on Igor's comments.
v7 --> v8
-Retained gpio based system_powerdown support for machines < 4.2.
-Reuse of virt_powerdown_req() for ACPI GED use.
-Dropped Eric's R-by for now because of above.
---
hw/acpi/generic_event_device.c | 8 ++++++++
hw/arm/virt-acpi-build.c | 6 +++---
hw/arm/virt.c | 16 +++++++++++-----
include/hw/acpi/acpi_dev_interface.h | 1 +
include/hw/acpi/generic_event_device.h | 3 +++
5 files changed, 26 insertions(+), 8 deletions(-)
diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
index f81d966058..47e178c583 100644
--- a/hw/acpi/generic_event_device.c
+++ b/hw/acpi/generic_event_device.c
@@ -22,6 +22,7 @@
static const uint32_t ged_supported_events[] = {
ACPI_GED_MEM_HOTPLUG_EVT,
+ ACPI_GED_PWR_DOWN_EVT,
};
/*
@@ -104,6 +105,11 @@ void build_ged_aml(Aml *table, const char *name, HotplugHandler *hotplug_dev,
aml_append(if_ctx, aml_call0(MEMORY_DEVICES_CONTAINER "."
MEMORY_SLOT_SCAN_METHOD));
break;
+ case ACPI_GED_PWR_DOWN_EVT:
+ aml_append(if_ctx,
+ aml_notify(aml_name(ACPI_POWER_BUTTON_DEVICE),
+ aml_int(0x80)));
+ break;
default:
/*
* Please make sure all the events in ged_supported_events[]
@@ -190,6 +196,8 @@ static void acpi_ged_send_event(AcpiDeviceIf *adev, AcpiEventStatusBits ev)
if (ev & ACPI_MEMORY_HOTPLUG_STATUS) {
sel = ACPI_GED_MEM_HOTPLUG_EVT;
+ } else if (ev & ACPI_POWER_DOWN_STATUS) {
+ sel = ACPI_GED_PWR_DOWN_EVT;
} else {
/* Unknown event. Return without generating interrupt. */
warn_report("GED: Unsupported event %d. No irq injected", ev);
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index a4872a0b00..bbf7f6fafa 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -50,7 +50,6 @@
#include "migration/vmstate.h"
#define ARM_SPI_BASE 32
-#define ACPI_POWER_BUTTON_DEVICE "PWRB"
static void acpi_dsdt_add_cpus(Aml *scope, int smp_cpus)
{
@@ -740,13 +739,14 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
(irqmap[VIRT_MMIO] + ARM_SPI_BASE), NUM_VIRTIO_TRANSPORTS);
acpi_dsdt_add_pci(scope, memmap, (irqmap[VIRT_PCIE] + ARM_SPI_BASE),
vms->highmem, vms->highmem_ecam);
- acpi_dsdt_add_gpio(scope, &memmap[VIRT_GPIO],
- (irqmap[VIRT_GPIO] + ARM_SPI_BASE));
if (vms->acpi_dev) {
build_ged_aml(scope, "\\_SB."GED_DEVICE,
HOTPLUG_HANDLER(vms->acpi_dev),
irqmap[VIRT_ACPI_GED] + ARM_SPI_BASE, AML_SYSTEM_MEMORY,
memmap[VIRT_ACPI_GED].base);
+ } else {
+ acpi_dsdt_add_gpio(scope, &memmap[VIRT_GPIO],
+ (irqmap[VIRT_GPIO] + ARM_SPI_BASE));
}
if (vms->acpi_dev && ms->ram_slots) {
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 89ee37d7de..32208e1058 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -532,7 +532,7 @@ static inline DeviceState *create_acpi_ged(VirtMachineState *vms, qemu_irq *pic)
{
DeviceState *dev;
int irq = vms->irqmap[VIRT_ACPI_GED];
- uint32_t event = ACPI_GED_MEM_HOTPLUG_EVT;
+ uint32_t event = ACPI_GED_MEM_HOTPLUG_EVT | ACPI_GED_PWR_DOWN_EVT;
dev = qdev_create(NULL, TYPE_ACPI_GED);
qdev_prop_set_uint32(dev, "ged-event", event);
@@ -787,8 +787,14 @@ static void create_rtc(const VirtMachineState *vms, qemu_irq *pic)
static DeviceState *gpio_key_dev;
static void virt_powerdown_req(Notifier *n, void *opaque)
{
- /* use gpio Pin 3 for power button event */
- qemu_set_irq(qdev_get_gpio_in(gpio_key_dev, 0), 1);
+ VirtMachineState *s = container_of(n, VirtMachineState, powerdown_notifier);
+
+ if (s->acpi_dev) {
+ acpi_send_event(s->acpi_dev, ACPI_POWER_DOWN_STATUS);
+ } else {
+ /* use gpio Pin 3 for power button event */
+ qemu_set_irq(qdev_get_gpio_in(gpio_key_dev, 0), 1);
+ }
}
static void create_gpio(const VirtMachineState *vms, qemu_irq *pic)
@@ -1716,10 +1722,10 @@ static void machvirt_init(MachineState *machine)
create_pcie(vms, pic);
- create_gpio(vms, pic);
-
if (has_ged && aarch64 && firmware_loaded && acpi_enabled) {
vms->acpi_dev = create_acpi_ged(vms, pic);
+ } else {
+ create_gpio(vms, pic);
}
/* connect powerdown request */
diff --git a/include/hw/acpi/acpi_dev_interface.h b/include/hw/acpi/acpi_dev_interface.h
index 0ba90effd2..a2a12af9b9 100644
--- a/include/hw/acpi/acpi_dev_interface.h
+++ b/include/hw/acpi/acpi_dev_interface.h
@@ -13,6 +13,7 @@ typedef enum {
ACPI_MEMORY_HOTPLUG_STATUS = 8,
ACPI_NVDIMM_HOTPLUG_STATUS = 16,
ACPI_VMGENID_CHANGE_STATUS = 32,
+ ACPI_POWER_DOWN_STATUS = 64,
} AcpiEventStatusBits;
#define TYPE_ACPI_DEVICE_IF "acpi-device-interface"
diff --git a/include/hw/acpi/generic_event_device.h b/include/hw/acpi/generic_event_device.h
index 2049e8d873..d157eac088 100644
--- a/include/hw/acpi/generic_event_device.h
+++ b/include/hw/acpi/generic_event_device.h
@@ -62,6 +62,8 @@
#include "hw/sysbus.h"
#include "hw/acpi/memory_hotplug.h"
+#define ACPI_POWER_BUTTON_DEVICE "PWRB"
+
#define TYPE_ACPI_GED "acpi-ged"
#define ACPI_GED(obj) \
OBJECT_CHECK(AcpiGedState, (obj), TYPE_ACPI_GED)
@@ -79,6 +81,7 @@
* through GED.
*/
#define ACPI_GED_MEM_HOTPLUG_EVT 0x1
+#define ACPI_GED_PWR_DOWN_EVT 0x2
typedef struct GEDState {
MemoryRegion io;
--
2.17.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Qemu-devel] [PATCH-for-4.2 v10 09/11] docs/specs: Add ACPI GED documentation
2019-09-04 8:56 [Qemu-devel] [PATCH-for-4.2 v10 00/11] ARM virt: ACPI memory hotplug support Shameer Kolothum
` (7 preceding siblings ...)
2019-09-04 8:56 ` [Qemu-devel] [PATCH-for-4.2 v10 08/11] hw/arm: Use GED for system_powerdown event Shameer Kolothum
@ 2019-09-04 8:56 ` Shameer Kolothum
2019-09-11 12:54 ` Peter Maydell
2019-09-04 8:56 ` [Qemu-devel] [PATCH-for-4.2 v10 10/11] tests: add dummy ACPI tables for arm/virt board Shameer Kolothum
2019-09-04 8:56 ` [Qemu-devel] [PATCH-for-4.2 v10 11/11] tests: Add bios tests to arm/virt Shameer Kolothum
10 siblings, 1 reply; 29+ messages in thread
From: Shameer Kolothum @ 2019-09-04 8:56 UTC (permalink / raw)
To: qemu-devel, qemu-arm, eric.auger, imammedo
Cc: peter.maydell, sameo, ard.biesheuvel, linuxarm, xuwei5,
shannon.zhaosl, sebastien.boeuf, lersek
Documents basic concepts of ACPI Generic Event device(GED)
and interface between QEMU and the ACPI BIOS.
Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
---
docs/specs/acpi_hw_reduced_hotplug.txt | 60 ++++++++++++++++++++++++++
1 file changed, 60 insertions(+)
create mode 100644 docs/specs/acpi_hw_reduced_hotplug.txt
diff --git a/docs/specs/acpi_hw_reduced_hotplug.txt b/docs/specs/acpi_hw_reduced_hotplug.txt
new file mode 100644
index 0000000000..2d78c40d6a
--- /dev/null
+++ b/docs/specs/acpi_hw_reduced_hotplug.txt
@@ -0,0 +1,60 @@
+QEMU<->ACPI BIOS Generic Event Device interface
+
+The ACPI Generic Event Device (GED) is a HW reduced platform
+specific device introduced in ACPI v6.1 that handles all platform
+events, including the hotplug ones. GED is modelled as a device
+in the namespace with a _HID defined to be ACPI0013. This document
+describes the interface between QEMU and the ACPI BIOS.
+
+GED allows HW reduced platforms to handle interrupts in ACPI ASL
+statements. It follows a very similar approach like the _EVT method
+from GPIO events. All interrupts are listed in _CRS and the handler
+is written in _EVT method. However, Qemu implementation uses a single
+interrupt for the GED device, relying on IO memory region to communicate
+the type of device affected by the interrupt. This way, we can support
+up to 32 events with a unique interrupt.
+
+Here is an example.
+
+Device (\_SB.GED)
+{
+ Name (_HID, "ACPI0013")
+ Name (_UID, Zero)
+ Name (_CRS, ResourceTemplate ()
+ {
+ Interrupt (ResourceConsumer, Edge, ActiveHigh, Exclusive, ,, )
+ {
+ 0x00000029,
+ }
+ })
+ OperationRegion (EREG, SystemMemory, 0x09080000, 0x04)
+ Field (EREG, DWordAcc, NoLock, WriteAsZeros)
+ {
+ ESEL, 32
+ }
+ Method (_EVT, 1, Serialized)
+ {
+ Local0 = ESEL // ESEL = IO memory region which specifies the
+ // device type.
+ If (((Local0 & One) == One))
+ {
+ MethodEvent1()
+ }
+ If ((Local0 & 0x2) == 0x2)
+ {
+ MethodEvent2()
+ }
+ ...
+ }
+}
+
+GED IO interface (4 byte access):
+read access:
+ [0x0-0x3] Event selector bit field(32 bit) set by Qemu.
+ bits:
+ 0: Memory hotplug event
+ 1: System power down event
+ 2-31: Reserved
+
+write_access:
+ Nothing is expected to be written into GED IO memory
--
2.17.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Qemu-devel] [PATCH-for-4.2 v10 10/11] tests: add dummy ACPI tables for arm/virt board
2019-09-04 8:56 [Qemu-devel] [PATCH-for-4.2 v10 00/11] ARM virt: ACPI memory hotplug support Shameer Kolothum
` (8 preceding siblings ...)
2019-09-04 8:56 ` [Qemu-devel] [PATCH-for-4.2 v10 09/11] docs/specs: Add ACPI GED documentation Shameer Kolothum
@ 2019-09-04 8:56 ` Shameer Kolothum
2019-09-11 12:57 ` Peter Maydell
2019-09-04 8:56 ` [Qemu-devel] [PATCH-for-4.2 v10 11/11] tests: Add bios tests to arm/virt Shameer Kolothum
10 siblings, 1 reply; 29+ messages in thread
From: Shameer Kolothum @ 2019-09-04 8:56 UTC (permalink / raw)
To: qemu-devel, qemu-arm, eric.auger, imammedo
Cc: peter.maydell, sameo, ard.biesheuvel, linuxarm, xuwei5,
shannon.zhaosl, sebastien.boeuf, lersek
This patch is in preparation for adding numamem and memhp tests
to arm/virt board so that 'make check' is happy. This may not
be required once the scripts are run and new tables are
generated with ".numamem" and ".memhp" extensions.
Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
---
I am not sure this is the right way to do this. But without this, when
the numamem and memhp tests are added, you will get,
Looking for expected file 'tests/data/acpi/virt/SRAT.numamem'
Looking for expected file 'tests/data/acpi/virt/SRAT'
**
ERROR:tests/bios-tables-test.c:327:load_expected_aml: assertion failed: (exp_sdt.aml_file)
---
tests/data/acpi/virt/SLIT | Bin 0 -> 48 bytes
tests/data/acpi/virt/SRAT | Bin 0 -> 224 bytes
2 files changed, 0 insertions(+), 0 deletions(-)
create mode 100644 tests/data/acpi/virt/SLIT
create mode 100644 tests/data/acpi/virt/SRAT
diff --git a/tests/data/acpi/virt/SLIT b/tests/data/acpi/virt/SLIT
new file mode 100644
index 0000000000000000000000000000000000000000..74ec3b4b461ffecca36d8537975c202a5f011185
GIT binary patch
literal 48
scmWIc@eDCwU|?X>aq@Te2v%^42yhMtiZKGkKx`1r1jHb~B`V4V0NaKK0RR91
literal 0
HcmV?d00001
diff --git a/tests/data/acpi/virt/SRAT b/tests/data/acpi/virt/SRAT
new file mode 100644
index 0000000000000000000000000000000000000000..119922f4973f621602047d1dc160519f810922a3
GIT binary patch
literal 224
zcmWFzatwLEz`(%x)yd!4BUr&HBEUHqD8>jB1F=Cg2*ZH@DxXmUMHZ-x3$7Gd2B8jU
X02q8=hbcr=2NT6lGiu<Mhsgo}c|r;S
literal 0
HcmV?d00001
--
2.17.1
^ permalink raw reply [flat|nested] 29+ messages in thread
* [Qemu-devel] [PATCH-for-4.2 v10 11/11] tests: Add bios tests to arm/virt
2019-09-04 8:56 [Qemu-devel] [PATCH-for-4.2 v10 00/11] ARM virt: ACPI memory hotplug support Shameer Kolothum
` (9 preceding siblings ...)
2019-09-04 8:56 ` [Qemu-devel] [PATCH-for-4.2 v10 10/11] tests: add dummy ACPI tables for arm/virt board Shameer Kolothum
@ 2019-09-04 8:56 ` Shameer Kolothum
2019-09-11 13:46 ` Igor Mammedov
10 siblings, 1 reply; 29+ messages in thread
From: Shameer Kolothum @ 2019-09-04 8:56 UTC (permalink / raw)
To: qemu-devel, qemu-arm, eric.auger, imammedo
Cc: peter.maydell, sameo, ard.biesheuvel, linuxarm, xuwei5,
shannon.zhaosl, sebastien.boeuf, lersek
This adds numamem and memhp tests for arm/virt platform
Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
---
tests/bios-tables-test-allowed-diff.h | 1 +
tests/bios-tables-test.c | 49 +++++++++++++++++++++++++++
2 files changed, 50 insertions(+)
diff --git a/tests/bios-tables-test-allowed-diff.h b/tests/bios-tables-test-allowed-diff.h
index 7b4adbc822..d181a4da4a 100644
--- a/tests/bios-tables-test-allowed-diff.h
+++ b/tests/bios-tables-test-allowed-diff.h
@@ -1,2 +1,3 @@
/* List of comma-separated changed AML files to ignore */
"tests/data/acpi/virt/DSDT",
+"tests/data/acpi/virt/SRAT",
diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
index a356ac3489..1d6f330d53 100644
--- a/tests/bios-tables-test.c
+++ b/tests/bios-tables-test.c
@@ -871,6 +871,53 @@ static void test_acpi_piix4_tcg_dimm_pxm(void)
test_acpi_tcg_dimm_pxm(MACHINE_PC);
}
+static void test_acpi_virt_tcg_memhp(void)
+{
+ test_data data = {
+ .machine = "virt",
+ .accel = "tcg",
+ .uefi_fl1 = "pc-bios/edk2-aarch64-code.fd",
+ .uefi_fl2 = "pc-bios/edk2-arm-vars.fd",
+ .cd = "tests/data/uefi-boot-images/bios-tables-test.aarch64.iso.qcow2",
+ .ram_start = 0x40000000ULL,
+ .scan_len = 256ULL * 1024 * 1024,
+ };
+
+ data.variant = ".memhp";
+ test_acpi_one(" -cpu cortex-a57"
+ " -m 256M,slots=3,maxmem=1G"
+ " -object memory-backend-ram,id=ram0,size=128M"
+ " -object memory-backend-ram,id=ram1,size=128M"
+ " -numa node,memdev=ram0 -numa node,memdev=ram1"
+ " -numa dist,src=0,dst=1,val=21",
+ &data);
+
+ free_test_data(&data);
+
+}
+
+static void test_acpi_virt_tcg_numamem(void)
+{
+ test_data data = {
+ .machine = "virt",
+ .accel = "tcg",
+ .uefi_fl1 = "pc-bios/edk2-aarch64-code.fd",
+ .uefi_fl2 = "pc-bios/edk2-arm-vars.fd",
+ .cd = "tests/data/uefi-boot-images/bios-tables-test.aarch64.iso.qcow2",
+ .ram_start = 0x40000000ULL,
+ .scan_len = 128ULL * 1024 * 1024,
+ };
+
+ data.variant = ".numamem";
+ test_acpi_one(" -cpu cortex-a57"
+ " -object memory-backend-ram,id=ram0,size=128M"
+ " -numa node,memdev=ram0",
+ &data);
+
+ free_test_data(&data);
+
+}
+
static void test_acpi_virt_tcg(void)
{
test_data data = {
@@ -917,6 +964,8 @@ int main(int argc, char *argv[])
qtest_add_func("acpi/q35/dimmpxm", test_acpi_q35_tcg_dimm_pxm);
} else if (strcmp(arch, "aarch64") == 0) {
qtest_add_func("acpi/virt", test_acpi_virt_tcg);
+ qtest_add_func("acpi/virt/numamem", test_acpi_virt_tcg_numamem);
+ qtest_add_func("acpi/virt/memhp", test_acpi_virt_tcg_memhp);
}
ret = g_test_run();
boot_sector_cleanup(disk);
--
2.17.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH-for-4.2 v10 01/11] hw/acpi: Make ACPI IO address space configurable
2019-09-04 8:56 ` [Qemu-devel] [PATCH-for-4.2 v10 01/11] hw/acpi: Make ACPI IO address space configurable Shameer Kolothum
@ 2019-09-04 11:27 ` Igor Mammedov
0 siblings, 0 replies; 29+ messages in thread
From: Igor Mammedov @ 2019-09-04 11:27 UTC (permalink / raw)
To: Shameer Kolothum
Cc: peter.maydell, sameo, shannon.zhaosl, ard.biesheuvel, qemu-devel,
xuwei5, linuxarm, eric.auger, qemu-arm, sebastien.boeuf, lersek
On Wed, 4 Sep 2019 09:56:19 +0100
Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> wrote:
> This is in preparation for adding support for ARM64 platforms
> where it doesn't use port mapped IO for ACPI IO space. We are
> making changes so that MMIO region can be accommodated
> and board can pass the base address into the aml build function.
>
> Also move few MEMORY_* definitions to header so that other memory
> hotplug event signalling mechanisms (eg. Generic Event Device on
> HW-reduced acpi platforms) can use the same from their respective
> event handler code.
>
> Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
> Reviewed-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> ---
> v9 --> v10
> - Fixed "make check" failure on x86_64.
>
> v8 --> v9
> -base address is an input into build_memory_hotplug_aml()
> -Removed R-by tags from Igor and Eric for now.
> ---
> hw/acpi/memory_hotplug.c | 33 ++++++++++++++------------------
> hw/i386/acpi-build.c | 7 ++++++-
> hw/i386/pc.c | 3 +++
> include/hw/acpi/memory_hotplug.h | 9 +++++++--
> include/hw/i386/pc.h | 3 +++
> 5 files changed, 33 insertions(+), 22 deletions(-)
>
> diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
> index 9483d66e86..9b0b150f4f 100644
> --- a/hw/acpi/memory_hotplug.c
> +++ b/hw/acpi/memory_hotplug.c
> @@ -30,12 +30,7 @@
> #define MEMORY_SLOT_PROXIMITY_METHOD "MPXM"
> #define MEMORY_SLOT_EJECT_METHOD "MEJ0"
> #define MEMORY_SLOT_NOTIFY_METHOD "MTFY"
> -#define MEMORY_SLOT_SCAN_METHOD "MSCN"
> #define MEMORY_HOTPLUG_DEVICE "MHPD"
> -#define MEMORY_HOTPLUG_IO_LEN 24
> -#define MEMORY_DEVICES_CONTAINER "\\_SB.MHPC"
> -
> -static uint16_t memhp_io_base;
>
> static ACPIOSTInfo *acpi_memory_device_status(int slot, MemStatus *mdev)
> {
> @@ -210,7 +205,7 @@ static const MemoryRegionOps acpi_memory_hotplug_ops = {
> };
>
> void acpi_memory_hotplug_init(MemoryRegion *as, Object *owner,
> - MemHotplugState *state, uint16_t io_base)
> + MemHotplugState *state, hwaddr io_base)
> {
> MachineState *machine = MACHINE(qdev_get_machine());
>
> @@ -219,12 +214,10 @@ void acpi_memory_hotplug_init(MemoryRegion *as, Object *owner,
> return;
> }
>
> - assert(!memhp_io_base);
> - memhp_io_base = io_base;
> state->devs = g_malloc0(sizeof(*state->devs) * state->dev_count);
> memory_region_init_io(&state->io, owner, &acpi_memory_hotplug_ops, state,
> "acpi-mem-hotplug", MEMORY_HOTPLUG_IO_LEN);
> - memory_region_add_subregion(as, memhp_io_base, &state->io);
> + memory_region_add_subregion(as, io_base, &state->io);
> }
>
> /**
> @@ -343,7 +336,8 @@ const VMStateDescription vmstate_memory_hotplug = {
>
> void build_memory_hotplug_aml(Aml *table, uint32_t nr_mem,
> const char *res_root,
> - const char *event_handler_method)
> + const char *event_handler_method,
> + AmlRegionSpace rs, hwaddr memhp_io_base)
> {
> int i;
> Aml *ifctx;
> @@ -352,10 +346,6 @@ void build_memory_hotplug_aml(Aml *table, uint32_t nr_mem,
> Aml *mem_ctrl_dev;
> char *mhp_res_path;
>
> - if (!memhp_io_base) {
> - return;
> - }
> -
> mhp_res_path = g_strdup_printf("%s." MEMORY_HOTPLUG_DEVICE, res_root);
> mem_ctrl_dev = aml_device("%s", mhp_res_path);
> {
> @@ -366,14 +356,19 @@ void build_memory_hotplug_aml(Aml *table, uint32_t nr_mem,
> aml_name_decl("_UID", aml_string("Memory hotplug resources")));
>
> crs = aml_resource_template();
> - aml_append(crs,
> - aml_io(AML_DECODE16, memhp_io_base, memhp_io_base, 0,
> - MEMORY_HOTPLUG_IO_LEN)
> - );
> + if (rs == AML_SYSTEM_IO) {
> + aml_append(crs,
> + aml_io(AML_DECODE16, memhp_io_base, memhp_io_base, 0,
> + MEMORY_HOTPLUG_IO_LEN)
> + );
> + } else {
> + aml_append(crs, aml_memory32_fixed(memhp_io_base,
> + MEMORY_HOTPLUG_IO_LEN, AML_READ_WRITE));
> + }
> aml_append(mem_ctrl_dev, aml_name_decl("_CRS", crs));
>
> aml_append(mem_ctrl_dev, aml_operation_region(
> - MEMORY_HOTPLUG_IO_REGION, AML_SYSTEM_IO,
> + MEMORY_HOTPLUG_IO_REGION, rs,
> aml_int(memhp_io_base), MEMORY_HOTPLUG_IO_LEN)
> );
>
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 034e413fd0..0d97cc701d 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -1873,7 +1873,12 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> build_cpus_aml(dsdt, machine, opts, pm->cpu_hp_io_base,
> "\\_SB.PCI0", "\\_GPE._E02");
> }
> - build_memory_hotplug_aml(dsdt, nr_mem, "\\_SB.PCI0", "\\_GPE._E03");
> +
> + if (pcms->memhp_io_base && nr_mem) {
> + build_memory_hotplug_aml(dsdt, nr_mem, "\\_SB.PCI0",
> + "\\_GPE._E03", AML_SYSTEM_IO,
> + pcms->memhp_io_base);
> + }
>
> scope = aml_scope("_GPE");
> {
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index c14ed86439..b38cd07231 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1951,6 +1951,9 @@ void pc_memory_init(PCMachineState *pcms,
>
> /* Init default IOAPIC address space */
> pcms->ioapic_as = &address_space_memory;
> +
> + /* Init ACPI memory hotplug IO base address */
> + pcms->memhp_io_base = ACPI_MEMORY_HOTPLUG_BASE;
> }
>
> /*
> diff --git a/include/hw/acpi/memory_hotplug.h b/include/hw/acpi/memory_hotplug.h
> index 77c65765d6..dfe9cf3fde 100644
> --- a/include/hw/acpi/memory_hotplug.h
> +++ b/include/hw/acpi/memory_hotplug.h
> @@ -5,6 +5,10 @@
> #include "hw/acpi/acpi.h"
> #include "hw/acpi/aml-build.h"
>
> +#define MEMORY_SLOT_SCAN_METHOD "MSCN"
> +#define MEMORY_DEVICES_CONTAINER "\\_SB.MHPC"
> +#define MEMORY_HOTPLUG_IO_LEN 24
> +
> /**
> * MemStatus:
> * @is_removing: the memory device in slot has been requested to be ejected.
> @@ -29,7 +33,7 @@ typedef struct MemHotplugState {
> } MemHotplugState;
>
> void acpi_memory_hotplug_init(MemoryRegion *as, Object *owner,
> - MemHotplugState *state, uint16_t io_base);
> + MemHotplugState *state, hwaddr io_base);
>
> void acpi_memory_plug_cb(HotplugHandler *hotplug_dev, MemHotplugState *mem_st,
> DeviceState *dev, Error **errp);
> @@ -48,5 +52,6 @@ void acpi_memory_ospm_status(MemHotplugState *mem_st, ACPIOSTInfoList ***list);
>
> void build_memory_hotplug_aml(Aml *table, uint32_t nr_mem,
> const char *res_root,
> - const char *event_handler_method);
> + const char *event_handler_method,
> + AmlRegionSpace rs, hwaddr memhp_io_base);
> #endif
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 19a837889d..ac09e42030 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -69,6 +69,9 @@ struct PCMachineState {
> /* Address space used by IOAPIC device. All IOAPIC interrupts
> * will be translated to MSI messages in the address space. */
> AddressSpace *ioapic_as;
> +
> + /* ACPI Memory hotplug IO base address */
> + hwaddr memhp_io_base;
> };
>
> #define PC_MACHINE_ACPI_DEVICE_PROP "acpi-device"
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH-for-4.2 v10 03/11] hw/acpi: Add ACPI Generic Event Device Support
2019-09-04 8:56 ` [Qemu-devel] [PATCH-for-4.2 v10 03/11] hw/acpi: Add ACPI Generic Event Device Support Shameer Kolothum
@ 2019-09-11 12:34 ` Igor Mammedov
0 siblings, 0 replies; 29+ messages in thread
From: Igor Mammedov @ 2019-09-11 12:34 UTC (permalink / raw)
To: Shameer Kolothum
Cc: peter.maydell, sameo, shannon.zhaosl, ard.biesheuvel, qemu-devel,
xuwei5, linuxarm, eric.auger, qemu-arm, sebastien.boeuf, lersek
On Wed, 4 Sep 2019 09:56:21 +0100
Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> wrote:
> From: Samuel Ortiz <sameo@linux.intel.com>
>
> The ACPI Generic Event Device (GED) is a hardware-reduced specific
> device[ACPI v6.1 Section 5.6.9] that handles all platform events,
> including the hotplug ones. This patch generates the AML code that
> defines GEDs.
>
> Platforms need to specify their own GED Event bitmap to describe
> what kind of events they want to support through GED. Also this
> uses a a single interrupt for the GED device, relying on IO
> memory region to communicate the type of device affected by the
> interrupt. This way, we can support up to 32 events with a unique
> interrupt.
>
> This supports only memory hotplug for now.
>
> Signed-off-by: Samuel Ortiz <sameo@linux.intel.com>
> Signed-off-by: Sebastien Boeuf <sebastien.boeuf@intel.com>
> Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
> Reviewed-by: Eric Auger <eric.auger@redhat.com>
> ---
> v9 --> v10
> -Added few required headers.
> -Added Eric's R-by
>
> v8 --> v9
> -Changes related to GED being a TYPE_SYS_BUS_DEVICE now.
> -Removed Eric's R-by tag for now.
>
> v7 --> v8.
> -Removed qemu_mutex_lock() across the ged state selector access.
> -Rephrased comments section in acpi_ged_send_event().
> -Moved acpi_ged_event() code into acpi_ged_send_event().
> -Added check for memhp_base and ged_base in realize().
> ---
> hw/acpi/Kconfig | 4 +
> hw/acpi/Makefile.objs | 1 +
> hw/acpi/generic_event_device.c | 317 +++++++++++++++++++++++++
> include/hw/acpi/generic_event_device.h | 100 ++++++++
> 4 files changed, 422 insertions(+)
> create mode 100644 hw/acpi/generic_event_device.c
> create mode 100644 include/hw/acpi/generic_event_device.h
>
> diff --git a/hw/acpi/Kconfig b/hw/acpi/Kconfig
> index 7c59cf900b..12e3f1e86e 100644
> --- a/hw/acpi/Kconfig
> +++ b/hw/acpi/Kconfig
> @@ -31,3 +31,7 @@ config ACPI_VMGENID
> bool
> default y
> depends on PC
> +
> +config ACPI_HW_REDUCED
> + bool
> + depends on ACPI
> diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs
> index 9bb2101e3b..655a9c1973 100644
> --- a/hw/acpi/Makefile.objs
> +++ b/hw/acpi/Makefile.objs
> @@ -6,6 +6,7 @@ common-obj-$(CONFIG_ACPI_MEMORY_HOTPLUG) += memory_hotplug.o
> common-obj-$(CONFIG_ACPI_CPU_HOTPLUG) += cpu.o
> common-obj-$(CONFIG_ACPI_NVDIMM) += nvdimm.o
> common-obj-$(CONFIG_ACPI_VMGENID) += vmgenid.o
> +common-obj-$(CONFIG_ACPI_HW_REDUCED) += generic_event_device.o
> common-obj-$(call lnot,$(CONFIG_ACPI_X86)) += acpi-stub.o
>
> common-obj-y += acpi_interface.o
> diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
> new file mode 100644
> index 0000000000..f81d966058
> --- /dev/null
> +++ b/hw/acpi/generic_event_device.c
> @@ -0,0 +1,317 @@
> +/*
> + *
> + * Copyright (c) 2018 Intel Corporation
> + * Copyright (c) 2019 Huawei Technologies R & D (UK) Ltd
> + * Written by Samuel Ortiz, Shameer Kolothum
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2 or later, as published by the Free Software Foundation.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qapi/error.h"
> +#include "exec/address-spaces.h"
> +#include "hw/acpi/acpi.h"
> +#include "hw/acpi/generic_event_device.h"
> +#include "hw/irq.h"
> +#include "hw/mem/pc-dimm.h"
> +#include "hw/qdev-properties.h"
> +#include "migration/vmstate.h"
> +#include "qemu/error-report.h"
> +
> +static const uint32_t ged_supported_events[] = {
> + ACPI_GED_MEM_HOTPLUG_EVT,
> +};
> +
> +/*
> + * The ACPI Generic Event Device (GED) is a hardware-reduced specific
> + * device[ACPI v6.1 Section 5.6.9] that handles all platform events,
> + * including the hotplug ones. Platforms need to specify their own
> + * GED Event bitmap to describe what kind of events they want to support
> + * through GED. This routine uses a single interrupt for the GED device,
> + * relying on IO memory region to communicate the type of device
> + * affected by the interrupt. This way, we can support up to 32 events
> + * with a unique interrupt.
> + */
> +void build_ged_aml(Aml *table, const char *name, HotplugHandler *hotplug_dev,
> + uint32_t ged_irq, AmlRegionSpace rs, hwaddr ged_base)
> +{
> + AcpiGedState *s = ACPI_GED(hotplug_dev);
> + Aml *crs = aml_resource_template();
> + Aml *evt, *field;
> + Aml *dev = aml_device("%s", name);
> + Aml *evt_sel = aml_local(0);
> + Aml *esel = aml_name(AML_GED_EVT_SEL);
> +
> + /* _CRS interrupt */
> + aml_append(crs, aml_interrupt(AML_CONSUMER, AML_EDGE, AML_ACTIVE_HIGH,
> + AML_EXCLUSIVE, &ged_irq, 1));
> +
> + aml_append(dev, aml_name_decl("_HID", aml_string("ACPI0013")));
> + aml_append(dev, aml_name_decl("_UID", aml_string(GED_DEVICE)));
> + aml_append(dev, aml_name_decl("_CRS", crs));
> +
> + /* Append IO region */
> + aml_append(dev, aml_operation_region(AML_GED_EVT_REG, rs,
> + aml_int(ged_base + ACPI_GED_EVT_SEL_OFFSET),
> + ACPI_GED_EVT_SEL_LEN));
> + field = aml_field(AML_GED_EVT_REG, AML_DWORD_ACC, AML_NOLOCK,
> + AML_WRITE_AS_ZEROS);
> + aml_append(field, aml_named_field(AML_GED_EVT_SEL,
> + ACPI_GED_EVT_SEL_LEN * BITS_PER_BYTE));
> + aml_append(dev, field);
> +
> + /*
> + * For each GED event we:
> + * - Add a conditional block for each event, inside a loop.
> + * - Call a method for each supported GED event type.
> + *
> + * The resulting ASL code looks like:
> + *
> + * Local0 = ESEL
> + * If ((Local0 & One) == One)
> + * {
> + * MethodEvent0()
> + * }
> + *
> + * If ((Local0 & 0x2) == 0x2)
> + * {
> + * MethodEvent1()
> + * }
> + * ...
> + */
> + evt = aml_method("_EVT", 1, AML_SERIALIZED);
> + {
> + Aml *if_ctx;
> + uint32_t i;
> + uint32_t ged_events = ctpop32(s->ged_event_bitmap);
> +
> + /* Local0 = ESEL */
> + aml_append(evt, aml_store(esel, evt_sel));
> +
> + for (i = 0; i < ARRAY_SIZE(ged_supported_events) && ged_events; i++) {
> + uint32_t event = s->ged_event_bitmap & ged_supported_events[i];
> +
> + if (!event) {
> + continue;
> + }
> +
> + if_ctx = aml_if(aml_equal(aml_and(evt_sel, aml_int(event), NULL),
> + aml_int(event)));
> + switch (event) {
> + case ACPI_GED_MEM_HOTPLUG_EVT:
> + aml_append(if_ctx, aml_call0(MEMORY_DEVICES_CONTAINER "."
> + MEMORY_SLOT_SCAN_METHOD));
> + break;
> + default:
> + /*
> + * Please make sure all the events in ged_supported_events[]
> + * are handled above.
> + */
> + g_assert_not_reached();
> + }
> +
> + aml_append(evt, if_ctx);
> + ged_events--;
> + }
> +
> + if (ged_events) {
> + error_report("Unsupported events specified");
> + abort();
> + }
> + }
> +
> + /* Append _EVT method */
> + aml_append(dev, evt);
> +
> + aml_append(table, dev);
> +}
> +
> +/* Memory read by the GED _EVT AML dynamic method */
> +static uint64_t ged_read(void *opaque, hwaddr addr, unsigned size)
> +{
> + uint64_t val = 0;
> + GEDState *ged_st = opaque;
> +
> + switch (addr) {
> + case ACPI_GED_EVT_SEL_OFFSET:
> + /* Read the selector value and reset it */
> + val = ged_st->sel;
> + ged_st->sel = 0;
> + break;
> + default:
> + break;
> + }
> +
> + return val;
> +}
> +
> +/* Nothing is expected to be written to the GED memory region */
> +static void ged_write(void *opaque, hwaddr addr, uint64_t data,
> + unsigned int size)
> +{
> +}
> +
> +static const MemoryRegionOps ged_ops = {
> + .read = ged_read,
> + .write = ged_write,
> + .endianness = DEVICE_LITTLE_ENDIAN,
> + .valid = {
> + .min_access_size = 4,
> + .max_access_size = 4,
> + },
> +};
> +
> +static void acpi_ged_device_plug_cb(HotplugHandler *hotplug_dev,
> + DeviceState *dev, Error **errp)
> +{
> + AcpiGedState *s = ACPI_GED(hotplug_dev);
> +
> + if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
> + if (s->memhp_state.is_enabled) {
> + acpi_memory_plug_cb(hotplug_dev, &s->memhp_state, dev, errp);
> + } else {
> + error_setg(errp,
> + "memory hotplug is not enabled: %s.memory-hotplug-support "
> + "is not set", object_get_typename(OBJECT(s)));
> + }
> + } else {
> + error_setg(errp, "virt: device plug request for unsupported device"
> + " type: %s", object_get_typename(OBJECT(dev)));
> + }
> +}
> +
> +static void acpi_ged_send_event(AcpiDeviceIf *adev, AcpiEventStatusBits ev)
> +{
> + AcpiGedState *s = ACPI_GED(adev);
> + GEDState *ged_st = &s->ged_state;
> + uint32_t sel;
> +
> + if (ev & ACPI_MEMORY_HOTPLUG_STATUS) {
> + sel = ACPI_GED_MEM_HOTPLUG_EVT;
> + } else {
> + /* Unknown event. Return without generating interrupt. */
> + warn_report("GED: Unsupported event %d. No irq injected", ev);
> + return;
> + }
> +
> + /*
> + * Set the GED selector field to communicate the event type.
> + * This will be read by GED aml code to select the appropriate
> + * event method.
> + */
> + ged_st->sel |= sel;
> +
> + /* Trigger the event by sending an interrupt to the guest. */
> + qemu_irq_pulse(s->irq);
> +}
> +
> +static void acpi_ged_device_realize(DeviceState *dev, Error **errp)
> +{
> + AcpiGedState *s = ACPI_GED(dev);
> + GEDState *ged_st = &s->ged_state;
> +
> + memory_region_init_io(&ged_st->io, OBJECT(dev), &ged_ops, ged_st,
> + TYPE_ACPI_GED, ACPI_GED_EVT_SEL_LEN);
> + sysbus_init_mmio(SYS_BUS_DEVICE(dev), &ged_st->io);
> +
> + sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq);
> +
> + if (s->memhp_state.is_enabled) {
> + /*
> + * GED handles memory hotplug event and acpi-mem-hotplug
> + * memory region gets initialized here. Create an exclusive
> + * container for memory hotplug IO and expose it as GED sysbus
> + * MMIO so that boards can map it separately.
> + */
> + memory_region_init(&s->container_memhp, OBJECT(dev), "container",
s/container/memhp container/
so it would be more descriptive in "info mtree"
> + MEMORY_HOTPLUG_IO_LEN);
> + sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->container_memhp);
> +
> + acpi_memory_hotplug_init(&s->container_memhp, OBJECT(dev),
> + &s->memhp_state, 0);
> + }
I suggest to move all of above code to instance_init() and drop
if (s->memhp_state.is_enabled) condition
(for x86 GPE usecase we use condition to make sure that old
machines won't be affected, but it's not the case here since in 5/11
you create GED only since 4.2 machine type
)
> +}
> +
> +static Property acpi_ged_properties[] = {
> + DEFINE_PROP_BOOL("memory-hotplug-support", AcpiGedState,
> + memhp_state.is_enabled, true),
same goes here, it's not like it would be used as compat property nor by user
and be set to False. Suggest to initialize it to true in initfn() and drop property
> + DEFINE_PROP_UINT32("ged-event", AcpiGedState, ged_event_bitmap, 0),
> + DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static bool vmstate_test_use_memhp(void *opaque)
> +{
> + AcpiGedState *s = opaque;
> + return s->memhp_state.is_enabled;
> +}
> +
> +static const VMStateDescription vmstate_memhp_state = {
> + .name = "acpi-ged/memhp",
> + .version_id = 1,
> + .minimum_version_id = 1,
> + .needed = vmstate_test_use_memhp,
this section could be make unconditional as well
> + .fields = (VMStateField[]) {
> + VMSTATE_MEMORY_HOTPLUG(memhp_state, AcpiGedState),
> + VMSTATE_END_OF_LIST()
> + }
> +};
> +
> +static const VMStateDescription vmstate_ged_state = {
> + .name = "acpi-ged-state",
> + .version_id = 1,
> + .minimum_version_id = 1,
> + .fields = (VMStateField[]) {
> + VMSTATE_UINT32(sel, GEDState),
> + VMSTATE_END_OF_LIST()
> + }
> +};
> +
> +static const VMStateDescription vmstate_acpi_ged = {
> + .name = "acpi-ged",
> + .version_id = 1,
> + .minimum_version_id = 1,
> + .fields = (VMStateField[]) {
> + VMSTATE_STRUCT(ged_state, AcpiGedState, 1, vmstate_ged_state, GEDState),
> + VMSTATE_END_OF_LIST(),
> + },
> + .subsections = (const VMStateDescription * []) {
> + &vmstate_memhp_state,
> + NULL
> + }
> +};
> +
> +static void acpi_ged_class_init(ObjectClass *class, void *data)
> +{
> + DeviceClass *dc = DEVICE_CLASS(class);
> + HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(class);
> + AcpiDeviceIfClass *adevc = ACPI_DEVICE_IF_CLASS(class);
> +
> + dc->desc = "ACPI Generic Event Device";
> + dc->props = acpi_ged_properties;
> + dc->realize = acpi_ged_device_realize;
> + dc->vmsd = &vmstate_acpi_ged;
> +
> + hc->plug = acpi_ged_device_plug_cb;
> +
> + adevc->send_event = acpi_ged_send_event;
> +}
> +
> +static const TypeInfo acpi_ged_info = {
> + .name = TYPE_ACPI_GED,
> + .parent = TYPE_SYS_BUS_DEVICE,
> + .instance_size = sizeof(AcpiGedState),
> + .class_init = acpi_ged_class_init,
> + .interfaces = (InterfaceInfo[]) {
> + { TYPE_HOTPLUG_HANDLER },
> + { TYPE_ACPI_DEVICE_IF },
> + { }
> + }
> +};
> +
> +static void acpi_ged_register_types(void)
> +{
> + type_register_static(&acpi_ged_info);
> +}
> +
> +type_init(acpi_ged_register_types)
> diff --git a/include/hw/acpi/generic_event_device.h b/include/hw/acpi/generic_event_device.h
> new file mode 100644
> index 0000000000..2049e8d873
> --- /dev/null
> +++ b/include/hw/acpi/generic_event_device.h
> @@ -0,0 +1,100 @@
> +/*
> + *
> + * Copyright (c) 2018 Intel Corporation
> + * Copyright (c) 2019 Huawei Technologies R & D (UK) Ltd
> + * Written by Samuel Ortiz, Shameer Kolothum
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2 or later, as published by the Free Software Foundation.
> + *
> + * The ACPI Generic Event Device (GED) is a hardware-reduced specific
> + * device[ACPI v6.1 Section 5.6.9] that handles all platform events,
> + * including the hotplug ones. Generic Event Device allows platforms
> + * to handle interrupts in ACPI ASL statements. It follows a very
> + * similar approach like the _EVT method from GPIO events. All
> + * interrupts are listed in _CRS and the handler is written in _EVT
> + * method. Here, we use a single interrupt for the GED device, relying
> + * on IO memory region to communicate the type of device affected by
> + * the interrupt. This way, we can support up to 32 events with a
> + * unique interrupt.
> + *
> + * Here is an example.
> + *
> + * Device (\_SB.GED)
> + * {
> + * Name (_HID, "ACPI0013")
> + * Name (_UID, Zero)
> + * Name (_CRS, ResourceTemplate ()
> + * {
> + * Interrupt (ResourceConsumer, Edge, ActiveHigh, Exclusive, ,, )
> + * {
> + * 0x00000029,
> + * }
> + * })
> + * OperationRegion (EREG, SystemMemory, 0x09080000, 0x04)
> + * Field (EREG, DWordAcc, NoLock, WriteAsZeros)
> + * {
> + * ESEL, 32
> + * }
> + *
> + * Method (_EVT, 1, Serialized) // _EVT: Event
> + * {
> + * Local0 = ESEL // ESEL = IO memory region which specifies the
> + * // device type.
> + * If (((Local0 & One) == One))
> + * {
> + * MethodEvent1()
> + * }
> + * If ((Local0 & 0x2) == 0x2)
> + * {
> + * MethodEvent2()
> + * }
> + * ...
> + * }
> + * }
> + *
> + */
> +
> +#ifndef HW_ACPI_GED_H
> +#define HW_ACPI_GED_H
> +
> +#include "hw/sysbus.h"
> +#include "hw/acpi/memory_hotplug.h"
> +
> +#define TYPE_ACPI_GED "acpi-ged"
> +#define ACPI_GED(obj) \
> + OBJECT_CHECK(AcpiGedState, (obj), TYPE_ACPI_GED)
> +
> +#define ACPI_GED_EVT_SEL_OFFSET 0x0
> +#define ACPI_GED_EVT_SEL_LEN 0x4
> +
> +#define GED_DEVICE "GED"
> +#define AML_GED_EVT_REG "EREG"
> +#define AML_GED_EVT_SEL "ESEL"
> +
> +/*
> + * Platforms need to specify the GED event bitmap
> + * to describe what kind of events they want to support
> + * through GED.
> + */
> +#define ACPI_GED_MEM_HOTPLUG_EVT 0x1
> +
> +typedef struct GEDState {
> + MemoryRegion io;
> + uint32_t sel;
> +} GEDState;
> +
> +typedef struct AcpiGedState {
> + SysBusDevice parent_obj;
> + MemHotplugState memhp_state;
> + MemoryRegion container_memhp;
> + GEDState ged_state;
> + uint32_t ged_event_bitmap;
> + qemu_irq irq;
> +} AcpiGedState;
> +
> +void build_ged_aml(Aml *table, const char* name, HotplugHandler *hotplug_dev,
> + uint32_t ged_irq, AmlRegionSpace rs, hwaddr ged_base);
> +
> +#endif
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH-for-4.2 v10 05/11] hw/arm/virt: Enable device memory cold/hot plug with ACPI boot
2019-09-04 8:56 ` [Qemu-devel] [PATCH-for-4.2 v10 05/11] hw/arm/virt: Enable device memory cold/hot plug with ACPI boot Shameer Kolothum
@ 2019-09-11 12:48 ` Igor Mammedov
2019-09-11 13:07 ` Igor Mammedov
1 sibling, 0 replies; 29+ messages in thread
From: Igor Mammedov @ 2019-09-11 12:48 UTC (permalink / raw)
To: Shameer Kolothum
Cc: peter.maydell, sameo, ard.biesheuvel, shannon.zhaosl, qemu-devel,
xuwei5, linuxarm, eric.auger, qemu-arm, sebastien.boeuf, lersek
On Wed, 4 Sep 2019 09:56:23 +0100
Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> wrote:
> This initializes the GED device with base memory and irq, configures
> ged memory hotplug event and builds the corresponding aml code. With
> this, both hot and cold plug of device memory is enabled now for Guest
> with ACPI boot. Memory cold plug support with Guest DT boot is not yet
> supported.
>
> As DSDT table gets changed by this, update bios-tables-test-allowed-diff.h
> to avoid "make check" failure.
>
> Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
> ---
> v9 --> v10
> -updated commit log with bios-tables-test-allowed-diff.h info.
> -Removed object_property_add_child() from create_acpi_ged().
>
> v8 --> v9
> -Changes related to GED being a TYPE_SYS_BUS_DEVICE now.
> -Error propagation to _plug() handler.
> -Removed R-by by Eric for now.
>
> v7 --> v8
> -Changed no_acpi_dev to no_ged.
> -Fixed 'dev' reference leak by object_new().
> -Updated bios-tables-test-allowed-diff.h to avoid "make check"
> failure.
>
> ---
> hw/arm/Kconfig | 2 +
> hw/arm/virt-acpi-build.c | 16 +++++++
> hw/arm/virt.c | 60 +++++++++++++++++++++++----
> include/hw/arm/virt.h | 4 ++
> tests/bios-tables-test-allowed-diff.h | 1 +
> 5 files changed, 76 insertions(+), 7 deletions(-)
>
> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> index 39d285ad3d..c6e7782580 100644
> --- a/hw/arm/Kconfig
> +++ b/hw/arm/Kconfig
> @@ -22,6 +22,8 @@ config ARM_VIRT
> select ACPI_PCI
> select MEM_DEVICE
> select DIMM
> + select ACPI_MEMORY_HOTPLUG
> + select ACPI_HW_REDUCED
>
> config CHEETAH
> bool
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index a8b2d97fe9..5063f28812 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -39,6 +39,8 @@
> #include "hw/acpi/aml-build.h"
> #include "hw/acpi/utils.h"
> #include "hw/acpi/pci.h"
> +#include "hw/acpi/memory_hotplug.h"
> +#include "hw/acpi/generic_event_device.h"
> #include "hw/pci/pcie_host.h"
> #include "hw/pci/pci.h"
> #include "hw/arm/virt.h"
> @@ -706,6 +708,7 @@ static void
> build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
> {
> Aml *scope, *dsdt;
> + MachineState *ms = MACHINE(vms);
> const MemMapEntry *memmap = vms->memmap;
> const int *irqmap = vms->irqmap;
>
> @@ -730,6 +733,19 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
> vms->highmem, vms->highmem_ecam);
> acpi_dsdt_add_gpio(scope, &memmap[VIRT_GPIO],
> (irqmap[VIRT_GPIO] + ARM_SPI_BASE));
> + if (vms->acpi_dev) {
> + build_ged_aml(scope, "\\_SB."GED_DEVICE,
> + HOTPLUG_HANDLER(vms->acpi_dev),
> + irqmap[VIRT_ACPI_GED] + ARM_SPI_BASE, AML_SYSTEM_MEMORY,
> + memmap[VIRT_ACPI_GED].base);
> + }
> +
> + if (vms->acpi_dev && ms->ram_slots) {
> + build_memory_hotplug_aml(scope, ms->ram_slots, "\\_SB", NULL,
> + AML_SYSTEM_MEMORY,
> + memmap[VIRT_PCDIMM_ACPI].base);
> + }
> +
> acpi_dsdt_add_power_button(scope);
>
> aml_append(dsdt, scope);
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 7c492c31a0..b99c12ad9c 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -70,6 +70,7 @@
> #include "target/arm/internals.h"
> #include "hw/mem/pc-dimm.h"
> #include "hw/mem/nvdimm.h"
> +#include "hw/acpi/generic_event_device.h"
>
> #define DEFINE_VIRT_MACHINE_LATEST(major, minor, latest) \
> static void virt_##major##_##minor##_class_init(ObjectClass *oc, \
> @@ -140,6 +141,8 @@ static const MemMapEntry base_memmap[] = {
> [VIRT_GPIO] = { 0x09030000, 0x00001000 },
> [VIRT_SECURE_UART] = { 0x09040000, 0x00001000 },
> [VIRT_SMMU] = { 0x09050000, 0x00020000 },
> + [VIRT_PCDIMM_ACPI] = { 0x09070000, MEMORY_HOTPLUG_IO_LEN },
> + [VIRT_ACPI_GED] = { 0x09080000, ACPI_GED_EVT_SEL_LEN },
> [VIRT_MMIO] = { 0x0a000000, 0x00000200 },
> /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */
> [VIRT_PLATFORM_BUS] = { 0x0c000000, 0x02000000 },
> @@ -175,6 +178,7 @@ static const int a15irqmap[] = {
> [VIRT_PCIE] = 3, /* ... to 6 */
> [VIRT_GPIO] = 7,
> [VIRT_SECURE_UART] = 8,
> + [VIRT_ACPI_GED] = 9,
> [VIRT_MMIO] = 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */
> [VIRT_GIC_V2M] = 48, /* ...to 48 + NUM_GICV2M_SPIS - 1 */
> [VIRT_SMMU] = 74, /* ...to 74 + NUM_SMMU_IRQS - 1 */
> @@ -524,6 +528,24 @@ static void fdt_add_pmu_nodes(const VirtMachineState *vms)
> }
> }
>
> +static inline DeviceState *create_acpi_ged(VirtMachineState *vms, qemu_irq *pic)
> +{
> + DeviceState *dev;
> + int irq = vms->irqmap[VIRT_ACPI_GED];
> + uint32_t event = ACPI_GED_MEM_HOTPLUG_EVT;
> +
> + dev = qdev_create(NULL, TYPE_ACPI_GED);
> + qdev_prop_set_uint32(dev, "ged-event", event);
> + qdev_init_nofail(dev)
I suggest to move this line past property initialization
(i.e. sysbus_connect_irq) so it would follow typical QOM object init flow.
> +
> + sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, vms->memmap[VIRT_ACPI_GED].base);
> + sysbus_mmio_map(SYS_BUS_DEVICE(dev), 1, vms->memmap[VIRT_PCDIMM_ACPI].base);
> +
> + sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[irq]);
> +
> + return dev;
> +}
> +
> static void create_its(VirtMachineState *vms, DeviceState *gicdev)
> {
> const char *itsclass = its_class_name();
> @@ -1487,6 +1509,7 @@ static void machvirt_init(MachineState *machine)
> MemoryRegion *ram = g_new(MemoryRegion, 1);
> bool firmware_loaded;
> bool aarch64 = true;
> + bool has_ged = !vmc->no_ged;
> unsigned int smp_cpus = machine->smp.cpus;
> unsigned int max_cpus = machine->smp.max_cpus;
>
> @@ -1701,6 +1724,10 @@ static void machvirt_init(MachineState *machine)
>
> create_gpio(vms, pic);
>
> + if (has_ged && aarch64 && firmware_loaded && acpi_enabled) {
> + vms->acpi_dev = create_acpi_ged(vms, pic);
> + }
> +
> /* Create mmio transports, so the user can create virtio backends
> * (which will be automatically plugged in to the transports). If
> * no backend is created the transport will just sit harmlessly idle.
> @@ -1880,14 +1907,23 @@ static const CPUArchIdList *virt_possible_cpu_arch_ids(MachineState *ms)
> static void virt_memory_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
> Error **errp)
> {
> + VirtMachineState *vms = VIRT_MACHINE(hotplug_dev);
> + const bool is_nvdimm = object_dynamic_cast(OBJECT(dev), TYPE_NVDIMM);
> + Error *local_err = NULL;
>
> - /*
> - * The device memory is not yet exposed to the Guest either through
> - * DT or ACPI and hence both cold/hot plug of memory is explicitly
> - * disabled for now.
> - */
> - if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
> - error_setg(errp, "memory cold/hot plug is not yet supported");
> + if (is_nvdimm) {
> + error_setg(errp, "nvdimm is not yet supported");
> + return;
> + }
> +
> + if (!vms->acpi_dev) {
> + error_setg(errp, "memory hotplug is not enabled: missing acpi device");
It could be more clear if s/acpi/acpi-ged/
> + return;
> + }
> +
> + hotplug_handler_pre_plug(HOTPLUG_HANDLER(vms->acpi_dev), dev, &local_err);
Does this call do anything?
> + if (local_err) {
> + error_propagate(errp, local_err);
> return;
> }
>
> @@ -1897,11 +1933,18 @@ static void virt_memory_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
> static void virt_memory_plug(HotplugHandler *hotplug_dev,
> DeviceState *dev, Error **errp)
> {
> + HotplugHandlerClass *hhc;
> VirtMachineState *vms = VIRT_MACHINE(hotplug_dev);
> Error *local_err = NULL;
>
> pc_dimm_plug(PC_DIMM(dev), MACHINE(vms), &local_err);
> + if (local_err) {
> + goto out;
> + }
>
> + hhc = HOTPLUG_HANDLER_GET_CLASS(vms->acpi_dev);
> + hhc->plug(HOTPLUG_HANDLER(vms->acpi_dev), dev, &local_err);
if error after this fails is recoverable, there should be code that
will undo whatever previous steps done.
There is no rollback in x86 case and we use error_abort
since it should never fail.
> +out:
> error_propagate(errp, local_err);
> }
>
> @@ -2108,8 +2151,11 @@ DEFINE_VIRT_MACHINE_AS_LATEST(4, 2)
>
> static void virt_machine_4_1_options(MachineClass *mc)
> {
> + VirtMachineClass *vmc = VIRT_MACHINE_CLASS(OBJECT_CLASS(mc));
> +
> virt_machine_4_2_options(mc);
> compat_props_add(mc->compat_props, hw_compat_4_1, hw_compat_4_1_len);
> + vmc->no_ged = true;
> }
> DEFINE_VIRT_MACHINE(4, 1)
>
> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
> index a72094204e..577ee49b4b 100644
> --- a/include/hw/arm/virt.h
> +++ b/include/hw/arm/virt.h
> @@ -77,6 +77,8 @@ enum {
> VIRT_GPIO,
> VIRT_SECURE_UART,
> VIRT_SECURE_MEM,
> + VIRT_PCDIMM_ACPI,
> + VIRT_ACPI_GED,
> VIRT_LOWMEMMAP_LAST,
> };
>
> @@ -106,6 +108,7 @@ typedef struct {
> bool claim_edge_triggered_timers;
> bool smbios_old_sys_ver;
> bool no_highmem_ecam;
> + bool no_ged; /* Machines < 4.2 has no support for ACPI GED device */
> } VirtMachineClass;
>
> typedef struct {
> @@ -133,6 +136,7 @@ typedef struct {
> uint32_t iommu_phandle;
> int psci_conduit;
> hwaddr highest_gpa;
> + DeviceState *acpi_dev;
> } VirtMachineState;
>
> #define VIRT_ECAM_ID(high) (high ? VIRT_HIGH_PCIE_ECAM : VIRT_PCIE_ECAM)
> diff --git a/tests/bios-tables-test-allowed-diff.h b/tests/bios-tables-test-allowed-diff.h
> index dfb8523c8b..7b4adbc822 100644
> --- a/tests/bios-tables-test-allowed-diff.h
> +++ b/tests/bios-tables-test-allowed-diff.h
> @@ -1 +1,2 @@
> /* List of comma-separated changed AML files to ignore */
> +"tests/data/acpi/virt/DSDT",
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH-for-4.2 v10 09/11] docs/specs: Add ACPI GED documentation
2019-09-04 8:56 ` [Qemu-devel] [PATCH-for-4.2 v10 09/11] docs/specs: Add ACPI GED documentation Shameer Kolothum
@ 2019-09-11 12:54 ` Peter Maydell
0 siblings, 0 replies; 29+ messages in thread
From: Peter Maydell @ 2019-09-11 12:54 UTC (permalink / raw)
To: Shameer Kolothum
Cc: Samuel Ortiz, Shannon Zhao, Ard Biesheuvel, QEMU Developers,
Xu Wei, Linuxarm, Eric Auger, qemu-arm, Igor Mammedov,
sebastien.boeuf, Laszlo Ersek
On Wed, 4 Sep 2019 at 09:58, Shameer Kolothum
<shameerali.kolothum.thodi@huawei.com> wrote:
>
> Documents basic concepts of ACPI Generic Event device(GED)
> and interface between QEMU and the ACPI BIOS.
>
> Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
> Reviewed-by: Eric Auger <eric.auger@redhat.com>
> ---
> docs/specs/acpi_hw_reduced_hotplug.txt | 60 ++++++++++++++++++++++++++
> 1 file changed, 60 insertions(+)
> create mode 100644 docs/specs/acpi_hw_reduced_hotplug.txt
New documentation in docs/specs should be in .rst format
and listed in docs/specs/index.rst, please. That way
it gets built into the sphinx manual sets we're transitioning
to. The rst markup should hopefully not be too difficult to add.
Some minor typo/grammar fixes below:
> +GED allows HW reduced platforms to handle interrupts in ACPI ASL
> +statements. It follows a very similar approach like the _EVT method
"similar approach to the"
> +from GPIO events. All interrupts are listed in _CRS and the handler
> +is written in _EVT method. However, Qemu implementation uses a single
"the QEMU implementation"
>+ interrupt for the GED device, relying on IO memory region to communicate
"on an IO memory region"
> +the type of device affected by the interrupt. This way, we can support
> +up to 32 events with a unique interrupt.
> + [0x0-0x3] Event selector bit field(32 bit) set by Qemu.
Missing space before '('.
"QEMU" should be all-capitals.
thanks
-- PMM
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH-for-4.2 v10 10/11] tests: add dummy ACPI tables for arm/virt board
2019-09-04 8:56 ` [Qemu-devel] [PATCH-for-4.2 v10 10/11] tests: add dummy ACPI tables for arm/virt board Shameer Kolothum
@ 2019-09-11 12:57 ` Peter Maydell
2019-09-11 13:50 ` Igor Mammedov
0 siblings, 1 reply; 29+ messages in thread
From: Peter Maydell @ 2019-09-11 12:57 UTC (permalink / raw)
To: Shameer Kolothum
Cc: Samuel Ortiz, Shannon Zhao, Ard Biesheuvel, QEMU Developers,
Xu Wei, Linuxarm, Eric Auger, qemu-arm, Igor Mammedov,
sebastien.boeuf, Laszlo Ersek
On Wed, 4 Sep 2019 at 09:58, Shameer Kolothum
<shameerali.kolothum.thodi@huawei.com> wrote:
>
> This patch is in preparation for adding numamem and memhp tests
> to arm/virt board so that 'make check' is happy. This may not
> be required once the scripts are run and new tables are
> generated with ".numamem" and ".memhp" extensions.
>
> Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
> ---
> I am not sure this is the right way to do this. But without this, when
> the numamem and memhp tests are added, you will get,
>
> Looking for expected file 'tests/data/acpi/virt/SRAT.numamem'
> Looking for expected file 'tests/data/acpi/virt/SRAT'
> **
> ERROR:tests/bios-tables-test.c:327:load_expected_aml: assertion failed: (exp_sdt.aml_file)
>
> ---
> tests/data/acpi/virt/SLIT | Bin 0 -> 48 bytes
> tests/data/acpi/virt/SRAT | Bin 0 -> 224 bytes
> 2 files changed, 0 insertions(+), 0 deletions(-)
> create mode 100644 tests/data/acpi/virt/SLIT
> create mode 100644 tests/data/acpi/virt/SRAT
Do the tests pass with this patch and without the
patch that adds the tests? (That is, can we keep the
two patches separate without breaking bisection, or
do we need to squash them together?)
I'll leave it to somebody who understands the ACPI
tests stuff to answer whether there's a better way to
do this.
thanks
-- PMM
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH-for-4.2 v10 05/11] hw/arm/virt: Enable device memory cold/hot plug with ACPI boot
2019-09-04 8:56 ` [Qemu-devel] [PATCH-for-4.2 v10 05/11] hw/arm/virt: Enable device memory cold/hot plug with ACPI boot Shameer Kolothum
2019-09-11 12:48 ` Igor Mammedov
@ 2019-09-11 13:07 ` Igor Mammedov
2019-09-16 10:30 ` Shameerali Kolothum Thodi
1 sibling, 1 reply; 29+ messages in thread
From: Igor Mammedov @ 2019-09-11 13:07 UTC (permalink / raw)
To: Shameer Kolothum
Cc: peter.maydell, sameo, shannon.zhaosl, ard.biesheuvel, qemu-devel,
xuwei5, linuxarm, eric.auger, qemu-arm, sebastien.boeuf, lersek
On Wed, 4 Sep 2019 09:56:23 +0100
Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> wrote:
[...]
> @@ -730,6 +733,19 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
> vms->highmem, vms->highmem_ecam);
> acpi_dsdt_add_gpio(scope, &memmap[VIRT_GPIO],
> (irqmap[VIRT_GPIO] + ARM_SPI_BASE));
> + if (vms->acpi_dev) {
> + build_ged_aml(scope, "\\_SB."GED_DEVICE,
> + HOTPLUG_HANDLER(vms->acpi_dev),
> + irqmap[VIRT_ACPI_GED] + ARM_SPI_BASE, AML_SYSTEM_MEMORY,
> + memmap[VIRT_ACPI_GED].base);
> + }
> +
> + if (vms->acpi_dev && ms->ram_slots) {
> + build_memory_hotplug_aml(scope, ms->ram_slots, "\\_SB", NULL,
> + AML_SYSTEM_MEMORY,
> + memmap[VIRT_PCDIMM_ACPI].base);
> + }
one more thing (though non critical), if ms->ram_slots == 0 ^^^^
makes IASL spew a warning
External (_SB_.MHPC.MSCN, MethodObj) // Warning: Unknown method, guessing 0 arguments
In general non-existing references within methods are fine if they aren't ever used.
however we can be a little bit less sloppy.
Below you advertise "event = ACPI_GED_MEM_HOTPLUG_EVT", and then here suddenly
don't generate essential AML part for it here.
For consistency if above is conditioned on ms->ram_slots != 0, probably
it would be better to move that condition where you set 'event' value and
check property value above instead of ms->ram_slots
[...]
> +static inline DeviceState *create_acpi_ged(VirtMachineState *vms, qemu_irq *pic)
> +{
> + DeviceState *dev;
> + int irq = vms->irqmap[VIRT_ACPI_GED];
> + uint32_t event = ACPI_GED_MEM_HOTPLUG_EVT;
> +
> + dev = qdev_create(NULL, TYPE_ACPI_GED);
> + qdev_prop_set_uint32(dev, "ged-event", event);
> + qdev_init_nofail(dev);
> +
> + sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, vms->memmap[VIRT_ACPI_GED].base);
> + sysbus_mmio_map(SYS_BUS_DEVICE(dev), 1, vms->memmap[VIRT_PCDIMM_ACPI].base);
> +
> + sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[irq]);
> +
> + return dev;
> +}
> +
[...]
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH-for-4.2 v10 07/11] hw/arm: Factor out powerdown notifier from GPIO
2019-09-04 8:56 ` [Qemu-devel] [PATCH-for-4.2 v10 07/11] hw/arm: Factor out powerdown notifier from GPIO Shameer Kolothum
@ 2019-09-11 13:08 ` Igor Mammedov
0 siblings, 0 replies; 29+ messages in thread
From: Igor Mammedov @ 2019-09-11 13:08 UTC (permalink / raw)
To: Shameer Kolothum
Cc: peter.maydell, sameo, shannon.zhaosl, ard.biesheuvel, qemu-devel,
xuwei5, linuxarm, eric.auger, qemu-arm, sebastien.boeuf, lersek
On Wed, 4 Sep 2019 09:56:25 +0100
Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> wrote:
> This is in preparation of using GED device for
> system_powerdown event. Make the powerdown notifier
> registration independent of create_gpio() fn.
>
> Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
> Reviewed-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> ---
> hw/arm/virt.c | 12 ++++--------
> include/hw/arm/virt.h | 1 +
> 2 files changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 95accc28f1..89ee37d7de 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -791,10 +791,6 @@ static void virt_powerdown_req(Notifier *n, void *opaque)
> qemu_set_irq(qdev_get_gpio_in(gpio_key_dev, 0), 1);
> }
>
> -static Notifier virt_system_powerdown_notifier = {
> - .notify = virt_powerdown_req
> -};
> -
> static void create_gpio(const VirtMachineState *vms, qemu_irq *pic)
> {
> char *nodename;
> @@ -835,10 +831,6 @@ static void create_gpio(const VirtMachineState *vms, qemu_irq *pic)
> KEY_POWER);
> qemu_fdt_setprop_cells(vms->fdt, "/gpio-keys/poweroff",
> "gpios", phandle, 3, 0);
> -
> - /* connect powerdown request */
> - qemu_register_powerdown_notifier(&virt_system_powerdown_notifier);
> -
> g_free(nodename);
> }
>
> @@ -1730,6 +1722,10 @@ static void machvirt_init(MachineState *machine)
> vms->acpi_dev = create_acpi_ged(vms, pic);
> }
>
> + /* connect powerdown request */
> + vms->powerdown_notifier.notify = virt_powerdown_req;
> + qemu_register_powerdown_notifier(&vms->powerdown_notifier);
> +
> /* Create mmio transports, so the user can create virtio backends
> * (which will be automatically plugged in to the transports). If
> * no backend is created the transport will just sit harmlessly idle.
> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
> index 577ee49b4b..0b41083e9d 100644
> --- a/include/hw/arm/virt.h
> +++ b/include/hw/arm/virt.h
> @@ -137,6 +137,7 @@ typedef struct {
> int psci_conduit;
> hwaddr highest_gpa;
> DeviceState *acpi_dev;
> + Notifier powerdown_notifier;
> } VirtMachineState;
>
> #define VIRT_ECAM_ID(high) (high ? VIRT_HIGH_PCIE_ECAM : VIRT_PCIE_ECAM)
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH-for-4.2 v10 06/11] hw/arm/virt-acpi-build: Add PC-DIMM in SRAT
2019-09-04 8:56 ` [Qemu-devel] [PATCH-for-4.2 v10 06/11] hw/arm/virt-acpi-build: Add PC-DIMM in SRAT Shameer Kolothum
@ 2019-09-11 13:14 ` Igor Mammedov
0 siblings, 0 replies; 29+ messages in thread
From: Igor Mammedov @ 2019-09-11 13:14 UTC (permalink / raw)
To: Shameer Kolothum
Cc: peter.maydell, sameo, ard.biesheuvel, shannon.zhaosl, qemu-devel,
xuwei5, linuxarm, eric.auger, qemu-arm, sebastien.boeuf, lersek
On Wed, 4 Sep 2019 09:56:24 +0100
Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> wrote:
> Generate Memory Affinity Structures for PC-DIMM ranges.
>
> Also, Linux and Windows need ACPI SRAT table to make memory hotplug
> work properly, however currently QEMU doesn't create SRAT table if
> numa options aren't present on CLI. Hence add support(>=4.2) to
> create numa node automatically (auto_enable_numa_with_memhp) when
> QEMU is started with memory hotplug enabled but without '-numa'
> options on CLI.
>
> Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
this patch needs rebase (due changes in numa code)
> ---
> v8 --> v9
> - Added auto_enable_numa_with_memhp support.
> ---
> hw/arm/virt-acpi-build.c | 9 +++++++++
> hw/arm/virt.c | 2 ++
> 2 files changed, 11 insertions(+)
>
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 5063f28812..a4872a0b00 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -519,6 +519,7 @@ build_srat(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
> int i, srat_start;
> uint64_t mem_base;
> MachineClass *mc = MACHINE_GET_CLASS(vms);
> + MachineState *ms = MACHINE(vms);
> const CPUArchIdList *cpu_list = mc->possible_cpu_arch_ids(MACHINE(vms));
>
> srat_start = table_data->len;
> @@ -544,6 +545,14 @@ build_srat(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
> }
> }
>
> + if (ms->device_memory) {
> + numamem = acpi_data_push(table_data, sizeof *numamem);
> + build_srat_memory(numamem, ms->device_memory->base,
> + memory_region_size(&ms->device_memory->mr),
> + nb_numa_nodes - 1,
> + MEM_AFFINITY_HOTPLUGGABLE | MEM_AFFINITY_ENABLED);
> + }
> +
> build_header(linker, table_data, (void *)(table_data->data + srat_start),
> "SRAT", table_data->len - srat_start, 3, NULL, NULL);
> }
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 29bc2837ce..95accc28f1 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -2053,6 +2053,7 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
> hc->plug = virt_machine_device_plug_cb;
> hc->unplug_request = virt_machine_device_unplug_request_cb;
> mc->numa_mem_supported = true;
> + mc->auto_enable_numa_with_memhp = true;
> }
>
> static void virt_instance_init(Object *obj)
> @@ -2158,6 +2159,7 @@ static void virt_machine_4_1_options(MachineClass *mc)
> virt_machine_4_2_options(mc);
> compat_props_add(mc->compat_props, hw_compat_4_1, hw_compat_4_1_len);
> vmc->no_ged = true;
> + mc->auto_enable_numa_with_memhp = false;
> }
> DEFINE_VIRT_MACHINE(4, 1)
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH-for-4.2 v10 08/11] hw/arm: Use GED for system_powerdown event
2019-09-04 8:56 ` [Qemu-devel] [PATCH-for-4.2 v10 08/11] hw/arm: Use GED for system_powerdown event Shameer Kolothum
@ 2019-09-11 13:20 ` Igor Mammedov
0 siblings, 0 replies; 29+ messages in thread
From: Igor Mammedov @ 2019-09-11 13:20 UTC (permalink / raw)
To: Shameer Kolothum
Cc: peter.maydell, sameo, ard.biesheuvel, shannon.zhaosl, qemu-devel,
xuwei5, linuxarm, eric.auger, qemu-arm, sebastien.boeuf, lersek
On Wed, 4 Sep 2019 09:56:26 +0100
Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> wrote:
> For machines 4.2 or higher with ACPI boot use GED for system_powerdown
> event instead of GPIO. Guest boot with DT still uses GPIO.
>
> Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
> Reviewed-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> ---
> v9 --> v10
> -Eric's R-by
>
> v8 --> v9
> -Re-arranged patches 8 & 9 from v8 based on Igor's comments.
>
> v7 --> v8
> -Retained gpio based system_powerdown support for machines < 4.2.
> -Reuse of virt_powerdown_req() for ACPI GED use.
> -Dropped Eric's R-by for now because of above.
> ---
> hw/acpi/generic_event_device.c | 8 ++++++++
> hw/arm/virt-acpi-build.c | 6 +++---
> hw/arm/virt.c | 16 +++++++++++-----
> include/hw/acpi/acpi_dev_interface.h | 1 +
> include/hw/acpi/generic_event_device.h | 3 +++
> 5 files changed, 26 insertions(+), 8 deletions(-)
>
> diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
> index f81d966058..47e178c583 100644
> --- a/hw/acpi/generic_event_device.c
> +++ b/hw/acpi/generic_event_device.c
> @@ -22,6 +22,7 @@
>
> static const uint32_t ged_supported_events[] = {
> ACPI_GED_MEM_HOTPLUG_EVT,
> + ACPI_GED_PWR_DOWN_EVT,
> };
>
> /*
> @@ -104,6 +105,11 @@ void build_ged_aml(Aml *table, const char *name, HotplugHandler *hotplug_dev,
> aml_append(if_ctx, aml_call0(MEMORY_DEVICES_CONTAINER "."
> MEMORY_SLOT_SCAN_METHOD));
> break;
> + case ACPI_GED_PWR_DOWN_EVT:
> + aml_append(if_ctx,
> + aml_notify(aml_name(ACPI_POWER_BUTTON_DEVICE),
> + aml_int(0x80)));
> + break;
> default:
> /*
> * Please make sure all the events in ged_supported_events[]
> @@ -190,6 +196,8 @@ static void acpi_ged_send_event(AcpiDeviceIf *adev, AcpiEventStatusBits ev)
>
> if (ev & ACPI_MEMORY_HOTPLUG_STATUS) {
> sel = ACPI_GED_MEM_HOTPLUG_EVT;
> + } else if (ev & ACPI_POWER_DOWN_STATUS) {
> + sel = ACPI_GED_PWR_DOWN_EVT;
> } else {
> /* Unknown event. Return without generating interrupt. */
> warn_report("GED: Unsupported event %d. No irq injected", ev);
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index a4872a0b00..bbf7f6fafa 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -50,7 +50,6 @@
> #include "migration/vmstate.h"
>
> #define ARM_SPI_BASE 32
> -#define ACPI_POWER_BUTTON_DEVICE "PWRB"
>
> static void acpi_dsdt_add_cpus(Aml *scope, int smp_cpus)
> {
> @@ -740,13 +739,14 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
> (irqmap[VIRT_MMIO] + ARM_SPI_BASE), NUM_VIRTIO_TRANSPORTS);
> acpi_dsdt_add_pci(scope, memmap, (irqmap[VIRT_PCIE] + ARM_SPI_BASE),
> vms->highmem, vms->highmem_ecam);
> - acpi_dsdt_add_gpio(scope, &memmap[VIRT_GPIO],
> - (irqmap[VIRT_GPIO] + ARM_SPI_BASE));
> if (vms->acpi_dev) {
> build_ged_aml(scope, "\\_SB."GED_DEVICE,
> HOTPLUG_HANDLER(vms->acpi_dev),
> irqmap[VIRT_ACPI_GED] + ARM_SPI_BASE, AML_SYSTEM_MEMORY,
> memmap[VIRT_ACPI_GED].base);
> + } else {
> + acpi_dsdt_add_gpio(scope, &memmap[VIRT_GPIO],
> + (irqmap[VIRT_GPIO] + ARM_SPI_BASE));
> }
>
> if (vms->acpi_dev && ms->ram_slots) {
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 89ee37d7de..32208e1058 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -532,7 +532,7 @@ static inline DeviceState *create_acpi_ged(VirtMachineState *vms, qemu_irq *pic)
> {
> DeviceState *dev;
> int irq = vms->irqmap[VIRT_ACPI_GED];
> - uint32_t event = ACPI_GED_MEM_HOTPLUG_EVT;
> + uint32_t event = ACPI_GED_MEM_HOTPLUG_EVT | ACPI_GED_PWR_DOWN_EVT;
>
> dev = qdev_create(NULL, TYPE_ACPI_GED);
> qdev_prop_set_uint32(dev, "ged-event", event);
> @@ -787,8 +787,14 @@ static void create_rtc(const VirtMachineState *vms, qemu_irq *pic)
> static DeviceState *gpio_key_dev;
> static void virt_powerdown_req(Notifier *n, void *opaque)
> {
> - /* use gpio Pin 3 for power button event */
> - qemu_set_irq(qdev_get_gpio_in(gpio_key_dev, 0), 1);
> + VirtMachineState *s = container_of(n, VirtMachineState, powerdown_notifier);
> +
> + if (s->acpi_dev) {
> + acpi_send_event(s->acpi_dev, ACPI_POWER_DOWN_STATUS);
> + } else {
> + /* use gpio Pin 3 for power button event */
> + qemu_set_irq(qdev_get_gpio_in(gpio_key_dev, 0), 1);
> + }
> }
>
> static void create_gpio(const VirtMachineState *vms, qemu_irq *pic)
> @@ -1716,10 +1722,10 @@ static void machvirt_init(MachineState *machine)
>
> create_pcie(vms, pic);
>
> - create_gpio(vms, pic);
> -
> if (has_ged && aarch64 && firmware_loaded && acpi_enabled) {
> vms->acpi_dev = create_acpi_ged(vms, pic);
> + } else {
> + create_gpio(vms, pic);
> }
>
> /* connect powerdown request */
> diff --git a/include/hw/acpi/acpi_dev_interface.h b/include/hw/acpi/acpi_dev_interface.h
> index 0ba90effd2..a2a12af9b9 100644
> --- a/include/hw/acpi/acpi_dev_interface.h
> +++ b/include/hw/acpi/acpi_dev_interface.h
> @@ -13,6 +13,7 @@ typedef enum {
> ACPI_MEMORY_HOTPLUG_STATUS = 8,
> ACPI_NVDIMM_HOTPLUG_STATUS = 16,
> ACPI_VMGENID_CHANGE_STATUS = 32,
> + ACPI_POWER_DOWN_STATUS = 64,
> } AcpiEventStatusBits;
>
> #define TYPE_ACPI_DEVICE_IF "acpi-device-interface"
> diff --git a/include/hw/acpi/generic_event_device.h b/include/hw/acpi/generic_event_device.h
> index 2049e8d873..d157eac088 100644
> --- a/include/hw/acpi/generic_event_device.h
> +++ b/include/hw/acpi/generic_event_device.h
> @@ -62,6 +62,8 @@
> #include "hw/sysbus.h"
> #include "hw/acpi/memory_hotplug.h"
>
> +#define ACPI_POWER_BUTTON_DEVICE "PWRB"
> +
> #define TYPE_ACPI_GED "acpi-ged"
> #define ACPI_GED(obj) \
> OBJECT_CHECK(AcpiGedState, (obj), TYPE_ACPI_GED)
> @@ -79,6 +81,7 @@
> * through GED.
> */
> #define ACPI_GED_MEM_HOTPLUG_EVT 0x1
> +#define ACPI_GED_PWR_DOWN_EVT 0x2
>
> typedef struct GEDState {
> MemoryRegion io;
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH-for-4.2 v10 11/11] tests: Add bios tests to arm/virt
2019-09-04 8:56 ` [Qemu-devel] [PATCH-for-4.2 v10 11/11] tests: Add bios tests to arm/virt Shameer Kolothum
@ 2019-09-11 13:46 ` Igor Mammedov
0 siblings, 0 replies; 29+ messages in thread
From: Igor Mammedov @ 2019-09-11 13:46 UTC (permalink / raw)
To: Shameer Kolothum
Cc: peter.maydell, sameo, shannon.zhaosl, ard.biesheuvel, qemu-devel,
xuwei5, linuxarm, eric.auger, qemu-arm, sebastien.boeuf, lersek
On Wed, 4 Sep 2019 09:56:29 +0100
Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> wrote:
> This adds numamem and memhp tests for arm/virt platform
>
> Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> ---
> tests/bios-tables-test-allowed-diff.h | 1 +
> tests/bios-tables-test.c | 49 +++++++++++++++++++++++++++
> 2 files changed, 50 insertions(+)
>
> diff --git a/tests/bios-tables-test-allowed-diff.h b/tests/bios-tables-test-allowed-diff.h
> index 7b4adbc822..d181a4da4a 100644
> --- a/tests/bios-tables-test-allowed-diff.h
> +++ b/tests/bios-tables-test-allowed-diff.h
> @@ -1,2 +1,3 @@
> /* List of comma-separated changed AML files to ignore */
> "tests/data/acpi/virt/DSDT",
> +"tests/data/acpi/virt/SRAT",
> diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
> index a356ac3489..1d6f330d53 100644
> --- a/tests/bios-tables-test.c
> +++ b/tests/bios-tables-test.c
> @@ -871,6 +871,53 @@ static void test_acpi_piix4_tcg_dimm_pxm(void)
> test_acpi_tcg_dimm_pxm(MACHINE_PC);
> }
>
> +static void test_acpi_virt_tcg_memhp(void)
> +{
> + test_data data = {
> + .machine = "virt",
> + .accel = "tcg",
> + .uefi_fl1 = "pc-bios/edk2-aarch64-code.fd",
> + .uefi_fl2 = "pc-bios/edk2-arm-vars.fd",
> + .cd = "tests/data/uefi-boot-images/bios-tables-test.aarch64.iso.qcow2",
> + .ram_start = 0x40000000ULL,
> + .scan_len = 256ULL * 1024 * 1024,
> + };
> +
> + data.variant = ".memhp";
> + test_acpi_one(" -cpu cortex-a57"
> + " -object memory-backend-ram,id=ram0,size=128M"
> + " -object memory-backend-ram,id=ram1,size=128M"
> + " -numa node,memdev=ram0 -numa node,memdev=ram1"
> + " -numa dist,src=0,dst=1,val=21",
> + &data);
> +
> + free_test_data(&data);
> +
> +}
> +
> +static void test_acpi_virt_tcg_numamem(void)
> +{
> + test_data data = {
> + .machine = "virt",
> + .accel = "tcg",
> + .uefi_fl1 = "pc-bios/edk2-aarch64-code.fd",
> + .uefi_fl2 = "pc-bios/edk2-arm-vars.fd",
> + .cd = "tests/data/uefi-boot-images/bios-tables-test.aarch64.iso.qcow2",
> + .ram_start = 0x40000000ULL,
> + .scan_len = 128ULL * 1024 * 1024,
> + };
> +
> + data.variant = ".numamem";
> + test_acpi_one(" -cpu cortex-a57"
> + " -object memory-backend-ram,id=ram0,size=128M"
> + " -numa node,memdev=ram0",
> + &data);
> +
> + free_test_data(&data);
> +
> +}
> +
> static void test_acpi_virt_tcg(void)
> {
> test_data data = {
> @@ -917,6 +964,8 @@ int main(int argc, char *argv[])
> qtest_add_func("acpi/q35/dimmpxm", test_acpi_q35_tcg_dimm_pxm);
> } else if (strcmp(arch, "aarch64") == 0) {
> qtest_add_func("acpi/virt", test_acpi_virt_tcg);
> + qtest_add_func("acpi/virt/numamem", test_acpi_virt_tcg_numamem);
> + qtest_add_func("acpi/virt/memhp", test_acpi_virt_tcg_memhp);
> }
> ret = g_test_run();
> boot_sector_cleanup(disk);
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH-for-4.2 v10 10/11] tests: add dummy ACPI tables for arm/virt board
2019-09-11 12:57 ` Peter Maydell
@ 2019-09-11 13:50 ` Igor Mammedov
2019-09-11 13:55 ` Michael S. Tsirkin
0 siblings, 1 reply; 29+ messages in thread
From: Igor Mammedov @ 2019-09-11 13:50 UTC (permalink / raw)
To: Peter Maydell
Cc: Samuel Ortiz, Ard Biesheuvel, mst, QEMU Developers, Xu Wei,
Linuxarm, Shannon Zhao, qemu-arm, Shameer Kolothum, Eric Auger,
sebastien.boeuf, Laszlo Ersek
On Wed, 11 Sep 2019 13:57:06 +0100
Peter Maydell <peter.maydell@linaro.org> wrote:
> On Wed, 4 Sep 2019 at 09:58, Shameer Kolothum
> <shameerali.kolothum.thodi@huawei.com> wrote:
> >
> > This patch is in preparation for adding numamem and memhp tests
> > to arm/virt board so that 'make check' is happy. This may not
> > be required once the scripts are run and new tables are
> > generated with ".numamem" and ".memhp" extensions.
> >
> > Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
> > ---
> > I am not sure this is the right way to do this. But without this, when
> > the numamem and memhp tests are added, you will get,
> >
> > Looking for expected file 'tests/data/acpi/virt/SRAT.numamem'
> > Looking for expected file 'tests/data/acpi/virt/SRAT'
> > **
> > ERROR:tests/bios-tables-test.c:327:load_expected_aml: assertion failed: (exp_sdt.aml_file)
> >
> > ---
> > tests/data/acpi/virt/SLIT | Bin 0 -> 48 bytes
> > tests/data/acpi/virt/SRAT | Bin 0 -> 224 bytes
> > 2 files changed, 0 insertions(+), 0 deletions(-)
> > create mode 100644 tests/data/acpi/virt/SLIT
> > create mode 100644 tests/data/acpi/virt/SRAT
>
> Do the tests pass with this patch and without the
> patch that adds the tests? (That is, can we keep the
> two patches separate without breaking bisection, or
> do we need to squash them together?)
>
> I'll leave it to somebody who understands the ACPI
> tests stuff to answer whether there's a better way to
I'd squash this patch into 11/11 test case,
CCing Michael (since he's the one who applies ACPI patches)
> do this.
>
> thanks
> -- PMM
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH-for-4.2 v10 10/11] tests: add dummy ACPI tables for arm/virt board
2019-09-11 13:50 ` Igor Mammedov
@ 2019-09-11 13:55 ` Michael S. Tsirkin
2019-09-11 14:33 ` Shameerali Kolothum Thodi
2019-09-17 15:11 ` Michael S. Tsirkin
0 siblings, 2 replies; 29+ messages in thread
From: Michael S. Tsirkin @ 2019-09-11 13:55 UTC (permalink / raw)
To: Igor Mammedov
Cc: Peter Maydell, Samuel Ortiz, Ard Biesheuvel, QEMU Developers,
Shameer Kolothum, Linuxarm, Shannon Zhao, qemu-arm, Xu Wei,
Eric Auger, sebastien.boeuf, Laszlo Ersek
On Wed, Sep 11, 2019 at 03:50:15PM +0200, Igor Mammedov wrote:
> On Wed, 11 Sep 2019 13:57:06 +0100
> Peter Maydell <peter.maydell@linaro.org> wrote:
>
> > On Wed, 4 Sep 2019 at 09:58, Shameer Kolothum
> > <shameerali.kolothum.thodi@huawei.com> wrote:
> > >
> > > This patch is in preparation for adding numamem and memhp tests
> > > to arm/virt board so that 'make check' is happy. This may not
> > > be required once the scripts are run and new tables are
> > > generated with ".numamem" and ".memhp" extensions.
> > >
> > > Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
> > > ---
> > > I am not sure this is the right way to do this. But without this, when
> > > the numamem and memhp tests are added, you will get,
> > >
> > > Looking for expected file 'tests/data/acpi/virt/SRAT.numamem'
> > > Looking for expected file 'tests/data/acpi/virt/SRAT'
> > > **
> > > ERROR:tests/bios-tables-test.c:327:load_expected_aml: assertion failed: (exp_sdt.aml_file)
> > >
> > > ---
> > > tests/data/acpi/virt/SLIT | Bin 0 -> 48 bytes
> > > tests/data/acpi/virt/SRAT | Bin 0 -> 224 bytes
> > > 2 files changed, 0 insertions(+), 0 deletions(-)
> > > create mode 100644 tests/data/acpi/virt/SLIT
> > > create mode 100644 tests/data/acpi/virt/SRAT
> >
> > Do the tests pass with this patch and without the
> > patch that adds the tests? (That is, can we keep the
> > two patches separate without breaking bisection, or
> > do we need to squash them together?)
> >
> > I'll leave it to somebody who understands the ACPI
> > tests stuff to answer whether there's a better way to
> I'd squash this patch into 11/11 test case,
Pls don't - the way to add this is to add the files in question to
tests/bios-tables-test-allowed-diff.h.
Maintainer will create a separate commit updating
the binaries and removing them from the whitelist.
This way things like rebase work seemlessly.
> CCing Michael (since he's the one who applies ACPI patches)
>
> > do this.
> >
> > thanks
> > -- PMM
> >
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH-for-4.2 v10 10/11] tests: add dummy ACPI tables for arm/virt board
2019-09-11 13:55 ` Michael S. Tsirkin
@ 2019-09-11 14:33 ` Shameerali Kolothum Thodi
2019-09-17 15:11 ` Michael S. Tsirkin
1 sibling, 0 replies; 29+ messages in thread
From: Shameerali Kolothum Thodi @ 2019-09-11 14:33 UTC (permalink / raw)
To: Michael S. Tsirkin, Igor Mammedov
Cc: Peter Maydell, Samuel Ortiz, Ard Biesheuvel, QEMU Developers,
Linuxarm, Shannon Zhao, qemu-arm, xuwei (O),
sebastien.boeuf, Laszlo Ersek, Eric Auger
> -----Original Message-----
> From: Qemu-devel
> [mailto:qemu-devel-bounces+shameerali.kolothum.thodi=huawei.com@nongn
> u.org] On Behalf Of Michael S. Tsirkin
> Sent: 11 September 2019 14:56
> To: Igor Mammedov <imammedo@redhat.com>
> Cc: Peter Maydell <peter.maydell@linaro.org>; Samuel Ortiz
> <sameo@linux.intel.com>; Ard Biesheuvel <ard.biesheuvel@linaro.org>;
> QEMU Developers <qemu-devel@nongnu.org>; Shameerali Kolothum Thodi
> <shameerali.kolothum.thodi@huawei.com>; Linuxarm
> <linuxarm@huawei.com>; Shannon Zhao <shannon.zhaosl@gmail.com>;
> qemu-arm <qemu-arm@nongnu.org>; xuwei (O) <xuwei5@huawei.com>; Eric
> Auger <eric.auger@redhat.com>; sebastien.boeuf@intel.com; Laszlo Ersek
> <lersek@redhat.com>
> Subject: Re: [Qemu-devel] [PATCH-for-4.2 v10 10/11] tests: add dummy ACPI
> tables for arm/virt board
>
> On Wed, Sep 11, 2019 at 03:50:15PM +0200, Igor Mammedov wrote:
> > On Wed, 11 Sep 2019 13:57:06 +0100
> > Peter Maydell <peter.maydell@linaro.org> wrote:
> >
> > > On Wed, 4 Sep 2019 at 09:58, Shameer Kolothum
> > > <shameerali.kolothum.thodi@huawei.com> wrote:
> > > >
> > > > This patch is in preparation for adding numamem and memhp tests
> > > > to arm/virt board so that 'make check' is happy. This may not
> > > > be required once the scripts are run and new tables are
> > > > generated with ".numamem" and ".memhp" extensions.
> > > >
> > > > Signed-off-by: Shameer Kolothum
> <shameerali.kolothum.thodi@huawei.com>
> > > > ---
> > > > I am not sure this is the right way to do this. But without this, when
> > > > the numamem and memhp tests are added, you will get,
> > > >
> > > > Looking for expected file 'tests/data/acpi/virt/SRAT.numamem'
> > > > Looking for expected file 'tests/data/acpi/virt/SRAT'
> > > > **
> > > > ERROR:tests/bios-tables-test.c:327:load_expected_aml: assertion failed:
> (exp_sdt.aml_file)
> > > >
> > > > ---
> > > > tests/data/acpi/virt/SLIT | Bin 0 -> 48 bytes
> > > > tests/data/acpi/virt/SRAT | Bin 0 -> 224 bytes
> > > > 2 files changed, 0 insertions(+), 0 deletions(-)
> > > > create mode 100644 tests/data/acpi/virt/SLIT
> > > > create mode 100644 tests/data/acpi/virt/SRAT
> > >
> > > Do the tests pass with this patch and without the
> > > patch that adds the tests? (That is, can we keep the
> > > two patches separate without breaking bisection, or
> > > do we need to squash them together?)
> > >
> > > I'll leave it to somebody who understands the ACPI
> > > tests stuff to answer whether there's a better way to
> > I'd squash this patch into 11/11 test case,
>
>
> Pls don't - the way to add this is to add the files in question to
> tests/bios-tables-test-allowed-diff.h.
IIRC, I have tried that but didn't work. I think the reason being, these
are new test cases for arm/virt and both SRAT and SLIT tables are not
present in the tests/data/acpi/virt folder.
As you can see the error is different,
> > > > Looking for expected file 'tests/data/acpi/virt/SRAT.numamem'
> > > > Looking for expected file 'tests/data/acpi/virt/SRAT'
> > > > **
> > > > ERROR:tests/bios-tables-test.c:327:load_expected_aml: assertion failed:
Not sure I missed anything though.
Thanks,
Shameer
> Maintainer will create a separate commit updating
> the binaries and removing them from the whitelist.
>
> This way things like rebase work seemlessly.
>
>
> > CCing Michael (since he's the one who applies ACPI patches)
> >
> > > do this.
> > >
> > > thanks
> > > -- PMM
> > >
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH-for-4.2 v10 05/11] hw/arm/virt: Enable device memory cold/hot plug with ACPI boot
2019-09-11 13:07 ` Igor Mammedov
@ 2019-09-16 10:30 ` Shameerali Kolothum Thodi
2019-09-17 15:25 ` Igor Mammedov
0 siblings, 1 reply; 29+ messages in thread
From: Shameerali Kolothum Thodi @ 2019-09-16 10:30 UTC (permalink / raw)
To: Igor Mammedov
Cc: peter.maydell, sameo, shannon.zhaosl, ard.biesheuvel, qemu-devel,
Linuxarm, eric.auger, qemu-arm, xuwei (O),
sebastien.boeuf, lersek
Hi Igor,
> -----Original Message-----
> From: Igor Mammedov [mailto:imammedo@redhat.com]
> Sent: 11 September 2019 14:07
> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>
> Cc: qemu-devel@nongnu.org; qemu-arm@nongnu.org;
> eric.auger@redhat.com; peter.maydell@linaro.org;
> shannon.zhaosl@gmail.com; sameo@linux.intel.com;
> sebastien.boeuf@intel.com; xuwei (O) <xuwei5@huawei.com>;
> lersek@redhat.com; ard.biesheuvel@linaro.org; Linuxarm
> <linuxarm@huawei.com>
> Subject: Re: [PATCH-for-4.2 v10 05/11] hw/arm/virt: Enable device memory
> cold/hot plug with ACPI boot
>
> On Wed, 4 Sep 2019 09:56:23 +0100
> Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> wrote:
>
> [...]
> > @@ -730,6 +733,19 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> VirtMachineState *vms)
> > vms->highmem, vms->highmem_ecam);
> > acpi_dsdt_add_gpio(scope, &memmap[VIRT_GPIO],
> > (irqmap[VIRT_GPIO] + ARM_SPI_BASE));
> > + if (vms->acpi_dev) {
> > + build_ged_aml(scope, "\\_SB."GED_DEVICE,
> > + HOTPLUG_HANDLER(vms->acpi_dev),
> > + irqmap[VIRT_ACPI_GED] + ARM_SPI_BASE,
> AML_SYSTEM_MEMORY,
> > + memmap[VIRT_ACPI_GED].base);
> > + }
> > +
> > + if (vms->acpi_dev && ms->ram_slots) {
> > + build_memory_hotplug_aml(scope, ms->ram_slots, "\\_SB",
> NULL,
> > + AML_SYSTEM_MEMORY,
> > +
> memmap[VIRT_PCDIMM_ACPI].base);
> > + }
> one more thing (though non critical), if ms->ram_slots == 0 ^^^^
> makes IASL spew a warning
>
> External (_SB_.MHPC.MSCN, MethodObj) // Warning: Unknown
> method, guessing 0 arguments
>
> In general non-existing references within methods are fine if they aren't ever
> used.
> however we can be a little bit less sloppy.
> Below you advertise "event = ACPI_GED_MEM_HOTPLUG_EVT", and then here
> suddenly
> don't generate essential AML part for it here.
Ok.
> For consistency if above is conditioned on ms->ram_slots != 0, probably
> it would be better to move that condition where you set 'event' value and
> check property value above instead of ms->ram_slots
I understand the concern here, but not sure I get the suggestion to check
the "property" instead of ms->ram_slots correctly.
Is this what you have in mind?
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 538b3bbefa..5c9269dca1 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -742,10 +742,15 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
memmap[VIRT_ACPI_GED].base);
}
- if (vms->acpi_dev && ms->ram_slots) {
- build_memory_hotplug_aml(scope, ms->ram_slots, "\\_SB", NULL,
- AML_SYSTEM_MEMORY,
- memmap[VIRT_PCDIMM_ACPI].base);
+ if (vms->acpi_dev) {
+ uint32_t event = object_property_get_uint(OBJECT(vms->acpi_dev),
+ "ged-event", NULL);
+
+ if (event & ACPI_GED_MEM_HOTPLUG_EVT) {
+ build_memory_hotplug_aml(scope, ms->ram_slots, "\\_SB", NULL,
+ AML_SYSTEM_MEMORY,
+ memmap[VIRT_PCDIMM_ACPI].base);
+ }
}
acpi_dsdt_add_power_button(scope);
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index bc152ea2b0..6b024b16df 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -534,8 +534,13 @@ static void fdt_add_pmu_nodes(const VirtMachineState *vms)
static inline DeviceState *create_acpi_ged(VirtMachineState *vms, qemu_irq *pic)
{
DeviceState *dev;
+ MachineState *ms = MACHINE(vms);
int irq = vms->irqmap[VIRT_ACPI_GED];
- uint32_t event = ACPI_GED_MEM_HOTPLUG_EVT;
+ uint32_t event = 0;
+
+ if (ms->ram_slots) {
+ event = ACPI_GED_MEM_HOTPLUG_EVT;
+ }
dev = qdev_create(NULL, TYPE_ACPI_GED);
qdev_prop_set_uint32(dev, "ged-event", event);
---8---
Please let me know.
Thanks,
Shameer
> [...]
> > +static inline DeviceState *create_acpi_ged(VirtMachineState *vms,
> qemu_irq *pic)
> > +{
> > + DeviceState *dev;
> > + int irq = vms->irqmap[VIRT_ACPI_GED];
> > + uint32_t event = ACPI_GED_MEM_HOTPLUG_EVT;
> > +
> > + dev = qdev_create(NULL, TYPE_ACPI_GED);
> > + qdev_prop_set_uint32(dev, "ged-event", event);
> > + qdev_init_nofail(dev);
> > +
> > + sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0,
> vms->memmap[VIRT_ACPI_GED].base);
> > + sysbus_mmio_map(SYS_BUS_DEVICE(dev), 1,
> vms->memmap[VIRT_PCDIMM_ACPI].base);
> > +
> > + sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[irq]);
> > +
> > + return dev;
> > +}
> > +
> [...]
^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH-for-4.2 v10 10/11] tests: add dummy ACPI tables for arm/virt board
2019-09-11 13:55 ` Michael S. Tsirkin
2019-09-11 14:33 ` Shameerali Kolothum Thodi
@ 2019-09-17 15:11 ` Michael S. Tsirkin
2019-09-17 15:23 ` Peter Maydell
1 sibling, 1 reply; 29+ messages in thread
From: Michael S. Tsirkin @ 2019-09-17 15:11 UTC (permalink / raw)
To: Igor Mammedov
Cc: Peter Maydell, Samuel Ortiz, Ard Biesheuvel, QEMU Developers,
Shameer Kolothum, Linuxarm, Shannon Zhao, qemu-arm, Xu Wei,
Eric Auger, sebastien.boeuf, Laszlo Ersek
On Wed, Sep 11, 2019 at 09:55:34AM -0400, Michael S. Tsirkin wrote:
> On Wed, Sep 11, 2019 at 03:50:15PM +0200, Igor Mammedov wrote:
> > On Wed, 11 Sep 2019 13:57:06 +0100
> > Peter Maydell <peter.maydell@linaro.org> wrote:
> >
> > > On Wed, 4 Sep 2019 at 09:58, Shameer Kolothum
> > > <shameerali.kolothum.thodi@huawei.com> wrote:
> > > >
> > > > This patch is in preparation for adding numamem and memhp tests
> > > > to arm/virt board so that 'make check' is happy. This may not
> > > > be required once the scripts are run and new tables are
> > > > generated with ".numamem" and ".memhp" extensions.
> > > >
> > > > Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
> > > > ---
> > > > I am not sure this is the right way to do this. But without this, when
> > > > the numamem and memhp tests are added, you will get,
> > > >
> > > > Looking for expected file 'tests/data/acpi/virt/SRAT.numamem'
> > > > Looking for expected file 'tests/data/acpi/virt/SRAT'
> > > > **
> > > > ERROR:tests/bios-tables-test.c:327:load_expected_aml: assertion failed: (exp_sdt.aml_file)
> > > >
> > > > ---
> > > > tests/data/acpi/virt/SLIT | Bin 0 -> 48 bytes
> > > > tests/data/acpi/virt/SRAT | Bin 0 -> 224 bytes
> > > > 2 files changed, 0 insertions(+), 0 deletions(-)
> > > > create mode 100644 tests/data/acpi/virt/SLIT
> > > > create mode 100644 tests/data/acpi/virt/SRAT
> > >
> > > Do the tests pass with this patch and without the
> > > patch that adds the tests? (That is, can we keep the
> > > two patches separate without breaking bisection, or
> > > do we need to squash them together?)
> > >
> > > I'll leave it to somebody who understands the ACPI
> > > tests stuff to answer whether there's a better way to
> > I'd squash this patch into 11/11 test case,
>
>
> Pls don't - the way to add this is to add the files in question to
> tests/bios-tables-test-allowed-diff.h.
>
> Maintainer will create a separate commit updating
> the binaries and removing them from the whitelist.
>
> This way things like rebase work seemlessly.
OK?
Can you post v11 like this? I'll ack then.
>
> > CCing Michael (since he's the one who applies ACPI patches)
> >
> > > do this.
> > >
> > > thanks
> > > -- PMM
> > >
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH-for-4.2 v10 10/11] tests: add dummy ACPI tables for arm/virt board
2019-09-17 15:11 ` Michael S. Tsirkin
@ 2019-09-17 15:23 ` Peter Maydell
0 siblings, 0 replies; 29+ messages in thread
From: Peter Maydell @ 2019-09-17 15:23 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Samuel Ortiz, Ard Biesheuvel, QEMU Developers, Shameer Kolothum,
Linuxarm, Shannon Zhao, qemu-arm, Xu Wei, Eric Auger,
Igor Mammedov, sebastien.boeuf, Laszlo Ersek
On Tue, 17 Sep 2019 at 16:11, Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Wed, Sep 11, 2019 at 09:55:34AM -0400, Michael S. Tsirkin wrote:
> > On Wed, Sep 11, 2019 at 03:50:15PM +0200, Igor Mammedov wrote:
> > > On Wed, 11 Sep 2019 13:57:06 +0100
> > > Peter Maydell <peter.maydell@linaro.org> wrote:
> > > > Do the tests pass with this patch and without the
> > > > patch that adds the tests? (That is, can we keep the
> > > > two patches separate without breaking bisection, or
> > > > do we need to squash them together?)
> > > >
> > > > I'll leave it to somebody who understands the ACPI
> > > > tests stuff to answer whether there's a better way to
> > > I'd squash this patch into 11/11 test case,
> >
> >
> > Pls don't - the way to add this is to add the files in question to
> > tests/bios-tables-test-allowed-diff.h.
> >
> > Maintainer will create a separate commit updating
> > the binaries and removing them from the whitelist.
Who is "maintainer" in this part of the process? Why
can't the submitter just create the patches and send them?
> > This way things like rebase work seemlessly.
>
>
> OK?
>
> Can you post v11 like this? I'll ack then.
thanks
-- PMM
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH-for-4.2 v10 05/11] hw/arm/virt: Enable device memory cold/hot plug with ACPI boot
2019-09-16 10:30 ` Shameerali Kolothum Thodi
@ 2019-09-17 15:25 ` Igor Mammedov
0 siblings, 0 replies; 29+ messages in thread
From: Igor Mammedov @ 2019-09-17 15:25 UTC (permalink / raw)
To: Shameerali Kolothum Thodi
Cc: peter.maydell, sameo, shannon.zhaosl, ard.biesheuvel, qemu-devel,
Linuxarm, eric.auger, qemu-arm, xuwei (O),
sebastien.boeuf, lersek
On Mon, 16 Sep 2019 10:30:45 +0000
Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com> wrote:
> Hi Igor,
>
> > -----Original Message-----
> > From: Igor Mammedov [mailto:imammedo@redhat.com]
> > Sent: 11 September 2019 14:07
> > To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>
> > Cc: qemu-devel@nongnu.org; qemu-arm@nongnu.org;
> > eric.auger@redhat.com; peter.maydell@linaro.org;
> > shannon.zhaosl@gmail.com; sameo@linux.intel.com;
> > sebastien.boeuf@intel.com; xuwei (O) <xuwei5@huawei.com>;
> > lersek@redhat.com; ard.biesheuvel@linaro.org; Linuxarm
> > <linuxarm@huawei.com>
> > Subject: Re: [PATCH-for-4.2 v10 05/11] hw/arm/virt: Enable device memory
> > cold/hot plug with ACPI boot
> >
> > On Wed, 4 Sep 2019 09:56:23 +0100
> > Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> wrote:
> >
> > [...]
> > > @@ -730,6 +733,19 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> > VirtMachineState *vms)
> > > vms->highmem, vms->highmem_ecam);
> > > acpi_dsdt_add_gpio(scope, &memmap[VIRT_GPIO],
> > > (irqmap[VIRT_GPIO] + ARM_SPI_BASE));
> > > + if (vms->acpi_dev) {
> > > + build_ged_aml(scope, "\\_SB."GED_DEVICE,
> > > + HOTPLUG_HANDLER(vms->acpi_dev),
> > > + irqmap[VIRT_ACPI_GED] + ARM_SPI_BASE,
> > AML_SYSTEM_MEMORY,
> > > + memmap[VIRT_ACPI_GED].base);
> > > + }
> > > +
> > > + if (vms->acpi_dev && ms->ram_slots) {
> > > + build_memory_hotplug_aml(scope, ms->ram_slots, "\\_SB",
> > NULL,
> > > + AML_SYSTEM_MEMORY,
> > > +
> > memmap[VIRT_PCDIMM_ACPI].base);
> > > + }
> > one more thing (though non critical), if ms->ram_slots == 0 ^^^^
> > makes IASL spew a warning
> >
> > External (_SB_.MHPC.MSCN, MethodObj) // Warning: Unknown
> > method, guessing 0 arguments
> >
> > In general non-existing references within methods are fine if they aren't ever
> > used.
> > however we can be a little bit less sloppy.
> > Below you advertise "event = ACPI_GED_MEM_HOTPLUG_EVT", and then here
> > suddenly
> > don't generate essential AML part for it here.
>
> Ok.
>
> > For consistency if above is conditioned on ms->ram_slots != 0, probably
> > it would be better to move that condition where you set 'event' value and
> > check property value above instead of ms->ram_slots
>
> I understand the concern here, but not sure I get the suggestion to check
> the "property" instead of ms->ram_slots correctly.
>
> Is this what you have in mind?
>
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 538b3bbefa..5c9269dca1 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -742,10 +742,15 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
> memmap[VIRT_ACPI_GED].base);
> }
>
> - if (vms->acpi_dev && ms->ram_slots) {
> - build_memory_hotplug_aml(scope, ms->ram_slots, "\\_SB", NULL,
> - AML_SYSTEM_MEMORY,
> - memmap[VIRT_PCDIMM_ACPI].base);
> + if (vms->acpi_dev) {
> + uint32_t event = object_property_get_uint(OBJECT(vms->acpi_dev),
> + "ged-event", NULL);
s/NULL/error_abort/
> +
> + if (event & ACPI_GED_MEM_HOTPLUG_EVT) {
> + build_memory_hotplug_aml(scope, ms->ram_slots, "\\_SB", NULL,
> + AML_SYSTEM_MEMORY,
> + memmap[VIRT_PCDIMM_ACPI].base);
> + }
> }
>
> acpi_dsdt_add_power_button(scope);
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index bc152ea2b0..6b024b16df 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -534,8 +534,13 @@ static void fdt_add_pmu_nodes(const VirtMachineState *vms)
> static inline DeviceState *create_acpi_ged(VirtMachineState *vms, qemu_irq *pic)
> {
> DeviceState *dev;
> + MachineState *ms = MACHINE(vms);
> int irq = vms->irqmap[VIRT_ACPI_GED];
> - uint32_t event = ACPI_GED_MEM_HOTPLUG_EVT;
> + uint32_t event = 0;
> +
> + if (ms->ram_slots) {
> + event = ACPI_GED_MEM_HOTPLUG_EVT;
> + }
>
> dev = qdev_create(NULL, TYPE_ACPI_GED);
> qdev_prop_set_uint32(dev, "ged-event", event);
>
> ---8---
>
> Please let me know.
Looks good to me.
Thanks!
>
> Thanks,
> Shameer
>
>
> > [...]
> > > +static inline DeviceState *create_acpi_ged(VirtMachineState *vms,
> > qemu_irq *pic)
> > > +{
> > > + DeviceState *dev;
> > > + int irq = vms->irqmap[VIRT_ACPI_GED];
> > > + uint32_t event = ACPI_GED_MEM_HOTPLUG_EVT;
> > > +
> > > + dev = qdev_create(NULL, TYPE_ACPI_GED);
> > > + qdev_prop_set_uint32(dev, "ged-event", event);
> > > + qdev_init_nofail(dev);
> > > +
> > > + sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0,
> > vms->memmap[VIRT_ACPI_GED].base);
> > > + sysbus_mmio_map(SYS_BUS_DEVICE(dev), 1,
> > vms->memmap[VIRT_PCDIMM_ACPI].base);
> > > +
> > > + sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[irq]);
> > > +
> > > + return dev;
> > > +}
> > > +
> > [...]
^ permalink raw reply [flat|nested] 29+ messages in thread
end of thread, other threads:[~2019-09-17 15:30 UTC | newest]
Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-04 8:56 [Qemu-devel] [PATCH-for-4.2 v10 00/11] ARM virt: ACPI memory hotplug support Shameer Kolothum
2019-09-04 8:56 ` [Qemu-devel] [PATCH-for-4.2 v10 01/11] hw/acpi: Make ACPI IO address space configurable Shameer Kolothum
2019-09-04 11:27 ` Igor Mammedov
2019-09-04 8:56 ` [Qemu-devel] [PATCH-for-4.2 v10 02/11] hw/acpi: Do not create memory hotplug method when handler is not defined Shameer Kolothum
2019-09-04 8:56 ` [Qemu-devel] [PATCH-for-4.2 v10 03/11] hw/acpi: Add ACPI Generic Event Device Support Shameer Kolothum
2019-09-11 12:34 ` Igor Mammedov
2019-09-04 8:56 ` [Qemu-devel] [PATCH-for-4.2 v10 04/11] hw/arm/virt: Add memory hotplug framework Shameer Kolothum
2019-09-04 8:56 ` [Qemu-devel] [PATCH-for-4.2 v10 05/11] hw/arm/virt: Enable device memory cold/hot plug with ACPI boot Shameer Kolothum
2019-09-11 12:48 ` Igor Mammedov
2019-09-11 13:07 ` Igor Mammedov
2019-09-16 10:30 ` Shameerali Kolothum Thodi
2019-09-17 15:25 ` Igor Mammedov
2019-09-04 8:56 ` [Qemu-devel] [PATCH-for-4.2 v10 06/11] hw/arm/virt-acpi-build: Add PC-DIMM in SRAT Shameer Kolothum
2019-09-11 13:14 ` Igor Mammedov
2019-09-04 8:56 ` [Qemu-devel] [PATCH-for-4.2 v10 07/11] hw/arm: Factor out powerdown notifier from GPIO Shameer Kolothum
2019-09-11 13:08 ` Igor Mammedov
2019-09-04 8:56 ` [Qemu-devel] [PATCH-for-4.2 v10 08/11] hw/arm: Use GED for system_powerdown event Shameer Kolothum
2019-09-11 13:20 ` Igor Mammedov
2019-09-04 8:56 ` [Qemu-devel] [PATCH-for-4.2 v10 09/11] docs/specs: Add ACPI GED documentation Shameer Kolothum
2019-09-11 12:54 ` Peter Maydell
2019-09-04 8:56 ` [Qemu-devel] [PATCH-for-4.2 v10 10/11] tests: add dummy ACPI tables for arm/virt board Shameer Kolothum
2019-09-11 12:57 ` Peter Maydell
2019-09-11 13:50 ` Igor Mammedov
2019-09-11 13:55 ` Michael S. Tsirkin
2019-09-11 14:33 ` Shameerali Kolothum Thodi
2019-09-17 15:11 ` Michael S. Tsirkin
2019-09-17 15:23 ` Peter Maydell
2019-09-04 8:56 ` [Qemu-devel] [PATCH-for-4.2 v10 11/11] tests: Add bios tests to arm/virt Shameer Kolothum
2019-09-11 13:46 ` Igor Mammedov
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.