linux-nvme.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Andrey Smirnov <andrew.smirnov@gmail.com>
To: Guenter Roeck <linux@roeck-us.net>
Cc: Sagi Grimberg <sagi@grimberg.me>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	linux-nvme@lists.infradead.org, Jens Axboe <axboe@fb.com>,
	Chris Healy <Chris.Healy@zii.aero>,
	Keith Busch <kbusch@kernel.org>, Christoph Hellwig <hch@lst.de>
Subject: Re: [PATCH] nvme: Add hardware monitoring support
Date: Mon, 28 Oct 2019 11:01:03 -0700	[thread overview]
Message-ID: <CAHQ1cqFCraPayphD4WWmGP87adGxxmj2Nyae8imotio0r972rA@mail.gmail.com> (raw)
In-Reply-To: <20191028024156.23964-1-linux@roeck-us.net>

On Mon, Oct 28, 2019 at 7:04 AM Guenter Roeck <linux@roeck-us.net> wrote:
>
> nvme devices report temperature information in the controller information
> (for limits) and in the smart log. Currently, the only means to retrieve
> this information is the nvme command line interface, which requires
> super-user privileges.
>
> At the same time, it would be desirable to use NVME temperature information
> for thermal control.
>
> This patch adds support to read NVME temperatures from the kernel using the
> hwmon API and adds temperature zones for NVME drives. The thermal subsystem
> can use this information to set thermal policies, and userspace can access
> it using libsensors and/or the "sensors" command.
>
> Example output from the "sensors" command:
>
> nvme0-pci-0100
> Adapter: PCI adapter
> Composite:    +39.0°C  (high = +85.0°C, crit = +85.0°C)
> Sensor 1:     +39.0°C
> Sensor 2:     +41.0°C
>

Tried this on ZII i.MX8MQ Ultra Zest Board with NVMe device attached.
Seems to work as advertised, so:

Tested-by: Andrey Smirnov <andrew.smirnov@gmail.com>

> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
>  drivers/nvme/host/Kconfig      |  10 +++
>  drivers/nvme/host/Makefile     |   1 +
>  drivers/nvme/host/core.c       |   5 ++
>  drivers/nvme/host/nvme-hwmon.c | 160 +++++++++++++++++++++++++++++++++
>  drivers/nvme/host/nvme.h       |   8 ++
>  5 files changed, 184 insertions(+)
>  create mode 100644 drivers/nvme/host/nvme-hwmon.c
>
> diff --git a/drivers/nvme/host/Kconfig b/drivers/nvme/host/Kconfig
> index 2b36f052bfb9..aeb49e16e386 100644
> --- a/drivers/nvme/host/Kconfig
> +++ b/drivers/nvme/host/Kconfig
> @@ -23,6 +23,16 @@ config NVME_MULTIPATH
>            /dev/nvmeXnY device will show up for each NVMe namespaces,
>            even if it is accessible through multiple controllers.
>
> +config NVME_HWMON
> +       bool "NVME hardware monitoring"
> +       depends on (NVME_CORE=y && HWMON=y) || (NVME_CORE=m && HWMON)
> +       help
> +         This provides support for NVME hardware monitoring. If enabled,
> +         a hardware monitoring device will be created for each NVME drive
> +         in the system.
> +
> +         If unsure, say N.
> +
>  config NVME_FABRICS
>         tristate
>
> diff --git a/drivers/nvme/host/Makefile b/drivers/nvme/host/Makefile
> index 8a4b671c5f0c..03de4797a877 100644
> --- a/drivers/nvme/host/Makefile
> +++ b/drivers/nvme/host/Makefile
> @@ -14,6 +14,7 @@ nvme-core-$(CONFIG_TRACING)           += trace.o
>  nvme-core-$(CONFIG_NVME_MULTIPATH)     += multipath.o
>  nvme-core-$(CONFIG_NVM)                        += lightnvm.o
>  nvme-core-$(CONFIG_FAULT_INJECTION_DEBUG_FS)   += fault_inject.o
> +nvme-core-$(CONFIG_NVME_HWMON)         += nvme-hwmon.o
>
>  nvme-y                                 += pci.o
>
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index fa7ba09dca77..fc1d4b146717 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -2796,6 +2796,9 @@ int nvme_init_identify(struct nvme_ctrl *ctrl)
>         ctrl->oncs = le16_to_cpu(id->oncs);
>         ctrl->mtfa = le16_to_cpu(id->mtfa);
>         ctrl->oaes = le32_to_cpu(id->oaes);
> +       ctrl->wctemp = le16_to_cpu(id->wctemp);
> +       ctrl->cctemp = le16_to_cpu(id->cctemp);
> +
>         atomic_set(&ctrl->abort_limit, id->acl + 1);
>         ctrl->vwc = id->vwc;
>         if (id->mdts)
> @@ -2897,6 +2900,8 @@ int nvme_init_identify(struct nvme_ctrl *ctrl)
>
>         ctrl->identified = true;
>
> +       nvme_hwmon_init(ctrl);
> +
>         return 0;
>
>  out_free:
> diff --git a/drivers/nvme/host/nvme-hwmon.c b/drivers/nvme/host/nvme-hwmon.c
> new file mode 100644
> index 000000000000..f19098bc3228
> --- /dev/null
> +++ b/drivers/nvme/host/nvme-hwmon.c
> @@ -0,0 +1,160 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * NVM Express hardware monitoring support
> + * Copyright (c) 2019, Guenter Roeck
> + */
> +
> +#include <linux/hwmon.h>
> +
> +#include "nvme.h"
> +
> +struct nvme_hwmon_data {
> +       struct nvme_ctrl *ctrl;
> +       struct nvme_smart_log log;
> +};
> +
> +static int nvme_hwmon_get_smart_log(struct nvme_hwmon_data *data)
> +{
> +       return nvme_get_log(data->ctrl, NVME_NSID_ALL, NVME_LOG_SMART, 0,
> +                           &data->log, sizeof(data->log), 0);
> +}
> +
> +static int nvme_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
> +                          u32 attr, int channel, long *val)
> +{
> +       struct nvme_hwmon_data *data = dev_get_drvdata(dev);
> +       struct nvme_smart_log *log = &data->log;
> +       int err;
> +       int temp;
> +
> +       err = nvme_hwmon_get_smart_log(data);
> +       if (err)
> +               return err < 0 ? err : -EPROTO;
> +
> +       switch (attr) {
> +       case hwmon_temp_max:
> +               *val = (data->ctrl->wctemp - 273) * 1000;
> +               break;
> +       case hwmon_temp_crit:
> +               *val = (data->ctrl->cctemp - 273) * 1000;
> +               break;
> +       case hwmon_temp_input:
> +               if (!channel)
> +                       temp = le16_to_cpup((__le16 *)log->temperature);
> +               else
> +                       temp = le16_to_cpu(log->temp_sensor[channel - 1]);
> +               *val = (temp - 273) * 1000;
> +               break;
> +       case hwmon_temp_crit_alarm:
> +               *val = !!(log->critical_warning & NVME_SMART_CRIT_TEMPERATURE);
> +               break;
> +       default:
> +               err = -EOPNOTSUPP;
> +               break;
> +       }
> +       return err;
> +}
> +
> +static const char * const nvme_hwmon_sensor_names[] = {
> +       "Composite",
> +       "Sensor 1",
> +       "Sensor 2",
> +       "Sensor 3",
> +       "Sensor 4",
> +       "Sensor 5",
> +       "Sensor 6",
> +       "Sensor 7",
> +       "Sensor 8",
> +};
> +
> +static int nvme_hwmon_read_string(struct device *dev,
> +                                 enum hwmon_sensor_types type, u32 attr,
> +                                 int channel, const char **str)
> +{
> +       *str = nvme_hwmon_sensor_names[channel];
> +       return 0;
> +}
> +
> +static umode_t nvme_hwmon_is_visible(const void *_data,
> +                                    enum hwmon_sensor_types type,
> +                                    u32 attr, int channel)
> +{
> +       const struct nvme_hwmon_data *data = _data;
> +
> +       switch (attr) {
> +       case hwmon_temp_crit:
> +               if (!channel && data->ctrl->cctemp)
> +                       return 0444;
> +               break;
> +       case hwmon_temp_max:
> +               if (!channel && data->ctrl->wctemp)
> +                       return 0444;
> +               break;
> +       case hwmon_temp_crit_alarm:
> +               if (!channel)
> +                       return 0444;
> +               break;
> +       case hwmon_temp_input:
> +       case hwmon_temp_label:
> +               if (!channel || data->log.temp_sensor[channel - 1])
> +                       return 0444;
> +               break;
> +       default:
> +               break;
> +       }
> +       return 0;
> +}
> +
> +static const struct hwmon_channel_info *nvme_hwmon_info[] = {
> +       HWMON_CHANNEL_INFO(chip, HWMON_C_REGISTER_TZ),
> +       HWMON_CHANNEL_INFO(temp,
> +                          HWMON_T_INPUT | HWMON_T_MAX | HWMON_T_CRIT |
> +                               HWMON_T_LABEL | HWMON_T_CRIT_ALARM,
> +                          HWMON_T_INPUT | HWMON_T_LABEL,
> +                          HWMON_T_INPUT | HWMON_T_LABEL,
> +                          HWMON_T_INPUT | HWMON_T_LABEL,
> +                          HWMON_T_INPUT | HWMON_T_LABEL,
> +                          HWMON_T_INPUT | HWMON_T_LABEL,
> +                          HWMON_T_INPUT | HWMON_T_LABEL,
> +                          HWMON_T_INPUT | HWMON_T_LABEL,
> +                          HWMON_T_INPUT | HWMON_T_LABEL),
> +       NULL
> +};
> +
> +static const struct hwmon_ops nvme_hwmon_ops = {
> +       .is_visible = nvme_hwmon_is_visible,
> +       .read = nvme_hwmon_read,
> +       .read_string = nvme_hwmon_read_string,
> +};
> +
> +static const struct hwmon_chip_info nvme_hwmon_chip_info = {
> +       .ops = &nvme_hwmon_ops,
> +       .info = nvme_hwmon_info,
> +};
> +
> +void nvme_hwmon_init(struct nvme_ctrl *ctrl)
> +{
> +       struct device *dev = ctrl->device;
> +       struct nvme_hwmon_data *data;
> +       struct device *hwmon;
> +       int err;
> +
> +       data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> +       if (!data)
> +               return;
> +
> +       data->ctrl = ctrl;
> +
> +       err = nvme_hwmon_get_smart_log(data);
> +       if (err) {
> +               dev_warn(dev, "Failed to read smart log (error %d)\n", err);
> +               return;
> +       }
> +
> +       hwmon = devm_hwmon_device_register_with_info(dev, dev_name(dev),
> +                                                    data,
> +                                                    &nvme_hwmon_chip_info,
> +                                                    NULL);
> +       if (IS_ERR(hwmon))
> +               dev_warn(dev, "Failed to instantiate hwmon device\n");
> +}
> diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
> index 22e8401352c2..e6460c1216bc 100644
> --- a/drivers/nvme/host/nvme.h
> +++ b/drivers/nvme/host/nvme.h
> @@ -231,6 +231,8 @@ struct nvme_ctrl {
>         u16 kas;
>         u8 npss;
>         u8 apsta;
> +       u16 wctemp;
> +       u16 cctemp;
>         u32 oaes;
>         u32 aen_result;
>         u32 ctratt;
> @@ -652,4 +654,10 @@ static inline struct nvme_ns *nvme_get_ns_from_dev(struct device *dev)
>         return dev_to_disk(dev)->private_data;
>  }
>
> +#if IS_ENABLED(CONFIG_NVME_HWMON)
> +void nvme_hwmon_init(struct nvme_ctrl *ctrl);
> +#else
> +static inline void nvme_hwmon_init(struct nvme_ctrl *ctrl) { }
> +#endif
> +
>  #endif /* _NVME_H */
> --
> 2.17.1
>

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

      parent reply	other threads:[~2019-10-28 18:01 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-28  2:41 Guenter Roeck
2019-10-28  7:39 ` Christoph Hellwig
2019-10-28  8:08   ` Keith Busch
2019-10-28 13:27     ` Guenter Roeck
2019-10-28 18:01 ` Andrey Smirnov [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAHQ1cqFCraPayphD4WWmGP87adGxxmj2Nyae8imotio0r972rA@mail.gmail.com \
    --to=andrew.smirnov@gmail.com \
    --cc=Chris.Healy@zii.aero \
    --cc=axboe@fb.com \
    --cc=hch@lst.de \
    --cc=kbusch@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nvme@lists.infradead.org \
    --cc=linux@roeck-us.net \
    --cc=sagi@grimberg.me \
    --subject='Re: [PATCH] nvme: Add hardware monitoring support' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).