diff mbox series

[3/9] hw/block/nvme: support per-namespace smart log

Message ID 20200930220414.562527-4-kbusch@kernel.org
State Accepted
Commit 2fbbecc5cd90ec00027a155f7044f2f70ed84f30
Headers show
Series nvme qemu cleanups and fixes | expand

Commit Message

Keith Busch Sept. 30, 2020, 10:04 p.m. UTC
Let the user specify a specific namespace if they want to get access
stats for a specific namespace.

Signed-off-by: Keith Busch <kbusch@kernel.org>
---
 hw/block/nvme.c      | 66 +++++++++++++++++++++++++++-----------------
 include/block/nvme.h |  1 +
 2 files changed, 41 insertions(+), 26 deletions(-)

Comments

Klaus Jensen Oct. 1, 2020, 4:10 a.m. UTC | #1
On Sep 30 15:04, Keith Busch wrote:
> Let the user specify a specific namespace if they want to get access
> stats for a specific namespace.
> 

I don't think this makes sense for v1.3+.

NVM Express v1.3d, Section 5.14.1.2: "There is no namespace specific
information defined in the SMART / Health log page in this revision of
the specification.  therefore the controller log page and namespace
specific log page contain identical information".

I have no idea why the TWG decided this, but that's the way it is ;)

> Signed-off-by: Keith Busch <kbusch@kernel.org>
> ---
>  hw/block/nvme.c      | 66 +++++++++++++++++++++++++++-----------------
>  include/block/nvme.h |  1 +
>  2 files changed, 41 insertions(+), 26 deletions(-)
> 
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index 8d2b5be567..41389b2b09 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -1164,48 +1164,62 @@ static uint16_t nvme_create_sq(NvmeCtrl *n, NvmeRequest *req)
>      return NVME_SUCCESS;
>  }
>  
> +struct nvme_stats {
> +    uint64_t units_read;
> +    uint64_t units_written;
> +    uint64_t read_commands;
> +    uint64_t write_commands;
> +};
> +
> +static void nvme_set_blk_stats(NvmeNamespace *ns, struct nvme_stats *stats)
> +{
> +    BlockAcctStats *s = blk_get_stats(ns->blkconf.blk);
> +
> +    stats->units_read += s->nr_bytes[BLOCK_ACCT_READ] >> BDRV_SECTOR_BITS;
> +    stats->units_written += s->nr_bytes[BLOCK_ACCT_WRITE] >> BDRV_SECTOR_BITS;
> +    stats->read_commands += s->nr_ops[BLOCK_ACCT_READ];
> +    stats->write_commands += s->nr_ops[BLOCK_ACCT_WRITE];
> +}
> +
>  static uint16_t nvme_smart_info(NvmeCtrl *n, uint8_t rae, uint32_t buf_len,
>                                  uint64_t off, NvmeRequest *req)
>  {
>      uint32_t nsid = le32_to_cpu(req->cmd.nsid);
> -
> +    struct nvme_stats stats = { 0 };
> +    NvmeSmartLog smart = { 0 };
>      uint32_t trans_len;
> +    NvmeNamespace *ns;
>      time_t current_ms;
> -    uint64_t units_read = 0, units_written = 0;
> -    uint64_t read_commands = 0, write_commands = 0;
> -    NvmeSmartLog smart;
> -
> -    if (nsid && nsid != 0xffffffff) {
> -        return NVME_INVALID_FIELD | NVME_DNR;
> -    }
>  
>      if (off >= sizeof(smart)) {
>          return NVME_INVALID_FIELD | NVME_DNR;
>      }
>  
> -    for (int i = 1; i <= n->num_namespaces; i++) {
> -        NvmeNamespace *ns = nvme_ns(n, i);
> -        if (!ns) {
> -            continue;
> -        }
> -
> -        BlockAcctStats *s = blk_get_stats(ns->blkconf.blk);
> +    if (nsid != 0xffffffff) {
> +        ns = nvme_ns(n, nsid);
> +        if (!ns)
> +            return NVME_INVALID_NSID | NVME_DNR;
> +        nvme_set_blk_stats(ns, &stats);
> +    } else {
> +        int i;
>  
> -        units_read += s->nr_bytes[BLOCK_ACCT_READ] >> BDRV_SECTOR_BITS;
> -        units_written += s->nr_bytes[BLOCK_ACCT_WRITE] >> BDRV_SECTOR_BITS;
> -        read_commands += s->nr_ops[BLOCK_ACCT_READ];
> -        write_commands += s->nr_ops[BLOCK_ACCT_WRITE];
> +        for (i = 1; i <= n->num_namespaces; i++) {
> +            ns = nvme_ns(n, i);
> +            if (!ns) {
> +                continue;
> +            }
> +            nvme_set_blk_stats(ns, &stats);
> +        }
>      }
>  
>      trans_len = MIN(sizeof(smart) - off, buf_len);
>  
> -    memset(&smart, 0x0, sizeof(smart));
> -
> -    smart.data_units_read[0] = cpu_to_le64(DIV_ROUND_UP(units_read, 1000));
> -    smart.data_units_written[0] = cpu_to_le64(DIV_ROUND_UP(units_written,
> +    smart.data_units_read[0] = cpu_to_le64(DIV_ROUND_UP(stats.units_read,
> +                                                        1000));
> +    smart.data_units_written[0] = cpu_to_le64(DIV_ROUND_UP(stats.units_written,
>                                                             1000));
> -    smart.host_read_commands[0] = cpu_to_le64(read_commands);
> -    smart.host_write_commands[0] = cpu_to_le64(write_commands);
> +    smart.host_read_commands[0] = cpu_to_le64(stats.read_commands);
> +    smart.host_write_commands[0] = cpu_to_le64(stats.write_commands);
>  
>      smart.temperature = cpu_to_le16(n->temperature);
>  
> @@ -2708,7 +2722,7 @@ static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice *pci_dev)
>      id->acl = 3;
>      id->aerl = n->params.aerl;
>      id->frmw = (NVME_NUM_FW_SLOTS << 1) | NVME_FRMW_SLOT1_RO;
> -    id->lpa = NVME_LPA_EXTENDED;
> +    id->lpa = NVME_LPA_NS_SMART | NVME_LPA_EXTENDED;
>  
>      /* recommended default value (~70 C) */
>      id->wctemp = cpu_to_le16(NVME_TEMPERATURE_WARNING);
> diff --git a/include/block/nvme.h b/include/block/nvme.h
> index 58647bcdad..868cf53f0b 100644
> --- a/include/block/nvme.h
> +++ b/include/block/nvme.h
> @@ -849,6 +849,7 @@ enum NvmeIdCtrlFrmw {
>  };
>  
>  enum NvmeIdCtrlLpa {
> +    NVME_LPA_NS_SMART = 1 << 0,
>      NVME_LPA_EXTENDED = 1 << 2,
>  };
>  
> -- 
> 2.24.1
> 
>
Keith Busch Oct. 1, 2020, 3:20 p.m. UTC | #2
On Thu, Oct 01, 2020 at 06:10:57AM +0200, Klaus Jensen wrote:
> On Sep 30 15:04, Keith Busch wrote:

> > Let the user specify a specific namespace if they want to get access

> > stats for a specific namespace.

> > 

> 

> I don't think this makes sense for v1.3+.

> 

> NVM Express v1.3d, Section 5.14.1.2: "There is no namespace specific

> information defined in the SMART / Health log page in this revision of

> the specification.  therefore the controller log page and namespace

> specific log page contain identical information".

> 

> I have no idea why the TWG decided this, but that's the way it is ;)


I don't think they did that. The behavior you're referring to is specific to
controllers that operate a particular way: "If the log page is not supported on
a per namespace basis ...". They were trying to provide a spec compliant way
for controllers to return a success status if you supplied a valid NSID when
the controller doesn't support per-namespace smart data..

The previous paragraph is more clear on this: "The controller may also support
requesting the log page on a per namespace basis, as indicated by bit 0 of the
LPA field in the Identify Controller data structure".
Klaus Jensen Oct. 1, 2020, 5:18 p.m. UTC | #3
On Oct  1 09:20, Keith Busch wrote:
> On Thu, Oct 01, 2020 at 06:10:57AM +0200, Klaus Jensen wrote:

> > On Sep 30 15:04, Keith Busch wrote:

> > > Let the user specify a specific namespace if they want to get access

> > > stats for a specific namespace.

> > > 

> > 

> > I don't think this makes sense for v1.3+.

> > 

> > NVM Express v1.3d, Section 5.14.1.2: "There is no namespace specific

> > information defined in the SMART / Health log page in this revision of

> > the specification.  therefore the controller log page and namespace

> > specific log page contain identical information".

> > 

> > I have no idea why the TWG decided this, but that's the way it is ;)

> 

> I don't think they did that. The behavior you're referring to is specific to

> controllers that operate a particular way: "If the log page is not supported on

> a per namespace basis ...". They were trying to provide a spec compliant way

> for controllers to return a success status if you supplied a valid NSID when

> the controller doesn't support per-namespace smart data..

> 

> The previous paragraph is more clear on this: "The controller may also support

> requesting the log page on a per namespace basis, as indicated by bit 0 of the

> LPA field in the Identify Controller data structure".


OK, so I agree that it makes sense for it to be supported on a per
namespace basis, but I think the spec is just keeping the door open for
future namespace specific stuff in the log page - currently there is
none.

Figure 94 (the actual SMART log page) says that the Data Units
Read/Written are controller wide, so there really is no namespace
specific information. Maybe this could be in the context of shared
namespaces? How would a controller know how much data has been
read/written from it without asking the other controllers? What if a
controller is detached from the namespace - you'd lose those numbers.
Keith Busch Oct. 1, 2020, 5:30 p.m. UTC | #4
On Thu, Oct 01, 2020 at 07:18:37PM +0200, Klaus Jensen wrote:
> OK, so I agree that it makes sense for it to be supported on a per

> namespace basis, but I think the spec is just keeping the door open for

> future namespace specific stuff in the log page - currently there is

> none.

> 

> Figure 94 (the actual SMART log page) says that the Data Units

> Read/Written are controller wide, so there really is no namespace

> specific information. Maybe this could be in the context of shared

> namespaces? How would a controller know how much data has been

> read/written from it without asking the other controllers? What if a

> controller is detached from the namespace - you'd lose those numbers.


That text is wrong. There is no "controller" scope to the smart log.
Figure 191 says the smart scope is to the subsystem or the namespace. It
doesn't matter which controller performed an IO to a particular
namespace; the log needs to report the same information regardless of
which controller you query. How that is coordinated within the subsystem
is a detail not defined by spec.

Not that that particular detail matters here, as we don't support
multi-controller subsystems (yet!). But the smart log text has missed an
update to reflect this, so it looks like trivial ECN material to me.
Klaus Jensen Oct. 1, 2020, 5:34 p.m. UTC | #5
On Oct  1 10:30, Keith Busch wrote:
> On Thu, Oct 01, 2020 at 07:18:37PM +0200, Klaus Jensen wrote:

> > OK, so I agree that it makes sense for it to be supported on a per

> > namespace basis, but I think the spec is just keeping the door open for

> > future namespace specific stuff in the log page - currently there is

> > none.

> > 

> > Figure 94 (the actual SMART log page) says that the Data Units

> > Read/Written are controller wide, so there really is no namespace

> > specific information. Maybe this could be in the context of shared

> > namespaces? How would a controller know how much data has been

> > read/written from it without asking the other controllers? What if a

> > controller is detached from the namespace - you'd lose those numbers.

> 

> That text is wrong. There is no "controller" scope to the smart log.

> Figure 191 says the smart scope is to the subsystem or the namespace. It

> doesn't matter which controller performed an IO to a particular

> namespace; the log needs to report the same information regardless of

> which controller you query. How that is coordinated within the subsystem

> is a detail not defined by spec.

> 


Oh! Thats new in v1.4. So they updated that, but forgot figure 194. In
v1.3 it is controller and namespace scope.

Anyway, fair enough. In that case,

Reviewed-by: Klaus Jensen <k.jensen@samsung.com>
Klaus Jensen Oct. 2, 2020, 8:48 a.m. UTC | #6
On Sep 30 15:04, Keith Busch wrote:
> Let the user specify a specific namespace if they want to get access

> stats for a specific namespace.

> 

> Signed-off-by: Keith Busch <kbusch@kernel.org>

> ---

>  hw/block/nvme.c      | 66 +++++++++++++++++++++++++++-----------------

>  include/block/nvme.h |  1 +

>  2 files changed, 41 insertions(+), 26 deletions(-)

> 

> diff --git a/hw/block/nvme.c b/hw/block/nvme.c

> index 8d2b5be567..41389b2b09 100644

> --- a/hw/block/nvme.c

> +++ b/hw/block/nvme.c

>  static uint16_t nvme_smart_info(NvmeCtrl *n, uint8_t rae, uint32_t buf_len,

>                                  uint64_t off, NvmeRequest *req)

>  {

>      uint32_t nsid = le32_to_cpu(req->cmd.nsid);

> -

> +    struct nvme_stats stats = { 0 };

> +    NvmeSmartLog smart = { 0 };

>      uint32_t trans_len;

> +    NvmeNamespace *ns;

>      time_t current_ms;

> -    uint64_t units_read = 0, units_written = 0;

> -    uint64_t read_commands = 0, write_commands = 0;

> -    NvmeSmartLog smart;

> -

> -    if (nsid && nsid != 0xffffffff) {

> -        return NVME_INVALID_FIELD | NVME_DNR;

> -    }

>  

>      if (off >= sizeof(smart)) {

>          return NVME_INVALID_FIELD | NVME_DNR;

>      }

>  

> -    for (int i = 1; i <= n->num_namespaces; i++) {

> -        NvmeNamespace *ns = nvme_ns(n, i);

> -        if (!ns) {

> -            continue;

> -        }

> -

> -        BlockAcctStats *s = blk_get_stats(ns->blkconf.blk);

> +    if (nsid != 0xffffffff) {

> +        ns = nvme_ns(n, nsid);

> +        if (!ns)

> +            return NVME_INVALID_NSID | NVME_DNR;


Btw, this is failing style check (missing braces).
Dmitry Fomichev Oct. 6, 2020, 1:57 a.m. UTC | #7
> -----Original Message-----
> From: Keith Busch <kbusch@kernel.org>
> Sent: Wednesday, September 30, 2020 6:04 PM
> To: qemu-block@nongnu.org; qemu-devel@nongnu.org; Klaus Jensen
> <k.jensen@samsung.com>
> Cc: Niklas Cassel <Niklas.Cassel@wdc.com>; Dmitry Fomichev
> <Dmitry.Fomichev@wdc.com>; Kevin Wolf <kwolf@redhat.com>; Philippe
> Mathieu-Daudé <philmd@redhat.com>; Keith Busch <kbusch@kernel.org>
> Subject: [PATCH 3/9] hw/block/nvme: support per-namespace smart log
> 
> Let the user specify a specific namespace if they want to get access
> stats for a specific namespace.
> 
> Signed-off-by: Keith Busch <kbusch@kernel.org>
> ---
>  hw/block/nvme.c      | 66 +++++++++++++++++++++++++++-----------------
>  include/block/nvme.h |  1 +
>  2 files changed, 41 insertions(+), 26 deletions(-)
> 
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index 8d2b5be567..41389b2b09 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -1164,48 +1164,62 @@ static uint16_t nvme_create_sq(NvmeCtrl *n,
> NvmeRequest *req)
>      return NVME_SUCCESS;
>  }
> 
> +struct nvme_stats {
> +    uint64_t units_read;
> +    uint64_t units_written;
> +    uint64_t read_commands;
> +    uint64_t write_commands;
> +};
> +
> +static void nvme_set_blk_stats(NvmeNamespace *ns, struct nvme_stats
> *stats)
> +{
> +    BlockAcctStats *s = blk_get_stats(ns->blkconf.blk);
> +
> +    stats->units_read += s->nr_bytes[BLOCK_ACCT_READ] >>
> BDRV_SECTOR_BITS;
> +    stats->units_written += s->nr_bytes[BLOCK_ACCT_WRITE] >>
> BDRV_SECTOR_BITS;
> +    stats->read_commands += s->nr_ops[BLOCK_ACCT_READ];
> +    stats->write_commands += s->nr_ops[BLOCK_ACCT_WRITE];
> +}
> +
>  static uint16_t nvme_smart_info(NvmeCtrl *n, uint8_t rae, uint32_t
> buf_len,
>                                  uint64_t off, NvmeRequest *req)
>  {
>      uint32_t nsid = le32_to_cpu(req->cmd.nsid);
> -
> +    struct nvme_stats stats = { 0 };
> +    NvmeSmartLog smart = { 0 };
>      uint32_t trans_len;
> +    NvmeNamespace *ns;
>      time_t current_ms;
> -    uint64_t units_read = 0, units_written = 0;
> -    uint64_t read_commands = 0, write_commands = 0;
> -    NvmeSmartLog smart;
> -
> -    if (nsid && nsid != 0xffffffff) {
> -        return NVME_INVALID_FIELD | NVME_DNR;
> -    }
> 
>      if (off >= sizeof(smart)) {
>          return NVME_INVALID_FIELD | NVME_DNR;
>      }
> 
> -    for (int i = 1; i <= n->num_namespaces; i++) {
> -        NvmeNamespace *ns = nvme_ns(n, i);
> -        if (!ns) {
> -            continue;
> -        }
> -
> -        BlockAcctStats *s = blk_get_stats(ns->blkconf.blk);
> +    if (nsid != 0xffffffff) {
> +        ns = nvme_ns(n, nsid);
> +        if (!ns)
> +            return NVME_INVALID_NSID | NVME_DNR;

checkpatch will complain about missing curly braces here

> +        nvme_set_blk_stats(ns, &stats);
> +    } else {
> +        int i;
> 
> -        units_read += s->nr_bytes[BLOCK_ACCT_READ] >>
> BDRV_SECTOR_BITS;
> -        units_written += s->nr_bytes[BLOCK_ACCT_WRITE] >>
> BDRV_SECTOR_BITS;
> -        read_commands += s->nr_ops[BLOCK_ACCT_READ];
> -        write_commands += s->nr_ops[BLOCK_ACCT_WRITE];
> +        for (i = 1; i <= n->num_namespaces; i++) {
> +            ns = nvme_ns(n, i);
> +            if (!ns) {
> +                continue;
> +            }
> +            nvme_set_blk_stats(ns, &stats);
> +        }
>      }
> 
>      trans_len = MIN(sizeof(smart) - off, buf_len);
> 
> -    memset(&smart, 0x0, sizeof(smart));
> -
> -    smart.data_units_read[0] = cpu_to_le64(DIV_ROUND_UP(units_read,
> 1000));
> -    smart.data_units_written[0] =
> cpu_to_le64(DIV_ROUND_UP(units_written,
> +    smart.data_units_read[0] =
> cpu_to_le64(DIV_ROUND_UP(stats.units_read,
> +                                                        1000));
> +    smart.data_units_written[0] =
> cpu_to_le64(DIV_ROUND_UP(stats.units_written,
>                                                             1000));
> -    smart.host_read_commands[0] = cpu_to_le64(read_commands);
> -    smart.host_write_commands[0] = cpu_to_le64(write_commands);
> +    smart.host_read_commands[0] = cpu_to_le64(stats.read_commands);
> +    smart.host_write_commands[0] = cpu_to_le64(stats.write_commands);
> 
>      smart.temperature = cpu_to_le16(n->temperature);
> 
> @@ -2708,7 +2722,7 @@ static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice
> *pci_dev)
>      id->acl = 3;
>      id->aerl = n->params.aerl;
>      id->frmw = (NVME_NUM_FW_SLOTS << 1) | NVME_FRMW_SLOT1_RO;
> -    id->lpa = NVME_LPA_EXTENDED;
> +    id->lpa = NVME_LPA_NS_SMART | NVME_LPA_EXTENDED;
> 
>      /* recommended default value (~70 C) */
>      id->wctemp = cpu_to_le16(NVME_TEMPERATURE_WARNING);
> diff --git a/include/block/nvme.h b/include/block/nvme.h
> index 58647bcdad..868cf53f0b 100644
> --- a/include/block/nvme.h
> +++ b/include/block/nvme.h
> @@ -849,6 +849,7 @@ enum NvmeIdCtrlFrmw {
>  };
> 
>  enum NvmeIdCtrlLpa {
> +    NVME_LPA_NS_SMART = 1 << 0,
>      NVME_LPA_EXTENDED = 1 << 2,
>  };
> 
> --
> 2.24.1
diff mbox series

Patch

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 8d2b5be567..41389b2b09 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -1164,48 +1164,62 @@  static uint16_t nvme_create_sq(NvmeCtrl *n, NvmeRequest *req)
     return NVME_SUCCESS;
 }
 
+struct nvme_stats {
+    uint64_t units_read;
+    uint64_t units_written;
+    uint64_t read_commands;
+    uint64_t write_commands;
+};
+
+static void nvme_set_blk_stats(NvmeNamespace *ns, struct nvme_stats *stats)
+{
+    BlockAcctStats *s = blk_get_stats(ns->blkconf.blk);
+
+    stats->units_read += s->nr_bytes[BLOCK_ACCT_READ] >> BDRV_SECTOR_BITS;
+    stats->units_written += s->nr_bytes[BLOCK_ACCT_WRITE] >> BDRV_SECTOR_BITS;
+    stats->read_commands += s->nr_ops[BLOCK_ACCT_READ];
+    stats->write_commands += s->nr_ops[BLOCK_ACCT_WRITE];
+}
+
 static uint16_t nvme_smart_info(NvmeCtrl *n, uint8_t rae, uint32_t buf_len,
                                 uint64_t off, NvmeRequest *req)
 {
     uint32_t nsid = le32_to_cpu(req->cmd.nsid);
-
+    struct nvme_stats stats = { 0 };
+    NvmeSmartLog smart = { 0 };
     uint32_t trans_len;
+    NvmeNamespace *ns;
     time_t current_ms;
-    uint64_t units_read = 0, units_written = 0;
-    uint64_t read_commands = 0, write_commands = 0;
-    NvmeSmartLog smart;
-
-    if (nsid && nsid != 0xffffffff) {
-        return NVME_INVALID_FIELD | NVME_DNR;
-    }
 
     if (off >= sizeof(smart)) {
         return NVME_INVALID_FIELD | NVME_DNR;
     }
 
-    for (int i = 1; i <= n->num_namespaces; i++) {
-        NvmeNamespace *ns = nvme_ns(n, i);
-        if (!ns) {
-            continue;
-        }
-
-        BlockAcctStats *s = blk_get_stats(ns->blkconf.blk);
+    if (nsid != 0xffffffff) {
+        ns = nvme_ns(n, nsid);
+        if (!ns)
+            return NVME_INVALID_NSID | NVME_DNR;
+        nvme_set_blk_stats(ns, &stats);
+    } else {
+        int i;
 
-        units_read += s->nr_bytes[BLOCK_ACCT_READ] >> BDRV_SECTOR_BITS;
-        units_written += s->nr_bytes[BLOCK_ACCT_WRITE] >> BDRV_SECTOR_BITS;
-        read_commands += s->nr_ops[BLOCK_ACCT_READ];
-        write_commands += s->nr_ops[BLOCK_ACCT_WRITE];
+        for (i = 1; i <= n->num_namespaces; i++) {
+            ns = nvme_ns(n, i);
+            if (!ns) {
+                continue;
+            }
+            nvme_set_blk_stats(ns, &stats);
+        }
     }
 
     trans_len = MIN(sizeof(smart) - off, buf_len);
 
-    memset(&smart, 0x0, sizeof(smart));
-
-    smart.data_units_read[0] = cpu_to_le64(DIV_ROUND_UP(units_read, 1000));
-    smart.data_units_written[0] = cpu_to_le64(DIV_ROUND_UP(units_written,
+    smart.data_units_read[0] = cpu_to_le64(DIV_ROUND_UP(stats.units_read,
+                                                        1000));
+    smart.data_units_written[0] = cpu_to_le64(DIV_ROUND_UP(stats.units_written,
                                                            1000));
-    smart.host_read_commands[0] = cpu_to_le64(read_commands);
-    smart.host_write_commands[0] = cpu_to_le64(write_commands);
+    smart.host_read_commands[0] = cpu_to_le64(stats.read_commands);
+    smart.host_write_commands[0] = cpu_to_le64(stats.write_commands);
 
     smart.temperature = cpu_to_le16(n->temperature);
 
@@ -2708,7 +2722,7 @@  static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice *pci_dev)
     id->acl = 3;
     id->aerl = n->params.aerl;
     id->frmw = (NVME_NUM_FW_SLOTS << 1) | NVME_FRMW_SLOT1_RO;
-    id->lpa = NVME_LPA_EXTENDED;
+    id->lpa = NVME_LPA_NS_SMART | NVME_LPA_EXTENDED;
 
     /* recommended default value (~70 C) */
     id->wctemp = cpu_to_le16(NVME_TEMPERATURE_WARNING);
diff --git a/include/block/nvme.h b/include/block/nvme.h
index 58647bcdad..868cf53f0b 100644
--- a/include/block/nvme.h
+++ b/include/block/nvme.h
@@ -849,6 +849,7 @@  enum NvmeIdCtrlFrmw {
 };
 
 enum NvmeIdCtrlLpa {
+    NVME_LPA_NS_SMART = 1 << 0,
     NVME_LPA_EXTENDED = 1 << 2,
 };