Re: [PATCH v3 10/11] PM, libnvdimm: Add runtime firmware activation support
On Wed, Jul 29, 2020 at 3:35 AM Vishal Verma wrote: > > On Mon, 2020-07-27 at 14:37 +0200, Rafael J. Wysocki wrote: > > On Tue, Jul 21, 2020 at 12:24 AM Dan Williams > > wrote: > > > Abstract platform specific mechanics for nvdimm firmware activation > > > behind a handful of generic ops. At the bus level ->activate_state() > > > indicates the unified state (idle, busy, armed) of all DIMMs on the bus, > > > and ->capability() indicates the system state expectations for activate. > > > At the DIMM level ->activate_state() indicates the per-DIMM state, > > > ->activate_result() indicates the outcome of the last activation > > > attempt, and ->arm() attempts to transition the DIMM from 'idle' to > > > 'armed'. > > > > > > A new hibernate_quiet_exec() facility is added to support firmware > > > activation in an OS defined system quiesce state. It leverages the fact > > > that the hibernate-freeze state wants to assert that a memory > > > hibernation snapshot can be taken. This is in contrast to a platform > > > firmware defined quiesce state that may forcefully quiet the memory > > > controller independent of whether an individual device-driver properly > > > supports hibernate-freeze. > > > > > > The libnvdimm sysfs interface is extended to support detection of a > > > firmware activate capability. The mechanism supports enumeration and > > > triggering of firmware activate, optionally in the > > > hibernate_quiet_exec() context. > > > > > > Cc: Pavel Machek > > > Cc: Ira Weiny > > > Cc: Len Brown > > > Cc: Jonathan Corbet > > > Cc: Dave Jiang > > > Cc: Vishal Verma > > > [rafael: hibernate_quiet_exec() proposal] > > > Co-developed-by: "Rafael J. Wysocki" > > > > IMO it's better to change this to > > > > Co-developed-by: "Rafael J. Wysocki" > > > > and please to add > > > > Signed-off-by: Rafael J. Wysocki > > > > to it as per the development process documentation. > > Thanks Rafael, I've fixed this up in the branch I've prepared for the pull > request: > > https://git.kernel.org/pub/scm/linux/kernel/git/nvdimm/nvdimm.git/log/?h=libnvdimm-for-next LGTM, thanks!
Re: [PATCH v3 10/11] PM, libnvdimm: Add runtime firmware activation support
On Mon, 2020-07-27 at 14:37 +0200, Rafael J. Wysocki wrote:
> On Tue, Jul 21, 2020 at 12:24 AM Dan Williams
> wrote:
> > Abstract platform specific mechanics for nvdimm firmware activation
> > behind a handful of generic ops. At the bus level ->activate_state()
> > indicates the unified state (idle, busy, armed) of all DIMMs on the bus,
> > and ->capability() indicates the system state expectations for activate.
> > At the DIMM level ->activate_state() indicates the per-DIMM state,
> > ->activate_result() indicates the outcome of the last activation
> > attempt, and ->arm() attempts to transition the DIMM from 'idle' to
> > 'armed'.
> >
> > A new hibernate_quiet_exec() facility is added to support firmware
> > activation in an OS defined system quiesce state. It leverages the fact
> > that the hibernate-freeze state wants to assert that a memory
> > hibernation snapshot can be taken. This is in contrast to a platform
> > firmware defined quiesce state that may forcefully quiet the memory
> > controller independent of whether an individual device-driver properly
> > supports hibernate-freeze.
> >
> > The libnvdimm sysfs interface is extended to support detection of a
> > firmware activate capability. The mechanism supports enumeration and
> > triggering of firmware activate, optionally in the
> > hibernate_quiet_exec() context.
> >
> > Cc: Pavel Machek
> > Cc: Ira Weiny
> > Cc: Len Brown
> > Cc: Jonathan Corbet
> > Cc: Dave Jiang
> > Cc: Vishal Verma
> > [rafael: hibernate_quiet_exec() proposal]
> > Co-developed-by: "Rafael J. Wysocki"
>
> IMO it's better to change this to
>
> Co-developed-by: "Rafael J. Wysocki"
>
> and please to add
>
> Signed-off-by: Rafael J. Wysocki
>
> to it as per the development process documentation.
Thanks Rafael, I've fixed this up in the branch I've prepared for the pull
request:
https://git.kernel.org/pub/scm/linux/kernel/git/nvdimm/nvdimm.git/log/?h=libnvdimm-for-next
>
> > Signed-off-by: Dan Williams
> > ---
> > Documentation/ABI/testing/sysfs-bus-nvdimm |2
> > .../driver-api/nvdimm/firmware-activate.rst| 86
> > drivers/nvdimm/core.c | 149
> >
> > drivers/nvdimm/dimm_devs.c | 115 +++
> > drivers/nvdimm/nd-core.h |1
> > include/linux/libnvdimm.h | 44 ++
> > include/linux/suspend.h|6 +
> > kernel/power/hibernate.c | 97 +
> > 8 files changed, 500 insertions(+)
> > create mode 100644 Documentation/ABI/testing/sysfs-bus-nvdimm
> > create mode 100644 Documentation/driver-api/nvdimm/firmware-activate.rst
> >
[..]
> > @@ -464,6 +466,10 @@ static inline void hibernation_set_ops(const struct
> > platform_hibernation_ops *op
> > static inline int hibernate(void) { return -ENOSYS; }
> > static inline bool system_entering_hibernation(void) { return false; }
> > static inline bool hibernation_available(void) { return false; }
> > +
> > +static inline hibernate_quiet_exec(int (*func)(void *data), void *data) {
>
> This needs to be "static inline int".
>
Yep I got a build warning for this and also fixed it up.
Thanks,
-Vishal
Re: [PATCH v3 10/11] PM, libnvdimm: Add runtime firmware activation support
On Tue, Jul 21, 2020 at 12:24 AM Dan Williams wrote: > > Abstract platform specific mechanics for nvdimm firmware activation > behind a handful of generic ops. At the bus level ->activate_state() > indicates the unified state (idle, busy, armed) of all DIMMs on the bus, > and ->capability() indicates the system state expectations for activate. > At the DIMM level ->activate_state() indicates the per-DIMM state, > ->activate_result() indicates the outcome of the last activation > attempt, and ->arm() attempts to transition the DIMM from 'idle' to > 'armed'. > > A new hibernate_quiet_exec() facility is added to support firmware > activation in an OS defined system quiesce state. It leverages the fact > that the hibernate-freeze state wants to assert that a memory > hibernation snapshot can be taken. This is in contrast to a platform > firmware defined quiesce state that may forcefully quiet the memory > controller independent of whether an individual device-driver properly > supports hibernate-freeze. > > The libnvdimm sysfs interface is extended to support detection of a > firmware activate capability. The mechanism supports enumeration and > triggering of firmware activate, optionally in the > hibernate_quiet_exec() context. > > Cc: Pavel Machek > Cc: Ira Weiny > Cc: Len Brown > Cc: Jonathan Corbet > Cc: Dave Jiang > Cc: Vishal Verma > [rafael: hibernate_quiet_exec() proposal] > Co-developed-by: "Rafael J. Wysocki" IMO it's better to change this to Co-developed-by: "Rafael J. Wysocki" and please to add Signed-off-by: Rafael J. Wysocki to it as per the development process documentation. > Signed-off-by: Dan Williams > --- > Documentation/ABI/testing/sysfs-bus-nvdimm |2 > .../driver-api/nvdimm/firmware-activate.rst| 86 > drivers/nvdimm/core.c | 149 > > drivers/nvdimm/dimm_devs.c | 115 +++ > drivers/nvdimm/nd-core.h |1 > include/linux/libnvdimm.h | 44 ++ > include/linux/suspend.h|6 + > kernel/power/hibernate.c | 97 + > 8 files changed, 500 insertions(+) > create mode 100644 Documentation/ABI/testing/sysfs-bus-nvdimm > create mode 100644 Documentation/driver-api/nvdimm/firmware-activate.rst > > diff --git a/Documentation/ABI/testing/sysfs-bus-nvdimm > b/Documentation/ABI/testing/sysfs-bus-nvdimm > new file mode 100644 > index ..d64380262be8 > --- /dev/null > +++ b/Documentation/ABI/testing/sysfs-bus-nvdimm > @@ -0,0 +1,2 @@ > +The libnvdimm sub-system implements a common sysfs interface for > +platform nvdimm resources. See Documentation/driver-api/nvdimm/. > diff --git a/Documentation/driver-api/nvdimm/firmware-activate.rst > b/Documentation/driver-api/nvdimm/firmware-activate.rst > new file mode 100644 > index ..9eb98aa833c5 > --- /dev/null > +++ b/Documentation/driver-api/nvdimm/firmware-activate.rst > @@ -0,0 +1,86 @@ > +.. SPDX-License-Identifier: GPL-2.0 > + > +== > +NVDIMM Runtime Firmware Activation > +== > + > +Some persistent memory devices run a firmware locally on the device / > +"DIMM" to perform tasks like media management, capacity provisioning, > +and health monitoring. The process of updating that firmware typically > +involves a reboot because it has implications for in-flight memory > +transactions. However, reboots are disruptive and at least the Intel > +persistent memory platform implementation, described by the Intel ACPI > +DSM specification [1], has added support for activating firmware at > +runtime. > + > +A native sysfs interface is implemented in libnvdimm to allow platform > +to advertise and control their local runtime firmware activation > +capability. > + > +The libnvdimm bus object, ndbusX, implements an ndbusX/firmware/activate > +attribute that shows the state of the firmware activation as one of 'idle', > +'armed', 'overflow', and 'busy'. > + > +- idle: > + No devices are set / armed to activate firmware > + > +- armed: > + At least one device is armed > + > +- busy: > + In the busy state armed devices are in the process of transitioning > + back to idle and completing an activation cycle. > + > +- overflow: > + If the platform has a concept of incremental work needed to perform > + the activation it could be the case that too many DIMMs are armed for > + activation. In that scenario the potential for firmware activation to > + timeout is indicated by the 'overflow' state. > + > +The 'ndbusX/firmware/activate' property can be written with a value of > +either 'live', or 'quiesce'. A value of 'quiesce' triggers the kernel to > +run firmware activation from within the equivalent of the hibernation > +'freeze' state where drivers and applications are notified to stop their > +modifications of system
Re: [PATCH v3 10/11] PM, libnvdimm: Add runtime firmware activation support
Hi Dan, I love your patch! Perhaps something to improve: [auto build test WARNING on 48778464bb7d346b47157d21ffde2af6b2d39110] url: https://github.com/0day-ci/linux/commits/Dan-Williams/ACPI-NVDIMM-Runtime-Firmware-Activation/20200721-062902 base:48778464bb7d346b47157d21ffde2af6b2d39110 :: branch date: 8 hours ago :: commit date: 8 hours ago config: x86_64-randconfig-s021-20200719 (attached as .config) compiler: gcc-9 (Debian 9.3.0-14) 9.3.0 reproduce: # apt-get install sparse # sparse version: v0.6.2-49-g707c5017-dirty # save the attached .config to linux build tree make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=x86_64 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot sparse warnings: (new ones prefixed by >>) >> include/linux/suspend.h:470:15: sparse: sparse: 'hibernate_quiet_exec()' has >> implicit return type -- >> include/linux/suspend.h:470:15: sparse: sparse: 'hibernate_quiet_exec()' has >> implicit return type drivers/bluetooth/btusb.c:2245:25: sparse: sparse: cast to restricted __le16 drivers/bluetooth/btusb.c:2254:25: sparse: sparse: cast to restricted __le16 drivers/bluetooth/btusb.c:2255:25: sparse: sparse: cast to restricted __le16 drivers/bluetooth/btusb.c:2256:25: sparse: sparse: cast to restricted __le16 -- >> include/linux/suspend.h:470:15: sparse: sparse: 'hibernate_quiet_exec()' has >> implicit return type drivers/cpufreq/cpufreq.c:471:17: sparse: sparse: incorrect type in assignment (different address spaces) @@ expected struct notifier_block *nb @@ got struct notifier_block [noderef] __rcu *static [addressable] [toplevel] head @@ drivers/cpufreq/cpufreq.c:471:17: sparse: expected struct notifier_block *nb drivers/cpufreq/cpufreq.c:471:17: sparse: got struct notifier_block [noderef] __rcu *static [addressable] [toplevel] head drivers/cpufreq/cpufreq.c:471:65: sparse: sparse: incorrect type in assignment (different address spaces) @@ expected struct notifier_block *nb @@ got struct notifier_block [noderef] __rcu *next @@ drivers/cpufreq/cpufreq.c:471:65: sparse: expected struct notifier_block *nb drivers/cpufreq/cpufreq.c:471:65: sparse: got struct notifier_block [noderef] __rcu *next -- >> include/linux/suspend.h:470:15: sparse: sparse: 'hibernate_quiet_exec()' has >> implicit return type drivers/regulator/internal.h:43:42: sparse: sparse: restricted suspend_state_t degrades to integer drivers/regulator/core.c:1627:56: sparse: sparse: restricted suspend_state_t degrades to integer drivers/regulator/core.c:1629:56: sparse: sparse: restricted suspend_state_t degrades to integer drivers/regulator/core.c:455:17: sparse: sparse: restricted suspend_state_t degrades to integer drivers/regulator/core.c:455:25: sparse: sparse: restricted suspend_state_t degrades to integer drivers/regulator/core.c:469:47: sparse: sparse: restricted suspend_state_t degrades to integer drivers/regulator/core.c:3347:65: sparse: sparse: restricted suspend_state_t degrades to integer drivers/regulator/core.c:3823:47: sparse: sparse: restricted suspend_state_t degrades to integer drivers/regulator/core.c:3965:65: sparse: sparse: restricted suspend_state_t degrades to integer drivers/regulator/core.c:5527:54: sparse: sparse: restricted suspend_state_t degrades to integer drivers/regulator/core.c:5528:54: sparse: sparse: restricted suspend_state_t degrades to integer -- >> include/linux/suspend.h:470:15: sparse: sparse: 'hibernate_quiet_exec()' has >> implicit return type drivers/regulator/internal.h:43:42: sparse: sparse: restricted suspend_state_t degrades to integer -- >> include/linux/suspend.h:470:15: sparse: sparse: 'hibernate_quiet_exec()' has >> implicit return type drivers/regulator/of_regulator.c:18:43: sparse: sparse: restricted suspend_state_t degrades to integer drivers/regulator/of_regulator.c:193:22: sparse: sparse: restricted suspend_state_t degrades to integer drivers/regulator/of_regulator.c:196:22: sparse: sparse: restricted suspend_state_t degrades to integer drivers/regulator/of_regulator.c:199:22: sparse: sparse: restricted suspend_state_t degrades to integer drivers/regulator/of_regulator.c:202:22: sparse: sparse: restricted suspend_state_t degrades to integer drivers/regulator/of_regulator.c:203:22: sparse: sparse: restricted suspend_state_t degrades to integer drivers/regulator/of_regulator.c:252:26: sparse: sparse: restricted suspend_state_t degrades to integer -- >> include/linux/suspend.h:470:15: sparse: sparse: 'hibernate_quiet_exec()' has >> implicit return type drivers/regulator/da9063-regulator.c:514:17: sparse: sparse: Initializer entry defined twice drivers/regulator/da9063-regulator.c:515:18: sparse: also defined here -- >> include/linux/suspend.h:470:15: sparse: sparse: 'hibernate_quiet_exec()' has >> implicit
Re: [PATCH v3 10/11] PM, libnvdimm: Add runtime firmware activation support
On Mon, Jul 20, 2020 at 5:14 PM Vishal Verma wrote: > > On Mon, 2020-07-20 at 17:02 -0700, Randy Dunlap wrote: > > Hi Dan, > > > > Documentation comments below: > > Dan, Randy, > > I'm happy to fix these up when applying. Sounds good. Thanks Vishal.
Re: [PATCH v3 10/11] PM, libnvdimm: Add runtime firmware activation support
On Mon, Jul 20, 2020 at 5:02 PM Randy Dunlap wrote: > > Hi Dan, > > Documentation comments below: > > On 7/20/20 3:08 PM, Dan Williams wrote: > > Abstract platform specific mechanics for nvdimm firmware activation > > behind a handful of generic ops. At the bus level ->activate_state() > > indicates the unified state (idle, busy, armed) of all DIMMs on the bus, > > and ->capability() indicates the system state expectations for activate. > > At the DIMM level ->activate_state() indicates the per-DIMM state, > > ->activate_result() indicates the outcome of the last activation > > attempt, and ->arm() attempts to transition the DIMM from 'idle' to > > 'armed'. > > > > A new hibernate_quiet_exec() facility is added to support firmware > > activation in an OS defined system quiesce state. It leverages the fact > > that the hibernate-freeze state wants to assert that a memory > > hibernation snapshot can be taken. This is in contrast to a platform > > firmware defined quiesce state that may forcefully quiet the memory > > controller independent of whether an individual device-driver properly > > supports hibernate-freeze. > > > > The libnvdimm sysfs interface is extended to support detection of a > > firmware activate capability. The mechanism supports enumeration and > > triggering of firmware activate, optionally in the > > hibernate_quiet_exec() context. > > > > Cc: Pavel Machek > > Cc: Ira Weiny > > Cc: Len Brown > > Cc: Jonathan Corbet > > Cc: Dave Jiang > > Cc: Vishal Verma > > [rafael: hibernate_quiet_exec() proposal] > > Co-developed-by: "Rafael J. Wysocki" > > Signed-off-by: Dan Williams > > --- > > Documentation/ABI/testing/sysfs-bus-nvdimm |2 > > .../driver-api/nvdimm/firmware-activate.rst| 86 > > drivers/nvdimm/core.c | 149 > > > > drivers/nvdimm/dimm_devs.c | 115 +++ > > drivers/nvdimm/nd-core.h |1 > > include/linux/libnvdimm.h | 44 ++ > > include/linux/suspend.h|6 + > > kernel/power/hibernate.c | 97 + > > 8 files changed, 500 insertions(+) > > create mode 100644 Documentation/ABI/testing/sysfs-bus-nvdimm > > create mode 100644 Documentation/driver-api/nvdimm/firmware-activate.rst > > > > diff --git a/Documentation/driver-api/nvdimm/firmware-activate.rst > > b/Documentation/driver-api/nvdimm/firmware-activate.rst > > new file mode 100644 > > index ..9eb98aa833c5 > > --- /dev/null > > +++ b/Documentation/driver-api/nvdimm/firmware-activate.rst > > @@ -0,0 +1,86 @@ > > +.. SPDX-License-Identifier: GPL-2.0 > > + > > +== > > +NVDIMM Runtime Firmware Activation > > +== > > + > > +Some persistent memory devices run a firmware locally on the device / > > run firmware That works too. I was going to say "run a firmware image", but "run firmware" is clearer. > > > +"DIMM" to perform tasks like media management, capacity provisioning, > > +and health monitoring. The process of updating that firmware typically > > +involves a reboot because it has implications for in-flight memory > > +transactions. However, reboots are disruptive and at least the Intel > > +persistent memory platform implementation, described by the Intel ACPI > > +DSM specification [1], has added support for activating firmware at > > that's an Intel spec? just checking. Correct. It's a public specification of the ACPI methods that Intel platform BIOS or virtual-machine BIOS deploys to talk to NVDIMM devices. > > > +runtime. > > + > > +A native sysfs interface is implemented in libnvdimm to allow platform > > platforms Ack. > > > +to advertise and control their local runtime firmware activation > > +capability. > > + > > +The libnvdimm bus object, ndbusX, implements an ndbusX/firmware/activate > > +attribute that shows the state of the firmware activation as one of 'idle', > > +'armed', 'overflow', and 'busy'. > > or Yup. > > > + > > +- idle: > > + No devices are set / armed to activate firmware > > + > > +- armed: > > + At least one device is armed > > + > > +- busy: > > + In the busy state armed devices are in the process of transitioning > > + back to idle and completing an activation cycle. > > + > > +- overflow: > > + If the platform has a concept of incremental work needed to perform > > + the activation it could be the case that too many DIMMs are armed for > > + activation. In that scenario the potential for firmware activation to > > + timeout is indicated by the 'overflow' state. > > + > > +The 'ndbusX/firmware/activate' property can be written with a value of > > +either 'live', or 'quiesce'. A value of 'quiesce' triggers the kernel to > > +run
Re: [PATCH v3 10/11] PM, libnvdimm: Add runtime firmware activation support
On Mon, 2020-07-20 at 17:02 -0700, Randy Dunlap wrote: > Hi Dan, > > Documentation comments below: Dan, Randy, I'm happy to fix these up when applying. > > On 7/20/20 3:08 PM, Dan Williams wrote: > > Abstract platform specific mechanics for nvdimm firmware activation > > behind a handful of generic ops. At the bus level ->activate_state() > > indicates the unified state (idle, busy, armed) of all DIMMs on the bus, > > and ->capability() indicates the system state expectations for activate. > > At the DIMM level ->activate_state() indicates the per-DIMM state, > > ->activate_result() indicates the outcome of the last activation > > attempt, and ->arm() attempts to transition the DIMM from 'idle' to > > 'armed'. > > > > A new hibernate_quiet_exec() facility is added to support firmware > > activation in an OS defined system quiesce state. It leverages the fact > > that the hibernate-freeze state wants to assert that a memory > > hibernation snapshot can be taken. This is in contrast to a platform > > firmware defined quiesce state that may forcefully quiet the memory > > controller independent of whether an individual device-driver properly > > supports hibernate-freeze. > > > > The libnvdimm sysfs interface is extended to support detection of a > > firmware activate capability. The mechanism supports enumeration and > > triggering of firmware activate, optionally in the > > hibernate_quiet_exec() context. > > > > Cc: Pavel Machek > > Cc: Ira Weiny > > Cc: Len Brown > > Cc: Jonathan Corbet > > Cc: Dave Jiang > > Cc: Vishal Verma > > [rafael: hibernate_quiet_exec() proposal] > > Co-developed-by: "Rafael J. Wysocki" > > Signed-off-by: Dan Williams > > --- > > Documentation/ABI/testing/sysfs-bus-nvdimm |2 > > .../driver-api/nvdimm/firmware-activate.rst| 86 > > drivers/nvdimm/core.c | 149 > > > > drivers/nvdimm/dimm_devs.c | 115 +++ > > drivers/nvdimm/nd-core.h |1 > > include/linux/libnvdimm.h | 44 ++ > > include/linux/suspend.h|6 + > > kernel/power/hibernate.c | 97 + > > 8 files changed, 500 insertions(+) > > create mode 100644 Documentation/ABI/testing/sysfs-bus-nvdimm > > create mode 100644 Documentation/driver-api/nvdimm/firmware-activate.rst > > diff --git a/Documentation/driver-api/nvdimm/firmware-activate.rst > > b/Documentation/driver-api/nvdimm/firmware-activate.rst > > new file mode 100644 > > index ..9eb98aa833c5 > > --- /dev/null > > +++ b/Documentation/driver-api/nvdimm/firmware-activate.rst > > @@ -0,0 +1,86 @@ > > +.. SPDX-License-Identifier: GPL-2.0 > > + > > +== > > +NVDIMM Runtime Firmware Activation > > +== > > + > > +Some persistent memory devices run a firmware locally on the device / > > run firmware > > > +"DIMM" to perform tasks like media management, capacity provisioning, > > +and health monitoring. The process of updating that firmware typically > > +involves a reboot because it has implications for in-flight memory > > +transactions. However, reboots are disruptive and at least the Intel > > +persistent memory platform implementation, described by the Intel ACPI > > +DSM specification [1], has added support for activating firmware at > > that's an Intel spec? just checking. > > > +runtime. > > + > > +A native sysfs interface is implemented in libnvdimm to allow platform > > platforms > > > +to advertise and control their local runtime firmware activation > > +capability. > > + > > +The libnvdimm bus object, ndbusX, implements an ndbusX/firmware/activate > > +attribute that shows the state of the firmware activation as one of 'idle', > > +'armed', 'overflow', and 'busy'. > > or > > > + > > +- idle: > > + No devices are set / armed to activate firmware > > + > > +- armed: > > + At least one device is armed > > + > > +- busy: > > + In the busy state armed devices are in the process of transitioning > > + back to idle and completing an activation cycle. > > + > > +- overflow: > > + If the platform has a concept of incremental work needed to perform > > + the activation it could be the case that too many DIMMs are armed for > > + activation. In that scenario the potential for firmware activation to > > + timeout is indicated by the 'overflow' state. > > + > > +The 'ndbusX/firmware/activate' property can be written with a value of > > +either 'live', or 'quiesce'. A value of 'quiesce' triggers the kernel to > > +run firmware activation from within the equivalent of the hibernation > > +'freeze' state where drivers and applications are notified to stop their > > +modifications of system memory. A
Re: [PATCH v3 10/11] PM, libnvdimm: Add runtime firmware activation support
Hi Dan, Documentation comments below: On 7/20/20 3:08 PM, Dan Williams wrote: > Abstract platform specific mechanics for nvdimm firmware activation > behind a handful of generic ops. At the bus level ->activate_state() > indicates the unified state (idle, busy, armed) of all DIMMs on the bus, > and ->capability() indicates the system state expectations for activate. > At the DIMM level ->activate_state() indicates the per-DIMM state, > ->activate_result() indicates the outcome of the last activation > attempt, and ->arm() attempts to transition the DIMM from 'idle' to > 'armed'. > > A new hibernate_quiet_exec() facility is added to support firmware > activation in an OS defined system quiesce state. It leverages the fact > that the hibernate-freeze state wants to assert that a memory > hibernation snapshot can be taken. This is in contrast to a platform > firmware defined quiesce state that may forcefully quiet the memory > controller independent of whether an individual device-driver properly > supports hibernate-freeze. > > The libnvdimm sysfs interface is extended to support detection of a > firmware activate capability. The mechanism supports enumeration and > triggering of firmware activate, optionally in the > hibernate_quiet_exec() context. > > Cc: Pavel Machek > Cc: Ira Weiny > Cc: Len Brown > Cc: Jonathan Corbet > Cc: Dave Jiang > Cc: Vishal Verma > [rafael: hibernate_quiet_exec() proposal] > Co-developed-by: "Rafael J. Wysocki" > Signed-off-by: Dan Williams > --- > Documentation/ABI/testing/sysfs-bus-nvdimm |2 > .../driver-api/nvdimm/firmware-activate.rst| 86 > drivers/nvdimm/core.c | 149 > > drivers/nvdimm/dimm_devs.c | 115 +++ > drivers/nvdimm/nd-core.h |1 > include/linux/libnvdimm.h | 44 ++ > include/linux/suspend.h|6 + > kernel/power/hibernate.c | 97 + > 8 files changed, 500 insertions(+) > create mode 100644 Documentation/ABI/testing/sysfs-bus-nvdimm > create mode 100644 Documentation/driver-api/nvdimm/firmware-activate.rst > diff --git a/Documentation/driver-api/nvdimm/firmware-activate.rst > b/Documentation/driver-api/nvdimm/firmware-activate.rst > new file mode 100644 > index ..9eb98aa833c5 > --- /dev/null > +++ b/Documentation/driver-api/nvdimm/firmware-activate.rst > @@ -0,0 +1,86 @@ > +.. SPDX-License-Identifier: GPL-2.0 > + > +== > +NVDIMM Runtime Firmware Activation > +== > + > +Some persistent memory devices run a firmware locally on the device / run firmware > +"DIMM" to perform tasks like media management, capacity provisioning, > +and health monitoring. The process of updating that firmware typically > +involves a reboot because it has implications for in-flight memory > +transactions. However, reboots are disruptive and at least the Intel > +persistent memory platform implementation, described by the Intel ACPI > +DSM specification [1], has added support for activating firmware at that's an Intel spec? just checking. > +runtime. > + > +A native sysfs interface is implemented in libnvdimm to allow platform platforms > +to advertise and control their local runtime firmware activation > +capability. > + > +The libnvdimm bus object, ndbusX, implements an ndbusX/firmware/activate > +attribute that shows the state of the firmware activation as one of 'idle', > +'armed', 'overflow', and 'busy'. or > + > +- idle: > + No devices are set / armed to activate firmware > + > +- armed: > + At least one device is armed > + > +- busy: > + In the busy state armed devices are in the process of transitioning > + back to idle and completing an activation cycle. > + > +- overflow: > + If the platform has a concept of incremental work needed to perform > + the activation it could be the case that too many DIMMs are armed for > + activation. In that scenario the potential for firmware activation to > + timeout is indicated by the 'overflow' state. > + > +The 'ndbusX/firmware/activate' property can be written with a value of > +either 'live', or 'quiesce'. A value of 'quiesce' triggers the kernel to > +run firmware activation from within the equivalent of the hibernation > +'freeze' state where drivers and applications are notified to stop their > +modifications of system memory. A value of 'live' attempts > +firmware-activation without this hibernation cycle. The no hyphen^^ > +'ndbusX/firmware/activate' property will be elided completely if no > +firmware activation capability is detected. > + > +Another property 'ndbusX/firmware/capability' indicates a value of > +'live', or 'quiesce'. Where 'live'
