From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id B5D91D6B6A7 for ; Wed, 30 Oct 2024 16:16:42 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 30E288D0003; Wed, 30 Oct 2024 12:16:42 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 2BC398D0001; Wed, 30 Oct 2024 12:16:42 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 136D08D0003; Wed, 30 Oct 2024 12:16:42 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id E4DA68D0001 for ; Wed, 30 Oct 2024 12:16:41 -0400 (EDT) Received: from smtpin22.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 524F180120 for ; Wed, 30 Oct 2024 16:16:41 +0000 (UTC) X-FDA: 82730771046.22.4919A10 Received: from frasgout.his.huawei.com (frasgout.his.huawei.com [185.176.79.56]) by imf12.hostedemail.com (Postfix) with ESMTP id 06D314001F for ; Wed, 30 Oct 2024 16:16:25 +0000 (UTC) Authentication-Results: imf12.hostedemail.com; dkim=none; dmarc=pass (policy=quarantine) header.from=huawei.com; spf=pass (imf12.hostedemail.com: domain of jonathan.cameron@huawei.com designates 185.176.79.56 as permitted sender) smtp.mailfrom=jonathan.cameron@huawei.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1730304840; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=i6k4/R3B2e76ooEFhV8kJNyUGOljOVb9rNb3lXmRV98=; b=APzoep3pi+NYh1doDQiIANcWvd1eItmXPjosuWtxeWLJ5RV9FvL7KFRZ4Q8tsuaTalL/+R LG6AWFA7PtDedBc3Pf7VJ+y34/T2F4pR7AQEfMW7SWJImjJqlV/8uGUeiNDwoqM5KKbIYM t6XbwtJ3Y22KbcRdq+BTeuQjsGsur5s= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1730304840; a=rsa-sha256; cv=none; b=1AuwEKwbceusbTbCQFhpksv2IYnQfTUhycFGmu4H9aRt8+GyFooExQYNpSStlt+oaAhMDB 6QX3W9E1x9eOoeNJ81n/aOSH+2BwZbXTvAkRQ/H1n8+ZR1O7sP4iMbAufZJAXSh8Hz4QcL oQOlj7NkQpmyp/5PvyJl7h2XhP8cVO4= ARC-Authentication-Results: i=1; imf12.hostedemail.com; dkim=none; dmarc=pass (policy=quarantine) header.from=huawei.com; spf=pass (imf12.hostedemail.com: domain of jonathan.cameron@huawei.com designates 185.176.79.56 as permitted sender) smtp.mailfrom=jonathan.cameron@huawei.com Received: from mail.maildlp.com (unknown [172.18.186.31]) by frasgout.his.huawei.com (SkyGuard) with ESMTP id 4XdsdB5CCXz6K6Gy; Thu, 31 Oct 2024 00:14:06 +0800 (CST) Received: from frapeml500008.china.huawei.com (unknown [7.182.85.71]) by mail.maildlp.com (Postfix) with ESMTPS id 36F341400D3; Thu, 31 Oct 2024 00:16:32 +0800 (CST) Received: from localhost (10.203.177.66) by frapeml500008.china.huawei.com (7.182.85.71) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.39; Wed, 30 Oct 2024 17:16:30 +0100 Date: Wed, 30 Oct 2024 16:16:28 +0000 From: Jonathan Cameron To: Dave Jiang CC: Shiju Jose , "linux-edac@vger.kernel.org" , "linux-cxl@vger.kernel.org" , "linux-acpi@vger.kernel.org" , "linux-mm@kvack.org" , "linux-kernel@vger.kernel.org" , "bp@alien8.de" , "tony.luck@intel.com" , "rafael@kernel.org" , "lenb@kernel.org" , "mchehab@kernel.org" , "dan.j.williams@intel.com" , "dave@stgolabs.net" , "gregkh@linuxfoundation.org" , "sudeep.holla@arm.com" , "jassisinghbrar@gmail.com" , "alison.schofield@intel.com" , "vishal.l.verma@intel.com" , "ira.weiny@intel.com" , "david@redhat.com" , "Vilas.Sridharan@amd.com" , "leo.duran@amd.com" , "Yazen.Ghannam@amd.com" , "rientjes@google.com" , "jiaqiyan@google.com" , "Jon.Grimm@amd.com" , "dave.hansen@linux.intel.com" , "naoya.horiguchi@nec.com" , "james.morse@arm.com" , "jthoughton@google.com" , "somasundaram.a@hpe.com" , "erdemaktas@google.com" , "pgonda@google.com" , "duenwen@google.com" , "gthelen@google.com" , "wschwartz@amperecomputing.com" , "dferguson@amperecomputing.com" , "wbs@os.amperecomputing.com" , "nifan.cxl@gmail.com" , tanxiaofei , "Zengtao (B)" , "Roberto Sassu" , "kangkang.shen@futurewei.com" , wanghuiqiang , Linuxarm Subject: Re: [PATCH v14 07/14] cxl/memfeature: Add CXL memory device patrol scrub control feature Message-ID: <20241030161628.00001fdc@Huawei.com> In-Reply-To: <67b569b0-1cd5-44e0-8465-064b41a1afd8@intel.com> References: <20241025171356.1377-1-shiju.jose@huawei.com> <20241025171356.1377-8-shiju.jose@huawei.com> <3a007a70-136b-4a45-8dd2-d33725ea96bc@intel.com> <67b569b0-1cd5-44e0-8465-064b41a1afd8@intel.com> Organization: Huawei Technologies Research and Development (UK) Ltd. X-Mailer: Claws Mail 4.1.0 (GTK 3.24.33; x86_64-w64-mingw32) MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.203.177.66] X-ClientProxiedBy: lhrpeml500004.china.huawei.com (7.191.163.9) To frapeml500008.china.huawei.com (7.182.85.71) X-Rspamd-Queue-Id: 06D314001F X-Stat-Signature: 6x7tt6fs968bap6p4ypawm7bnz34weu3 X-Rspamd-Server: rspam09 X-Rspam-User: X-HE-Tag: 1730304985-483411 X-HE-Meta: U2FsdGVkX1+WWYxdzqdKlghUVspAdqvsxMA5jHaRGSN2tW2IiovLgOjyrYvkKz/oFy6qHzgdEh7wChFElDD9fU83+7JhB5fP+4oYy7j9TGUj6yUysT82TR1r9gIk3ZJ3NVH4rUnMDe1AcDxJi9td/BJfZRUHj2dPeOagvR/4WL9ItMV0BG6WDQiQslHCKDByRbttUWv7+JabSzrAXMwyuf8GG+2kWchsDebQl7NDBe3SqpLHZhGxIZ583vFGGghEWJcZxtCcRAn5iK74CoM1TbR6HQq0bzhemqW6AynTMxwALusOb5XW2qTolL/rcLpnvsaOfjUZQ2ijLvX1OEW/ZR3AzTe45+dJ1MWYtxEZTYhlgp2VX3N1LhRuBjn49bCIqbxHNr0q5ORmeSPMCNAL09uOhUdpHhm7oEMLTGq629XXaNYyQnZsZ5Sd/HyiFFGLa6lx3W4MvjnFkQkvpu8/z4KudSv170ecu2nvm+LB+S7qMbhcsgOyGJ9iyd1vDcVImPuVepBDIMh0NBN0T9K3fHpNiZq9Tl8ysTiIcJo90DxMi9SgLVHzVZKqrlj6Ny5YF4Z4ykoLnz4DY3DZ5SXqId1K1wFQUW5oKdG4DZf0jN4k33MiZx3GHtnsPeUtYyrcEk0kHYiIlFVIRp1bjB0NQJm25TASp7X6xbxv75A30AyBX5Q1DgqddRAxTv/JleByvtMt2RuSW3+4ySKXUgwHNXlEaKlYt3FcnNRdqBiwnFr7CHhqRSHjqMVWUgR3UTrdluMzO4111nKdo+Ks8K8MpUGJrAm8Ouv2Zbq6zk2e2d7z/5a5yezGk002XmY6ULfeYqxY5vg5GYCX3S+OwXCKficlfXLRepNipWMZtHeHZxW7OKSxeZ+yBzU0ZbYCHPUCKus1Qc+M8KPIAeXkuauBI+2AtDwmcMJFbNdWLh3ZF24E3pLFX+LadHENYY96gW0v+AkAFStjcQ3S/s4Aeg2 XFiLshCj zcEJ+X1d31xsYPaiARgV1GkLvjMUnZD6WjYN8GpijOjExOkCBQ81ISsbFz6cGHarNIAOkMS6lmbR3Ywpytkr1HI+5A+j6mtWV+T56vyDfEkvQ9M8Csg/FmMrT+macDJ1TUQDbu5TJHMqV9yRBzOba4BKlTDeidBz2Ky8ZXLcy95cfbY2DY1H+kf/Ko1vG0hKsx46TVdsAaVSA/tr7Jk0obQ9JTYoeTPUe1W66If5MKOroO8FNYfS/EySZRW1GZuduCbUPMQpo6hloeG/z/nutBARc533mpOMGnQcpI48+8tT1rGXR8Te7fJAMHVJMsSTWU/0X3biO4nZFBw5XbXVx/u8V7dwvURxl/oMBb716K+ZxBaDGdHPBsleb9cpwVInT9ukBiaOfIwW7OBTAd8UAuaZBWViyg0BbQUZHoBR0nU1c0p/r1G6KtePKhyoNBVCNLPZwwfPWhHIReMQxrwNra5dcYgxafHdVdH0snIb0MDRWGW2ig6CiXuDMDq6gHZI+zQ+W4JGfVGMOsilraSEzlB5N64yXc09yjRsuqRRxBxIfEj5ctuV6bdqdEK4vRgVVk+ZUrtdnaiYgojY/l2qBla7axMYk9fr9mbFU8RxHH/MlEOZ7apgRzQq3JQ8ZduWx6otrJom7fuGBOA7rHbNgtRPwccubsx2u4yOk9k4eGXjgwYcqhYC1XV36a001axCA5uRIMKof3UGyGnc+KpZEvK8Iw3AwX2xi++dWIvxWDbPCJBjYGRcSzq99dOXpL6nbUw448LL3jQh8kASVZu7GtI/3wEWVYgdr0wro/n1HYoC+xSkdaUw//G2C3S940ahSbNIyM6t7w2OYyvr+sSwgHNWOMRAawumSgGv5rHLF6KYkFvek/QKIzZGHVxV3g4YlkCexWhadFuhexOXlIOjptIOeDxhiH3i8nX/RzHBQcx9YzQJTXH11kcI6ZA== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Tue, 29 Oct 2024 11:32:47 -0700 Dave Jiang wrote: > On 10/29/24 10:00 AM, Shiju Jose wrote: > > > > > >> -----Original Message----- > >> From: Dave Jiang > >> Sent: 29 October 2024 16:32 > >> To: Shiju Jose ; linux-edac@vger.kernel.org; linux- > >> cxl@vger.kernel.org; linux-acpi@vger.kernel.org; linux-mm@kvack.org; linux- > >> kernel@vger.kernel.org > >> Cc: bp@alien8.de; tony.luck@intel.com; rafael@kernel.org; lenb@kernel.org; > >> mchehab@kernel.org; dan.j.williams@intel.com; dave@stgolabs.net; Jonathan > >> Cameron ; gregkh@linuxfoundation.org; > >> sudeep.holla@arm.com; jassisinghbrar@gmail.com; alison.schofield@intel.com; > >> vishal.l.verma@intel.com; ira.weiny@intel.com; david@redhat.com; > >> Vilas.Sridharan@amd.com; leo.duran@amd.com; Yazen.Ghannam@amd.com; > >> rientjes@google.com; jiaqiyan@google.com; Jon.Grimm@amd.com; > >> dave.hansen@linux.intel.com; naoya.horiguchi@nec.com; > >> james.morse@arm.com; jthoughton@google.com; somasundaram.a@hpe.com; > >> erdemaktas@google.com; pgonda@google.com; duenwen@google.com; > >> gthelen@google.com; wschwartz@amperecomputing.com; > >> dferguson@amperecomputing.com; wbs@os.amperecomputing.com; > >> nifan.cxl@gmail.com; tanxiaofei ; Zengtao (B) > >> ; Roberto Sassu ; > >> kangkang.shen@futurewei.com; wanghuiqiang ; > >> Linuxarm > >> Subject: Re: [PATCH v14 07/14] cxl/memfeature: Add CXL memory device patrol > >> scrub control feature > >> > >> > >> > >> On 10/25/24 10:13 AM, shiju.jose@huawei.com wrote: > >>> From: Shiju Jose > >>> > >>> CXL spec 3.1 section 8.2.9.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/. > >>> > >>> Co-developed-by: Jonathan Cameron > >>> Signed-off-by: Jonathan Cameron > >>> Signed-off-by: Shiju Jose > >>> --- > >>> Documentation/edac/edac-scrub.rst | 74 ++++++ > >>> drivers/cxl/Kconfig | 18 ++ > >>> drivers/cxl/core/Makefile | 1 + > >>> drivers/cxl/core/memfeature.c | 381 ++++++++++++++++++++++++++++++ > >>> drivers/cxl/core/region.c | 6 + > >>> drivers/cxl/cxlmem.h | 7 + > >>> drivers/cxl/mem.c | 4 + > >>> 7 files changed, 491 insertions(+) > >>> create mode 100644 Documentation/edac/edac-scrub.rst create mode > >>> 100644 drivers/cxl/core/memfeature.c > >>> > >>> diff --git a/Documentation/edac/edac-scrub.rst > >>> b/Documentation/edac/edac-scrub.rst > >>> new file mode 100644 > >>> index 000000000000..4aad4974b208 > >>> --- /dev/null > >>> +++ b/Documentation/edac/edac-scrub.rst > >>> @@ -0,0 +1,74 @@ > >>> +.. SPDX-License-Identifier: GPL-2.0 > >>> + > > [...] > > > >>> +static int cxl_mem_ps_get_attrs(struct cxl_memdev_state *mds, > >>> + struct cxl_memdev_ps_params *params) { > >>> + size_t rd_data_size = sizeof(struct cxl_memdev_ps_rd_attrs); > >>> + size_t data_size; > >>> + struct cxl_memdev_ps_rd_attrs *rd_attrs __free(kfree) = > >>> + kmalloc(rd_data_size, > >> GFP_KERNEL); > >>> + if (!rd_attrs) > >>> + return -ENOMEM; > >>> + > >>> + data_size = cxl_get_feature(mds, cxl_patrol_scrub_uuid, > >>> + CXL_GET_FEAT_SEL_CURRENT_VALUE, > >>> + rd_attrs, rd_data_size); > >>> + if (!data_size) > >>> + return -EIO; > >>> + > >>> + params->scrub_cycle_changeable = > >> FIELD_GET(CXL_MEMDEV_PS_SCRUB_CYCLE_CHANGE_CAP_MASK, > >>> + rd_attrs->scrub_cycle_cap); > >>> + params->enable = > >> FIELD_GET(CXL_MEMDEV_PS_FLAG_ENABLED_MASK, > >>> + rd_attrs->scrub_flags); > >>> + params->scrub_cycle_hrs = > >> FIELD_GET(CXL_MEMDEV_PS_CUR_SCRUB_CYCLE_MASK, > >>> + rd_attrs->scrub_cycle_hrs); > >>> + params->min_scrub_cycle_hrs = > >> FIELD_GET(CXL_MEMDEV_PS_MIN_SCRUB_CYCLE_MASK, > >>> + rd_attrs->scrub_cycle_hrs); > >>> + > >>> + return 0; > >>> +} > >>> + > >>> +static int cxl_ps_get_attrs(struct device *dev, void *drv_data, > >> > >> Would a union be better than a void *drv_data for all the places this is used as a > >> parameter? How many variations of this are there? > >> > >> DJ > > Hi Dave, > > > > Can you give more info on this given this is a generic callback for the scrub control and each > > implementation will have its own context struct (for eg. struct cxl_patrol_scrub_context here > > for CXL scrub control), which in turn will be passed in and out as opaque data. > > Mainly I'm just seeing a lot of calls with (void *). Just asking if we want to make it a union that contains 'struct cxl_patrol_scrub_context' and etc. You could but then every new driver would need to include changes in the edac core to add it's own entry to that union. Not sure that's a good way to go for opaque driver specific context. This particular function though can use a struct cxl_patrol_scrub_context * anyway as it's not part of the core interface, but rather one called only indirectly by functions that are passed a void * but know it is a struct clx_patrol_scrub_context *. Jonathan > > > > > Thanks, > > Shiju > >> > >>> + struct cxl_memdev_ps_params *params) { > >>> + struct cxl_patrol_scrub_context *cxl_ps_ctx = drv_data; > >>> + struct cxl_memdev *cxlmd; > >>> + struct cxl_dev_state *cxlds; > >>> + struct cxl_memdev_state *mds; > >>> + u16 min_scrub_cycle = 0; > >>> + int i, ret; > >>> + > >>> + if (cxl_ps_ctx->cxlr) { > >>> + struct cxl_region *cxlr = cxl_ps_ctx->cxlr; > >>> + struct cxl_region_params *p = &cxlr->params; > >>> + > >>> + for (i = p->interleave_ways - 1; i >= 0; i--) { > >>> + struct cxl_endpoint_decoder *cxled = p->targets[i]; > >>> + > >>> + cxlmd = cxled_to_memdev(cxled); > >>> + cxlds = cxlmd->cxlds; > >>> + mds = to_cxl_memdev_state(cxlds); > >>> + ret = cxl_mem_ps_get_attrs(mds, params); > >>> + if (ret) > >>> + return ret; > >>> + > >>> + if (params->min_scrub_cycle_hrs > min_scrub_cycle) > >>> + min_scrub_cycle = params- > >>> min_scrub_cycle_hrs; > >>> + } > >>> + params->min_scrub_cycle_hrs = min_scrub_cycle; > >>> + return 0; > >>> + } > >>> + cxlmd = cxl_ps_ctx->cxlmd; > >>> + cxlds = cxlmd->cxlds; > >>> + mds = to_cxl_memdev_state(cxlds); > >>> + > >>> + return cxl_mem_ps_get_attrs(mds, params); } > >>> + > > [...] > >> > > > >