Re: [PATCH v4 09/10] hw/cxl/events: Add qmp interfaces to add/release dynamic capacity extents

[Wonjae Lee]

On 2024-02-22 오전 3:16, nifan.cxl@gmail.com wrote:
> From: Fan Ni <fan.ni@samsung.com>
>
> Since fabric manager emulation is not supported yet, the change implements
> the functions to add/release dynamic capacity extents as QMP interfaces.
>
> 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.

[snip]

> +/* CXL r3.1 Table 8-50: Dynanic Capacity Event Record */

It needs a spell check. Dynamic.

...

> +    switch (type) {
> +    case DC_EVENT_ADD_CAPACITY:
> +        break;
> +    default:
> +        break;
> +    }

Perhaps this switch statement has become unnecessary in the revision to 
patch v4?

...

> diff --git a/qapi/cxl.json b/qapi/cxl.json
> index 8cc4c72fa9..19927629c7 100644
> --- a/qapi/cxl.json
> +++ b/qapi/cxl.json

[snip]

> +##
> +# @cxl-add-dynamic-capacity:
> +#
> +# Command to start add dynamic capacity extents flow. The device will
> +# have to ackowledged the acceptance of the extents before they are usable.
> +#
> +# @path: CXL DCD canonical QOM path
> +# @region-id: id of the region where the extent to add/release

How about just using "add" instead of "add/release"?

...

> +##
> +# @cxl-release-dynamic-capacity:
> +#
> +# Command to start release dynamic capacity extents flow. The host will
> +# 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.
> +#
> +# @path: CXL DCD canonical QOM path
> +# @region-id: id of the region where the extent to add/release

And here. Only "release"?


Thanks,
Wonjae