subject:"\[PATCH v6 11\/42\] nvme\: add temperature threshold feature"

Re: [PATCH v6 11/42] nvme: add temperature threshold feature

2020-03-31 Thread Maxim Levitsky

On Tue, 2020-03-31 at 07:40 +0200, Klaus Birkelund Jensen wrote:
> On Mar 25 12:40, Maxim Levitsky wrote:
> > On Mon, 2020-03-16 at 07:28 -0700, Klaus Jensen wrote:
> > > From: Klaus Jensen 
> > > 
> > > 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 
> > > Acked-by: Keith Busch 
> > > ---
> > >  hw/block/nvme.c  | 48 
> > >  hw/block/nvme.h  |  2 ++
> > >  include/block/nvme.h |  8 +++-
> > >  3 files changed, 57 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/hw/block/nvme.h b/hw/block/nvme.h
> > > index b7c465560eea..8cda5f02c622 100644
> > > --- a/hw/block/nvme.h
> > > +++ b/hw/block/nvme.h
> > > @@ -108,6 +108,7 @@ typedef struct NvmeCtrl {
> > >  uint64_tirq_status;
> > >  uint64_thost_timestamp; /* Timestamp sent by the 
> > > host */
> > >  uint64_ttimestamp_set_qemu_clock_ms;/* QEMU clock time */
> > > +uint16_ttemperature;
> > 
> > You forgot to move this too.
> > 
> 
> Fixed!

Thanks.

> > 
> > With 'temperature' field removed from the header:
> > 
> > Reviewed-by: Maxim Levitsky 
> > 
> > Best regards,
> > Maxim Levitsky
> > 
> 
> 

Best regards,
Maxim Levitsky

Re: [PATCH v6 11/42] nvme: add temperature threshold feature

2020-03-30 Thread Klaus Birkelund Jensen

On Mar 25 12:40, Maxim Levitsky wrote:
> On Mon, 2020-03-16 at 07:28 -0700, Klaus Jensen wrote:
> > From: Klaus Jensen 
> > 
> > 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 
> > Acked-by: Keith Busch 
> > ---
> >  hw/block/nvme.c  | 48 
> >  hw/block/nvme.h  |  2 ++
> >  include/block/nvme.h |  8 +++-
> >  3 files changed, 57 insertions(+), 1 deletion(-)
> > 
> > diff --git a/hw/block/nvme.h b/hw/block/nvme.h
> > index b7c465560eea..8cda5f02c622 100644
> > --- a/hw/block/nvme.h
> > +++ b/hw/block/nvme.h
> > @@ -108,6 +108,7 @@ typedef struct NvmeCtrl {
> >  uint64_tirq_status;
> >  uint64_thost_timestamp; /* Timestamp sent by the 
> > host */
> >  uint64_ttimestamp_set_qemu_clock_ms;/* QEMU clock time */
> > +uint16_ttemperature;
> You forgot to move this too.
> 

Fixed!
> 
> With 'temperature' field removed from the header:
> 
> Reviewed-by: Maxim Levitsky 
> 
> Best regards,
>   Maxim Levitsky
>

Re: [PATCH v6 11/42] nvme: add temperature threshold feature

2020-03-25 Thread Maxim Levitsky

On Mon, 2020-03-16 at 07:28 -0700, Klaus Jensen wrote:
> From: Klaus Jensen 
> 
> 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 
> Acked-by: Keith Busch 
> ---
>  hw/block/nvme.c  | 48 
>  hw/block/nvme.h  |  2 ++
>  include/block/nvme.h |  8 +++-
>  3 files changed, 57 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index 665485045066..64c42101df5c 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -45,6 +45,9 @@
>  #include "nvme.h"
>  
>  #define NVME_CMB_BIR 2
> +#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;
> @@ -1374,6 +1416,7 @@ static void nvme_init_state(NvmeCtrl *n)
>  n->namespaces = g_new0(NvmeNamespace, n->num_namespaces);
>  n->sq = g_new0(NvmeSQueue *, n->params.max_ioqpairs + 1);
>  n->cq = g_new0(NvmeCQueue *, n->params.max_ioqpairs + 1);
> +n->features.temp_thresh_hi = NVME_TEMPERATURE_WARNING;
>  }
>  
>  static void nvme_init_cmb(NvmeCtrl *n, PCIDevice *pci_dev)
> @@ -1451,6 +1494,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 b7c465560eea..8cda5f02c622 100644
> --- a/hw/block/nvme.h
> +++ b/hw/block/nvme.h
> @@ -108,6 +108,7 @@ typedef struct NvmeCtrl {
>  uint64_tirq_status;
>  uint64_thost_timestamp; /* Timestamp sent by the 
> host */
>  uint64_ttimestamp_set_qemu_clock_ms;/* QEMU clock time */
> +uint16_ttemperature;
You forgot to move this too.

>  
>  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 a083c1b3a613..91fc4738a3e0 100644
> --- a/include/block/nvme.h
> +++ b/include/block/nvme.h
> @@ -688,7 +688,13 @@ enum NvmeIdCtrlOncs {
>  typedef struct NvmeFeatureVal {
>  uint32_tarbitration;
>  uint32_tpower_mgmt;
> -uint32_ttemp_thresh;
> +union {
> +struct {
> +uint16_t temp_thresh_hi;
> +uint16_t temp_thresh_low;
> +};
> +uint32_t temp_thresh;
> +};
>  uint32_terr_rec;
>

[PATCH v6 11/42] nvme: add temperature threshold feature

2020-03-16 Thread Klaus Jensen

From: Klaus Jensen 

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 
Acked-by: Keith Busch 
---
 hw/block/nvme.c  | 48 
 hw/block/nvme.h  |  2 ++
 include/block/nvme.h |  8 +++-
 3 files changed, 57 insertions(+), 1 deletion(-)

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 665485045066..64c42101df5c 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -45,6 +45,9 @@
 #include "nvme.h"
 
 #define NVME_CMB_BIR 2
+#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;
@@ -1374,6 +1416,7 @@ static void nvme_init_state(NvmeCtrl *n)
 n->namespaces = g_new0(NvmeNamespace, n->num_namespaces);
 n->sq = g_new0(NvmeSQueue *, n->params.max_ioqpairs + 1);
 n->cq = g_new0(NvmeCQueue *, n->params.max_ioqpairs + 1);
+n->features.temp_thresh_hi = NVME_TEMPERATURE_WARNING;
 }
 
 static void nvme_init_cmb(NvmeCtrl *n, PCIDevice *pci_dev)
@@ -1451,6 +1494,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 b7c465560eea..8cda5f02c622 100644
--- a/hw/block/nvme.h
+++ b/hw/block/nvme.h
@@ -108,6 +108,7 @@ typedef struct NvmeCtrl {
 uint64_tirq_status;
 uint64_thost_timestamp; /* Timestamp sent by the host 
*/
 uint64_ttimestamp_set_qemu_clock_ms;/* QEMU clock time */
+uint16_ttemperature;
 
 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 a083c1b3a613..91fc4738a3e0 100644
--- a/include/block/nvme.h
+++ b/include/block/nvme.h
@@ -688,7 +688,13 @@ enum NvmeIdCtrlOncs {
 typedef struct NvmeFeatureVal {
 uint32_tarbitration;
 uint32_tpower_mgmt;
-uint32_ttemp_thresh;
+union {
+struct {
+uint16_t temp_thresh_hi;
+uint16_t temp_thresh_low;
+};
+uint32_t temp_thresh;
+};
 uint32_terr_rec;
 uint32_tvolatile_wc;
 uint32_tnum_queues;
-- 
2.25.1

Re: [PATCH v6 11/42] nvme: add temperature threshold feature

Re: [PATCH v6 11/42] nvme: add temperature threshold feature

Re: [PATCH v6 11/42] nvme: add temperature threshold feature

[PATCH v6 11/42] nvme: add temperature threshold feature

4 matches

Site Navigation

Mail list logo

Footer information