Re: [PATCH v7 11/12] hw/cxl/cxl-mailbox-utils: Add superset extent release mailbox support

[Gregory Price]

On Thu, Apr 18, 2024 at 04:11:02PM -0700, nifan.cxl@gmail.com wrote:
> From: Fan Ni <fan.ni@samsung.com>
> 
> With the change, we extend the extent release mailbox command processing
> to allow more flexible release. As long as the DPA range of the extent to
> release is covered by accepted extent(s) in the device, the release can be
> performed.
> 
> Signed-off-by: Fan Ni <fan.ni@samsung.com>
> ---
>  hw/cxl/cxl-mailbox-utils.c | 21 ++++++++-------------
>  1 file changed, 8 insertions(+), 13 deletions(-)
>

Hmmm.  This will complicate MHD accounting, but it looks ok to me as-is.

Reviewed-by: Gregory Price <gregory.price@memverge.com>

> diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
> index 57f1ce9cce..89f0ab8116 100644
> --- a/hw/cxl/cxl-mailbox-utils.c
> +++ b/hw/cxl/cxl-mailbox-utils.c
> @@ -1704,6 +1704,13 @@ static CXLRetCode 
> cxl_dc_extent_release_dry_run(CXLType3Dev *ct3d,
>          dpa = in->updated_entries[i].start_dpa;
>          len = in->updated_entries[i].len;
>  
> +        /* Check if the DPA range is not fully backed with valid extents */
> +        if (!ct3_test_region_block_backed(ct3d, dpa, len)) {
> +            ret = CXL_MBOX_INVALID_PA;
> +            goto free_and_exit;
> +        }
> +
> +        /* After this point, extent overflow is the only error can happen */
>          while (len > 0) {
>              QTAILQ_FOREACH(ent, updated_list, node) {
>                  range_init_nofail(&range, ent->start_dpa, ent->len);
> @@ -1718,14 +1725,7 @@ static CXLRetCode 
> cxl_dc_extent_release_dry_run(CXLType3Dev *ct3d,
>                      if (range_contains(&range, dpa + len - 1)) {
>                          len2 = ent_start_dpa + ent_len - dpa - len;
>                      } else {
> -                        /*
> -                         * TODO: we reject the attempt to remove an extent
> -                         * that overlaps with multiple extents in the device
> -                         * for now. We will allow it once superset release
> -                         * support is added.
> -                         */
> -                        ret = CXL_MBOX_INVALID_PA;
> -                        goto free_and_exit;
> +                        dpa = ent_start_dpa + ent_len;
>                      }
>                      len_done = ent_len - len1 - len2;
>  
> @@ -1752,14 +1752,9 @@ static CXLRetCode 
> cxl_dc_extent_release_dry_run(CXLType3Dev *ct3d,
>                      }
>  
>                      len -= len_done;
> -                    /* len == 0 here until superset release is added */
>                      break;
>                  }
>              }
> -            if (len) {
> -                ret = CXL_MBOX_INVALID_PA;
> -                goto free_and_exit;
> -            }
>          }
>      }
>  free_and_exit:
> -- 
> 2.43.0
>