Re: [PATCH v13 11/15] cxl: Validate and synchronize HDM ranges around reset

[Srirangan Madhavan <smadhavan@nvidia.com>]

On 9/23/26 2:40 PM, Cheatham, Benjamin wrote:
> External email: Use caution opening links or attachments
> 
>> +
>> +     list_for_each_entry(range, &ctx->ranges, list)
>> +             if (range->hpa_range.start == hpa_range->start &&
>> +                 range->hpa_range.end == hpa_range->end)
> 
> Maybe resource_contains() instead? I don't think there'll be any overlap, but no point in reserving
> a fully contained resource. You'd have to add some extra logic to remove the smaller resource though.
>

Done in v14. I used range_contains() for the struct range values. 
Already covered is skipped, and a smaller collected range is removed 
when a larger one arrives.

>> +
>> +             if (!(config->flags & CXL_DECODER_F_ENABLE))
>> +                     continue;
>> +
>> +             /* A committed zero-size decoder maps no HPA. */
>> +             if (!range_len(&config->hpa_range))
>> +                     continue;
> 
> Nit: these two if statements could be on a single line and just put the comment above it, it's pretty
> obvious which condition the comment is talking about.
> 

Done

>> +
>> +out_destroy_ranges:
>> +     cxl_hdm_range_context_destroy(range_ctx);
>>
> 
> Same thing here as the last patch, but I'll let you do this one ;).
> 

Ack. V14 splits the reset sequence into smaller helpers and removes this 
cleanup-label flow. Please let me know if it is better now.

>> +
>> +     INIT_LIST_HEAD(&range_ctx.ranges);
>> +
>> +     scoped_guard(rwsem_write, &cxl_rwsem.region) {
>> +             rc = cxl_hdm_ranges_prepare(&range_ctx, pdev);
>> +             if (!rc)
>> +                     rc = cxl_reset_execute(pdev, dvsec, cap, &range_ctx);
>> +             else
>> +                     cxl_hdm_range_context_destroy(&range_ctx);
>> +     }
> 
> Probably better to just copy-paste the range context destroy call and re-order as:
> 
>                  rc = cxl_hdm_ranges_prepare(&range_ctx, pdev);
>                  if (rc) {
>                          cxl_hdm_range_context_destroy(&range_ctx);
>                          return rc;
>                  }
> 
> 
>                  rc = cxl_reset_execute(pdev, dvsec, cap, &range_ctx);
>                  cxl_hdm_range_context_destroy(&range_ctx);
> 
> instead.
> 

Ack. In v14, cxl_reset_execute() owns the range context from preparation 
through cleanup, so this handoff between functions is gone.

-- 
Regards,
Srirangan