On Mon, 2020-03-16 at 07:28 -0700, Klaus Jensen wrote: > From: Klaus Jensen <k.jen...@samsung.com> > > 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.jen...@samsung.com> > Acked-by: Keith Busch <kbu...@kernel.org> > --- > 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_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; 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_t arbitration; > uint32_t power_mgmt; > - uint32_t temp_thresh; > + union { > + struct { > + uint16_t temp_thresh_hi; > + uint16_t temp_thresh_low; > + }; > + uint32_t temp_thresh; > + }; > uint32_t err_rec; > uint32_t volatile_wc; > uint32_t num_queues; With 'temperature' field removed from the header: Reviewed-by: Maxim Levitsky <mlevi...@redhat.com> Best regards, Maxim Levitsky