> -----Original Message----- > From: Laszlo Ersek [mailto:ler...@redhat.com] > Sent: 22 February 2019 19:11 > To: Auger Eric <eric.au...@redhat.com>; Shameerali Kolothum Thodi > <shameerali.kolothum.th...@huawei.com>; shannon.zha...@gmail.com; > peter.mayd...@linaro.org; imamm...@redhat.com; qemu-devel@nongnu.org; > qemu-...@nongnu.org > Cc: xuwei (O) <xuw...@huawei.com>; Linuxarm <linux...@huawei.com>; Ard > Biesheuvel <ard.biesheu...@linaro.org>; Leif Lindholm (Linaro address) > <leif.lindh...@linaro.org> > Subject: Re: [RFC PATCH 0/4] ARM virt: ACPI memory hotplug support > > On 02/22/19 17:03, Auger Eric wrote: > > Hi Shameer, > > > > On 1/28/19 12:05 PM, Shameer Kolothum wrote: > >> This series is an attempt to provide hotplug support to both > >> pc-dimm and nvdimm device memory on ARM virt platform. This is > >> based on Eric's recent works to support PCDIMM/NVDIMM device > memory[1]. > >> The kernel support for arm64 memory hot add was added only > >> recently by Robin[2] and hence the guest kernel should be => 5.0-rc1. > >> > >> This makes use of PL061 GPIO controller to sent related ACPI events > > s/sent/send > >> to the Guest. The only reference I could find with respect to the GPIO > >> pins usage is here[3] which says, "use PIN 3 for system_powerdown, > >> reserving PIN 0, 1, 2 for PCI hotplug, CPU hotplug and memory hotplug". > >> Hence Pin 2 is used for PCDIMM and pin 4 for NVDIMM. > >> > >> This is sanity tested on a HiSilicon ARM64 platform and appreciate > >> any further testing. > > > > I did some testing on another platform and I got the exactly the same > > results as yours: PCDIMM hot plug works fine. Also after system_reset I > > still can see the slots. > > Hot-unplug is not supported though. > > For NVDIMM, hot-add works fine and and I can see the slots using ndctl > > on guest. But after system_reset, the guest does not boot properly. > > > >> > >> This series can be applied on top of Eric's branch here[4] > >> > >> Test: > >> ------ > >> Please use a Guest kernel image >5.0-rc1 with all the mem/nvdimm > >> hotplug related CONFIGs enabled. > >> > >> ./qemu-system-aarch64 \ > >> -machine virt,gic-version=3,nvdimm \ > >> -m 1G,maxmem=4G,slots=4 \ > >> -cpu host \ > >> -kernel Image \ > >> -initrd rootfs-iperf.cpio \ > >> -bios QEMU_EFI.fd \ > >> -numa node,nodeid=0 \ > >> -net none \ > >> -nographic -enable-kvm \ > >> -append "console=ttyAMA0 acpi=force earlycon=pl011,0x9000000" > >> > >> Enter Qemu monitor, > >> Add pc-dimm: > >> object_add memory-backend-ram,id=mem1,size=1G > >> device_add pc-dimm,id=dimm1,memdev=mem1 > >> > >> Add nvdimm: > >> object_add memory-backend-ram,id=mem2,size=1G > >> device_add nvdimm,id=dimm2,memdev=mem2 > >> > >> Known Issue: > >> > >> It is observed that hot adding nvdimm will results in guest reboot > >> failure. EDK2 fails to build the ACPI tables on reboot. Please find > >> below EDK2 log on Guest reboot after nvdimm hot-add, > >> > >> ProcessCmdAddChecksum: invalid checksum range in "etc/acpi/tables" > >> OnRootBridgesConnected: InstallAcpiTables: Protocol Error > >> > >> The root cause seems to be EDK2 ACPI table checksum failure > >> as NFIT table is getting updated on hot-add. This needs further > >> investigation. > > + Ard, Leif, Laszlo if they have any idea of what is missing/wrong. > > Huh, very interesting; I usually don't expect my sanity checks to fire > in practice. :) > > The message > > ProcessCmdAddChecksum: invalid checksum range in "etc/acpi/tables" > > is logged by OVMF's and ArmVirtQemu's ACPI Platform DXE Driver when it > finds an invalid COMMAND_ADD_CHECKSUM command in QEMU's ACPI > linker/loader script. > > Please see the command definition in QEMU's > "hw/acpi/bios-linker-loader.c". In particular, please refer to the > function bios_linker_loader_add_checksum(), which builds the command > structure, and documents the fields. > > (You may also refer to QEMU_LOADER_ADD_CHECKSUM in file > "OvmfPkg/AcpiPlatformDxe/QemuLoader.h" in the edk2 source tree, for the > same information.) > > The error message is logged if: > - the offset at which the checksum should be stored falls outside of the > size of the fw_cfg blob, or > - the range over which the checksum should be calculated falls outside > (at least in part) of the fw_cfg blob. > > To me this suggests that QEMU generates an invalid > COMMAND_ADD_CHECKSUM > command for the firmware. > > ... I've tried to skim the patches briefly. I think there must be an > error in the DSDT building logic that is only active on reboot if an > nvdimm module was hot-added before the reboot.
Thanks for taking a look and the pointers. I will debug this further and get back. Thanks, Shameer > Thanks, > Laszlo > > > >> [1]https://lists.gnu.org/archive/html/qemu-devel/2019-01/msg05740.html > >> [2]https://patchwork.kernel.org/patch/10724455/ > >> [3]https://lists.gnu.org/archive/html/qemu-arm/2015-12/msg00095.html > >> [4]https://github.com/eauger/qemu/tree/v3.1.0-dimm-v5 > >> > >> Shameer Kolothum (4): > >> hw:acpi: Make ACPI IO address space configurable > >> hw/arm/virt: Add GPIO based pcdimm hotplug ACPI event support > >> hw/arm/virt: Enable pc-dimm hotplug support > >> hw/arm/virt: Add nvdimm hotplug support > >> > >> default-configs/arm-softmmu.mak | 1 + > >> hw/acpi/memory_hotplug.c | 13 +++-- > >> hw/arm/virt-acpi-build.c | 45 +++++++++++++++-- > >> hw/arm/virt.c | 105 > ++++++++++++++++++++++++++++++++++++--- > >> hw/i386/acpi-build.c | 3 +- > >> include/hw/acpi/memory_hotplug.h | 6 ++- > >> include/hw/arm/virt.h | 15 ++++++ > >> 7 files changed, 168 insertions(+), 20 deletions(-) > >>
