Re: [PATCH v2 0/7] ARM virt: Add NVDIMM support

[Auger Eric]

Hi Shameer,

On 1/29/20 11:44 AM, Shameerali Kolothum Thodi wrote:
> Hi Eric,
> 
>> -----Original Message-----
>> From: Auger Eric [mailto:address@hidden]
>> Sent: 28 January 2020 15:29
>> To: Shameerali Kolothum Thodi <address@hidden>;
>> address@hidden; address@hidden; address@hidden
>> Cc: address@hidden; address@hidden; address@hidden;
>> address@hidden; xuwei (O) <address@hidden>;
>> address@hidden; Linuxarm <address@hidden>
>> Subject: Re: [PATCH v2 0/7] ARM virt: Add NVDIMM support
>>
>> Hi Shameer,
>>
>> On 1/17/20 6:45 PM, 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].
>>>
>>> Patch #1 is a fix to the Guest reboot issue on NVDIMM
>>> hot add case described here[2] and patch #2 is another
>>> fix to the nvdimm aml issue discussed here[3].
>>>
>>> I have done a basic sanity testing of NVDIMM deviecs
>>> with Guest booting with both ACPI and DT. Further testing
>>> is always welcome.
>>>
>>> Please let me know your feedback.
>>
>>
>> With this version, I do not get the former spurious warning reported on v1.
>>
>> I can see the nvdimm device topology using ndctl. So it looks fine to me.
> 
> Thanks for giving it a spin and confirming. 
> 
>> Unfortunately we cannot test with DAX as kernel dependencies are not yet
>> resolved yet but this is an independent problem.
> 
> True. I did previously test DAX with "arm64/mm: Enable memory hot remove"
> Patch series and that seems to work fine.

Yes you're correct. I tested with v12 which unfortunately missed the
next kernel merge window if I am not wrong
(https://lkml.org/lkml/2020/1/21/1217). With that series we can
effectovely test with DAX on guest.

ndctl create-namespace --force --mode=fsdax  --reconfig=namespace0.0
mkfs.xfs -f -m reflink=0 /dev/pmem0
sudo mount -o dax /dev/pmem0 /mnt/mem0
[  539.970608] XFS (pmem0): DAX enabled. Warning: EXPERIMENTAL, use at
your own risk
[  539.972947] XFS (pmem0): Mounting V5 Filesystem
[  539.977978] XFS (pmem0): Ending clean mount
[  539.979343] xfs filesystem being mounted at /mnt/mem0 supports
timestamps until 2038 (0x7fffffff)


It is useless for me to send my Tested-by at this point as you need to
remove the tiny conflict. However as soon as you respin I will be
pleased to send it.

As for the 2 first patches, I do not feel sufficiently comfortable on
that part to review them it in decent time and I cowardly leave it to
experts :-(

Thanks

Eric



> 
> Cheers,
> Shameer
> 
> 
>  
>> Thanks
>>
>> Eric
>>>
>>> Thanks,
>>> Shameer
>>>
>>> [1] https://patchwork.kernel.org/cover/10830777/
>>> [2] https://patchwork.kernel.org/patch/11154757/
>>> [3] https://patchwork.kernel.org/cover/11174959/
>>>
>>> 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):
>>>   exec: Fix for qemu_ram_resize() callback
>>>   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
>>>   tests/bios-tables-test: Update arm/virt memhp test
>>>
>>>  docs/specs/acpi_hw_reduced_hotplug.rst      |  1 +
>>>  exec.c                                      | 36 +++++++----
>>>  hw/acpi/generic_event_device.c              | 13 ++++
>>>  hw/acpi/nvdimm.c                            | 68
>> +++++++++++++++++----
>>>  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/exec/ram_addr.h                     |  5 +-
>>>  include/hw/acpi/generic_event_device.h      |  1 +
>>>  include/hw/arm/virt.h                       |  1 +
>>>  include/hw/mem/nvdimm.h                     |  3 +
>>>  tests/data/acpi/virt/NFIT.memhp             |  0
>>>  tests/data/acpi/virt/SSDT.memhp             |  0
>>>  tests/qtest/bios-tables-test-allowed-diff.h |  5 ++
>>>  tests/qtest/bios-tables-test.c              |  9 ++-
>>>  20 files changed, 163 insertions(+), 36 deletions(-)
>>>  create mode 100644 tests/data/acpi/virt/NFIT.memhp
>>>  create mode 100644 tests/data/acpi/virt/SSDT.memhp
>>>
> 
>