On 03/28/2017 07:14 PM, [email protected] wrote: > Dell - Internal Use - Confidential
Um, but you just sent this to a public mailing list. More below... > > > >> -----Original Message----- >> From: Linda Knippers [mailto:[email protected]] >> Sent: Tuesday, March 28, 2017 5:02 PM >> To: Lijun Pan <[email protected]>; [email protected] >> Cc: Pan, Lijun <[email protected]>; Hayes, Stuart >> <[email protected]> >> Subject: Re: [PATCH v2] libndctl: add support for the MSFT family of DSM >> functions >> >> [EXTERNAL EMAIL] >> >> On 03/26/2017 04:17 PM, Lijun Pan wrote: >>> From: Lijun Pan <[email protected]> >>> >>> This patch retrieves the health data from NVDIMM-N via >>> the MSFT _DSM function[1], following JESD245A[2] standards. >>> Now 'ndctl list --dimms --health --idle' could work >>> on MSFT type NVDIMM-N, but limited to health_state, >>> temperature_celsius, and life_used_percentage. >>> >>> [1]. https://msdn.microsoft.com/library/windows/hardware/mt604741 >>> [2]. https://www.jedec.org/sites/default/files/docs/JESD245A.pdf >> >> Is there a public version of the JEDEC spec? When I go there, I get >> a login screen. If it's not public, then perhaps you can extract whatever >> the interesting parts are. Or is the reference even needed if the >> MSFT DSM spec describes the fields you're exposing? > > You need to login to download a free version. You need a JEDEC spec to > understand > the meaning of the fields in MSFT DSM spec. I don't think I do for the fields that you're exposing. There was another comment further down about msft_cmd_smart_get_temperature() that you missed so keep going. > > Lijun > >>> >>> Cc: Stuart Hayes <[email protected]> >>> Signed-off-by: Lijun Pan <[email protected]> >>> --- >>> v2: >>> - v2 combines v1's 1/2 and 2/2 >>> - reuse the existing libndctl abstraction (suggested by Dan) >>> - move the MSFT _DSM code under ndctl/lib/ >>> - the closet common field we can have between MSFT _DSM and smart is >>> health_state, temperature_celsius, and life_used_percentage. >>> >>> >>> ndctl/lib/Makefile.am | 1 + >>> ndctl/lib/libndctl-msft.c | 143 >> +++++++++++++++++++++++++++++++++++++++++++ >>> ndctl/lib/libndctl-private.h | 4 ++ >>> ndctl/lib/libndctl.c | 2 + >>> ndctl/lib/ndctl-msft.h | 63 +++++++++++++++++++ >>> 5 files changed, 213 insertions(+) >>> create mode 100644 ndctl/lib/libndctl-msft.c >>> create mode 100644 ndctl/lib/ndctl-msft.h >>> >>> diff --git a/ndctl/lib/Makefile.am b/ndctl/lib/Makefile.am >>> index 58a0bb3..7a446be 100644 >>> --- a/ndctl/lib/Makefile.am >>> +++ b/ndctl/lib/Makefile.am >>> @@ -32,6 +32,7 @@ endif >>> if ENABLE_SMART >>> libndctl_la_SOURCES += libndctl-smart.c >>> libndctl_la_SOURCES += libndctl-hpe1.c >>> +libndctl_la_SOURCES += libndctl-msft.c >>> endif >>> >>> EXTRA_DIST += libndctl.sym >>> diff --git a/ndctl/lib/libndctl-msft.c b/ndctl/lib/libndctl-msft.c >>> new file mode 100644 >>> index 0000000..868e5c0 >>> --- /dev/null >>> +++ b/ndctl/lib/libndctl-msft.c >>> @@ -0,0 +1,143 @@ >>> +/* >>> + * Copyright (C) 2016-2017 Dell, Inc. >>> + * Copyright (C) 2016 Hewlett Packard Enterprise Development LP >>> + * Copyright (c) 2016, Intel Corporation. >>> + * >>> + * This program is free software; you can redistribute it and/or modify it >>> + * under the terms and conditions of the GNU Lesser General Public >> License, >>> + * version 2.1, as published by the Free Software Foundation. >>> + * >>> + * This program is distributed in the hope it will be useful, but WITHOUT >> ANY >>> + * WARRANTY; without even the implied warranty of MERCHANTABILITY >> or FITNESS >>> + * FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public >> License for >>> + * more details. >>> + */ >>> +#include <stdlib.h> >>> +#include <limits.h> >>> +#include <util/log.h> >>> +#include <ndctl/libndctl.h> >>> +#include "libndctl-private.h" >>> +#include "ndctl-msft.h" >>> + >>> +#define CMD_MSFT(_c) ((_c)->msft) >>> +#define CMD_MSFT_SMART(_c) (CMD_MSFT(_c)->u.smart.data) >>> + >>> +static struct ndctl_cmd *msft_dimm_cmd_new_smart(struct ndctl_dimm >> *dimm) >>> +{ >>> + struct ndctl_bus *bus = ndctl_dimm_get_bus(dimm); >>> + struct ndctl_ctx *ctx = ndctl_bus_get_ctx(bus); >>> + struct ndctl_cmd *cmd; >>> + size_t size; >>> + struct ndn_pkg_msft *msft; >>> + >>> + if (!ndctl_dimm_is_cmd_supported(dimm, ND_CMD_CALL)) { >>> + dbg(ctx, "unsupported cmd\n"); >>> + return NULL; >>> + } >>> + >>> + size = sizeof(*cmd) + sizeof(struct ndn_pkg_msft); >>> + cmd = calloc(1, size); >>> + if (!cmd) >>> + return NULL; >>> + >>> + cmd->dimm = dimm; >>> + ndctl_cmd_ref(cmd); >>> + cmd->type = ND_CMD_CALL; >>> + cmd->size = size; >>> + cmd->status = 1; >>> + >>> + msft = CMD_MSFT(cmd); >>> + msft->gen.nd_family = NVDIMM_FAMILY_MSFT; >>> + msft->gen.nd_command = NDN_MSFT_CMD_SMART; >>> + msft->gen.nd_fw_size = 0; >>> + msft->gen.nd_size_in = offsetof(struct ndn_msft_smart, status); >>> + msft->gen.nd_size_out = sizeof(msft->u.smart); >>> + msft->u.smart.status = 0; >>> + >>> + cmd->firmware_status = &msft->u.smart.status; >>> + >>> + return cmd; >>> +} >>> + >>> +static int msft_smart_valid(struct ndctl_cmd *cmd) >>> +{ >>> + if (cmd->type != ND_CMD_CALL || >>> + cmd->size != sizeof(*cmd) + sizeof(struct ndn_pkg_msft) || >>> + CMD_MSFT(cmd)->gen.nd_family != NVDIMM_FAMILY_MSFT || >>> + CMD_MSFT(cmd)->gen.nd_command != >> NDN_MSFT_CMD_SMART || >>> + cmd->status != 0) >>> + return cmd->status < 0 ? cmd->status : -EINVAL; >>> + return 0; >>> +} >>> + >>> +static unsigned int msft_cmd_smart_get_flags(struct ndctl_cmd *cmd) >>> +{ >>> + if (msft_smart_valid(cmd) < 0) >>> + return UINT_MAX; >>> + >>> + /* below health data can be retrieved via MSFT _DSM function 11 */ >>> + return NDN_MSFT_SMART_HEALTH_VALID | >>> + NDN_MSFT_SMART_TEMP_VALID | >>> + NDN_MSFT_SMART_USED_VALID; >>> +} >>> + >>> +static unsigned int num_set_bit_health(__u16 num) >>> +{ >>> + int i; >>> + __u16 n = num & 0x7FFF; >>> + unsigned int count = 0; >>> + >>> + for (i = 0; i < 15; i++) >>> + if (!!(n & (1 << i))) >>> + count++; >>> + >>> + return count; >>> +} >>> + >>> +static unsigned int msft_cmd_smart_get_health(struct ndctl_cmd *cmd) >>> +{ >>> + unsigned int health; >>> + unsigned int num; >>> + >>> + if (msft_smart_valid(cmd) < 0) >>> + return UINT_MAX; >>> + >>> + num = num_set_bit_health(CMD_MSFT_SMART(cmd)->health); >>> + if (num == 0) >>> + health = 0; >>> + else if (num < 2) >>> + health = ND_SMART_NON_CRITICAL_HEALTH; >>> + else if (num < 3) >>> + health = ND_SMART_CRITICAL_HEALTH; >>> + else >>> + health = ND_SMART_FATAL_HEALTH; >>> + >>> + return health; >>> +} >>> + >>> +static unsigned int msft_cmd_smart_get_temperature(struct ndctl_cmd >> *cmd) >>> +{ >>> + if (msft_smart_valid(cmd) < 0) >>> + return UINT_MAX; >>> + /* >>> + * refer to JESD245 spec section 7.8 to calculate the temperature >>> + * then convert to 1/16 Celsius granularity. >>> + */ >> >> I'm confused by this comment because the Microsoft DSM spec says this >> value >> is in degrees C. Regardless of what the JEDAC spec says, wouldn't the >> platform FW convert to degrees C? And then this function needs to convert >> to 1/16th of a degree? >> >>> + return CMD_MSFT_SMART(cmd)->temp & 0x0FFC; >> >>> +} >>> + >>> +static unsigned int msft_cmd_smart_get_life_used(struct ndctl_cmd >> *cmd) >>> +{ >>> + if (msft_smart_valid(cmd) < 0) >>> + return UINT_MAX; >>> + >>> + return 100 - CMD_MSFT_SMART(cmd)->nvm_lifetime; >>> +} >>> + >>> +struct ndctl_smart_ops * const msft_smart_ops = &(struct >> ndctl_smart_ops) { >>> + .new_smart = msft_dimm_cmd_new_smart, >>> + .smart_get_flags = msft_cmd_smart_get_flags, >>> + .smart_get_health = msft_cmd_smart_get_health, >>> + .smart_get_temperature = msft_cmd_smart_get_temperature, >>> + .smart_get_life_used = msft_cmd_smart_get_life_used, >>> +}; >>> diff --git a/ndctl/lib/libndctl-private.h b/ndctl/lib/libndctl-private.h >>> index 3e67db0..8f10fbc 100644 >>> --- a/ndctl/lib/libndctl-private.h >>> +++ b/ndctl/lib/libndctl-private.h >>> @@ -32,6 +32,7 @@ >>> #include <ccan/endian/endian.h> >>> #include <ccan/short_types/short_types.h> >>> #include "ndctl-hpe1.h" >>> +#include "ndctl-msft.h" >>> >>> #define SZ_16M 0x01000000 >>> >>> @@ -196,6 +197,7 @@ struct ndctl_cmd { >>> struct nd_cmd_clear_error clear_err[0]; >>> #endif >>> struct ndn_pkg_hpe1 hpe1[0]; >>> + struct ndn_pkg_msft msft[0]; >>> struct nd_cmd_smart smart[0]; >>> struct nd_cmd_smart_threshold smart_t[0]; >>> struct nd_cmd_get_config_size get_size[0]; >>> @@ -226,9 +228,11 @@ struct ndctl_smart_ops { >>> #if HAS_SMART == 1 >>> struct ndctl_smart_ops * const intel_smart_ops; >>> struct ndctl_smart_ops * const hpe1_smart_ops; >>> +struct ndctl_smart_ops * const msft_smart_ops; >>> #else >>> static struct ndctl_smart_ops * const intel_smart_ops = NULL; >>> static struct ndctl_smart_ops * const hpe1_smart_ops = NULL; >>> +static struct ndctl_smart_ops * const msft_smart_ops = NULL; >>> #endif >>> >>> /* internal library helpers for conditionally defined command numbers */ >>> diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c >>> index 565c969..e5e027a 100644 >>> --- a/ndctl/lib/libndctl.c >>> +++ b/ndctl/lib/libndctl.c >>> @@ -1254,6 +1254,8 @@ static void *add_dimm(void *parent, int id, const >> char *dimm_base) >>> dimm->dsm_family = strtoul(buf, NULL, 0); >>> if (dimm->dsm_family == NVDIMM_FAMILY_HPE1) >>> dimm->smart_ops = hpe1_smart_ops; >>> + if (dimm->dsm_family == NVDIMM_FAMILY_MSFT) >>> + dimm->smart_ops = msft_smart_ops; >>> >>> dimm->formats = formats; >>> sprintf(path, "%s/nfit/format", dimm_base); >>> diff --git a/ndctl/lib/ndctl-msft.h b/ndctl/lib/ndctl-msft.h >>> new file mode 100644 >>> index 0000000..0a1c7c6 >>> --- /dev/null >>> +++ b/ndctl/lib/ndctl-msft.h >>> @@ -0,0 +1,63 @@ >>> +/* >>> + * Copyright (C) 2016-2017 Dell, Inc. >>> + * Copyright (C) 2016 Hewlett Packard Enterprise Development LP >>> + * Copyright (c) 2014-2015, Intel Corporation. >>> + * >>> + * This program is free software; you can redistribute it and/or modify it >>> + * under the terms and conditions of the GNU Lesser General Public >> License, >>> + * version 2.1, as published by the Free Software Foundation. >>> + * >>> + * This program is distributed in the hope it will be useful, but WITHOUT >> ANY >>> + * WARRANTY; without even the implied warranty of MERCHANTABILITY >> or FITNESS >>> + * FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public >> License for >>> + * more details. >>> + */ >>> +#ifndef __NDCTL_MSFT_H__ >>> +#define __NDCTL_MSFT_H__ >>> + >>> +enum { >>> + NDN_MSFT_CMD_QUERY = 0, >>> + >>> + /* non-root commands */ >>> + NDN_MSFT_CMD_SMART = 11, >>> +}; >>> + >>> +/* NDN_MSFT_CMD_SMART */ >>> +#define NDN_MSFT_SMART_HEALTH_VALID >> ND_SMART_HEALTH_VALID >>> +#define NDN_MSFT_SMART_TEMP_VALID ND_SMART_TEMP_VALID >>> +#define NDN_MSFT_SMART_USED_VALID ND_SMART_USED_VALID >>> + >>> +/* >>> + * This is actually function 11 data, >>> + * This is the closest I can find to match smart >>> + * Microsoft _DSM does not have smart function >>> + */ >>> +struct ndn_msft_smart_data { >>> + __u16 health; >>> + __u16 temp; >>> + __u8 err_thresh_stat; >>> + __u8 warn_thresh_stat; >>> + __u8 nvm_lifetime; >>> + __u8 count_dram_uncorr_err; >>> + __u8 count_dram_corr_err; >>> +} __attribute__((packed)); >>> + >>> +struct ndn_msft_smart { >>> + __u32 status; >>> + union { >>> + __u8 buf[9]; >>> + struct ndn_msft_smart_data data[0]; >>> + }; >>> +} __attribute__((packed)); >>> + >>> +union ndn_msft_cmd { >>> + __u32 query; >>> + struct ndn_msft_smart smart; >>> +} __attribute__((packed)); >>> + >>> +struct ndn_pkg_msft { >>> + struct nd_cmd_pkg gen; >>> + union ndn_msft_cmd u; >>> +} __attribute__((packed)); >>> + >>> +#endif /* __NDCTL_MSFT_H__ */ >>> >> > _______________________________________________ Linux-nvdimm mailing list [email protected] https://lists.01.org/mailman/listinfo/linux-nvdimm
