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

[Jonathan Cameron]

...

> > > I cannot find anything specific to this in the specification either.
> > > Since we have already detected the case where the extent range across
> > > multiple regions, the only case we need to capture here is one/multiple
> > > portions of an extents getting released and causing extent overflow.
> > > I think we can handle it after we introduce the bitmaps (PATCH 10) which
> > > indicates DPA ranges mapped by valid extents in the device.
> > > 
> > > With that, The release workflow would be
> > > 
> > > 1) detecting malformed extent lists; if passed
> > > 2) do cxl_detect_extent_overflow {
> > >     delta = 0;
> > >     make a copy of the bitmap as bitmap_copy;
> > >     for each extent in the updated_extent_list; do
> > >         if (extent range not fully set in the bitmap_copy)
> > >             return error;
> > >         else {
> > >             if gap at the front based on the bitmap_copy:
> > >                 delta += 1;
> > >             if gap at the end based on the bitmap_copy:
> > >                 delta += 1;
> > >             delta -= 1;
> > >             // NOTE: current_extent_count will not be updated in the
> > >             // loop since delta will track the whole loop
> > >             if (delta + current_extent_count > max_extent_count)
> > >                 return resource exhausted;
> > >             update bitmap_copy to clear the range covered by the extent
> > >             under consideration;
> > >         }
> > >     done
> > > 
> > > }; if pass
> > > 3. do real release: in the pass, we will not need to detect extent
> > > errors;
> > > 
> > > Does the above solution sound reasonable? If so, do we want to go this
> > > way? do we need to introduce the bitmap earlier in the series?  
> > 
> > Yes, something along these lines should work nicely.
> > 
> > Jonathan  
> 
> Hi Jonathan,
> I updated the code based on your feedback and now we can process extent
> release request more flexible.

Excellent!

> We can now support superset release (actually it can do even more,
> as long as the DPA range is coverd by accepted extents, we can release).
> 
> I have run following tests and the code works as expected,
> 1. Add multiple extents, and removing them one by one, passed;
> 2. Superset release: add multiple extents with continuous DPA ranges, and
>    remove all of them with a single release request with an extent covering 
> the
>    whole DPA range, passed;
> 3. Partial extent release: add a large extent and release only part of it,
>    passed;
> 4. Partial+superset release: add multiple extents,and release it with some
>    leftover with one request with an extent. For example, add extents [0-128M]
>    and [128M-256M], release [64M-256M]. Passed;
> 5. Release extent not aligned to block size, failed as expected;
> 6. Extents have overlaps, fail the request as expected;
> 7. Extent has uncovered DPA range, skip the extent as expected;
> 
> The only limitation is that for superset release case, if we find
> part of its DPA range is still pending to add, while the other is
> accepted, we reject it through QMP interface.

I think that is a reasonable limitation as we don't expect people
to do that crazy on QMP side.   Maybe long term we'll want a
'release all' type command (I'm thinking virtualized device usecases)
but we can deal with that later.

> 
> The latest code is https://github.com/moking/qemu/tree/dcd-v5.
> 
> The main changes are in the last three commits. 
> Btw, in the last commit, I introduce new QMP interfaces to print out
> accepted and pending-to-add list in the device to a file "/tmp/qmp.txt",
> do we want it? If yes, I can polish it a little bit, otherwise I will
> keep it for my own test purpose.
Ah. I missed this mail and replied directly.  That needs a rethink
as the thread has concluded I think. I'll carry it on my tree, but not
look to upstream it.
> 
> I will test more and send out v5 if the above looks reasonable to you.
> 
Sorry for slow reply - I'm a bit behind with mailing lists.
Great you sent it out in the meantime.

Jonathan