On Jul 3 00:44, Dmitry Fomichev wrote: > On Mon, 2020-06-29 at 20:26 +0200, Klaus Jensen wrote: > > From: Klaus Jensen <k.jen...@samsung.com> > > > > It might seem weird 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> > > Reviewed-by: Maxim Levitsky <mlevi...@redhat.com> > > --- > > hw/block/nvme.c | 48 ++++++++++++++++++++++++++++++++++++++++++++ > > hw/block/nvme.h | 1 + > > include/block/nvme.h | 8 +++++++- > > 3 files changed, 56 insertions(+), 1 deletion(-) > > > > diff --git a/hw/block/nvme.c b/hw/block/nvme.c > > index b7037a7d3504..5ca50646369e 100644 > > --- a/hw/block/nvme.c > > +++ b/hw/block/nvme.c > > @@ -59,6 +59,9 @@ > > #define NVME_DB_SIZE 4 > > #define NVME_CMB_BIR 2 > > #define NVME_PMR_BIR 2 > > +#define NVME_TEMPERATURE 0x143 > > +#define NVME_TEMPERATURE_WARNING 0x157 > > +#define NVME_TEMPERATURE_CRITICAL 0x175 > > > > #define NVME_GUEST_ERR(trace, fmt, ...) \ > > do { \ > > @@ -827,9 +830,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) != NVME_TEMP_TMPSEL_COMPOSITE) { > > + break; > > + } > > + > > + switch (NVME_TEMP_THSEL(dw11)) { > > + case NVME_TEMP_THSEL_OVER: > > + result = cpu_to_le16(n->features.temp_thresh_hi); > > + break; > > + case NVME_TEMP_THSEL_UNDER: > > + 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_pci_nvme_getfeat_vwcache(result ? "enabled" : "disabled"); > > @@ -874,6 +899,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) != NVME_TEMP_TMPSEL_COMPOSITE) { > > + break; > > + } > > + > > + switch (NVME_TEMP_THSEL(dw11)) { > > + case NVME_TEMP_THSEL_OVER: > > + n->features.temp_thresh_hi = NVME_TEMP_TMPTH(dw11); > > + break; > > + case NVME_TEMP_THSEL_UNDER: > > + 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; > > @@ -1454,6 +1496,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_blk(NvmeCtrl *n, Error **errp) > > @@ -1611,6 +1654,11 @@ static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice > > *pci_dev) > > 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 1bf5c80ed843..3acde10e1d2a 100644 > > --- a/hw/block/nvme.h > > +++ b/hw/block/nvme.h > > @@ -107,6 +107,7 @@ typedef struct NvmeCtrl { > > NvmeSQueue admin_sq; > > NvmeCQueue admin_cq; > > NvmeIdCtrl id_ctrl; > > + NvmeFeatureVal features; > > } NvmeCtrl; > > > > /* calculate the number of LBAs that the namespace can accomodate */ > > diff --git a/include/block/nvme.h b/include/block/nvme.h > > index 6d1fa6ff2228..bb651d0cbf5a 100644 > > --- a/include/block/nvme.h > > +++ b/include/block/nvme.h > > @@ -860,7 +860,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; > > temp_thresh seems unused, is this union really needed? >
True. I kept the 32bit member there because it aligns with all other features. But I guess it can be re-added if it is needed for some reason in the future, so I removed it. > > + }; > > uint32_t err_rec; > > uint32_t volatile_wc; > > uint32_t num_queues;