Re: [PATCH 2/2] hw/cxl/cxl-mailbox-utils: Add support for Media operations Sanitize and Write Zeros commands (8.2.9.9.5.3)

[Jonathan Cameron via <qemu-devel@nongnu.org>]

> >   
> > > +    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.

> > > +    }
> > > +
> > > +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. 

> 
> 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!

> 

Jonathan