From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from frasgout.his.huawei.com (frasgout.his.huawei.com [185.176.79.56]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 9CE5B1F4176; Wed, 12 Nov 2025 16:11:03 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=185.176.79.56 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1762963866; cv=none; b=YUlCvf8pSYugyOL7bKMUfpXcKPvWs8bku4uhMKAxEKVJSg+AMOIOJH9h0DU3PC5oLhPMM+WyLNogkfTydAzYSWLyIMZoblULUhQi9wQFsFfTiErgIusM6Raea4YSMMgdLj5M9T4KVd6ezRbotQ7xVJhKL6/hvBWQy3ZECzRLSLM= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1762963866; c=relaxed/simple; bh=Hl7cj5szMspfIuzcNNGsidp6HmcFgFkc2lGAZtEzj5c=; h=Date:From:To:CC:Subject:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=C9MJsFKqC8GVmefgQ+87kT7o/eLdr+gV30ZAl+a2JwTeL3HrkY5RMnC1IDx2JBtFkwUXHTGAiKyR1vIDSIv7RhBOqjvRk8PgkQeT5iaG6DxVRyxa8LRaElS1ogA8yXdJjtxWqr5qGOX9fBOCCkI3Sxu6rgIGQAs0I20fl8RIvWM= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=huawei.com; spf=pass smtp.mailfrom=huawei.com; arc=none smtp.client-ip=185.176.79.56 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=huawei.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=huawei.com Received: from mail.maildlp.com (unknown [172.18.186.216]) by frasgout.his.huawei.com (SkyGuard) with ESMTPS id 4d67fX3f3DzJ46Cf; Thu, 13 Nov 2025 00:10:28 +0800 (CST) Received: from dubpeml100005.china.huawei.com (unknown [7.214.146.113]) by mail.maildlp.com (Postfix) with ESMTPS id 3CEC31402F5; Thu, 13 Nov 2025 00:11:01 +0800 (CST) Received: from localhost (10.203.177.15) by dubpeml100005.china.huawei.com (7.214.146.113) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.1544.36; Wed, 12 Nov 2025 16:11:00 +0000 Date: Wed, 12 Nov 2025 16:10:59 +0000 From: Jonathan Cameron To: CC: , , , , , , , , , Alejandro Lucero Subject: Re: [PATCH v20 11/22] cxl: Define a driver interface for HPA free space enumeration Message-ID: <20251112161059.000033bc@huawei.com> In-Reply-To: <20251110153657.2706192-12-alejandro.lucero-palau@amd.com> References: <20251110153657.2706192-1-alejandro.lucero-palau@amd.com> <20251110153657.2706192-12-alejandro.lucero-palau@amd.com> X-Mailer: Claws Mail 4.3.0 (GTK 3.24.42; x86_64-w64-mingw32) Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Content-Transfer-Encoding: 7bit X-ClientProxiedBy: lhrpeml500009.china.huawei.com (7.191.174.84) To dubpeml100005.china.huawei.com (7.214.146.113) On Mon, 10 Nov 2025 15:36:46 +0000 wrote: > From: Alejandro Lucero > > CXL region creation involves allocating capacity from Device Physical > Address (DPA) and assigning it to decode a given Host Physical Address > (HPA). Before determining how much DPA to allocate the amount of available > HPA must be determined. Also, not all HPA is created equal, some HPA > targets RAM, some targets PMEM, some is prepared for device-memory flows > like HDM-D and HDM-DB, and some is HDM-H (host-only). > > In order to support Type2 CXL devices, wrap all of those concerns into > an API that retrieves a root decoder (platform CXL window) that fits the > specified constraints and the capacity available for a new region. > > Add a complementary function for releasing the reference to such root > decoder. > > Based on https://lore.kernel.org/linux-cxl/168592159290.1948938.13522227102445462976.stgit@dwillia2-xfh.jf.intel.com/ > > Signed-off-by: Alejandro Lucero > Reviewed-by: Jonathan Cameron Took a fresh look and I think there are some algorithm optimizations available that also simplify the code by getting rid of next. > --- > drivers/cxl/core/region.c | 164 ++++++++++++++++++++++++++++++++++++++ > drivers/cxl/cxl.h | 3 + > include/cxl/cxl.h | 6 ++ > 3 files changed, 173 insertions(+) > > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c > index b06fee1978ba..99e47d261c9f 100644 > --- a/drivers/cxl/core/region.c > +++ b/drivers/cxl/core/region.c > +static int find_max_hpa(struct device *dev, void *data) > +{ > + struct cxlrd_max_context *ctx = data; ... > + for (prev = NULL; res; prev = res, res = res->sibling) { > + struct resource *next = res->sibling; > + resource_size_t free = 0; > + > + /* > + * Sanity check for preventing arithmetic problems below as a > + * resource with size 0 could imply using the end field below > + * when set to unsigned zero - 1 or all f in hex. > + */ > + if (prev && !resource_size(prev)) > + continue; > + > + if (!prev && res->start > cxlrd->res->start) { > + free = res->start - cxlrd->res->start; > + max = max(free, max); > + } > + if (prev && res->start > prev->end + 1) { > + free = res->start - prev->end + 1; > + max = max(free, max); > + } > + if (next && res->end + 1 < next->start) { > + free = next->start - res->end + 1; > + max = max(free, max); > + } Why doesn't the next case happen on the following loop iteration? I think this if (next ... ) is checking same thing as the if (prev ...) of the following iteration. Given the !next only happens on final iteration you should also be bale to do that out side the loop which might simplify thing further... > + if (!next && res->end + 1 < cxlrd->res->end + 1) { > + free = cxlrd->res->end + 1 - res->end + 1; > + max = max(free, max); > + } > + } outside the loop the final test is something like if (prev && prev->end + 1 < cxlrd->res->end + 1) { free = cxlrd->res->end + 1 - prev->end + 1; max = max(free, max); } as prev is now what was res above due to the final loop variable update. > + > + dev_dbg(cxlrd_dev(cxlrd), "found %pa bytes of free space\n", &max); > + if (max > ctx->max_hpa) { > + if (ctx->cxlrd) > + put_device(cxlrd_dev(ctx->cxlrd)); > + get_device(cxlrd_dev(cxlrd)); > + ctx->cxlrd = cxlrd; > + ctx->max_hpa = max; > + } > + return 0; > +}