From: Alison Schofield <alison.schofield@intel.com>
To: Richard Cheng <icheng@nvidia.com>
Cc: <jic23@kernel.org>, <dave@stgolabs.net>, <dave.jiang@intel.com>,
<vishal.l.verma@intel.com>, <iweiny@kernel.org>,
<ming.li@zohomail.com>, <kaihengf@nvidia.com>, <kobak@nvidia.com>,
<vaslot@nvidia.com>, <newtonl@nvidia.com>, <mochs@nvidia.com>,
<kristinc@nvidia.com>, <linux-cxl@vger.kernel.org>,
<linux-kernel@vger.kernel.org>, Dan Williams <djbw@kernel.org>,
Jonathan Cameron <Jonathan.Cameron@huawei.com>
Subject: Re: [PATCH v10 2/3] cxl/hdm: Allow zero sized HDM decoders
Date: Mon, 14 Sep 2026 19:16:41 -0700 [thread overview]
Message-ID: <aqiqied1QPHrDMKA@aschofie-mobl2.lan> (raw)
In-Reply-To: <20260914090858.19181-3-icheng@nvidia.com>
On Mon, Sep 14, 2026 at 05:08:57PM +0800, Richard Cheng wrote:
> CXL r4.0 §8.2.4.20.12 ("Committing Decoder Programming") and §14.13.10
> ("CXL HDM Decoder Zero Size Commit") permit committing an HDM decoder
> with size 0. BIOS may commit and lock such decoders so the OS cannot
> program regions through them, this is a design choice rather than a spec
> requirement.
>
> The kernel rejected these with -ENXIO during port enumeration and
> aborted the whole port, so affected systems showed nothing under "cxl
> list".
>
> Treat empty decoders as first class reservations. Back them with a
> separately allocated resource, since the resource tree cannot represent
> an empty range, and keep the skip and hdm_end accounting intact. Exclude
> empty decoders from region assembly and avoid zero-length poison queries.
Reviewed-by: Alison Schofield <alison.schofield@intel.com>
>
> Suggested-by: Dan Williams <djbw@kernel.org>
> Signed-off-by: Vishal Aslot <vaslot@nvidia.com>
> Signed-off-by: Richard Cheng <icheng@nvidia.com>
> Reviewed-by: Dan Williams <djbw@kernel.org>
> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
> ---
> Changelog:
>
> v9 -> v10:
> - Preserve -ENOMEM when allocation of the standalone zero-sized resource
> fails.
> - Retain the !cxled->dpa_res guard in cxl_dpa_free().
> - Reconstruct the commit message
>
> Best regards,
> Richard Cheng
> ---
> drivers/cxl/core/hdm.c | 58 +++++++++++++++++++++++++++------------
> drivers/cxl/core/mbox.c | 3 ++
> drivers/cxl/core/region.c | 45 ++++++++++++++++++++----------
> drivers/cxl/cxl.h | 10 +++++++
> drivers/cxl/port.c | 3 ++
> 5 files changed, 86 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> index 0c80b76a5f9b..39fe283cbc74 100644
> --- a/drivers/cxl/core/hdm.c
> +++ b/drivers/cxl/core/hdm.c
> @@ -240,6 +240,18 @@ static resource_size_t __adjust_skip(struct cxl_dev_state *cxlds,
> }
> #define release_skip(c, b, l) __adjust_skip((c), (b), (l), NULL)
>
> +static void cxl_dpa_release_region(struct resource *parent,
> + struct resource *res)
> +{
> + /* zero sized decoders are not tracked in the resource tree */
> + if (resource_size(res) == 0) {
> + kfree(res);
> + return;
> + }
> +
> + __release_region(parent, res->start, resource_size(res));
> +}
> +
> /*
> * Must be called in a context that synchronizes against this decoder's
> * port ->remove() callback (like an endpoint decoder sysfs attribute)
> @@ -256,7 +268,7 @@ static void __cxl_dpa_release(struct cxl_endpoint_decoder *cxled)
>
> /* save @skip_start, before @res is released */
> skip_start = res->start - cxled->skip;
> - __release_region(&cxlds->dpa_res, res->start, resource_size(res));
> + cxl_dpa_release_region(&cxlds->dpa_res, res);
> if (cxled->skip)
> release_skip(cxlds, skip_start, cxled->skip);
> cxled->skip = 0;
> @@ -336,6 +348,27 @@ static int request_skip(struct cxl_dev_state *cxlds,
> return -EBUSY;
> }
>
> +static struct resource *cxl_dpa_request_region(struct resource *parent,
> + resource_size_t start,
> + resource_size_t n,
> + const char *name)
> +{
> + struct resource *res;
> +
> + if (!n) {
> + res = kmalloc_obj(*res);
> + if (!res)
> + return ERR_PTR(-ENOMEM);
> +
> + *res = DEFINE_RES_NAMED(start, 0, name, IORESOURCE_MEM);
> +
> + return res;
> + }
> +
> + res = __request_region(parent, start, n, name, 0);
> + return res ?: ERR_PTR(-EBUSY);
> +}
> +
> static int __cxl_dpa_reserve(struct cxl_endpoint_decoder *cxled,
> resource_size_t base, resource_size_t len,
> resource_size_t skipped)
> @@ -349,12 +382,6 @@ static int __cxl_dpa_reserve(struct cxl_endpoint_decoder *cxled,
>
> lockdep_assert_held_write(&cxl_rwsem.dpa);
>
> - if (!len) {
> - dev_warn(dev, "decoder%d.%d: empty reservation attempted\n",
> - port->id, cxled->cxld.id);
> - return -EINVAL;
> - }
> -
> if (cxled->dpa_res) {
> dev_dbg(dev, "decoder%d.%d: existing allocation %pr assigned\n",
> port->id, cxled->cxld.id, cxled->dpa_res);
> @@ -378,14 +405,14 @@ static int __cxl_dpa_reserve(struct cxl_endpoint_decoder *cxled,
> if (rc)
> return rc;
> }
> - res = __request_region(&cxlds->dpa_res, base, len,
> - dev_name(&cxled->cxld.dev), 0);
> - if (!res) {
> + res = cxl_dpa_request_region(&cxlds->dpa_res, base, len,
> + dev_name(&cxled->cxld.dev));
> + if (IS_ERR(res)) {
> dev_dbg(dev, "decoder%d.%d: failed to reserve allocation\n",
> port->id, cxled->cxld.id);
> if (skipped)
> release_skip(cxlds, base - skipped, skipped);
> - return -EBUSY;
> + return PTR_ERR(res);
> }
> cxled->dpa_res = res;
> cxled->skip = skipped;
> @@ -402,7 +429,8 @@ static int __cxl_dpa_reserve(struct cxl_endpoint_decoder *cxled,
> break;
> }
>
> - if (cxled->part < 0)
> + /* Empty decoders may not be contained by a partition boundary */
> + if (cxled->part < 0 && resource_size(res))
> dev_warn(dev, "decoder%d.%d: %pr does not map any partition\n",
> port->id, cxled->cxld.id, res);
>
> @@ -1031,12 +1059,6 @@ static int init_hdm_decoder(struct cxl_port *port, struct cxl_decoder *cxld,
> return -ENXIO;
> }
>
> - if (size == 0) {
> - dev_warn(&port->dev,
> - "decoder%d.%d: Committed with zero size\n",
> - port->id, cxld->id);
> - return -ENXIO;
> - }
> port->commit_end = cxld->id;
> } else {
> if (cxled) {
> diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
> index 55828a836c01..1a2553332801 100644
> --- a/drivers/cxl/core/mbox.c
> +++ b/drivers/cxl/core/mbox.c
> @@ -1386,6 +1386,9 @@ int cxl_mem_get_poison(struct cxl_memdev *cxlmd, u64 offset, u64 len,
> int nr_records = 0;
> int rc;
>
> + if (!len)
> + return 0;
> +
> ACQUIRE(mutex_intr, lock)(&mds->poison.mutex);
> if ((rc = ACQUIRE_ERR(mutex_intr, &lock)))
> return rc;
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 677ebec8f48d..f54acbf68e84 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -2113,7 +2113,7 @@ static int cxl_region_attach(struct cxl_region *cxlr,
> return -ENXIO;
> }
>
> - if (!cxled->dpa_res) {
> + if (cxled_empty(cxled)) {
> dev_dbg(&cxlr->dev, "%s:%s: missing DPA allocation.\n",
> dev_name(&cxlmd->dev), dev_name(&cxled->cxld.dev));
> return -ENXIO;
> @@ -2967,24 +2967,31 @@ static int poison_by_decoder(struct device *dev, void *arg)
> if (!cxled->dpa_res)
> return 0;
>
> - cxlmd = cxled_to_memdev(cxled);
> - cxlds = cxlmd->cxlds;
> - mode = cxlds->part[cxled->part].mode;
> + /*
> + * Handle the degenerate case of a device with only empty decoders. An
> + * empty decoder can still map a non-zero skip range, so advance the
> + * walk to commit_end either way.
> + */
> + if (cxled->part >= 0) {
> + cxlmd = cxled_to_memdev(cxled);
> + cxlds = cxlmd->cxlds;
> + mode = cxlds->part[cxled->part].mode;
>
> - if (cxled->skip) {
> - offset = cxled->dpa_res->start - cxled->skip;
> - length = cxled->skip;
> - rc = cxl_mem_get_poison(cxlmd, offset, length, NULL);
> + if (cxled->skip) {
> + offset = cxled->dpa_res->start - cxled->skip;
> + length = cxled->skip;
> + rc = cxl_mem_get_poison(cxlmd, offset, length, NULL);
> + if (rc && !poison_efault_forgiven(rc, mode))
> + return rc;
> + }
> +
> + offset = cxled->dpa_res->start;
> + length = cxled->dpa_res->end - offset + 1;
> + rc = cxl_mem_get_poison(cxlmd, offset, length, cxled->cxld.region);
> if (rc && !poison_efault_forgiven(rc, mode))
> return rc;
> }
>
> - offset = cxled->dpa_res->start;
> - length = cxled->dpa_res->end - offset + 1;
> - rc = cxl_mem_get_poison(cxlmd, offset, length, cxled->cxld.region);
> - if (rc && !poison_efault_forgiven(rc, mode))
> - return rc;
> -
> /* Iterate until commit_end is reached */
> if (cxled->cxld.id == ctx->port->commit_end) {
> ctx->offset = cxled->dpa_res->end + 1;
> @@ -3006,9 +3013,17 @@ int cxl_get_poison_by_endpoint(struct cxl_port *port)
> };
>
> rc = device_for_each_child(&port->dev, &ctx, poison_by_decoder);
> - if (rc == 1)
> + if (rc == 1) {
> + /*
> + * No decoder with a sized DPA reservation was walked
> + * (every committed decoder is zero-size): scan all
> + * partitions in full.
> + */
> + if (ctx.part < 0)
> + ctx.part = 0;
> rc = cxl_get_poison_unmapped(to_cxl_memdev(port->uport_dev),
> &ctx);
> + }
>
> return rc;
> }
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index cab8ce39f465..3ef0810ab86b 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -336,6 +336,16 @@ struct cxl_endpoint_decoder {
> int pos;
> };
>
> +/*
> + * The common case is decoders with no reservation, but also handle
> + * decoders with a zero-sized reservation that firmware may install for
> + * security lockdown purposes.
> + */
> +static inline bool cxled_empty(struct cxl_endpoint_decoder *cxled)
> +{
> + return !cxled->dpa_res || !resource_size(cxled->dpa_res);
> +}
> +
> /**
> * struct cxl_switch_decoder - Switch specific CXL HDM Decoder
> * @cxld: base cxl_decoder object
> diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c
> index 99cf77b6b699..c12fd0b89883 100644
> --- a/drivers/cxl/port.c
> +++ b/drivers/cxl/port.c
> @@ -46,6 +46,9 @@ static int discover_region(struct device *dev, void *unused)
> if (cxled->state != CXL_DECODER_STATE_AUTO)
> return 0;
>
> + if (cxled_empty(cxled))
> + return 0;
> +
> /*
> * Region enumeration is opportunistic, if this add-event fails,
> * continue to the next endpoint decoder.
> --
> 2.43.0
>
next prev parent reply other threads:[~2026-09-15 2:16 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-14 9:08 [PATCH v10 0/3] Support zero-sized " Richard Cheng
2026-09-14 9:08 ` [PATCH v10 1/3] cxl/region: Simplify poison_by_decoder() error handling Richard Cheng
2026-09-15 23:33 ` Jonathan Cameron
2026-09-14 9:08 ` [PATCH v10 2/3] cxl/hdm: Allow zero sized HDM decoders Richard Cheng
2026-09-15 2:16 ` Alison Schofield [this message]
2026-09-14 9:08 ` [PATCH v10 3/3] tools/testing/cxl: Enable zero sized decoders under hb0 Richard Cheng
2026-09-15 16:01 ` [PATCH v10 0/3] Support zero-sized HDM decoders Dave Jiang
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=aqiqied1QPHrDMKA@aschofie-mobl2.lan \
--to=alison.schofield@intel.com \
--cc=Jonathan.Cameron@huawei.com \
--cc=dave.jiang@intel.com \
--cc=dave@stgolabs.net \
--cc=djbw@kernel.org \
--cc=icheng@nvidia.com \
--cc=iweiny@kernel.org \
--cc=jic23@kernel.org \
--cc=kaihengf@nvidia.com \
--cc=kobak@nvidia.com \
--cc=kristinc@nvidia.com \
--cc=linux-cxl@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=ming.li@zohomail.com \
--cc=mochs@nvidia.com \
--cc=newtonl@nvidia.com \
--cc=vaslot@nvidia.com \
--cc=vishal.l.verma@intel.com \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®