[v3,4/9] cxl/mem: Skip intermediate port enumeration of restricted endpoints (RCDs)
Message ID | 20221109104059.766720-5-rrichter@amd.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a5d:6687:0:0:0:0:0 with SMTP id l7csp261307wru; Wed, 9 Nov 2022 02:42:18 -0800 (PST) X-Google-Smtp-Source: AMsMyM7LPXDlkAwKGEIxF6iR52ZU8Dcp76J5oTwJkKshLfad3fZVQpNNm5j+ubH+V6dUO/6xcnis X-Received: by 2002:a17:902:c702:b0:186:e151:6b27 with SMTP id p2-20020a170902c70200b00186e1516b27mr61618768plp.69.1667990537532; Wed, 09 Nov 2022 02:42:17 -0800 (PST) ARC-Seal: i=2; a=rsa-sha256; t=1667990537; cv=pass; d=google.com; s=arc-20160816; b=AOHpMFBti4v/bxqCKic3uoME4kiSIG4P24YGCW1tquOh5mUYdaWjWTAjxtotAnrB9B I/6WuZn/opYR9dXX81T0eod8mQdQ55dgx2XkNtzOXx/8yJOTQpa1CvL22ANZ+3kALRkJ odNkNIFtDjXzsp1rtakrLzBeVTSxukn4vWlqwkajQuoikKVW6ZYqg9O7EsX4lH/jO8eE ni2FnSydhcHyh77lDmJ/I59F3NnkYbI7YjMMBn7Evkl/+7dNGUJxqprW6arEP9Ka8Bvw GnLVOE+Oo78B1oeeRqMR7EobOpdChMP771any1o8Z/RdGeIbqIRSwPYcCNnwOlFsaoER 0xvA== ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=73NwBsh+4VgARIZnefxl1VUD3O8ZaMkAlO+5ST6uapw=; b=ASl1xz8FKEVgG9HR8MuA7SUDU3/2rTJwl/IGH9ygcbNZe2Kt1jqkWpdA+Qekfp/T8a qOAgufeAgqoz7w7Ao7wVTHZMMso+h/TM7YMWZyt+5aB87n3pv+L4+J7okExIVZBxgOg5 vHVzpTjYKN/5GheGojcZs6n8jwPTzqdklGplOkgyWF1r7smVABdkMXQkSb0GiwcGcP39 Pt0rGMSBvRQyBluMoDtS3sediQ6J6mbUR+8/1f1VLk/R37zZ7lXMFTA8irN/C6sqHevx A4e7i6KX+E6Fx/sBFF9fRQq7tMSK8oqm+XHDoEW6OBqMH7kPp6Kh/zF6rkO6AVuzOudT 8NWA== ARC-Authentication-Results: i=2; mx.google.com; dkim=pass header.i=@amd.com header.s=selector1 header.b=pk4QiuI2; arc=pass (i=1 spf=pass spfdomain=amd.com dmarc=pass fromdomain=amd.com); spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=amd.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id e12-20020a170902ed8c00b001886cce9ab1si15167631plj.247.2022.11.09.02.42.04; Wed, 09 Nov 2022 02:42:17 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@amd.com header.s=selector1 header.b=pk4QiuI2; arc=pass (i=1 spf=pass spfdomain=amd.com dmarc=pass fromdomain=amd.com); spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=amd.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231166AbiKIKlr (ORCPT <rfc822;dexuan.linux@gmail.com> + 99 others); Wed, 9 Nov 2022 05:41:47 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55700 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231132AbiKIKlh (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Wed, 9 Nov 2022 05:41:37 -0500 Received: from NAM11-CO1-obe.outbound.protection.outlook.com (mail-co1nam11on2086.outbound.protection.outlook.com [40.107.220.86]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id EE40822BD8; Wed, 9 Nov 2022 02:41:34 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=TJTEcmIM8Clu+YV4zjzekBpYqoWF0kUvAB7GTRuE8YShl5ZKDeOrEpmZE2p5sRO/5AsVFeX5YxnX17Oa0+rSNqYNfZXW5iZV2J5HdSgCzslRNtEt4cHnQTHqwwhWqcDTEvTKcuDEfbpsvJRmlhTRBk8/x9Fn56ERPAio6vV5Bz+kKCFs9bBCKtCGVe5uYOZTLBVkNX7DvqVridrDhlsmHtf1tgbPWIGux8NHOIPUUXJJNSTyBisojlEh6z+mfAETXM7VRcY+dfbOERZLFdGpumYaTbyfcMGQ6HqoqQq9wbIysXfVsFQ2MMMqEq9jyo+3Fner2ZBqpTrPEpffqJyiKg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=73NwBsh+4VgARIZnefxl1VUD3O8ZaMkAlO+5ST6uapw=; b=UQ58EmE7yXELtA5l31/qGJOeEy7qSXpAR50onXICE/lyAa5Lfw5h8gDlEIb+BfQV1xtNROSLqqL5yyynT2DOYanGF82m5hbaX5DElx+SzcECAs+rK4AHCFnr9cIOvgLRvRBKwqAAP9+QhW8Zw4671IExwbDHngAnjwgdO/zBIwhwajQEcI1Fithpzgknnkc7V/HnUG26PoY8xt62T3vyepPP2jjeGrFO1+JmhNyxaRQo842YIizkB5EZ5REbG2KrIeb6A9Q5y32ZKQLlqePMVhn7ekgNDb6iDKZ28YYD1hooWAsDTJ3iGzfDTuR/5q8oP1O/RS5pP/AnmAwh9eAMKQ== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass (sender ip is 165.204.84.17) smtp.rcpttodomain=intel.com smtp.mailfrom=amd.com; dmarc=pass (p=quarantine sp=quarantine pct=100) action=none header.from=amd.com; dkim=none (message not signed); arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amd.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=73NwBsh+4VgARIZnefxl1VUD3O8ZaMkAlO+5ST6uapw=; b=pk4QiuI2MfaNBSCzScbbEN+AlWU3klOcUqE7abOndZMB+M334xDdYQc+MzQEReM7vQ54ZPjpT71aGkp35xY0wfFBXVQISvrKJK9xtcQjniF5ON5yw1CUDtJkcUYlc5elSlFFmalQkNUjSxhkb7hK+Hh8lPubzSlPUwIwpW2ajOQ= Received: from BN0PR04CA0079.namprd04.prod.outlook.com (2603:10b6:408:ea::24) by DM6PR12MB4217.namprd12.prod.outlook.com (2603:10b6:5:219::24) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5813.12; Wed, 9 Nov 2022 10:41:32 +0000 Received: from BN8NAM11FT031.eop-nam11.prod.protection.outlook.com (2603:10b6:408:ea:cafe::c8) by BN0PR04CA0079.outlook.office365.com (2603:10b6:408:ea::24) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5791.27 via Frontend Transport; Wed, 9 Nov 2022 10:41:32 +0000 X-MS-Exchange-Authentication-Results: spf=pass (sender IP is 165.204.84.17) smtp.mailfrom=amd.com; dkim=none (message not signed) header.d=none;dmarc=pass action=none header.from=amd.com; Received-SPF: Pass (protection.outlook.com: domain of amd.com designates 165.204.84.17 as permitted sender) receiver=protection.outlook.com; client-ip=165.204.84.17; helo=SATLEXMB04.amd.com; pr=C Received: from SATLEXMB04.amd.com (165.204.84.17) by BN8NAM11FT031.mail.protection.outlook.com (10.13.177.25) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.20.5813.12 via Frontend Transport; Wed, 9 Nov 2022 10:41:32 +0000 Received: from rric.localdomain (10.180.168.240) by SATLEXMB04.amd.com (10.181.40.145) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.31; Wed, 9 Nov 2022 04:41:29 -0600 From: Robert Richter <rrichter@amd.com> To: Alison Schofield <alison.schofield@intel.com>, Vishal Verma <vishal.l.verma@intel.com>, Ira Weiny <ira.weiny@intel.com>, Ben Widawsky <bwidawsk@kernel.org>, Dan Williams <dan.j.williams@intel.com> CC: <linux-cxl@vger.kernel.org>, <linux-kernel@vger.kernel.org>, Bjorn Helgaas <bhelgaas@google.com>, "Rafael J. Wysocki" <rafael@kernel.org>, Len Brown <lenb@kernel.org>, Jonathan Cameron <Jonathan.Cameron@huawei.com>, "Davidlohr Bueso" <dave@stgolabs.net>, Dave Jiang <dave.jiang@intel.com>, Robert Richter <rrichter@amd.com> Subject: [PATCH v3 4/9] cxl/mem: Skip intermediate port enumeration of restricted endpoints (RCDs) Date: Wed, 9 Nov 2022 11:40:54 +0100 Message-ID: <20221109104059.766720-5-rrichter@amd.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20221109104059.766720-1-rrichter@amd.com> References: <20221109104059.766720-1-rrichter@amd.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain X-Originating-IP: [10.180.168.240] X-ClientProxiedBy: SATLEXMB04.amd.com (10.181.40.145) To SATLEXMB04.amd.com (10.181.40.145) X-EOPAttributedMessage: 0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: BN8NAM11FT031:EE_|DM6PR12MB4217:EE_ X-MS-Office365-Filtering-Correlation-Id: f02be110-c5fe-4432-c41d-08dac23ef78e X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: FfGGGblVlpaC5xC8B9kIIV8/QMzZl+3QX7kfyHCVzq2jfPK/1Box8j9P5o8fWKVp2j8AOjHt6Hncug/CvrStzfO8LbX2jk4Y26/neDhFO+AEkfqMioWAL5XGBIPU059vh/Mwfr0bKRmcD1sox/VNXv8AJfFxet8DJbXfhGKJSvDmML1NIC7iw25QLbnmuKdAYz6I2nHuCa2g/oSdeqBmz8K/Xt7n/5aAemorkz/tt6MFoC5AFfr64EvzM+CDUUJLkb3Zbk/b/CBOVBwKhoneMZKhoPNgZf4Wz49xFGs1tXgJ5zODV/dC2Ajx+rhC5c9aX8Jk7o6oV6yzjYIbXmsYzhTmcrn2BI1X1BtCuJN1xnW4G7LweQbCc53GvhQdEX9lX+UNlm0uiraQAgwPSkrcnqjPPm2nxwcmEC7CRFnU62y6lNH1LSUKBHudgz3g+L6HLMWEbbovciq6PGPVRefKHjNTcrpIJ3xSv0SfBaWJ2OFdwDhO7UhlRMmmP9Wq8WWIluRAv1LZk+ldDUjguMGpwTQSTRhSVcWxYDJWvOFWBGkvdCG87AG4jSFjUxawIKDsOAN/aHkiocwb5+Z6uV8dP/lbE/qCyT8cQfCPVPXPl/u6mKb0fgdghpVNXgpJgNTdq6BDza+kJNC0MLOaYC25Kkl4VTZ5lLx6VkZwgbTqhlQrMoG2PQmAEq1yHfL+AQ9YgHoo7tNfNk3Vnex2BfOUy9OcqTi8sT8iVQCEv3xhqCORpUY/ARupMPo1VH+12c4WklNv2yh1vXN0RKsKRQIdaaABgmMJW3EgMIGr6+2fKiS5D+SyEQeJ/vLnAFkhTs9U X-Forefront-Antispam-Report: CIP:165.204.84.17;CTRY:US;LANG:en;SCL:1;SRV:;IPV:CAL;SFV:NSPM;H:SATLEXMB04.amd.com;PTR:InfoDomainNonexistent;CAT:NONE;SFS:(13230022)(4636009)(346002)(136003)(376002)(39860400002)(396003)(451199015)(46966006)(40470700004)(36840700001)(82310400005)(478600001)(356005)(6666004)(81166007)(54906003)(110136005)(316002)(26005)(426003)(47076005)(40480700001)(70206006)(70586007)(4326008)(82740400003)(8676002)(7416002)(41300700001)(5660300002)(8936002)(36756003)(2616005)(186003)(1076003)(16526019)(40460700003)(83380400001)(2906002)(36860700001)(336012)(36900700001);DIR:OUT;SFP:1101; X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 09 Nov 2022 10:41:32.2967 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: f02be110-c5fe-4432-c41d-08dac23ef78e X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=3dd8961f-e488-4e60-8e11-a82d994e183d;Ip=[165.204.84.17];Helo=[SATLEXMB04.amd.com] X-MS-Exchange-CrossTenant-AuthSource: BN8NAM11FT031.eop-nam11.prod.protection.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Anonymous X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR12MB4217 X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1749014846064279643?= X-GMAIL-MSGID: =?utf-8?q?1749014846064279643?= |
Series |
cxl: Add support for Restricted CXL hosts (RCD mode)
|
|
Commit Message
Robert Richter
Nov. 9, 2022, 10:40 a.m. UTC
When an endpoint is found, all ports in beetween the endpoint and the
CXL host bridge need to be created. In the RCH case there are no ports
in between a host bridge and the endpoint. Skip the enumeration of
intermediate ports.
The port enumeration does not only create all ports, it also
initializes the endpoint chain by adding the endpoint to every
downstream port up to the root bridge. This must be done also in RCD
mode, but is much more simple as the endpoint only needs to be added
to the host bridge's dport.
Note: For endpoint removal the cxl_detach_ep() is not needed as it is
released in cxl_port_release().
Signed-off-by: Robert Richter <rrichter@amd.com>
---
drivers/cxl/core/port.c | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)
Comments
On 11/9/2022 2:40 AM, Robert Richter wrote: > When an endpoint is found, all ports in beetween the endpoint and the s/beetween/between/ DJ > CXL host bridge need to be created. In the RCH case there are no ports > in between a host bridge and the endpoint. Skip the enumeration of > intermediate ports. > > The port enumeration does not only create all ports, it also > initializes the endpoint chain by adding the endpoint to every > downstream port up to the root bridge. This must be done also in RCD > mode, but is much more simple as the endpoint only needs to be added > to the host bridge's dport. > > Note: For endpoint removal the cxl_detach_ep() is not needed as it is > released in cxl_port_release(). > > Signed-off-by: Robert Richter <rrichter@amd.com> > --- > drivers/cxl/core/port.c | 18 ++++++++++++++++-- > 1 file changed, 16 insertions(+), 2 deletions(-) > > diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c > index d10c3580719b..e21a9c3fe4da 100644 > --- a/drivers/cxl/core/port.c > +++ b/drivers/cxl/core/port.c > @@ -1366,8 +1366,24 @@ int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd) > { > struct device *dev = &cxlmd->dev; > struct device *iter; > + struct cxl_dport *dport; > + struct cxl_port *port; > int rc; > > + /* > + * Skip intermediate port enumeration in the RCH case, there > + * are no ports in between a host bridge and an endpoint. Only > + * initialize the EP chain. > + */ > + if (is_cxl_restricted(cxlmd)) { > + port = cxl_mem_find_port(cxlmd, &dport); > + if (!port) > + return -ENXIO; > + rc = cxl_add_ep(dport, &cxlmd->dev); > + put_device(&port->dev); > + return rc; > + } > + > rc = devm_add_action_or_reset(&cxlmd->dev, cxl_detach_ep, cxlmd); > if (rc) > return rc; > @@ -1381,8 +1397,6 @@ int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd) > for (iter = dev; iter; iter = grandparent(iter)) { > struct device *dport_dev = grandparent(iter); > struct device *uport_dev; > - struct cxl_dport *dport; > - struct cxl_port *port; > > if (!dport_dev) > return 0;
Robert Richter wrote: > When an endpoint is found, all ports in beetween the endpoint and the > CXL host bridge need to be created. In the RCH case there are no ports > in between a host bridge and the endpoint. Skip the enumeration of > intermediate ports. > > The port enumeration does not only create all ports, it also > initializes the endpoint chain by adding the endpoint to every > downstream port up to the root bridge. This must be done also in RCD > mode, but is much more simple as the endpoint only needs to be added > to the host bridge's dport. > > Note: For endpoint removal the cxl_detach_ep() is not needed as it is > released in cxl_port_release(). > > Signed-off-by: Robert Richter <rrichter@amd.com> > --- > drivers/cxl/core/port.c | 18 ++++++++++++++++-- > 1 file changed, 16 insertions(+), 2 deletions(-) > > diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c > index d10c3580719b..e21a9c3fe4da 100644 > --- a/drivers/cxl/core/port.c > +++ b/drivers/cxl/core/port.c > @@ -1366,8 +1366,24 @@ int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd) > { > struct device *dev = &cxlmd->dev; > struct device *iter; > + struct cxl_dport *dport; > + struct cxl_port *port; > int rc; > > + /* > + * Skip intermediate port enumeration in the RCH case, there > + * are no ports in between a host bridge and an endpoint. Only > + * initialize the EP chain. > + */ > + if (is_cxl_restricted(cxlmd)) { I changed this to: if (cxlmd->cxlds->rcd) { ...where the cxl_pci driver sets that rcd flag. Aside from keeping some PCI specifics out of this helper it also allows RCH/RCD configurations to be mocked with cxl_test. > + port = cxl_mem_find_port(cxlmd, &dport); > + if (!port) > + return -ENXIO; > + rc = cxl_add_ep(dport, &cxlmd->dev); Ah, good catch, I had missed this detail previously. > + put_device(&port->dev); > + return rc; > + } > + > rc = devm_add_action_or_reset(&cxlmd->dev, cxl_detach_ep, cxlmd); > if (rc) > return rc; > @@ -1381,8 +1397,6 @@ int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd) > for (iter = dev; iter; iter = grandparent(iter)) { > struct device *dport_dev = grandparent(iter); > struct device *uport_dev; > - struct cxl_dport *dport; > - struct cxl_port *port; > > if (!dport_dev) > return 0; > -- > 2.30.2 >
Robert Richter wrote: > When an endpoint is found, all ports in beetween the endpoint and the > CXL host bridge need to be created. In the RCH case there are no ports > in between a host bridge and the endpoint. Skip the enumeration of > intermediate ports. > > The port enumeration does not only create all ports, it also > initializes the endpoint chain by adding the endpoint to every > downstream port up to the root bridge. This must be done also in RCD > mode, but is much more simple as the endpoint only needs to be added > to the host bridge's dport. > > Note: For endpoint removal the cxl_detach_ep() is not needed as it is > released in cxl_port_release(). > > Signed-off-by: Robert Richter <rrichter@amd.com> > --- > drivers/cxl/core/port.c | 18 ++++++++++++++++-- > 1 file changed, 16 insertions(+), 2 deletions(-) > > diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c > index d10c3580719b..e21a9c3fe4da 100644 > --- a/drivers/cxl/core/port.c > +++ b/drivers/cxl/core/port.c > @@ -1366,8 +1366,24 @@ int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd) > { > struct device *dev = &cxlmd->dev; > struct device *iter; > + struct cxl_dport *dport; > + struct cxl_port *port; > int rc; > > + /* > + * Skip intermediate port enumeration in the RCH case, there > + * are no ports in between a host bridge and an endpoint. Only > + * initialize the EP chain. > + */ > + if (is_cxl_restricted(cxlmd)) { > + port = cxl_mem_find_port(cxlmd, &dport); > + if (!port) > + return -ENXIO; > + rc = cxl_add_ep(dport, &cxlmd->dev); On second look, this seems problematic. While yes it will be deleted when the root CXL port dies, it will not be deleted if the cxl_pci driver is reloaded. I will code up a unit test to double check. I note that cxl_add_ep() for the VH case is skipped for the root CXL port, so I do not suspect it is needed here either. Did you add it for a specific reason?
On 14.11.22 16:07:58, Dan Williams wrote: > Robert Richter wrote: > > When an endpoint is found, all ports in beetween the endpoint and the > > CXL host bridge need to be created. In the RCH case there are no ports > > in between a host bridge and the endpoint. Skip the enumeration of > > intermediate ports. > > > > The port enumeration does not only create all ports, it also > > initializes the endpoint chain by adding the endpoint to every > > downstream port up to the root bridge. This must be done also in RCD > > mode, but is much more simple as the endpoint only needs to be added > > to the host bridge's dport. > > > > Note: For endpoint removal the cxl_detach_ep() is not needed as it is > > released in cxl_port_release(). > > > > Signed-off-by: Robert Richter <rrichter@amd.com> > > --- > > drivers/cxl/core/port.c | 18 ++++++++++++++++-- > > 1 file changed, 16 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c > > index d10c3580719b..e21a9c3fe4da 100644 > > --- a/drivers/cxl/core/port.c > > +++ b/drivers/cxl/core/port.c > > @@ -1366,8 +1366,24 @@ int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd) > > { > > struct device *dev = &cxlmd->dev; > > struct device *iter; > > + struct cxl_dport *dport; > > + struct cxl_port *port; > > int rc; > > > > + /* > > + * Skip intermediate port enumeration in the RCH case, there > > + * are no ports in between a host bridge and an endpoint. Only > > + * initialize the EP chain. > > + */ > > + if (is_cxl_restricted(cxlmd)) { > > I changed this to: > > if (cxlmd->cxlds->rcd) { I a mail to Bjorn I suggested to have cxl_dev_cap and a cxl_port_cap in the pci_dev struct that could be looked up too including RCD mode. Checking the pci_dev looks more reasonable to me here, though we could have a flag of it in cxlds too. -Robert > > ...where the cxl_pci driver sets that rcd flag. Aside from keeping some > PCI specifics out of this helper it also allows RCH/RCD configurations > to be mocked with cxl_test. > > > + port = cxl_mem_find_port(cxlmd, &dport); > > + if (!port) > > + return -ENXIO; > > + rc = cxl_add_ep(dport, &cxlmd->dev); > > Ah, good catch, I had missed this detail previously. > > > + put_device(&port->dev); > > + return rc; > > + } > > + > > rc = devm_add_action_or_reset(&cxlmd->dev, cxl_detach_ep, cxlmd); > > if (rc) > > return rc; > > @@ -1381,8 +1397,6 @@ int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd) > > for (iter = dev; iter; iter = grandparent(iter)) { > > struct device *dport_dev = grandparent(iter); > > struct device *uport_dev; > > - struct cxl_dport *dport; > > - struct cxl_port *port; > > > > if (!dport_dev) > > return 0; > > -- > > 2.30.2 > > > >
On 14.11.22 16:24:06, Dan Williams wrote: > Robert Richter wrote: > > When an endpoint is found, all ports in beetween the endpoint and the > > CXL host bridge need to be created. In the RCH case there are no ports > > in between a host bridge and the endpoint. Skip the enumeration of > > intermediate ports. > > > > The port enumeration does not only create all ports, it also > > initializes the endpoint chain by adding the endpoint to every > > downstream port up to the root bridge. This must be done also in RCD > > mode, but is much more simple as the endpoint only needs to be added > > to the host bridge's dport. > > > > Note: For endpoint removal the cxl_detach_ep() is not needed as it is > > released in cxl_port_release(). > > > > Signed-off-by: Robert Richter <rrichter@amd.com> > > --- > > drivers/cxl/core/port.c | 18 ++++++++++++++++-- > > 1 file changed, 16 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c > > index d10c3580719b..e21a9c3fe4da 100644 > > --- a/drivers/cxl/core/port.c > > +++ b/drivers/cxl/core/port.c > > @@ -1366,8 +1366,24 @@ int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd) > > { > > struct device *dev = &cxlmd->dev; > > struct device *iter; > > + struct cxl_dport *dport; > > + struct cxl_port *port; > > int rc; > > > > + /* > > + * Skip intermediate port enumeration in the RCH case, there > > + * are no ports in between a host bridge and an endpoint. Only > > + * initialize the EP chain. > > + */ > > + if (is_cxl_restricted(cxlmd)) { > > + port = cxl_mem_find_port(cxlmd, &dport); > > + if (!port) > > + return -ENXIO; > > + rc = cxl_add_ep(dport, &cxlmd->dev); > > On second look, this seems problematic. While yes it will be deleted > when the root CXL port dies, it will not be deleted if the cxl_pci > driver is reloaded. I will code up a unit test to double check. > > I note that cxl_add_ep() for the VH case is skipped for the root CXL > port, so I do not suspect it is needed here either. Did you add it for a > specific reason? Yes, all endpoint iterators need to be reworked. Also true, the first endpoint is skipped in the chain. So only intermediate EP structs are touched by the loops actually. In particular, cxl_ep_load() returned NULL for the first lookup if the ep is missing. We could stop the iteration then. I tried to avoid a rework here, but maybe it is not too extensive as I expected first. -Robert
Robert Richter wrote: > On 14.11.22 16:07:58, Dan Williams wrote: > > Robert Richter wrote: > > > When an endpoint is found, all ports in beetween the endpoint and the > > > CXL host bridge need to be created. In the RCH case there are no ports > > > in between a host bridge and the endpoint. Skip the enumeration of > > > intermediate ports. > > > > > > The port enumeration does not only create all ports, it also > > > initializes the endpoint chain by adding the endpoint to every > > > downstream port up to the root bridge. This must be done also in RCD > > > mode, but is much more simple as the endpoint only needs to be added > > > to the host bridge's dport. > > > > > > Note: For endpoint removal the cxl_detach_ep() is not needed as it is > > > released in cxl_port_release(). > > > > > > Signed-off-by: Robert Richter <rrichter@amd.com> > > > --- > > > drivers/cxl/core/port.c | 18 ++++++++++++++++-- > > > 1 file changed, 16 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c > > > index d10c3580719b..e21a9c3fe4da 100644 > > > --- a/drivers/cxl/core/port.c > > > +++ b/drivers/cxl/core/port.c > > > @@ -1366,8 +1366,24 @@ int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd) > > > { > > > struct device *dev = &cxlmd->dev; > > > struct device *iter; > > > + struct cxl_dport *dport; > > > + struct cxl_port *port; > > > int rc; > > > > > > + /* > > > + * Skip intermediate port enumeration in the RCH case, there > > > + * are no ports in between a host bridge and an endpoint. Only > > > + * initialize the EP chain. > > > + */ > > > + if (is_cxl_restricted(cxlmd)) { > > > > I changed this to: > > > > if (cxlmd->cxlds->rcd) { > > I a mail to Bjorn I suggested to have cxl_dev_cap and a cxl_port_cap > in the pci_dev struct that could be looked up too including RCD mode. > Checking the pci_dev looks more reasonable to me here, though we could > have a flag of it in cxlds too. Would that not need the PCI core to understand how to walk the RCRB generically? As far as I understand the RCRB association is ACPI.CEDT specific.
Robert Richter wrote: > On 14.11.22 16:24:06, Dan Williams wrote: > > Robert Richter wrote: > > > When an endpoint is found, all ports in beetween the endpoint and the > > > CXL host bridge need to be created. In the RCH case there are no ports > > > in between a host bridge and the endpoint. Skip the enumeration of > > > intermediate ports. > > > > > > The port enumeration does not only create all ports, it also > > > initializes the endpoint chain by adding the endpoint to every > > > downstream port up to the root bridge. This must be done also in RCD > > > mode, but is much more simple as the endpoint only needs to be added > > > to the host bridge's dport. > > > > > > Note: For endpoint removal the cxl_detach_ep() is not needed as it is > > > released in cxl_port_release(). > > > > > > Signed-off-by: Robert Richter <rrichter@amd.com> > > > --- > > > drivers/cxl/core/port.c | 18 ++++++++++++++++-- > > > 1 file changed, 16 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c > > > index d10c3580719b..e21a9c3fe4da 100644 > > > --- a/drivers/cxl/core/port.c > > > +++ b/drivers/cxl/core/port.c > > > @@ -1366,8 +1366,24 @@ int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd) > > > { > > > struct device *dev = &cxlmd->dev; > > > struct device *iter; > > > + struct cxl_dport *dport; > > > + struct cxl_port *port; > > > int rc; > > > > > > + /* > > > + * Skip intermediate port enumeration in the RCH case, there > > > + * are no ports in between a host bridge and an endpoint. Only > > > + * initialize the EP chain. > > > + */ > > > + if (is_cxl_restricted(cxlmd)) { > > > + port = cxl_mem_find_port(cxlmd, &dport); > > > + if (!port) > > > + return -ENXIO; > > > + rc = cxl_add_ep(dport, &cxlmd->dev); > > > > On second look, this seems problematic. While yes it will be deleted > > when the root CXL port dies, it will not be deleted if the cxl_pci > > driver is reloaded. I will code up a unit test to double check. > > > > I note that cxl_add_ep() for the VH case is skipped for the root CXL > > port, so I do not suspect it is needed here either. Did you add it for a > > specific reason? > > Yes, all endpoint iterators need to be reworked. Also true, the first > endpoint is skipped in the chain. So only intermediate EP structs are > touched by the loops actually. > > In particular, cxl_ep_load() returned NULL for the first lookup if the > ep is missing. We could stop the iteration then. I tried to avoid a > rework here, but maybe it is not too extensive as I expected first. Hmm, ok, let me get the unit test topology working for this to make sure my assumptions are correct about when an @ep reference is used / needed.
On 15.11.22 10:08:51, Dan Williams wrote: > Robert Richter wrote: > > On 14.11.22 16:07:58, Dan Williams wrote: > > > Robert Richter wrote: > > > > When an endpoint is found, all ports in beetween the endpoint and the > > > > CXL host bridge need to be created. In the RCH case there are no ports > > > > in between a host bridge and the endpoint. Skip the enumeration of > > > > intermediate ports. > > > > > > > > The port enumeration does not only create all ports, it also > > > > initializes the endpoint chain by adding the endpoint to every > > > > downstream port up to the root bridge. This must be done also in RCD > > > > mode, but is much more simple as the endpoint only needs to be added > > > > to the host bridge's dport. > > > > > > > > Note: For endpoint removal the cxl_detach_ep() is not needed as it is > > > > released in cxl_port_release(). > > > > > > > > Signed-off-by: Robert Richter <rrichter@amd.com> > > > > --- > > > > drivers/cxl/core/port.c | 18 ++++++++++++++++-- > > > > 1 file changed, 16 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c > > > > index d10c3580719b..e21a9c3fe4da 100644 > > > > --- a/drivers/cxl/core/port.c > > > > +++ b/drivers/cxl/core/port.c > > > > @@ -1366,8 +1366,24 @@ int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd) > > > > { > > > > struct device *dev = &cxlmd->dev; > > > > struct device *iter; > > > > + struct cxl_dport *dport; > > > > + struct cxl_port *port; > > > > int rc; > > > > > > > > + /* > > > > + * Skip intermediate port enumeration in the RCH case, there > > > > + * are no ports in between a host bridge and an endpoint. Only > > > > + * initialize the EP chain. > > > > + */ > > > > + if (is_cxl_restricted(cxlmd)) { > > > > > > I changed this to: > > > > > > if (cxlmd->cxlds->rcd) { > > > > I a mail to Bjorn I suggested to have cxl_dev_cap and a cxl_port_cap > > in the pci_dev struct that could be looked up too including RCD mode. > > Checking the pci_dev looks more reasonable to me here, though we could > > have a flag of it in cxlds too. > > Would that not need the PCI core to understand how to walk the RCRB > generically? As far as I understand the RCRB association is ACPI.CEDT > specific. I am thinking of doing some sort of cxl_setup_pci_dev(pdev) in cxl_pci_probe() which extracts the caps and writes them into a struct pci_dev. Possibly the CXL 68B Flit and VH Capable/Enable bit could be used for RCD mode. But if that is not feasible for some reason a flag somewhere could work too. -Robert
diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c index d10c3580719b..e21a9c3fe4da 100644 --- a/drivers/cxl/core/port.c +++ b/drivers/cxl/core/port.c @@ -1366,8 +1366,24 @@ int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd) { struct device *dev = &cxlmd->dev; struct device *iter; + struct cxl_dport *dport; + struct cxl_port *port; int rc; + /* + * Skip intermediate port enumeration in the RCH case, there + * are no ports in between a host bridge and an endpoint. Only + * initialize the EP chain. + */ + if (is_cxl_restricted(cxlmd)) { + port = cxl_mem_find_port(cxlmd, &dport); + if (!port) + return -ENXIO; + rc = cxl_add_ep(dport, &cxlmd->dev); + put_device(&port->dev); + return rc; + } + rc = devm_add_action_or_reset(&cxlmd->dev, cxl_detach_ep, cxlmd); if (rc) return rc; @@ -1381,8 +1397,6 @@ int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd) for (iter = dev; iter; iter = grandparent(iter)) { struct device *dport_dev = grandparent(iter); struct device *uport_dev; - struct cxl_dport *dport; - struct cxl_port *port; if (!dport_dev) return 0;