diff mbox series

[v5,09/26] nvme: add temperature threshold feature

Message ID 20200204095208.269131-10-k.jensen@samsung.com (mailing list archive)
State New, archived
Headers show
Series nvme: support NVMe v1.3d, SGLs and multiple namespaces | expand

Commit Message

Klaus Jensen Feb. 4, 2020, 9:51 a.m. UTC 
It might seem wierd to implement this feature for an emulated device,
but it is mandatory to support and the feature is useful for testing
asynchronous event request support, which will be added in a later
patch.

Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
---
 hw/block/nvme.c      | 50 ++++++++++++++++++++++++++++++++++++++++++++
 hw/block/nvme.h      |  2 ++
 include/block/nvme.h |  7 ++++++-
 3 files changed, 58 insertions(+), 1 deletion(-)

Comments

Maxim Levitsky Feb. 12, 2020, 9:31 a.m. UTC | #1 
On Tue, 2020-02-04 at 10:51 +0100, Klaus Jensen wrote:
> It might seem wierd to implement this feature for an emulated device,
> but it is mandatory to support and the feature is useful for testing
> asynchronous event request support, which will be added in a later
> patch.

Absolutely but as the old saying is, rules are rules.
At least, to the defense of the spec, making this mandatory
forced the vendors to actually report some statistics about
the device in neutral format as opposed to yet another
vendor proprietary thing (I am talking about SMART log page).

> 
> Signed-off-by: Klaus Jensen <k.jensen@samsung.com>

I noticed that you sign off some patches with your @samsung.com email,
and some with @cnexlabs.com
Is there a reason for that?


> ---
>  hw/block/nvme.c      | 50 ++++++++++++++++++++++++++++++++++++++++++++
>  hw/block/nvme.h      |  2 ++
>  include/block/nvme.h |  7 ++++++-
>  3 files changed, 58 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index 81514eaef63a..f72348344832 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -45,6 +45,9 @@
>  
>  #define NVME_SPEC_VER 0x00010201
>  #define NVME_MAX_QS PCI_MSIX_FLAGS_QSIZE
> +#define NVME_TEMPERATURE 0x143
> +#define NVME_TEMPERATURE_WARNING 0x157
> +#define NVME_TEMPERATURE_CRITICAL 0x175
>  
>  #define NVME_GUEST_ERR(trace, fmt, ...) \
>      do { \
> @@ -798,9 +801,31 @@ static uint16_t nvme_get_feature_timestamp(NvmeCtrl *n, NvmeCmd *cmd)
>  static uint16_t nvme_get_feature(NvmeCtrl *n, NvmeCmd *cmd, NvmeRequest *req)
>  {
>      uint32_t dw10 = le32_to_cpu(cmd->cdw10);
> +    uint32_t dw11 = le32_to_cpu(cmd->cdw11);
>      uint32_t result;
>  
>      switch (dw10) {
> +    case NVME_TEMPERATURE_THRESHOLD:
> +        result = 0;
> +
> +        /*
> +         * The controller only implements the Composite Temperature sensor, so
> +         * return 0 for all other sensors.
> +         */
> +        if (NVME_TEMP_TMPSEL(dw11)) {
> +            break;
> +        }
> +
> +        switch (NVME_TEMP_THSEL(dw11)) {
> +        case 0x0:
> +            result = cpu_to_le16(n->features.temp_thresh_hi);
> +            break;
> +        case 0x1:
> +            result = cpu_to_le16(n->features.temp_thresh_low);
> +            break;
> +        }
> +
> +        break;
>      case NVME_VOLATILE_WRITE_CACHE:
>          result = blk_enable_write_cache(n->conf.blk);
>          trace_nvme_dev_getfeat_vwcache(result ? "enabled" : "disabled");
> @@ -845,6 +870,23 @@ static uint16_t nvme_set_feature(NvmeCtrl *n, NvmeCmd *cmd, NvmeRequest *req)
>      uint32_t dw11 = le32_to_cpu(cmd->cdw11);
>  
>      switch (dw10) {
> +    case NVME_TEMPERATURE_THRESHOLD:
> +        if (NVME_TEMP_TMPSEL(dw11)) {
> +            break;
> +        }
> +
> +        switch (NVME_TEMP_THSEL(dw11)) {
> +        case 0x0:
> +            n->features.temp_thresh_hi = NVME_TEMP_TMPTH(dw11);
> +            break;
> +        case 0x1:
> +            n->features.temp_thresh_low = NVME_TEMP_TMPTH(dw11);
> +            break;
> +        default:
> +            return NVME_INVALID_FIELD | NVME_DNR;
> +        }
> +
> +        break;
>      case NVME_VOLATILE_WRITE_CACHE:
>          blk_set_enable_write_cache(n->conf.blk, dw11 & 1);
>          break;
> @@ -1366,6 +1408,9 @@ static void nvme_init_state(NvmeCtrl *n)
>      n->namespaces = g_new0(NvmeNamespace, n->num_namespaces);
>      n->sq = g_new0(NvmeSQueue *, n->params.num_queues);
>      n->cq = g_new0(NvmeCQueue *, n->params.num_queues);
> +
> +    n->temperature = NVME_TEMPERATURE;

This appears not to be used in the patch.
I think you should move that to the next patch that
adds the get log page support.

> +    n->features.temp_thresh_hi = NVME_TEMPERATURE_WARNING;
>  }
>  
>  static void nvme_init_cmb(NvmeCtrl *n, PCIDevice *pci_dev)
> @@ -1447,6 +1492,11 @@ static void nvme_init_ctrl(NvmeCtrl *n)
>      id->acl = 3;
>      id->frmw = 7 << 1;
>      id->lpa = 1 << 0;
> +
> +    /* recommended default value (~70 C) */
> +    id->wctemp = cpu_to_le16(NVME_TEMPERATURE_WARNING);
> +    id->cctemp = cpu_to_le16(NVME_TEMPERATURE_CRITICAL);
> +
>      id->sqes = (0x6 << 4) | 0x6;
>      id->cqes = (0x4 << 4) | 0x4;
>      id->nn = cpu_to_le32(n->num_namespaces);
> diff --git a/hw/block/nvme.h b/hw/block/nvme.h
> index a867bdfabafd..1518f32557a3 100644
> --- a/hw/block/nvme.h
> +++ b/hw/block/nvme.h
> @@ -108,6 +108,7 @@ typedef struct NvmeCtrl {
>      uint64_t    irq_status;
>      uint64_t    host_timestamp;                 /* Timestamp sent by the host */
>      uint64_t    timestamp_set_qemu_clock_ms;    /* QEMU clock time */
> +    uint16_t    temperature;
>  
>      NvmeNamespace   *namespaces;
>      NvmeSQueue      **sq;
> @@ -115,6 +116,7 @@ typedef struct NvmeCtrl {
>      NvmeSQueue      admin_sq;
>      NvmeCQueue      admin_cq;
>      NvmeIdCtrl      id_ctrl;
> +    NvmeFeatureVal  features;
>  } NvmeCtrl;
>  
>  static inline uint64_t nvme_ns_nlbas(NvmeCtrl *n, NvmeNamespace *ns)
> diff --git a/include/block/nvme.h b/include/block/nvme.h
> index d2f65e8fe496..ff31cb32117c 100644
> --- a/include/block/nvme.h
> +++ b/include/block/nvme.h
> @@ -616,7 +616,8 @@ enum NvmeIdCtrlOncs {
>  typedef struct NvmeFeatureVal {
>      uint32_t    arbitration;
>      uint32_t    power_mgmt;
> -    uint32_t    temp_thresh;
> +    uint16_t    temp_thresh_hi;
> +    uint16_t    temp_thresh_low;
>      uint32_t    err_rec;
>      uint32_t    volatile_wc;
>      uint32_t    num_queues;
> @@ -635,6 +636,10 @@ typedef struct NvmeFeatureVal {
>  #define NVME_INTC_THR(intc)     (intc & 0xff)
>  #define NVME_INTC_TIME(intc)    ((intc >> 8) & 0xff)
>  
> +#define NVME_TEMP_THSEL(temp)  ((temp >> 20) & 0x3)
> +#define NVME_TEMP_TMPSEL(temp) ((temp >> 16) & 0xf)
> +#define NVME_TEMP_TMPTH(temp)  (temp & 0xffff)
> +
>  enum NvmeFeatureIds {
>      NVME_ARBITRATION                = 0x1,
>      NVME_POWER_MANAGEMENT           = 0x2,


Best regards,
	Maxim Levitsky
Klaus Jensen March 16, 2020, 7:44 a.m. UTC | #2 
On Feb 12 11:31, Maxim Levitsky wrote:
> On Tue, 2020-02-04 at 10:51 +0100, Klaus Jensen wrote:
> > It might seem wierd to implement this feature for an emulated device,
> > but it is mandatory to support and the feature is useful for testing
> > asynchronous event request support, which will be added in a later
> > patch.
> 
> Absolutely but as the old saying is, rules are rules.
> At least, to the defense of the spec, making this mandatory
> forced the vendors to actually report some statistics about
> the device in neutral format as opposed to yet another
> vendor proprietary thing (I am talking about SMART log page).
> 
> > 
> > Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
> 
> I noticed that you sign off some patches with your @samsung.com email,
> and some with @cnexlabs.com
> Is there a reason for that?

Yeah. Some of this code was made while I was at CNEX Labs. I've since
moved to Samsung. But credit where credit's due.

> 
> 
> > ---
> >  hw/block/nvme.c      | 50 ++++++++++++++++++++++++++++++++++++++++++++
> >  hw/block/nvme.h      |  2 ++
> >  include/block/nvme.h |  7 ++++++-
> >  3 files changed, 58 insertions(+), 1 deletion(-)
> > 
> > diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> > index 81514eaef63a..f72348344832 100644
> > --- a/hw/block/nvme.c
> > +++ b/hw/block/nvme.c
> > @@ -45,6 +45,9 @@
> >  
> >  #define NVME_SPEC_VER 0x00010201
> >  #define NVME_MAX_QS PCI_MSIX_FLAGS_QSIZE
> > +#define NVME_TEMPERATURE 0x143
> > +#define NVME_TEMPERATURE_WARNING 0x157
> > +#define NVME_TEMPERATURE_CRITICAL 0x175
> >  
> >  #define NVME_GUEST_ERR(trace, fmt, ...) \
> >      do { \
> > @@ -798,9 +801,31 @@ static uint16_t nvme_get_feature_timestamp(NvmeCtrl *n, NvmeCmd *cmd)
> >  static uint16_t nvme_get_feature(NvmeCtrl *n, NvmeCmd *cmd, NvmeRequest *req)
> >  {
> >      uint32_t dw10 = le32_to_cpu(cmd->cdw10);
> > +    uint32_t dw11 = le32_to_cpu(cmd->cdw11);
> >      uint32_t result;
> >  
> >      switch (dw10) {
> > +    case NVME_TEMPERATURE_THRESHOLD:
> > +        result = 0;
> > +
> > +        /*
> > +         * The controller only implements the Composite Temperature sensor, so
> > +         * return 0 for all other sensors.
> > +         */
> > +        if (NVME_TEMP_TMPSEL(dw11)) {
> > +            break;
> > +        }
> > +
> > +        switch (NVME_TEMP_THSEL(dw11)) {
> > +        case 0x0:
> > +            result = cpu_to_le16(n->features.temp_thresh_hi);
> > +            break;
> > +        case 0x1:
> > +            result = cpu_to_le16(n->features.temp_thresh_low);
> > +            break;
> > +        }
> > +
> > +        break;
> >      case NVME_VOLATILE_WRITE_CACHE:
> >          result = blk_enable_write_cache(n->conf.blk);
> >          trace_nvme_dev_getfeat_vwcache(result ? "enabled" : "disabled");
> > @@ -845,6 +870,23 @@ static uint16_t nvme_set_feature(NvmeCtrl *n, NvmeCmd *cmd, NvmeRequest *req)
> >      uint32_t dw11 = le32_to_cpu(cmd->cdw11);
> >  
> >      switch (dw10) {
> > +    case NVME_TEMPERATURE_THRESHOLD:
> > +        if (NVME_TEMP_TMPSEL(dw11)) {
> > +            break;
> > +        }
> > +
> > +        switch (NVME_TEMP_THSEL(dw11)) {
> > +        case 0x0:
> > +            n->features.temp_thresh_hi = NVME_TEMP_TMPTH(dw11);
> > +            break;
> > +        case 0x1:
> > +            n->features.temp_thresh_low = NVME_TEMP_TMPTH(dw11);
> > +            break;
> > +        default:
> > +            return NVME_INVALID_FIELD | NVME_DNR;
> > +        }
> > +
> > +        break;
> >      case NVME_VOLATILE_WRITE_CACHE:
> >          blk_set_enable_write_cache(n->conf.blk, dw11 & 1);
> >          break;
> > @@ -1366,6 +1408,9 @@ static void nvme_init_state(NvmeCtrl *n)
> >      n->namespaces = g_new0(NvmeNamespace, n->num_namespaces);
> >      n->sq = g_new0(NvmeSQueue *, n->params.num_queues);
> >      n->cq = g_new0(NvmeCQueue *, n->params.num_queues);
> > +
> > +    n->temperature = NVME_TEMPERATURE;
> 
> This appears not to be used in the patch.
> I think you should move that to the next patch that
> adds the get log page support.
> 

Fixed.

> > +    n->features.temp_thresh_hi = NVME_TEMPERATURE_WARNING;
> >  }
> >  
> >  static void nvme_init_cmb(NvmeCtrl *n, PCIDevice *pci_dev)
> > @@ -1447,6 +1492,11 @@ static void nvme_init_ctrl(NvmeCtrl *n)
> >      id->acl = 3;
> >      id->frmw = 7 << 1;
> >      id->lpa = 1 << 0;
> > +
> > +    /* recommended default value (~70 C) */
> > +    id->wctemp = cpu_to_le16(NVME_TEMPERATURE_WARNING);
> > +    id->cctemp = cpu_to_le16(NVME_TEMPERATURE_CRITICAL);
> > +
> >      id->sqes = (0x6 << 4) | 0x6;
> >      id->cqes = (0x4 << 4) | 0x4;
> >      id->nn = cpu_to_le32(n->num_namespaces);
> > diff --git a/hw/block/nvme.h b/hw/block/nvme.h
> > index a867bdfabafd..1518f32557a3 100644
> > --- a/hw/block/nvme.h
> > +++ b/hw/block/nvme.h
> > @@ -108,6 +108,7 @@ typedef struct NvmeCtrl {
> >      uint64_t    irq_status;
> >      uint64_t    host_timestamp;                 /* Timestamp sent by the host */
> >      uint64_t    timestamp_set_qemu_clock_ms;    /* QEMU clock time */
> > +    uint16_t    temperature;
> >  
> >      NvmeNamespace   *namespaces;
> >      NvmeSQueue      **sq;
> > @@ -115,6 +116,7 @@ typedef struct NvmeCtrl {
> >      NvmeSQueue      admin_sq;
> >      NvmeCQueue      admin_cq;
> >      NvmeIdCtrl      id_ctrl;
> > +    NvmeFeatureVal  features;
> >  } NvmeCtrl;
> >  
> >  static inline uint64_t nvme_ns_nlbas(NvmeCtrl *n, NvmeNamespace *ns)
> > diff --git a/include/block/nvme.h b/include/block/nvme.h
> > index d2f65e8fe496..ff31cb32117c 100644
> > --- a/include/block/nvme.h
> > +++ b/include/block/nvme.h
> > @@ -616,7 +616,8 @@ enum NvmeIdCtrlOncs {
> >  typedef struct NvmeFeatureVal {
> >      uint32_t    arbitration;
> >      uint32_t    power_mgmt;
> > -    uint32_t    temp_thresh;
> > +    uint16_t    temp_thresh_hi;
> > +    uint16_t    temp_thresh_low;
> >      uint32_t    err_rec;
> >      uint32_t    volatile_wc;
> >      uint32_t    num_queues;
> > @@ -635,6 +636,10 @@ typedef struct NvmeFeatureVal {
> >  #define NVME_INTC_THR(intc)     (intc & 0xff)
> >  #define NVME_INTC_TIME(intc)    ((intc >> 8) & 0xff)
> >  
> > +#define NVME_TEMP_THSEL(temp)  ((temp >> 20) & 0x3)
> > +#define NVME_TEMP_TMPSEL(temp) ((temp >> 16) & 0xf)
> > +#define NVME_TEMP_TMPTH(temp)  (temp & 0xffff)
> > +
> >  enum NvmeFeatureIds {
> >      NVME_ARBITRATION                = 0x1,
> >      NVME_POWER_MANAGEMENT           = 0x2,
> 
> 
> Best regards,
> 	Maxim Levitsky
>
Maxim Levitsky March 25, 2020, 10:21 a.m. UTC | #3 
On Mon, 2020-03-16 at 00:44 -0700, Klaus Birkelund Jensen wrote:
> On Feb 12 11:31, Maxim Levitsky wrote:
> > On Tue, 2020-02-04 at 10:51 +0100, Klaus Jensen wrote:
> > > It might seem wierd to implement this feature for an emulated device,
> > > but it is mandatory to support and the feature is useful for testing
> > > asynchronous event request support, which will be added in a later
> > > patch.
> > 
> > Absolutely but as the old saying is, rules are rules.
> > At least, to the defense of the spec, making this mandatory
> > forced the vendors to actually report some statistics about
> > the device in neutral format as opposed to yet another
> > vendor proprietary thing (I am talking about SMART log page).
> > 
> > > 
> > > Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
> > 
> > I noticed that you sign off some patches with your @samsung.com email,
> > and some with @cnexlabs.com
> > Is there a reason for that?
> 
> Yeah. Some of this code was made while I was at CNEX Labs. I've since
> moved to Samsung. But credit where credit's due.
I suspected something like that, but I just wanted to be sure that this is intentional,
and it looks all right to me now.

> 
> > 
> > 
> > > ---
> > >  hw/block/nvme.c      | 50 ++++++++++++++++++++++++++++++++++++++++++++
> > >  hw/block/nvme.h      |  2 ++
> > >  include/block/nvme.h |  7 ++++++-
> > >  3 files changed, 58 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> > > index 81514eaef63a..f72348344832 100644
> > > --- a/hw/block/nvme.c
> > > +++ b/hw/block/nvme.c
> > > @@ -45,6 +45,9 @@
> > >  
> > >  #define NVME_SPEC_VER 0x00010201
> > >  #define NVME_MAX_QS PCI_MSIX_FLAGS_QSIZE
> > > +#define NVME_TEMPERATURE 0x143
> > > +#define NVME_TEMPERATURE_WARNING 0x157
> > > +#define NVME_TEMPERATURE_CRITICAL 0x175
> > >  
> > >  #define NVME_GUEST_ERR(trace, fmt, ...) \
> > >      do { \
> > > @@ -798,9 +801,31 @@ static uint16_t nvme_get_feature_timestamp(NvmeCtrl *n, NvmeCmd *cmd)
> > >  static uint16_t nvme_get_feature(NvmeCtrl *n, NvmeCmd *cmd, NvmeRequest *req)
> > >  {
> > >      uint32_t dw10 = le32_to_cpu(cmd->cdw10);
> > > +    uint32_t dw11 = le32_to_cpu(cmd->cdw11);
> > >      uint32_t result;
> > >  
> > >      switch (dw10) {
> > > +    case NVME_TEMPERATURE_THRESHOLD:
> > > +        result = 0;
> > > +
> > > +        /*
> > > +         * The controller only implements the Composite Temperature sensor, so
> > > +         * return 0 for all other sensors.
> > > +         */
> > > +        if (NVME_TEMP_TMPSEL(dw11)) {
> > > +            break;
> > > +        }
> > > +
> > > +        switch (NVME_TEMP_THSEL(dw11)) {
> > > +        case 0x0:
> > > +            result = cpu_to_le16(n->features.temp_thresh_hi);
> > > +            break;
> > > +        case 0x1:
> > > +            result = cpu_to_le16(n->features.temp_thresh_low);
> > > +            break;
> > > +        }
> > > +
> > > +        break;
> > >      case NVME_VOLATILE_WRITE_CACHE:
> > >          result = blk_enable_write_cache(n->conf.blk);
> > >          trace_nvme_dev_getfeat_vwcache(result ? "enabled" : "disabled");
> > > @@ -845,6 +870,23 @@ static uint16_t nvme_set_feature(NvmeCtrl *n, NvmeCmd *cmd, NvmeRequest *req)
> > >      uint32_t dw11 = le32_to_cpu(cmd->cdw11);
> > >  
> > >      switch (dw10) {
> > > +    case NVME_TEMPERATURE_THRESHOLD:
> > > +        if (NVME_TEMP_TMPSEL(dw11)) {
> > > +            break;
> > > +        }
> > > +
> > > +        switch (NVME_TEMP_THSEL(dw11)) {
> > > +        case 0x0:
> > > +            n->features.temp_thresh_hi = NVME_TEMP_TMPTH(dw11);
> > > +            break;
> > > +        case 0x1:
> > > +            n->features.temp_thresh_low = NVME_TEMP_TMPTH(dw11);
> > > +            break;
> > > +        default:
> > > +            return NVME_INVALID_FIELD | NVME_DNR;
> > > +        }
> > > +
> > > +        break;
> > >      case NVME_VOLATILE_WRITE_CACHE:
> > >          blk_set_enable_write_cache(n->conf.blk, dw11 & 1);
> > >          break;
> > > @@ -1366,6 +1408,9 @@ static void nvme_init_state(NvmeCtrl *n)
> > >      n->namespaces = g_new0(NvmeNamespace, n->num_namespaces);
> > >      n->sq = g_new0(NvmeSQueue *, n->params.num_queues);
> > >      n->cq = g_new0(NvmeCQueue *, n->params.num_queues);
> > > +
> > > +    n->temperature = NVME_TEMPERATURE;
> > 
> > This appears not to be used in the patch.
> > I think you should move that to the next patch that
> > adds the get log page support.
> > 
> 
> Fixed.
Thanks
> 
> > > +    n->features.temp_thresh_hi = NVME_TEMPERATURE_WARNING;
> > >  }
> > >  
> > >  static void nvme_init_cmb(NvmeCtrl *n, PCIDevice *pci_dev)
> > > @@ -1447,6 +1492,11 @@ static void nvme_init_ctrl(NvmeCtrl *n)
> > >      id->acl = 3;
> > >      id->frmw = 7 << 1;
> > >      id->lpa = 1 << 0;
> > > +
> > > +    /* recommended default value (~70 C) */
> > > +    id->wctemp = cpu_to_le16(NVME_TEMPERATURE_WARNING);
> > > +    id->cctemp = cpu_to_le16(NVME_TEMPERATURE_CRITICAL);
> > > +
> > >      id->sqes = (0x6 << 4) | 0x6;
> > >      id->cqes = (0x4 << 4) | 0x4;
> > >      id->nn = cpu_to_le32(n->num_namespaces);
> > > diff --git a/hw/block/nvme.h b/hw/block/nvme.h
> > > index a867bdfabafd..1518f32557a3 100644
> > > --- a/hw/block/nvme.h
> > > +++ b/hw/block/nvme.h
> > > @@ -108,6 +108,7 @@ typedef struct NvmeCtrl {
> > >      uint64_t    irq_status;
> > >      uint64_t    host_timestamp;                 /* Timestamp sent by the host */
> > >      uint64_t    timestamp_set_qemu_clock_ms;    /* QEMU clock time */
> > > +    uint16_t    temperature;
> > >  
> > >      NvmeNamespace   *namespaces;
> > >      NvmeSQueue      **sq;
> > > @@ -115,6 +116,7 @@ typedef struct NvmeCtrl {
> > >      NvmeSQueue      admin_sq;
> > >      NvmeCQueue      admin_cq;
> > >      NvmeIdCtrl      id_ctrl;
> > > +    NvmeFeatureVal  features;
> > >  } NvmeCtrl;
> > >  
> > >  static inline uint64_t nvme_ns_nlbas(NvmeCtrl *n, NvmeNamespace *ns)
> > > diff --git a/include/block/nvme.h b/include/block/nvme.h
> > > index d2f65e8fe496..ff31cb32117c 100644
> > > --- a/include/block/nvme.h
> > > +++ b/include/block/nvme.h
> > > @@ -616,7 +616,8 @@ enum NvmeIdCtrlOncs {
> > >  typedef struct NvmeFeatureVal {
> > >      uint32_t    arbitration;
> > >      uint32_t    power_mgmt;
> > > -    uint32_t    temp_thresh;
> > > +    uint16_t    temp_thresh_hi;
> > > +    uint16_t    temp_thresh_low;
> > >      uint32_t    err_rec;
> > >      uint32_t    volatile_wc;
> > >      uint32_t    num_queues;
> > > @@ -635,6 +636,10 @@ typedef struct NvmeFeatureVal {
> > >  #define NVME_INTC_THR(intc)     (intc & 0xff)
> > >  #define NVME_INTC_TIME(intc)    ((intc >> 8) & 0xff)
> > >  
> > > +#define NVME_TEMP_THSEL(temp)  ((temp >> 20) & 0x3)
> > > +#define NVME_TEMP_TMPSEL(temp) ((temp >> 16) & 0xf)
> > > +#define NVME_TEMP_TMPTH(temp)  (temp & 0xffff)
> > > +
> > >  enum NvmeFeatureIds {
> > >      NVME_ARBITRATION                = 0x1,
> > >      NVME_POWER_MANAGEMENT           = 0x2,
> > 
> > 
> > Best regards,
> > 	Maxim Levitsky
> > 
> 
> 

Best regards,
	Maxim Levitsky
diff mbox series

Patch

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 81514eaef63a..f72348344832 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -45,6 +45,9 @@ 
 
 #define NVME_SPEC_VER 0x00010201
 #define NVME_MAX_QS PCI_MSIX_FLAGS_QSIZE
+#define NVME_TEMPERATURE 0x143
+#define NVME_TEMPERATURE_WARNING 0x157
+#define NVME_TEMPERATURE_CRITICAL 0x175
 
 #define NVME_GUEST_ERR(trace, fmt, ...) \
     do { \
@@ -798,9 +801,31 @@  static uint16_t nvme_get_feature_timestamp(NvmeCtrl *n, NvmeCmd *cmd)
 static uint16_t nvme_get_feature(NvmeCtrl *n, NvmeCmd *cmd, NvmeRequest *req)
 {
     uint32_t dw10 = le32_to_cpu(cmd->cdw10);
+    uint32_t dw11 = le32_to_cpu(cmd->cdw11);
     uint32_t result;
 
     switch (dw10) {
+    case NVME_TEMPERATURE_THRESHOLD:
+        result = 0;
+
+        /*
+         * The controller only implements the Composite Temperature sensor, so
+         * return 0 for all other sensors.
+         */
+        if (NVME_TEMP_TMPSEL(dw11)) {
+            break;
+        }
+
+        switch (NVME_TEMP_THSEL(dw11)) {
+        case 0x0:
+            result = cpu_to_le16(n->features.temp_thresh_hi);
+            break;
+        case 0x1:
+            result = cpu_to_le16(n->features.temp_thresh_low);
+            break;
+        }
+
+        break;
     case NVME_VOLATILE_WRITE_CACHE:
         result = blk_enable_write_cache(n->conf.blk);
         trace_nvme_dev_getfeat_vwcache(result ? "enabled" : "disabled");
@@ -845,6 +870,23 @@  static uint16_t nvme_set_feature(NvmeCtrl *n, NvmeCmd *cmd, NvmeRequest *req)
     uint32_t dw11 = le32_to_cpu(cmd->cdw11);
 
     switch (dw10) {
+    case NVME_TEMPERATURE_THRESHOLD:
+        if (NVME_TEMP_TMPSEL(dw11)) {
+            break;
+        }
+
+        switch (NVME_TEMP_THSEL(dw11)) {
+        case 0x0:
+            n->features.temp_thresh_hi = NVME_TEMP_TMPTH(dw11);
+            break;
+        case 0x1:
+            n->features.temp_thresh_low = NVME_TEMP_TMPTH(dw11);
+            break;
+        default:
+            return NVME_INVALID_FIELD | NVME_DNR;
+        }
+
+        break;
     case NVME_VOLATILE_WRITE_CACHE:
         blk_set_enable_write_cache(n->conf.blk, dw11 & 1);
         break;
@@ -1366,6 +1408,9 @@  static void nvme_init_state(NvmeCtrl *n)
     n->namespaces = g_new0(NvmeNamespace, n->num_namespaces);
     n->sq = g_new0(NvmeSQueue *, n->params.num_queues);
     n->cq = g_new0(NvmeCQueue *, n->params.num_queues);
+
+    n->temperature = NVME_TEMPERATURE;
+    n->features.temp_thresh_hi = NVME_TEMPERATURE_WARNING;
 }
 
 static void nvme_init_cmb(NvmeCtrl *n, PCIDevice *pci_dev)
@@ -1447,6 +1492,11 @@  static void nvme_init_ctrl(NvmeCtrl *n)
     id->acl = 3;
     id->frmw = 7 << 1;
     id->lpa = 1 << 0;
+
+    /* recommended default value (~70 C) */
+    id->wctemp = cpu_to_le16(NVME_TEMPERATURE_WARNING);
+    id->cctemp = cpu_to_le16(NVME_TEMPERATURE_CRITICAL);
+
     id->sqes = (0x6 << 4) | 0x6;
     id->cqes = (0x4 << 4) | 0x4;
     id->nn = cpu_to_le32(n->num_namespaces);
diff --git a/hw/block/nvme.h b/hw/block/nvme.h
index a867bdfabafd..1518f32557a3 100644
--- a/hw/block/nvme.h
+++ b/hw/block/nvme.h
@@ -108,6 +108,7 @@  typedef struct NvmeCtrl {
     uint64_t    irq_status;
     uint64_t    host_timestamp;                 /* Timestamp sent by the host */
     uint64_t    timestamp_set_qemu_clock_ms;    /* QEMU clock time */
+    uint16_t    temperature;
 
     NvmeNamespace   *namespaces;
     NvmeSQueue      **sq;
@@ -115,6 +116,7 @@  typedef struct NvmeCtrl {
     NvmeSQueue      admin_sq;
     NvmeCQueue      admin_cq;
     NvmeIdCtrl      id_ctrl;
+    NvmeFeatureVal  features;
 } NvmeCtrl;
 
 static inline uint64_t nvme_ns_nlbas(NvmeCtrl *n, NvmeNamespace *ns)
diff --git a/include/block/nvme.h b/include/block/nvme.h
index d2f65e8fe496..ff31cb32117c 100644
--- a/include/block/nvme.h
+++ b/include/block/nvme.h
@@ -616,7 +616,8 @@  enum NvmeIdCtrlOncs {
 typedef struct NvmeFeatureVal {
     uint32_t    arbitration;
     uint32_t    power_mgmt;
-    uint32_t    temp_thresh;
+    uint16_t    temp_thresh_hi;
+    uint16_t    temp_thresh_low;
     uint32_t    err_rec;
     uint32_t    volatile_wc;
     uint32_t    num_queues;
@@ -635,6 +636,10 @@  typedef struct NvmeFeatureVal {
 #define NVME_INTC_THR(intc)     (intc & 0xff)
 #define NVME_INTC_TIME(intc)    ((intc >> 8) & 0xff)
 
+#define NVME_TEMP_THSEL(temp)  ((temp >> 20) & 0x3)
+#define NVME_TEMP_TMPSEL(temp) ((temp >> 16) & 0xf)
+#define NVME_TEMP_TMPTH(temp)  (temp & 0xffff)
+
 enum NvmeFeatureIds {
     NVME_ARBITRATION                = 0x1,
     NVME_POWER_MANAGEMENT           = 0x2,