* [PATCH v4 1/7] hw/acpi/nvdimm: Fix for NVDIMM incorrect DSM output buffer length
2020-04-21 12:59 [PATCH v4 0/7] ARM virt: Add NVDIMM support Shameer Kolothum
@ 2020-04-21 12:59 ` Shameer Kolothum
2020-04-21 12:59 ` [PATCH v4 2/7] nvdimm: Use configurable ACPI IO base and size Shameer Kolothum
` (7 subsequent siblings)
8 siblings, 0 replies; 17+ messages in thread
From: Shameer Kolothum @ 2020-04-21 12:59 UTC (permalink / raw)
To: qemu-devel, qemu-arm, eric.auger, imammedo
Cc: peter.maydell, xiaoguangrong.eric, mst, linuxarm, xuwei5,
shannon.zhaosl, prime.zeng, lersek
As per ACPI spec 6.3, Table 19-419 Object Conversion Rules, if
the Buffer Field <= to the size of an Integer (in bits), it will
be treated as an integer. Moreover, the integer size depends on
DSDT tables revision number. If revision number is < 2, integer
size is 32 bits, otherwise it is 64 bits. Current NVDIMM common
DSM aml code (NCAL) uses CreateField() for creating DSM output
buffer. This creates an issue in arm/virt platform where DSDT
revision number is 2 and results in DSM buffer with a wrong
size(8 bytes) gets returned when actual length is < 8 bytes.
This causes guest kernel to report,
"nfit ACPI0012:00: found a zero length table '0' parsing nfit"
In order to fix this, aml code is now modified such that it builds
the DSM output buffer in a byte by byte fashion when length is
smaller than Integer size.
Suggested-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
---
hw/acpi/nvdimm.c | 40 +++++++++++++++++++--
tests/qtest/bios-tables-test-allowed-diff.h | 2 ++
2 files changed, 39 insertions(+), 3 deletions(-)
diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index eb6a37b14e..df0790719a 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -938,6 +938,7 @@ static void nvdimm_build_common_dsm(Aml *dev)
Aml *method, *ifctx, *function, *handle, *uuid, *dsm_mem, *elsectx2;
Aml *elsectx, *unsupport, *unpatched, *expected_uuid, *uuid_invalid;
Aml *pckg, *pckg_index, *pckg_buf, *field, *dsm_out_buf, *dsm_out_buf_size;
+ Aml *whilectx, *offset;
uint8_t byte_list[1];
method = aml_method(NVDIMM_COMMON_DSM, 5, AML_SERIALIZED);
@@ -1091,13 +1092,46 @@ static void nvdimm_build_common_dsm(Aml *dev)
/* RLEN is not included in the payload returned to guest. */
aml_append(method, aml_subtract(aml_name(NVDIMM_DSM_OUT_BUF_SIZE),
aml_int(4), dsm_out_buf_size));
+
+ /*
+ * As per ACPI spec 6.3, Table 19-419 Object Conversion Rules, if
+ * the Buffer Field <= to the size of an Integer (in bits), it will
+ * be treated as an integer. Moreover, the integer size depends on
+ * DSDT tables revision number. If revision number is < 2, integer
+ * size is 32 bits, otherwise it is 64 bits.
+ * Because of this CreateField() canot be used if RLEN < Integer Size.
+ *
+ * Also please note that APCI ASL operator SizeOf() doesn't support
+ * Integer and there isn't any other way to figure out the Integer
+ * size. Hence we assume 8 byte as Integer size and if RLEN < 8 bytes,
+ * build dsm_out_buf byte by byte.
+ */
+ ifctx = aml_if(aml_lless(dsm_out_buf_size, aml_int(8)));
+ offset = aml_local(2);
+ aml_append(ifctx, aml_store(aml_int(0), offset));
+ aml_append(ifctx, aml_name_decl("TBUF", aml_buffer(1, NULL)));
+ aml_append(ifctx, aml_store(aml_buffer(0, NULL), dsm_out_buf));
+
+ whilectx = aml_while(aml_lless(offset, dsm_out_buf_size));
+ /* Copy 1 byte at offset from ODAT to temporary buffer(TBUF). */
+ aml_append(whilectx, aml_store(aml_derefof(aml_index(
+ aml_name(NVDIMM_DSM_OUT_BUF), offset)),
+ aml_index(aml_name("TBUF"), aml_int(0))));
+ aml_append(whilectx, aml_concatenate(dsm_out_buf, aml_name("TBUF"),
+ dsm_out_buf));
+ aml_append(whilectx, aml_increment(offset));
+ aml_append(ifctx, whilectx);
+
+ aml_append(ifctx, aml_return(dsm_out_buf));
+ aml_append(method, ifctx);
+
+ /* If RLEN >= Integer size, just use CreateField() operator */
aml_append(method, aml_store(aml_shiftleft(dsm_out_buf_size, aml_int(3)),
dsm_out_buf_size));
aml_append(method, aml_create_field(aml_name(NVDIMM_DSM_OUT_BUF),
aml_int(0), dsm_out_buf_size, "OBUF"));
- aml_append(method, aml_concatenate(aml_buffer(0, NULL), aml_name("OBUF"),
- dsm_out_buf));
- aml_append(method, aml_return(dsm_out_buf));
+ aml_append(method, aml_return(aml_name("OBUF")));
+
aml_append(dev, method);
}
diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index dfb8523c8b..eb8bae1407 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1 +1,3 @@
/* List of comma-separated changed AML files to ignore */
+"tests/data/acpi/pc/SSDT.dimmpxm",
+"tests/data/acpi/q35/SSDT.dimmpxm",
--
2.17.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v4 2/7] nvdimm: Use configurable ACPI IO base and size
2020-04-21 12:59 [PATCH v4 0/7] ARM virt: Add NVDIMM support Shameer Kolothum
2020-04-21 12:59 ` [PATCH v4 1/7] hw/acpi/nvdimm: Fix for NVDIMM incorrect DSM output buffer length Shameer Kolothum
@ 2020-04-21 12:59 ` Shameer Kolothum
2020-04-21 12:59 ` [PATCH v4 3/7] hw/arm/virt: Add nvdimm hot-plug infrastructure Shameer Kolothum
` (6 subsequent siblings)
8 siblings, 0 replies; 17+ messages in thread
From: Shameer Kolothum @ 2020-04-21 12:59 UTC (permalink / raw)
To: qemu-devel, qemu-arm, eric.auger, imammedo
Cc: peter.maydell, xiaoguangrong.eric, mst, linuxarm, xuwei5,
shannon.zhaosl, prime.zeng, lersek
From: Kwangwoo Lee <kwangwoo.lee@sk.com>
This patch makes IO base and size configurable to create NPIO AML for
ACPI NFIT. Since a different architecture like AArch64 does not use
port-mapped IO, a configurable IO base is required to create correct
mapping of ACPI IO address and size.
Signed-off-by: Kwangwoo Lee <kwangwoo.lee@sk.com>
Signed-off-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
---
hw/acpi/nvdimm.c | 32 ++++++++++++++++++++++----------
hw/i386/acpi-build.c | 6 ++++++
hw/i386/acpi-build.h | 3 +++
hw/i386/pc_piix.c | 2 ++
hw/i386/pc_q35.c | 2 ++
include/hw/mem/nvdimm.h | 3 +++
6 files changed, 38 insertions(+), 10 deletions(-)
diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index df0790719a..fa7bf8b507 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -900,11 +900,13 @@ void nvdimm_acpi_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev)
}
void nvdimm_init_acpi_state(NVDIMMState *state, MemoryRegion *io,
+ struct AcpiGenericAddress dsm_io,
FWCfgState *fw_cfg, Object *owner)
{
+ state->dsm_io = dsm_io;
memory_region_init_io(&state->io_mr, owner, &nvdimm_dsm_ops, state,
- "nvdimm-acpi-io", NVDIMM_ACPI_IO_LEN);
- memory_region_add_subregion(io, NVDIMM_ACPI_IO_BASE, &state->io_mr);
+ "nvdimm-acpi-io", dsm_io.bit_width >> 3);
+ memory_region_add_subregion(io, dsm_io.address, &state->io_mr);
state->dsm_mem = g_array_new(false, true /* clear */, 1);
acpi_data_push(state->dsm_mem, sizeof(NvdimmDsmIn));
@@ -933,13 +935,15 @@ void nvdimm_init_acpi_state(NVDIMMState *state, MemoryRegion *io,
#define NVDIMM_QEMU_RSVD_UUID "648B9CF2-CDA1-4312-8AD9-49C4AF32BD62"
-static void nvdimm_build_common_dsm(Aml *dev)
+static void nvdimm_build_common_dsm(Aml *dev,
+ NVDIMMState *nvdimm_state)
{
Aml *method, *ifctx, *function, *handle, *uuid, *dsm_mem, *elsectx2;
Aml *elsectx, *unsupport, *unpatched, *expected_uuid, *uuid_invalid;
Aml *pckg, *pckg_index, *pckg_buf, *field, *dsm_out_buf, *dsm_out_buf_size;
Aml *whilectx, *offset;
uint8_t byte_list[1];
+ AmlRegionSpace rs;
method = aml_method(NVDIMM_COMMON_DSM, 5, AML_SERIALIZED);
uuid = aml_arg(0);
@@ -950,9 +954,16 @@ static void nvdimm_build_common_dsm(Aml *dev)
aml_append(method, aml_store(aml_name(NVDIMM_ACPI_MEM_ADDR), dsm_mem));
+ if (nvdimm_state->dsm_io.space_id == AML_AS_SYSTEM_IO) {
+ rs = AML_SYSTEM_IO;
+ } else {
+ rs = AML_SYSTEM_MEMORY;
+ }
+
/* map DSM memory and IO into ACPI namespace. */
- aml_append(method, aml_operation_region(NVDIMM_DSM_IOPORT, AML_SYSTEM_IO,
- aml_int(NVDIMM_ACPI_IO_BASE), NVDIMM_ACPI_IO_LEN));
+ aml_append(method, aml_operation_region(NVDIMM_DSM_IOPORT, rs,
+ aml_int(nvdimm_state->dsm_io.address),
+ nvdimm_state->dsm_io.bit_width >> 3));
aml_append(method, aml_operation_region(NVDIMM_DSM_MEMORY,
AML_SYSTEM_MEMORY, dsm_mem, sizeof(NvdimmDsmIn)));
@@ -967,7 +978,7 @@ static void nvdimm_build_common_dsm(Aml *dev)
field = aml_field(NVDIMM_DSM_IOPORT, AML_DWORD_ACC, AML_NOLOCK,
AML_PRESERVE);
aml_append(field, aml_named_field(NVDIMM_DSM_NOTIFY,
- NVDIMM_ACPI_IO_LEN * BITS_PER_BYTE));
+ nvdimm_state->dsm_io.bit_width));
aml_append(method, field);
/*
@@ -1268,7 +1279,8 @@ static void nvdimm_build_nvdimm_devices(Aml *root_dev, uint32_t ram_slots)
}
static void nvdimm_build_ssdt(GArray *table_offsets, GArray *table_data,
- BIOSLinker *linker, GArray *dsm_dma_area,
+ BIOSLinker *linker,
+ NVDIMMState *nvdimm_state,
uint32_t ram_slots)
{
Aml *ssdt, *sb_scope, *dev;
@@ -1296,7 +1308,7 @@ static void nvdimm_build_ssdt(GArray *table_offsets, GArray *table_data,
*/
aml_append(dev, aml_name_decl("_HID", aml_string("ACPI0012")));
- nvdimm_build_common_dsm(dev);
+ nvdimm_build_common_dsm(dev, nvdimm_state);
/* 0 is reserved for root device. */
nvdimm_build_device_dsm(dev, 0);
@@ -1315,7 +1327,7 @@ static void nvdimm_build_ssdt(GArray *table_offsets, GArray *table_data,
NVDIMM_ACPI_MEM_ADDR);
bios_linker_loader_alloc(linker,
- NVDIMM_DSM_MEM_FILE, dsm_dma_area,
+ NVDIMM_DSM_MEM_FILE, nvdimm_state->dsm_mem,
sizeof(NvdimmDsmIn), false /* high memory */);
bios_linker_loader_add_pointer(linker,
ACPI_BUILD_TABLE_FILE, mem_addr_offset, sizeof(uint32_t),
@@ -1337,7 +1349,7 @@ void nvdimm_build_acpi(GArray *table_offsets, GArray *table_data,
return;
}
- nvdimm_build_ssdt(table_offsets, table_data, linker, state->dsm_mem,
+ nvdimm_build_ssdt(table_offsets, table_data, linker, state,
ram_slots);
device_list = nvdimm_get_device_list();
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 23c77eeb95..1646077035 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -128,6 +128,12 @@ typedef struct FwCfgTPMConfig {
static bool acpi_get_mcfg(AcpiMcfgInfo *mcfg);
+const struct AcpiGenericAddress x86_nvdimm_acpi_dsmio = {
+ .space_id = AML_AS_SYSTEM_IO,
+ .address = NVDIMM_ACPI_IO_BASE,
+ .bit_width = NVDIMM_ACPI_IO_LEN << 3
+};
+
static void init_common_fadt_data(MachineState *ms, Object *o,
AcpiFadtData *data)
{
diff --git a/hw/i386/acpi-build.h b/hw/i386/acpi-build.h
index 007332e51c..74df5fc612 100644
--- a/hw/i386/acpi-build.h
+++ b/hw/i386/acpi-build.h
@@ -1,6 +1,9 @@
#ifndef HW_I386_ACPI_BUILD_H
#define HW_I386_ACPI_BUILD_H
+#include "hw/acpi/acpi-defs.h"
+
+extern const struct AcpiGenericAddress x86_nvdimm_acpi_dsmio;
void acpi_setup(void);
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 22dee0e76c..b75087d71b 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -61,6 +61,7 @@
#include "migration/misc.h"
#include "sysemu/numa.h"
#include "hw/mem/nvdimm.h"
+#include "hw/i386/acpi-build.h"
#define MAX_IDE_BUS 2
@@ -297,6 +298,7 @@ static void pc_init1(MachineState *machine,
if (machine->nvdimms_state->is_enabled) {
nvdimm_init_acpi_state(machine->nvdimms_state, system_io,
+ x86_nvdimm_acpi_dsmio,
x86ms->fw_cfg, OBJECT(pcms));
}
}
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index d37c425e22..d2806c1b29 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -54,6 +54,7 @@
#include "qemu/error-report.h"
#include "sysemu/numa.h"
#include "hw/mem/nvdimm.h"
+#include "hw/i386/acpi-build.h"
/* ICH9 AHCI has 6 ports */
#define MAX_SATA_PORTS 6
@@ -315,6 +316,7 @@ static void pc_q35_init(MachineState *machine)
if (machine->nvdimms_state->is_enabled) {
nvdimm_init_acpi_state(machine->nvdimms_state, system_io,
+ x86_nvdimm_acpi_dsmio,
x86ms->fw_cfg, OBJECT(pcms));
}
}
diff --git a/include/hw/mem/nvdimm.h b/include/hw/mem/nvdimm.h
index 4807ca615b..a3c08955e8 100644
--- a/include/hw/mem/nvdimm.h
+++ b/include/hw/mem/nvdimm.h
@@ -26,6 +26,7 @@
#include "hw/mem/pc-dimm.h"
#include "hw/acpi/bios-linker-loader.h"
#include "qemu/uuid.h"
+#include "hw/acpi/aml-build.h"
#define NVDIMM_DEBUG 0
#define nvdimm_debug(fmt, ...) \
@@ -147,10 +148,12 @@ struct NVDIMMState {
*/
int32_t persistence;
char *persistence_string;
+ struct AcpiGenericAddress dsm_io;
};
typedef struct NVDIMMState NVDIMMState;
void nvdimm_init_acpi_state(NVDIMMState *state, MemoryRegion *io,
+ struct AcpiGenericAddress dsm_io,
FWCfgState *fw_cfg, Object *owner);
void nvdimm_build_acpi(GArray *table_offsets, GArray *table_data,
BIOSLinker *linker, NVDIMMState *state,
--
2.17.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v4 3/7] hw/arm/virt: Add nvdimm hot-plug infrastructure
2020-04-21 12:59 [PATCH v4 0/7] ARM virt: Add NVDIMM support Shameer Kolothum
2020-04-21 12:59 ` [PATCH v4 1/7] hw/acpi/nvdimm: Fix for NVDIMM incorrect DSM output buffer length Shameer Kolothum
2020-04-21 12:59 ` [PATCH v4 2/7] nvdimm: Use configurable ACPI IO base and size Shameer Kolothum
@ 2020-04-21 12:59 ` Shameer Kolothum
2020-04-21 12:59 ` [PATCH v4 4/7] hw/arm/virt: Add nvdimm hotplug support Shameer Kolothum
` (5 subsequent siblings)
8 siblings, 0 replies; 17+ messages in thread
From: Shameer Kolothum @ 2020-04-21 12:59 UTC (permalink / raw)
To: qemu-devel, qemu-arm, eric.auger, imammedo
Cc: peter.maydell, xiaoguangrong.eric, mst, linuxarm, xuwei5,
shannon.zhaosl, prime.zeng, lersek
From: Kwangwoo Lee <kwangwoo.lee@sk.com>
This adds support to init nvdimm acpi state and build nvdimm acpi tables.
Please note nvdimm_support is not yet enabled.
Signed-off-by: Kwangwoo Lee <kwangwoo.lee@sk.com>
Signed-off-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
---
hw/arm/Kconfig | 1 +
hw/arm/virt-acpi-build.c | 6 ++++++
hw/arm/virt.c | 19 +++++++++++++++++++
hw/mem/Kconfig | 2 +-
include/hw/arm/virt.h | 1 +
5 files changed, 28 insertions(+), 1 deletion(-)
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 188419dc1e..5364172537 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -25,6 +25,7 @@ config ARM_VIRT
select DIMM
select ACPI_MEMORY_HOTPLUG
select ACPI_HW_REDUCED
+ select ACPI_NVDIMM
config CHEETAH
bool
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 81d41a3990..f22b1e6097 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -44,6 +44,7 @@
#include "hw/pci/pcie_host.h"
#include "hw/pci/pci.h"
#include "hw/arm/virt.h"
+#include "hw/mem/nvdimm.h"
#include "sysemu/numa.h"
#include "sysemu/reset.h"
#include "kvm_arm.h"
@@ -826,6 +827,11 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables)
}
}
+ if (ms->nvdimms_state->is_enabled) {
+ nvdimm_build_acpi(table_offsets, tables_blob, tables->linker,
+ ms->nvdimms_state, ms->ram_slots);
+ }
+
if (its_class_name() && !vmc->no_its) {
acpi_add_table(table_offsets, tables_blob);
build_iort(tables_blob, tables->linker, vms);
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 7dc96abf72..8a2beaf650 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -149,6 +149,7 @@ static const MemMapEntry base_memmap[] = {
[VIRT_SMMU] = { 0x09050000, 0x00020000 },
[VIRT_PCDIMM_ACPI] = { 0x09070000, MEMORY_HOTPLUG_IO_LEN },
[VIRT_ACPI_GED] = { 0x09080000, ACPI_GED_EVT_SEL_LEN },
+ [VIRT_NVDIMM_ACPI] = { 0x09090000, NVDIMM_ACPI_IO_LEN},
[VIRT_MMIO] = { 0x0a000000, 0x00000200 },
/* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */
[VIRT_PLATFORM_BUS] = { 0x0c000000, 0x02000000 },
@@ -1866,6 +1867,18 @@ static void machvirt_init(MachineState *machine)
create_platform_bus(vms);
+ if (machine->nvdimms_state->is_enabled) {
+ const struct AcpiGenericAddress arm_virt_nvdimm_acpi_dsmio = {
+ .space_id = AML_AS_SYSTEM_MEMORY,
+ .address = vms->memmap[VIRT_NVDIMM_ACPI].base,
+ .bit_width = NVDIMM_ACPI_IO_LEN << 3
+ };
+
+ nvdimm_init_acpi_state(machine->nvdimms_state, sysmem,
+ arm_virt_nvdimm_acpi_dsmio,
+ vms->fw_cfg, OBJECT(vms));
+ }
+
vms->bootinfo.ram_size = machine->ram_size;
vms->bootinfo.nb_cpus = smp_cpus;
vms->bootinfo.board_id = -1;
@@ -2077,6 +2090,8 @@ static void virt_memory_plug(HotplugHandler *hotplug_dev,
DeviceState *dev, Error **errp)
{
VirtMachineState *vms = VIRT_MACHINE(hotplug_dev);
+ MachineState *ms = MACHINE(hotplug_dev);
+ bool is_nvdimm = object_dynamic_cast(OBJECT(dev), TYPE_NVDIMM);
Error *local_err = NULL;
pc_dimm_plug(PC_DIMM(dev), MACHINE(vms), &local_err);
@@ -2084,6 +2099,10 @@ static void virt_memory_plug(HotplugHandler *hotplug_dev,
goto out;
}
+ if (is_nvdimm) {
+ nvdimm_plug(ms->nvdimms_state);
+ }
+
hotplug_handler_plug(HOTPLUG_HANDLER(vms->acpi_dev),
dev, &error_abort);
diff --git a/hw/mem/Kconfig b/hw/mem/Kconfig
index 2ad052a536..c27844900d 100644
--- a/hw/mem/Kconfig
+++ b/hw/mem/Kconfig
@@ -8,4 +8,4 @@ config MEM_DEVICE
config NVDIMM
bool
default y
- depends on (PC || PSERIES)
+ depends on (PC || PSERIES || ARM_VIRT)
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index 60b2f521eb..6d67ace76e 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -79,6 +79,7 @@ enum {
VIRT_SECURE_MEM,
VIRT_PCDIMM_ACPI,
VIRT_ACPI_GED,
+ VIRT_NVDIMM_ACPI,
VIRT_LOWMEMMAP_LAST,
};
--
2.17.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v4 4/7] hw/arm/virt: Add nvdimm hotplug support
2020-04-21 12:59 [PATCH v4 0/7] ARM virt: Add NVDIMM support Shameer Kolothum
` (2 preceding siblings ...)
2020-04-21 12:59 ` [PATCH v4 3/7] hw/arm/virt: Add nvdimm hot-plug infrastructure Shameer Kolothum
@ 2020-04-21 12:59 ` Shameer Kolothum
2020-04-21 12:59 ` [PATCH v4 5/7] tests: Update ACPI tables list for upcoming arm/virt test changes Shameer Kolothum
` (4 subsequent siblings)
8 siblings, 0 replies; 17+ messages in thread
From: Shameer Kolothum @ 2020-04-21 12:59 UTC (permalink / raw)
To: qemu-devel, qemu-arm, eric.auger, imammedo
Cc: peter.maydell, xiaoguangrong.eric, mst, linuxarm, xuwei5,
shannon.zhaosl, prime.zeng, lersek
This adds support for nvdimm hotplug events through GED
and enables nvdimm for the arm/virt. Now Guests with ACPI
can have both cold and hot plug of nvdimms.
Hot removal functionality is not yet supported.
Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
---
docs/specs/acpi_hw_reduced_hotplug.rst | 3 ++-
hw/acpi/generic_event_device.c | 15 ++++++++++++++-
hw/arm/virt.c | 16 +++++++++++-----
include/hw/acpi/generic_event_device.h | 1 +
4 files changed, 28 insertions(+), 7 deletions(-)
diff --git a/docs/specs/acpi_hw_reduced_hotplug.rst b/docs/specs/acpi_hw_reduced_hotplug.rst
index 911a98255b..0bd3f9399f 100644
--- a/docs/specs/acpi_hw_reduced_hotplug.rst
+++ b/docs/specs/acpi_hw_reduced_hotplug.rst
@@ -63,7 +63,8 @@ GED IO interface (4 byte access)
bits:
0: Memory hotplug event
1: System power down event
- 2-31: Reserved
+ 2: NVDIMM hotplug event
+ 3-31: Reserved
**write_access:**
diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
index 021ed2bf23..5d17f78a1e 100644
--- a/hw/acpi/generic_event_device.c
+++ b/hw/acpi/generic_event_device.c
@@ -16,6 +16,7 @@
#include "hw/acpi/generic_event_device.h"
#include "hw/irq.h"
#include "hw/mem/pc-dimm.h"
+#include "hw/mem/nvdimm.h"
#include "hw/qdev-properties.h"
#include "migration/vmstate.h"
#include "qemu/error-report.h"
@@ -23,6 +24,7 @@
static const uint32_t ged_supported_events[] = {
ACPI_GED_MEM_HOTPLUG_EVT,
ACPI_GED_PWR_DOWN_EVT,
+ ACPI_GED_NVDIMM_HOTPLUG_EVT,
};
/*
@@ -110,6 +112,11 @@ void build_ged_aml(Aml *table, const char *name, HotplugHandler *hotplug_dev,
aml_notify(aml_name(ACPI_POWER_BUTTON_DEVICE),
aml_int(0x80)));
break;
+ case ACPI_GED_NVDIMM_HOTPLUG_EVT:
+ aml_append(if_ctx,
+ aml_notify(aml_name("\\_SB.NVDR"),
+ aml_int(0x80)));
+ break;
default:
/*
* Please make sure all the events in ged_supported_events[]
@@ -175,7 +182,11 @@ static void acpi_ged_device_plug_cb(HotplugHandler *hotplug_dev,
AcpiGedState *s = ACPI_GED(hotplug_dev);
if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
- acpi_memory_plug_cb(hotplug_dev, &s->memhp_state, dev, errp);
+ if (object_dynamic_cast(OBJECT(dev), TYPE_NVDIMM)) {
+ nvdimm_acpi_plug_cb(hotplug_dev, dev);
+ } else {
+ acpi_memory_plug_cb(hotplug_dev, &s->memhp_state, dev, errp);
+ }
} else {
error_setg(errp, "virt: device plug request for unsupported device"
" type: %s", object_get_typename(OBJECT(dev)));
@@ -192,6 +203,8 @@ static void acpi_ged_send_event(AcpiDeviceIf *adev, AcpiEventStatusBits ev)
sel = ACPI_GED_MEM_HOTPLUG_EVT;
} else if (ev & ACPI_POWER_DOWN_STATUS) {
sel = ACPI_GED_PWR_DOWN_EVT;
+ } else if (ev & ACPI_NVDIMM_HOTPLUG_STATUS) {
+ sel = ACPI_GED_NVDIMM_HOTPLUG_EVT;
} else {
/* Unknown event. Return without generating interrupt. */
warn_report("GED: Unsupported event %d. No irq injected", ev);
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 8a2beaf650..da4994c55a 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -549,6 +549,10 @@ static inline DeviceState *create_acpi_ged(VirtMachineState *vms)
event |= ACPI_GED_MEM_HOTPLUG_EVT;
}
+ if (ms->nvdimms_state->is_enabled) {
+ event |= ACPI_GED_NVDIMM_HOTPLUG_EVT;
+ }
+
dev = qdev_create(NULL, TYPE_ACPI_GED);
qdev_prop_set_uint32(dev, "ged-event", event);
@@ -2070,19 +2074,20 @@ static void virt_memory_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
Error **errp)
{
VirtMachineState *vms = VIRT_MACHINE(hotplug_dev);
+ const MachineState *ms = MACHINE(hotplug_dev);
const bool is_nvdimm = object_dynamic_cast(OBJECT(dev), TYPE_NVDIMM);
- 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-ged device");
return;
}
+ if (is_nvdimm && !ms->nvdimms_state->is_enabled) {
+ error_setg(errp, "nvdimm is not enabled: add 'nvdimm=on' to '-M'");
+ return;
+ }
+
pc_dimm_pre_plug(PC_DIMM(dev), MACHINE(hotplug_dev), NULL, errp);
}
@@ -2227,6 +2232,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->nvdimm_supported = true;
mc->auto_enable_numa_with_memhp = true;
mc->default_ram_id = "mach-virt.ram";
diff --git a/include/hw/acpi/generic_event_device.h b/include/hw/acpi/generic_event_device.h
index d157eac088..9eb86ca4fd 100644
--- a/include/hw/acpi/generic_event_device.h
+++ b/include/hw/acpi/generic_event_device.h
@@ -82,6 +82,7 @@
*/
#define ACPI_GED_MEM_HOTPLUG_EVT 0x1
#define ACPI_GED_PWR_DOWN_EVT 0x2
+#define ACPI_GED_NVDIMM_HOTPLUG_EVT 0x4
typedef struct GEDState {
MemoryRegion io;
--
2.17.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v4 5/7] tests: Update ACPI tables list for upcoming arm/virt test changes
2020-04-21 12:59 [PATCH v4 0/7] ARM virt: Add NVDIMM support Shameer Kolothum
` (3 preceding siblings ...)
2020-04-21 12:59 ` [PATCH v4 4/7] hw/arm/virt: Add nvdimm hotplug support Shameer Kolothum
@ 2020-04-21 12:59 ` Shameer Kolothum
2020-04-21 12:59 ` [PATCH v4 6/7] bios-tables-test: test pc-dimm and nvdimm coldplug for arm/virt Shameer Kolothum
` (3 subsequent siblings)
8 siblings, 0 replies; 17+ messages in thread
From: Shameer Kolothum @ 2020-04-21 12:59 UTC (permalink / raw)
To: qemu-devel, qemu-arm, eric.auger, imammedo
Cc: peter.maydell, xiaoguangrong.eric, mst, linuxarm, xuwei5,
shannon.zhaosl, prime.zeng, lersek
This is in preparation to update test_acpi_virt_tcg_memhp()
with pc-dimm and nvdimm. Update the bios-tables-test-allowed-diff.h
with the affected ACPI tables so that "make check" doesn't fail.
Also add empty files for new tables required for new test.
Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
---
tests/data/acpi/virt/NFIT.memhp | 0
tests/data/acpi/virt/SSDT.memhp | 0
tests/qtest/bios-tables-test-allowed-diff.h | 3 +++
3 files changed, 3 insertions(+)
create mode 100644 tests/data/acpi/virt/NFIT.memhp
create mode 100644 tests/data/acpi/virt/SSDT.memhp
diff --git a/tests/data/acpi/virt/NFIT.memhp b/tests/data/acpi/virt/NFIT.memhp
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/data/acpi/virt/SSDT.memhp b/tests/data/acpi/virt/SSDT.memhp
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index eb8bae1407..862c49e675 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1,3 +1,6 @@
/* List of comma-separated changed AML files to ignore */
"tests/data/acpi/pc/SSDT.dimmpxm",
"tests/data/acpi/q35/SSDT.dimmpxm",
+"tests/data/acpi/virt/DSDT.memhp",
+"tests/data/acpi/virt/SSDT.memhp",
+"tests/data/acpi/virt/NFIT.memhp",
--
2.17.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v4 6/7] bios-tables-test: test pc-dimm and nvdimm coldplug for arm/virt
2020-04-21 12:59 [PATCH v4 0/7] ARM virt: Add NVDIMM support Shameer Kolothum
` (4 preceding siblings ...)
2020-04-21 12:59 ` [PATCH v4 5/7] tests: Update ACPI tables list for upcoming arm/virt test changes Shameer Kolothum
@ 2020-04-21 12:59 ` Shameer Kolothum
2020-04-21 12:59 ` [PATCH v4 7/7] tests/acpi: add expected tables for bios-tables-test Shameer Kolothum
` (2 subsequent siblings)
8 siblings, 0 replies; 17+ messages in thread
From: Shameer Kolothum @ 2020-04-21 12:59 UTC (permalink / raw)
To: qemu-devel, qemu-arm, eric.auger, imammedo
Cc: peter.maydell, xiaoguangrong.eric, mst, linuxarm, xuwei5,
shannon.zhaosl, prime.zeng, lersek
Since we now have both pc-dimm and nvdimm support, update
test_acpi_virt_tcg_memhp() to include those.
Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
---
tests/qtest/bios-tables-test.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
index 0a597bbacf..c9843829b3 100644
--- a/tests/qtest/bios-tables-test.c
+++ b/tests/qtest/bios-tables-test.c
@@ -927,12 +927,17 @@ static void test_acpi_virt_tcg_memhp(void)
};
data.variant = ".memhp";
- test_acpi_one(" -cpu cortex-a57"
+ test_acpi_one(" -machine nvdimm=on"
+ " -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",
+ " -numa dist,src=0,dst=1,val=21"
+ " -object memory-backend-ram,id=ram2,size=128M"
+ " -object memory-backend-ram,id=nvm0,size=128M"
+ " -device pc-dimm,id=dimm0,memdev=ram2,node=0"
+ " -device nvdimm,id=dimm1,memdev=nvm0,node=1",
&data);
free_test_data(&data);
--
2.17.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v4 7/7] tests/acpi: add expected tables for bios-tables-test
2020-04-21 12:59 [PATCH v4 0/7] ARM virt: Add NVDIMM support Shameer Kolothum
` (5 preceding siblings ...)
2020-04-21 12:59 ` [PATCH v4 6/7] bios-tables-test: test pc-dimm and nvdimm coldplug for arm/virt Shameer Kolothum
@ 2020-04-21 12:59 ` Shameer Kolothum
2020-04-21 15:12 ` [PATCH v4 0/7] ARM virt: Add NVDIMM support no-reply
2020-05-04 5:13 ` Michael S. Tsirkin
8 siblings, 0 replies; 17+ messages in thread
From: Shameer Kolothum @ 2020-04-21 12:59 UTC (permalink / raw)
To: qemu-devel, qemu-arm, eric.auger, imammedo
Cc: peter.maydell, xiaoguangrong.eric, mst, linuxarm, xuwei5,
shannon.zhaosl, prime.zeng, lersek
Because of the following changes, the expeacted tables for bios-tables-test
needs to be updated.
1. Changed NVDIM DSM output buffer AML code.
2. Updated arm/virt test_acpi_virt_tcg_memhp() to add pc-dimm/nvdimm
Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
---
tests/data/acpi/pc/SSDT.dimmpxm | Bin 685 -> 734 bytes
tests/data/acpi/q35/SSDT.dimmpxm | Bin 685 -> 734 bytes
tests/data/acpi/virt/DSDT.memhp | Bin 6644 -> 6668 bytes
tests/data/acpi/virt/NFIT.memhp | Bin 0 -> 224 bytes
tests/data/acpi/virt/SSDT.memhp | Bin 0 -> 736 bytes
tests/qtest/bios-tables-test-allowed-diff.h | 5 -----
6 files changed, 5 deletions(-)
diff --git a/tests/data/acpi/pc/SSDT.dimmpxm b/tests/data/acpi/pc/SSDT.dimmpxm
index 8ba0e67cb72daa81a65da4906d37a5e0f4af1fd4..ac55387d57e48adb99eb738a102308688a262fb8 100644
GIT binary patch
delta 125
zcmZ3>dXJSWIM^lR9uortW7tG4X>Nb5nD}6)_~<4#t%(LAOunKU-FO)N7nn^=<l-n`
zNa6@_3Uw1?W@IQ3WMW8PBs(=Jv7oub-^DQ`iJ=1|$G}jW4x-abla{0xR3>sUGbBa}
TgA_0%`UAz6fQZcnjJAvbW$GgJ
delta 76
zcmcb|x|WqIIM^j*EfWI+W57f%X>LFDnD}6)_~<5A^@#=|Og=&z-FO(~3Mv!1m>CkI
dh5cO|Ll_eMokHD;1(_H?bo!F?%?lZA83Fei6ZHT9
diff --git a/tests/data/acpi/q35/SSDT.dimmpxm b/tests/data/acpi/q35/SSDT.dimmpxm
index 2d5b721bcf9c398feb6d005761f898015042e8a4..98e6f0e3f3bb02dd419e36bdd1db9b94c728c406 100644
GIT binary patch
delta 125
zcmZ3>dXJSWIM^lR9uortquWF-X>Nb5nD}6)_~<4#t%(LAOunKU-FO)N7nn^=<l-n`
zNa6@_3Uw1?W@IQ3WMW8PBs(=Jv7oub-^DQ`iJ=1|$G}jW4x-abla{0xR3>sUGbBa}
TgA_0%`UAz6fQZcnjJAvbUiu>C
delta 76
zcmcb|x|WqIIM^j*EfWI+qr*flX>LFDnD}6)_~<5A^@#=|Og=&z-FO(~3Mv!1m>CkI
dh5cO|Ll_eMokHD;1(_H?bo!F?%?lZA83FS;6XgH^
diff --git a/tests/data/acpi/virt/DSDT.memhp b/tests/data/acpi/virt/DSDT.memhp
index c527ac4739af3df3c3e042bf91c412033a2b73c3..730e95a46d2cce0af011ffc051d7342beb8f1328 100644
GIT binary patch
delta 66
zcmexj++)J!66_MfBgMeL^l>7WG*kP$jq2<oOpaWW4Mc*Od{`#8h!~455Sv_`z{SGA
W#nKj|7a!~t?-%A0w0WzDJu3iIl@a3r
delta 43
zcmeA%`C`oF66_N4MUsJmsc|BgG*kbajq2<oOr9K*4Mc(&O(wUB7;io=V#^8u9B2%1
diff --git a/tests/data/acpi/virt/NFIT.memhp b/tests/data/acpi/virt/NFIT.memhp
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..738c6f74c0ce7dc329cc72cc7b930460ceb9b6a0 100644
GIT binary patch
literal 224
zcmeZs^9*^wz`($G(aGQ0BUr&HBEZ=XD8>jB1F=Cg1XwVzFffCeAhF8JAJRT=DREfi
z%xf&Mz2`Pir~{&ofdfQyG(dQa3<eBL5GGJRD~JV_hYBH45Sv&S0)VR88W<T6HiHy}
Pn+P#5Laay9LzoEwN>da8
literal 0
HcmV?d00001
diff --git a/tests/data/acpi/virt/SSDT.memhp b/tests/data/acpi/virt/SSDT.memhp
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..db61d6733284c90153e0e2c1d6c2ac25c22b1d84 100644
GIT binary patch
literal 736
zcmZWnJ&4m_7=F{1Ynryuv=ve6ky!_O6-025G-+(0NlTIzAqUNmw7s^&hj$JlLfadz
zo*?wXEh3U083lL2&DF`tL2x*5cM%-K?@e*=T;7-Od*1JPpXcE-P1*XE0AOy_+fvgM
z^q#D08a)F*{Xs{<UJ!5W_RV_VUNo;$++h1nRnCi2N3B+OYEW)p(j~E#Ct5=j2lh<w
zsBOprWHMZeo(xfgK^6-3Jc~$Dw-i;d=M@B!O}W(&j7HBCl&&rbifG)Q{Yu;OVvHKC
zhAy;a(VG2EhgX9s5Wd6}qVI*9t2gKT^UuY*!Oz|I&iYQc9Zcy)W;5{P^^1q=PetkY
zi~8|h?Stp6FUJq7Z!;2~Kj&8~7KGfZffHVN77XAd7&#DTf-yvVl8FmLrl^eUp)@i^
zZDHkU@N&YXf};stvz!2U<k0hpkDAE<T^*oaF6D-|PhE~B;IUS{s~{)g6EHlxcIvt0
zaEXRISRw|nKg2$K>_tQJ#O0FU+9ezSw@vROEagI9HnmiQA&>UP6JQO~5}~Z64Mnvw
zY+ErcAR}x9XE39S5im?~i@}LFCFj@yO3@`)kSxrd1o2Hog_0x3J#f(n8@M!%1lT2q
z6Jm8#juK%h-|N%mEE@9&_-9pypc>!7*C8JuYZjQl__`2oA({8ccl~|O@$5xK?^Ua8
nUz82zqC>`BY*Tb6M!7_p|2F_GvB7H_Joz7<WbrFft6!x*uUOFm
literal 0
HcmV?d00001
diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index 862c49e675..dfb8523c8b 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1,6 +1 @@
/* List of comma-separated changed AML files to ignore */
-"tests/data/acpi/pc/SSDT.dimmpxm",
-"tests/data/acpi/q35/SSDT.dimmpxm",
-"tests/data/acpi/virt/DSDT.memhp",
-"tests/data/acpi/virt/SSDT.memhp",
-"tests/data/acpi/virt/NFIT.memhp",
--
2.17.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v4 0/7] ARM virt: Add NVDIMM support
2020-04-21 12:59 [PATCH v4 0/7] ARM virt: Add NVDIMM support Shameer Kolothum
` (6 preceding siblings ...)
2020-04-21 12:59 ` [PATCH v4 7/7] tests/acpi: add expected tables for bios-tables-test Shameer Kolothum
@ 2020-04-21 15:12 ` no-reply
2020-05-04 9:57 ` Michael S. Tsirkin
2020-05-04 5:13 ` Michael S. Tsirkin
8 siblings, 1 reply; 17+ messages in thread
From: no-reply @ 2020-04-21 15:12 UTC (permalink / raw)
To: shameerali.kolothum.thodi
Cc: peter.maydell, xiaoguangrong.eric, mst, shannon.zhaosl,
qemu-devel, xuwei5, linuxarm, eric.auger, qemu-arm, prime.zeng,
imammedo, lersek
Patchew URL: https://patchew.org/QEMU/20200421125934.14952-1-shameerali.kolothum.thodi@huawei.com/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Subject: [PATCH v4 0/7] ARM virt: Add NVDIMM support
Message-id: 20200421125934.14952-1-shameerali.kolothum.thodi@huawei.com
Type: series
=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===
Switched to a new branch 'test'
c4f3ad1 tests/acpi: add expected tables for bios-tables-test
5b55be7 bios-tables-test: test pc-dimm and nvdimm coldplug for arm/virt
f0c9bb6 tests: Update ACPI tables list for upcoming arm/virt test changes
c2dd728 hw/arm/virt: Add nvdimm hotplug support
f7dad84 hw/arm/virt: Add nvdimm hot-plug infrastructure
5554e78 nvdimm: Use configurable ACPI IO base and size
8058b6f hw/acpi/nvdimm: Fix for NVDIMM incorrect DSM output buffer length
=== OUTPUT BEGIN ===
1/7 Checking commit 8058b6f6d753 (hw/acpi/nvdimm: Fix for NVDIMM incorrect DSM output buffer length)
ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/acpi/nvdimm.c found
ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/acpi/nvdimm.c found
total: 2 errors, 0 warnings, 59 lines checked
Patch 1/7 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
2/7 Checking commit 5554e78b18ea (nvdimm: Use configurable ACPI IO base and size)
ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/acpi/nvdimm.c found
ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/acpi/nvdimm.c found
ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/i386/acpi-build.c found
ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/i386/acpi-build.c found
ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/i386/acpi-build.h found
ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/i386/acpi-build.h found
ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/i386/pc_piix.c found
ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/i386/pc_piix.c found
ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/i386/pc_q35.c found
ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/i386/pc_q35.c found
ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and include/hw/mem/nvdimm.h found
ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and include/hw/mem/nvdimm.h found
total: 12 errors, 0 warnings, 158 lines checked
Patch 2/7 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
3/7 Checking commit f7dad84068ce (hw/arm/virt: Add nvdimm hot-plug infrastructure)
ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/arm/Kconfig found
ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/arm/Kconfig found
ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/arm/virt-acpi-build.c found
ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/arm/virt-acpi-build.c found
ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/arm/virt.c found
ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/arm/virt.c found
ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/mem/Kconfig found
ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/mem/Kconfig found
ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and include/hw/arm/virt.h found
ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and include/hw/arm/virt.h found
total: 10 errors, 0 warnings, 80 lines checked
Patch 3/7 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
4/7 Checking commit c2dd7289fec4 (hw/arm/virt: Add nvdimm hotplug support)
ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and docs/specs/acpi_hw_reduced_hotplug.rst found
ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and docs/specs/acpi_hw_reduced_hotplug.rst found
ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/acpi/generic_event_device.c found
ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/acpi/generic_event_device.c found
ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/arm/virt.c found
ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/arm/virt.c found
ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and include/hw/acpi/generic_event_device.h found
ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and include/hw/acpi/generic_event_device.h found
total: 8 errors, 0 warnings, 103 lines checked
Patch 4/7 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
5/7 Checking commit f0c9bb65828f (tests: Update ACPI tables list for upcoming arm/virt test changes)
ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/data/acpi/virt/NFIT.memhp and include/hw/acpi/generic_event_device.h found
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#17:
new file mode 100644
ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/data/acpi/virt/SSDT.memhp and include/hw/acpi/generic_event_device.h found
ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and include/hw/acpi/generic_event_device.h found
ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and include/hw/acpi/generic_event_device.h found
total: 4 errors, 1 warnings, 6 lines checked
Patch 5/7 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
6/7 Checking commit 5b55be7a85b5 (bios-tables-test: test pc-dimm and nvdimm coldplug for arm/virt)
ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and tests/qtest/bios-tables-test.c found
ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and tests/qtest/bios-tables-test.c found
total: 2 errors, 0 warnings, 19 lines checked
Patch 6/7 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
7/7 Checking commit c4f3ad1b593c (tests/acpi: add expected tables for bios-tables-test)
ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/data/acpi/pc/SSDT.dimmpxm and tests/qtest/bios-tables-test.c found
ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/data/acpi/q35/SSDT.dimmpxm and tests/qtest/bios-tables-test.c found
ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/data/acpi/virt/DSDT.memhp and tests/qtest/bios-tables-test.c found
ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/data/acpi/virt/NFIT.memhp and tests/qtest/bios-tables-test.c found
ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/data/acpi/virt/SSDT.memhp and tests/qtest/bios-tables-test.c found
ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and tests/qtest/bios-tables-test.c found
ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and tests/qtest/bios-tables-test.c found
total: 7 errors, 0 warnings, 1 lines checked
Patch 7/7 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20200421125934.14952-1-shameerali.kolothum.thodi@huawei.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v4 0/7] ARM virt: Add NVDIMM support
2020-04-21 15:12 ` [PATCH v4 0/7] ARM virt: Add NVDIMM support no-reply
@ 2020-05-04 9:57 ` Michael S. Tsirkin
2020-05-04 10:06 ` Peter Maydell
0 siblings, 1 reply; 17+ messages in thread
From: Michael S. Tsirkin @ 2020-05-04 9:57 UTC (permalink / raw)
To: qemu-devel
Cc: peter.maydell, xiaoguangrong.eric, shannon.zhaosl, linuxarm,
shameerali.kolothum.thodi, eric.auger, qemu-arm, xuwei5,
prime.zeng, imammedo, lersek
On Tue, Apr 21, 2020 at 08:12:57AM -0700, no-reply@patchew.org wrote:
> Patchew URL: https://patchew.org/QEMU/20200421125934.14952-1-shameerali.kolothum.thodi@huawei.com/
>
>
>
> Hi,
>
> This series seems to have some coding style problems. See output below for
> more information:
>
> Subject: [PATCH v4 0/7] ARM virt: Add NVDIMM support
> Message-id: 20200421125934.14952-1-shameerali.kolothum.thodi@huawei.com
> Type: series
>
> === TEST SCRIPT BEGIN ===
> #!/bin/bash
> git rev-parse base > /dev/null || exit 0
> git config --local diff.renamelimit 0
> git config --local diff.renames True
> git config --local diff.algorithm histogram
> ./scripts/checkpatch.pl --mailback base..
> === TEST SCRIPT END ===
>
> Switched to a new branch 'test'
> c4f3ad1 tests/acpi: add expected tables for bios-tables-test
> 5b55be7 bios-tables-test: test pc-dimm and nvdimm coldplug for arm/virt
> f0c9bb6 tests: Update ACPI tables list for upcoming arm/virt test changes
> c2dd728 hw/arm/virt: Add nvdimm hotplug support
> f7dad84 hw/arm/virt: Add nvdimm hot-plug infrastructure
> 5554e78 nvdimm: Use configurable ACPI IO base and size
> 8058b6f hw/acpi/nvdimm: Fix for NVDIMM incorrect DSM output buffer length
>
> === OUTPUT BEGIN ===
> 1/7 Checking commit 8058b6f6d753 (hw/acpi/nvdimm: Fix for NVDIMM incorrect DSM output buffer length)
> ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/acpi/nvdimm.c found
>
> ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/acpi/nvdimm.c found
>
> total: 2 errors, 0 warnings, 59 lines checked
OK so this is a false positive in the script. I will fix it.
> Patch 1/7 has style problems, please review. If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
>
> 2/7 Checking commit 5554e78b18ea (nvdimm: Use configurable ACPI IO base and size)
> ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/acpi/nvdimm.c found
This beats me. Where did we get
tests/qtest/bios-tables-test-allowed-diff.h from?
It's a different patch, isn't it?
> ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/acpi/nvdimm.c found
>
> ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/i386/acpi-build.c found
>
> ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/i386/acpi-build.c found
>
> ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/i386/acpi-build.h found
>
> ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/i386/acpi-build.h found
>
> ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/i386/pc_piix.c found
>
> ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/i386/pc_piix.c found
>
> ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/i386/pc_q35.c found
>
> ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/i386/pc_q35.c found
>
> ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and include/hw/mem/nvdimm.h found
>
> ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and include/hw/mem/nvdimm.h found
>
> total: 12 errors, 0 warnings, 158 lines checked
>
> Patch 2/7 has style problems, please review. If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
>
> 3/7 Checking commit f7dad84068ce (hw/arm/virt: Add nvdimm hot-plug infrastructure)
> ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/arm/Kconfig found
>
> ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/arm/Kconfig found
>
> ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/arm/virt-acpi-build.c found
>
> ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/arm/virt-acpi-build.c found
>
> ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/arm/virt.c found
>
> ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/arm/virt.c found
>
> ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/mem/Kconfig found
>
> ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/mem/Kconfig found
>
> ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and include/hw/arm/virt.h found
>
> ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and include/hw/arm/virt.h found
>
> total: 10 errors, 0 warnings, 80 lines checked
>
> Patch 3/7 has style problems, please review. If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
>
> 4/7 Checking commit c2dd7289fec4 (hw/arm/virt: Add nvdimm hotplug support)
> ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and docs/specs/acpi_hw_reduced_hotplug.rst found
>
> ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and docs/specs/acpi_hw_reduced_hotplug.rst found
>
> ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/acpi/generic_event_device.c found
>
> ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/acpi/generic_event_device.c found
>
> ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/arm/virt.c found
>
> ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/arm/virt.c found
>
> ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and include/hw/acpi/generic_event_device.h found
>
> ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and include/hw/acpi/generic_event_device.h found
>
> total: 8 errors, 0 warnings, 103 lines checked
>
> Patch 4/7 has style problems, please review. If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
>
> 5/7 Checking commit f0c9bb65828f (tests: Update ACPI tables list for upcoming arm/virt test changes)
> ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/data/acpi/virt/NFIT.memhp and include/hw/acpi/generic_event_device.h found
>
> WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
> #17:
> new file mode 100644
>
> ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/data/acpi/virt/SSDT.memhp and include/hw/acpi/generic_event_device.h found
>
> ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and include/hw/acpi/generic_event_device.h found
>
> ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and include/hw/acpi/generic_event_device.h found
>
> total: 4 errors, 1 warnings, 6 lines checked
>
> Patch 5/7 has style problems, please review. If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
>
> 6/7 Checking commit 5b55be7a85b5 (bios-tables-test: test pc-dimm and nvdimm coldplug for arm/virt)
> ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and tests/qtest/bios-tables-test.c found
>
> ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and tests/qtest/bios-tables-test.c found
>
> total: 2 errors, 0 warnings, 19 lines checked
>
> Patch 6/7 has style problems, please review. If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
>
> 7/7 Checking commit c4f3ad1b593c (tests/acpi: add expected tables for bios-tables-test)
> ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/data/acpi/pc/SSDT.dimmpxm and tests/qtest/bios-tables-test.c found
>
> ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/data/acpi/q35/SSDT.dimmpxm and tests/qtest/bios-tables-test.c found
>
> ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/data/acpi/virt/DSDT.memhp and tests/qtest/bios-tables-test.c found
>
> ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/data/acpi/virt/NFIT.memhp and tests/qtest/bios-tables-test.c found
>
> ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/data/acpi/virt/SSDT.memhp and tests/qtest/bios-tables-test.c found
>
> ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and tests/qtest/bios-tables-test.c found
>
> ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and tests/qtest/bios-tables-test.c found
>
> total: 7 errors, 0 warnings, 1 lines checked
>
> Patch 7/7 has style problems, please review. If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
>
> === OUTPUT END ===
>
> Test command exited with code: 1
>
>
> The full log is available at
> http://patchew.org/logs/20200421125934.14952-1-shameerali.kolothum.thodi@huawei.com/testing.checkpatch/?type=message.
> ---
> Email generated automatically by Patchew [https://patchew.org/].
> Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 0/7] ARM virt: Add NVDIMM support
2020-05-04 9:57 ` Michael S. Tsirkin
@ 2020-05-04 10:06 ` Peter Maydell
2020-05-04 11:10 ` Michael S. Tsirkin
0 siblings, 1 reply; 17+ messages in thread
From: Peter Maydell @ 2020-05-04 10:06 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Xiao Guangrong, Shannon Zhao, QEMU Developers,
Shameerali Kolothum Thodi, Linuxarm, Eric Auger, qemu-arm,
Xu Wei, prime.zeng, Igor Mammedov, Laszlo Ersek
On Mon, 4 May 2020 at 10:57, Michael S. Tsirkin <mst@redhat.com> wrote:
> > ./scripts/checkpatch.pl --mailback base..
> > 2/7 Checking commit 5554e78b18ea (nvdimm: Use configurable ACPI IO base and size)
> > ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/acpi/nvdimm.c found
>
> This beats me. Where did we get
> tests/qtest/bios-tables-test-allowed-diff.h from?
> It's a different patch, isn't it?
Ah, this is a bug in the checkfilename() function -- it uses
some globals $acpi_testexpected and $acpi_nontestexpected, but
there is no code to reset these when checkpatch starts checking
a new patch. So if you only check one patch in a checkpatch run
(eg by just passing it a single patch file) then it will work, but if
a single checkpatch execution is checking several commits
(eg in the way patchew runs it to check the whole series of
git commits at once, or if you pass it several patch files) then
it will give wrong results for the second and later patches.
I think the variables need to be reset at the top of 'sub process()'.
thanks
-- PMM
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 0/7] ARM virt: Add NVDIMM support
2020-05-04 10:06 ` Peter Maydell
@ 2020-05-04 11:10 ` Michael S. Tsirkin
0 siblings, 0 replies; 17+ messages in thread
From: Michael S. Tsirkin @ 2020-05-04 11:10 UTC (permalink / raw)
To: Peter Maydell
Cc: Xiao Guangrong, Shannon Zhao, QEMU Developers,
Shameerali Kolothum Thodi, Linuxarm, Eric Auger, qemu-arm,
Xu Wei, prime.zeng, Igor Mammedov, Laszlo Ersek
On Mon, May 04, 2020 at 11:06:48AM +0100, Peter Maydell wrote:
> On Mon, 4 May 2020 at 10:57, Michael S. Tsirkin <mst@redhat.com> wrote:
>
> > > ./scripts/checkpatch.pl --mailback base..
>
> > > 2/7 Checking commit 5554e78b18ea (nvdimm: Use configurable ACPI IO base and size)
> > > ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/acpi/nvdimm.c found
> >
> > This beats me. Where did we get
> > tests/qtest/bios-tables-test-allowed-diff.h from?
> > It's a different patch, isn't it?
>
> Ah, this is a bug in the checkfilename() function -- it uses
> some globals $acpi_testexpected and $acpi_nontestexpected, but
> there is no code to reset these when checkpatch starts checking
> a new patch. So if you only check one patch in a checkpatch run
> (eg by just passing it a single patch file) then it will work, but if
> a single checkpatch execution is checking several commits
> (eg in the way patchew runs it to check the whole series of
> git commits at once, or if you pass it several patch files) then
> it will give wrong results for the second and later patches.
> I think the variables need to be reset at the top of 'sub process()'.
>
> thanks
> -- PMM
Good point. Will fix.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 0/7] ARM virt: Add NVDIMM support
2020-04-21 12:59 [PATCH v4 0/7] ARM virt: Add NVDIMM support Shameer Kolothum
` (7 preceding siblings ...)
2020-04-21 15:12 ` [PATCH v4 0/7] ARM virt: Add NVDIMM support no-reply
@ 2020-05-04 5:13 ` Michael S. Tsirkin
2020-05-04 9:29 ` Peter Maydell
8 siblings, 1 reply; 17+ messages in thread
From: Michael S. Tsirkin @ 2020-05-04 5:13 UTC (permalink / raw)
To: Shameer Kolothum
Cc: peter.maydell, xiaoguangrong.eric, shannon.zhaosl, qemu-devel,
xuwei5, linuxarm, eric.auger, qemu-arm, prime.zeng, imammedo,
lersek
On Tue, Apr 21, 2020 at 01:59:27PM +0100, Shameer Kolothum wrote:
> This series adds NVDIMM support to arm/virt platform.
> The series reuses some of the patches posted by Eric
> in his earlier attempt here[1].
>
> This series previously had few fixes to qemu in general
> which were discovered while adding nvdimm support to arm/virt.
> Those were sent out seperately[2] and are now part of Qemu.
Mostly ACPI stuff so I can merge it if I get an ack for ARM side.
Alternatively, for ACPI things:
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> Patch #1 is another fix to the nvdimm aml issue discussed
> here[3].
>
> I have done a basic sanity testing of NVDIMM devices
> with Guest booting with ACPI. Further testing is always
> welcome.
>
> Please let me know your feedback.
>
> Thanks,
> Shameer
>
> [1] https://patchwork.kernel.org/cover/10830777/
> [2] https://patchwork.kernel.org/cover/11472501/
> [3] https://patchwork.kernel.org/cover/11174959/#23020961
>
> v3 --> v4
> -Removed patches #1 to #3 from v3 as they are now part of Qemu.
> -Addressed comments from Igor(#6) and Shannon(#4).
> -Added R-by from Igor(#1,#2,#3).
>
> v2 --> v3
> - Added patch #1 and # 2 to fix the inconsistency in acpi
> table memory region sizes during migration. Thanks to
> David H.
> - The fix for qemu_ram_resize() callback was modified to
> the one in patch #3. Again thanks to David H.
> - Addressed comments from MST and Eric on tests added.
> - Addressed comments from Igor/MST on Integer size in patch #4
> - Added Eric's R-by to patch #7.
>
> v1 --> v2
> -Reworked patch #1 and now fix is inside qemu_ram_resize().
> -Added patch #2 to fix the nvdim aml issue.
> -Dropped support to DT cold plug.
> -Updated test_acpi_virt_tcg_memhp() with pc-dimm and nvdimms(patch #7)
>
> Kwangwoo Lee (2):
> nvdimm: Use configurable ACPI IO base and size
> hw/arm/virt: Add nvdimm hot-plug infrastructure
>
> Shameer Kolothum (5):
> hw/acpi/nvdimm: Fix for NVDIMM incorrect DSM output buffer length
> hw/arm/virt: Add nvdimm hotplug support
> tests: Update ACPI tables list for upcoming arm/virt test changes
> bios-tables-test: test pc-dimm and nvdimm coldplug for arm/virt
> tests/acpi: add expected tables for bios-tables-test
>
> docs/specs/acpi_hw_reduced_hotplug.rst | 3 +-
> hw/acpi/generic_event_device.c | 15 +++++-
> hw/acpi/nvdimm.c | 72 ++++++++++++++++++++-----
> hw/arm/Kconfig | 1 +
> hw/arm/virt-acpi-build.c | 6 +++
> hw/arm/virt.c | 35 ++++++++++--
> hw/i386/acpi-build.c | 6 +++
> hw/i386/acpi-build.h | 3 ++
> hw/i386/pc_piix.c | 2 +
> hw/i386/pc_q35.c | 2 +
> hw/mem/Kconfig | 2 +-
> include/hw/acpi/generic_event_device.h | 1 +
> include/hw/arm/virt.h | 1 +
> include/hw/mem/nvdimm.h | 3 ++
> tests/data/acpi/pc/SSDT.dimmpxm | Bin 685 -> 734 bytes
> tests/data/acpi/q35/SSDT.dimmpxm | Bin 685 -> 734 bytes
> tests/data/acpi/virt/DSDT.memhp | Bin 6644 -> 6668 bytes
> tests/data/acpi/virt/NFIT.memhp | Bin 0 -> 224 bytes
> tests/data/acpi/virt/SSDT.memhp | Bin 0 -> 736 bytes
> tests/qtest/bios-tables-test.c | 9 +++-
> 20 files changed, 138 insertions(+), 23 deletions(-)
> create mode 100644 tests/data/acpi/virt/NFIT.memhp
> create mode 100644 tests/data/acpi/virt/SSDT.memhp
>
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v4 0/7] ARM virt: Add NVDIMM support
2020-05-04 5:13 ` Michael S. Tsirkin
@ 2020-05-04 9:29 ` Peter Maydell
2020-05-04 9:44 ` Auger Eric
2020-05-04 9:46 ` Michael S. Tsirkin
0 siblings, 2 replies; 17+ messages in thread
From: Peter Maydell @ 2020-05-04 9:29 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Xiao Guangrong, Shannon Zhao, QEMU Developers, Shameer Kolothum,
Linuxarm, Eric Auger, qemu-arm, Xu Wei, prime.zeng,
Igor Mammedov, Laszlo Ersek
On Mon, 4 May 2020 at 06:13, Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Tue, Apr 21, 2020 at 01:59:27PM +0100, Shameer Kolothum wrote:
> > This series adds NVDIMM support to arm/virt platform.
> > The series reuses some of the patches posted by Eric
> > in his earlier attempt here[1].
> >
> > This series previously had few fixes to qemu in general
> > which were discovered while adding nvdimm support to arm/virt.
> > Those were sent out seperately[2] and are now part of Qemu.
>
>
> Mostly ACPI stuff so I can merge it if I get an ack for ARM side.
Happy for you to take it; all the arm-specific bits have
been reviewed by various people (thanks!) so here's my
Acked-by: Peter Maydell <peter.maydell@linaro.org>
I notice that checkpatch complains a lot about
ERROR: Do not add expected files together with tests, follow
instructions in tests/qtest/bios-tables-test.c: both
tests/qtest/bios-tables-test-allowed-diff.h and
tests/qtest/bios-tables-test.c found
Does that need fixing, or is the checkpatch test wrong?
thanks
-- PMM
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 0/7] ARM virt: Add NVDIMM support
2020-05-04 9:29 ` Peter Maydell
@ 2020-05-04 9:44 ` Auger Eric
2020-05-04 9:46 ` Michael S. Tsirkin
1 sibling, 0 replies; 17+ messages in thread
From: Auger Eric @ 2020-05-04 9:44 UTC (permalink / raw)
To: Peter Maydell, Michael S. Tsirkin
Cc: Xiao Guangrong, Linuxarm, Shameer Kolothum, QEMU Developers,
Shannon Zhao, qemu-arm, Xu Wei, prime.zeng, Igor Mammedov,
Laszlo Ersek
Hi,
On 5/4/20 11:29 AM, Peter Maydell wrote:
> On Mon, 4 May 2020 at 06:13, Michael S. Tsirkin <mst@redhat.com> wrote:
>>
>> On Tue, Apr 21, 2020 at 01:59:27PM +0100, Shameer Kolothum wrote:
>>> This series adds NVDIMM support to arm/virt platform.
>>> The series reuses some of the patches posted by Eric
>>> in his earlier attempt here[1].
>>>
>>> This series previously had few fixes to qemu in general
>>> which were discovered while adding nvdimm support to arm/virt.
>>> Those were sent out seperately[2] and are now part of Qemu.
>>
>>
>> Mostly ACPI stuff so I can merge it if I get an ack for ARM side.
>
> Happy for you to take it; all the arm-specific bits have
> been reviewed by various people (thanks!) so here's my
>
> Acked-by: Peter Maydell <peter.maydell@linaro.org>
Tested-by: Eric Auger <eric.auger@redhat.com>
Thanks
Eric
>
> I notice that checkpatch complains a lot about
>
> ERROR: Do not add expected files together with tests, follow
> instructions in tests/qtest/bios-tables-test.c: both
> tests/qtest/bios-tables-test-allowed-diff.h and
> tests/qtest/bios-tables-test.c found
>
> Does that need fixing, or is the checkpatch test wrong?
>
> thanks
> -- PMM
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 0/7] ARM virt: Add NVDIMM support
2020-05-04 9:29 ` Peter Maydell
2020-05-04 9:44 ` Auger Eric
@ 2020-05-04 9:46 ` Michael S. Tsirkin
2020-05-04 9:57 ` Peter Maydell
1 sibling, 1 reply; 17+ messages in thread
From: Michael S. Tsirkin @ 2020-05-04 9:46 UTC (permalink / raw)
To: Peter Maydell
Cc: Xiao Guangrong, Shannon Zhao, QEMU Developers, Shameer Kolothum,
Linuxarm, Eric Auger, qemu-arm, Xu Wei, prime.zeng,
Igor Mammedov, Laszlo Ersek
On Mon, May 04, 2020 at 10:29:18AM +0100, Peter Maydell wrote:
> On Mon, 4 May 2020 at 06:13, Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > On Tue, Apr 21, 2020 at 01:59:27PM +0100, Shameer Kolothum wrote:
> > > This series adds NVDIMM support to arm/virt platform.
> > > The series reuses some of the patches posted by Eric
> > > in his earlier attempt here[1].
> > >
> > > This series previously had few fixes to qemu in general
> > > which were discovered while adding nvdimm support to arm/virt.
> > > Those were sent out seperately[2] and are now part of Qemu.
> >
> >
> > Mostly ACPI stuff so I can merge it if I get an ack for ARM side.
>
> Happy for you to take it; all the arm-specific bits have
> been reviewed by various people (thanks!) so here's my
>
> Acked-by: Peter Maydell <peter.maydell@linaro.org>
>
> I notice that checkpatch complains a lot about
>
> ERROR: Do not add expected files together with tests, follow
> instructions in tests/qtest/bios-tables-test.c: both
> tests/qtest/bios-tables-test-allowed-diff.h and
> tests/qtest/bios-tables-test.c found
>
> Does that need fixing, or is the checkpatch test wrong?
>
> thanks
> -- PMM
Hmm I don't see a patch in this series that changes both
tests/qtest/bios-tables-test-allowed-diff.h and
tests/qtest/bios-tables-test.c. Do you?
--
MST
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 0/7] ARM virt: Add NVDIMM support
2020-05-04 9:46 ` Michael S. Tsirkin
@ 2020-05-04 9:57 ` Peter Maydell
0 siblings, 0 replies; 17+ messages in thread
From: Peter Maydell @ 2020-05-04 9:57 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Xiao Guangrong, Shannon Zhao, QEMU Developers, Shameer Kolothum,
Linuxarm, Eric Auger, qemu-arm, Xu Wei, prime.zeng,
Igor Mammedov, Laszlo Ersek
On Mon, 4 May 2020 at 10:46, Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Mon, May 04, 2020 at 10:29:18AM +0100, Peter Maydell wrote:
> > I notice that checkpatch complains a lot about
> >
> > ERROR: Do not add expected files together with tests, follow
> > instructions in tests/qtest/bios-tables-test.c: both
> > tests/qtest/bios-tables-test-allowed-diff.h and
> > tests/qtest/bios-tables-test.c found
> >
> > Does that need fixing, or is the checkpatch test wrong?
> Hmm I don't see a patch in this series that changes both
> tests/qtest/bios-tables-test-allowed-diff.h and
> tests/qtest/bios-tables-test.c. Do you?
No, but that's not the pair of files that checkpatch is
complaining about. It warns about:
patch 1:
tests/qtest/bios-tables-test-allowed-diff.h and hw/acpi/nvdimm.c
patch 2:
tests/qtest/bios-tables-test-allowed-diff.h and hw/acpi/nvdimm.c
tests/qtest/bios-tables-test-allowed-diff.h and hw/i386/acpi-build.c
tests/qtest/bios-tables-test-allowed-diff.h and hw/i386/acpi-build.h
tests/qtest/bios-tables-test-allowed-diff.h and hw/i386/pc_piix.c
tests/qtest/bios-tables-test-allowed-diff.h and hw/i386/pc_q35.c
tests/qtest/bios-tables-test-allowed-diff.h and include/hw/mem/nvdimm.h
patch 3:
tests/qtest/bios-tables-test-allowed-diff.h and hw/arm/Kconfig
tests/qtest/bios-tables-test-allowed-diff.h and hw/arm/virt-acpi-build.c
tests/qtest/bios-tables-test-allowed-diff.h and hw/arm/virt.c
tests/qtest/bios-tables-test-allowed-diff.h and hw/mem/Kconfig
tests/qtest/bios-tables-test-allowed-diff.h and include/hw/arm/virt.h
etc, and the patches really do touch the pairs of files listed.
(It also seems to warn about each file combination multiple times.)
Are these false positives (if so, we should change the checkpatch test,
since it's clearly misfiring a lot) or problems with the patches?
thanks
-- PMM
^ permalink raw reply [flat|nested] 17+ messages in thread