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 176AFC35FFF for ; Fri, 21 Mar 2025 09:38:33 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 900E4280002; Fri, 21 Mar 2025 05:38:31 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 8B199280001; Fri, 21 Mar 2025 05:38:31 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 752B8280002; Fri, 21 Mar 2025 05:38:31 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 5436E280001 for ; Fri, 21 Mar 2025 05:38:31 -0400 (EDT) Received: from smtpin30.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 98DDDAD694 for ; Fri, 21 Mar 2025 09:38:32 +0000 (UTC) X-FDA: 83245058064.30.2A9C35C Received: from frasgout.his.huawei.com (frasgout.his.huawei.com [185.176.79.56]) by imf28.hostedemail.com (Postfix) with ESMTP id A3FB9C0010 for ; Fri, 21 Mar 2025 09:38:30 +0000 (UTC) Authentication-Results: imf28.hostedemail.com; dkim=none; spf=pass (imf28.hostedemail.com: domain of jonathan.cameron@huawei.com designates 185.176.79.56 as permitted sender) smtp.mailfrom=jonathan.cameron@huawei.com; dmarc=pass (policy=quarantine) header.from=huawei.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1742549910; 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=GrXyuQ2AZF/rxz81XvK1/BykFOMnT3aFbvO2iFjE5g4=; b=NObKQE1VqxLsMXQApAlNMwp4GK/61MYKj7CqyoIay2yf2O1dkxjrm0vVCKMEcw0316tYmn slIcWtBDWS2YPGLDlH5k+0JqlZK3H1GN5tXjhIzG+S9zXrLqKDni1J3To8p+bNnMHFnm59 W7ypi3+4kQ/C7yTNie/rQHx+Y+NTz0E= ARC-Authentication-Results: i=1; imf28.hostedemail.com; dkim=none; spf=pass (imf28.hostedemail.com: domain of jonathan.cameron@huawei.com designates 185.176.79.56 as permitted sender) smtp.mailfrom=jonathan.cameron@huawei.com; dmarc=pass (policy=quarantine) header.from=huawei.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1742549910; a=rsa-sha256; cv=none; b=jetFjnM+8og1UlJ8LAompgnX37jPd8cE16OpeFu0xlVSw/2BXx/INsLe0StDrZrmqDkWI4 BQswBC4E/6UjmmsEkyOlzPfc9tfN5trYoEhdiIIeeN5EgES9ifHUgVqwY8BbP7sEi6dgwt k2qsh7RpKz8p+mg8yJSMPPKQE5npkaU= Received: from mail.maildlp.com (unknown [172.18.186.231]) by frasgout.his.huawei.com (SkyGuard) with ESMTP id 4ZJy1R5Bsvz6L5GL; Fri, 21 Mar 2025 17:33:31 +0800 (CST) Received: from frapeml500008.china.huawei.com (unknown [7.182.85.71]) by mail.maildlp.com (Postfix) with ESMTPS id 4DB00140517; Fri, 21 Mar 2025 17:38:28 +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; Fri, 21 Mar 2025 10:38:27 +0100 Date: Fri, 21 Mar 2025 09:38:25 +0000 From: Jonathan Cameron To: Greg KH CC: , , , , Yicong Yang , , , , Yushan Wang , , Lorenzo Pieralisi , Mark Rutland , Catalin Marinas , "Will Deacon" , Dan Williams Subject: Re: [RFC PATCH 3/6] cache: coherency device class Message-ID: <20250321093825.00004d6b@huawei.com> In-Reply-To: <2025032013-venus-request-b026@gregkh> References: <20250320174118.39173-1-Jonathan.Cameron@huawei.com> <20250320174118.39173-4-Jonathan.Cameron@huawei.com> <2025032013-venus-request-b026@gregkh> X-Mailer: Claws Mail 4.3.0 (GTK 3.24.42; 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: lhrpeml100006.china.huawei.com (7.191.160.224) To frapeml500008.china.huawei.com (7.182.85.71) X-Rspamd-Queue-Id: A3FB9C0010 X-Stat-Signature: cwga68aijanr5wgbkkxwmkfh314ndk8m X-Rspam-User: X-Rspamd-Server: rspam12 X-HE-Tag: 1742549910-640100 X-HE-Meta: U2FsdGVkX1/UWZvQrs19Eg528v8UsysKmOhbqovd9BZ4Al2cD/JXeDFiyWa8WtB9R5gu7SsM57cCNxulz7JQc5weLVOb7SYV7OKQjrlmYVczd/paNPY+o7z1EAzbSZrFEYfC1p3QzkCoXY6kPY7o4OrbrVlMFDIdWRNeWtzE+pPe01ybFVX31cIipsGPBNgqYoO6D67wsyqZnl205UTlTBo8ZI01dT3pXSljr5JqdVMIPK6Px/oZXXniaU9BqrQ9BMqjbKWXJW+Vw9BK/prygi0aCgdgDgEOu233Brpc2wQfch0eMg6snJlwaJwF+aeesE7DIKROdH06r+BWCYV3D0mJ05nrw7k9n1i3OVap3L3qQ4E0tAKypUWdCwpOiMVPaJMYL3EGLTp9JWIXx0un4corCW82BqwYfH7WwN3lejAOxvDJBlZkTUskJGsx2HvsS7xiAKfz4UX8/B32olNJxu9rf1gpUddmPIydDj7Aj9cTmcQ8TRuQ9roHF3W2ndL3PPpnF+fbrYerbREuzh+ERD4llX8KmG1SXEDUR8M3dsvqzj5rhmIBHY7Au2qwx2haJVRoNT0ocaFHHAAmpGjEHyMORIYyg2IcCvDCxXo/XUuurWD61ECJFkhK2bmEDB9EY8h/JCw2zTXUJjASdO4Cvo/Vjvd0TzlUD/6zS5JfzEcnJybvWu0H6i1cyskL7kTB84ccgwZI/hmi/i7IQAVRc94VgsK3QEV/tl0rQ2R9ebH3LcioG2aVWA2cTvVJV1qEmDRC40rEkGsuXZPhDrC08q30rrkBlj7zgGz5rEPzRqt9lDecycmsQ7Y2/3YHLQvb071j2jEY0nUz2ENL8Cr3H6Oi505tpFjdhCoYil+OrV7QCS3svc+shoh5z52pmMFiPs6obSQ3wxsldZx6DdpBJvxJfcsA26edw4U8B1d2NHA88V7sbctoV0q5KLoD4DfOFuIcUlc0l4j8U9zO3kz GeXRA+ip GXuDFRZ7jPOCbrA1Kt80RtPscq1/oenAIQpgSRHYypvju+bLS1uhRlYxlz4XyogYPOWujxXqIdLV5Fkgt/dN4l5v7/9rXTdbBQaXi0yayqhkdR1EGh14k9mPT78cwYEd5uxQ3rpY6r/rRHOqPh/lNtnuejtuT0FQBsD4TAcWXg1yN5e42aapt74FmlR8eKtwXkXkx04Sl4WhmR9SKCW+LOJKOa2vZw6G0MQ//a6bGFiD3qXM= 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 Thu, 20 Mar 2025 14:12:15 -0700 Greg KH wrote: > On Thu, Mar 20, 2025 at 05:41:15PM +0000, Jonathan Cameron wrote: > > --- a/drivers/cache/Kconfig > > +++ b/drivers/cache/Kconfig > > @@ -1,6 +1,12 @@ > > # SPDX-License-Identifier: GPL-2.0 > > menu "Cache Drivers" > > > > +config CACHE_COHERENCY_CLASS > > + bool "Cache coherency control class" > > Why can't this be a module? And why would anyone want to turn it off? If you don't have the hardware you won't want the infrastructure. I'll add a note. If you do have the hardware and don't have memory subsystems that need to use it (maybe no CXL hardware for instance and that's the only user on a particular platform). > > > + help > > + Class to which coherency control drivers register allowing core kernel > > + subsystems to issue invalidations and similar coherency operations. > > What "core kernel subsystems". I'll expand on that. Memory hotplug related stuff (currently CXL and NVDIMM) but the thing is more general that that. > > > + > > config AX45MP_L2_CACHE > > bool "Andes Technology AX45MP L2 Cache controller" > > depends on RISCV > > Shouldn't all of these now depend on CACHE_COHERENCY_CLASS? Nope. They are unrelated existing cache related drivers. The question to Conor is whether he minds me putting this in the existing directory and to others on whether it's a good idea. > > > diff --git a/drivers/cache/Makefile b/drivers/cache/Makefile > > index 55c5e851034d..b72b20f4248f 100644 > > --- a/drivers/cache/Makefile > > +++ b/drivers/cache/Makefile > > @@ -3,3 +3,5 @@ > > obj-$(CONFIG_AX45MP_L2_CACHE) += ax45mp_cache.o > > obj-$(CONFIG_SIFIVE_CCACHE) += sifive_ccache.o > > obj-$(CONFIG_STARFIVE_STARLINK_CACHE) += starfive_starlink_cache.o > > + > > +obj-$(CONFIG_CACHE_COHERENCY_CLASS) += coherency_core.o > > Why the blank line? To separate existing stuff that happens to be cache related from this new class. Kind of camping in a directory because seemed silly to have drivers/cache and drivers/cache_coherency > > > diff --git a/drivers/cache/coherency_core.c b/drivers/cache/coherency_core.c > > new file mode 100644 > > index 000000000000..52cb4ceae00c > > --- /dev/null > > +++ b/drivers/cache/coherency_core.c > > @@ -0,0 +1,130 @@ > > +// SPDX-License-Identifier: GPL-2.0-only > > +/* > > + * Class to manage OS controlled coherency agents within the system. > > + * Specifically to enable operations such as write back and invalidate. > > + * > > + * Copyright: Huawei 2025 > > + * Some elements based on fwctl class as an example of a modern > > + * lightweight class. > > + */ > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +#include > > + > > +static DEFINE_IDA(cache_coherency_ida); > > + > > +static void cache_coherency_device_release(struct device *device) > > +{ > > + struct cache_coherency_device *ccd = > > + container_of(device, struct cache_coherency_device, dev); > > + > > + ida_free(&cache_coherency_ida, ccd->id); > > +} > > + > > +static struct class cache_coherency_class = { > > + .name = "cache_coherency", > > + .dev_release = cache_coherency_device_release, > > +}; > > + > > +static int cache_inval_one(struct device *dev, void *data) > > +{ > > + struct cache_coherency_device *ccd = > > + container_of(dev, struct cache_coherency_device, dev); > > + > > + if (!ccd->ops) > > + return -EINVAL; > > + > > + return ccd->ops->wbinv(ccd, data); > > +} > > + > > +static int cache_inval_done_one(struct device *dev, void *data) > > +{ > > + struct cache_coherency_device *ccd = > > + container_of(dev, struct cache_coherency_device, dev); > > + if (!ccd->ops) > > + return -EINVAL; > > + > > + return ccd->ops->done(ccd); > > +} > > + > > +static int cache_invalidate_memregion(int res_desc, > > + phys_addr_t addr, size_t size) > > +{ > > + int ret; > > + struct cc_inval_params params = { > > + .addr = addr, > > + .size = size, > > + }; > > + > > + ret = class_for_each_device(&cache_coherency_class, NULL, ¶ms, > > + cache_inval_one); > > + if (ret) > > + return ret; > > + > > + return class_for_each_device(&cache_coherency_class, NULL, NULL, > > + cache_inval_done_one); > > +} > > + > > +static const struct system_cache_flush_method cache_flush_method = { > > + .invalidate_memregion = cache_invalidate_memregion, > > +}; > > + > > +struct cache_coherency_device * > > +_cache_coherency_alloc_device(struct device *parent, > > + const struct coherency_ops *ops, size_t size) > > +{ > > + > > + if (!ops || !ops->wbinv) > > + return NULL; > > + > > + struct cache_coherency_device *ccd __free(kfree) = kzalloc(size, GFP_KERNEL); > > + > > + if (!ccd) > > + return NULL; > > + > > + ccd->dev.class = &cache_coherency_class; > > + ccd->dev.parent = parent; > > + ccd->ops = ops; > > + ccd->id = ida_alloc(&cache_coherency_ida, GFP_KERNEL); > > + > > + if (dev_set_name(&ccd->dev, "cache_coherency%d", ccd->id)) > > + return NULL; > > + > > + device_initialize(&ccd->dev); > > + > > + return_ptr(ccd); > > +} > > +EXPORT_SYMBOL_NS_GPL(_cache_coherency_alloc_device, "CACHE_COHERENCY"); > > + > > +int cache_coherency_device_register(struct cache_coherency_device *ccd) > > +{ > > + return device_add(&ccd->dev); > > +} > > +EXPORT_SYMBOL_NS_GPL(cache_coherency_device_register, "CACHE_COHERENCY"); > > + > > +void cache_coherency_device_unregister(struct cache_coherency_device *ccd) > > +{ > > + device_del(&ccd->dev); > > +} > > +EXPORT_SYMBOL_NS_GPL(cache_coherency_device_unregister, "CACHE_COHERENCY"); > > + > > +static int __init cache_coherency_init(void) > > +{ > > + int ret; > > + > > + ret = class_register(&cache_coherency_class); > > + if (ret) > > + return ret; > > + > > + //TODO: generalize > > + arm64_set_sys_cache_flush_method(&cache_flush_method); > > I'm guessing this will blow up the build on non-x86 builds :) Yup. That's a TODO That needs fixing. > > > +struct cache_coherency_device { > > + struct device dev; > > + const struct coherency_ops *ops; > > + int id; > > +}; > > Classes are normally for user/kernel apis, what is this going to be used > for? I don't see any new user/kernel apis happening, so why do you need > a struct device to be created? I'm kind of expecting to grow some userspace ABI around a few things but indeed there isn't any yet. Stuff that has been suggested is: * Descriptive stuff useful for performance estimation. * Cache locking - as kind of the opposite of flushes. * Maybe more direct user interfaces to control it (I'm wary of that though). Mostly thought, the idea was avoid rolling my own similar registration infrastructure for this case. Absolutely can do a subsystem without a class if that seems to be the way to go. It'll be a little more complex though. Thanks, Jonathan > > thanks, > > greg k-h >