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 ---)
>
> 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;
> +}
