| Message ID | 165603878173.551046.17541236959392713646.stgit@dwillia2-xfh (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | CXL PMEM Region Provisioning | expand |
On Thu, 23 Jun 2022 19:46:21 -0700 Dan Williams <dan.j.williams@intel.com> wrote: > Previously the target routing specifics of switch decoders and platfom > CXL window resource tracking of root decoders were factored out of > 'struct cxl_decoder'. While switch decoders translate from SPA to > downstream ports, endpoint decoders translate from SPA to DPA. > > This patch, 3 of 3, adds a 'struct cxl_endpoint_decoder' that tracks an > endpoint-specific Device Physical Address (DPA) resource. For now this > just defines ->dpa_res, a follow-on patch will handle requesting DPA > resource ranges from a device-DPA resource tree. > > Co-developed-by: Ben Widawsky <bwidawsk@kernel.org> > Signed-off-by: Ben Widawsky <bwidawsk@kernel.org> > Signed-off-by: Dan Williams <dan.j.williams@intel.com> > --- > drivers/cxl/core/hdm.c | 12 +++++++++--- > drivers/cxl/core/port.c | 36 +++++++++++++++++++++++++++--------- > drivers/cxl/cxl.h | 15 ++++++++++++++- > tools/testing/cxl/test/cxl.c | 11 +++++++++-- > 4 files changed, 59 insertions(+), 15 deletions(-) > > diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h > index 6dd1e4c57a67..579f2d802396 100644 > int cxl_decoder_add(struct cxl_decoder *cxld, int *target_map); > -struct cxl_decoder *cxl_endpoint_decoder_alloc(struct cxl_port *port); > +struct cxl_endpoint_decoder *cxl_endpoint_decoder_alloc(struct cxl_port *port); > int cxl_decoder_add_locked(struct cxl_decoder *cxld, int *target_map); > int cxl_decoder_autoremove(struct device *host, struct cxl_decoder *cxld); > int cxl_endpoint_autoremove(struct cxl_memdev *cxlmd, struct cxl_port *endpoint); > diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c > index 68288354b419..f52a5dd69d36 100644 > --- a/tools/testing/cxl/test/cxl.c > +++ b/tools/testing/cxl/test/cxl.c > @@ -459,8 +459,15 @@ static int mock_cxl_enumerate_decoders(struct cxl_hdm *cxlhdm) > cxld = ERR_CAST(cxlsd); > else > cxld = &cxlsd->cxld; > - } else > - cxld = cxl_endpoint_decoder_alloc(port); > + } else { > + struct cxl_endpoint_decoder *cxled; > + > + cxled = cxl_endpoint_decoder_alloc(port); > + if (IS_ERR(cxled)) > + cxld = ERR_CAST(cxled); It's my favourite code pattern to moan about today :) Same thing - just handle error here and it'll be easier to read for cost of a few lines of additional code. Few other cases of it in here. > + else > + cxld = &cxled->cxld; > + } > if (IS_ERR(cxld)) { > dev_warn(&port->dev, > "Failed to allocate the decoder\n"); >
Jonathan Cameron wrote: > On Thu, 23 Jun 2022 19:46:21 -0700 > Dan Williams <dan.j.williams@intel.com> wrote: > > > Previously the target routing specifics of switch decoders and platfom > > CXL window resource tracking of root decoders were factored out of > > 'struct cxl_decoder'. While switch decoders translate from SPA to > > downstream ports, endpoint decoders translate from SPA to DPA. > > > > This patch, 3 of 3, adds a 'struct cxl_endpoint_decoder' that tracks an > > endpoint-specific Device Physical Address (DPA) resource. For now this > > just defines ->dpa_res, a follow-on patch will handle requesting DPA > > resource ranges from a device-DPA resource tree. > > > > Co-developed-by: Ben Widawsky <bwidawsk@kernel.org> > > Signed-off-by: Ben Widawsky <bwidawsk@kernel.org> > > Signed-off-by: Dan Williams <dan.j.williams@intel.com> > > --- > > drivers/cxl/core/hdm.c | 12 +++++++++--- > > drivers/cxl/core/port.c | 36 +++++++++++++++++++++++++++--------- > > drivers/cxl/cxl.h | 15 ++++++++++++++- > > tools/testing/cxl/test/cxl.c | 11 +++++++++-- > > 4 files changed, 59 insertions(+), 15 deletions(-) > > > > > > > diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h > > index 6dd1e4c57a67..579f2d802396 100644 > > > > int cxl_decoder_add(struct cxl_decoder *cxld, int *target_map); > > -struct cxl_decoder *cxl_endpoint_decoder_alloc(struct cxl_port *port); > > +struct cxl_endpoint_decoder *cxl_endpoint_decoder_alloc(struct cxl_port *port); > > int cxl_decoder_add_locked(struct cxl_decoder *cxld, int *target_map); > > int cxl_decoder_autoremove(struct device *host, struct cxl_decoder *cxld); > > int cxl_endpoint_autoremove(struct cxl_memdev *cxlmd, struct cxl_port *endpoint); > > diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c > > index 68288354b419..f52a5dd69d36 100644 > > --- a/tools/testing/cxl/test/cxl.c > > +++ b/tools/testing/cxl/test/cxl.c > > @@ -459,8 +459,15 @@ static int mock_cxl_enumerate_decoders(struct cxl_hdm *cxlhdm) > > cxld = ERR_CAST(cxlsd); > > else > > cxld = &cxlsd->cxld; > > - } else > > - cxld = cxl_endpoint_decoder_alloc(port); > > + } else { > > + struct cxl_endpoint_decoder *cxled; > > + > > + cxled = cxl_endpoint_decoder_alloc(port); > > + if (IS_ERR(cxled)) > > + cxld = ERR_CAST(cxled); > > It's my favourite code pattern to moan about today :) > Same thing - just handle error here and it'll be easier to read for cost of a few > lines of additional code. Few other cases of it in here. Done and done.
diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c index 2d1f3e6eebea..2223d151b61b 100644 --- a/drivers/cxl/core/hdm.c +++ b/drivers/cxl/core/hdm.c @@ -224,9 +224,15 @@ int devm_cxl_enumerate_decoders(struct cxl_hdm *cxlhdm) int rc, target_count = cxlhdm->target_count; struct cxl_decoder *cxld; - if (is_cxl_endpoint(port)) - cxld = cxl_endpoint_decoder_alloc(port); - else { + if (is_cxl_endpoint(port)) { + struct cxl_endpoint_decoder *cxled; + + cxled = cxl_endpoint_decoder_alloc(port); + if (IS_ERR(cxled)) + cxld = ERR_CAST(cxled); + else + cxld = &cxled->cxld; + } else { struct cxl_switch_decoder *cxlsd; cxlsd = cxl_switch_decoder_alloc(port, target_count); diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c index abf3455c4eff..b5f5fb9aa4b7 100644 --- a/drivers/cxl/core/port.c +++ b/drivers/cxl/core/port.c @@ -243,12 +243,12 @@ static void __cxl_decoder_release(struct cxl_decoder *cxld) put_device(&port->dev); } -static void cxl_decoder_release(struct device *dev) +static void cxl_endpoint_decoder_release(struct device *dev) { - struct cxl_decoder *cxld = to_cxl_decoder(dev); + struct cxl_endpoint_decoder *cxled = to_cxl_endpoint_decoder(dev); - __cxl_decoder_release(cxld); - kfree(cxld); + __cxl_decoder_release(&cxled->cxld); + kfree(cxled); } static void cxl_switch_decoder_release(struct device *dev) @@ -278,7 +278,7 @@ static void cxl_root_decoder_release(struct device *dev) static const struct device_type cxl_decoder_endpoint_type = { .name = "cxl_decoder_endpoint", - .release = cxl_decoder_release, + .release = cxl_endpoint_decoder_release, .groups = cxl_decoder_endpoint_attribute_groups, }; @@ -320,6 +320,15 @@ struct cxl_decoder *to_cxl_decoder(struct device *dev) } EXPORT_SYMBOL_NS_GPL(to_cxl_decoder, CXL); +struct cxl_endpoint_decoder *to_cxl_endpoint_decoder(struct device *dev) +{ + if (dev_WARN_ONCE(dev, !is_endpoint_decoder(dev), + "not a cxl_endpoint_decoder device\n")) + return NULL; + return container_of(dev, struct cxl_endpoint_decoder, cxld.dev); +} +EXPORT_SYMBOL_NS_GPL(to_cxl_endpoint_decoder, CXL); + static struct cxl_switch_decoder *to_cxl_switch_decoder(struct device *dev) { if (dev_WARN_ONCE(dev, !is_switch_decoder(dev), @@ -1258,8 +1267,12 @@ static struct cxl_decoder *cxl_decoder_alloc(struct cxl_port *port, cxld = &cxlsd->cxld; } } else { - alloc = kzalloc(sizeof(*cxld), GFP_KERNEL); - cxld = alloc; + struct cxl_endpoint_decoder *cxled; + + alloc = kzalloc(sizeof(*cxled), GFP_KERNEL); + cxled = alloc; + if (cxled) + cxld = &cxled->cxld; } if (!alloc) return ERR_PTR(-ENOMEM); @@ -1357,12 +1370,17 @@ EXPORT_SYMBOL_NS_GPL(cxl_switch_decoder_alloc, CXL); * * Return: A new cxl decoder to be registered by cxl_decoder_add() */ -struct cxl_decoder *cxl_endpoint_decoder_alloc(struct cxl_port *port) +struct cxl_endpoint_decoder *cxl_endpoint_decoder_alloc(struct cxl_port *port) { + struct cxl_decoder *cxld; + if (!is_cxl_endpoint(port)) return ERR_PTR(-EINVAL); - return cxl_decoder_alloc(port, 0); + cxld = cxl_decoder_alloc(port, 0); + if (IS_ERR(cxld)) + return ERR_CAST(cxld); + return to_cxl_endpoint_decoder(&cxld->dev); } EXPORT_SYMBOL_NS_GPL(cxl_endpoint_decoder_alloc, CXL); diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h index 6dd1e4c57a67..579f2d802396 100644 --- a/drivers/cxl/cxl.h +++ b/drivers/cxl/cxl.h @@ -239,6 +239,18 @@ struct cxl_decoder { unsigned long flags; }; +/** + * struct cxl_endpoint_decoder - Endpoint / SPA to DPA decoder + * @cxld: base cxl_decoder_object + * @dpa_res: actively claimed DPA span of this decoder + * @skip: offset into @dpa_res where @cxld.hpa_range maps + */ +struct cxl_endpoint_decoder { + struct cxl_decoder cxld; + struct resource *dpa_res; + resource_size_t skip; +}; + /** * struct cxl_switch_decoder - Switch specific CXL HDM Decoder * @cxld: base cxl_decoder object @@ -379,6 +391,7 @@ struct cxl_dport *cxl_find_dport_by_dev(struct cxl_port *port, struct cxl_decoder *to_cxl_decoder(struct device *dev); struct cxl_root_decoder *to_cxl_root_decoder(struct device *dev); +struct cxl_endpoint_decoder *to_cxl_endpoint_decoder(struct device *dev); bool is_root_decoder(struct device *dev); bool is_endpoint_decoder(struct device *dev); struct cxl_root_decoder *cxl_root_decoder_alloc(struct cxl_port *port, @@ -386,7 +399,7 @@ struct cxl_root_decoder *cxl_root_decoder_alloc(struct cxl_port *port, struct cxl_switch_decoder *cxl_switch_decoder_alloc(struct cxl_port *port, unsigned int nr_targets); int cxl_decoder_add(struct cxl_decoder *cxld, int *target_map); -struct cxl_decoder *cxl_endpoint_decoder_alloc(struct cxl_port *port); +struct cxl_endpoint_decoder *cxl_endpoint_decoder_alloc(struct cxl_port *port); int cxl_decoder_add_locked(struct cxl_decoder *cxld, int *target_map); int cxl_decoder_autoremove(struct device *host, struct cxl_decoder *cxld); int cxl_endpoint_autoremove(struct cxl_memdev *cxlmd, struct cxl_port *endpoint); diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c index 68288354b419..f52a5dd69d36 100644 --- a/tools/testing/cxl/test/cxl.c +++ b/tools/testing/cxl/test/cxl.c @@ -459,8 +459,15 @@ static int mock_cxl_enumerate_decoders(struct cxl_hdm *cxlhdm) cxld = ERR_CAST(cxlsd); else cxld = &cxlsd->cxld; - } else - cxld = cxl_endpoint_decoder_alloc(port); + } else { + struct cxl_endpoint_decoder *cxled; + + cxled = cxl_endpoint_decoder_alloc(port); + if (IS_ERR(cxled)) + cxld = ERR_CAST(cxled); + else + cxld = &cxled->cxld; + } if (IS_ERR(cxld)) { dev_warn(&port->dev, "Failed to allocate the decoder\n");