Re: [PATCH v2 10/15] cxl: define a driver interface for DPA allocation

Jonathan Cameron
2024-08-04T18:07:26Z

On Mon, 15 Jul 2024 18:28:30 +0100
alejandro.lucero-palau@amd.com wrote:

> From: Alejandro Lucero <alucerop@amd.com>
>
> Region creation involves finding available DPA (device-physical-address)
> capacity to map into HPA (host-physical-address) space. Given the HPA
> capacity constraint, define an API, cxl_request_dpa(), that has the
> flexibility to map the minimum amount of memory the driver needs to
> operate vs the total possible that can be mapped given HPA availability.
>
> Factor out the core of cxl_dpa_alloc, that does free space scanning,
> into a cxl_dpa_freespace() helper, and use that to balance the capacity
> available to map vs the @min and @max arguments to cxl_request_dpa.
>
> Based on https://lore.kernel.org/linux-cxl/168592149709.1948938.8663425987110396027.stgit@dwillia2-xfh.jf.intel.com/T/#m4271ee49a91615c8af54e3ab20679f8be3099393
>
Use the permalink link under these to get shorter links.
https://lore.kernel.org/linux-cxl/168592158743.1948938.7622563891193802610.stgit@dwillia2-xfh.jf.intel.com/
goes to the same patch.

> Signed-off-by: Alejandro Lucero <alucerop@amd.com>
> Co-developed-by: Dan Williams <dan.j.williams@intel.com>

> +
> +int cxl_dpa_alloc(struct cxl_endpoint_decoder *cxled, unsigned long long size)
> +{
> + struct cxl_port *port = cxled_to_port(cxled);
> + struct device *dev = &cxled->cxld.dev;
> + resource_size_t start, avail, skip;
> + int rc;
> +
> + down_write(&cxl_dpa_rwsem);

Some cleanup.h magic would help here by allowing early returns.
Needs the scoped lock though to ensure it's released before the
devm_add_action_or_reset() as I'd guess we will deadlock otherwise
if that fails.

> + if (cxled->cxld.region) {
> + dev_dbg(dev, "EBUSY, decoder attached to %s\n",
> + dev_name(&cxled->cxld.region->dev));
> + rc = -EBUSY;
> goto out;
> }
>
> + if (cxled->cxld.flags & CXL_DECODER_F_ENABLE) {
> + dev_dbg(dev, "EBUSY, decoder enabled\n");
> + rc = -EBUSY;
> + goto out;
> + }
> +
> + avail = cxl_dpa_freespace(cxled, &start, &skip);
> +
> if (size > avail) {
> dev_dbg(dev, "%pa exceeds available %s capacity: %pa\n", &size,
> - cxl_decoder_mode_name(cxled->mode), &avail);
> + cxled->mode == CXL_DECODER_RAM ? "ram" : "pmem",
> + &avail);
> rc = -ENOSPC;
> goto out;
> }
> @@ -550,6 +570,99 @@ int cxl_dpa_alloc(struct cxl_endpoint_decoder *cxled, unsigned long long size)
> return devm_add_action_or_reset(&port->dev, cxl_dpa_release, cxled);
> }
>
> +static int find_free_decoder(struct device *dev, void *data)
> +{
> + struct cxl_endpoint_decoder *cxled;
> + struct cxl_port *port;
> +
> + if (!is_endpoint_decoder(dev))
> + return 0;
> +
> + cxled = to_cxl_endpoint_decoder(dev);
> + port = cxled_to_port(cxled);
> +
> + if (cxled->cxld.id != port->hdm_end + 1) {
> + return 0;

No brackets

> + }
> + return 1;
> +}
> +
> +/**
> + * cxl_request_dpa - search and reserve DPA given input constraints
> + * @endpoint: an endpoint port with available decoders
> + * @mode: DPA operation mode (ram vs pmem)
> + * @min: the minimum amount of capacity the call needs
> + * @max: extra capacity to allocate after min is satisfied
> + *
> + * Given that a region needs to allocate from limited HPA capacity it
> + * may be the case that a device has more mappable DPA capacity than
> + * available HPA. So, the expectation is that @min is a driver known
> + * value for how much capacity is needed, and @max is based the limit of
> + * how much HPA space is available for a new region.
We are going to need a policy control on the max value.
Otherwise, if you have two devices that support huge capacity and
not enough space, who gets it will just be a race.

Not a problem for now though!

> + *
> + * Returns a pinned cxl_decoder with at least @min bytes of capacity
> + * reserved, or an error pointer. The caller is also expected to own the
> + * lifetime of the memdev registration associated with the endpoint to
> + * pin the decoder registered as well.
> + */