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

Shameerali Kolothum Thodi posted 7 patches 4 years, 3 months ago
Only 0 patches received!
RE: [PATCH v2 0/7] ARM virt: Add NVDIMM support
Posted by Shameerali Kolothum Thodi 4 years, 3 months ago
Hi Eric,

> -----Original Message-----
> From: Auger Eric [mailto:eric.auger@redhat.com]
> Sent: 28 January 2020 15:29
> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>;
> qemu-devel@nongnu.org; qemu-arm@nongnu.org; imammedo@redhat.com
> Cc: peter.maydell@linaro.org; shannon.zhaosl@gmail.com; mst@redhat.com;
> xiaoguangrong.eric@gmail.com; xuwei (O) <xuwei5@huawei.com>;
> lersek@redhat.com; Linuxarm <linuxarm@huawei.com>
> 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.

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
> >


Re: [PATCH v2 0/7] ARM virt: Add NVDIMM support
Posted by Auger Eric 4 years, 3 months ago
Hi Shameer,

On 1/29/20 11:44 AM, Shameerali Kolothum Thodi wrote:
> Hi Eric,
> 
>> -----Original Message-----
>> From: Auger Eric [mailto:eric.auger@redhat.com]
>> Sent: 28 January 2020 15:29
>> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>;
>> qemu-devel@nongnu.org; qemu-arm@nongnu.org; imammedo@redhat.com
>> Cc: peter.maydell@linaro.org; shannon.zhaosl@gmail.com; mst@redhat.com;
>> xiaoguangrong.eric@gmail.com; xuwei (O) <xuwei5@huawei.com>;
>> lersek@redhat.com; Linuxarm <linuxarm@huawei.com>
>> 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
>>>
> 
>