On Mon, 4 Mar 2024 11:34:08 -0800
nifan....@gmail.com wrote:
> From: Fan Ni <fan...@samsung.com>
>
> With the change, we add the following two QMP interfaces to print out
> extents information in the device,
> 1. cxl-display-accepted-dc-extents: print out the accepted DC extents in
> the device;
> 2. cxl-display-pending-to-add-dc-extents: print out the pending-to-add
> DC extents in the device;
> The output is appended to a file passed to the command and by default
> it is /tmp/dc-extent.txt.
Hi Fan,
Is there precedence for this sort of logging to a file from a qmp
command? I can see something like this being useful.
A few comments inline.
Jonathan
>
> Signed-off-by: Fan Ni <fan...@samsung.com>
> ---
> hw/mem/cxl_type3.c | 80 ++++++++++++++++++++++++++++++++++++++++
> hw/mem/cxl_type3_stubs.c | 12 ++++++
> qapi/cxl.json | 32 ++++++++++++++++
> 3 files changed, 124 insertions(+)
>
> diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
> index 5bd64e604e..6a08e7ae40 100644
> --- a/hw/mem/cxl_type3.c
> +++ b/hw/mem/cxl_type3.c
> @@ -2089,6 +2089,86 @@ void qmp_cxl_release_dynamic_capacity(const char
> *path, uint8_t region_id,
> region_id, records, errp);
> }
>
> +static void cxl_dcd_display_extent_list(const CXLType3Dev *dcd, const char
> *f,
> + bool accepted_list, Error **errp)
> +{
> + const CXLDCExtentList *list;
> + CXLDCExtent *ent;
> + FILE *fp = NULL;
> + int i = 0;
> +
> + if (!dcd->dc.num_regions) {
> + error_setg(errp, "No dynamic capacity support from the device");
> + return;
> + }
> +
> + if (!f) {
> + fp = fopen("/tmp/dc-extent.txt", "a+");
> + } else {
> + fp = fopen(f, "a+");
> + }
> +
> + if (!fp) {
> + error_setg(errp, "Open log file failed");
> + return;
> + }
> + if (accepted_list) {
> + list = &dcd->dc.extents;
> + fprintf(fp, "Print accepted extent info:\n");
> + } else {
> + list = &dcd->dc.extents_pending_to_add;
> + fprintf(fp, "Print pending-to-add extent info:\n");
> + }
> +
> + QTAILQ_FOREACH(ent, list, node) {
> + fprintf(fp, "%d: [0x%lx - 0x%lx]\n", i++, ent->start_dpa,
> + ent->start_dpa + ent->len);
> + }
> + fprintf(fp, "In total, %d extents printed!\n", i);
> + fclose(fp);
> +}
> +void qmp_cxl_display_pending_to_add_dc_extents(const char *path, const char
> *f,
> + Error **errp)
> +{
> + Object *obj;
> + CXLType3Dev *dcd;
> +
> + obj = object_resolve_path(path, NULL);
As an aside, we could probably flatten a lot of these cases into
object_resolve_path_type()
> + if (!obj) {
> + error_setg(errp, "Unable to resolve path");
> + return;
> + }
> + if (!object_dynamic_cast(obj, TYPE_CXL_TYPE3)) {
> + error_setg(errp, "Path not point to a valid CXL type3 device");
> + return;
> + }
> +
> +
> + dcd = CXL_TYPE3(obj);
> + cxl_dcd_display_extent_list(dcd, f, false, errp);
> +}
> +
> static void ct3_class_init(ObjectClass *oc, void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(oc);
> diff --git a/qapi/cxl.json b/qapi/cxl.json
> index 2645004666..6f10300ec6 100644
> --- a/qapi/cxl.json
> +++ b/qapi/cxl.json
> @@ -420,3 +420,35 @@
> 'extents': [ 'CXLDCExtentRecord' ]
> }
> }
> +
> +##
> +# @cxl-display-accepted-dc-extents:
> +#
> +# Command to print out all the accepted DC extents in the device
> +#
> +# @path: CXL DCD canonical QOM path
> +# @output: path of output file to dump the results to
We take a path, but dump to the same file whatever this is set to?
I'm not sure what precedence there is for qom commands that
dump to a debug log. Perhaps reference any other cases in the
patch description.
> +#
> +# Since : 9.0
> +##
> +{ 'command': 'cxl-display-accepted-dc-extents',
> + 'data': { 'path': 'str',
> + 'output': 'str'
> + }
> +}
> +
> +##
> +# @cxl-display-pending-to-add-dc-extents:
> +#
> +# Command to print out all the pending-to-add DC extents in the device
> +#
> +# @path: CXL DCD canonical QOM path
> +# @output: path of output file to dump the results to
> +#
> +# Since : 9.0
> +##
> +{ 'command': 'cxl-display-pending-to-add-dc-extents',
> + 'data': { 'path': 'str',
> + 'output': 'str'
> + }
> +}
