Markus Armbruster wrote: > nifan....@gmail.com writes: > > > From: Fan Ni <fan...@samsung.com> > > > > Since fabric manager emulation is not supported yet, the change implements > > the functions to add/release dynamic capacity extents as QMP interfaces. > > Will fabric manager emulation obsolete these commands?
I don't think so. In the development of the kernel, I see these being valuable to do CI and regression testing without the complexity of an FM. Ira > > > Note: we skips any FM issued extent release request if the exact extent > > does not exist in the extent list of the device. We will loose the > > restriction later once we have partial release support in the kernel. > > > > 1. Add dynamic capacity extents: > > > > For example, the command to add two continuous extents (each 128MiB long) > > to region 0 (starting at DPA offset 0) looks like below: > > > > { "execute": "qmp_capabilities" } > > > > { "execute": "cxl-add-dynamic-capacity", > > "arguments": { > > "path": "/machine/peripheral/cxl-dcd0", > > "region-id": 0, > > "extents": [ > > { > > "dpa": 0, > > "len": 134217728 > > }, > > { > > "dpa": 134217728, > > "len": 134217728 > > } > > ] > > } > > } > > > > 2. Release dynamic capacity extents: > > > > For example, the command to release an extent of size 128MiB from region 0 > > (DPA offset 128MiB) look like below: > > > > { "execute": "cxl-release-dynamic-capacity", > > "arguments": { > > "path": "/machine/peripheral/cxl-dcd0", > > "region-id": 0, > > "extents": [ > > { > > "dpa": 134217728, > > "len": 134217728 > > } > > ] > > } > > } > > > > Signed-off-by: Fan Ni <fan...@samsung.com> > > [...] > > > diff --git a/qapi/cxl.json b/qapi/cxl.json > > index 8cc4c72fa9..2645004666 100644 > > --- a/qapi/cxl.json > > +++ b/qapi/cxl.json > > @@ -19,13 +19,16 @@ > > # > > # @fatal: Fatal Event Log > > # > > +# @dyncap: Dynamic Capacity Event Log > > +# > > # Since: 8.1 > > ## > > { 'enum': 'CxlEventLog', > > 'data': ['informational', > > 'warning', > > 'failure', > > - 'fatal'] > > + 'fatal', > > + 'dyncap'] > > We tend to avoid abbreviations in QMP identifiers: dynamic-capacity. > > > } > > > > ## > > @@ -361,3 +364,59 @@ > > ## > > {'command': 'cxl-inject-correctable-error', > > 'data': {'path': 'str', 'type': 'CxlCorErrorType'}} > > + > > +## > > +# @CXLDCExtentRecord: > > Such traffic jams of capital letters are hard to read. > > What does DC mean? > > > +# > > +# Record of a single extent to add/release > > +# > > +# @offset: offset to the start of the region where the extent to be > > operated > > Blank line here, please > > > +# @len: length of the extent > > +# > > +# Since: 9.0 > > +## > > +{ 'struct': 'CXLDCExtentRecord', > > + 'data': { > > + 'offset':'uint64', > > + 'len': 'uint64' > > + } > > +} > > + > > +## > > +# @cxl-add-dynamic-capacity: > > +# > > +# Command to start add dynamic capacity extents flow. The device will > > I think we're missing an article here. Is it "a flow" or "the flow"? > > > +# have to acknowledged the acceptance of the extents before they are > > usable. > > to acknowledge > > docs/devel/qapi-code-gen.rst: > > For legibility, wrap text paragraphs so every line is at most 70 > characters long. > > Separate sentences with two spaces. > > > +# > > +# @path: CXL DCD canonical QOM path > > What is a CXL DCD? Is it a device? > > I'd prefer @qom-path, unless you can make a consistency argument for > @path. > > > +# @region-id: id of the region where the extent to add > > What's a region, and how do they get their IDs? > > > +# @extents: Extents to add > > Blank lines between argument descriptions, please. > > > +# > > +# Since : 9.0 > > 9.1 > > > +## > > +{ 'command': 'cxl-add-dynamic-capacity', > > + 'data': { 'path': 'str', > > + 'region-id': 'uint8', > > + 'extents': [ 'CXLDCExtentRecord' ] > > + } > > +} > > + > > +## > > +# @cxl-release-dynamic-capacity: > > +# > > +# Command to start release dynamic capacity extents flow. The host will > > Article again. > > The host? In cxl-add-dynamic-capacity's doc comment, it's the device. > > > +# need to respond to indicate that it has released the capacity before it > > +# is made unavailable for read and write and can be re-added. > > Is "and can be re-added" relevant here? > > > +# > > +# @path: CXL DCD canonical QOM path > > +# @region-id: id of the region where the extent to release > > +# @extents: Extents to release > > +# > > +# Since : 9.0 > > 9.1 > > > +## > > +{ 'command': 'cxl-release-dynamic-capacity', > > + 'data': { 'path': 'str', > > + 'region-id': 'uint8', > > + 'extents': [ 'CXLDCExtentRecord' ] > > + } > > +} >