Re: [PATCH v7 02/11] cxl: Cache decoder settings on PCI devices

["Dan Williams (nvidia)" <djbw@kernel.org>]

Srirangan Madhavan wrote:
> Cache CXL core's HDM decoder settings in pci_dev->hdm as decoders are
> enumerated, committed, or reset. PCI reset paths can use this snapshot to
> restore HDM programming without walking CXL topology during reset recovery.
> 
> Signed-off-by: Srirangan Madhavan <smadhavan@nvidia.com>
> ---
>  drivers/cxl/core/hdm.c | 81 +++++++++++++++++++++++++++++++++++++++++-
>  include/cxl/cxl.h      | 12 +++++++
>  include/linux/pci.h    |  6 ++++
>  3 files changed, 98 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> index fa978c297546..83cda63f76a5 100644
> --- a/drivers/cxl/core/hdm.c
> +++ b/drivers/cxl/core/hdm.c
> @@ -84,6 +84,76 @@ static void parse_hdm_decoder_caps(struct cxl_hdm *cxlhdm)
>  		cxlhdm->iw_cap_mask |= BIT(16);
>  }
>  
> +static void clear_hdm_info(void *data)
> +{
> +	struct pci_dev *pdev = data;
> +
> +	WRITE_ONCE(pdev->hdm, NULL);
> +}
> +
> +static int devm_cxl_pci_setup_hdm_info(struct cxl_hdm *cxlhdm)
> +{
> +	struct cxl_port *port = cxlhdm->port;
> +	struct cxl_hdm_info *info;
> +	struct pci_dev *pdev;
> +	struct device *uport;
> +
> +	if (is_cxl_endpoint(port)) {
> +		struct cxl_memdev *cxlmd = to_cxl_memdev(port->uport_dev);
> +
> +		uport = cxlmd->dev.parent;
> +	} else {
> +		uport = port->uport_dev;
> +	}
> +
> +	if (!dev_is_pci(uport))
> +		return 0;
> +
> +	pdev = to_pci_dev(uport);
> +	info = devm_kzalloc(&pdev->dev,
> +			    struct_size(info, settings, cxlhdm->decoder_count),
> +			    GFP_KERNEL);
> +	if (!info)
> +		return -ENOMEM;
> +
> +	info->decoder_count = cxlhdm->decoder_count;
> +	WRITE_ONCE(pdev->hdm, info);
> +
> +	return devm_add_action_or_reset(&pdev->dev, clear_hdm_info, pdev);

The CXL core can update the PCI cached HDM settings under the device
lock, but it should not be doing its own allocation. It also should not
clear the cached settings on shutdown. That would defeat the purpose of
having the HDM settings available while the device is disabled.