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 F346FE7717F for ; Thu, 12 Dec 2024 22:42:22 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 46A8C6B00A5; Thu, 12 Dec 2024 17:42:22 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 3F2E06B00A6; Thu, 12 Dec 2024 17:42:22 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 26C2C6B00A7; Thu, 12 Dec 2024 17:42:22 -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 0458F6B00A5 for ; Thu, 12 Dec 2024 17:42:21 -0500 (EST) Received: from smtpin15.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 545891A026F for ; Thu, 12 Dec 2024 22:42:21 +0000 (UTC) X-FDA: 82887781494.15.AF9A5BD Received: from mail-qk1-f171.google.com (mail-qk1-f171.google.com [209.85.222.171]) by imf23.hostedemail.com (Postfix) with ESMTP id D63DC140006 for ; Thu, 12 Dec 2024 22:42:02 +0000 (UTC) Authentication-Results: imf23.hostedemail.com; dkim=pass header.d=gourry.net header.s=google header.b=li1BxNxg; dmarc=none; spf=pass (imf23.hostedemail.com: domain of gourry@gourry.net designates 209.85.222.171 as permitted sender) smtp.mailfrom=gourry@gourry.net ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1734043316; a=rsa-sha256; cv=none; b=6MRoiNTNn+uLzm12d9j8QhhSrB/t8YN9sw332IPSsWD8Avfu4MDNIKrksh5Ll+nubX7SY5 BzqygTEoUsu8roybl0Cp9X/hsanvTF3piE5UOI3Aw5txtAc6HiO6GJaS5Df8kBngF/c4rw OCr3AxJ6p2O8p5F4bBzmuCLeO3QhtC0= ARC-Authentication-Results: i=1; imf23.hostedemail.com; dkim=pass header.d=gourry.net header.s=google header.b=li1BxNxg; dmarc=none; spf=pass (imf23.hostedemail.com: domain of gourry@gourry.net designates 209.85.222.171 as permitted sender) smtp.mailfrom=gourry@gourry.net ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1734043316; 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=fDR/LFuljYmdIkvdxk0XRHGqL3q2O+pQlUnwlg06J80=; b=RmDXq5KNFB29X77l6YIh5SKVf4B8mQXj7KOIXat/YKQff8Hmf8BUlC0AAcrk/K5ARBs0yj zH+6LdLEqiS2OI115lEgL/yzRBUHewSqki/QTYzL+6Vl41RnzUsMiAlb3GS/MiNBPNhE0a 0LAxrb5Y4nA69iN40ReE1EB6dV5hVfQ= Received: by mail-qk1-f171.google.com with SMTP id af79cd13be357-7b6d63ab374so70955885a.3 for ; Thu, 12 Dec 2024 14:42:19 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gourry.net; s=google; t=1734043338; x=1734648138; darn=kvack.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=fDR/LFuljYmdIkvdxk0XRHGqL3q2O+pQlUnwlg06J80=; b=li1BxNxg7GleTG+DvacQv8QGT+CaLEzbonko2RH6GhMdJ1LAEyvwunpfNVoKtME11z xBHhGWkfMQ4hThP3zfMbTDU7zB6jmSCvGcXhQFvqR8u4JrIk0NDl7YfOBZviLgndFpJy IId8T03AkHruoG4qS/8pWXGv6ZxoeSm0XmehxCr4qXFE0qvMNErU6dYeva/+1ZJRXLru 7uYa+Ks6y3HgPZksm9Qw69+gz8zyYUExTKHuhJ7RYrZmlLYZKGVKGDkTIG/S5Wc5b5wU LWPPeLSeQiBSnrTqJWNP6BkitEnWHJ6uwBMihxTQklY8zIHpb5yfg1L47Enx9XvdWRx9 gkDg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1734043338; x=1734648138; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=fDR/LFuljYmdIkvdxk0XRHGqL3q2O+pQlUnwlg06J80=; b=q+MhDCF47soLHucZwVuk6Np8817in5UBZFPYwooNhuTI1UmdYJm3H9OAPnpUZAdS65 4ijAHEbbxlM2DM9gy0ppg7+WMM9iwnPnOq7bN/Hd2iFEDO1ij3Qh7k93JiSHbLQrJ9wZ j6+p4HZweYEKEQfR0vOgD8Ylx5Wx8zgdgtdjcm+nNN69MEqR0h26MsgkOpS6eSQcPCS3 dZ+Hp0qMQufoPg3FyQYLMkWiJQenPrx0HiIKgdCyQvWr8gPvQdSMsRU6kQk75eWYTLVl jLW4QgEmGRpKbDY92lad19NlBMcqrSSS9oNBUSF8C7gcknJ7hqGFwgMgDGXmqjfTZew/ o5Ag== X-Forwarded-Encrypted: i=1; AJvYcCXxG1DDGRRu3zTmRGCoFNoyxCX9ShyM9jcP+x+JXlhptrrXMOrZt69hkEILt/eLJvQJXFY9/2h8sw==@kvack.org X-Gm-Message-State: AOJu0YyqzsCVDVchL7h0CSNhhY67DYqoYfQBStt20TNM6Q8GId5FIXYX Nvd4DDNKV/ADLIIP8ZZYIu8BGbLQIB1HSmlif09FWnLPHv6yLmw78dkWP+NwAIc= X-Gm-Gg: ASbGncsn71NrlCXXQ0sFMWdcB7wg/YedJk6twXETAtnNaWJl/2DacNvVFXksBklWmHX nur74OX4fQSiTaHpJipBcq7s0b1SAUW95AlIvAqjDjOGo+JN7zkgAmTVkcfLCm4ZLkHNqY46ZtK bTd+N1Tii1uQH0KwyICRwT5md5Xr464lHT7uaVO+aWXHfbiki97KsevdcVMtDOk2iWWrxITmcHz eeGcrkwNOzGK2fOarKXVPLR9iLVKsmu0E1nSm8d7QZrcWG7ZL6Cxur7qehSoufybZSzcBFxfvnG WxquOXhpJo8qoK6fmmJPibth+B4HoP/x90X4SAUrIg== X-Google-Smtp-Source: AGHT+IGsxcm3PfM5EAymYDCEpO8WpGhXaFdTJXIlPkZ/khgvSIYnAHVOH8iTHDVckjG1aaG2MTOf1Q== X-Received: by 2002:a05:6214:f07:b0:6d4:2131:563c with SMTP id 6a1803df08f44-6dc8ca8999emr4699046d6.27.1734043338364; Thu, 12 Dec 2024 14:42:18 -0800 (PST) Received: from PC2K9PVX.TheFacebook.com (pool-173-79-56-208.washdc.fios.verizon.net. [173.79.56.208]) by smtp.gmail.com with ESMTPSA id af79cd13be357-7b6e6faf428sm268774085a.119.2024.12.12.14.42.17 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 12 Dec 2024 14:42:17 -0800 (PST) Date: Thu, 12 Dec 2024 17:42:08 -0500 From: Gregory Price 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> X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: D63DC140006 X-Stat-Signature: hy1t8by7118o9cjpb9kpafag4qg3jbf8 X-Rspam-User: X-HE-Tag: 1734043322-911295 X-HE-Meta: U2FsdGVkX1+Yrtz87TZX9WkZWs1IgElgMkZ4kBvdndQ/59V95XydPVzipp4lsdJuUPRsiW5pHPluPCMVclZHcR3n8zBiLaugC0RnD/eAx9tKFBCEEL9kE49WPbulvg4sc5W0rrvA6/XuIu68mqUY9sSHe8AS+oXa9rthC/InESBg2AGCjqYRIeYxlVmaeytx9AHEctkbDwCkIG8CsV4z9WSL/e+37UPskqGg3oMMolL32sX0Nj54N9iuJO2PtDsXTifkRzZ1+j4hJGN6CJ9nC5DuQjCdBa6YqPoOVZVE0RoU2a3nmfzC6FAW2Dfhq259KNP0+7mDMLVpj3GunT6RWl1VgRU6wyv/JY3QxHI1eKwD7hMs0DqekfYRm5eHYM3PdTbCK9K4jH4j8wNJ4AOOEyKHGKRm5rWKl8OqkDwGQtqW/30+rxv/GeiKstW2ahkinURuW/DNlJcZbopXiVJnjgJJOc7NC1hKdLdxJ6lyBuAN+iJwQSr9KL+Y/X/s4HJ3tOWrgdXUblJ9+M4IHnUMBrPPss0fEin46n3FmE+C9HPRkzIBPh2dTT1R6USBmz5FnCGREeZTXz7MhH2E5au74YARX96IAj7YHOVOFt+0TjSJ9GnsG6GjYIyquzSMyldDNKCMnb+oRO1HCUuYomTh2MkMeQY4vkQODr4Np/0GSrkA5oGx8OkaoQCBqVo+wGg6tz8gu5PHJdOae+Usa3vKT2uwgvYCfqhmyT6+sInMtVDJkLRrO23ZpZIsFOvDw9rOL6xx8PDS/I1+b6JIkfG+yroy+6SoCbSAHjeK9v8H+fKa8b6EsXI+N5jpUychSya9mRv2bcXcmWruuBJVEqRwUk0WECQnEu1KCrgdws6GYKf1/hf2Z4dBIwUnsv8+dXnRu6565MLWT/mUB33eBIl1E9ZGlNBXA1D2dkVxw2uMFAFAd5wShS44gsrke+UUXZmFZjEFuda0XbMOjZcfr1C 9ao8Clbe UbAJxiVQ4gBvupnBh9QVuwYZWvvgM5t+ancRC4VyQk9lEUiCjp4qk/0dpI/MdT0bUIf0zFvTkoVyu/I08MGOaFVZZEJVE++SOxhU8T7Xx9eC8c4iW+tF5bSbaMI/Mx+MfITVPA6cDFY4n5aJHMBNd6kmJHKY39PXp4xtO6pdLXOMpwHN29EGdMHwAzMUpV982IpooAmLzNt+cuMbCex6fv+INdfOEc6xGV+EvLExqMU3lHXG7qRlX2RJYn94KhjeztHq+NxWI9749yKNYKzMIrNoMAPvpaTroUs4U X-Bogosity: Unsure, tests=bogofilter, spamicity=0.462264, 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: ... snip ... > diff --git a/kernel/resource.c b/kernel/resource.c > index a83040fde236..8fc4121a1887 100644 > --- a/kernel/resource.c > +++ b/kernel/resource.c ... snip ... > +static int srmem_parse_cfmws(union acpi_subtable_headers *hdr, > + void *arg, const unsigned long unused) > +{ Chiming in a little late to the party here I don't think encoding CXL-specific terminology and functionality directly into kernel/resource.c is wise by any measure. The abstraction here is completely inverted, and this is probably in line with Dan's comments. The comments in e820.c allude to a similar issue * Prior to inserting SOFT_RESERVED resources we want to check * for an intersection with potential CXL resources. This is similarly inverted - e820 doesn't know anthing about CXL and it shouldn't have to be made aware of CXL. Mucking with e820 is *begging* to be bitten elsewhere in parts of the system that depend on it to be a relatively stable source of truth. This tells me that this patch is trying to solve the wrong problem. Your changelog alludes to supporting hotplug replace """ The current approach of leaving the SOFT RESERVE resource as is can cause failure during hotplug replace of CXL devices because the resource is not available for reuse after teardown of the CXL device. """ It sounds like we should be making the SR resource available for re-use through proper teardown and cleanup of the resource tree, rather than trying to change fundamental components like e820. If the driver was capable of using the SOFT_RESERVED region on initial setup, it should be capable of re-using that region. Is the issue here that the hotplug-replaced component has a different capacity? It it being assigned a new region entirely? Is it exactly the same, but the resource isn't being cleaned up? Can you provide more specifics about the exact hotplug interaction that is happening? That might help understand the issue a bit better. Much of this sounds like we need additional better tear-down management and possibly additional cxl/acpi features to handle hotplug of these devices - rather than changing resource.c. ~Gregory > + struct acpi_cedt_cfmws *cfmws; > + struct srmem_arg *args = arg; > + struct resource cfmws_res; > + struct resource *res; > + > + res = args->res; > + > + cfmws = (struct acpi_cedt_cfmws *)hdr; > + cfmws_res = DEFINE_RES_MEM(cfmws->base_hpa, > + cfmws->base_hpa + cfmws->window_size); > + > + if (resource_overlaps(&cfmws_res, res)) { > + args->overlaps += 1; > + return 1; > + } > + > + return 0; > +} > + > +static bool resource_overlaps_cfmws(struct resource *res) > +{ > + struct srmem_arg arg = { > + .res = res, > + .overlaps = 0 > + }; > + > + acpi_table_parse_cedt(ACPI_CEDT_TYPE_CFMWS, srmem_parse_cfmws, &arg); > + > + if (arg.overlaps) > + return true; > + > + return false; > +} > + > +int insert_soft_reserve_resource(struct resource *res) > +{ > + if (resource_overlaps_cfmws(res)) { > + pr_info("Reserving Soft Reserve %pr\n", res); > + return insert_resource(&srmem_resource, res); > + } > + > + return insert_resource(&iomem_resource, res); > +} > +EXPORT_SYMBOL(insert_soft_reserve_resource); > + > static void __init > __reserve_region_with_split(struct resource *root, resource_size_t start, > resource_size_t end, const char *name) > -- > 2.43.0 >