| Message ID | 169698641993.1991735.8821776839011657844.stgit@djiang5-mobl3 |
|---|---|
| State | Superseded |
| Headers | show |
| Series | cxl: Add support for QTG ID retrieval for CXL subsystem | expand |
On Tue, 10 Oct 2023 18:06:59 -0700 Dave Jiang <dave.jiang@intel.com> wrote: > Once the QTG ID _DSM is executed successfully, the QTG ID is retrieved from > the return package. Create a list of entries in the cxl_memdev context and > store the QTG ID as qos_class token and the associated DPA range. This > information can be exposed to user space via sysfs in order to help region > setup for hot-plugged CXL memory devices. > > Signed-off-by: Dave Jiang <dave.jiang@intel.com> > > --- > v10: > - Store single qos_class value. (Dan) I'm fine with doing this, but I'd like a print if more than one is provided by the DSMAS (e.g. multiple DSMAS entries for a given region) Mostly so we have something to let us know if anyone is shipping such a device. Otherwise a couple of minor comments inline. Jonathan > - Rename cxl_memdev_set_qtg() to cxl_memdev_set_qos_class() > - Removed Jonathan's review tag due to code changes. > diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c > index 99a619360bc5..049a16b7eb1f 100644 > --- a/drivers/cxl/port.c > +++ b/drivers/cxl/port.c > @@ -105,6 +105,40 @@ static int cxl_port_perf_data_calculate(struct cxl_port *port, > return 0; > } > > +static void cxl_memdev_set_qos_class(struct cxl_dev_state *cxlds, > + struct list_head *dsmas_list) > +{ > + struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlds); > + struct range pmem_range = { > + .start = cxlds->pmem_res.start, > + .end = cxlds->pmem_res.end, > + }; > + struct range ram_range = { > + .start = cxlds->ram_res.start, > + .end = cxlds->ram_res.end, > + }; > + struct perf_prop_entry *perf; > + struct dsmas_entry *dent; > + > + list_for_each_entry(dent, dsmas_list, list) { > + perf = devm_kzalloc(cxlds->dev, sizeof(*perf), GFP_KERNEL); > + if (!perf) > + return; > + > + perf->dpa_range = dent->dpa_range; > + perf->coord = dent->coord; > + perf->qos_class = dent->qos_class; > + list_add_tail(&perf->list, &mds->perf_list); > + > + if (resource_size(&cxlds->ram_res) && > + range_contains(&ram_range, &dent->dpa_range)) > + mds->ram_qos_class = perf->qos_class; So this assumes one DSMAS per memory type. Not an unreasonable starting place, but I think this should print something to the log if it does see more that one. > + else if (resource_size(&cxlds->pmem_res) && > + range_contains(&pmem_range, &dent->dpa_range)) > + mds->pmem_qos_class = perf->qos_class; > + } > +} > + > static int cxl_switch_port_probe(struct cxl_port *port) > { > struct cxl_hdm *cxlhdm; > @@ -201,6 +235,8 @@ static int cxl_endpoint_port_probe(struct cxl_port *port) > if (rc) > dev_dbg(&port->dev, > "Failed to do perf coord calculations.\n"); > + else > + cxl_memdev_set_qos_class(cxlds, &dsmas_list); This is getting a bit deeply nested. Perhaps a follow up patch to factor it out makes sense so we can use a goto to cleanup the dmsmas_list without that label being nested as well. > } > > cxl_cdat_dsmas_list_destroy(&dsmas_list); > >
On 10/11/23 06:19, Jonathan Cameron wrote: > On Tue, 10 Oct 2023 18:06:59 -0700 > Dave Jiang <dave.jiang@intel.com> wrote: > >> Once the QTG ID _DSM is executed successfully, the QTG ID is retrieved from >> the return package. Create a list of entries in the cxl_memdev context and >> store the QTG ID as qos_class token and the associated DPA range. This >> information can be exposed to user space via sysfs in order to help region >> setup for hot-plugged CXL memory devices. >> >> Signed-off-by: Dave Jiang <dave.jiang@intel.com> >> >> --- >> v10: >> - Store single qos_class value. (Dan) > > I'm fine with doing this, but I'd like a print if more than one is > provided by the DSMAS (e.g. multiple DSMAS entries for a given region) > Mostly so we have something to let us know if anyone is shipping such a > device. > > Otherwise a couple of minor comments inline. > > Jonathan > > >> - Rename cxl_memdev_set_qtg() to cxl_memdev_set_qos_class() >> - Removed Jonathan's review tag due to code changes. > >> diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c >> index 99a619360bc5..049a16b7eb1f 100644 >> --- a/drivers/cxl/port.c >> +++ b/drivers/cxl/port.c >> @@ -105,6 +105,40 @@ static int cxl_port_perf_data_calculate(struct cxl_port *port, >> return 0; >> } >> >> +static void cxl_memdev_set_qos_class(struct cxl_dev_state *cxlds, >> + struct list_head *dsmas_list) >> +{ >> + struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlds); >> + struct range pmem_range = { >> + .start = cxlds->pmem_res.start, >> + .end = cxlds->pmem_res.end, >> + }; >> + struct range ram_range = { >> + .start = cxlds->ram_res.start, >> + .end = cxlds->ram_res.end, >> + }; >> + struct perf_prop_entry *perf; >> + struct dsmas_entry *dent; >> + >> + list_for_each_entry(dent, dsmas_list, list) { >> + perf = devm_kzalloc(cxlds->dev, sizeof(*perf), GFP_KERNEL); >> + if (!perf) >> + return; >> + >> + perf->dpa_range = dent->dpa_range; >> + perf->coord = dent->coord; >> + perf->qos_class = dent->qos_class; >> + list_add_tail(&perf->list, &mds->perf_list); >> + >> + if (resource_size(&cxlds->ram_res) && >> + range_contains(&ram_range, &dent->dpa_range)) >> + mds->ram_qos_class = perf->qos_class; > > So this assumes one DSMAS per memory type. > Not an unreasonable starting place, but I think this should > print something to the log if it does see more that one. I'll add that. Also I seem to have dropped the check from before for multiple entries so we aren't clobbering the previous set value. > >> + else if (resource_size(&cxlds->pmem_res) && >> + range_contains(&pmem_range, &dent->dpa_range)) >> + mds->pmem_qos_class = perf->qos_class; >> + } >> +} >> + >> static int cxl_switch_port_probe(struct cxl_port *port) >> { >> struct cxl_hdm *cxlhdm; >> @@ -201,6 +235,8 @@ static int cxl_endpoint_port_probe(struct cxl_port *port) >> if (rc) >> dev_dbg(&port->dev, >> "Failed to do perf coord calculations.\n"); >> + else >> + cxl_memdev_set_qos_class(cxlds, &dsmas_list); > > This is getting a bit deeply nested. Perhaps a follow up patch to factor > it out makes sense so we can use a goto to cleanup the dmsmas_list without > that label being nested as well. I'll just fix it in place. It doesn't look too bad. > >> } >> >> cxl_cdat_dsmas_list_destroy(&dsmas_list); >> >> >
diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c index 4df4f614f490..6193b8d57469 100644 --- a/drivers/cxl/core/mbox.c +++ b/drivers/cxl/core/mbox.c @@ -1378,6 +1378,7 @@ struct cxl_memdev_state *cxl_memdev_state_create(struct device *dev) mutex_init(&mds->event.log_lock); mds->cxlds.dev = dev; mds->cxlds.type = CXL_DEVTYPE_CLASSMEM; + INIT_LIST_HEAD(&mds->perf_list); return mds; } diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h index 706f8a6d1ef4..a310a51a3fa4 100644 --- a/drivers/cxl/cxlmem.h +++ b/drivers/cxl/cxlmem.h @@ -6,6 +6,7 @@ #include <linux/cdev.h> #include <linux/uuid.h> #include <linux/rcuwait.h> +#include <linux/node.h> #include "cxl.h" /* CXL 2.0 8.2.8.5.1.1 Memory Device Status Register */ @@ -388,6 +389,20 @@ enum cxl_devtype { CXL_DEVTYPE_CLASSMEM, }; +/** + * struct perf_prop - performance property entry + * @list - list entry + * @dpa_range - range for DPA address + * @coord - QoS performance data (i.e. latency, bandwidth) + * @qos_class - QoS Class cookies + */ +struct perf_prop_entry { + struct list_head list; + struct range dpa_range; + struct access_coordinate coord; + int qos_class; +}; + /** * struct cxl_dev_state - The driver device state * @@ -452,6 +467,9 @@ struct cxl_dev_state { * @security: security driver state info * @fw: firmware upload / activation state * @mbox_send: @dev specific transport for transmitting mailbox commands + * @ram_qos_class: QoS class cookies for volatile region + * @pmem_qos_class: QoS class cookies for persistent region + * @perf_list: performance data entries list * * See CXL 3.0 8.2.9.8.2 Capacity Configuration and Label Storage for * details on capacity parameters. @@ -472,6 +490,11 @@ struct cxl_memdev_state { u64 active_persistent_bytes; u64 next_volatile_bytes; u64 next_persistent_bytes; + + int ram_qos_class; + int pmem_qos_class; + struct list_head perf_list; + struct cxl_event_state event; struct cxl_poison_state poison; struct cxl_security_state security; diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c index 99a619360bc5..049a16b7eb1f 100644 --- a/drivers/cxl/port.c +++ b/drivers/cxl/port.c @@ -105,6 +105,40 @@ static int cxl_port_perf_data_calculate(struct cxl_port *port, return 0; } +static void cxl_memdev_set_qos_class(struct cxl_dev_state *cxlds, + struct list_head *dsmas_list) +{ + struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlds); + struct range pmem_range = { + .start = cxlds->pmem_res.start, + .end = cxlds->pmem_res.end, + }; + struct range ram_range = { + .start = cxlds->ram_res.start, + .end = cxlds->ram_res.end, + }; + struct perf_prop_entry *perf; + struct dsmas_entry *dent; + + list_for_each_entry(dent, dsmas_list, list) { + perf = devm_kzalloc(cxlds->dev, sizeof(*perf), GFP_KERNEL); + if (!perf) + return; + + perf->dpa_range = dent->dpa_range; + perf->coord = dent->coord; + perf->qos_class = dent->qos_class; + list_add_tail(&perf->list, &mds->perf_list); + + if (resource_size(&cxlds->ram_res) && + range_contains(&ram_range, &dent->dpa_range)) + mds->ram_qos_class = perf->qos_class; + else if (resource_size(&cxlds->pmem_res) && + range_contains(&pmem_range, &dent->dpa_range)) + mds->pmem_qos_class = perf->qos_class; + } +} + static int cxl_switch_port_probe(struct cxl_port *port) { struct cxl_hdm *cxlhdm; @@ -201,6 +235,8 @@ static int cxl_endpoint_port_probe(struct cxl_port *port) if (rc) dev_dbg(&port->dev, "Failed to do perf coord calculations.\n"); + else + cxl_memdev_set_qos_class(cxlds, &dsmas_list); } cxl_cdat_dsmas_list_destroy(&dsmas_list);
Once the QTG ID _DSM is executed successfully, the QTG ID is retrieved from the return package. Create a list of entries in the cxl_memdev context and store the QTG ID as qos_class token and the associated DPA range. This information can be exposed to user space via sysfs in order to help region setup for hot-plugged CXL memory devices. Signed-off-by: Dave Jiang <dave.jiang@intel.com> --- v10: - Store single qos_class value. (Dan) - Rename cxl_memdev_set_qtg() to cxl_memdev_set_qos_class() - Removed Jonathan's review tag due to code changes. --- drivers/cxl/core/mbox.c | 1 + drivers/cxl/cxlmem.h | 23 +++++++++++++++++++++++ drivers/cxl/port.c | 36 ++++++++++++++++++++++++++++++++++++ 3 files changed, 60 insertions(+)