[v3,24/28] tools/testing/cxl: Introduce a mocked-up CXL port hierarchy

Commit Message

Dan Williams Aug. 24, 2021, 4:07 p.m. UTC

Create an environment for CXL plumbing unit tests. Especially when it
comes to an algorithm for HDM Decoder (Host-managed Device Memory
Decoder) programming, the availability of an in-kernel-tree emulation
environment for CXL configuration complexity and corner cases speeds
development and deters regressions.

The approach taken mirrors what was done for tools/testing/nvdimm/. I.e.
an external module, cxl_test.ko built out of the tools/testing/cxl/
directory, provides mock implementations of kernel APIs and kernel
objects to simulate a real world device hierarchy.

One feedback for the tools/testing/nvdimm/ proposal was "why not do this
in QEMU?". In fact, the CXL development community has developed a QEMU
model for CXL [1]. However, there are a few blocking issues that keep
QEMU from being a tight fit for topology + provisioning unit tests:

1/ The QEMU community has yet to show interest in merging any of this
   support that has had patches on the list since November 2020. So,
   testing CXL to date involves building custom QEMU with out-of-tree
   patches.

2/ CXL mechanisms like cross-host-bridge interleave do not have a clear
   path to be emulated by QEMU without major infrastructure work. This
   is easier to achieve with the alloc_mock_res() approach taken in this
   patch to shortcut-define emulated system physical address ranges with
   interleave behavior.

The QEMU enabling has been critical to get the driver off the ground,
and may still move forward, but it does not address the ongoing needs of
a regression testing environment and test driven development.

This patch adds an ACPI CXL Platform definition with emulated CXL
multi-ported host-bridges. A follow on patch adds emulated memory
expander devices.

Acked-by: Ben Widawsky <[e-mail]>
Reported-by: Vishal Verma <[e-mail]>
Link: https://lore.kernel.org/r/[e-mail] [1]
Signed-off-by: Dan Williams <[e-mail]>
---
 drivers/cxl/acpi.c               |   52 ++--
 drivers/cxl/cxl.h                |   13 +
 tools/testing/cxl/Kbuild         |   36 +++
 tools/testing/cxl/config_check.c |   13 +
 tools/testing/cxl/mock_acpi.c    |  109 ++++++++
 tools/testing/cxl/test/Kbuild    |    6 
 tools/testing/cxl/test/cxl.c     |  516 ++++++++++++++++++++++++++++++++++++++
 tools/testing/cxl/test/mock.c    |  171 +++++++++++++
 tools/testing/cxl/test/mock.h    |   27 ++
 9 files changed, 922 insertions(+), 21 deletions(-)
 create mode 100644 tools/testing/cxl/Kbuild
 create mode 100644 tools/testing/cxl/config_check.c
 create mode 100644 tools/testing/cxl/mock_acpi.c
 create mode 100644 tools/testing/cxl/test/Kbuild
 create mode 100644 tools/testing/cxl/test/cxl.c
 create mode 100644 tools/testing/cxl/test/mock.c
 create mode 100644 tools/testing/cxl/test/mock.h

Comments

Jonathan Cameron Sept. 3, 2021, 12:52 p.m. UTC | #1

On Tue, 24 Aug 2021 09:07:33 -0700
Dan Williams <[e-mail]> wrote:

> Create an environment for CXL plumbing unit tests. Especially when it
> comes to an algorithm for HDM Decoder (Host-managed Device Memory
> Decoder) programming, the availability of an in-kernel-tree emulation
> environment for CXL configuration complexity and corner cases speeds
> development and deters regressions.
> 
> The approach taken mirrors what was done for tools/testing/nvdimm/. I.e.
> an external module, cxl_test.ko built out of the tools/testing/cxl/
> directory, provides mock implementations of kernel APIs and kernel
> objects to simulate a real world device hierarchy.
> 
> One feedback for the tools/testing/nvdimm/ proposal was "why not do this
> in QEMU?". In fact, the CXL development community has developed a QEMU
> model for CXL [1]. However, there are a few blocking issues that keep
> QEMU from being a tight fit for topology + provisioning unit tests:
> 
> 1/ The QEMU community has yet to show interest in merging any of this
>    support that has had patches on the list since November 2020. So,
>    testing CXL to date involves building custom QEMU with out-of-tree
>    patches.

That's a separate discussion I've been meaning to kick off. I'd like
to get that moving because there are various things we can do there
which can't necessarily be done with this approach or at least are easier
done in QEMU. I'll raise it on the qemu list and drag a few people in
who might be able to help us get things moving + help find solutions to
the bits that we can't currently do.

> 
> 2/ CXL mechanisms like cross-host-bridge interleave do not have a clear
>    path to be emulated by QEMU without major infrastructure work. This
>    is easier to achieve with the alloc_mock_res() approach taken in this
>    patch to shortcut-define emulated system physical address ranges with
>    interleave behavior.
> 
> The QEMU enabling has been critical to get the driver off the ground,
> and may still move forward, but it does not address the ongoing needs of
> a regression testing environment and test driven development.

Different purposes, so I would see having both as beneficial
(in principle - I haven't played with this yet :)

> 
> This patch adds an ACPI CXL Platform definition with emulated CXL
> multi-ported host-bridges. A follow on patch adds emulated memory
> expander devices.
> 
> Acked-by: Ben Widawsky <[e-mail]>
> Reported-by: Vishal Verma <[e-mail]>
> Link: https://lore.kernel.org/r/[e-mail] [1]
> Signed-off-by: Dan Williams <[e-mail]>
> ---
>  drivers/cxl/acpi.c               |   52 ++--
>  drivers/cxl/cxl.h                |   13 +
>  tools/testing/cxl/Kbuild         |   36 +++
>  tools/testing/cxl/config_check.c |   13 +
>  tools/testing/cxl/mock_acpi.c    |  109 ++++++++
>  tools/testing/cxl/test/Kbuild    |    6 
>  tools/testing/cxl/test/cxl.c     |  516 ++++++++++++++++++++++++++++++++++++++
>  tools/testing/cxl/test/mock.c    |  171 +++++++++++++
>  tools/testing/cxl/test/mock.h    |   27 ++
>  9 files changed, 922 insertions(+), 21 deletions(-)
>  create mode 100644 tools/testing/cxl/Kbuild
>  create mode 100644 tools/testing/cxl/config_check.c
>  create mode 100644 tools/testing/cxl/mock_acpi.c
>  create mode 100644 tools/testing/cxl/test/Kbuild
>  create mode 100644 tools/testing/cxl/test/cxl.c
>  create mode 100644 tools/testing/cxl/test/mock.c
>  create mode 100644 tools/testing/cxl/test/mock.h
> 
> diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
> index 2d8f1ec1abff..3fcb704c647f 100644
> --- a/drivers/cxl/acpi.c
> +++ b/drivers/cxl/acpi.c
> @@ -182,15 +182,7 @@ static resource_size_t get_chbcr(struct acpi_cedt_chbs *chbs)
>  	return IS_ERR(chbs) ? CXL_RESOURCE_NONE : chbs->base;
>  }

> -static int match_add_root_ports(struct pci_dev *pdev, void *data)
> +__mock int match_add_root_ports(struct pci_dev *pdev, void *data)
>  {
>  	struct cxl_walk_context *ctx = data;
>  	struct pci_bus *root_bus = ctx->root;
> @@ -214,6 +206,8 @@ static int match_add_root_ports(struct pci_dev *pdev, void *data)
>  	port_num = FIELD_GET(PCI_EXP_LNKCAP_PN, lnkcap);
>  	rc = cxl_add_dport(port, &pdev->dev, port_num, CXL_RESOURCE_NONE);
>  	if (rc) {
> +		dev_err(dev, "failed to add dport: %s (%d)\n",
> +			dev_name(&pdev->dev), rc);

*grumble*  This extra debug is reasonable but not much to do with the
functionality of this patch.  So should have been separate to cut down on noise
here.


>  		ctx->error = rc;
>  		return rc;
>  	}
> @@ -239,15 +233,18 @@ static struct cxl_dport *find_dport_by_dev(struct cxl_port *port, struct device
>  	return NULL;
>  }
>  
> -static struct acpi_device *to_cxl_host_bridge(struct device *dev)
> +__mock struct acpi_device *to_cxl_host_bridge(struct device *host,
> +					      struct device *dev)
>  {
>  	struct acpi_device *adev = to_acpi_device(dev);
>  
>  	if (!acpi_pci_find_root(adev->handle))
>  		return NULL;
>  
> -	if (strcmp(acpi_device_hid(adev), "ACPI0016") == 0)
> +	if (strcmp(acpi_device_hid(adev), "ACPI0016") == 0) {
> +		dev_dbg(host, "found host bridge %s\n", dev_name(&adev->dev));
>  		return adev;
> +	}
>  	return NULL;
>  }
>  
> @@ -257,14 +254,14 @@ static struct acpi_device *to_cxl_host_bridge(struct device *dev)
>   */
>  static int add_host_bridge_uport(struct device *match, void *arg)
>  {
> -	struct acpi_device *bridge = to_cxl_host_bridge(match);
> +	struct cxl_port *port;
> +	struct cxl_dport *dport;
> +	struct cxl_decoder *cxld;
> +	struct cxl_walk_context ctx;
> +	struct acpi_pci_root *pci_root;
>  	struct cxl_port *root_port = arg;
>  	struct device *host = root_port->dev.parent;
> -	struct acpi_pci_root *pci_root;
> -	struct cxl_walk_context ctx;
> -	struct cxl_decoder *cxld;
> -	struct cxl_dport *dport;
> -	struct cxl_port *port;

Going from reverse xmas tree (almost) to xmas tree in a patch doing something else?
 
> +	struct acpi_device *bridge = to_cxl_host_bridge(host, match);
>  
>  	if (!bridge)
>  		return 0;
> @@ -319,7 +316,7 @@ static int add_host_bridge_dport(struct device *match, void *arg)
>  	struct acpi_cedt_chbs *chbs;
>  	struct cxl_port *root_port = arg;
>  	struct device *host = root_port->dev.parent;
> -	struct acpi_device *bridge = to_cxl_host_bridge(match);
> +	struct acpi_device *bridge = to_cxl_host_bridge(host, match);
>  
>  	if (!bridge)
>  		return 0;
> @@ -371,6 +368,17 @@ static int add_root_nvdimm_bridge(struct device *match, void *data)
>  	return 1;
>  }
>  
...


> diff --git a/tools/testing/cxl/mock_acpi.c b/tools/testing/cxl/mock_acpi.c
> new file mode 100644
> index 000000000000..4c8a493ace56
> --- /dev/null
> +++ b/tools/testing/cxl/mock_acpi.c
> @@ -0,0 +1,109 @@

> +static int match_add_root_port(struct pci_dev *pdev, void *data)

Hmm. Nice not to duplicate this code, but I guess a bit tricky to
work around.  Maybe a comment next to the 'main' version so we
remember to update this one as well if it is changed?

> +{
> +	struct cxl_walk_context *ctx = data;
> +	struct pci_bus *root_bus = ctx->root;
> +	struct cxl_port *port = ctx->port;
> +	int type = pci_pcie_type(pdev);
> +	struct device *dev = ctx->dev;
> +	u32 lnkcap, port_num;
> +	int rc;
> +
> +	if (pdev->bus != root_bus)
> +		return 0;
> +	if (!pci_is_pcie(pdev))
> +		return 0;
> +	if (type != PCI_EXP_TYPE_ROOT_PORT)
> +		return 0;
> +	if (pci_read_config_dword(pdev, pci_pcie_cap(pdev) + PCI_EXP_LNKCAP,
> +				  &lnkcap) != PCIBIOS_SUCCESSFUL)
> +		return 0;
> +
> +	/* TODO walk DVSEC to find component register base */
> +	port_num = FIELD_GET(PCI_EXP_LNKCAP_PN, lnkcap);
> +	rc = cxl_add_dport(port, &pdev->dev, port_num, CXL_RESOURCE_NONE);
> +	if (rc) {
> +		dev_err(dev, "failed to add dport: %s (%d)\n",
> +			dev_name(&pdev->dev), rc);
> +		ctx->error = rc;
> +		return rc;
> +	}
> +	ctx->count++;
> +
> +	dev_dbg(dev, "add dport%d: %s\n", port_num, dev_name(&pdev->dev));
> +
> +	return 0;
> +}
> +
> +static int mock_add_root_port(struct platform_device *pdev, void *data)
> +{
> +	struct cxl_walk_context *ctx = data;
> +	struct cxl_port *port = ctx->port;
> +	struct device *dev = ctx->dev;
> +	int rc;
> +
> +	rc = cxl_add_dport(port, &pdev->dev, pdev->id, CXL_RESOURCE_NONE);
> +	if (rc) {
> +		dev_err(dev, "failed to add dport: %s (%d)\n",
> +			dev_name(&pdev->dev), rc);
> +		ctx->error = rc;
> +		return rc;
> +	}
> +	ctx->count++;
> +
> +	dev_dbg(dev, "add dport%d: %s\n", pdev->id, dev_name(&pdev->dev));
> +
> +	return 0;
> +}
> +
> +int match_add_root_ports(struct pci_dev *dev, void *data)
> +{
> +	int index, rc;
> +	struct cxl_mock_ops *ops = get_cxl_mock_ops(&index);
> +	struct platform_device *pdev = (struct platform_device *) dev;
> +
> +	if (ops && ops->is_mock_port(pdev))
> +		rc = mock_add_root_port(pdev, data);
> +	else
> +		rc = match_add_root_port(dev, data);
> +
> +	put_cxl_mock_ops(index);
> +
> +	return rc;
> +}

...

> diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c
> new file mode 100644
> index 000000000000..0710e0062e58
> --- /dev/null
> +++ b/tools/testing/cxl/test/cxl.c
> @@ -0,0 +1,516 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +// Copyright(c) 2021 Intel Corporation. All rights reserved.
> +
> +#include <linux/platform_device.h>
> +#include <linux/genalloc.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/acpi.h>
> +#include <linux/pci.h>
> +#include <linux/mm.h>
> +#include "mock.h"
> +
> +#define NR_CXL_HOST_BRIDGES 4
> +#define NR_CXL_ROOT_PORTS 2
> +
> +static struct platform_device *cxl_acpi;
> +static struct platform_device *cxl_host_bridge[NR_CXL_HOST_BRIDGES];
> +static struct platform_device
> +	*cxl_root_port[NR_CXL_HOST_BRIDGES * NR_CXL_ROOT_PORTS];
> +
> +static struct acpi_device acpi0017_mock;
> +static struct acpi_device host_bridge[NR_CXL_HOST_BRIDGES] = {
> +	[0] = {
> +		.handle = &host_bridge[0],
> +	},
> +	[1] = {
> +		.handle = &host_bridge[1],
> +	},
> +	[2] = {
> +		.handle = &host_bridge[2],
> +	},
> +	[3] = {
> +		.handle = &host_bridge[3],
> +	},
> +};
> +
> +static bool is_mock_dev(struct device *dev)
> +{

You be paranoid and check acxl_acpi here but I guess
its always safe...

> +	if (dev == &cxl_acpi->dev)
> +		return true;
> +	return false;
> +}
> +

...

> +
> +struct cxl_mock_res {
> +	struct list_head list;
> +	struct range range;
> +};
> +
> +static LIST_HEAD(mock_res);
> +static DEFINE_MUTEX(mock_res_lock);
> +static struct gen_pool *cxl_mock_pool;
> +
> +static void free_mock_res(void)
Seems like a missmatch in naming.
free_mock_res() frees all resouces,
alloc_moc_res() allocates one.


> +{
> +	struct cxl_mock_res *res, *_res;
> +
> +	mutex_lock(&mock_res_lock);
> +	list_for_each_entry_safe(res, _res, &mock_res, list) {
> +		gen_pool_free(cxl_mock_pool, res->range.start,
> +			      range_len(&res->range));
> +		list_del(&res->list);
> +		kfree(res);
> +	}
> +	mutex_unlock(&mock_res_lock);
> +}
> +
> +static struct cxl_mock_res *alloc_mock_res(resource_size_t size)
> +{
> +	struct cxl_mock_res *res = kzalloc(sizeof(*res), GFP_KERNEL);
> +	struct genpool_data_align data = {
> +		.align = SZ_256M,
> +	};
> +	unsigned long phys;
> +
> +	INIT_LIST_HEAD(&res->list);
> +	phys = gen_pool_alloc_algo(cxl_mock_pool, size,
> +				   gen_pool_first_fit_align, &data);
> +	if (!phys)
> +		return NULL;
> +
> +	res->range = (struct range) {
> +		.start = phys,
> +		.end = phys + size - 1,
> +	};
> +	mutex_lock(&mock_res_lock);
> +	list_add(&res->list, &mock_res);
> +	mutex_unlock(&mock_res_lock);
> +
> +	return res;
> +}
> +
> +static int populate_cedt(void)
> +{
> +	struct acpi_cedt_cfmws *cfmws[4] = {
> +		[0] = &mock_cedt.cfmws0.cfmws,
> +		[1] = &mock_cedt.cfmws1.cfmws,
> +		[2] = &mock_cedt.cfmws2.cfmws,
> +		[3] = &mock_cedt.cfmws3.cfmws,
> +	};
> +	struct cxl_mock_res *res;
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(mock_cedt.chbs); i++) {
> +		struct acpi_cedt_chbs *chbs = &mock_cedt.chbs[i];
> +		resource_size_t size;
> +
> +		if (chbs->cxl_version == ACPI_CEDT_CHBS_VERSION_CXL20)
> +			size = ACPI_CEDT_CHBS_LENGTH_CXL20;
> +		else
> +			size = ACPI_CEDT_CHBS_LENGTH_CXL11;
> +
> +		res = alloc_mock_res(size);
> +		if (!res)
> +			return -ENOMEM;
> +		chbs->base = res->range.start;
> +		chbs->length = size;
> +	}
> +
> +	for (i = 0; i < ARRAY_SIZE(cfmws); i++) {
> +		struct acpi_cedt_cfmws *window = cfmws[i];
> +		int ways = 1 << window->interleave_ways;
> +
> +		res = alloc_mock_res(SZ_256M * ways);

why that size?  Should take window_size into account I think..

> +		if (!res)
> +			return -ENOMEM;
> +		window->base_hpa = res->range.start;
> +		window->window_size = range_len(&res->range);
> +	}
> +
> +	return 0;
> +}
> +


...

> +
> +static struct cxl_mock_ops cxl_mock_ops = {
> +	.is_mock_adev = is_mock_adev,
> +	.is_mock_bridge = is_mock_bridge,
> +	.is_mock_bus = is_mock_bus,
> +	.is_mock_port = is_mock_port,
> +	.is_mock_dev = is_mock_dev,
> +	.mock_port = mock_cxl_root_port,
> +	.acpi_get_table = mock_acpi_get_table,
> +	.acpi_put_table = mock_acpi_put_table,
> +	.acpi_evaluate_integer = mock_acpi_evaluate_integer,
> +	.acpi_pci_find_root = mock_acpi_pci_find_root,
> +	.list = LIST_HEAD_INIT(cxl_mock_ops.list),
> +};

...

> +#ifndef SZ_64G
> +#define SZ_64G (SZ_32G * 2)
> +#endif
> +
> +#ifndef SZ_512G
> +#define SZ_512G (SZ_64G * 8)
> +#endif

Why not add to sizes.h?

> +
> +static __init int cxl_test_init(void)
> +{
> +	int rc, i;
> +
> +	register_cxl_mock_ops(&cxl_mock_ops);
> +
> +	cxl_mock_pool = gen_pool_create(ilog2(SZ_2M), NUMA_NO_NODE);
> +	if (!cxl_mock_pool) {
> +		rc = -ENOMEM;
> +		goto err_gen_pool_create;
> +	}
> +
> +	rc = gen_pool_add(cxl_mock_pool, SZ_512G, SZ_64G, NUMA_NO_NODE);
> +	if (rc)
> +		goto err_gen_pool_add;
> +
> +	rc = populate_cedt();
> +	if (rc)
> +		goto err_populate;
> +
> +	for (i = 0; i < ARRAY_SIZE(cxl_host_bridge); i++) {
> +		struct acpi_device *adev = &host_bridge[i];
> +		struct platform_device *pdev;
> +
> +		pdev = platform_device_alloc("cxl_host_bridge", i);
> +		if (!pdev)
> +			goto err_bridge;
> +
> +		mock_companion(adev, &pdev->dev);
> +		rc = platform_device_add(pdev);
> +		if (rc) {
> +			platform_device_put(pdev);
> +			goto err_bridge;
> +		}
> +		cxl_host_bridge[i] = pdev;
> +	}
> +
> +	for (i = 0; i < ARRAY_SIZE(cxl_root_port); i++) {
> +		struct platform_device *bridge =
> +			cxl_host_bridge[i / NR_CXL_ROOT_PORTS];
> +		struct platform_device *pdev;
> +
> +		pdev = platform_device_alloc("cxl_root_port", i);
> +		if (!pdev)
> +			goto err_port;
> +		pdev->dev.parent = &bridge->dev;
> +
> +		rc = platform_device_add(pdev);
> +		if (rc) {
> +			platform_device_put(pdev);
> +			goto err_port;
> +		}
> +		cxl_root_port[i] = pdev;
> +	}
> +
> +	cxl_acpi = platform_device_alloc("cxl_acpi", 0);
> +	if (!cxl_acpi)
> +		goto err_port;
> +
> +	mock_companion(&acpi0017_mock, &cxl_acpi->dev);
> +	acpi0017_mock.dev.bus = &platform_bus_type;
> +
> +	rc = platform_device_add(cxl_acpi);
> +	if (rc)
> +		goto err_add;
> +
> +	return 0;
> +
> +err_add:
> +	platform_device_put(cxl_acpi);
> +err_port:
> +	for (i = ARRAY_SIZE(cxl_root_port) - 1; i >= 0; i--) {
> +		platform_device_del(cxl_root_port[i]);
> +		platform_device_put(cxl_root_port[i]);
> +	}
> +err_bridge:
> +	for (i = ARRAY_SIZE(cxl_host_bridge) - 1; i >= 0; i--) {
> +		platform_device_del(cxl_host_bridge[i]);
> +		platform_device_put(cxl_host_bridge[i]);
> +	}
> +err_populate:
> +	free_mock_res();

Might be worth a function wrapping this that makes it clear this
is unwinding what happened in populate_cedt()

> +err_gen_pool_add:
> +	gen_pool_destroy(cxl_mock_pool);
> +err_gen_pool_create:
> +	unregister_cxl_mock_ops(&cxl_mock_ops);
> +	return rc;
> +}
> +
> +static __exit void cxl_test_exit(void)
> +{
> +	int i;
> +
> +	platform_device_del(cxl_acpi);
> +	platform_device_put(cxl_acpi);

Given the evil warning comments about platform_device_del() in platform.c
about it only being appropriate to call it in error cases...

Perhaps it's better to call platform_device_unregister() even if
that looks locally less obvious?  Or maybe we should suggest
the warning comments are more refined in what usage to rule out!

> +	for (i = ARRAY_SIZE(cxl_root_port) - 1; i >= 0; i--) {
> +		platform_device_del(cxl_root_port[i]);
> +		platform_device_put(cxl_root_port[i]);
> +	}
> +	for (i = ARRAY_SIZE(cxl_host_bridge) - 1; i >= 0; i--) {
> +		platform_device_del(cxl_host_bridge[i]);
> +		platform_device_put(cxl_host_bridge[i]);
> +	}
> +	free_mock_res();
> +	gen_pool_destroy(cxl_mock_pool);
> +	unregister_cxl_mock_ops(&cxl_mock_ops);
> +}
> +
> +module_init(cxl_test_init);
> +module_exit(cxl_test_exit);
> +MODULE_LICENSE("GPL v2");

...

Thanks,

Jonathan

Dan Williams Sept. 3, 2021, 9:49 p.m. UTC | #2

On Fri, Sep 3, 2021 at 5:53 AM Jonathan Cameron
<[e-mail]> wrote:
>
> On Tue, 24 Aug 2021 09:07:33 -0700
> Dan Williams <[e-mail]> wrote:
>
> > Create an environment for CXL plumbing unit tests. Especially when it
> > comes to an algorithm for HDM Decoder (Host-managed Device Memory
> > Decoder) programming, the availability of an in-kernel-tree emulation
> > environment for CXL configuration complexity and corner cases speeds
> > development and deters regressions.
> >
> > The approach taken mirrors what was done for tools/testing/nvdimm/. I.e.
> > an external module, cxl_test.ko built out of the tools/testing/cxl/
> > directory, provides mock implementations of kernel APIs and kernel
> > objects to simulate a real world device hierarchy.
> >
> > One feedback for the tools/testing/nvdimm/ proposal was "why not do this
> > in QEMU?". In fact, the CXL development community has developed a QEMU
> > model for CXL [1]. However, there are a few blocking issues that keep
> > QEMU from being a tight fit for topology + provisioning unit tests:
> >
> > 1/ The QEMU community has yet to show interest in merging any of this
> >    support that has had patches on the list since November 2020. So,
> >    testing CXL to date involves building custom QEMU with out-of-tree
> >    patches.
>
> That's a separate discussion I've been meaning to kick off. I'd like
> to get that moving because there are various things we can do there
> which can't necessarily be done with this approach or at least are easier
> done in QEMU. I'll raise it on the qemu list and drag a few people in
> who might be able to help us get things moving + help find solutions to
> the bits that we can't currently do.
>
> >
> > 2/ CXL mechanisms like cross-host-bridge interleave do not have a clear
> >    path to be emulated by QEMU without major infrastructure work. This
> >    is easier to achieve with the alloc_mock_res() approach taken in this
> >    patch to shortcut-define emulated system physical address ranges with
> >    interleave behavior.
> >
> > The QEMU enabling has been critical to get the driver off the ground,
> > and may still move forward, but it does not address the ongoing needs of
> > a regression testing environment and test driven development.
>
> Different purposes, so I would see having both as beneficial

Oh certainly, especially because cxl_test skips all the PCI details.
This regression environment is mainly for user space ABI regressions
and the PCI agnostic machinery in the subsystem. I'd love for the QEMU
work to move forward.

> (in principle - I haven't played with this yet :)

I have wondered if having a version of DOE emulation in tools/testing/
makes regression testing those protocols easier, but again that's PCI
details where QEMU is more suitable.

>
> >
> > This patch adds an ACPI CXL Platform definition with emulated CXL
> > multi-ported host-bridges. A follow on patch adds emulated memory
> > expander devices.
> >
> > Acked-by: Ben Widawsky <[e-mail]>
> > Reported-by: Vishal Verma <[e-mail]>
> > Link: https://lore.kernel.org/r/[e-mail] [1]
> > Signed-off-by: Dan Williams <[e-mail]>
> > ---
> >  drivers/cxl/acpi.c               |   52 ++--
> >  drivers/cxl/cxl.h                |   13 +
> >  tools/testing/cxl/Kbuild         |   36 +++
> >  tools/testing/cxl/config_check.c |   13 +
> >  tools/testing/cxl/mock_acpi.c    |  109 ++++++++
> >  tools/testing/cxl/test/Kbuild    |    6
> >  tools/testing/cxl/test/cxl.c     |  516 ++++++++++++++++++++++++++++++++++++++
> >  tools/testing/cxl/test/mock.c    |  171 +++++++++++++
> >  tools/testing/cxl/test/mock.h    |   27 ++
> >  9 files changed, 922 insertions(+), 21 deletions(-)
> >  create mode 100644 tools/testing/cxl/Kbuild
> >  create mode 100644 tools/testing/cxl/config_check.c
> >  create mode 100644 tools/testing/cxl/mock_acpi.c
> >  create mode 100644 tools/testing/cxl/test/Kbuild
> >  create mode 100644 tools/testing/cxl/test/cxl.c
> >  create mode 100644 tools/testing/cxl/test/mock.c
> >  create mode 100644 tools/testing/cxl/test/mock.h
> >
> > diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
> > index 2d8f1ec1abff..3fcb704c647f 100644
> > --- a/drivers/cxl/acpi.c
> > +++ b/drivers/cxl/acpi.c
> > @@ -182,15 +182,7 @@ static resource_size_t get_chbcr(struct acpi_cedt_chbs *chbs)
> >       return IS_ERR(chbs) ? CXL_RESOURCE_NONE : chbs->base;
> >  }
>
> > -static int match_add_root_ports(struct pci_dev *pdev, void *data)
> > +__mock int match_add_root_ports(struct pci_dev *pdev, void *data)
> >  {
> >       struct cxl_walk_context *ctx = data;
> >       struct pci_bus *root_bus = ctx->root;
> > @@ -214,6 +206,8 @@ static int match_add_root_ports(struct pci_dev *pdev, void *data)
> >       port_num = FIELD_GET(PCI_EXP_LNKCAP_PN, lnkcap);
> >       rc = cxl_add_dport(port, &pdev->dev, port_num, CXL_RESOURCE_NONE);
> >       if (rc) {
> > +             dev_err(dev, "failed to add dport: %s (%d)\n",
> > +                     dev_name(&pdev->dev), rc);
>
> *grumble*  This extra debug is reasonable but not much to do with the
> functionality of this patch.  So should have been separate to cut down on noise
> here.

True, can remove for now.

>
>
> >               ctx->error = rc;
> >               return rc;
> >       }
> > @@ -239,15 +233,18 @@ static struct cxl_dport *find_dport_by_dev(struct cxl_port *port, struct device
> >       return NULL;
> >  }
> >
> > -static struct acpi_device *to_cxl_host_bridge(struct device *dev)
> > +__mock struct acpi_device *to_cxl_host_bridge(struct device *host,
> > +                                           struct device *dev)
> >  {
> >       struct acpi_device *adev = to_acpi_device(dev);
> >
> >       if (!acpi_pci_find_root(adev->handle))
> >               return NULL;
> >
> > -     if (strcmp(acpi_device_hid(adev), "ACPI0016") == 0)
> > +     if (strcmp(acpi_device_hid(adev), "ACPI0016") == 0) {
> > +             dev_dbg(host, "found host bridge %s\n", dev_name(&adev->dev));
> >               return adev;
> > +     }
> >       return NULL;
> >  }
> >
> > @@ -257,14 +254,14 @@ static struct acpi_device *to_cxl_host_bridge(struct device *dev)
> >   */
> >  static int add_host_bridge_uport(struct device *match, void *arg)
> >  {
> > -     struct acpi_device *bridge = to_cxl_host_bridge(match);
> > +     struct cxl_port *port;
> > +     struct cxl_dport *dport;
> > +     struct cxl_decoder *cxld;
> > +     struct cxl_walk_context ctx;
> > +     struct acpi_pci_root *pci_root;
> >       struct cxl_port *root_port = arg;
> >       struct device *host = root_port->dev.parent;
> > -     struct acpi_pci_root *pci_root;
> > -     struct cxl_walk_context ctx;
> > -     struct cxl_decoder *cxld;
> > -     struct cxl_dport *dport;
> > -     struct cxl_port *port;
>
> Going from reverse xmas tree (almost) to xmas tree in a patch doing something else?

I can leave it "untreed" for now.

>
> > +     struct acpi_device *bridge = to_cxl_host_bridge(host, match);
> >
> >       if (!bridge)
> >               return 0;
> > @@ -319,7 +316,7 @@ static int add_host_bridge_dport(struct device *match, void *arg)
> >       struct acpi_cedt_chbs *chbs;
> >       struct cxl_port *root_port = arg;
> >       struct device *host = root_port->dev.parent;
> > -     struct acpi_device *bridge = to_cxl_host_bridge(match);
> > +     struct acpi_device *bridge = to_cxl_host_bridge(host, match);
> >
> >       if (!bridge)
> >               return 0;
> > @@ -371,6 +368,17 @@ static int add_root_nvdimm_bridge(struct device *match, void *data)
> >       return 1;
> >  }
> >
> ...
>
>
> > diff --git a/tools/testing/cxl/mock_acpi.c b/tools/testing/cxl/mock_acpi.c
> > new file mode 100644
> > index 000000000000..4c8a493ace56
> > --- /dev/null
> > +++ b/tools/testing/cxl/mock_acpi.c
> > @@ -0,0 +1,109 @@
>
> > +static int match_add_root_port(struct pci_dev *pdev, void *data)
>
> Hmm. Nice not to duplicate this code, but I guess a bit tricky to
> work around.  Maybe a comment next to the 'main' version so we
> remember to update this one as well if it is changed?

I'd like to think that the __mock annotation next to the real one is
the indication that a unit test might need updating. Sufficient?

>
> > +{
> > +     struct cxl_walk_context *ctx = data;
> > +     struct pci_bus *root_bus = ctx->root;
> > +     struct cxl_port *port = ctx->port;
> > +     int type = pci_pcie_type(pdev);
> > +     struct device *dev = ctx->dev;
> > +     u32 lnkcap, port_num;
> > +     int rc;
> > +
> > +     if (pdev->bus != root_bus)
> > +             return 0;
> > +     if (!pci_is_pcie(pdev))
> > +             return 0;
> > +     if (type != PCI_EXP_TYPE_ROOT_PORT)
> > +             return 0;
> > +     if (pci_read_config_dword(pdev, pci_pcie_cap(pdev) + PCI_EXP_LNKCAP,
> > +                               &lnkcap) != PCIBIOS_SUCCESSFUL)
> > +             return 0;
> > +
> > +     /* TODO walk DVSEC to find component register base */
> > +     port_num = FIELD_GET(PCI_EXP_LNKCAP_PN, lnkcap);
> > +     rc = cxl_add_dport(port, &pdev->dev, port_num, CXL_RESOURCE_NONE);
> > +     if (rc) {
> > +             dev_err(dev, "failed to add dport: %s (%d)\n",
> > +                     dev_name(&pdev->dev), rc);
> > +             ctx->error = rc;
> > +             return rc;
> > +     }
> > +     ctx->count++;
> > +
> > +     dev_dbg(dev, "add dport%d: %s\n", port_num, dev_name(&pdev->dev));
> > +
> > +     return 0;
> > +}
> > +
> > +static int mock_add_root_port(struct platform_device *pdev, void *data)
> > +{
> > +     struct cxl_walk_context *ctx = data;
> > +     struct cxl_port *port = ctx->port;
> > +     struct device *dev = ctx->dev;
> > +     int rc;
> > +
> > +     rc = cxl_add_dport(port, &pdev->dev, pdev->id, CXL_RESOURCE_NONE);
> > +     if (rc) {
> > +             dev_err(dev, "failed to add dport: %s (%d)\n",
> > +                     dev_name(&pdev->dev), rc);
> > +             ctx->error = rc;
> > +             return rc;
> > +     }
> > +     ctx->count++;
> > +
> > +     dev_dbg(dev, "add dport%d: %s\n", pdev->id, dev_name(&pdev->dev));
> > +
> > +     return 0;
> > +}
> > +
> > +int match_add_root_ports(struct pci_dev *dev, void *data)
> > +{
> > +     int index, rc;
> > +     struct cxl_mock_ops *ops = get_cxl_mock_ops(&index);
> > +     struct platform_device *pdev = (struct platform_device *) dev;
> > +
> > +     if (ops && ops->is_mock_port(pdev))
> > +             rc = mock_add_root_port(pdev, data);
> > +     else
> > +             rc = match_add_root_port(dev, data);
> > +
> > +     put_cxl_mock_ops(index);
> > +
> > +     return rc;
> > +}
>
> ...
>
> > diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c
> > new file mode 100644
> > index 000000000000..0710e0062e58
> > --- /dev/null
> > +++ b/tools/testing/cxl/test/cxl.c
> > @@ -0,0 +1,516 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +// Copyright(c) 2021 Intel Corporation. All rights reserved.
> > +
> > +#include <linux/platform_device.h>
> > +#include <linux/genalloc.h>
> > +#include <linux/module.h>
> > +#include <linux/mutex.h>
> > +#include <linux/acpi.h>
> > +#include <linux/pci.h>
> > +#include <linux/mm.h>
> > +#include "mock.h"
> > +
> > +#define NR_CXL_HOST_BRIDGES 4
> > +#define NR_CXL_ROOT_PORTS 2
> > +
> > +static struct platform_device *cxl_acpi;
> > +static struct platform_device *cxl_host_bridge[NR_CXL_HOST_BRIDGES];
> > +static struct platform_device
> > +     *cxl_root_port[NR_CXL_HOST_BRIDGES * NR_CXL_ROOT_PORTS];
> > +
> > +static struct acpi_device acpi0017_mock;
> > +static struct acpi_device host_bridge[NR_CXL_HOST_BRIDGES] = {
> > +     [0] = {
> > +             .handle = &host_bridge[0],
> > +     },
> > +     [1] = {
> > +             .handle = &host_bridge[1],
> > +     },
> > +     [2] = {
> > +             .handle = &host_bridge[2],
> > +     },
> > +     [3] = {
> > +             .handle = &host_bridge[3],
> > +     },
> > +};
> > +
> > +static bool is_mock_dev(struct device *dev)
> > +{
>
> You be paranoid and check acxl_acpi here but I guess
> its always safe...

Yeah, it's always safe because is_mock_dev() won't be called if the
platform_device_alloc() for cxl_acpi failed.

>
> > +     if (dev == &cxl_acpi->dev)
> > +             return true;
> > +     return false;
> > +}
> > +
>
> ...
>
> > +
> > +struct cxl_mock_res {
> > +     struct list_head list;
> > +     struct range range;
> > +};
> > +
> > +static LIST_HEAD(mock_res);
> > +static DEFINE_MUTEX(mock_res_lock);
> > +static struct gen_pool *cxl_mock_pool;
> > +
> > +static void free_mock_res(void)
> Seems like a missmatch in naming.
> free_mock_res() frees all resouces,
> alloc_moc_res() allocates one.

Ok, will rename.

>
>
> > +{
> > +     struct cxl_mock_res *res, *_res;
> > +
> > +     mutex_lock(&mock_res_lock);
> > +     list_for_each_entry_safe(res, _res, &mock_res, list) {
> > +             gen_pool_free(cxl_mock_pool, res->range.start,
> > +                           range_len(&res->range));
> > +             list_del(&res->list);
> > +             kfree(res);
> > +     }
> > +     mutex_unlock(&mock_res_lock);
> > +}
> > +
> > +static struct cxl_mock_res *alloc_mock_res(resource_size_t size)
> > +{
> > +     struct cxl_mock_res *res = kzalloc(sizeof(*res), GFP_KERNEL);
> > +     struct genpool_data_align data = {
> > +             .align = SZ_256M,
> > +     };
> > +     unsigned long phys;
> > +
> > +     INIT_LIST_HEAD(&res->list);
> > +     phys = gen_pool_alloc_algo(cxl_mock_pool, size,
> > +                                gen_pool_first_fit_align, &data);
> > +     if (!phys)
> > +             return NULL;
> > +
> > +     res->range = (struct range) {
> > +             .start = phys,
> > +             .end = phys + size - 1,
> > +     };
> > +     mutex_lock(&mock_res_lock);
> > +     list_add(&res->list, &mock_res);
> > +     mutex_unlock(&mock_res_lock);
> > +
> > +     return res;
> > +}
> > +
> > +static int populate_cedt(void)
> > +{
> > +     struct acpi_cedt_cfmws *cfmws[4] = {
> > +             [0] = &mock_cedt.cfmws0.cfmws,
> > +             [1] = &mock_cedt.cfmws1.cfmws,
> > +             [2] = &mock_cedt.cfmws2.cfmws,
> > +             [3] = &mock_cedt.cfmws3.cfmws,
> > +     };
> > +     struct cxl_mock_res *res;
> > +     int i;
> > +
> > +     for (i = 0; i < ARRAY_SIZE(mock_cedt.chbs); i++) {
> > +             struct acpi_cedt_chbs *chbs = &mock_cedt.chbs[i];
> > +             resource_size_t size;
> > +
> > +             if (chbs->cxl_version == ACPI_CEDT_CHBS_VERSION_CXL20)
> > +                     size = ACPI_CEDT_CHBS_LENGTH_CXL20;
> > +             else
> > +                     size = ACPI_CEDT_CHBS_LENGTH_CXL11;
> > +
> > +             res = alloc_mock_res(size);
> > +             if (!res)
> > +                     return -ENOMEM;
> > +             chbs->base = res->range.start;
> > +             chbs->length = size;
> > +     }
> > +
> > +     for (i = 0; i < ARRAY_SIZE(cfmws); i++) {
> > +             struct acpi_cedt_cfmws *window = cfmws[i];
> > +             int ways = 1 << window->interleave_ways;
> > +
> > +             res = alloc_mock_res(SZ_256M * ways);
>
> why that size?  Should take window_size into account I think..

This *is* the window size, but you're right if ->interleave_ways is
populated above and used here ->window_size can also be populated
there. Then all that is left to do is dynamically populate the
emulated ->base_hpa.

>
> > +             if (!res)
> > +                     return -ENOMEM;
> > +             window->base_hpa = res->range.start;
> > +             window->window_size = range_len(&res->range);
> > +     }
> > +
> > +     return 0;
> > +}
> > +
>
>
> ...
>
> > +
> > +static struct cxl_mock_ops cxl_mock_ops = {
> > +     .is_mock_adev = is_mock_adev,
> > +     .is_mock_bridge = is_mock_bridge,
> > +     .is_mock_bus = is_mock_bus,
> > +     .is_mock_port = is_mock_port,
> > +     .is_mock_dev = is_mock_dev,
> > +     .mock_port = mock_cxl_root_port,
> > +     .acpi_get_table = mock_acpi_get_table,
> > +     .acpi_put_table = mock_acpi_put_table,
> > +     .acpi_evaluate_integer = mock_acpi_evaluate_integer,
> > +     .acpi_pci_find_root = mock_acpi_pci_find_root,
> > +     .list = LIST_HEAD_INIT(cxl_mock_ops.list),
> > +};
>
> ...
>
> > +#ifndef SZ_64G
> > +#define SZ_64G (SZ_32G * 2)
> > +#endif
> > +
> > +#ifndef SZ_512G
> > +#define SZ_512G (SZ_64G * 8)
> > +#endif
>
> Why not add to sizes.h?

...because nothing in the main kernel needs these yet.

>
> > +
> > +static __init int cxl_test_init(void)
> > +{
> > +     int rc, i;
> > +
> > +     register_cxl_mock_ops(&cxl_mock_ops);
> > +
> > +     cxl_mock_pool = gen_pool_create(ilog2(SZ_2M), NUMA_NO_NODE);
> > +     if (!cxl_mock_pool) {
> > +             rc = -ENOMEM;
> > +             goto err_gen_pool_create;
> > +     }
> > +
> > +     rc = gen_pool_add(cxl_mock_pool, SZ_512G, SZ_64G, NUMA_NO_NODE);
> > +     if (rc)
> > +             goto err_gen_pool_add;
> > +
> > +     rc = populate_cedt();
> > +     if (rc)
> > +             goto err_populate;
> > +
> > +     for (i = 0; i < ARRAY_SIZE(cxl_host_bridge); i++) {
> > +             struct acpi_device *adev = &host_bridge[i];
> > +             struct platform_device *pdev;
> > +
> > +             pdev = platform_device_alloc("cxl_host_bridge", i);
> > +             if (!pdev)
> > +                     goto err_bridge;
> > +
> > +             mock_companion(adev, &pdev->dev);
> > +             rc = platform_device_add(pdev);
> > +             if (rc) {
> > +                     platform_device_put(pdev);
> > +                     goto err_bridge;
> > +             }
> > +             cxl_host_bridge[i] = pdev;
> > +     }
> > +
> > +     for (i = 0; i < ARRAY_SIZE(cxl_root_port); i++) {
> > +             struct platform_device *bridge =
> > +                     cxl_host_bridge[i / NR_CXL_ROOT_PORTS];
> > +             struct platform_device *pdev;
> > +
> > +             pdev = platform_device_alloc("cxl_root_port", i);
> > +             if (!pdev)
> > +                     goto err_port;
> > +             pdev->dev.parent = &bridge->dev;
> > +
> > +             rc = platform_device_add(pdev);
> > +             if (rc) {
> > +                     platform_device_put(pdev);
> > +                     goto err_port;
> > +             }
> > +             cxl_root_port[i] = pdev;
> > +     }
> > +
> > +     cxl_acpi = platform_device_alloc("cxl_acpi", 0);
> > +     if (!cxl_acpi)
> > +             goto err_port;
> > +
> > +     mock_companion(&acpi0017_mock, &cxl_acpi->dev);
> > +     acpi0017_mock.dev.bus = &platform_bus_type;
> > +
> > +     rc = platform_device_add(cxl_acpi);
> > +     if (rc)
> > +             goto err_add;
> > +
> > +     return 0;
> > +
> > +err_add:
> > +     platform_device_put(cxl_acpi);
> > +err_port:
> > +     for (i = ARRAY_SIZE(cxl_root_port) - 1; i >= 0; i--) {
> > +             platform_device_del(cxl_root_port[i]);
> > +             platform_device_put(cxl_root_port[i]);
> > +     }
> > +err_bridge:
> > +     for (i = ARRAY_SIZE(cxl_host_bridge) - 1; i >= 0; i--) {
> > +             platform_device_del(cxl_host_bridge[i]);
> > +             platform_device_put(cxl_host_bridge[i]);
> > +     }
> > +err_populate:
> > +     free_mock_res();
>
> Might be worth a function wrapping this that makes it clear this
> is unwinding what happened in populate_cedt()

I'll just call the function depopulate_all_mock_resources().

>
> > +err_gen_pool_add:
> > +     gen_pool_destroy(cxl_mock_pool);
> > +err_gen_pool_create:
> > +     unregister_cxl_mock_ops(&cxl_mock_ops);
> > +     return rc;
> > +}
> > +
> > +static __exit void cxl_test_exit(void)
> > +{
> > +     int i;
> > +
> > +     platform_device_del(cxl_acpi);
> > +     platform_device_put(cxl_acpi);
>
> Given the evil warning comments about platform_device_del() in platform.c
> about it only being appropriate to call it in error cases...
>
> Perhaps it's better to call platform_device_unregister() even if
> that looks locally less obvious?  Or maybe we should suggest
> the warning comments are more refined in what usage to rule out!

I'll switch to platform_device_unregister(). I don't feel like
charging the hill to change that comment.

>
> > +     for (i = ARRAY_SIZE(cxl_root_port) - 1; i >= 0; i--) {
> > +             platform_device_del(cxl_root_port[i]);
> > +             platform_device_put(cxl_root_port[i]);
> > +     }
> > +     for (i = ARRAY_SIZE(cxl_host_bridge) - 1; i >= 0; i--) {
> > +             platform_device_del(cxl_host_bridge[i]);
> > +             platform_device_put(cxl_host_bridge[i]);
> > +     }
> > +     free_mock_res();
> > +     gen_pool_destroy(cxl_mock_pool);
> > +     unregister_cxl_mock_ops(&cxl_mock_ops);
> > +}
> > +
> > +module_init(cxl_test_init);
> > +module_exit(cxl_test_exit);
> > +MODULE_LICENSE("GPL v2");
>
> ...
>
> Thanks,
>
> Jonathan
>
>

Jonathan Cameron Sept. 6, 2021, 8:32 a.m. UTC | #3

On Fri, 3 Sep 2021 14:49:34 -0700
Dan Williams <[e-mail]> wrote:

> On Fri, Sep 3, 2021 at 5:53 AM Jonathan Cameron
> <[e-mail]> wrote:
> >
> > On Tue, 24 Aug 2021 09:07:33 -0700
> > Dan Williams <[e-mail]> wrote:
> >  
> > > Create an environment for CXL plumbing unit tests. Especially when it
> > > comes to an algorithm for HDM Decoder (Host-managed Device Memory
> > > Decoder) programming, the availability of an in-kernel-tree emulation
> > > environment for CXL configuration complexity and corner cases speeds
> > > development and deters regressions.
> > >
> > > The approach taken mirrors what was done for tools/testing/nvdimm/. I.e.
> > > an external module, cxl_test.ko built out of the tools/testing/cxl/
> > > directory, provides mock implementations of kernel APIs and kernel
> > > objects to simulate a real world device hierarchy.
> > >
> > > One feedback for the tools/testing/nvdimm/ proposal was "why not do this
> > > in QEMU?". In fact, the CXL development community has developed a QEMU
> > > model for CXL [1]. However, there are a few blocking issues that keep
> > > QEMU from being a tight fit for topology + provisioning unit tests:
> > >
> > > 1/ The QEMU community has yet to show interest in merging any of this
> > >    support that has had patches on the list since November 2020. So,
> > >    testing CXL to date involves building custom QEMU with out-of-tree
> > >    patches.  
> >
> > That's a separate discussion I've been meaning to kick off. I'd like
> > to get that moving because there are various things we can do there
> > which can't necessarily be done with this approach or at least are easier
> > done in QEMU. I'll raise it on the qemu list and drag a few people in
> > who might be able to help us get things moving + help find solutions to
> > the bits that we can't currently do.
> >  
> > >
> > > 2/ CXL mechanisms like cross-host-bridge interleave do not have a clear
> > >    path to be emulated by QEMU without major infrastructure work. This
> > >    is easier to achieve with the alloc_mock_res() approach taken in this
> > >    patch to shortcut-define emulated system physical address ranges with
> > >    interleave behavior.
> > >
> > > The QEMU enabling has been critical to get the driver off the ground,
> > > and may still move forward, but it does not address the ongoing needs of
> > > a regression testing environment and test driven development.  
> >
> > Different purposes, so I would see having both as beneficial  
> 
> Oh certainly, especially because cxl_test skips all the PCI details.
> This regression environment is mainly for user space ABI regressions
> and the PCI agnostic machinery in the subsystem. I'd love for the QEMU
> work to move forward.
> 
> > (in principle - I haven't played with this yet :)  
> 
> I have wondered if having a version of DOE emulation in tools/testing/
> makes regression testing those protocols easier, but again that's PCI
> details where QEMU is more suitable.

Maybe, but I'm not convinced yet.  Particularly as the protocol complexity
that we are interested in can get pretty nasty and I'm not sure we want
the pain of implementing that anywhere near the kernel (e.g. CMA with
having to hook an SPDM implementation in).

Could do discovery only I guess which would exercise the basics.
> 
> >  
> > >
> > > This patch adds an ACPI CXL Platform definition with emulated CXL
> > > multi-ported host-bridges. A follow on patch adds emulated memory
> > > expander devices.
> > >
> > > Acked-by: Ben Widawsky <[e-mail]>
> > > Reported-by: Vishal Verma <[e-mail]>
> > > Link: https://lore.kernel.org/r/[e-mail] [1]
> > > Signed-off-by: Dan Williams <[e-mail]>
> > > ---

...


> 
> >  
> > > +     struct acpi_device *bridge = to_cxl_host_bridge(host, match);
> > >
> > >       if (!bridge)
> > >               return 0;
> > > @@ -319,7 +316,7 @@ static int add_host_bridge_dport(struct device *match, void *arg)
> > >       struct acpi_cedt_chbs *chbs;
> > >       struct cxl_port *root_port = arg;
> > >       struct device *host = root_port->dev.parent;
> > > -     struct acpi_device *bridge = to_cxl_host_bridge(match);
> > > +     struct acpi_device *bridge = to_cxl_host_bridge(host, match);
> > >
> > >       if (!bridge)
> > >               return 0;
> > > @@ -371,6 +368,17 @@ static int add_root_nvdimm_bridge(struct device *match, void *data)
> > >       return 1;
> > >  }
> > >  
> > ...
> >
> >  
> > > diff --git a/tools/testing/cxl/mock_acpi.c b/tools/testing/cxl/mock_acpi.c
> > > new file mode 100644
> > > index 000000000000..4c8a493ace56
> > > --- /dev/null
> > > +++ b/tools/testing/cxl/mock_acpi.c
> > > @@ -0,0 +1,109 @@  
> >  
> > > +static int match_add_root_port(struct pci_dev *pdev, void *data)  
> >
> > Hmm. Nice not to duplicate this code, but I guess a bit tricky to
> > work around.  Maybe a comment next to the 'main' version so we
> > remember to update this one as well if it is changed?  
> 
> I'd like to think that the __mock annotation next to the real one is
> the indication that a unit test might need updating. Sufficient?

Agreed in general, but this particular function isn't annotated, the
caller of it is, so people have to notice that to be aware there is
a possible issue.  If the change is something local to this they might
not notice.

> 
> >  
> > > +{
> > > +     struct cxl_walk_context *ctx = data;
> > > +     struct pci_bus *root_bus = ctx->root;
> > > +     struct cxl_port *port = ctx->port;
> > > +     int type = pci_pcie_type(pdev);
> > > +     struct device *dev = ctx->dev;
> > > +     u32 lnkcap, port_num;
> > > +     int rc;
> > > +
> > > +     if (pdev->bus != root_bus)
> > > +             return 0;
> > > +     if (!pci_is_pcie(pdev))
> > > +             return 0;
> > > +     if (type != PCI_EXP_TYPE_ROOT_PORT)
> > > +             return 0;
> > > +     if (pci_read_config_dword(pdev, pci_pcie_cap(pdev) + PCI_EXP_LNKCAP,
> > > +                               &lnkcap) != PCIBIOS_SUCCESSFUL)
> > > +             return 0;
> > > +
> > > +     /* TODO walk DVSEC to find component register base */
> > > +     port_num = FIELD_GET(PCI_EXP_LNKCAP_PN, lnkcap);
> > > +     rc = cxl_add_dport(port, &pdev->dev, port_num, CXL_RESOURCE_NONE);
> > > +     if (rc) {
> > > +             dev_err(dev, "failed to add dport: %s (%d)\n",
> > > +                     dev_name(&pdev->dev), rc);
> > > +             ctx->error = rc;
> > > +             return rc;
> > > +     }
> > > +     ctx->count++;
> > > +
> > > +     dev_dbg(dev, "add dport%d: %s\n", port_num, dev_name(&pdev->dev));
> > > +
> > > +     return 0;
> > > +}
> > > +
...

> >  
> > > diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c
> > > new file mode 100644
> > > index 000000000000..0710e0062e58
> > > --- /dev/null
> > > +++ b/tools/testing/cxl/test/cxl.c
> > > @@ -0,0 +1,516 @@


> >
> >  
> > > +{
> > > +     struct cxl_mock_res *res, *_res;
> > > +
> > > +     mutex_lock(&mock_res_lock);
> > > +     list_for_each_entry_safe(res, _res, &mock_res, list) {
> > > +             gen_pool_free(cxl_mock_pool, res->range.start,
> > > +                           range_len(&res->range));
> > > +             list_del(&res->list);
> > > +             kfree(res);
> > > +     }
> > > +     mutex_unlock(&mock_res_lock);
> > > +}
> > > +
> > > +static struct cxl_mock_res *alloc_mock_res(resource_size_t size)
> > > +{
> > > +     struct cxl_mock_res *res = kzalloc(sizeof(*res), GFP_KERNEL);
> > > +     struct genpool_data_align data = {
> > > +             .align = SZ_256M,
> > > +     };
> > > +     unsigned long phys;
> > > +
> > > +     INIT_LIST_HEAD(&res->list);
> > > +     phys = gen_pool_alloc_algo(cxl_mock_pool, size,
> > > +                                gen_pool_first_fit_align, &data);
> > > +     if (!phys)
> > > +             return NULL;
> > > +
> > > +     res->range = (struct range) {
> > > +             .start = phys,
> > > +             .end = phys + size - 1,
> > > +     };
> > > +     mutex_lock(&mock_res_lock);
> > > +     list_add(&res->list, &mock_res);
> > > +     mutex_unlock(&mock_res_lock);
> > > +
> > > +     return res;
> > > +}
> > > +
> > > +static int populate_cedt(void)
> > > +{
> > > +     struct acpi_cedt_cfmws *cfmws[4] = {
> > > +             [0] = &mock_cedt.cfmws0.cfmws,
> > > +             [1] = &mock_cedt.cfmws1.cfmws,
> > > +             [2] = &mock_cedt.cfmws2.cfmws,
> > > +             [3] = &mock_cedt.cfmws3.cfmws,
> > > +     };
> > > +     struct cxl_mock_res *res;
> > > +     int i;
> > > +
> > > +     for (i = 0; i < ARRAY_SIZE(mock_cedt.chbs); i++) {
> > > +             struct acpi_cedt_chbs *chbs = &mock_cedt.chbs[i];
> > > +             resource_size_t size;
> > > +
> > > +             if (chbs->cxl_version == ACPI_CEDT_CHBS_VERSION_CXL20)
> > > +                     size = ACPI_CEDT_CHBS_LENGTH_CXL20;
> > > +             else
> > > +                     size = ACPI_CEDT_CHBS_LENGTH_CXL11;
> > > +
> > > +             res = alloc_mock_res(size);
> > > +             if (!res)
> > > +                     return -ENOMEM;
> > > +             chbs->base = res->range.start;
> > > +             chbs->length = size;
> > > +     }
> > > +
> > > +     for (i = 0; i < ARRAY_SIZE(cfmws); i++) {
> > > +             struct acpi_cedt_cfmws *window = cfmws[i];
> > > +             int ways = 1 << window->interleave_ways;
> > > +
> > > +             res = alloc_mock_res(SZ_256M * ways);  
> >
> > why that size?  Should take window_size into account I think..  
> 
> This *is* the window size, but you're right if ->interleave_ways is
> populated above and used here ->window_size can also be populated
> there. Then all that is left to do is dynamically populate the
> emulated ->base_hpa.

Ok, so my confusion is that this code is alays using SZ_256M * ways
rather than say SZ_512M * ways.  

Perhaps a define at the top of the file or even a module parameter
to allow larger sizes?

> 
> >  
> > > +             if (!res)
> > > +                     return -ENOMEM;
> > > +             window->base_hpa = res->range.start;
> > > +             window->window_size = range_len(&res->range);
> > > +     }
> > > +
> > > +     return 0;
> > > +}
> > > +  
> >
> >
> > ...
> >  
> > > +
> > > +static struct cxl_mock_ops cxl_mock_ops = {
> > > +     .is_mock_adev = is_mock_adev,
> > > +     .is_mock_bridge = is_mock_bridge,
> > > +     .is_mock_bus = is_mock_bus,
> > > +     .is_mock_port = is_mock_port,
> > > +     .is_mock_dev = is_mock_dev,
> > > +     .mock_port = mock_cxl_root_port,
> > > +     .acpi_get_table = mock_acpi_get_table,
> > > +     .acpi_put_table = mock_acpi_put_table,
> > > +     .acpi_evaluate_integer = mock_acpi_evaluate_integer,
> > > +     .acpi_pci_find_root = mock_acpi_pci_find_root,
> > > +     .list = LIST_HEAD_INIT(cxl_mock_ops.list),
> > > +};  
> >
> > ...
> >  
> > > +#ifndef SZ_64G
> > > +#define SZ_64G (SZ_32G * 2)
> > > +#endif
> > > +
> > > +#ifndef SZ_512G
> > > +#define SZ_512G (SZ_64G * 8)
> > > +#endif  
> >
> > Why not add to sizes.h?  
> 
> ...because nothing in the main kernel needs these yet.

Nothing in 5.14.1 uses SZ_32G either :)
Fair enough though.

> 
> >  
> > > +
> > > +static __init int cxl_test_init(void)
> > > +{
> > > +     int rc, i;
> > > +
> > > +     register_cxl_mock_ops(&cxl_mock_ops);
> > > +
> > > +     cxl_mock_pool = gen_pool_create(ilog2(SZ_2M), NUMA_NO_NODE);
> > > +     if (!cxl_mock_pool) {
> > > +             rc = -ENOMEM;
> > > +             goto err_gen_pool_create;
> > > +     }
> > > +
> > > +     rc = gen_pool_add(cxl_mock_pool, SZ_512G, SZ_64G, NUMA_NO_NODE);
> > > +     if (rc)
> > > +             goto err_gen_pool_add;
> > > +
> > > +     rc = populate_cedt();
> > > +     if (rc)
> > > +             goto err_populate;
> > > +
> > > +     for (i = 0; i < ARRAY_SIZE(cxl_host_bridge); i++) {
> > > +             struct acpi_device *adev = &host_bridge[i];
> > > +             struct platform_device *pdev;
> > > +
> > > +             pdev = platform_device_alloc("cxl_host_bridge", i);
> > > +             if (!pdev)
> > > +                     goto err_bridge;
> > > +
> > > +             mock_companion(adev, &pdev->dev);
> > > +             rc = platform_device_add(pdev);
> > > +             if (rc) {
> > > +                     platform_device_put(pdev);
> > > +                     goto err_bridge;
> > > +             }
> > > +             cxl_host_bridge[i] = pdev;
> > > +     }
> > > +
> > > +     for (i = 0; i < ARRAY_SIZE(cxl_root_port); i++) {
> > > +             struct platform_device *bridge =
> > > +                     cxl_host_bridge[i / NR_CXL_ROOT_PORTS];
> > > +             struct platform_device *pdev;
> > > +
> > > +             pdev = platform_device_alloc("cxl_root_port", i);
> > > +             if (!pdev)
> > > +                     goto err_port;
> > > +             pdev->dev.parent = &bridge->dev;
> > > +
> > > +             rc = platform_device_add(pdev);
> > > +             if (rc) {
> > > +                     platform_device_put(pdev);
> > > +                     goto err_port;
> > > +             }
> > > +             cxl_root_port[i] = pdev;
> > > +     }
> > > +
> > > +     cxl_acpi = platform_device_alloc("cxl_acpi", 0);
> > > +     if (!cxl_acpi)
> > > +             goto err_port;
> > > +
> > > +     mock_companion(&acpi0017_mock, &cxl_acpi->dev);
> > > +     acpi0017_mock.dev.bus = &platform_bus_type;
> > > +
> > > +     rc = platform_device_add(cxl_acpi);
> > > +     if (rc)
> > > +             goto err_add;
> > > +
> > > +     return 0;
> > > +
> > > +err_add:
> > > +     platform_device_put(cxl_acpi);
> > > +err_port:
> > > +     for (i = ARRAY_SIZE(cxl_root_port) - 1; i >= 0; i--) {
> > > +             platform_device_del(cxl_root_port[i]);
> > > +             platform_device_put(cxl_root_port[i]);
> > > +     }
> > > +err_bridge:
> > > +     for (i = ARRAY_SIZE(cxl_host_bridge) - 1; i >= 0; i--) {
> > > +             platform_device_del(cxl_host_bridge[i]);
> > > +             platform_device_put(cxl_host_bridge[i]);
> > > +     }
> > > +err_populate:
> > > +     free_mock_res();  
> >
> > Might be worth a function wrapping this that makes it clear this
> > is unwinding what happened in populate_cedt()  
> 
> I'll just call the function depopulate_all_mock_resources().
> 
> >  
> > > +err_gen_pool_add:
> > > +     gen_pool_destroy(cxl_mock_pool);
> > > +err_gen_pool_create:
> > > +     unregister_cxl_mock_ops(&cxl_mock_ops);
> > > +     return rc;
> > > +}
> > > +
> > > +static __exit void cxl_test_exit(void)
> > > +{
> > > +     int i;
> > > +
> > > +     platform_device_del(cxl_acpi);
> > > +     platform_device_put(cxl_acpi);  
> >
> > Given the evil warning comments about platform_device_del() in platform.c
> > about it only being appropriate to call it in error cases...
> >
> > Perhaps it's better to call platform_device_unregister() even if
> > that looks locally less obvious?  Or maybe we should suggest
> > the warning comments are more refined in what usage to rule out!  
> 
> I'll switch to platform_device_unregister(). I don't feel like
> charging the hill to change that comment.

Wise move

Jonathan

Dan Williams Sept. 7, 2021, 3:57 p.m. UTC | #4

On Mon, Sep 6, 2021 at 1:32 AM Jonathan Cameron
<[e-mail]> wrote:
>
> On Fri, 3 Sep 2021 14:49:34 -0700
> Dan Williams <[e-mail]> wrote:
>
> > On Fri, Sep 3, 2021 at 5:53 AM Jonathan Cameron
> > <[e-mail]> wrote:
> > >
> > > On Tue, 24 Aug 2021 09:07:33 -0700
> > > Dan Williams <[e-mail]> wrote:
> > >
> > > > Create an environment for CXL plumbing unit tests. Especially when it
> > > > comes to an algorithm for HDM Decoder (Host-managed Device Memory
> > > > Decoder) programming, the availability of an in-kernel-tree emulation
> > > > environment for CXL configuration complexity and corner cases speeds
> > > > development and deters regressions.
> > > >
> > > > The approach taken mirrors what was done for tools/testing/nvdimm/. I.e.
> > > > an external module, cxl_test.ko built out of the tools/testing/cxl/
> > > > directory, provides mock implementations of kernel APIs and kernel
> > > > objects to simulate a real world device hierarchy.
> > > >
> > > > One feedback for the tools/testing/nvdimm/ proposal was "why not do this
> > > > in QEMU?". In fact, the CXL development community has developed a QEMU
> > > > model for CXL [1]. However, there are a few blocking issues that keep
> > > > QEMU from being a tight fit for topology + provisioning unit tests:
> > > >
> > > > 1/ The QEMU community has yet to show interest in merging any of this
> > > >    support that has had patches on the list since November 2020. So,
> > > >    testing CXL to date involves building custom QEMU with out-of-tree
> > > >    patches.
> > >
> > > That's a separate discussion I've been meaning to kick off. I'd like
> > > to get that moving because there are various things we can do there
> > > which can't necessarily be done with this approach or at least are easier
> > > done in QEMU. I'll raise it on the qemu list and drag a few people in
> > > who might be able to help us get things moving + help find solutions to
> > > the bits that we can't currently do.
> > >
> > > >
> > > > 2/ CXL mechanisms like cross-host-bridge interleave do not have a clear
> > > >    path to be emulated by QEMU without major infrastructure work. This
> > > >    is easier to achieve with the alloc_mock_res() approach taken in this
> > > >    patch to shortcut-define emulated system physical address ranges with
> > > >    interleave behavior.
> > > >
> > > > The QEMU enabling has been critical to get the driver off the ground,
> > > > and may still move forward, but it does not address the ongoing needs of
> > > > a regression testing environment and test driven development.
> > >
> > > Different purposes, so I would see having both as beneficial
> >
> > Oh certainly, especially because cxl_test skips all the PCI details.
> > This regression environment is mainly for user space ABI regressions
> > and the PCI agnostic machinery in the subsystem. I'd love for the QEMU
> > work to move forward.
> >
> > > (in principle - I haven't played with this yet :)
> >
> > I have wondered if having a version of DOE emulation in tools/testing/
> > makes regression testing those protocols easier, but again that's PCI
> > details where QEMU is more suitable.
>
> Maybe, but I'm not convinced yet.  Particularly as the protocol complexity
> that we are interested in can get pretty nasty and I'm not sure we want
> the pain of implementing that anywhere near the kernel (e.g. CMA with
> having to hook an SPDM implementation in).
>
> Could do discovery only I guess which would exercise the basics.
> >
> > >
> > > >
> > > > This patch adds an ACPI CXL Platform definition with emulated CXL
> > > > multi-ported host-bridges. A follow on patch adds emulated memory
> > > > expander devices.
> > > >
> > > > Acked-by: Ben Widawsky <[e-mail]>
> > > > Reported-by: Vishal Verma <[e-mail]>
> > > > Link: https://lore.kernel.org/r/[e-mail] [1]
> > > > Signed-off-by: Dan Williams <[e-mail]>
> > > > ---
>
> ...
>
>
> >
> > >
> > > > +     struct acpi_device *bridge = to_cxl_host_bridge(host, match);
> > > >
> > > >       if (!bridge)
> > > >               return 0;
> > > > @@ -319,7 +316,7 @@ static int add_host_bridge_dport(struct device *match, void *arg)
> > > >       struct acpi_cedt_chbs *chbs;
> > > >       struct cxl_port *root_port = arg;
> > > >       struct device *host = root_port->dev.parent;
> > > > -     struct acpi_device *bridge = to_cxl_host_bridge(match);
> > > > +     struct acpi_device *bridge = to_cxl_host_bridge(host, match);
> > > >
> > > >       if (!bridge)
> > > >               return 0;
> > > > @@ -371,6 +368,17 @@ static int add_root_nvdimm_bridge(struct device *match, void *data)
> > > >       return 1;
> > > >  }
> > > >
> > > ...
> > >
> > >
> > > > diff --git a/tools/testing/cxl/mock_acpi.c b/tools/testing/cxl/mock_acpi.c
> > > > new file mode 100644
> > > > index 000000000000..4c8a493ace56
> > > > --- /dev/null
> > > > +++ b/tools/testing/cxl/mock_acpi.c
> > > > @@ -0,0 +1,109 @@
> > >
> > > > +static int match_add_root_port(struct pci_dev *pdev, void *data)
> > >
> > > Hmm. Nice not to duplicate this code, but I guess a bit tricky to
> > > work around.  Maybe a comment next to the 'main' version so we
> > > remember to update this one as well if it is changed?
> >
> > I'd like to think that the __mock annotation next to the real one is
> > the indication that a unit test might need updating. Sufficient?
>
> Agreed in general, but this particular function isn't annotated, the
> caller of it is, so people have to notice that to be aware there is
> a possible issue.  If the change is something local to this they might
> not notice.

The regression test will notice. Its primary function is to catch
regressions of this nature.


[..]
> > > why that size?  Should take window_size into account I think..
> >
> > This *is* the window size, but you're right if ->interleave_ways is
> > populated above and used here ->window_size can also be populated
> > there. Then all that is left to do is dynamically populate the
> > emulated ->base_hpa.
>
> Ok, so my confusion is that this code is alays using SZ_256M * ways
> rather than say SZ_512M * ways.
>
> Perhaps a define at the top of the file or even a module parameter
> to allow larger sizes?

I changed this to put the size in the table definition directly so it
can be edited there. The intent is for this size to be static / known
to the unit test in advance. I.e. unlike QEMU testing where the test
would need to be told of the configuration that was specified to the
VM.

Patch

diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
index 2d8f1ec1abff..3fcb704c647f 100644
--- a/drivers/cxl/acpi.c
+++ b/drivers/cxl/acpi.c
@@ -182,15 +182,7 @@  static resource_size_t get_chbcr(struct acpi_cedt_chbs *chbs)
 	return IS_ERR(chbs) ? CXL_RESOURCE_NONE : chbs->base;
 }
 
-struct cxl_walk_context {
-	struct device *dev;
-	struct pci_bus *root;
-	struct cxl_port *port;
-	int error;
-	int count;
-};
-
-static int match_add_root_ports(struct pci_dev *pdev, void *data)
+__mock int match_add_root_ports(struct pci_dev *pdev, void *data)
 {
 	struct cxl_walk_context *ctx = data;
 	struct pci_bus *root_bus = ctx->root;
@@ -214,6 +206,8 @@  static int match_add_root_ports(struct pci_dev *pdev, void *data)
 	port_num = FIELD_GET(PCI_EXP_LNKCAP_PN, lnkcap);
 	rc = cxl_add_dport(port, &pdev->dev, port_num, CXL_RESOURCE_NONE);
 	if (rc) {
+		dev_err(dev, "failed to add dport: %s (%d)\n",
+			dev_name(&pdev->dev), rc);
 		ctx->error = rc;
 		return rc;
 	}
@@ -239,15 +233,18 @@  static struct cxl_dport *find_dport_by_dev(struct cxl_port *port, struct device
 	return NULL;
 }
 
-static struct acpi_device *to_cxl_host_bridge(struct device *dev)
+__mock struct acpi_device *to_cxl_host_bridge(struct device *host,
+					      struct device *dev)
 {
 	struct acpi_device *adev = to_acpi_device(dev);
 
 	if (!acpi_pci_find_root(adev->handle))
 		return NULL;
 
-	if (strcmp(acpi_device_hid(adev), "ACPI0016") == 0)
+	if (strcmp(acpi_device_hid(adev), "ACPI0016") == 0) {
+		dev_dbg(host, "found host bridge %s\n", dev_name(&adev->dev));
 		return adev;
+	}
 	return NULL;
 }
 
@@ -257,14 +254,14 @@  static struct acpi_device *to_cxl_host_bridge(struct device *dev)
  */
 static int add_host_bridge_uport(struct device *match, void *arg)
 {
-	struct acpi_device *bridge = to_cxl_host_bridge(match);
+	struct cxl_port *port;
+	struct cxl_dport *dport;
+	struct cxl_decoder *cxld;
+	struct cxl_walk_context ctx;
+	struct acpi_pci_root *pci_root;
 	struct cxl_port *root_port = arg;
 	struct device *host = root_port->dev.parent;
-	struct acpi_pci_root *pci_root;
-	struct cxl_walk_context ctx;
-	struct cxl_decoder *cxld;
-	struct cxl_dport *dport;
-	struct cxl_port *port;
+	struct acpi_device *bridge = to_cxl_host_bridge(host, match);
 
 	if (!bridge)
 		return 0;
@@ -319,7 +316,7 @@  static int add_host_bridge_dport(struct device *match, void *arg)
 	struct acpi_cedt_chbs *chbs;
 	struct cxl_port *root_port = arg;
 	struct device *host = root_port->dev.parent;
-	struct acpi_device *bridge = to_cxl_host_bridge(match);
+	struct acpi_device *bridge = to_cxl_host_bridge(host, match);
 
 	if (!bridge)
 		return 0;
@@ -371,6 +368,17 @@  static int add_root_nvdimm_bridge(struct device *match, void *data)
 	return 1;
 }
 
+static u32 cedt_instance(struct platform_device *pdev)
+{
+	const bool *native_acpi0017 = acpi_device_get_match_data(&pdev->dev);
+
+	if (native_acpi0017 && *native_acpi0017)
+		return 0;
+
+	/* for cxl_test request a non-canonical instance */
+	return U32_MAX;
+}
+
 static int cxl_acpi_probe(struct platform_device *pdev)
 {
 	int rc;
@@ -384,7 +392,7 @@  static int cxl_acpi_probe(struct platform_device *pdev)
 		return PTR_ERR(root_port);
 	dev_dbg(host, "add: %s\n", dev_name(&root_port->dev));
 
-	status = acpi_get_table(ACPI_SIG_CEDT, 0, &acpi_cedt);
+	status = acpi_get_table(ACPI_SIG_CEDT, cedt_instance(pdev), &acpi_cedt);
 	if (ACPI_FAILURE(status))
 		return -ENXIO;
 
@@ -415,9 +423,11 @@  static int cxl_acpi_probe(struct platform_device *pdev)
 	return 0;
 }
 
+static bool native_acpi0017 = true;
+
 static const struct acpi_device_id cxl_acpi_ids[] = {
-	{ "ACPI0017", 0 },
-	{ "", 0 },
+	{ "ACPI0017", (unsigned long) &native_acpi0017 },
+	{ },
 };
 MODULE_DEVICE_TABLE(acpi, cxl_acpi_ids);
 
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index 1b2e816e061e..a9d4051722bf 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -226,6 +226,14 @@  struct cxl_nvdimm {
 	struct nvdimm *nvdimm;
 };
 
+struct cxl_walk_context {
+	struct device *dev;
+	struct pci_bus *root;
+	struct cxl_port *port;
+	int error;
+	int count;
+};
+
 /**
  * struct cxl_port - logical collection of upstream port devices and
  *		     downstream port devices to construct a CXL memory
@@ -325,4 +333,9 @@  struct cxl_nvdimm *to_cxl_nvdimm(struct device *dev);
 bool is_cxl_nvdimm(struct device *dev);
 int devm_cxl_add_nvdimm(struct device *host, struct cxl_memdev *cxlmd);
 struct cxl_nvdimm_bridge *cxl_find_nvdimm_bridge(void);
+
+/* unit test build overrides this to __weak */
+#ifndef __mock
+#define __mock static
+#endif
 #endif /* __CXL_H__ */
diff --git a/tools/testing/cxl/Kbuild b/tools/testing/cxl/Kbuild
new file mode 100644
index 000000000000..63a4a07e71c4
--- /dev/null
+++ b/tools/testing/cxl/Kbuild
@@ -0,0 +1,36 @@ 
+# SPDX-License-Identifier: GPL-2.0
+ldflags-y += --wrap=is_acpi_device_node
+ldflags-y += --wrap=acpi_get_table
+ldflags-y += --wrap=acpi_put_table
+ldflags-y += --wrap=acpi_evaluate_integer
+ldflags-y += --wrap=acpi_pci_find_root
+ldflags-y += --wrap=pci_walk_bus
+ldflags-y += --wrap=nvdimm_bus_register
+
+DRIVERS := ../../../drivers
+CXL_SRC := $(DRIVERS)/cxl
+CXL_CORE_SRC := $(DRIVERS)/cxl/core
+ccflags-y := -I$(srctree)/drivers/cxl/
+ccflags-y += -D__mock=__weak
+
+obj-m += cxl_acpi.o
+
+cxl_acpi-y := $(CXL_SRC)/acpi.o
+cxl_acpi-y += mock_acpi.o
+cxl_acpi-y += config_check.o
+
+obj-m += cxl_pmem.o
+
+cxl_pmem-y := $(CXL_SRC)/pmem.o
+cxl_pmem-y += config_check.o
+
+obj-m += cxl_core.o
+
+cxl_core-y := $(CXL_CORE_SRC)/bus.o
+cxl_core-y += $(CXL_CORE_SRC)/pmem.o
+cxl_core-y += $(CXL_CORE_SRC)/regs.o
+cxl_core-y += $(CXL_CORE_SRC)/memdev.o
+cxl_core-y += $(CXL_CORE_SRC)/mbox.o
+cxl_core-y += config_check.o
+
+obj-m += test/
diff --git a/tools/testing/cxl/config_check.c b/tools/testing/cxl/config_check.c
new file mode 100644
index 000000000000..de5e5b3652fd
--- /dev/null
+++ b/tools/testing/cxl/config_check.c
@@ -0,0 +1,13 @@ 
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/bug.h>
+
+void check(void)
+{
+	/*
+	 * These kconfig symbols must be set to "m" for cxl_test to load
+	 * and operate.
+	 */
+	BUILD_BUG_ON(!IS_MODULE(CONFIG_CXL_BUS));
+	BUILD_BUG_ON(!IS_MODULE(CONFIG_CXL_ACPI));
+	BUILD_BUG_ON(!IS_MODULE(CONFIG_CXL_PMEM));
+}
diff --git a/tools/testing/cxl/mock_acpi.c b/tools/testing/cxl/mock_acpi.c
new file mode 100644
index 000000000000..4c8a493ace56
--- /dev/null
+++ b/tools/testing/cxl/mock_acpi.c
@@ -0,0 +1,109 @@ 
+// SPDX-License-Identifier: GPL-2.0-only
+/* Copyright(c) 2021 Intel Corporation. All rights reserved. */
+
+#include <linux/platform_device.h>
+#include <linux/device.h>
+#include <linux/acpi.h>
+#include <linux/pci.h>
+#include <cxl.h>
+#include "test/mock.h"
+
+struct acpi_device *to_cxl_host_bridge(struct device *host, struct device *dev)
+{
+	int index;
+	struct acpi_device *adev, *found = NULL;
+	struct cxl_mock_ops *ops = get_cxl_mock_ops(&index);
+
+	if (ops && ops->is_mock_bridge(dev)) {
+		found = ACPI_COMPANION(dev);
+		goto out;
+	}
+
+	if (dev->bus == &platform_bus_type)
+		goto out;
+
+	adev = to_acpi_device(dev);
+	if (!acpi_pci_find_root(adev->handle))
+		goto out;
+
+	if (strcmp(acpi_device_hid(adev), "ACPI0016") == 0) {
+		found = adev;
+		dev_dbg(host, "found host bridge %s\n", dev_name(&adev->dev));
+	}
+out:
+	put_cxl_mock_ops(index);
+	return found;
+}
+
+static int match_add_root_port(struct pci_dev *pdev, void *data)
+{
+	struct cxl_walk_context *ctx = data;
+	struct pci_bus *root_bus = ctx->root;
+	struct cxl_port *port = ctx->port;
+	int type = pci_pcie_type(pdev);
+	struct device *dev = ctx->dev;
+	u32 lnkcap, port_num;
+	int rc;
+
+	if (pdev->bus != root_bus)
+		return 0;
+	if (!pci_is_pcie(pdev))
+		return 0;
+	if (type != PCI_EXP_TYPE_ROOT_PORT)
+		return 0;
+	if (pci_read_config_dword(pdev, pci_pcie_cap(pdev) + PCI_EXP_LNKCAP,
+				  &lnkcap) != PCIBIOS_SUCCESSFUL)
+		return 0;
+
+	/* TODO walk DVSEC to find component register base */
+	port_num = FIELD_GET(PCI_EXP_LNKCAP_PN, lnkcap);
+	rc = cxl_add_dport(port, &pdev->dev, port_num, CXL_RESOURCE_NONE);
+	if (rc) {
+		dev_err(dev, "failed to add dport: %s (%d)\n",
+			dev_name(&pdev->dev), rc);
+		ctx->error = rc;
+		return rc;
+	}
+	ctx->count++;
+
+	dev_dbg(dev, "add dport%d: %s\n", port_num, dev_name(&pdev->dev));
+
+	return 0;
+}
+
+static int mock_add_root_port(struct platform_device *pdev, void *data)
+{
+	struct cxl_walk_context *ctx = data;
+	struct cxl_port *port = ctx->port;
+	struct device *dev = ctx->dev;
+	int rc;
+
+	rc = cxl_add_dport(port, &pdev->dev, pdev->id, CXL_RESOURCE_NONE);
+	if (rc) {
+		dev_err(dev, "failed to add dport: %s (%d)\n",
+			dev_name(&pdev->dev), rc);
+		ctx->error = rc;
+		return rc;
+	}
+	ctx->count++;
+
+	dev_dbg(dev, "add dport%d: %s\n", pdev->id, dev_name(&pdev->dev));
+
+	return 0;
+}
+
+int match_add_root_ports(struct pci_dev *dev, void *data)
+{
+	int index, rc;
+	struct cxl_mock_ops *ops = get_cxl_mock_ops(&index);
+	struct platform_device *pdev = (struct platform_device *) dev;
+
+	if (ops && ops->is_mock_port(pdev))
+		rc = mock_add_root_port(pdev, data);
+	else
+		rc = match_add_root_port(dev, data);
+
+	put_cxl_mock_ops(index);
+
+	return rc;
+}
diff --git a/tools/testing/cxl/test/Kbuild b/tools/testing/cxl/test/Kbuild
new file mode 100644
index 000000000000..7de4ddecfd21
--- /dev/null
+++ b/tools/testing/cxl/test/Kbuild
@@ -0,0 +1,6 @@ 
+# SPDX-License-Identifier: GPL-2.0
+obj-m += cxl_test.o
+obj-m += cxl_mock.o
+
+cxl_test-y := cxl.o
+cxl_mock-y := mock.o
diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c
new file mode 100644
index 000000000000..0710e0062e58
--- /dev/null
+++ b/tools/testing/cxl/test/cxl.c
@@ -0,0 +1,516 @@ 
+// SPDX-License-Identifier: GPL-2.0-only
+// Copyright(c) 2021 Intel Corporation. All rights reserved.
+
+#include <linux/platform_device.h>
+#include <linux/genalloc.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/acpi.h>
+#include <linux/pci.h>
+#include <linux/mm.h>
+#include "mock.h"
+
+#define NR_CXL_HOST_BRIDGES 4
+#define NR_CXL_ROOT_PORTS 2
+
+static struct platform_device *cxl_acpi;
+static struct platform_device *cxl_host_bridge[NR_CXL_HOST_BRIDGES];
+static struct platform_device
+	*cxl_root_port[NR_CXL_HOST_BRIDGES * NR_CXL_ROOT_PORTS];
+
+static struct acpi_device acpi0017_mock;
+static struct acpi_device host_bridge[NR_CXL_HOST_BRIDGES] = {
+	[0] = {
+		.handle = &host_bridge[0],
+	},
+	[1] = {
+		.handle = &host_bridge[1],
+	},
+	[2] = {
+		.handle = &host_bridge[2],
+	},
+	[3] = {
+		.handle = &host_bridge[3],
+	},
+};
+
+static bool is_mock_dev(struct device *dev)
+{
+	if (dev == &cxl_acpi->dev)
+		return true;
+	return false;
+}
+
+static bool is_mock_adev(struct acpi_device *adev)
+{
+	int i;
+
+	if (adev == &acpi0017_mock)
+		return true;
+
+	for (i = 0; i < ARRAY_SIZE(host_bridge); i++)
+		if (adev == &host_bridge[i])
+			return true;
+
+	return false;
+}
+
+static struct {
+	struct acpi_table_cedt cedt;
+	struct acpi_cedt_chbs chbs[NR_CXL_HOST_BRIDGES];
+	struct {
+		struct acpi_cedt_cfmws cfmws;
+		u32 target[1];
+	} cfmws0;
+	struct {
+		struct acpi_cedt_cfmws cfmws;
+		u32 target[4];
+	} cfmws1;
+	struct {
+		struct acpi_cedt_cfmws cfmws;
+		u32 target[1];
+	} cfmws2;
+	struct {
+		struct acpi_cedt_cfmws cfmws;
+		u32 target[4];
+	} cfmws3;
+} __packed mock_cedt = {
+	.cedt = {
+		.header = {
+			.signature = "CEDT",
+			.length = sizeof(mock_cedt),
+			.revision = 1,
+		},
+	},
+	.chbs[0] = {
+		.header = {
+			.type = ACPI_CEDT_TYPE_CHBS,
+			.length = sizeof(mock_cedt.chbs[0]),
+		},
+		.uid = 0,
+		.cxl_version = ACPI_CEDT_CHBS_VERSION_CXL20,
+	},
+	.chbs[1] = {
+		.header = {
+			.type = ACPI_CEDT_TYPE_CHBS,
+			.length = sizeof(mock_cedt.chbs[0]),
+		},
+		.uid = 1,
+		.cxl_version = ACPI_CEDT_CHBS_VERSION_CXL20,
+	},
+	.chbs[2] = {
+		.header = {
+			.type = ACPI_CEDT_TYPE_CHBS,
+			.length = sizeof(mock_cedt.chbs[0]),
+		},
+		.uid = 2,
+		.cxl_version = ACPI_CEDT_CHBS_VERSION_CXL20,
+	},
+	.chbs[3] = {
+		.header = {
+			.type = ACPI_CEDT_TYPE_CHBS,
+			.length = sizeof(mock_cedt.chbs[0]),
+		},
+		.uid = 3,
+		.cxl_version = ACPI_CEDT_CHBS_VERSION_CXL20,
+	},
+	.cfmws0 = {
+		.cfmws = {
+			.header = {
+				.type = ACPI_CEDT_TYPE_CFMWS,
+				.length = sizeof(mock_cedt.cfmws0),
+			},
+			.interleave_ways = 0,
+			.granularity = 4,
+			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_TYPE3 |
+					ACPI_CEDT_CFMWS_RESTRICT_VOLATILE,
+			.qtg_id = 0,
+		},
+		.target = { 0 },
+	},
+	.cfmws1 = {
+		.cfmws = {
+			.header = {
+				.type = ACPI_CEDT_TYPE_CFMWS,
+				.length = sizeof(mock_cedt.cfmws1),
+			},
+			.interleave_ways = 2,
+			.granularity = 4,
+			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_TYPE3 |
+					ACPI_CEDT_CFMWS_RESTRICT_VOLATILE,
+			.qtg_id = 1,
+		},
+		.target = { 0, 1, 2, 3 },
+	},
+	.cfmws2 = {
+		.cfmws = {
+			.header = {
+				.type = ACPI_CEDT_TYPE_CFMWS,
+				.length = sizeof(mock_cedt.cfmws2),
+			},
+			.interleave_ways = 0,
+			.granularity = 4,
+			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_TYPE3 |
+					ACPI_CEDT_CFMWS_RESTRICT_PMEM,
+			.qtg_id = 2,
+		},
+		.target = { 0 },
+	},
+	.cfmws3 = {
+		.cfmws = {
+			.header = {
+				.type = ACPI_CEDT_TYPE_CFMWS,
+				.length = sizeof(mock_cedt.cfmws3),
+			},
+			.interleave_ways = 2,
+			.granularity = 4,
+			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_TYPE3 |
+					ACPI_CEDT_CFMWS_RESTRICT_PMEM,
+			.qtg_id = 3,
+		},
+		.target = { 0, 1, 2, 3 },
+	},
+};
+
+struct cxl_mock_res {
+	struct list_head list;
+	struct range range;
+};
+
+static LIST_HEAD(mock_res);
+static DEFINE_MUTEX(mock_res_lock);
+static struct gen_pool *cxl_mock_pool;
+
+static void free_mock_res(void)
+{
+	struct cxl_mock_res *res, *_res;
+
+	mutex_lock(&mock_res_lock);
+	list_for_each_entry_safe(res, _res, &mock_res, list) {
+		gen_pool_free(cxl_mock_pool, res->range.start,
+			      range_len(&res->range));
+		list_del(&res->list);
+		kfree(res);
+	}
+	mutex_unlock(&mock_res_lock);
+}
+
+static struct cxl_mock_res *alloc_mock_res(resource_size_t size)
+{
+	struct cxl_mock_res *res = kzalloc(sizeof(*res), GFP_KERNEL);
+	struct genpool_data_align data = {
+		.align = SZ_256M,
+	};
+	unsigned long phys;
+
+	INIT_LIST_HEAD(&res->list);
+	phys = gen_pool_alloc_algo(cxl_mock_pool, size,
+				   gen_pool_first_fit_align, &data);
+	if (!phys)
+		return NULL;
+
+	res->range = (struct range) {
+		.start = phys,
+		.end = phys + size - 1,
+	};
+	mutex_lock(&mock_res_lock);
+	list_add(&res->list, &mock_res);
+	mutex_unlock(&mock_res_lock);
+
+	return res;
+}
+
+static int populate_cedt(void)
+{
+	struct acpi_cedt_cfmws *cfmws[4] = {
+		[0] = &mock_cedt.cfmws0.cfmws,
+		[1] = &mock_cedt.cfmws1.cfmws,
+		[2] = &mock_cedt.cfmws2.cfmws,
+		[3] = &mock_cedt.cfmws3.cfmws,
+	};
+	struct cxl_mock_res *res;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(mock_cedt.chbs); i++) {
+		struct acpi_cedt_chbs *chbs = &mock_cedt.chbs[i];
+		resource_size_t size;
+
+		if (chbs->cxl_version == ACPI_CEDT_CHBS_VERSION_CXL20)
+			size = ACPI_CEDT_CHBS_LENGTH_CXL20;
+		else
+			size = ACPI_CEDT_CHBS_LENGTH_CXL11;
+
+		res = alloc_mock_res(size);
+		if (!res)
+			return -ENOMEM;
+		chbs->base = res->range.start;
+		chbs->length = size;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(cfmws); i++) {
+		struct acpi_cedt_cfmws *window = cfmws[i];
+		int ways = 1 << window->interleave_ways;
+
+		res = alloc_mock_res(SZ_256M * ways);
+		if (!res)
+			return -ENOMEM;
+		window->base_hpa = res->range.start;
+		window->window_size = range_len(&res->range);
+	}
+
+	return 0;
+}
+
+static acpi_status mock_acpi_get_table(char *signature, u32 instance,
+				       struct acpi_table_header **out_table)
+{
+	if (instance < U32_MAX || strcmp(signature, ACPI_SIG_CEDT) != 0)
+		return acpi_get_table(signature, instance, out_table);
+
+	*out_table = (struct acpi_table_header *) &mock_cedt;
+	return AE_OK;
+}
+
+static void mock_acpi_put_table(struct acpi_table_header *table)
+{
+	if (table == (struct acpi_table_header *) &mock_cedt)
+		return;
+	acpi_put_table(table);
+}
+
+static bool is_mock_bridge(struct device *dev)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(cxl_host_bridge); i++)
+		if (dev == &cxl_host_bridge[i]->dev)
+			return true;
+
+	return false;
+}
+
+static int host_bridge_index(struct acpi_device *adev)
+{
+	return adev - host_bridge;
+}
+
+static struct acpi_device *find_host_bridge(acpi_handle handle)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(host_bridge); i++)
+		if (handle == host_bridge[i].handle)
+			return &host_bridge[i];
+	return NULL;
+}
+
+static acpi_status
+mock_acpi_evaluate_integer(acpi_handle handle, acpi_string pathname,
+			   struct acpi_object_list *arguments,
+			   unsigned long long *data)
+{
+	struct acpi_device *adev = find_host_bridge(handle);
+
+	if (!adev || strcmp(pathname, METHOD_NAME__UID) != 0)
+		return acpi_evaluate_integer(handle, pathname, arguments, data);
+
+	*data = host_bridge_index(adev);
+	return AE_OK;
+}
+
+static struct pci_bus mock_pci_bus[NR_CXL_HOST_BRIDGES];
+static struct acpi_pci_root mock_pci_root[NR_CXL_HOST_BRIDGES] = {
+	[0] = {
+		.bus = &mock_pci_bus[0],
+	},
+	[1] = {
+		.bus = &mock_pci_bus[1],
+	},
+	[2] = {
+		.bus = &mock_pci_bus[2],
+	},
+	[3] = {
+		.bus = &mock_pci_bus[3],
+	},
+};
+
+static struct platform_device *mock_cxl_root_port(struct pci_bus *bus, int index)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(mock_pci_bus); i++)
+		if (bus == &mock_pci_bus[i])
+			return cxl_root_port[index + i * NR_CXL_ROOT_PORTS];
+	return NULL;
+}
+
+static bool is_mock_port(struct platform_device *pdev)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(cxl_root_port); i++)
+		if (pdev == cxl_root_port[i])
+			return true;
+	return false;
+}
+
+static bool is_mock_bus(struct pci_bus *bus)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(mock_pci_bus); i++)
+		if (bus == &mock_pci_bus[i])
+			return true;
+	return false;
+}
+
+static struct acpi_pci_root *mock_acpi_pci_find_root(acpi_handle handle)
+{
+	struct acpi_device *adev = find_host_bridge(handle);
+
+	if (!adev)
+		return acpi_pci_find_root(handle);
+	return &mock_pci_root[host_bridge_index(adev)];
+}
+
+static struct cxl_mock_ops cxl_mock_ops = {
+	.is_mock_adev = is_mock_adev,
+	.is_mock_bridge = is_mock_bridge,
+	.is_mock_bus = is_mock_bus,
+	.is_mock_port = is_mock_port,
+	.is_mock_dev = is_mock_dev,
+	.mock_port = mock_cxl_root_port,
+	.acpi_get_table = mock_acpi_get_table,
+	.acpi_put_table = mock_acpi_put_table,
+	.acpi_evaluate_integer = mock_acpi_evaluate_integer,
+	.acpi_pci_find_root = mock_acpi_pci_find_root,
+	.list = LIST_HEAD_INIT(cxl_mock_ops.list),
+};
+
+static void mock_companion(struct acpi_device *adev, struct device *dev)
+{
+	device_initialize(&adev->dev);
+	fwnode_init(&adev->fwnode, NULL);
+	dev->fwnode = &adev->fwnode;
+	adev->fwnode.dev = dev;
+}
+
+#ifndef SZ_64G
+#define SZ_64G (SZ_32G * 2)
+#endif
+
+#ifndef SZ_512G
+#define SZ_512G (SZ_64G * 8)
+#endif
+
+static __init int cxl_test_init(void)
+{
+	int rc, i;
+
+	register_cxl_mock_ops(&cxl_mock_ops);
+
+	cxl_mock_pool = gen_pool_create(ilog2(SZ_2M), NUMA_NO_NODE);
+	if (!cxl_mock_pool) {
+		rc = -ENOMEM;
+		goto err_gen_pool_create;
+	}
+
+	rc = gen_pool_add(cxl_mock_pool, SZ_512G, SZ_64G, NUMA_NO_NODE);
+	if (rc)
+		goto err_gen_pool_add;
+
+	rc = populate_cedt();
+	if (rc)
+		goto err_populate;
+
+	for (i = 0; i < ARRAY_SIZE(cxl_host_bridge); i++) {
+		struct acpi_device *adev = &host_bridge[i];
+		struct platform_device *pdev;
+
+		pdev = platform_device_alloc("cxl_host_bridge", i);
+		if (!pdev)
+			goto err_bridge;
+
+		mock_companion(adev, &pdev->dev);
+		rc = platform_device_add(pdev);
+		if (rc) {
+			platform_device_put(pdev);
+			goto err_bridge;
+		}
+		cxl_host_bridge[i] = pdev;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(cxl_root_port); i++) {
+		struct platform_device *bridge =
+			cxl_host_bridge[i / NR_CXL_ROOT_PORTS];
+		struct platform_device *pdev;
+
+		pdev = platform_device_alloc("cxl_root_port", i);
+		if (!pdev)
+			goto err_port;
+		pdev->dev.parent = &bridge->dev;
+
+		rc = platform_device_add(pdev);
+		if (rc) {
+			platform_device_put(pdev);
+			goto err_port;
+		}
+		cxl_root_port[i] = pdev;
+	}
+
+	cxl_acpi = platform_device_alloc("cxl_acpi", 0);
+	if (!cxl_acpi)
+		goto err_port;
+
+	mock_companion(&acpi0017_mock, &cxl_acpi->dev);
+	acpi0017_mock.dev.bus = &platform_bus_type;
+
+	rc = platform_device_add(cxl_acpi);
+	if (rc)
+		goto err_add;
+
+	return 0;
+
+err_add:
+	platform_device_put(cxl_acpi);
+err_port:
+	for (i = ARRAY_SIZE(cxl_root_port) - 1; i >= 0; i--) {
+		platform_device_del(cxl_root_port[i]);
+		platform_device_put(cxl_root_port[i]);
+	}
+err_bridge:
+	for (i = ARRAY_SIZE(cxl_host_bridge) - 1; i >= 0; i--) {
+		platform_device_del(cxl_host_bridge[i]);
+		platform_device_put(cxl_host_bridge[i]);
+	}
+err_populate:
+	free_mock_res();
+err_gen_pool_add:
+	gen_pool_destroy(cxl_mock_pool);
+err_gen_pool_create:
+	unregister_cxl_mock_ops(&cxl_mock_ops);
+	return rc;
+}
+
+static __exit void cxl_test_exit(void)
+{
+	int i;
+
+	platform_device_del(cxl_acpi);
+	platform_device_put(cxl_acpi);
+	for (i = ARRAY_SIZE(cxl_root_port) - 1; i >= 0; i--) {
+		platform_device_del(cxl_root_port[i]);
+		platform_device_put(cxl_root_port[i]);
+	}
+	for (i = ARRAY_SIZE(cxl_host_bridge) - 1; i >= 0; i--) {
+		platform_device_del(cxl_host_bridge[i]);
+		platform_device_put(cxl_host_bridge[i]);
+	}
+	free_mock_res();
+	gen_pool_destroy(cxl_mock_pool);
+	unregister_cxl_mock_ops(&cxl_mock_ops);
+}
+
+module_init(cxl_test_init);
+module_exit(cxl_test_exit);
+MODULE_LICENSE("GPL v2");
diff --git a/tools/testing/cxl/test/mock.c b/tools/testing/cxl/test/mock.c
new file mode 100644
index 000000000000..b8c108abcf07
--- /dev/null
+++ b/tools/testing/cxl/test/mock.c
@@ -0,0 +1,171 @@ 
+// SPDX-License-Identifier: GPL-2.0-only
+//Copyright(c) 2021 Intel Corporation. All rights reserved.
+
+#include <linux/libnvdimm.h>
+#include <linux/rculist.h>
+#include <linux/device.h>
+#include <linux/export.h>
+#include <linux/acpi.h>
+#include <linux/pci.h>
+#include "mock.h"
+
+static LIST_HEAD(mock);
+
+void register_cxl_mock_ops(struct cxl_mock_ops *ops)
+{
+	list_add_rcu(&ops->list, &mock);
+}
+EXPORT_SYMBOL_GPL(register_cxl_mock_ops);
+
+static DEFINE_SRCU(cxl_mock_srcu);
+
+void unregister_cxl_mock_ops(struct cxl_mock_ops *ops)
+{
+	list_del_rcu(&ops->list);
+	synchronize_srcu(&cxl_mock_srcu);
+}
+EXPORT_SYMBOL_GPL(unregister_cxl_mock_ops);
+
+struct cxl_mock_ops *get_cxl_mock_ops(int *index)
+{
+	*index = srcu_read_lock(&cxl_mock_srcu);
+	return list_first_or_null_rcu(&mock, struct cxl_mock_ops, list);
+}
+EXPORT_SYMBOL_GPL(get_cxl_mock_ops);
+
+void put_cxl_mock_ops(int index)
+{
+	srcu_read_unlock(&cxl_mock_srcu, index);
+}
+EXPORT_SYMBOL_GPL(put_cxl_mock_ops);
+
+bool __wrap_is_acpi_device_node(const struct fwnode_handle *fwnode)
+{
+	struct acpi_device *adev =
+		container_of(fwnode, struct acpi_device, fwnode);
+	int index;
+	struct cxl_mock_ops *ops = get_cxl_mock_ops(&index);
+	bool retval = false;
+
+	if (ops)
+		retval = ops->is_mock_adev(adev);
+
+	if (!retval)
+		retval = is_acpi_device_node(fwnode);
+
+	put_cxl_mock_ops(index);
+	return retval;
+}
+EXPORT_SYMBOL(__wrap_is_acpi_device_node);
+
+acpi_status __wrap_acpi_get_table(char *signature, u32 instance,
+				  struct acpi_table_header **out_table)
+{
+	int index;
+	struct cxl_mock_ops *ops = get_cxl_mock_ops(&index);
+	acpi_status status;
+
+	if (ops)
+		status = ops->acpi_get_table(signature, instance, out_table);
+	else
+		status = acpi_get_table(signature, instance, out_table);
+
+	put_cxl_mock_ops(index);
+
+	return status;
+}
+EXPORT_SYMBOL(__wrap_acpi_get_table);
+
+void __wrap_acpi_put_table(struct acpi_table_header *table)
+{
+	int index;
+	struct cxl_mock_ops *ops = get_cxl_mock_ops(&index);
+
+	if (ops)
+		ops->acpi_put_table(table);
+	else
+		acpi_put_table(table);
+	put_cxl_mock_ops(index);
+}
+EXPORT_SYMBOL(__wrap_acpi_put_table);
+
+acpi_status __wrap_acpi_evaluate_integer(acpi_handle handle,
+					 acpi_string pathname,
+					 struct acpi_object_list *arguments,
+					 unsigned long long *data)
+{
+	int index;
+	struct cxl_mock_ops *ops = get_cxl_mock_ops(&index);
+	acpi_status status;
+
+	if (ops)
+		status = ops->acpi_evaluate_integer(handle, pathname, arguments,
+						    data);
+	else
+		status = acpi_evaluate_integer(handle, pathname, arguments,
+					       data);
+	put_cxl_mock_ops(index);
+
+	return status;
+}
+EXPORT_SYMBOL(__wrap_acpi_evaluate_integer);
+
+struct acpi_pci_root *__wrap_acpi_pci_find_root(acpi_handle handle)
+{
+	int index;
+	struct acpi_pci_root *root;
+	struct cxl_mock_ops *ops = get_cxl_mock_ops(&index);
+
+	if (ops)
+		root = ops->acpi_pci_find_root(handle);
+	else
+		root = acpi_pci_find_root(handle);
+
+	put_cxl_mock_ops(index);
+
+	return root;
+}
+EXPORT_SYMBOL_GPL(__wrap_acpi_pci_find_root);
+
+void __wrap_pci_walk_bus(struct pci_bus *bus,
+			 int (*cb)(struct pci_dev *, void *), void *userdata)
+{
+	int index;
+	struct cxl_mock_ops *ops = get_cxl_mock_ops(&index);
+
+	if (ops && ops->is_mock_bus(bus)) {
+		int rc, i;
+
+		/*
+		 * Simulate 2 root ports per host-bridge and no
+		 * depth recursion.
+		 */
+		for (i = 0; i < 2; i++) {
+			rc = cb((struct pci_dev *) ops->mock_port(bus, i),
+				userdata);
+			if (rc)
+				break;
+		}
+	} else
+		pci_walk_bus(bus, cb, userdata);
+
+	put_cxl_mock_ops(index);
+}
+EXPORT_SYMBOL_GPL(__wrap_pci_walk_bus);
+
+struct nvdimm_bus *
+__wrap_nvdimm_bus_register(struct device *dev,
+			   struct nvdimm_bus_descriptor *nd_desc)
+{
+	int index;
+	struct cxl_mock_ops *ops = get_cxl_mock_ops(&index);
+
+	if (ops && ops->is_mock_dev(dev->parent->parent))
+		nd_desc->provider_name = "cxl_test";
+	put_cxl_mock_ops(index);
+
+	return nvdimm_bus_register(dev, nd_desc);
+}
+EXPORT_SYMBOL_GPL(__wrap_nvdimm_bus_register);
+
+MODULE_LICENSE("GPL v2");
diff --git a/tools/testing/cxl/test/mock.h b/tools/testing/cxl/test/mock.h
new file mode 100644
index 000000000000..805a94cb3fbe
--- /dev/null
+++ b/tools/testing/cxl/test/mock.h
@@ -0,0 +1,27 @@ 
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#include <linux/list.h>
+#include <linux/acpi.h>
+
+struct cxl_mock_ops {
+	struct list_head list;
+	bool (*is_mock_adev)(struct acpi_device *dev);
+	acpi_status (*acpi_get_table)(char *signature, u32 instance,
+				      struct acpi_table_header **out_table);
+	void (*acpi_put_table)(struct acpi_table_header *table);
+	bool (*is_mock_bridge)(struct device *dev);
+	acpi_status (*acpi_evaluate_integer)(acpi_handle handle,
+					     acpi_string pathname,
+					     struct acpi_object_list *arguments,
+					     unsigned long long *data);
+	struct acpi_pci_root *(*acpi_pci_find_root)(acpi_handle handle);
+	struct platform_device *(*mock_port)(struct pci_bus *bus, int index);
+	bool (*is_mock_bus)(struct pci_bus *bus);
+	bool (*is_mock_port)(struct platform_device *pdev);
+	bool (*is_mock_dev)(struct device *dev);
+};
+
+void register_cxl_mock_ops(struct cxl_mock_ops *ops);
+void unregister_cxl_mock_ops(struct cxl_mock_ops *ops);
+struct cxl_mock_ops *get_cxl_mock_ops(int *index);
+void put_cxl_mock_ops(int index);