Re: [PATCH v5 08/13] hw/cxl/cxl-mailbox-utils: Add mailbox commands to support add/release dynamic capacity response

[Jonathan Cameron]

On Wed, 6 Mar 2024 13:39:50 -0800
fan <nifan.cxl@gmail.com> wrote:



> > > +                    }
> > > +                    if (len2) {
> > > +                        cxl_insert_extent_to_extent_list(extent_list, 
> > > dpa + len,
> > > +                                                         len2, NULL, 0);
> > > +                        ct3d->dc.total_extent_count += 1;
> > > +                    }
> > > +                    break;  
> > Maybe this makes sense after the support below is added, but at this
> > point in the series 
> >                     return CXL_MBOX_SUCCESS;
> > then found isn't relevant so can drop that. Looks like you drop it later in 
> > the
> > series anyway.  
> 
> We cannot return directly as we have more extents to release.

Ah good point. I'd missed the double loop.

> One thing I think I need to add is a dry run to test if any extent in
> the income list is not contained by an extent in the extent list and
> return error before starting to do the real release. The spec just says
> we need to return invalid PA but not specify whether we should update the list
> until we found a "bad" extent or reject the request directly. Current code
> leaves a situation where we may have updated the extent list until we found a
> "bad" extent to release.

Yes, I'm not sure on the correct answer to this either. My assumption is that in
error cases there are no side effects, but I don't see a clear statement of 
that.

So I think we are in the world of best practice, not spec compliance.
If we wanted to recover from such an error case we'd have to verify the current
extent list.  I'll fire off a question to relevant folk in appropriate forum.

Jonathan