On 03/02/25 05:02PM, Adam Manzanares wrote:
On Mon, Feb 03, 2025 at 11:33:54AM +0000, Jonathan Cameron wrote:
> >
> > > + int dpa_range_count = san_info->dpa_range_count;
> > > + int rc = 0;
> > > +
> > > + for (int i = 0; i < dpa_range_count; i++) {
> > > + rc = sanitize_range(ct3d,
san_info->dpa_range_list[i].starting_dpa,
> > > + san_info->dpa_range_list[i].length,
san_info->fill_value);
> > > + if (rc) {
> > > + goto exit;
> > > + }
> > > + }
> > > + cxl_discard_all_event_records(&ct3d->cxl_dstate);
> >
> > Add a comment on why we are deleting event records when sanitizing a small
> > part of memory?
> >
>
> See response below for disabling the media state. Same section referenced
> below, 8.2.10.9.5.1 states all event logs should be deleted. Outcome
> depends on how we interpret "follow the method described in 8.2.10.9.5.1".
>
This also sounds like reading too much into that comment.
Agreed, Vinayak let's drop the discard of all event records
from this patch.
Ok Adam will drop the discard of all event records
> > > + }
> > > +
> > > +start_bg:
> > > + /* EBUSY other bg cmds as of now */
> > > + cci->bg.runtime = secs * 1000UL;
> > > + *len_out = 0;
> > > + /* sanitize when done */
> > > + cxl_dev_disable_media(&ct3d->cxl_dstate);
> > Why? This is santizing part of the device. As I undestand it the
> > main aim is to offload cleanup when the device is in use. Definitely
> > don't want to disable media. If I'm missing a reason please give
> > a spec reference.
>
> Table 8-164, sanitize description mentions to follow method
> in 8.2.10.9.5.1, which does call out placing device in disabled
> state, but I'm not sure if method refers to all text in 8.2.10.9.5.1
> or the method devices uses to sanitize internally.
I think it is meant to just be the method of sanitizing.
Ok, let's use this interpretation. Vinayak, can you remove this as well
and then we put a comment in the patch that media op sanitize is targeted
so no need to disable media or clear event logs.
ok Adam, will remove this as well and comment as needed
>
> I would imagine since sanitize is destructive we would not want to return
> any data from device ranges impacted by sanitize. I believe a simple
> way to achieve this is to disable entire device.
Hmm. That rather destroys the main use case I'm aware of for this
(unlike the general santize commands from earlier CXL versions)/
Superficially sounds like we need a spec clarification as
clearly not super clear!
For this series, let's drive the work with the use case you have in
mind. We will start a thread with the consortium, but I don't think
that should delay this work.
>
Jonathan
Vinayak Holikatti
