On 5/21/25 7:40 AM, Jonathan Cameron wrote:
> On Wed, 21 May 2025 13:47:41 +0100
> <shiju.jose@xxxxxxxxxx> wrote:
>
>> From: Shiju Jose <shiju.jose@xxxxxxxxxx>
>>
>> CXL spec 3.2 section 8.2.10.9.11.1 describes the device patrol scrub
>> control feature. The device patrol scrub proactively locates and makes
>> corrections to errors in regular cycle.
>>
>> Allow specifying the number of hours within which the patrol scrub must be
>> completed, subject to minimum and maximum limits reported by the device.
>> Also allow disabling scrub allowing trade-off error rates against
>> performance.
>>
>> Add support for patrol scrub control on CXL memory devices.
>> Register with the EDAC device driver, which retrieves the scrub attribute
>> descriptors from EDAC scrub and exposes the sysfs scrub control attributes
>> to userspace. For example, scrub control for the CXL memory device
>> "cxl_mem0" is exposed in /sys/bus/edac/devices/cxl_mem0/scrubX/.
>>
>> Additionally, add support for region-based CXL memory patrol scrub control.
>> CXL memory regions may be interleaved across one or more CXL memory
>> devices. For example, region-based scrub control for "cxl_region1" is
>> exposed in /sys/bus/edac/devices/cxl_region1/scrubX/.
>>
>> [dj: Add cxl_test inclusion of edac.o]
>> [dj: Check return from cxl_feature_info() with IS_ERR]
>
> Trivial question on these. What do they reflect? Some changes
> Dave made on a prior version? Or changes in response to feedback
> (in which case they should be below the ---)
Ah those are the notations I inserted when I pulled the the branch for merge testing and Shiju picked up. Normally those would go to Linus. But in this case, it can be dropped since there is another reversion for the series and the changes and folded in by Shiju.
DJ
>
>>
>> Reviewed-by: Dave Jiang <dave.jiang@xxxxxxxxx>
>> Co-developed-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
>> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
>> Signed-off-by: Shiju Jose <shiju.jose@xxxxxxxxxx>
>
> A couple of formatting trivial things inline from the refactors
> in this version. Maybe Dave can tweak them whilst applying if
> nothing else comes up?
>
> J
>
>> diff --git a/drivers/cxl/core/edac.c b/drivers/cxl/core/edac.c
>> new file mode 100644
>> index 000000000000..eae99ed7c018
>> --- /dev/null
>> +++ b/drivers/cxl/core/edac.c
>> @@ -0,0 +1,520 @@
>
>> +static int cxl_scrub_get_attrbs(struct cxl_patrol_scrub_context *cxl_ps_ctx,
>> + u8 *cap, u16 *cycle, u8 *flags, u8 *min_cycle)
>> +{
>> + struct cxl_mailbox *cxl_mbox;
>> + u8 min_scrub_cycle = U8_MAX;
>> + struct cxl_region_params *p;
>> + struct cxl_memdev *cxlmd;
>> + struct cxl_region *cxlr;
>> + int i, ret;
>> +
>> + if (!cxl_ps_ctx->cxlr) {
>> + cxl_mbox = &cxl_ps_ctx->cxlmd->cxlds->cxl_mbox;
>> + return cxl_mem_scrub_get_attrbs(cxl_mbox, cap, cycle,
>> + flags, min_cycle);
>> + }
>> +
>> + struct rw_semaphore *region_lock __free(rwsem_read_release) =
>> + rwsem_read_intr_acquire(&cxl_region_rwsem);
>
> Trivial but that should be indented one tab more.
>
>> + if (!region_lock)
>> + return -EINTR;
>> +
>> + cxlr = cxl_ps_ctx->cxlr;
>> + p = &cxlr->params;
>> +
>> + for (i = 0; i < p->nr_targets; i++) {
>> + struct cxl_endpoint_decoder *cxled = p->targets[i];
>> +
>> + cxlmd = cxled_to_memdev(cxled);
>> + cxl_mbox = &cxlmd->cxlds->cxl_mbox;
>> + ret = cxl_mem_scrub_get_attrbs(cxl_mbox, cap, cycle,
>> + flags, min_cycle);
>
> Maybe move flags to previous line.
>
>> + if (ret)
>> + return ret;
>> +
>> + if (min_cycle)
>> + min_scrub_cycle =
>> + min(*min_cycle, min_scrub_cycle);
>
> No need for the line wrap any more.
>
>
>> + }
>> +
>> + if (min_cycle)
>> + *min_cycle = min_scrub_cycle;
>> +
>> + return 0;
>> +}
>
>
