From: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
To: fan <nifan.cxl@gmail.com>
Cc: <qemu-devel@nongnu.org>, <linux-cxl@vger.kernel.org>,
<gregory.price@memverge.com>, <ira.weiny@intel.com>,
<dan.j.williams@intel.com>, <a.manzanares@samsung.com>,
<dave@stgolabs.net>, <nmtadam.samsung@gmail.com>,
<jim.harris@samsung.com>, Fan Ni <fan.ni@samsung.com>
Subject: Re: [PATCH v4 08/10] hw/cxl/cxl-mailbox-utils: Add mailbox commands to support add/release dynamic capacity response
Date: Wed, 6 Mar 2024 14:58:23 +0000 [thread overview]
Message-ID: <20240306145823.000028ee@Huawei.com> (raw)
In-Reply-To: <ZeFR_Z9nInhyf-W_@debian>
...
> > > 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
next prev parent reply other threads:[~2024-03-06 14:58 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-21 18:15 [PATCH v4 00/10] Enabling DCD emulation support in Qemu nifan.cxl
2024-02-21 18:15 ` [PATCH v4 01/10] hw/cxl/cxl-mailbox-utils: Add dc_event_log_size field to output payload of identify memory device command nifan.cxl
2024-02-21 18:15 ` [PATCH v4 02/10] hw/cxl/cxl-mailbox-utils: Add dynamic capacity region representative and mailbox command support nifan.cxl
2024-02-26 17:33 ` Jonathan Cameron
2024-02-26 19:16 ` fan
2024-03-04 12:40 ` Jørgen Hansen
2024-03-04 17:35 ` fan
2024-02-21 18:15 ` [PATCH v4 03/10] include/hw/cxl/cxl_device: Rename mem_size as static_mem_size for type3 memory devices nifan.cxl
2024-02-21 18:15 ` [PATCH v4 04/10] hw/mem/cxl_type3: Add support to create DC regions to " nifan.cxl
2024-02-26 17:38 ` Jonathan Cameron
2024-03-04 13:10 ` Jørgen Hansen
2024-03-04 17:31 ` fan
2024-02-21 18:15 ` [PATCH v4 05/10] hw/mem/cxl-type3: Refactor ct3_build_cdat_entries_for_mr to take mr size insead of mr as argument nifan.cxl
2024-02-21 18:15 ` [PATCH v4 06/10] hw/mem/cxl_type3: Add host backend and address space handling for DC regions nifan.cxl
2024-02-26 17:45 ` Jonathan Cameron
2024-02-21 18:16 ` [PATCH v4 07/10] hw/mem/cxl_type3: Add DC extent list representative and get DC extent list mailbox support nifan.cxl
2024-02-23 7:16 ` Wonjae Lee
2024-02-23 16:56 ` fan
2024-02-26 17:48 ` Jonathan Cameron
2024-02-21 18:16 ` [PATCH v4 08/10] hw/cxl/cxl-mailbox-utils: Add mailbox commands to support add/release dynamic capacity response nifan.cxl
2024-02-23 9:10 ` Wonjae Lee
2024-02-26 18:04 ` Jonathan Cameron
2024-02-27 1:01 ` fan
2024-02-27 10:39 ` Jonathan Cameron
2024-03-01 3:56 ` fan
2024-03-06 14:58 ` Jonathan Cameron [this message]
2024-02-27 1:06 ` fan
2024-02-21 18:16 ` [PATCH v4 09/10] hw/cxl/events: Add qmp interfaces to add/release dynamic capacity extents nifan.cxl
2024-02-23 12:16 ` Wonjae Lee
2024-02-26 18:10 ` Jonathan Cameron
2024-02-21 18:16 ` [PATCH v4 10/10] hw/mem/cxl_type3: Add dpa range validation for accesses to DC regions nifan.cxl
[not found] ` <CGME20240221182126epcas2p1b684f9239e4262f17ff484939658a382@epcms2p1>
2024-02-22 7:45 ` [PATCH v4 02/10] hw/cxl/cxl-mailbox-utils: Add dynamic capacity region representative and mailbox command support Wonjae Lee
2024-02-22 16:54 ` fan
[not found] ` <CGME20240221182137epcas2p276d22514caaa9412d0119ded6f9a63d4@epcms2p3>
2024-02-22 9:22 ` [PATCH v4 06/10] hw/mem/cxl_type3: Add host backend and address space handling for DC regions Wonjae Lee
2024-02-22 16:56 ` fan
2024-02-23 18:05 ` [PATCH v4 00/10] Enabling DCD emulation support in Qemu fan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240306145823.000028ee@Huawei.com \
--to=jonathan.cameron@huawei.com \
--cc=a.manzanares@samsung.com \
--cc=dan.j.williams@intel.com \
--cc=dave@stgolabs.net \
--cc=fan.ni@samsung.com \
--cc=gregory.price@memverge.com \
--cc=ira.weiny@intel.com \
--cc=jim.harris@samsung.com \
--cc=linux-cxl@vger.kernel.org \
--cc=nifan.cxl@gmail.com \
--cc=nmtadam.samsung@gmail.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).