* [PATCH 10/19] cxl/memdev: Indicate probe deferral
2023-06-04 23:31 [PATCH 00/19] cxl: Device memory setup Dan Williams
@ 2023-06-04 23:32 ` Dan Williams
2023-06-06 13:54 ` Jonathan Cameron
0 siblings, 1 reply; 3+ messages in thread
From: Dan Williams @ 2023-06-04 23:32 UTC (permalink / raw)
To: linux-cxl; +Cc: ira.weiny, navneet.singh
The first stop for a CXL accelerator driver that wants to establish new
CXL.mem regions is to register a 'struct cxl_memdev'. That kicks off
cxl_mem_probe() to enumerate all 'struct cxl_port' instances in the
topology up to the root.
If the root driver has not attached yet the expectation is that the
driver waits until that link is established. The common cxl_pci driver
has reason to keep the 'struct cxl_memdev' device attached to the bus
until the root driver attaches. An accelerator may want to instead defer
probing until CXL resources can be acquired.
Use the @endpoint attribute of a 'struct cxl_memdev' to convey when
accelerator driver probing should be deferred vs failed. Provide that
indication via a new cxl_acquire_endpoint() API that can retrieve the
probe status of the memdev.
The first consumer of this API is a test driver that exercises the CXL
Type-2 flow.
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
drivers/cxl/core/memdev.c | 41 +++++++++++++++++++++++++++++++++++++++++
drivers/cxl/core/port.c | 2 +-
drivers/cxl/cxlmem.h | 2 ++
drivers/cxl/mem.c | 7 +++++--
4 files changed, 49 insertions(+), 3 deletions(-)
diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
index 65a685e5616f..859c43c340bb 100644
--- a/drivers/cxl/core/memdev.c
+++ b/drivers/cxl/core/memdev.c
@@ -609,6 +609,47 @@ struct cxl_memdev *devm_cxl_add_memdev(struct cxl_dev_state *cxlds)
}
EXPORT_SYMBOL_NS_GPL(devm_cxl_add_memdev, CXL);
+/*
+ * Try to get a locked reference on a memdev's CXL port topology
+ * connection. Be careful to observe when cxl_mem_probe() has deposited
+ * a probe deferral awaiting the arrival of the CXL root driver
+ */
+struct cxl_port *cxl_acquire_endpoint(struct cxl_memdev *cxlmd)
+{
+ struct cxl_port *endpoint;
+ int rc = -ENXIO;
+
+ device_lock(&cxlmd->dev);
+ endpoint = cxlmd->endpoint;
+ if (!endpoint)
+ goto err;
+
+ if (IS_ERR(endpoint)) {
+ rc = PTR_ERR(endpoint);
+ goto err;
+ }
+
+ device_lock(&endpoint->dev);
+ if (!endpoint->dev.driver)
+ goto err_endpoint;
+
+ return endpoint;
+
+err_endpoint:
+ device_unlock(&endpoint->dev);
+err:
+ device_unlock(&cxlmd->dev);
+ return ERR_PTR(rc);
+}
+EXPORT_SYMBOL_NS(cxl_acquire_endpoint, CXL);
+
+void cxl_release_endpoint(struct cxl_memdev *cxlmd, struct cxl_port *endpoint)
+{
+ device_unlock(&endpoint->dev);
+ device_unlock(&cxlmd->dev);
+}
+EXPORT_SYMBOL_NS(cxl_release_endpoint, CXL);
+
__init int cxl_memdev_init(void)
{
dev_t devt;
diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
index 6720ab22a494..5e21b53362e6 100644
--- a/drivers/cxl/core/port.c
+++ b/drivers/cxl/core/port.c
@@ -1336,7 +1336,7 @@ static int add_port_attach_ep(struct cxl_memdev *cxlmd,
*/
dev_dbg(&cxlmd->dev, "%s is a root dport\n",
dev_name(dport_dev));
- return -ENXIO;
+ return -EPROBE_DEFER;
}
parent_port = find_cxl_port(dparent, &parent_dport);
diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
index 7ee78e79933c..e3bcd6d12a1c 100644
--- a/drivers/cxl/cxlmem.h
+++ b/drivers/cxl/cxlmem.h
@@ -83,6 +83,8 @@ static inline bool is_cxl_endpoint(struct cxl_port *port)
return is_cxl_memdev(port->uport);
}
+struct cxl_port *cxl_acquire_endpoint(struct cxl_memdev *cxlmd);
+void cxl_release_endpoint(struct cxl_memdev *cxlmd, struct cxl_port *endpoint);
struct cxl_memdev *devm_cxl_add_memdev(struct cxl_dev_state *cxlds);
int devm_cxl_dpa_reserve(struct cxl_endpoint_decoder *cxled,
resource_size_t base, resource_size_t len,
diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
index 584f9eec57e4..2470c6f2621c 100644
--- a/drivers/cxl/mem.c
+++ b/drivers/cxl/mem.c
@@ -154,13 +154,16 @@ static int cxl_mem_probe(struct device *dev)
return rc;
rc = devm_cxl_enumerate_ports(cxlmd);
- if (rc)
+ if (rc) {
+ cxlmd->endpoint = ERR_PTR(rc);
return rc;
+ }
parent_port = cxl_mem_find_port(cxlmd, &dport);
if (!parent_port) {
dev_err(dev, "CXL port topology not found\n");
- return -ENXIO;
+ cxlmd->endpoint = ERR_PTR(-EPROBE_DEFER);
+ return -EPROBE_DEFER;
}
if (dport->rch)
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH 10/19] cxl/memdev: Indicate probe deferral
2023-06-04 23:32 ` [PATCH 10/19] cxl/memdev: Indicate probe deferral Dan Williams
@ 2023-06-06 13:54 ` Jonathan Cameron
0 siblings, 0 replies; 3+ messages in thread
From: Jonathan Cameron @ 2023-06-06 13:54 UTC (permalink / raw)
To: Dan Williams; +Cc: linux-cxl, ira.weiny, navneet.singh
On Sun, 04 Jun 2023 16:32:32 -0700
Dan Williams <dan.j.williams@intel.com> wrote:
> The first stop for a CXL accelerator driver that wants to establish new
> CXL.mem regions is to register a 'struct cxl_memdev'. That kicks off
> cxl_mem_probe() to enumerate all 'struct cxl_port' instances in the
> topology up to the root.
>
> If the root driver has not attached yet the expectation is that the
> driver waits until that link is established. The common cxl_pci driver
> has reason to keep the 'struct cxl_memdev' device attached to the bus
> until the root driver attaches. An accelerator may want to instead defer
> probing until CXL resources can be acquired.
>
> Use the @endpoint attribute of a 'struct cxl_memdev' to convey when
> accelerator driver probing should be deferred vs failed. Provide that
> indication via a new cxl_acquire_endpoint() API that can retrieve the
> probe status of the memdev.
>
> The first consumer of this API is a test driver that exercises the CXL
> Type-2 flow.
>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
> drivers/cxl/core/memdev.c | 41 +++++++++++++++++++++++++++++++++++++++++
> drivers/cxl/core/port.c | 2 +-
> drivers/cxl/cxlmem.h | 2 ++
> drivers/cxl/mem.c | 7 +++++--
> 4 files changed, 49 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
> index 65a685e5616f..859c43c340bb 100644
> --- a/drivers/cxl/core/memdev.c
> +++ b/drivers/cxl/core/memdev.c
> @@ -609,6 +609,47 @@ struct cxl_memdev *devm_cxl_add_memdev(struct cxl_dev_state *cxlds)
> }
> EXPORT_SYMBOL_NS_GPL(devm_cxl_add_memdev, CXL);
>
> +/*
> + * Try to get a locked reference on a memdev's CXL port topology
> + * connection. Be careful to observe when cxl_mem_probe() has deposited
> + * a probe deferral awaiting the arrival of the CXL root driver
> + */
> +struct cxl_port *cxl_acquire_endpoint(struct cxl_memdev *cxlmd)
> +{
> + struct cxl_port *endpoint;
> + int rc = -ENXIO;
> +
> + device_lock(&cxlmd->dev);
> + endpoint = cxlmd->endpoint;
> + if (!endpoint)
> + goto err;
> +
> + if (IS_ERR(endpoint)) {
> + rc = PTR_ERR(endpoint);
Whilst the above comment talks about deferred, there are paths where it's
potentially another error pointer currently. Maybe suppress those at
source so the comment is 'precise' or make the comment above incorporate
those other error pointers.
> + goto err;
> + }
> +
> + device_lock(&endpoint->dev);
> + if (!endpoint->dev.driver)
> + goto err_endpoint;
> +
> + return endpoint;
> +
> +err_endpoint:
> + device_unlock(&endpoint->dev);
> +err:
> + device_unlock(&cxlmd->dev);
> + return ERR_PTR(rc);
> +}
> +EXPORT_SYMBOL_NS(cxl_acquire_endpoint, CXL);
> +
> +void cxl_release_endpoint(struct cxl_memdev *cxlmd, struct cxl_port *endpoint)
> +{
> + device_unlock(&endpoint->dev);
> + device_unlock(&cxlmd->dev);
> +}
> +EXPORT_SYMBOL_NS(cxl_release_endpoint, CXL);
> +
> __init int cxl_memdev_init(void)
> {
> dev_t devt;
> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> index 6720ab22a494..5e21b53362e6 100644
> --- a/drivers/cxl/core/port.c
> +++ b/drivers/cxl/core/port.c
> @@ -1336,7 +1336,7 @@ static int add_port_attach_ep(struct cxl_memdev *cxlmd,
> */
> dev_dbg(&cxlmd->dev, "%s is a root dport\n",
> dev_name(dport_dev));
> - return -ENXIO;
> + return -EPROBE_DEFER;
> }
>
> parent_port = find_cxl_port(dparent, &parent_dport);
> diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
> index 7ee78e79933c..e3bcd6d12a1c 100644
> --- a/drivers/cxl/cxlmem.h
> +++ b/drivers/cxl/cxlmem.h
> @@ -83,6 +83,8 @@ static inline bool is_cxl_endpoint(struct cxl_port *port)
> return is_cxl_memdev(port->uport);
> }
>
> +struct cxl_port *cxl_acquire_endpoint(struct cxl_memdev *cxlmd);
> +void cxl_release_endpoint(struct cxl_memdev *cxlmd, struct cxl_port *endpoint);
> struct cxl_memdev *devm_cxl_add_memdev(struct cxl_dev_state *cxlds);
> int devm_cxl_dpa_reserve(struct cxl_endpoint_decoder *cxled,
> resource_size_t base, resource_size_t len,
> diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
> index 584f9eec57e4..2470c6f2621c 100644
> --- a/drivers/cxl/mem.c
> +++ b/drivers/cxl/mem.c
> @@ -154,13 +154,16 @@ static int cxl_mem_probe(struct device *dev)
> return rc;
>
> rc = devm_cxl_enumerate_ports(cxlmd);
> - if (rc)
> + if (rc) {
> + cxlmd->endpoint = ERR_PTR(rc);
This can be other errors than defer. Which seems inconsistent
for the above check.
> return rc;
> + }
>
> parent_port = cxl_mem_find_port(cxlmd, &dport);
> if (!parent_port) {
> dev_err(dev, "CXL port topology not found\n");
> - return -ENXIO;
> + cxlmd->endpoint = ERR_PTR(-EPROBE_DEFER);
> + return -EPROBE_DEFER;
This is a little inelegant as we aren't setting this in the same
function as were the endpoint is otherwise set up.
Partly that's a naming thing. In wonder if can pull out the bit
related to the endpoint from cxl_mem_probe() to a new
cxl_mem_ep_probe() at which case this will feel more logical.
That would be fine for this case, but the one above doesn't
really work for that...
Perhaps this is the best that can be done.
> }
>
> if (dport->rch)
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH 10/19] cxl/memdev: Indicate probe deferral
@ 2023-06-08 23:16 kernel test robot
0 siblings, 0 replies; 3+ messages in thread
From: kernel test robot @ 2023-06-08 23:16 UTC (permalink / raw)
To: oe-kbuild; +Cc: lkp, Dan Carpenter
BCC: lkp@intel.com
CC: oe-kbuild-all@lists.linux.dev
In-Reply-To: <168592155270.1948938.11536845108449547920.stgit@dwillia2-xfh.jf.intel.com>
References: <168592155270.1948938.11536845108449547920.stgit@dwillia2-xfh.jf.intel.com>
TO: Dan Williams <dan.j.williams@intel.com>
Hi Dan,
kernel test robot noticed the following build warnings:
[auto build test WARNING on 9561de3a55bed6bdd44a12820ba81ec416e705a7]
url: https://github.com/intel-lab-lkp/linux/commits/Dan-Williams/cxl-regs-Clarify-when-a-struct-cxl_register_map-is-input-vs-output/20230605-073402
base: 9561de3a55bed6bdd44a12820ba81ec416e705a7
patch link: https://lore.kernel.org/r/168592155270.1948938.11536845108449547920.stgit%40dwillia2-xfh.jf.intel.com
patch subject: [PATCH 10/19] cxl/memdev: Indicate probe deferral
:::::: branch date: 4 days ago
:::::: commit date: 4 days ago
config: i386-randconfig-m021-20230608 (https://download.01.org/0day-ci/archive/20230609/202306090732.SLD5PmoG-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>
| Closes: https://lore.kernel.org/r/202306090732.SLD5PmoG-lkp@intel.com/
smatch warnings:
drivers/cxl/core/memdev.c:642 cxl_acquire_endpoint() warn: inconsistent returns '&cxlmd->dev.mutex'.
vim +642 drivers/cxl/core/memdev.c
3d135db510240f Ben Widawsky 2021-08-02 611
126ed1e685e817 Dan Williams 2023-06-04 612 /*
126ed1e685e817 Dan Williams 2023-06-04 613 * Try to get a locked reference on a memdev's CXL port topology
126ed1e685e817 Dan Williams 2023-06-04 614 * connection. Be careful to observe when cxl_mem_probe() has deposited
126ed1e685e817 Dan Williams 2023-06-04 615 * a probe deferral awaiting the arrival of the CXL root driver
126ed1e685e817 Dan Williams 2023-06-04 616 */
126ed1e685e817 Dan Williams 2023-06-04 617 struct cxl_port *cxl_acquire_endpoint(struct cxl_memdev *cxlmd)
126ed1e685e817 Dan Williams 2023-06-04 618 {
126ed1e685e817 Dan Williams 2023-06-04 619 struct cxl_port *endpoint;
126ed1e685e817 Dan Williams 2023-06-04 620 int rc = -ENXIO;
126ed1e685e817 Dan Williams 2023-06-04 621
126ed1e685e817 Dan Williams 2023-06-04 622 device_lock(&cxlmd->dev);
126ed1e685e817 Dan Williams 2023-06-04 623 endpoint = cxlmd->endpoint;
126ed1e685e817 Dan Williams 2023-06-04 624 if (!endpoint)
126ed1e685e817 Dan Williams 2023-06-04 625 goto err;
126ed1e685e817 Dan Williams 2023-06-04 626
126ed1e685e817 Dan Williams 2023-06-04 627 if (IS_ERR(endpoint)) {
126ed1e685e817 Dan Williams 2023-06-04 628 rc = PTR_ERR(endpoint);
126ed1e685e817 Dan Williams 2023-06-04 629 goto err;
126ed1e685e817 Dan Williams 2023-06-04 630 }
126ed1e685e817 Dan Williams 2023-06-04 631
126ed1e685e817 Dan Williams 2023-06-04 632 device_lock(&endpoint->dev);
126ed1e685e817 Dan Williams 2023-06-04 633 if (!endpoint->dev.driver)
126ed1e685e817 Dan Williams 2023-06-04 634 goto err_endpoint;
126ed1e685e817 Dan Williams 2023-06-04 635
126ed1e685e817 Dan Williams 2023-06-04 636 return endpoint;
126ed1e685e817 Dan Williams 2023-06-04 637
126ed1e685e817 Dan Williams 2023-06-04 638 err_endpoint:
126ed1e685e817 Dan Williams 2023-06-04 639 device_unlock(&endpoint->dev);
126ed1e685e817 Dan Williams 2023-06-04 640 err:
126ed1e685e817 Dan Williams 2023-06-04 641 device_unlock(&cxlmd->dev);
126ed1e685e817 Dan Williams 2023-06-04 @642 return ERR_PTR(rc);
126ed1e685e817 Dan Williams 2023-06-04 643 }
126ed1e685e817 Dan Williams 2023-06-04 644 EXPORT_SYMBOL_NS(cxl_acquire_endpoint, CXL);
126ed1e685e817 Dan Williams 2023-06-04 645
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-06-08 23:17 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-06-08 23:16 [PATCH 10/19] cxl/memdev: Indicate probe deferral kernel test robot
-- strict thread matches above, loose matches on Subject: below --
2023-06-04 23:31 [PATCH 00/19] cxl: Device memory setup Dan Williams
2023-06-04 23:32 ` [PATCH 10/19] cxl/memdev: Indicate probe deferral Dan Williams
2023-06-06 13:54 ` Jonathan Cameron
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.