Hi Eric, > -----Original Message----- > From: Auger Eric [mailto:eric.au...@redhat.com] > Sent: 18 October 2019 17:40 > To: Shameerali Kolothum Thodi <shameerali.kolothum.th...@huawei.com>; > qemu-devel@nongnu.org; qemu-...@nongnu.org; imamm...@redhat.com > Cc: peter.mayd...@linaro.org; shannon.zha...@gmail.com; xuwei (O) > <xuw...@huawei.com>; ler...@redhat.com; Linuxarm > <linux...@huawei.com> > Subject: Re: [PATCH 0/5] ARM virt: Add NVDIMM support > > Hi Shameer, > > On 10/4/19 5:52 PM, Shameer Kolothum wrote: > > This series adds NVDIMM support to arm/virt platform. > > This has a dependency on [0] and make use of the GED > > device for NVDIMM hotplug events. The series reuses > > some of the patches posted by Eric in his earlier > > attempt here[1]. > > > > Patch 1/5 is a fix to the Guest reboot issue on NVDIMM > > hot add case described here[2]. > > > > I have done basic sanity testing of NVDIMM deviecs with > devcies > > both ACPI and DT Guest boot. Further testing is always > > welcome. > > > > Please let me know your feedback. > > I tested it on my side. Looks to work pretty well.
Thanks for giving this a spin. > one question: I noticed that when a NVDIMM slot is hotplugged one get > the following trace on guest: > > nfit ACPI0012:00: found a zero length table '0' parsing nfit > pmem0: detected capacity change from 0 to 1073741824 > > Have you experienced the 0 length trace? I double checked and yes that trace is there. And I did a quick check with x86 and it is not there. The reason looks like, ARM64 kernel receives an additional 8 bytes size when the kernel evaluates the "_FIT" object. For the same test scenario, Qemu reports a FIT buffer size of 0xb8 and X86 Guest kernel, [ 1.601077] acpi_nfit_init: data 0xffff8a273dc12b18 sz 0xb8 ARM64 Guest, [ 0.933133] acpi_nfit_init: data 0xffff00003cbe6018 sz 0xc0 I am not sure how that size gets changed for ARM which results in the above mentioned 0 length trace. I need to debug this further. Please let me know if you have any pointers... > Besides when we reset the system we find the namespaces again using > "ndctl list -u" so the original bug seems to be fixed. > > Did you try to mount a DAX FS. I can mount but with DAX forced off. > sudo mkdir /mnt/mem0 Yes. I did try with DAX FS. But do we need to change the namespace mode to file system DAX mode? I used the below command before attempting to mount with -o dax, ndctl create-namespace -f -e namespace0.0 --mode=fsdax And in order to do the above you might need the ZONE_DEVICE support in the Kernel which in turn has dependency on hot remove. Hence I tried with "arm64/mm: Enable memory hot remove" patches, https://patchwork.kernel.org/cover/11185169/ > mkfs.xfs -f -m reflink=0 /dev/pmem0 > sudo mount -o dax /dev/pmem0 /mnt/mem0 > [ 2610.051830] XFS (pmem0): DAX enabled. Warning: EXPERIMENTAL, use at > your own risk > [ 2610.178580] XFS (pmem0): DAX unsupported by block device. Turning off > DAX. > [ 2610.180871] XFS (pmem0): Mounting V5 Filesystem > [ 2610.189797] XFS (pmem0): Ending clean mount > > I fail to remember if it was the case months ago. I am not sure if it is > an issue in my guest .config or if there is something not yet supported > on aarch64? Did you try on your side? > > Also if you forget to put the ",nvdimm" to the machvirt options you get, > on hotplug: > {"error": {"class": "GenericError", "desc": "nvdimm is not yet supported"}} > which is not correct anymore ;-) Ok. I will check this. Thanks, Shameer > Thanks > > Eric > > > > > > Thanks, > > Shameer > > > > [0] https://patchwork.kernel.org/cover/11150345/ > > [1] https://patchwork.kernel.org/cover/10830777/ > > [2] https://patchwork.kernel.org/patch/11154757/ > > > > Eric Auger (1): > > hw/arm/boot: Expose the pmem nodes in the DT > > > > Kwangwoo Lee (2): > > nvdimm: Use configurable ACPI IO base and size > > hw/arm/virt: Add nvdimm hot-plug infrastructure > > > > Shameer Kolothum (2): > > hw/arm: Align ACPI blob len to PAGE size > > hw/arm/virt: Add nvdimm hotplug support > > > > docs/specs/acpi_hw_reduced_hotplug.rst | 1 + > > hw/acpi/generic_event_device.c | 13 ++++++++ > > hw/acpi/nvdimm.c | 32 ++++++++++++------ > > hw/arm/Kconfig | 1 + > > hw/arm/boot.c | 45 > ++++++++++++++++++++++++++ > > hw/arm/virt-acpi-build.c | 20 ++++++++++++ > > hw/arm/virt.c | 42 > ++++++++++++++++++++---- > > 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 ++ > > 15 files changed, 157 insertions(+), 17 deletions(-) > >