| Message ID | 20250107141015.3367194-17-rrichter@amd.com |
|---|---|
| State | New |
| Headers | show |
| Series | cxl: Add address translation support and enable AMD Zen5 platforms | expand |
On Tue, Jan 07, 2025 at 03:10:02PM +0100, Robert Richter wrote: > To enable address translation, the calculation of the endpoint > position must use translated HPA ranges. The function > cxl_endpoint_initialize() already uses translation which could be > reused to calculate the endpoint position. > > Use translated HPA address ranges for the calculation of endpoint > position by moving it to cxl_endpoint_initialize(). Create a function > cxl_port_calc_pos() for use in the iterator there, but keep a > simplified version of cxl_calc_interleave_pos() for the > non-auto-discovery code path without address translation since it is > not support there. > > Signed-off-by: Robert Richter <rrichter@amd.com> > --- just one inline question > +static int cxl_calc_interleave_pos(struct cxl_endpoint_decoder *cxled) > +{ > + struct cxl_port *iter, *port = cxled_to_port(cxled); > + struct cxl_memdev *cxlmd = cxled_to_memdev(cxled); > + struct range *range = &cxled->cxld.hpa_range; > + int pos = 0; > + > + /* > + * Address translation is only supported for auto-discovery of > + * decoders. There is no need to support address translation > + * here. > + */ Just clarifying - it's only supported for discovery of already programmed decoders (programmed in BIOS)? i.e. driver-programmed decoders shouldn't need this translation / won't support this type of interleaving? Comment here begs some questions but not necessarily a review blocker. ~Gregory
On 1/7/25 8:10 AM, Robert Richter wrote: > To enable address translation, the calculation of the endpoint > position must use translated HPA ranges. The function > cxl_endpoint_initialize() already uses translation which could be > reused to calculate the endpoint position. > > Use translated HPA address ranges for the calculation of endpoint s/HPA address ranges/HPA ranges/ > position by moving it to cxl_endpoint_initialize(). Create a function > cxl_port_calc_pos() for use in the iterator there, but keep a > simplified version of cxl_calc_interleave_pos() for the > non-auto-discovery code path without address translation since it is > not support there. > > Signed-off-by: Robert Richter <rrichter@amd.com> > --- > drivers/cxl/core/region.c | 66 ++++++++++++++++++++++++++------------- > 1 file changed, 44 insertions(+), 22 deletions(-) > > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c > index 007a2016760d..c1e384e80d10 100644 > --- a/drivers/cxl/core/region.c > +++ b/drivers/cxl/core/region.c > @@ -1813,26 +1813,29 @@ static int find_pos_and_ways(struct cxl_port *port, struct range *range, > } > > /** > - * cxl_calc_interleave_pos() - calculate an endpoint position in a region > - * @cxled: endpoint decoder member of given region > + * cxl_port_calc_pos() - calculate an endpoint position for @port > + * @port: Port the new position is calculated for. > + * @range: The HPA address range for the port. Same as above, but it may be better to write Host Physical address range instead for the extra context. > + * @pos: Current position in the topology. > * > - * The endpoint position is calculated by traversing the topology from > - * the endpoint to the root decoder and iteratively applying this > - * calculation: > + * The endpoint position for the next port is calculated by applying > + * this calculation: > * > * position = position * parent_ways + parent_pos; > * > * ...where @position is inferred from switch and root decoder target lists. > * > + * The endpoint position of region can be calculated by traversing the I would word this as "The endpoint's position in a region can be..." instead since I think that's what you are doing below. This currently reads like the region's position is being found. > + * topology from the endpoint to the root decoder and iteratively > + * applying the function for each port. > + * > * Return: position >= 0 on success > * -ENXIO on failure > */ > -static int cxl_calc_interleave_pos(struct cxl_endpoint_decoder *cxled) > +static int cxl_port_calc_pos(struct cxl_port *port, struct range *range, > + int pos) > { > - struct cxl_port *iter, *port = cxled_to_port(cxled); > - struct cxl_memdev *cxlmd = cxled_to_memdev(cxled); > - struct range *range = &cxled->cxld.hpa_range; > - int parent_ways = 0, parent_pos = 0, pos = 0; > + int parent_ways = 0, parent_pos = 0; > int rc; > > /* > @@ -1864,17 +1867,30 @@ static int cxl_calc_interleave_pos(struct cxl_endpoint_decoder *cxled) > * complex topologies, including those with switches. > */ > > - /* Iterate from endpoint to root_port refining the position */ > - for (iter = port; iter; iter = parent_port_of(iter)) { > - if (is_cxl_root(iter)) > - break; > + if (is_cxl_root(port)) > + return pos; > > - rc = find_pos_and_ways(iter, range, &parent_pos, &parent_ways); > - if (rc) > - return rc; > + rc = find_pos_and_ways(port, range, &parent_pos, &parent_ways); > + if (rc) > + return rc; > > - pos = pos * parent_ways + parent_pos; > - } > + return pos * parent_ways + parent_pos; > +} > + > +static int cxl_calc_interleave_pos(struct cxl_endpoint_decoder *cxled) > +{ > + struct cxl_port *iter, *port = cxled_to_port(cxled); > + struct cxl_memdev *cxlmd = cxled_to_memdev(cxled); > + struct range *range = &cxled->cxld.hpa_range; > + int pos = 0; > + > + /* > + * Address translation is only supported for auto-discovery of > + * decoders. There is no need to support address translation > + * here. > + */ > + for (iter = cxled_to_port(cxled); iter; iter = parent_port_of(iter)) > + pos = cxl_port_calc_pos(iter, range, pos); > > dev_dbg(&cxlmd->dev, > "decoder:%s parent:%s port:%s range:%#llx-%#llx pos:%d\n", > @@ -1892,7 +1908,6 @@ static int cxl_region_sort_targets(struct cxl_region *cxlr) > for (i = 0; i < p->nr_targets; i++) { > struct cxl_endpoint_decoder *cxled = p->targets[i]; > > - cxled->pos = cxl_calc_interleave_pos(cxled); > /* > * Record that sorting failed, but still continue to calc > * cxled->pos so that follow-on code paths can reliably > @@ -3252,6 +3267,7 @@ static int cxl_endpoint_initialize(struct cxl_endpoint_decoder *cxled) > struct cxl_port *parent, *iter = cxled_to_port(cxled); > struct range hpa = cxled->cxld.hpa_range; > struct cxl_decoder *cxld = &cxled->cxld; > + int pos = 0; > > if (!iter || is_cxl_root(iter)) > return -ENXIO; > @@ -3280,16 +3296,22 @@ static int cxl_endpoint_initialize(struct cxl_endpoint_decoder *cxled) > if (cxl_port_calc_hpa(parent, cxld, &hpa)) > return -ENXIO; > > + /* Iterate from endpoint to root_port refining the position */ > + pos = cxl_port_calc_pos(iter, &hpa, pos); > + if (pos < 0) > + return pos; > + > iter = parent; > } > > dev_dbg(cxld->dev.parent, > - "%s:%s: range:%#llx-%#llx\n", > + "%s:%s: range:%#llx-%#llx pos:%d\n", > dev_name(&cxled->cxld.dev), dev_name(&cxld->dev), > - hpa.start, hpa.end); > + hpa.start, hpa.end, pos); > > cxled->cxlrd = to_cxl_root_decoder(&cxld->dev); > cxled->spa_range = hpa; > + cxled->pos = pos; > > return 0; > }
diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c index 007a2016760d..c1e384e80d10 100644 --- a/drivers/cxl/core/region.c +++ b/drivers/cxl/core/region.c @@ -1813,26 +1813,29 @@ static int find_pos_and_ways(struct cxl_port *port, struct range *range, } /** - * cxl_calc_interleave_pos() - calculate an endpoint position in a region - * @cxled: endpoint decoder member of given region + * cxl_port_calc_pos() - calculate an endpoint position for @port + * @port: Port the new position is calculated for. + * @range: The HPA address range for the port. + * @pos: Current position in the topology. * - * The endpoint position is calculated by traversing the topology from - * the endpoint to the root decoder and iteratively applying this - * calculation: + * The endpoint position for the next port is calculated by applying + * this calculation: * * position = position * parent_ways + parent_pos; * * ...where @position is inferred from switch and root decoder target lists. * + * The endpoint position of region can be calculated by traversing the + * topology from the endpoint to the root decoder and iteratively + * applying the function for each port. + * * Return: position >= 0 on success * -ENXIO on failure */ -static int cxl_calc_interleave_pos(struct cxl_endpoint_decoder *cxled) +static int cxl_port_calc_pos(struct cxl_port *port, struct range *range, + int pos) { - struct cxl_port *iter, *port = cxled_to_port(cxled); - struct cxl_memdev *cxlmd = cxled_to_memdev(cxled); - struct range *range = &cxled->cxld.hpa_range; - int parent_ways = 0, parent_pos = 0, pos = 0; + int parent_ways = 0, parent_pos = 0; int rc; /* @@ -1864,17 +1867,30 @@ static int cxl_calc_interleave_pos(struct cxl_endpoint_decoder *cxled) * complex topologies, including those with switches. */ - /* Iterate from endpoint to root_port refining the position */ - for (iter = port; iter; iter = parent_port_of(iter)) { - if (is_cxl_root(iter)) - break; + if (is_cxl_root(port)) + return pos; - rc = find_pos_and_ways(iter, range, &parent_pos, &parent_ways); - if (rc) - return rc; + rc = find_pos_and_ways(port, range, &parent_pos, &parent_ways); + if (rc) + return rc; - pos = pos * parent_ways + parent_pos; - } + return pos * parent_ways + parent_pos; +} + +static int cxl_calc_interleave_pos(struct cxl_endpoint_decoder *cxled) +{ + struct cxl_port *iter, *port = cxled_to_port(cxled); + struct cxl_memdev *cxlmd = cxled_to_memdev(cxled); + struct range *range = &cxled->cxld.hpa_range; + int pos = 0; + + /* + * Address translation is only supported for auto-discovery of + * decoders. There is no need to support address translation + * here. + */ + for (iter = cxled_to_port(cxled); iter; iter = parent_port_of(iter)) + pos = cxl_port_calc_pos(iter, range, pos); dev_dbg(&cxlmd->dev, "decoder:%s parent:%s port:%s range:%#llx-%#llx pos:%d\n", @@ -1892,7 +1908,6 @@ static int cxl_region_sort_targets(struct cxl_region *cxlr) for (i = 0; i < p->nr_targets; i++) { struct cxl_endpoint_decoder *cxled = p->targets[i]; - cxled->pos = cxl_calc_interleave_pos(cxled); /* * Record that sorting failed, but still continue to calc * cxled->pos so that follow-on code paths can reliably @@ -3252,6 +3267,7 @@ static int cxl_endpoint_initialize(struct cxl_endpoint_decoder *cxled) struct cxl_port *parent, *iter = cxled_to_port(cxled); struct range hpa = cxled->cxld.hpa_range; struct cxl_decoder *cxld = &cxled->cxld; + int pos = 0; if (!iter || is_cxl_root(iter)) return -ENXIO; @@ -3280,16 +3296,22 @@ static int cxl_endpoint_initialize(struct cxl_endpoint_decoder *cxled) if (cxl_port_calc_hpa(parent, cxld, &hpa)) return -ENXIO; + /* Iterate from endpoint to root_port refining the position */ + pos = cxl_port_calc_pos(iter, &hpa, pos); + if (pos < 0) + return pos; + iter = parent; } dev_dbg(cxld->dev.parent, - "%s:%s: range:%#llx-%#llx\n", + "%s:%s: range:%#llx-%#llx pos:%d\n", dev_name(&cxled->cxld.dev), dev_name(&cxld->dev), - hpa.start, hpa.end); + hpa.start, hpa.end, pos); cxled->cxlrd = to_cxl_root_decoder(&cxld->dev); cxled->spa_range = hpa; + cxled->pos = pos; return 0; }
To enable address translation, the calculation of the endpoint position must use translated HPA ranges. The function cxl_endpoint_initialize() already uses translation which could be reused to calculate the endpoint position. Use translated HPA address ranges for the calculation of endpoint position by moving it to cxl_endpoint_initialize(). Create a function cxl_port_calc_pos() for use in the iterator there, but keep a simplified version of cxl_calc_interleave_pos() for the non-auto-discovery code path without address translation since it is not support there. Signed-off-by: Robert Richter <rrichter@amd.com> --- drivers/cxl/core/region.c | 66 ++++++++++++++++++++++++++------------- 1 file changed, 44 insertions(+), 22 deletions(-)