| Message ID | 20230224194652.1990604-4-dave@stgolabs.net |
|---|---|
| State | Superseded |
| Headers | show |
| Series | cxl: Background cmds and device sanitation | expand |
On Fri, Feb 24, 2023 at 11:46:48AM -0800, Davidlohr Bueso wrote: > Track all regions associated to a memdev in order to > tell if the device might be in active use. Hi David, I took a look here as I've been poked around in this space when looking for 'region' names to associate with memdev poison trace events. [1] How does the list created get used? If we only need to know that this memdev is mapped in any region, so don't touch it, we can look at it's port->commit_end. If that commit_end >= 0, we can look at each endpoint to find the regions it's mapped to. Not sure if any of that is useful for what you need to do, but it sounded familiar. [1] https://lore.kernel.org/linux-cxl/62d24b380514c8c39b651aca79c81a424f0b5b37.1676685180.git.alison.schofield@intel.com/ Alison > > Signed-off-by: Davidlohr Bueso <dave@stgolabs.net> > --- > drivers/cxl/core/memdev.c | 1 + > drivers/cxl/core/region.c | 33 +++++++++++++++++++++++++++++++-- > drivers/cxl/cxl.h | 6 ++++++ > drivers/cxl/cxlmem.h | 4 ++++ > 4 files changed, 42 insertions(+), 2 deletions(-) > > diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c > index 47cc625bb1b0..68c0ab06b999 100644 > --- a/drivers/cxl/core/memdev.c > +++ b/drivers/cxl/core/memdev.c > @@ -306,6 +306,7 @@ static struct cxl_memdev *cxl_memdev_alloc(struct cxl_dev_state *cxlds, > dev->type = &cxl_memdev_type; > device_set_pm_not_required(dev); > INIT_WORK(&cxlmd->detach_work, detach_memdev); > + INIT_LIST_HEAD(&cxlmd->region_list); > > cdev = &cxlmd->cdev; > cdev_init(cdev, fops); > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c > index f29028148806..cea9de6457b9 100644 > --- a/drivers/cxl/core/region.c > +++ b/drivers/cxl/core/region.c > @@ -1730,7 +1730,10 @@ void cxl_decoder_kill_region(struct cxl_endpoint_decoder *cxled) > { > down_write(&cxl_region_rwsem); > cxled->mode = CXL_DECODER_DEAD; > - cxl_region_detach(cxled); > + if (!cxl_region_detach(cxled)) { > + struct cxl_region *cxlr = cxled->cxld.region; > + list_del(&cxlr->node); > + } > up_write(&cxl_region_rwsem); > } > > @@ -1749,8 +1752,12 @@ static int attach_target(struct cxl_region *cxlr, > > down_read(&cxl_dpa_rwsem); > rc = cxl_region_attach(cxlr, cxled, pos); > - if (rc == 0) > + if (rc == 0) { > + struct cxl_memdev *cxlmd = cxled_to_memdev(cxled); > + > set_bit(CXL_REGION_F_INCOHERENT, &cxlr->flags); > + list_add_tail(&cxlr->node, &cxlmd->region_list); > + } > up_read(&cxl_dpa_rwsem); > up_write(&cxl_region_rwsem); > return rc; > @@ -1778,6 +1785,8 @@ static int detach_target(struct cxl_region *cxlr, int pos) > } > > rc = cxl_region_detach(p->targets[pos]); > + if (rc == 0) > + list_del(&cxlr->node); > out: > up_write(&cxl_region_rwsem); > return rc; > @@ -2654,6 +2663,26 @@ int cxl_add_to_region(struct cxl_port *root, struct cxl_endpoint_decoder *cxled) > } > EXPORT_SYMBOL_NS_GPL(cxl_add_to_region, CXL); > > +bool cxl_memdev_active_region(struct cxl_memdev *cxlmd) > +{ > + bool ret = false; > + struct cxl_region *cxlr; > + > + down_read(&cxl_region_rwsem); > + list_for_each_entry(cxlr, &cxlmd->region_list, node) { > + struct cxl_region_params *p = &cxlr->params; > + > + if (p->state >= CXL_CONFIG_ACTIVE) { > + ret = true; > + break; > + } > + } > + up_read(&cxl_region_rwsem); > + > + return ret; > +} > +EXPORT_SYMBOL_NS_GPL(cxl_memdev_active_region, CXL); > + > static int cxl_region_invalidate_memregion(struct cxl_region *cxlr) > { > if (!test_bit(CXL_REGION_F_INCOHERENT, &cxlr->flags)) > diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h > index b834e55375e3..e211241b079b 100644 > --- a/drivers/cxl/cxl.h > +++ b/drivers/cxl/cxl.h > @@ -502,6 +502,7 @@ struct cxl_region { > struct cxl_pmem_region *cxlr_pmem; > unsigned long flags; > struct cxl_region_params params; > + struct list_head node; > }; > > struct cxl_nvdimm_bridge { > @@ -773,6 +774,7 @@ struct cxl_pmem_region *to_cxl_pmem_region(struct device *dev); > int cxl_add_to_region(struct cxl_port *root, > struct cxl_endpoint_decoder *cxled); > struct cxl_dax_region *to_cxl_dax_region(struct device *dev); > +bool cxl_memdev_active_region(struct cxl_memdev *cxlmd); > #else > static inline bool is_cxl_pmem_region(struct device *dev) > { > @@ -791,6 +793,10 @@ static inline struct cxl_dax_region *to_cxl_dax_region(struct device *dev) > { > return NULL; > } > +static inline bool cxl_memdev_active_region(struct cxl_memdev *cxlmd) > +{ > + return false; > +} > #endif > > /* > diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h > index 934076254d52..4e31f3234519 100644 > --- a/drivers/cxl/cxlmem.h > +++ b/drivers/cxl/cxlmem.h > @@ -5,6 +5,7 @@ > #include <uapi/linux/cxl_mem.h> > #include <linux/cdev.h> > #include <linux/uuid.h> > +#include <linux/list.h> > #include "cxl.h" > > /* CXL 2.0 8.2.8.5.1.1 Memory Device Status Register */ > @@ -40,6 +41,8 @@ > * @cxl_nvd: optional bridge to an nvdimm if the device supports pmem > * @id: id number of this memdev instance. > * @depth: endpoint port depth > + * @region_list: List of regions that have as target the endpoint > + * decoder associated with this memdev > */ > struct cxl_memdev { > struct device dev; > @@ -50,6 +53,7 @@ struct cxl_memdev { > struct cxl_nvdimm *cxl_nvd; > int id; > int depth; > + struct list_head region_list; > }; > > static inline struct cxl_memdev *to_cxl_memdev(struct device *dev) > -- > 2.39.2 >
Hi Alison, On Sun, 26 Feb 2023, Alison Schofield wrote: >I took a look here as I've been poked around in this space when looking >for 'region' names to associate with memdev poison trace events. [1] Thanks for the pointer. >How does the list created get used? It's used by the security commands, in the next patches in the series. This came about because Dan requested some way of knowing wether the memdev is being used (actively decoding HPA ranges). >If we only need to know that this memdev is mapped in any region, >so don't touch it, we can look at it's port->commit_end. If that >commit_end >= 0, we can look at each endpoint to find the regions >it's mapped to. I think this is a lot better than using the list approach. Thanks, Davidlohr
On Tue, Feb 28, 2023 at 12:26:23PM -0800, Davidlohr Bueso wrote: > Hi Alison, > > On Sun, 26 Feb 2023, Alison Schofield wrote: > > > I took a look here as I've been poked around in this space when looking > > for 'region' names to associate with memdev poison trace events. [1] > > Thanks for the pointer. > > > How does the list created get used? > > It's used by the security commands, in the next patches in the series. > This came about because Dan requested some way of knowing wether the > memdev is being used (actively decoding HPA ranges). > > > If we only need to know that this memdev is mapped in any region, > > so don't touch it, we can look at it's port->commit_end. If that > > commit_end >= 0, we can look at each endpoint to find the regions > > it's mapped to. commit_end will not be updated during commit if the decoder flag is with CXL_DECODER_F_ENABLE, not sure whether it will cause issue or not for your case. Can we use hdm_end instead? Fan > > I think this is a lot better than using the list approach. > > Thanks, > Davidlohr
Davidlohr Bueso wrote: [..] > >If we only need to know that this memdev is mapped in any region, > >so don't touch it, we can look at it's port->commit_end. If that > >commit_end >= 0, we can look at each endpoint to find the regions > >it's mapped to. > > I think this is a lot better than using the list approach. Seconded, thanks Alison!
diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c index 47cc625bb1b0..68c0ab06b999 100644 --- a/drivers/cxl/core/memdev.c +++ b/drivers/cxl/core/memdev.c @@ -306,6 +306,7 @@ static struct cxl_memdev *cxl_memdev_alloc(struct cxl_dev_state *cxlds, dev->type = &cxl_memdev_type; device_set_pm_not_required(dev); INIT_WORK(&cxlmd->detach_work, detach_memdev); + INIT_LIST_HEAD(&cxlmd->region_list); cdev = &cxlmd->cdev; cdev_init(cdev, fops); diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c index f29028148806..cea9de6457b9 100644 --- a/drivers/cxl/core/region.c +++ b/drivers/cxl/core/region.c @@ -1730,7 +1730,10 @@ void cxl_decoder_kill_region(struct cxl_endpoint_decoder *cxled) { down_write(&cxl_region_rwsem); cxled->mode = CXL_DECODER_DEAD; - cxl_region_detach(cxled); + if (!cxl_region_detach(cxled)) { + struct cxl_region *cxlr = cxled->cxld.region; + list_del(&cxlr->node); + } up_write(&cxl_region_rwsem); } @@ -1749,8 +1752,12 @@ static int attach_target(struct cxl_region *cxlr, down_read(&cxl_dpa_rwsem); rc = cxl_region_attach(cxlr, cxled, pos); - if (rc == 0) + if (rc == 0) { + struct cxl_memdev *cxlmd = cxled_to_memdev(cxled); + set_bit(CXL_REGION_F_INCOHERENT, &cxlr->flags); + list_add_tail(&cxlr->node, &cxlmd->region_list); + } up_read(&cxl_dpa_rwsem); up_write(&cxl_region_rwsem); return rc; @@ -1778,6 +1785,8 @@ static int detach_target(struct cxl_region *cxlr, int pos) } rc = cxl_region_detach(p->targets[pos]); + if (rc == 0) + list_del(&cxlr->node); out: up_write(&cxl_region_rwsem); return rc; @@ -2654,6 +2663,26 @@ int cxl_add_to_region(struct cxl_port *root, struct cxl_endpoint_decoder *cxled) } EXPORT_SYMBOL_NS_GPL(cxl_add_to_region, CXL); +bool cxl_memdev_active_region(struct cxl_memdev *cxlmd) +{ + bool ret = false; + struct cxl_region *cxlr; + + down_read(&cxl_region_rwsem); + list_for_each_entry(cxlr, &cxlmd->region_list, node) { + struct cxl_region_params *p = &cxlr->params; + + if (p->state >= CXL_CONFIG_ACTIVE) { + ret = true; + break; + } + } + up_read(&cxl_region_rwsem); + + return ret; +} +EXPORT_SYMBOL_NS_GPL(cxl_memdev_active_region, CXL); + static int cxl_region_invalidate_memregion(struct cxl_region *cxlr) { if (!test_bit(CXL_REGION_F_INCOHERENT, &cxlr->flags)) diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h index b834e55375e3..e211241b079b 100644 --- a/drivers/cxl/cxl.h +++ b/drivers/cxl/cxl.h @@ -502,6 +502,7 @@ struct cxl_region { struct cxl_pmem_region *cxlr_pmem; unsigned long flags; struct cxl_region_params params; + struct list_head node; }; struct cxl_nvdimm_bridge { @@ -773,6 +774,7 @@ struct cxl_pmem_region *to_cxl_pmem_region(struct device *dev); int cxl_add_to_region(struct cxl_port *root, struct cxl_endpoint_decoder *cxled); struct cxl_dax_region *to_cxl_dax_region(struct device *dev); +bool cxl_memdev_active_region(struct cxl_memdev *cxlmd); #else static inline bool is_cxl_pmem_region(struct device *dev) { @@ -791,6 +793,10 @@ static inline struct cxl_dax_region *to_cxl_dax_region(struct device *dev) { return NULL; } +static inline bool cxl_memdev_active_region(struct cxl_memdev *cxlmd) +{ + return false; +} #endif /* diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h index 934076254d52..4e31f3234519 100644 --- a/drivers/cxl/cxlmem.h +++ b/drivers/cxl/cxlmem.h @@ -5,6 +5,7 @@ #include <uapi/linux/cxl_mem.h> #include <linux/cdev.h> #include <linux/uuid.h> +#include <linux/list.h> #include "cxl.h" /* CXL 2.0 8.2.8.5.1.1 Memory Device Status Register */ @@ -40,6 +41,8 @@ * @cxl_nvd: optional bridge to an nvdimm if the device supports pmem * @id: id number of this memdev instance. * @depth: endpoint port depth + * @region_list: List of regions that have as target the endpoint + * decoder associated with this memdev */ struct cxl_memdev { struct device dev; @@ -50,6 +53,7 @@ struct cxl_memdev { struct cxl_nvdimm *cxl_nvd; int id; int depth; + struct list_head region_list; }; static inline struct cxl_memdev *to_cxl_memdev(struct device *dev)
Track all regions associated to a memdev in order to tell if the device might be in active use. Signed-off-by: Davidlohr Bueso <dave@stgolabs.net> --- drivers/cxl/core/memdev.c | 1 + drivers/cxl/core/region.c | 33 +++++++++++++++++++++++++++++++-- drivers/cxl/cxl.h | 6 ++++++ drivers/cxl/cxlmem.h | 4 ++++ 4 files changed, 42 insertions(+), 2 deletions(-)