nvdimm.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
From: Dan Williams <dan.j.williams@intel.com>
To: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: linux-cxl@vger.kernel.org, Linux NVDIMM <nvdimm@lists.linux.dev>,
	 Ben Widawsky <ben.widawsky@intel.com>,
	Vishal L Verma <vishal.l.verma@intel.com>,
	 "Schofield, Alison" <alison.schofield@intel.com>,
	"Weiny, Ira" <ira.weiny@intel.com>
Subject: Re: [PATCH 11/23] libnvdimm/labels: Introduce CXL labels
Date: Wed, 11 Aug 2021 16:01:26 -0700	[thread overview]
Message-ID: <CAPcyv4hD16UzKBDHDAbUwmTM01ycXcJCFmS8S11C4JmP1A8d6Q@mail.gmail.com> (raw)
In-Reply-To: <20210811194130.00006076@Huawei.com>

On Wed, Aug 11, 2021 at 11:42 AM Jonathan Cameron
<Jonathan.Cameron@huawei.com> wrote:
>
> On Mon, 9 Aug 2021 15:28:46 -0700
> Dan Williams <dan.j.williams@intel.com> wrote:
>
> > Now that all of use sites of label data have been converted to nsl_*
> > helpers, introduce the CXL label format. The ->cxl flag in
> > nvdimm_drvdata indicates the label format the device expects. A
> > follow-on patch allows a bus provider to select the label style.
> >
> > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
>
> A few trivial things inline. Nothing that actually 'needs' changing though.
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
> > index e6e77691dbec..71ffde56fac0 100644
> > --- a/drivers/nvdimm/label.h
> > +++ b/drivers/nvdimm/label.h
> > @@ -64,40 +64,77 @@ struct nd_namespace_index {
> >       u8 free[];
> >  };
> >
> > -/**
> > - * struct nd_namespace_label - namespace superblock
> > - * @uuid: UUID per RFC 4122
> > - * @name: optional name (NULL-terminated)
> > - * @flags: see NSLABEL_FLAG_*
> > - * @nlabel: num labels to describe this ns
> > - * @position: labels position in set
> > - * @isetcookie: interleave set cookie
> > - * @lbasize: LBA size in bytes or 0 for pmem
> > - * @dpa: DPA of NVM range on this DIMM
> > - * @rawsize: size of namespace
> > - * @slot: slot of this label in label area
> > - * @unused: must be zero
> > - */
> >  struct nd_namespace_label {
> > +     union {
> Cross reference might be a nice thing to include?
> Table 212 I think...
> > +             struct nvdimm_cxl_label {
> > +                     uuid_t type;
> > +                     uuid_t uuid;
> > +                     u8 name[NSLABEL_NAME_LEN];
> > +                     __le32 flags;
> > +                     __le16 nlabel;
>
> Perhaps call out nlabel is nrange in the spec?

Actually, this is a bug, because nlabel in EFI labels is the width of
the interleave set. In CXL labels that property is moved to the region
and this field is only for discontiguous namespace support. Good
indirect catch!

>
> > +                     __le16 position;
> > +                     __le64 dpa;
> > +                     __le64 rawsize;
> > +                     __le32 slot;
> > +                     __le32 align;
> > +                     uuid_t region_uuid;
> > +                     uuid_t abstraction_uuid;
> > +                     __le16 lbasize;
> > +                     u8 reserved[0x56];
> > +                     __le64 checksum;
> > +             } cxl;
> > +             /**
> > +              * struct nvdimm_efi_label - namespace superblock
> > +              * @uuid: UUID per RFC 4122
> > +              * @name: optional name (NULL-terminated)
> > +              * @flags: see NSLABEL_FLAG_*
> > +              * @nlabel: num labels to describe this ns
> > +              * @position: labels position in set
> > +              * @isetcookie: interleave set cookie
> > +              * @lbasize: LBA size in bytes or 0 for pmem
> > +              * @dpa: DPA of NVM range on this DIMM
> > +              * @rawsize: size of namespace
> > +              * @slot: slot of this label in label area
> > +              * @unused: must be zero
> > +              */
> > +             struct nvdimm_efi_label {
> > +                     uuid_t uuid;
> > +                     u8 name[NSLABEL_NAME_LEN];
> > +                     __le32 flags;
> > +                     __le16 nlabel;
> > +                     __le16 position;
> > +                     __le64 isetcookie;
> > +                     __le64 lbasize;
> > +                     __le64 dpa;
> > +                     __le64 rawsize;
> > +                     __le32 slot;
> > +                     /*
> > +                      * Accessing fields past this point should be
> > +                      * gated by a efi_namespace_label_has() check.
> > +                      */
> > +                     u8 align;
> > +                     u8 reserved[3];
> > +                     guid_t type_guid;
> > +                     guid_t abstraction_guid;
> > +                     u8 reserved2[88];
> > +                     __le64 checksum;
> > +             } efi;
> > +     };
> > +};
> > +
> > +struct cxl_region_label {
>
> Perhaps separate this out to another patch so the diff ends up less confusing?

I'll take a look.

  reply	other threads:[~2021-08-11 23:01 UTC|newest]

Thread overview: 61+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-09 22:27 [PATCH 00/23] cxl_test: Enable CXL Topology and UAPI regression tests Dan Williams
2021-08-09 22:27 ` [PATCH 01/23] libnvdimm/labels: Introduce getters for namespace label fields Dan Williams
2021-08-10 20:48   ` Ben Widawsky
2021-08-10 21:58     ` Dan Williams
2021-08-11 18:44   ` Jonathan Cameron
2021-08-09 22:27 ` [PATCH 02/23] libnvdimm/labels: Add isetcookie validation helper Dan Williams
2021-08-11 18:44   ` Jonathan Cameron
2021-08-09 22:28 ` [PATCH 03/23] libnvdimm/labels: Introduce label setter helpers Dan Williams
2021-08-11 17:27   ` Jonathan Cameron
2021-08-11 17:42     ` Dan Williams
2021-08-09 22:28 ` [PATCH 04/23] libnvdimm/labels: Add a checksum calculation helper Dan Williams
2021-08-11 18:44   ` Jonathan Cameron
2021-08-09 22:28 ` [PATCH 05/23] libnvdimm/labels: Add blk isetcookie set / validation helpers Dan Williams
2021-08-11 18:45   ` Jonathan Cameron
2021-08-09 22:28 ` [PATCH 06/23] libnvdimm/labels: Add blk special cases for nlabel and position helpers Dan Williams
2021-08-11 18:45   ` Jonathan Cameron
2021-08-09 22:28 ` [PATCH 07/23] libnvdimm/labels: Add type-guid helpers Dan Williams
2021-08-11 18:46   ` Jonathan Cameron
2021-08-09 22:28 ` [PATCH 08/23] libnvdimm/labels: Add claim class helpers Dan Williams
2021-08-11 18:46   ` Jonathan Cameron
2021-08-09 22:28 ` [PATCH 09/23] libnvdimm/labels: Add address-abstraction uuid definitions Dan Williams
2021-08-11 18:49   ` Jonathan Cameron
2021-08-11 22:47     ` Dan Williams
2021-08-09 22:28 ` [PATCH 10/23] libnvdimm/labels: Add uuid helpers Dan Williams
2021-08-11  8:05   ` Andy Shevchenko
2021-08-11 16:59     ` Andy Shevchenko
2021-08-11 17:11       ` Dan Williams
2021-08-11 19:18         ` Andy Shevchenko
2021-08-11 19:26           ` Dan Williams
2021-08-12 22:34           ` Dan Williams
2021-08-13 10:14             ` Andy Shevchenko
2021-08-14  7:35               ` Christoph Hellwig
2021-08-11 18:13   ` Jonathan Cameron
2021-08-12 21:17     ` Dan Williams
2021-08-09 22:28 ` [PATCH 11/23] libnvdimm/labels: Introduce CXL labels Dan Williams
2021-08-11 18:41   ` Jonathan Cameron
2021-08-11 23:01     ` Dan Williams [this message]
2021-08-09 22:28 ` [PATCH 12/23] cxl/pci: Make 'struct cxl_mem' device type generic Dan Williams
2021-08-09 22:28 ` [PATCH 13/23] cxl/mbox: Introduce the mbox_send operation Dan Williams
2021-08-09 22:29 ` [PATCH 14/23] cxl/mbox: Move mailbox and other non-PCI specific infrastructure to the core Dan Williams
2021-08-11  6:11   ` [PATCH v2 " Dan Williams
2021-08-09 22:29 ` [PATCH 15/23] cxl/pci: Use module_pci_driver Dan Williams
2021-08-09 22:29 ` [PATCH 16/23] cxl/mbox: Convert 'enabled_cmds' to DECLARE_BITMAP Dan Williams
2021-08-09 22:29 ` [PATCH 17/23] cxl/mbox: Add exclusive kernel command support Dan Williams
2021-08-10 21:34   ` Ben Widawsky
2021-08-10 21:52     ` Dan Williams
2021-08-10 22:06       ` Ben Widawsky
2021-08-11  1:22         ` Dan Williams
2021-08-11  2:14           ` Dan Williams
2021-08-09 22:29 ` [PATCH 18/23] cxl/pmem: Translate NVDIMM label commands to CXL label commands Dan Williams
2021-08-09 22:29 ` [PATCH 19/23] cxl/pmem: Add support for multiple nvdimm-bridge objects Dan Williams
2021-08-09 22:29 ` [PATCH 20/23] tools/testing/cxl: Introduce a mocked-up CXL port hierarchy Dan Williams
2021-08-10 21:57   ` Ben Widawsky
2021-08-10 22:40     ` Dan Williams
2021-08-11 15:18       ` Ben Widawsky
     [not found]       ` <xp0k4.l2r85dw1p7do@intel.com>
2021-08-11 21:03         ` Dan Williams
2021-08-09 22:29 ` [PATCH 21/23] cxl/bus: Populate the target list at decoder create Dan Williams
2021-08-09 22:29 ` [PATCH 22/23] cxl/mbox: Move command definitions to common location Dan Williams
2021-08-09 22:29 ` [PATCH 23/23] tools/testing/cxl: Introduce a mock memory device + driver Dan Williams
2021-08-10 22:10 ` [PATCH 00/23] cxl_test: Enable CXL Topology and UAPI regression tests Ben Widawsky
2021-08-10 22:58   ` Dan Williams

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAPcyv4hD16UzKBDHDAbUwmTM01ycXcJCFmS8S11C4JmP1A8d6Q@mail.gmail.com \
    --to=dan.j.williams@intel.com \
    --cc=Jonathan.Cameron@huawei.com \
    --cc=alison.schofield@intel.com \
    --cc=ben.widawsky@intel.com \
    --cc=ira.weiny@intel.com \
    --cc=linux-cxl@vger.kernel.org \
    --cc=nvdimm@lists.linux.dev \
    --cc=vishal.l.verma@intel.com \
    --subject='Re: [PATCH 11/23] libnvdimm/labels: Introduce CXL labels' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).