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?
> 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' ]
> + }
> +}
