| Message ID | 20200316142928.153431-12-its@irrelevant.dk |
|---|---|
| State | New |
| Headers | show |
| Series | nvme: support NVMe v1.3d, SGLs and multiple namespaces | expand |
On Mon, 2020-03-16 at 07:28 -0700, Klaus Jensen wrote: > From: Klaus Jensen <k.jensen@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.jensen@samsung.com> > Acked-by: Keith Busch <kbusch@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 <mlevitsk@redhat.com> Best regards, Maxim Levitsky
On Mar 25 12:40, Maxim Levitsky wrote: > On Mon, 2020-03-16 at 07:28 -0700, Klaus Jensen wrote: > > From: Klaus Jensen <k.jensen@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.jensen@samsung.com> > > Acked-by: Keith Busch <kbusch@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.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. > Fixed! > > With 'temperature' field removed from the header: > > Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com> > > Best regards, > 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 <k.jensen@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.jensen@samsung.com> > > > Acked-by: Keith Busch <kbusch@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.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. > > > > Fixed! Thanks. > > > > With 'temperature' field removed from the header: > > > > Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com> > > > > Best regards, > > Maxim Levitsky > > > > Best regards, Maxim Levitsky
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; 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;