Re: [QEMU PATCH v3 8/9] cxl-mailbox-utils: 0x5604 - FMAPI Initiate DC Add

[Jonathan Cameron <Jonathan.Cameron@huawei.com>]

> > > +/* 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);
> > > +    CXLUpdateDCExtentListInPl *list;
> > > +    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;
> > > +        }
> > > +
> > > +        list = calloc(1, (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];
> > > +             /* Check requested extents do not overlap with pending extents. */
> > > +            if (cxl_extent_groups_overlaps_dpa_range(&ct3d->dc.extents_pending,
> > > +                                                     ext.start_dpa, ext.len)) {
> > > +                return CXL_MBOX_INVALID_EXTENT_LIST;
> > > +            }
> > > +            /* Check requested extents do not overlap with existing extents. */
> > > +            if (cxl_extents_overlaps_dpa_range(&ct3d->dc.extents,
> > > +                                               ext.start_dpa, ext.len)) {
> > > +                return CXL_MBOX_INVALID_EXTENT_LIST;
> > > +            }
> > > +        }
> > > +
> > > +        if (rc) {
> > > +            return rc;  
> > 
> > is list free not require ?
> >   
> oh yeah it's definitely required :( that was dumb of me, same for the
> next patch.

Use a g_autofree marking for this probably given it is just
a local temp array I think.

> 
> Thanks for catching that!
> -Anisa
> > > +        }
> > > +
> > > +        cxl_mbox_dc_add_to_pending(ct3d, in->ext_count, in->extents);
> > > +        cxl_mbox_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;
> > > +    }
> > > +
> > > +    return CXL_MBOX_SUCCESS;  
> > 
> > CXL_MBOX_SUCCESS, this is unreachable
> >   
> > > +}
> > > +
> > >   static const struct cxl_cmd cxl_cmd_set[256][256] = {
> > >       [INFOSTAT][BACKGROUND_OPERATION_ABORT] = { "BACKGROUND_OPERATION_ABORT",
> > >           cmd_infostat_bg_op_abort, 0, 0 },
> > > @@ -3667,6 +3812,13 @@ static const struct cxl_cmd cxl_cmd_set_fm_dcd[256][256] = {
> > >            CXL_MBOX_IMMEDIATE_DATA_CHANGE) },
> > >       [FMAPI_DCD_MGMT][GET_DC_REGION_EXTENT_LIST] = { "GET_DC_REGION_EXTENT_LIST",
> > >           cmd_fm_get_dc_region_extent_list, 12, 0 },
> > > +    [FMAPI_DCD_MGMT][INITIATE_DC_ADD] = { "INIT_DC_ADD",
> > > +        cmd_fm_initiate_dc_add, ~0,
> > > +        (CXL_MBOX_CONFIG_CHANGE_COLD_RESET |
> > > +        CXL_MBOX_CONFIG_CHANGE_CONV_RESET |
> > > +        CXL_MBOX_CONFIG_CHANGE_CXL_RESET |
> > > +        CXL_MBOX_IMMEDIATE_CONFIG_CHANGE |
> > > +        CXL_MBOX_IMMEDIATE_DATA_CHANGE) },
> > >   };  
> > 
> > 
> > Thanks,
> > Alok