| Message ID | 20220805064918.925115-1-vishal.l.verma@intel.com |
|---|---|
| State | New, archived |
| Headers | show |
| Series | cxl/region: reset the cxl endpoint decoder HPA range on teardown | expand |
Vishal Verma wrote: > The endpoint decoder HPA range gets set up during cxl_region_attach, but > cxl_region_detach() neglects to reset it back to a 'free' state. As a > result, a create-destroy-create cycle with the same region parameters > results in the second create-region failing with an EBUSY when trying to > reserver DPA. > > # cxl create-region -d decoder3.2 -m mem4 -g 1024 > cxl region: create_region: decoder11.1: set_dpa_size failed: Device or resource busy > cxl region: cmd_create_region: created 0 regions > > Set the endpoint decoder's hpa_range to [0, -1] to mark it as free in > cxl_region_detach(). > > Fixes: 292bdc6af8f2 ("cxl/region: Move HPA setup to cxl_region_attach()") > Cc: Dan Williams <dan.j.williams@intel.com> > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> > --- > drivers/cxl/core/region.c | 4 ++++ > 1 file changed, 4 insertions(+) > > Hi Dan, > > This is a fixup-to-a-fixup, and the commit in Fixes: is only in > 'preview'. Feel free to squash this into the original fix if you so > prefer. Will do, I think this also needs to kill the cxld_clear_hpa() helper and call in cxl_decoder_reset(). > Another question - should we be resetting the endpoint decoder's mode to > CXL_DECODER_DEAD while in here? I notice that a freed decoder still > lists mode:pmem in cxl-list. The DEAD state is a internal-only hack while the decoder is being torn down, it's not visible to userspace. Perhaps just hide the none state and default the mode to 'ram' when no DPA is allocated?
On Fri, 2022-08-05 at 01:07 -0700, Dan Williams wrote: > Vishal Verma wrote: > > The endpoint decoder HPA range gets set up during cxl_region_attach, but > > cxl_region_detach() neglects to reset it back to a 'free' state. As a > > result, a create-destroy-create cycle with the same region parameters > > results in the second create-region failing with an EBUSY when trying to > > reserver DPA. > > > > # cxl create-region -d decoder3.2 -m mem4 -g 1024 > > cxl region: create_region: decoder11.1: set_dpa_size failed: Device or resource busy > > cxl region: cmd_create_region: created 0 regions > > > > Set the endpoint decoder's hpa_range to [0, -1] to mark it as free in > > cxl_region_detach(). > > > > Fixes: 292bdc6af8f2 ("cxl/region: Move HPA setup to cxl_region_attach()") > > Cc: Dan Williams <dan.j.williams@intel.com> > > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> > > --- > > drivers/cxl/core/region.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > Hi Dan, > > > > This is a fixup-to-a-fixup, and the commit in Fixes: is only in > > 'preview'. Feel free to squash this into the original fix if you so > > prefer. > > Will do, I think this also needs to kill the cxld_clear_hpa() helper and > call in cxl_decoder_reset(). > > > Another question - should we be resetting the endpoint decoder's mode to > > CXL_DECODER_DEAD while in here? I notice that a freed decoder still > > lists mode:pmem in cxl-list. > > The DEAD state is a internal-only hack while the decoder is being torn > down, it's not visible to userspace. Perhaps just hide the none state > and default the mode to 'ram' when no DPA is allocated? Yeah that seems reasonable.
diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c index a68e4e0cf169..f0c8966a69c1 100644 --- a/drivers/cxl/core/region.c +++ b/drivers/cxl/core/region.c @@ -1369,6 +1369,10 @@ static int cxl_region_detach(struct cxl_endpoint_decoder *cxled) } p->targets[cxled->pos] = NULL; p->nr_targets--; + cxled->cxld.hpa_range = (struct range) { + .start = 0, + .end = -1, + }; /* notify the region driver that one of its targets has departed */ up_write(&cxl_region_rwsem);
The endpoint decoder HPA range gets set up during cxl_region_attach, but cxl_region_detach() neglects to reset it back to a 'free' state. As a result, a create-destroy-create cycle with the same region parameters results in the second create-region failing with an EBUSY when trying to reserver DPA. # cxl create-region -d decoder3.2 -m mem4 -g 1024 cxl region: create_region: decoder11.1: set_dpa_size failed: Device or resource busy cxl region: cmd_create_region: created 0 regions Set the endpoint decoder's hpa_range to [0, -1] to mark it as free in cxl_region_detach(). Fixes: 292bdc6af8f2 ("cxl/region: Move HPA setup to cxl_region_attach()") Cc: Dan Williams <dan.j.williams@intel.com> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> --- drivers/cxl/core/region.c | 4 ++++ 1 file changed, 4 insertions(+) Hi Dan, This is a fixup-to-a-fixup, and the commit in Fixes: is only in 'preview'. Feel free to squash this into the original fix if you so prefer. Another question - should we be resetting the endpoint decoder's mode to CXL_DECODER_DEAD while in here? I notice that a freed decoder still lists mode:pmem in cxl-list. --- base-commit: 65fc1c3d26b96002a5aa1f4012fae4dc98fd5683 prerequisite-patch-id: 151ffe5b355fcc040b69b651037b239b28ce459d prerequisite-patch-id: 5c3f0d23262513eb991f7db058736e20b601a897 prerequisite-patch-id: 63825d8f66bf5eda9fe23ab3d995de15a516754c prerequisite-patch-id: d5ea0349921af5ce2670b4a0f9cc09662e8d6163 prerequisite-patch-id: 6f974183c2c44c4d5b32f2496af7935ec59bc402