Re: [PATCH 4/4] cxl/test: Add workaround for cxl_test for cxl_core calling mocked functions

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

On Fri, 4 Apr 2025 15:57:36 -0700
Dave Jiang <dave.jiang@intel.com> wrote:

> When cxl_core calls a cxl_core exported function and that function is
> mocked by cxl_test, the call chain causes a circular dependency issue. Dan
> provided a workaround to avoid this issue. Apply the method to changes from
> the late host bridge uport mapping update changes in order to enable
> cxl-test.
> 
> Suggested-by: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>

Perhaps some more breadcrumbs (e.g. comments in the code) so we remember why this
dance is used?

Otherwise I think this is fine though I'm not entirely sure what happens
wrt to races and the  register_cxl_mock_ops() / unregister paths.

> ---
>  drivers/cxl/core/hdm.c        | 51 +++++++++++++++++++++++++++++------
>  drivers/cxl/cxl.h             | 16 +++++++++++
>  tools/testing/cxl/Kbuild      |  3 ---
>  tools/testing/cxl/test/mock.c | 34 +++++++++++++++--------
>  4 files changed, 82 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> index 70cae4ebf8a4..ed6bdbd6b452 100644
> --- a/drivers/cxl/core/hdm.c
> +++ b/drivers/cxl/core/hdm.c
> @@ -39,14 +39,19 @@ static int add_hdm_decoder(struct cxl_port *port, struct cxl_decoder *cxld,
>  	return 0;
>  }
>  
> -/*
> +/**
> + * __devm_cxl_add_passthrough_decoder - Add passthrough decoder
> + * @port: The cxl_port context
> + *
> + * Return 0 on success or errno on failure.
> + *
>   * Per the CXL specification (8.2.5.12 CXL HDM Decoder Capability Structure)
>   * single ported host-bridges need not publish a decoder capability when a
>   * passthrough decode can be assumed, i.e. all transactions that the uport sees
>   * are claimed and passed to the single dport. Disable the range until the first
>   * CXL region is enumerated / activated.
>   */
> -int devm_cxl_add_passthrough_decoder(struct cxl_port *port)
> +int __devm_cxl_add_passthrough_decoder(struct cxl_port *port)
>  {
>  	struct cxl_switch_decoder *cxlsd;
>  	struct cxl_dport *dport = NULL;
> @@ -73,6 +78,16 @@ int devm_cxl_add_passthrough_decoder(struct cxl_port *port)
>  
>  	return add_hdm_decoder(port, &cxlsd->cxld, single_port_map);
>  }
> +EXPORT_SYMBOL_NS_GPL(__devm_cxl_add_passthrough_decoder, "CXL");
> +
> +cxl_add_pt_decoder_fn _devm_cxl_add_passthrough_decoder =
> +	__devm_cxl_add_passthrough_decoder;
> +EXPORT_SYMBOL_NS_GPL(_devm_cxl_add_passthrough_decoder, "CXL");
> +
> +int devm_cxl_add_passthrough_decoder(struct cxl_port *port)
> +{
> +	return _devm_cxl_add_passthrough_decoder(port);
> +}
>  EXPORT_SYMBOL_NS_GPL(devm_cxl_add_passthrough_decoder, "CXL");

>  void register_cxl_mock_ops(struct cxl_mock_ops *ops)
>  {
>  	list_add_rcu(&ops->list, &mock);
> +	_devm_cxl_setup_hdm = redirect_devm_cxl_setup_hdm;
> +	_devm_cxl_enumerate_decoders = redirect_devm_cxl_enumerate_decoders;
> +	_devm_cxl_add_passthrough_decoder =
> +		redirect_devm_cxl_add_passthrough_decoder;
>  }
>  EXPORT_SYMBOL_GPL(register_cxl_mock_ops);
>  
> @@ -23,6 +35,9 @@ DEFINE_STATIC_SRCU(cxl_mock_srcu);
>  
>  void unregister_cxl_mock_ops(struct cxl_mock_ops *ops)
>  {
> +	_devm_cxl_setup_hdm = __devm_cxl_setup_hdm;
> +	_devm_cxl_enumerate_decoders = __devm_cxl_enumerate_decoders;
> +	_devm_cxl_add_passthrough_decoder = __devm_cxl_add_passthrough_decoder;

I doubt we actually care but should we ever need to reason about this sequence
for some reason, it would be better if it reversed that in register_cxl_mock_ops.

>  	list_del_rcu(&ops->list);
>  	synchronize_srcu(&cxl_mock_srcu);
>  }
> @@ -131,8 +146,8 @@ __wrap_nvdimm_bus_register(struct device *dev,
>  }
>  EXPORT_SYMBOL_GPL(__wrap_nvdimm_bus_register);