Re: [Qemu PATCH v2] hw/cxl: fix DC extent capacity tracking
On Thu, May 29, 2025 at 06:07:34PM +0100, Jonathan Cameron wrote: > On Thu, 29 May 2025 16:34:25 + > [email protected] wrote: > > > From: Fan Ni > > > > Per cxl r3.2 Section 9.13.3.3, extent capacity tracking should include > > extents in different states including added, pending, etc. > > > > Before the change, for the in-device extent number tracking purpose, we only > > have "total_extent_count" defined, which only tracks the number of > > extents accepted. However, we need to track number of extents in other > > states also, for now it is extents pending-to-add. > > > > To fix that, we introduce a new counter for dynamic capacity > > "nr_extents_accepted" which explicitly tracks number of the extents > > accepted by the hosts, and fix "total_extent_count" to include > > both accepted and pending extents counting. > > > > Signed-off-by: Fan Ni > > Reviewed-by: Jonathan Cameron > > Thanks. Michael, would you mind picking this up directly if > you are happy with it? sure > It is an esoteric corner case but > we should emulate resource exhaustion for tracking extents correctly. > > I don't have many fixes queued up at the moment to make it worth > me bundling them up into a little series. There is just this one > and the one for Register Locator capability size I posted > earlier today (Fan can you take a look at that one?) > > > --- > > v2: > > 1) No functional changes; > > 2) Rebased the code to ToT of master branch; > > 3) Picked up tag; > > > > v1: > > https://lore.kernel.org/linux-cxl/[email protected]/ > > --- > > hw/cxl/cxl-mailbox-utils.c | 26 ++ > > hw/mem/cxl_type3.c | 1 + > > include/hw/cxl/cxl_device.h | 3 ++- > > 3 files changed, 21 insertions(+), 9 deletions(-) > > > > diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c > > index 299f232f26..0b615ea37a 100644 > > --- a/hw/cxl/cxl-mailbox-utils.c > > +++ b/hw/cxl/cxl-mailbox-utils.c > > @@ -2750,7 +2750,7 @@ static CXLRetCode cmd_dcd_get_dyn_cap_ext_list(const > > struct cxl_cmd *cmd, > > uint16_t out_pl_len, size; > > CXLDCExtent *ent; > > > > -if (start_extent_id > ct3d->dc.total_extent_count) { > > +if (start_extent_id > ct3d->dc.nr_extents_accepted) { > > return CXL_MBOX_INVALID_INPUT; > > } > > > > @@ -2761,7 +2761,7 @@ static CXLRetCode cmd_dcd_get_dyn_cap_ext_list(const > > struct cxl_cmd *cmd, > > out_pl_len = sizeof(*out) + record_count * sizeof(out->records[0]); > > > > stl_le_p(&out->count, record_count); > > -stl_le_p(&out->total_extents, ct3d->dc.total_extent_count); > > +stl_le_p(&out->total_extents, ct3d->dc.nr_extents_accepted); > > stl_le_p(&out->generation_num, ct3d->dc.ext_list_gen_seq); > > > > if (record_count > 0) { > > @@ -2883,16 +2883,20 @@ void > > cxl_extent_group_list_insert_tail(CXLDCExtentGroupList *list, > > QTAILQ_INSERT_TAIL(list, group, node); > > } > > > > -void cxl_extent_group_list_delete_front(CXLDCExtentGroupList *list) > > +uint32_t cxl_extent_group_list_delete_front(CXLDCExtentGroupList *list) > > { > > CXLDCExtent *ent, *ent_next; > > CXLDCExtentGroup *group = QTAILQ_FIRST(list); > > +uint32_t extents_deleted = 0; > > > > QTAILQ_REMOVE(list, group, node); > > QTAILQ_FOREACH_SAFE(ent, &group->list, node, ent_next) { > > cxl_remove_extent_from_extent_list(&group->list, ent); > > +extents_deleted++; > > } > > g_free(group); > > + > > +return extents_deleted; > > } > > > > /* > > @@ -3011,7 +3015,7 @@ static CXLRetCode cmd_dcd_add_dyn_cap_rsp(const > > struct cxl_cmd *cmd, > > CXLUpdateDCExtentListInPl *in = (void *)payload_in; > > CXLType3Dev *ct3d = CXL_TYPE3(cci->d); > > CXLDCExtentList *extent_list = &ct3d->dc.extents; > > -uint32_t i; > > +uint32_t i, num; > > uint64_t dpa, len; > > CXLRetCode ret; > > > > @@ -3020,7 +3024,8 @@ static CXLRetCode cmd_dcd_add_dyn_cap_rsp(const > > struct cxl_cmd *cmd, > > } > > > > if (in->num_entries_updated == 0) { > > -cxl_extent_group_list_delete_front(&ct3d->dc.extents_pending); > > +num = > > cxl_extent_group_list_delete_front(&ct3d->dc.extents_pending); > > +ct3d->dc.total_extent_count -= num; > > return CXL_MBOX_SUCCESS; > > } > > > > @@ -3051,10 +3056,12 @@ static CXLRetCode cmd_dcd_add_dyn_cap_rsp(const > > struct cxl_cmd *cmd, > > > > cxl_insert_extent_to_extent_list(extent_list, dpa, len, NULL, 0); > > ct3d->dc.total_extent_count += 1; > > +ct3d->dc.nr_extents_accepted += 1; > > ct3_set_region_block_backed(ct3d, dpa, len); > > } > > /* Remove the first extent group in the pending list */ > > -cxl_extent_group_list_delete_front(&ct3d->dc.extents_pending); > > +num = cxl_extent_group_list_delete_front(&ct3d->dc.extents_pending); > > +ct3d->dc.total_extent_count -= num;
Re: [Qemu PATCH v2] hw/cxl: fix DC extent capacity tracking
On Thu, 29 May 2025 16:34:25 + [email protected] wrote: > From: Fan Ni > > Per cxl r3.2 Section 9.13.3.3, extent capacity tracking should include > extents in different states including added, pending, etc. > > Before the change, for the in-device extent number tracking purpose, we only > have "total_extent_count" defined, which only tracks the number of > extents accepted. However, we need to track number of extents in other > states also, for now it is extents pending-to-add. > > To fix that, we introduce a new counter for dynamic capacity > "nr_extents_accepted" which explicitly tracks number of the extents > accepted by the hosts, and fix "total_extent_count" to include > both accepted and pending extents counting. > > Signed-off-by: Fan Ni > Reviewed-by: Jonathan Cameron Thanks. Michael, would you mind picking this up directly if you are happy with it? It is an esoteric corner case but we should emulate resource exhaustion for tracking extents correctly. I don't have many fixes queued up at the moment to make it worth me bundling them up into a little series. There is just this one and the one for Register Locator capability size I posted earlier today (Fan can you take a look at that one?) > --- > v2: > 1) No functional changes; > 2) Rebased the code to ToT of master branch; > 3) Picked up tag; > > v1: > https://lore.kernel.org/linux-cxl/[email protected]/ > --- > hw/cxl/cxl-mailbox-utils.c | 26 ++ > hw/mem/cxl_type3.c | 1 + > include/hw/cxl/cxl_device.h | 3 ++- > 3 files changed, 21 insertions(+), 9 deletions(-) > > diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c > index 299f232f26..0b615ea37a 100644 > --- a/hw/cxl/cxl-mailbox-utils.c > +++ b/hw/cxl/cxl-mailbox-utils.c > @@ -2750,7 +2750,7 @@ static CXLRetCode cmd_dcd_get_dyn_cap_ext_list(const > struct cxl_cmd *cmd, > uint16_t out_pl_len, size; > CXLDCExtent *ent; > > -if (start_extent_id > ct3d->dc.total_extent_count) { > +if (start_extent_id > ct3d->dc.nr_extents_accepted) { > return CXL_MBOX_INVALID_INPUT; > } > > @@ -2761,7 +2761,7 @@ static CXLRetCode cmd_dcd_get_dyn_cap_ext_list(const > struct cxl_cmd *cmd, > out_pl_len = sizeof(*out) + record_count * sizeof(out->records[0]); > > stl_le_p(&out->count, record_count); > -stl_le_p(&out->total_extents, ct3d->dc.total_extent_count); > +stl_le_p(&out->total_extents, ct3d->dc.nr_extents_accepted); > stl_le_p(&out->generation_num, ct3d->dc.ext_list_gen_seq); > > if (record_count > 0) { > @@ -2883,16 +2883,20 @@ void > cxl_extent_group_list_insert_tail(CXLDCExtentGroupList *list, > QTAILQ_INSERT_TAIL(list, group, node); > } > > -void cxl_extent_group_list_delete_front(CXLDCExtentGroupList *list) > +uint32_t cxl_extent_group_list_delete_front(CXLDCExtentGroupList *list) > { > CXLDCExtent *ent, *ent_next; > CXLDCExtentGroup *group = QTAILQ_FIRST(list); > +uint32_t extents_deleted = 0; > > QTAILQ_REMOVE(list, group, node); > QTAILQ_FOREACH_SAFE(ent, &group->list, node, ent_next) { > cxl_remove_extent_from_extent_list(&group->list, ent); > +extents_deleted++; > } > g_free(group); > + > +return extents_deleted; > } > > /* > @@ -3011,7 +3015,7 @@ static CXLRetCode cmd_dcd_add_dyn_cap_rsp(const struct > cxl_cmd *cmd, > CXLUpdateDCExtentListInPl *in = (void *)payload_in; > CXLType3Dev *ct3d = CXL_TYPE3(cci->d); > CXLDCExtentList *extent_list = &ct3d->dc.extents; > -uint32_t i; > +uint32_t i, num; > uint64_t dpa, len; > CXLRetCode ret; > > @@ -3020,7 +3024,8 @@ static CXLRetCode cmd_dcd_add_dyn_cap_rsp(const struct > cxl_cmd *cmd, > } > > if (in->num_entries_updated == 0) { > -cxl_extent_group_list_delete_front(&ct3d->dc.extents_pending); > +num = cxl_extent_group_list_delete_front(&ct3d->dc.extents_pending); > +ct3d->dc.total_extent_count -= num; > return CXL_MBOX_SUCCESS; > } > > @@ -3051,10 +3056,12 @@ static CXLRetCode cmd_dcd_add_dyn_cap_rsp(const > struct cxl_cmd *cmd, > > cxl_insert_extent_to_extent_list(extent_list, dpa, len, NULL, 0); > ct3d->dc.total_extent_count += 1; > +ct3d->dc.nr_extents_accepted += 1; > ct3_set_region_block_backed(ct3d, dpa, len); > } > /* Remove the first extent group in the pending list */ > -cxl_extent_group_list_delete_front(&ct3d->dc.extents_pending); > +num = cxl_extent_group_list_delete_front(&ct3d->dc.extents_pending); > +ct3d->dc.total_extent_count -= num; > > return CXL_MBOX_SUCCESS; > } > @@ -3160,7 +3167,7 @@ free_and_exit: > } > *updated_list_size = 0; > } else { > -*updated_list_size = ct3d->dc.total_extent_count + cnt_delta; > +*updated_list_size = ct3d->dc.nr_extents_accepted + cnt_delta; > } > > return ret; > @@
