qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Jonathan Cameron via <qemu-devel@nongnu.org>
To: <anisa.su887@gmail.com>
Cc: <qemu-devel@nongnu.org>, <nifan.cxl@gmail.com>,
	<dave@stgolabs.net>, <alok.a.tiwari@oracle.com>,
	<linux-cxl@vger.kernel.org>, Anisa Su <anisa.su@samsung.com>
Subject: Re: [QEMU PATCH v4 09/10] cxl-mailbox-utils: 0x5604 - FMAPI Initiate DC Add
Date: Mon, 30 Jun 2025 18:01:10 +0100	[thread overview]
Message-ID: <20250630180110.0000060d@huawei.com> (raw)
In-Reply-To: <20250626222743.1766404-10-anisa.su887@gmail.com>

On Thu, 26 Jun 2025 22:23:32 +0000
anisa.su887@gmail.com wrote:

> From: Anisa Su <anisa.su@samsung.com>
> 
> FM DCD Management command 0x5604 implemented per CXL r3.2 Spec Section 7.6.7.6.5

There are a few long lines in here check patch doesn't like.


I tweaked as described inline whilst picking them up.


> 
> Signed-off-by: Anisa Su <anisa.su@samsung.com>
> ---
>  hw/cxl/cxl-mailbox-utils.c  | 106 ++++++++++++++++++++++++++++++++++++
>  hw/mem/cxl_type3.c          |   8 +--
>  include/hw/cxl/cxl_device.h |   4 ++
>  3 files changed, 114 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
> index 382c41c025..787eeaa7b9 100644
> --- a/hw/cxl/cxl-mailbox-utils.c
> +++ b/hw/cxl/cxl-mailbox-utils.c


> +/* CXL r3.2 Section 7.6.7.6.5 Initiate Dynamic Capacity Add (Opcode 5604h) */
> +static CXLRetCode cmd_fm_initiate_dc_add(const struct cxl_cmd *cmd,
> +                                         uint8_t *payload_in,
> +                                         size_t len_in,
> +                                         uint8_t *payload_out,
> +                                         size_t *len_out,
> +                                         CXLCCI *cci)
> +{
> +    struct {
> +        uint16_t host_id;
> +        uint8_t selection_policy;
> +        uint8_t reg_num;
> +        uint64_t length;
> +        uint8_t tag[0x10];
> +        uint32_t ext_count;
> +        CXLDCExtentRaw extents[];
> +    } QEMU_PACKED *in = (void *)payload_in;
> +    CXLType3Dev *ct3d = CXL_TYPE3(cci->d);
> +    int i, rc;
> +
> +    switch (in->selection_policy) {
> +        case CXL_EXTENT_SELECTION_POLICY_PRESCRIPTIVE: {
> +            /* Adding extents exceeds device's extent tracking ability. */
> +            if (in->ext_count + ct3d->dc.total_extent_count >
> +                CXL_NUM_EXTENTS_SUPPORTED) {
> +                return CXL_MBOX_RESOURCES_EXHAUSTED;
> +            }
> +
> +            g_autofree CXLUpdateDCExtentListInPl *list =
> +                g_malloc0(sizeof(*list) +
> +                    in->ext_count * sizeof(*list->updated_entries));
> +
> +            convert_raw_extents(in->extents, list, in->ext_count);
> +            rc = cxl_detect_malformed_extent_list(ct3d, list);
> +
> +            for (i = 0; i < in->ext_count; i++) {
> +                CXLDCExtentRaw ext = in->extents[i];

Using a pointer here avoids copying data for no particular reason.

> +                /* Check requested extents do not overlap with pending extents. */
This is a touch long.   I tweaked to end with ... pending ones.
Given we are talking extents that should be clear.

> +                if (cxl_extent_groups_overlaps_dpa_range(&ct3d->dc.extents_pending,
This is the one case I'll leave as it is not nice to wrap it anywhere different.

> +                                                         ext.start_dpa, ext.len)) {
wrapped this

> +                    return CXL_MBOX_INVALID_EXTENT_LIST;
> +                }
> +                /* Check requested extents do not overlap with existing extents. */
exiting ones. */
> +                if (cxl_extents_overlaps_dpa_range(&ct3d->dc.extents,
> +                                                   ext.start_dpa, ext.len)) {
Wrapped this.

> +                    return CXL_MBOX_INVALID_EXTENT_LIST;
> +                }
> +            }
> +
> +            if (rc) {
> +                return rc;
> +            }
> +
> +            CXLDCExtentGroup *group = NULL;
> +            for (i = 0; i < in->ext_count; i++) {

Used
                   CXLDCExtentRaw *ext = &in->extents[i];

> +                group = cxl_insert_extent_to_extent_group(group,
> +                                                          in->extents[i].start_dpa,

To shorten all these (similar to previous loop)

> +                                                          in->extents[i].len,
> +                                                          in->extents[i].tag,
> +                                                          in->extents[i].shared_seq);
> +            }
> +
> +            cxl_extent_group_list_insert_tail(&ct3d->dc.extents_pending, group);
> +            ct3d->dc.total_extent_count += in->ext_count;
> +            cxl_create_dc_event_records_for_extents(ct3d,
> +                                                    DC_EVENT_ADD_CAPACITY,
> +                                                    in->extents,
> +                                                    in->ext_count);
> +
> +            return CXL_MBOX_SUCCESS;
> +        }
> +        default: {
> +            qemu_log_mask(LOG_UNIMP,
> +                          "CXL extent selection policy not supported.\n");
> +            return CXL_MBOX_INVALID_INPUT;
> +        }
> +    }
> +}




  reply	other threads:[~2025-06-30 17:02 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-26 22:23 [QEMU PATCH v4 00/10] CXL: FMAPI DCD Management Commands 0x5600-0x5605 anisa.su887
2025-06-26 22:23 ` [QEMU PATCH v4 01/10] cxl-mailbox-utils: 0x5600 - FMAPI Get DCD Info anisa.su887
2025-06-26 22:23 ` [QEMU PATCH v4 02/10] cxl/type3: Add dsmas_flags to CXLDCRegion struct anisa.su887
2025-06-26 22:23 ` [QEMU PATCH v4 03/10] cxl-mailbox-utils: 0x5601 - FMAPI Get Host Region Config anisa.su887
2025-06-30 16:36   ` Jonathan Cameron via
2025-06-26 22:23 ` [QEMU PATCH v4 04/10] cxl_events.h: Move definition for dynamic_capacity_uuid and enum for DC event types anisa.su887
2025-06-26 22:23 ` [QEMU PATCH v4 05/10] hw/cxl_type3: Add DC Region bitmap lock anisa.su887
2025-06-26 22:23 ` [QEMU PATCH v4 06/10] cxl-mailbox-utils: 0x5602 - FMAPI Set DC Region Config anisa.su887
2025-06-26 22:23 ` [QEMU PATCH v4 07/10] cxl-mailbox-utils: 0x5603 - FMAPI Get DC Region Extent Lists anisa.su887
2025-06-26 22:23 ` [QEMU PATCH v4 08/10] hw/cxl: create helper function to create DC Event Records from extents anisa.su887
2025-07-03  3:58   ` Fan Ni
2025-06-26 22:23 ` [QEMU PATCH v4 09/10] cxl-mailbox-utils: 0x5604 - FMAPI Initiate DC Add anisa.su887
2025-06-30 17:01   ` Jonathan Cameron via [this message]
2025-07-03  4:17   ` Fan Ni
2025-07-03  9:05     ` Jonathan Cameron via
2025-06-26 22:23 ` [QEMU PATCH v4 10/10] cxl-mailbox-utils: 0x5605 - FMAPI Initiate DC Release anisa.su887
2025-06-30 17:06   ` Jonathan Cameron via
2025-07-03  4:21   ` Fan Ni
2025-07-03  9:02     ` Jonathan Cameron via

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=20250630180110.0000060d@huawei.com \
    --to=qemu-devel@nongnu.org \
    --cc=Jonathan.Cameron@huawei.com \
    --cc=alok.a.tiwari@oracle.com \
    --cc=anisa.su887@gmail.com \
    --cc=anisa.su@samsung.com \
    --cc=dave@stgolabs.net \
    --cc=linux-cxl@vger.kernel.org \
    --cc=nifan.cxl@gmail.com \
    /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 NNTP newsgroup(s).