From: Alan Adamson <alan.adamson@oracle.com>
Add support for the namespace atomic paramters: NAWUN and NAWUN. Namespace
Atomic Compare and Write Unit (NACWU) is not currently supported.
Writes that adhere to the NACWU and NAWUPF parameters are guaranteed to be
atomic.
New NVMe QEMU Paramters (See NVMe Specification for details):
atomic.nawun=UINT16 (default: 0)
atomic.nawupf=UINT16 (default: 0)
atomic.nsfeat (default off) - Set Namespace Supported Atomic Boundary &
Power (NSABP) bit in Namespace Features (NSFEAT) in the Identify
Namespace Data Structure
See the NVMe Specification for more information.
Signed-off-by: Alan Adamson <alan.adamson@oracle.com>
Reviewed-by: Jesper Wendel Devantier <foss@defmacro.it>
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
---
hw/nvme/ctrl.c | 23 +++++++++++++++++++++++
hw/nvme/ns.c | 38 ++++++++++++++++++++++++++++++++++++++
hw/nvme/nvme.h | 6 ++++++
3 files changed, 67 insertions(+)
diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index fa003031e719..121a95b2e373 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -6705,6 +6705,23 @@ static uint16_t nvme_set_feature(NvmeCtrl *n, NvmeRequest *req)
} else {
atomic->atomic_writes = 1;
}
+ for (i = 1; i <= NVME_MAX_NAMESPACES; i++) {
+ ns = nvme_ns(n, i);
+ if (ns && ns->atomic.atomic_writes) {
+ if (n->dn) {
+ ns->atomic.atomic_max_write_size =
+ le16_to_cpu(ns->id_ns.nawupf) + 1;
+ } else {
+ ns->atomic.atomic_max_write_size =
+ le16_to_cpu(ns->id_ns.nawun) + 1;
+ }
+ if (ns->atomic.atomic_max_write_size == 1) {
+ ns->atomic.atomic_writes = 0;
+ } else {
+ ns->atomic.atomic_writes = 1;
+ }
+ }
+ }
break;
default:
return NVME_FEAT_NOT_CHANGEABLE | NVME_DNR;
@@ -7688,6 +7705,12 @@ static int nvme_atomic_write_check(NvmeCtrl *n, NvmeCmd *cmd,
static NvmeAtomic *nvme_get_atomic(NvmeCtrl *n, NvmeCmd *cmd)
{
+ NvmeNamespace *ns = nvme_ns(n, cmd->nsid);
+
+ if (ns && ns->atomic.atomic_writes) {
+ return &ns->atomic;
+ }
+
if (n->atomic.atomic_writes) {
return &n->atomic;
}
diff --git a/hw/nvme/ns.c b/hw/nvme/ns.c
index 6df2e8e7c5ac..28aacb8db59a 100644
--- a/hw/nvme/ns.c
+++ b/hw/nvme/ns.c
@@ -724,11 +724,46 @@ static void nvme_ns_realize(DeviceState *dev, Error **errp)
BusState *s = qdev_get_parent_bus(dev);
NvmeCtrl *n = NVME(s->parent);
NvmeSubsystem *subsys = n->subsys;
+ NvmeIdCtrl *id = &n->id_ctrl;
+ NvmeIdNs *id_ns = &ns->id_ns;
uint32_t nsid = ns->params.nsid;
int i;
assert(subsys);
+ /* Set atomic write parameters */
+ if (ns->params.atomic_nsfeat) {
+ id_ns->nsfeat |= NVME_ID_NS_NSFEAT_NSABPNS;
+ id_ns->nawun = cpu_to_le16(ns->params.atomic_nawun);
+ if (!id->awupf || (id_ns->nawun && (id_ns->nawun < id->awun))) {
+ error_report("Invalid NAWUN: %x AWUN=%x", id_ns->nawun, id->awun);
+ }
+ id_ns->nawupf = cpu_to_le16(ns->params.atomic_nawupf);
+ if (!id->awupf || (id_ns->nawupf && (id_ns->nawupf < id->awupf))) {
+ error_report("Invalid NAWUPF: %x AWUPF=%x",
+ id_ns->nawupf, id->awupf);
+ }
+ if (id_ns->nawupf > id_ns->nawun) {
+ error_report("Invalid: NAWUN=%x NAWUPF=%x",
+ id_ns->nawun, id_ns->nawupf);
+ }
+ }
+
+ if (id_ns->nawun || id_ns->nawupf) {
+ NvmeAtomic *atomic = &ns->atomic;
+
+ if (n->dn) {
+ atomic->atomic_max_write_size = cpu_to_le16(id_ns->nawupf) + 1;
+ } else {
+ atomic->atomic_max_write_size = cpu_to_le16(id_ns->nawun) + 1;
+ }
+ if (atomic->atomic_max_write_size == 1) {
+ atomic->atomic_writes = 0;
+ } else {
+ atomic->atomic_writes = 1;
+ }
+ }
+
/* reparent to subsystem bus */
if (!qdev_set_parent_bus(dev, &subsys->bus.parent_bus, errp)) {
return;
@@ -804,6 +839,9 @@ static const Property nvme_ns_props[] = {
DEFINE_PROP_BOOL("eui64-default", NvmeNamespace, params.eui64_default,
false),
DEFINE_PROP_STRING("fdp.ruhs", NvmeNamespace, params.fdp.ruhs),
+ DEFINE_PROP_UINT16("atomic.nawun", NvmeNamespace, params.atomic_nawun, 0),
+ DEFINE_PROP_UINT16("atomic.nawupf", NvmeNamespace, params.atomic_nawupf, 0),
+ DEFINE_PROP_BOOL("atomic.nsfeat", NvmeNamespace, params.atomic_nsfeat, 0),
};
static void nvme_ns_class_init(ObjectClass *oc, const void *data)
diff --git a/hw/nvme/nvme.h b/hw/nvme/nvme.h
index 67ed562e0086..7d01080fc1f9 100644
--- a/hw/nvme/nvme.h
+++ b/hw/nvme/nvme.h
@@ -218,6 +218,9 @@ typedef struct NvmeNamespaceParams {
struct {
char *ruhs;
} fdp;
+ uint16_t atomic_nawun;
+ uint16_t atomic_nawupf;
+ bool atomic_nsfeat;
} NvmeNamespaceParams;
typedef struct NvmeAtomic {
@@ -280,6 +283,9 @@ typedef struct NvmeNamespace {
/* reclaim unit handle identifiers indexed by placement handle */
uint16_t *phs;
} fdp;
+ uint16_t atomic_nawun;
+ uint16_t atomic_nawupf;
+ NvmeAtomic atomic;
} NvmeNamespace;
static inline uint32_t nvme_nsid(NvmeNamespace *ns)
--
2.51.0On Thu, 30 Oct 2025 at 07:30, Klaus Jensen <its@irrelevant.dk> wrote:
>
> From: Alan Adamson <alan.adamson@oracle.com>
>
> Add support for the namespace atomic paramters: NAWUN and NAWUN. Namespace
> Atomic Compare and Write Unit (NACWU) is not currently supported.
>
> Writes that adhere to the NACWU and NAWUPF parameters are guaranteed to be
> atomic.
>
> New NVMe QEMU Paramters (See NVMe Specification for details):
> atomic.nawun=UINT16 (default: 0)
> atomic.nawupf=UINT16 (default: 0)
> atomic.nsfeat (default off) - Set Namespace Supported Atomic Boundary &
> Power (NSABP) bit in Namespace Features (NSFEAT) in the Identify
> Namespace Data Structure
>
> See the NVMe Specification for more information.
>
> Signed-off-by: Alan Adamson <alan.adamson@oracle.com>
> Reviewed-by: Jesper Wendel Devantier <foss@defmacro.it>
> Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
> + /* Set atomic write parameters */
> + if (ns->params.atomic_nsfeat) {
> + id_ns->nsfeat |= NVME_ID_NS_NSFEAT_NSABPNS;
> + id_ns->nawun = cpu_to_le16(ns->params.atomic_nawun);
> + if (!id->awupf || (id_ns->nawun && (id_ns->nawun < id->awun))) {
> + error_report("Invalid NAWUN: %x AWUN=%x", id_ns->nawun, id->awun);
> + }
This error check is about NAWUN, but the condition looks at id->awpuf.
Is that intentional ? (Coverity wonders if this is a copy-paste error:
CID 1642811.)
We should return early if we've detected an error case in the properties.
By default we'll fall on through. Similarly below.
This is a realize method, so error handling should be by
setting the 'error' argument, not by error_report().
> + id_ns->nawupf = cpu_to_le16(ns->params.atomic_nawupf);
> + if (!id->awupf || (id_ns->nawupf && (id_ns->nawupf < id->awupf))) {
> + error_report("Invalid NAWUPF: %x AWUPF=%x",
> + id_ns->nawupf, id->awupf);
> + }
> + if (id_ns->nawupf > id_ns->nawun) {
> + error_report("Invalid: NAWUN=%x NAWUPF=%x",
> + id_ns->nawun, id_ns->nawupf);
> + }
Personally I find this stack of checks a bit confusing -- we
are presumably catching various different invalid combinations
of the properties, but the error messages we produce are rather
unspecific. If it's the case that (for instance) the NAWUPF
cannot be larger than the NAWUN, we could tell the user that
specifically rather than just saying "Invalid" and making them
go look up what the requirements are in the spec or the code.
> + }
thanks
-- PMM
On Nov 2 11:50, Peter Maydell wrote:
> On Thu, 30 Oct 2025 at 07:30, Klaus Jensen <its@irrelevant.dk> wrote:
> >
> > From: Alan Adamson <alan.adamson@oracle.com>
> >
> > Add support for the namespace atomic paramters: NAWUN and NAWUN. Namespace
> > Atomic Compare and Write Unit (NACWU) is not currently supported.
> >
> > Writes that adhere to the NACWU and NAWUPF parameters are guaranteed to be
> > atomic.
> >
> > New NVMe QEMU Paramters (See NVMe Specification for details):
> > atomic.nawun=UINT16 (default: 0)
> > atomic.nawupf=UINT16 (default: 0)
> > atomic.nsfeat (default off) - Set Namespace Supported Atomic Boundary &
> > Power (NSABP) bit in Namespace Features (NSFEAT) in the Identify
> > Namespace Data Structure
> >
> > See the NVMe Specification for more information.
> >
> > Signed-off-by: Alan Adamson <alan.adamson@oracle.com>
> > Reviewed-by: Jesper Wendel Devantier <foss@defmacro.it>
> > Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
>
>
>
> > + /* Set atomic write parameters */
> > + if (ns->params.atomic_nsfeat) {
> > + id_ns->nsfeat |= NVME_ID_NS_NSFEAT_NSABPNS;
> > + id_ns->nawun = cpu_to_le16(ns->params.atomic_nawun);
> > + if (!id->awupf || (id_ns->nawun && (id_ns->nawun < id->awun))) {
> > + error_report("Invalid NAWUN: %x AWUN=%x", id_ns->nawun, id->awun);
> > + }
>
> This error check is about NAWUN, but the condition looks at id->awpuf.
> Is that intentional ? (Coverity wonders if this is a copy-paste error:
> CID 1642811.)
>
The check is as intended, but I can understand why Coverity thinks it
may be off. I'll rework it.
> We should return early if we've detected an error case in the properties.
> By default we'll fall on through. Similarly below.
>
> This is a realize method, so error handling should be by
> setting the 'error' argument, not by error_report().
>
True, I'll fix that up.
> > + id_ns->nawupf = cpu_to_le16(ns->params.atomic_nawupf);
> > + if (!id->awupf || (id_ns->nawupf && (id_ns->nawupf < id->awupf))) {
> > + error_report("Invalid NAWUPF: %x AWUPF=%x",
> > + id_ns->nawupf, id->awupf);
> > + }
> > + if (id_ns->nawupf > id_ns->nawun) {
> > + error_report("Invalid: NAWUN=%x NAWUPF=%x",
> > + id_ns->nawun, id_ns->nawupf);
> > + }
>
> Personally I find this stack of checks a bit confusing -- we
> are presumably catching various different invalid combinations
> of the properties, but the error messages we produce are rather
> unspecific. If it's the case that (for instance) the NAWUPF
> cannot be larger than the NAWUN, we could tell the user that
> specifically rather than just saying "Invalid" and making them
> go look up what the requirements are in the spec or the code.
>
I'll fix up the parameter validation.
Thanks Peter!
© 2016 - 2025 Red Hat, Inc.