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 6C77DE7717D for ; Wed, 11 Dec 2024 11:32:08 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id A77508D0015; Wed, 11 Dec 2024 06:32:07 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id A26A38D0013; Wed, 11 Dec 2024 06:32:07 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 8C7878D0015; Wed, 11 Dec 2024 06:32:07 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 6A7298D0013 for ; Wed, 11 Dec 2024 06:32:07 -0500 (EST) Received: from smtpin26.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 0CE1980A1F for ; Wed, 11 Dec 2024 11:32:07 +0000 (UTC) X-FDA: 82882463580.26.DA6819D Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.16]) by imf13.hostedemail.com (Postfix) with ESMTP id 2410C20008 for ; Wed, 11 Dec 2024 11:31:40 +0000 (UTC) Authentication-Results: imf13.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=KA58u8R7; spf=pass (imf13.hostedemail.com: domain of andriy.shevchenko@intel.com designates 198.175.65.16 as permitted sender) smtp.mailfrom=andriy.shevchenko@intel.com; dmarc=pass (policy=none) header.from=intel.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1733916707; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=cMcWrz2YnyeF6c0ykcViqiJX2vIo+crqO923zieTHrs=; b=yWxtzYIevRlDR9XkDb/YUtSRt5hqvcWwcPDr4//CemVfZ743BwDzgwclCrsCIUUowRBaRz uc24h1zqedw29qctM9XrchINThn57ccPlHermtg4AwQOI/An451beCLmGWXw0BKlywwR+i A2v0TvfM9fpseoAFsGP/ZXJuoUDdABQ= ARC-Authentication-Results: i=1; imf13.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=KA58u8R7; spf=pass (imf13.hostedemail.com: domain of andriy.shevchenko@intel.com designates 198.175.65.16 as permitted sender) smtp.mailfrom=andriy.shevchenko@intel.com; dmarc=pass (policy=none) header.from=intel.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1733916707; a=rsa-sha256; cv=none; b=8ldAFexL1O4RKfigrfcPxBkxmG7Z0JrwIOs0Kk6jK0q+YPpwUzQbzuuVlctUBbQ7C4Amo6 3hJvB5W9I0c66eWqujiKfuufofSMdTIerMhRHP7JBDLToEntpShAMNoeIL2JRMrl6I3c5D BEhyhw/TCFmLuSPQWnx41wA1jb7ySeo= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1733916724; x=1765452724; h=date:from:to:cc:subject:message-id:references: mime-version:in-reply-to; bh=AKTqxmpi4mJNDPKknySvlHdWar61q7LraKA5yizjjz4=; b=KA58u8R7FkZanLLrw8ZNoFfUQ95nnz4Q4Gp1JGOHhkQyfcqS/0VbwypO kx2QvdBsraO9pTl26kRMxSJVrSQSd9OeYJr86VmeI06Da+Orwdkvyoegs 5W214JVnOGpXpHUN+xStVXtx9MqOicKcND9sVfAdo9IqhqlMLmsPqNixW zf9HD0SyyymfafyI8Uq3f4jaWUjPuyj95mbr+7aoKIxQfljhWPA+T3Hel 9SX38ot6V8/me24t/+9LcMhiVxuTuuLn94Uuah5Zz5YKuvMmWtRtbtIea 6Keieiue0s/uZrmOufiGuM/aw8BXlcGBV2ffxUGxUAyDLtoGTDQ1nNGQo Q==; X-CSE-ConnectionGUID: 0SmZWcZDRCSmnHTgOnH4UQ== X-CSE-MsgGUID: roIK+WiEStq5eJmuZdVk5w== X-IronPort-AV: E=McAfee;i="6700,10204,11282"; a="34431596" X-IronPort-AV: E=Sophos;i="6.12,225,1728975600"; d="scan'208";a="34431596" Received: from fmviesa009.fm.intel.com ([10.60.135.149]) by orvoesa108.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 11 Dec 2024 03:32:02 -0800 X-CSE-ConnectionGUID: X+fvPiiPQqKq4DAY4ZxPMg== X-CSE-MsgGUID: zfPiE9JwR7u0HlE9L2+aSA== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.12,225,1728975600"; d="scan'208";a="96241618" Received: from smile.fi.intel.com ([10.237.72.154]) by fmviesa009.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 11 Dec 2024 03:32:01 -0800 Received: from andy by smile.fi.intel.com with local (Exim 4.98) (envelope-from ) id 1tLKwf-00000006Vco-2NNL; Wed, 11 Dec 2024 13:31:57 +0200 Date: Wed, 11 Dec 2024 13:31:57 +0200 From: Andy Shevchenko To: Nathan Fontenot Cc: alison.schofield@intel.com, dan.j.williams@intel.com, linux-cxl@vger.kernel.org, linux-mm@kvack.org Subject: Re: [PATCH] cxl: Update Soft Reserved resources upon region creation Message-ID: References: <20241202155542.22111-1-nathan.fontenot@amd.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20241202155542.22111-1-nathan.fontenot@amd.com> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo X-Rspamd-Queue-Id: 2410C20008 X-Rspamd-Server: rspam12 X-Stat-Signature: 8bgj16um5i75gg4ch19h44z1xreyaye4 X-Rspam-User: X-HE-Tag: 1733916700-858157 X-HE-Meta: U2FsdGVkX18obmsJeSBAa+neSws99IgSB/jdP0ElACMG/ShC0h9x+SFWiFrzr7LbZjYtkQp/ZRkWwHh+RtFiWTZAkFHtSjt/LesOSXMJ98C/RCe+4kDTEQppqIVFcI4ZE7fEVsH/UDEq0afAMeOWLrRl5rilB4TSQ5Qbvw+0Rh21mx6eTIukFlc+09ucXsm8z91xE/dmRzy9nk3TZIWW0dizI6EEE1P1yFmfcFNz9Vl15Djtyu2D9qSAE5CZuBSW8+SXVV8b1O3x8NG8Gayq697qyImfvkc9lSt8ih79rQeuqddza4pVTXnbX+nO+WjDcGP4hK1AI5J8gyqCyN2X514y8qm0lpWGgbuE8P1tQ45mYs45GxOYh/Bm7fWNJ3s0EtQ1sD4nUXrxGXt95keULPlLxYAtmYqKInkdgxrt3UiGvJQzmtqHOJRbsYyGEm+p0fLgWGFf8Dv3JezFjujklGuT/BeQxAd+cykrxmg5o2qglEs7WAHfVZQ7bz2EYiOcJmHIABhXrScda0OqGFWV1tuqY/q7BdjZcB5GUeXGe/dx7vOp0+1HIa8F/KySkEi6vKvMbkozcArsYZABUOnKjZSIYTJtBIKgGhuZUx9DUm2RKKMl3VwfEE5mBnpnMKAx/xqfh/g/M7wkSKmYxIDB7FZDajauEL/ZG62aDqBYTPZ2QPF02kZbGjVcO4hoFQKAi8dZO139CtKOiP8S6ENBszTn0McMWE5QWuQ5lny1YZ5Cl2NhZXkG2zGXGGC/m23D/uBQhH1lEXT7xs4wql5RqE5omh6XZr4dJg9KYEUIFJvfnEkZNpeUjmA+anEkBHBfYAuB46OgXAOGboOQ3zI96SKWOIxjUcVZXw5tFbOuvtM27Tk0GgTHXPwovMdVti9yOF+SAMMLmgtVZLMRN01SGb4bKm7V1KzmINGrPgquPB9Ox4N9t9Y/8pxSrDY1ZZzXmPIdBy0Ws6+HRCNqKar bTRnM58z ggEu4lpJ9PU/mi0j8fKvANaR11W8jxH+8XkG2kbfnFiW6eZOUVPFidA9CMu2mOIg7jV8SysyAKfZPDZjy6+mb/6FL4eMLiEQcqCc3vlILj1Og0xCRZjILj5ZdZzOwFLmlS4Wz2V+Xtb5/q82Qy2YBbv7eAv1Ntk64j4Y8d5Ui7UI1N7owzAhEonC1kfkG8szlct/edIFoTnxns2cxV/aQ8w9D8vJUPexcyN5MdqjJDoXwwH0= 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 Mon, Dec 02, 2024 at 09:55:42AM -0600, Nathan Fontenot wrote: > Update handling of SOFT RESERVE iomem resources that intersect with > CXL region resources to remove the intersections from the SOFT RESERVE > resources. The current approach of leaving the SOFT RESERVE > resource as is can cause failures during hotplug replace of CXL > devices because the resource is not available for reuse after > teardown of the CXL device. > > The approach is to trim out any pieces of SOFT RESERVE resources > that intersect CXL regions. To do this, first set aside any SOFT RESERVE > resources that intersect with a CFMWS into a separate resource tree > during e820__reserve_resources_late() that would have been otherwise > added to the iomem resource tree. > > As CXL regions are created the cxl resource created for the new > region is used to trim intersections from the SOFT RESERVE > resources that were previously set aside. > > Once CXL device probe has completed ant remaining SOFT RESERVE resources > remaining are added to the iomem resource tree. As each resource > is added to the oiomem resource tree a new notifier chain is invoked > to notify the dax driver of newly added SOFT RESERVE resources so that > the dax driver can consume them. ... > void __init e820__reserve_resources_late(void) > { > - int i; > struct resource *res; > + int i; Unrelated change. ... > - for (i = 0; i < e820_table->nr_entries; i++) { > + for (i = 0; i < e820_table->nr_entries; i++, res++) { > - res++; Unrelated change. > } ... > +static struct notifier_block hmem_nb = { > + .notifier_call = dax_hmem_cb It's better to leave trailing comma as it reduces churn in the future is anything to add here. > +}; ... > +++ b/drivers/dax/hmem/hmem.h > @@ -0,0 +1,11 @@ > +// SPDX-License-Identifier: GPL-2.0 > +#ifndef _HMEM_H > +#define _HMEM_H This needs a few forward declarations struct device; struct platform_device; struct resource; > +typedef int (*walk_hmem_fn)(struct device *dev, int target_nid, > + const struct resource *res); > +int walk_hmem_resources(struct device *dev, walk_hmem_fn fn); > + > +extern struct platform_device *hmem_pdev; > + > +#endif ... > #include > #include > #include > +#include I would put it before types.h to have more ordered piece. ... > +extern void trim_soft_reserve_resources(const struct resource *res); > +extern void merge_soft_reserve_resources(void); > +extern int insert_soft_reserve_resource(struct resource *res); > +extern int register_soft_reserve_notifier(struct notifier_block *nb); > +extern int unregister_soft_reserve_notifier(struct notifier_block *nb); Why extern? ... > +++ b/kernel/resource.c > @@ -30,7 +30,7 @@ > #include > #include > #include > - > +#include We don't usually interleave linux and asm headers, moreover the list seems to be sorted (ordered, please preserve the ordering). ... > +struct resource srmem_resource = { > + .name = "Soft Reserved mem", > + .start = 0, > + .end = -1, > + .flags = IORESOURCE_MEM, This can use DEFINE_RES_MEM_NAMED() as well. > +}; ... > + if (sr_res->start == res->start && sr_res->end == res->end) { Wondering if we have a helper to exact match the resource by range... > + release_resource(sr_res); > + free_resource(sr_res); > + } else if (sr_res->start == res->start) { > + WARN_ON(adjust_resource(sr_res, res->end + 1, > + sr_res->end - res->end)); > + } else if (sr_res->end == res->end) { > + WARN_ON(adjust_resource(sr_res, sr_res->start, > + res->start - sr_res->start)); > + } else { > + /* > + * Adjust existing resource to cover the resource > + * range prior to the range to be trimmed. > + */ > + adjust_resource(sr_res, sr_res->start, > + res->start - sr_res->start); > + > + /* > + * Add new resource to cover the resource range for > + * the range after the range to be trimmed. > + */ > + new_res = alloc_resource(GFP_KERNEL); > + if (!new_res) > + return; > + > + *new_res = DEFINE_RES_NAMED(res->end + 1, sr_res->end - res->end, > + "Soft Reserved", sr_res->flags); > + new_res->desc = IORES_DESC_SOFT_RESERVED; > + insert_resource(&srmem_resource, new_res); > + } ... > +void trim_soft_reserve_resources(const struct resource *res) > +{ > + struct resource *sr_res; > + > + write_lock(&srmem_resource_lock); > + for (sr_res = srmem_resource.child; sr_res; sr_res = sr_res->sibling) { Can this utilise for_each_resource*()? Ditto for the rest of open coded for each type of loops. > + if (resource_contains(sr_res, res)) { > + trim_soft_reserve(sr_res, res); > + break; > + } > + } > + write_unlock(&srmem_resource_lock); > +} ... > + cfmws_res = DEFINE_RES_MEM(cfmws->base_hpa, > + cfmws->base_hpa + cfmws->window_size); Can be one line. But is this correct? The parameters are start,size, and here it seems like start,end. ... > +static bool resource_overlaps_cfmws(struct resource *res) > +{ > + struct srmem_arg arg = { > + .res = res, > + .overlaps = 0 Keep trailing comma. > + }; > + > + acpi_table_parse_cedt(ACPI_CEDT_TYPE_CFMWS, srmem_parse_cfmws, &arg); > + if (arg.overlaps) > + return true; > + > + return false; return arg.overlaps; > +} ... > +int insert_soft_reserve_resource(struct resource *res) > +{ > + if (resource_overlaps_cfmws(res)) { > + pr_info("Reserving Soft Reserve %pr\n", res); Btw, do we have pr_fmt() defined in this file? > + return insert_resource(&srmem_resource, res); > + } > + > + return insert_resource(&iomem_resource, res); > +} -- With Best Regards, Andy Shevchenko