Re: [PATCH 12/18] tools/testing/cxl: Define a fixed volatile configuration to parse

[Jonathan Cameron <Jonathan.Cameron@Huawei.com>]

On Sun, 05 Feb 2023 17:03:35 -0800
Dan Williams <dan.j.williams@intel.com> wrote:

> Take two endpoints attached to the first switch on the first host-bridge
> in the cxl_test topology and define a pre-initialized region. This is a
> x2 interleave underneath a x1 CXL Window.
> 
> $ modprobe cxl_test
> $ # cxl list -Ru
> {
>   "region":"region3",
>   "resource":"0xf010000000",
>   "size":"512.00 MiB (536.87 MB)",
>   "interleave_ways":2,
>   "interleave_granularity":4096,
>   "decode_state":"commit"
> }
> 
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>  drivers/cxl/core/core.h      |    3 -
>  drivers/cxl/core/hdm.c       |    3 +
>  drivers/cxl/core/port.c      |    4 +
>  drivers/cxl/cxl.h            |    4 +
>  drivers/cxl/cxlmem.h         |    3 +
>  tools/testing/cxl/test/cxl.c |  146 +++++++++++++++++++++++++++++++++++++++---
>  6 files changed, 148 insertions(+), 15 deletions(-)
> 

> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> index 8130430ffbcf..8d0895cbae93 100644
> --- a/drivers/cxl/core/port.c
> +++ b/drivers/cxl/core/port.c
> @@ -458,6 +458,7 @@ bool is_switch_decoder(struct device *dev)
>  {
>  	return is_root_decoder(dev) || dev->type == &cxl_decoder_switch_type;
>  }
> +EXPORT_SYMBOL_NS_GPL(is_switch_decoder, CXL);
>  
>  struct cxl_decoder *to_cxl_decoder(struct device *dev)
>  {
> @@ -485,6 +486,7 @@ struct cxl_switch_decoder *to_cxl_switch_decoder(struct device *dev)
>  		return NULL;
>  	return container_of(dev, struct cxl_switch_decoder, cxld.dev);
>  }
> +EXPORT_SYMBOL_NS_GPL(to_cxl_switch_decoder, CXL);
>  
>  static void cxl_ep_release(struct cxl_ep *ep)
>  {
> @@ -528,7 +530,7 @@ static const struct device_type cxl_port_type = {
>  
>  bool is_cxl_port(struct device *dev)
>  {
> -	return dev->type == &cxl_port_type;
> +	return dev && dev->type == &cxl_port_type;

Adding that protection just for mocking seems a bit nasty.
Could you push the sanity check out to the caller or am I missing something?
Perhaps worth calling out the reason we'd call this on a NULL dev with a comment
or similar.


>  }
>  EXPORT_SYMBOL_NS_GPL(is_cxl_port, CXL);
>  
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index 9b3765c5c81a..4c6ee6c96f23 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -452,6 +452,7 @@ struct cxl_region_params {
>   * struct cxl_region - CXL region
>   * @dev: This region's device
>   * @id: This region's id. Id is globally unique across all regions
> + * @fixed: At least one decoder in this region was locked down at init
>   * @mode: Endpoint decoder allocation / access mode
>   * @type: Endpoint decoder target type
>   * @cxl_nvb: nvdimm bridge for coordinating @cxlr_pmem setup / shutdown
> @@ -462,6 +463,7 @@ struct cxl_region_params {
>  struct cxl_region {
>  	struct device dev;
>  	int id;
> +	bool fixed;

I was wondering why a mocking patch was changing the region state structure...
It isn't - this isn't used that I can find.

>  	enum cxl_decoder_mode mode;
>  	enum cxl_decoder_type type;
>  	struct cxl_nvdimm_bridge *cxl_nvb;