[PATCH 0/5] ARM virt: Add NVDIMM support

Shameer Kolothum posted 5 patches 6 years, 1 month ago
Failed in applying to current master (apply log)
There is a newer version of this series
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(-)
[PATCH 0/5] ARM virt: Add NVDIMM support
Posted by Shameer Kolothum 6 years, 1 month ago
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
both ACPI and DT Guest boot. Further testing is always
welcome.

Please let me know your feedback.

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

-- 
2.17.1



Re: [PATCH 0/5] ARM virt: Add NVDIMM support
Posted by Igor Mammedov 5 years, 12 months ago
On Fri, 4 Oct 2019 16:52:57 +0100
Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> 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
> both ACPI and DT Guest boot. Further testing is always
> welcome.
> 
> Please let me know your feedback.
one more thing,

It's possible to run bios-tables-test for virt/arm now,
pls add corresponding test case
it might be easier to just extend test_acpi_virt_tcg_memhp() with nvdimms

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


Re: [PATCH 0/5] ARM virt: Add NVDIMM support
Posted by Auger Eric 6 years ago
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.

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?

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

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

RE: [PATCH 0/5] ARM virt: Add NVDIMM support
Posted by Shameerali Kolothum Thodi 6 years ago
Hi Eric,

> -----Original Message-----
> From: Auger Eric [mailto:eric.auger@redhat.com]
> Sent: 18 October 2019 17:40
> 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; xuwei (O)
> <xuwei5@huawei.com>; lersek@redhat.com; Linuxarm
> <linuxarm@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(-)
> >