Message ID | 20221018132341.76259-7-rrichter@amd.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a5d:4ac7:0:0:0:0:0 with SMTP id y7csp1962613wrs; Tue, 18 Oct 2022 06:33:03 -0700 (PDT) X-Google-Smtp-Source: AMsMyM6MKu0zpVhpJlBSYELmr1efOdw7lCq4nJHup+h1/ZQ9nP0MTnXyk10rgxWbJ5o161cZ6aS6 X-Received: by 2002:a05:6402:36b:b0:458:5ac2:8d56 with SMTP id s11-20020a056402036b00b004585ac28d56mr2708264edw.25.1666099983737; Tue, 18 Oct 2022 06:33:03 -0700 (PDT) ARC-Seal: i=2; a=rsa-sha256; t=1666099983; cv=pass; d=google.com; s=arc-20160816; b=iq+7W5YBKZt+t6o7vEJJ9g8jgGOZoCivIoOqR+6uDQBngl1Mo3V/7pTYkDUB0hM8B9 2nSiLovNU4y2Gk1uVNPAI3FIUYhhGX8jtTs356gBfIE6INKeOcc5toHygT8zjDxM4m64 BxzyT6Owmjl1F8gODMNGI5ut3npxyKYBgquFO8JV1RVfD326kMboRUpFFKmGIocuG+X0 Bj9MAv97NHNwsa/1Mqv53LtBilDoZG/IuZWoimPMl0D/0fOU6lqT4vxHoz3KMHUyvhqG 4iJ8pbyvlIAwPlfSeUiCz4prebVCdR+HbVbJzCFlJmFnOv1eLkAihKsFseiK3g73DPZq 6mSQ== 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=wVjwz2jaYW2t613ACvCnJfgCx2sRIEoo1AFxWYaapRo=; b=G2UFQyg9RNdDXrWAjq+eLGXfGO6WtBB2RM6Pb2IfP989CC/dG9JY58XNde+Ct6rgsr PBnK7HSYjfsBe+buRq8P/swZmNuHUcxzCEoS5SSZQ3WNSnY8gh/SZPK7BGn2ntZiScED tTr/fjklej6sawEm5qtWiKfrPqYUamwZ6j9eq/FR8ZCMGFd4mD9usPUXR/0Iq4jHeNk1 K05ewN2NsQCrg9kziXLjUVXzB6bOD+Wc8gSXIgqTC0F4mU+vSRfExLRuAKfptGKhhxml bZAk+ALy2aK65KOviNDCeIUCx+KlD73qN1vJZeSN3Ei2V/tPtDEBUX32CA3LSaXShWyJ dN6g== ARC-Authentication-Results: i=2; mx.google.com; dkim=pass header.i=@amd.com header.s=selector1 header.b=zsqzRNZA; 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 g6-20020a1709064e4600b0077cb9bc7984si10103556ejw.181.2022.10.18.06.32.38; Tue, 18 Oct 2022 06:33:03 -0700 (PDT) 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=zsqzRNZA; 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 S230430AbiJRNYv (ORCPT <rfc822;carlos.wei.hk@gmail.com> + 99 others); Tue, 18 Oct 2022 09:24:51 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:41134 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229907AbiJRNYf (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Tue, 18 Oct 2022 09:24:35 -0400 Received: from NAM04-MW2-obe.outbound.protection.outlook.com (mail-mw2nam04on2051.outbound.protection.outlook.com [40.107.101.51]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 2A314140B3; Tue, 18 Oct 2022 06:24:26 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=ISpqoUL9RAMcB78m33fW1psXBDsSg7BxtH3ISb21lOgp2tARMrDKbSmZUXXJs4QuKrhlzbpTFTA51yg6JQlMHMbemtQK+X0lBmEKTmiQA52DW5DtmCoRl+qUDRkKnbyIOdWg5v488AQrNQ+jDvw4JIitt0NxCYCrsWkgVHPoOMZVLedICC367oFw31nf4YfsolRkUyKKdUyRG9ALgSBWZ2QqXRb8evw9TH7JiLAWWwVQ2wNZJ0O9cyyNaJ+nYxhZfzLLPmVkEdsFjXrUuygdXXrundJuj9Dg9++iwMs+hesuPj/sVQBZtVvvporj6Hj9OCBvd6gRnezzngTgotm9Yg== 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=wVjwz2jaYW2t613ACvCnJfgCx2sRIEoo1AFxWYaapRo=; b=FczGjqyc8WRRqjz6zrHWa8Por1tRmd9KodVVAW6YaqExYVJVGJbAsVcIVLqhBgCru0GPQ7R4ezOVJ4nv9IjePRQeAFn61u6JeNHjZQqhWaBe+ycMZcvm/+t1Xk964p2NbdhdBtx/W4Zwp8GWCYoltcT+2UO7a2zgAtcikojfolFsKJ1szwDKV76zYYyL5E6OXvt7g9p1c9bE+XWHgaGk8SRWUhoy261vGjsZShGFRFd8gmIE2ENYxU8N1wyegSuJIcYLe7iWHsrwKSn02zvAKRm+as7TMTFJu+wQbRzmnFQm19P/CgAxXWObLuGV4m0jmafkPDB/3325yRjoLWLWUw== 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=wVjwz2jaYW2t613ACvCnJfgCx2sRIEoo1AFxWYaapRo=; b=zsqzRNZAt3duRSCvCuKuvw83ArjU/ZEJmW3GBaspzx8AUfq8HJjlFzWbEi9PAv6UGBxhn8WOrzLhknA6SIHsc5NMkRz44F9Pap/le4vh5cpddYv8Gr0VN5JueKhjiXnUQsDUsUZ94ZXf8KmJ7gZmVHHpIu3plFgH57EY02nkIT0= Received: from BN9P220CA0017.NAMP220.PROD.OUTLOOK.COM (2603:10b6:408:13e::22) by BN9PR12MB5305.namprd12.prod.outlook.com (2603:10b6:408:102::5) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5723.30; Tue, 18 Oct 2022 13:24:23 +0000 Received: from BN8NAM11FT107.eop-nam11.prod.protection.outlook.com (2603:10b6:408:13e:cafe::66) by BN9P220CA0017.outlook.office365.com (2603:10b6:408:13e::22) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5723.31 via Frontend Transport; Tue, 18 Oct 2022 13:24:23 +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 BN8NAM11FT107.mail.protection.outlook.com (10.13.176.149) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.20.5723.20 via Frontend Transport; Tue, 18 Oct 2022 13:24:23 +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; Tue, 18 Oct 2022 08:24:20 -0500 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>, Robert Richter <rrichter@amd.com>, Terry Bowman <terry.bowman@amd.com> Subject: [PATCH v2 06/12] cxl/acpi: Extract component registers of restricted hosts from RCRB Date: Tue, 18 Oct 2022 15:23:34 +0200 Message-ID: <20221018132341.76259-7-rrichter@amd.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20221018132341.76259-1-rrichter@amd.com> References: <20221018132341.76259-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: SATLEXMB03.amd.com (10.181.40.144) To SATLEXMB04.amd.com (10.181.40.145) X-EOPAttributedMessage: 0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: BN8NAM11FT107:EE_|BN9PR12MB5305:EE_ X-MS-Office365-Filtering-Correlation-Id: 1d1d5869-1341-4b32-89a5-08dab10c1244 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: TLrg3RxoS8c3Iw4iQmKYS7I9jn2YaPpamA1jgugsSVVINx+iyfYheycQKluMYBHkdIr4NuO/jnegWyi8ogbhMlm3MW8ZYbXSXYHtHaO25hM/MwsKCsafbtWamL2Z3Im3hP1Jl3tkIlcXq+wBKfUxwnhaUmKL/YXFTNIg3lbunMCnEsa+0MA+cCVdnmGunjazNJdAY0jOlt5l7GsJXnlRBODsn98LPNlVHCQDIYCImUbbBEzbcpedFMGTKooGEc9nRDBxjoLoJDDJaCAd4sqqRuKBPKDwrsUmMHio9SYdzM3m/Xdtu1+9F8qyg01rZAPM3eqGO2YeLMNUWIKRpMyhyEk6eFKkUnpjRE20iljHIszRVAM/jrGceNveFd4K4Oo5v2CP6d/KMreBYoFiw2MhOmDZRYUzjMMHipy5eSahdswmgofbv7XawWTcksGulSKbtBsCsI0uwb2VXZn762knaIAYWDKn+wCulEec3+2sNW8doojZwgq2hWO9HKP+C3tAPvHt/W1os9ntRYIk23J7z6WIEemtAeVWX/qHkoa+gPn4/WZWZfWNcPBmthMadKC77Kwc9QSogpYVQEJ0kNcSXOhJy+msqoxkc30sUz8ojRAL0RaLn303j0mS8PABd53CkfmY/Onj9QKGH15+3yUf8gEywgqW/JZ/ODGmNpQ8HGb8lIr/xmcOZne0TS1F0t8W/zIoe3BG4GYfSiQSwo2up8ZBOZsM+hQ81h4Nb5GTpfLcYgeG9yQ+7o6WX/NAdxCDDRfzw9RIrcNrMB5512IqC7DYRRXPwfu+F5RxA9tiiEY= 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)(376002)(136003)(396003)(39860400002)(346002)(451199015)(36840700001)(46966006)(40470700004)(26005)(41300700001)(40460700003)(186003)(6666004)(1076003)(16526019)(336012)(478600001)(2616005)(82310400005)(36756003)(2906002)(7416002)(5660300002)(4326008)(8936002)(36860700001)(110136005)(54906003)(82740400003)(40480700001)(316002)(426003)(47076005)(8676002)(70586007)(83380400001)(356005)(81166007)(70206006)(36900700001);DIR:OUT;SFP:1101; X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 18 Oct 2022 13:24:23.0083 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: 1d1d5869-1341-4b32-89a5-08dab10c1244 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: BN8NAM11FT107.eop-nam11.prod.protection.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Anonymous X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN9PR12MB5305 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?1747032456224305014?= X-GMAIL-MSGID: =?utf-8?q?1747032456224305014?= |
Series |
cxl: Add support for Restricted CXL hosts (RCD mode)
|
|
Commit Message
Robert Richter
Oct. 18, 2022, 1:23 p.m. UTC
A downstream port must be connected to a component register block. For restricted hosts the base address is determined from the RCRB. The RCRB is provided by the host's CEDT CHBS entry. Rework CEDT parser to get the RCRB and add code to extract the component register block from it. RCRB's BAR[0..1] point to the component block containing CXL subsystem component registers. MEMBAR extraction follows the PCI base spec here, esp. 64 bit extraction and memory range alignment (6.0, 7.5.1.2.1). Note: Right now the component register block is used for HDM decoder capability only which is optional for RCDs. If unsupported by the RCD, the HDM init will fail. It is future work to bypass it in this case. Signed-off-by: Terry Bowman <terry.bowman@amd.com> Signed-off-by: Robert Richter <rrichter@amd.com> --- drivers/cxl/acpi.c | 79 ++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 69 insertions(+), 10 deletions(-)
Comments
On Tue, Oct 18, 2022 at 3:24 PM Robert Richter <rrichter@amd.com> wrote: > > A downstream port must be connected to a component register block. > For restricted hosts the base address is determined from the RCRB. The > RCRB is provided by the host's CEDT CHBS entry. Rework CEDT parser to > get the RCRB and add code to extract the component register block from > it. > > RCRB's BAR[0..1] point to the component block containing CXL subsystem > component registers. MEMBAR extraction follows the PCI base spec here, > esp. 64 bit extraction and memory range alignment (6.0, 7.5.1.2.1). > > Note: Right now the component register block is used for HDM decoder > capability only which is optional for RCDs. If unsupported by the RCD, > the HDM init will fail. It is future work to bypass it in this case. > > Signed-off-by: Terry Bowman <terry.bowman@amd.com> What does this S-o-B mean? If this person is your co-developer, you need to add a Co-developed-by tag to clarify that. > Signed-off-by: Robert Richter <rrichter@amd.com> > --- > drivers/cxl/acpi.c | 79 ++++++++++++++++++++++++++++++++++++++++------ > 1 file changed, 69 insertions(+), 10 deletions(-) > > diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c > index fb9f72813067..a92d5d7b7a92 100644 > --- a/drivers/cxl/acpi.c > +++ b/drivers/cxl/acpi.c > @@ -9,6 +9,8 @@ > #include "cxlpci.h" > #include "cxl.h" > > +#define CXL_RCRB_SIZE SZ_8K > + > static unsigned long cfmws_to_decoder_flags(int restrictions) > { > unsigned long flags = CXL_DECODER_F_ENABLE; > @@ -229,27 +231,82 @@ static int add_host_bridge_uport(struct device *match, void *arg) > struct cxl_chbs_context { > struct device *dev; > unsigned long long uid; > - resource_size_t chbcr; > + struct acpi_cedt_chbs chbs; > }; > > -static int cxl_get_chbcr(union acpi_subtable_headers *header, void *arg, > - const unsigned long end) > +static int cxl_get_chbs(union acpi_subtable_headers *header, void *arg, > + const unsigned long end) > { > struct cxl_chbs_context *ctx = arg; > struct acpi_cedt_chbs *chbs; > > - if (ctx->chbcr) > + if (ctx->chbs.base) > return 0; > > chbs = (struct acpi_cedt_chbs *) header; > > if (ctx->uid != chbs->uid) > return 0; > - ctx->chbcr = chbs->base; > + ctx->chbs = *chbs; > > return 0; > } > > +static resource_size_t cxl_get_chbcr(struct cxl_chbs_context *ctx) > +{ > + struct acpi_cedt_chbs *chbs = &ctx->chbs; > + resource_size_t component_reg_phys, rcrb; > + u32 bar0, bar1; > + void *addr; > + > + if (!chbs->base) > + return CXL_RESOURCE_NONE; > + > + if (chbs->cxl_version != ACPI_CEDT_CHBS_VERSION_CXL11) > + return chbs->base; > + > + /* Extract RCRB */ > + > + if (chbs->length != CXL_RCRB_SIZE) > + return CXL_RESOURCE_NONE; > + > + rcrb = chbs->base; > + > + dev_dbg(ctx->dev, "RCRB found for UID %lld: 0x%08llx\n", > + ctx->uid, (u64)rcrb); > + > + /* > + * RCRB's BAR[0..1] point to component block containing CXL > + * subsystem component registers. MEMBAR extraction follows > + * the PCI Base spec here, esp. 64 bit extraction and memory > + * ranges alignment (6.0, 7.5.1.2.1). > + */ > + addr = ioremap(rcrb, PCI_BASE_ADDRESS_0 + SZ_8); > + bar0 = readl(addr + PCI_BASE_ADDRESS_0); > + bar1 = readl(addr + PCI_BASE_ADDRESS_1); > + iounmap(addr); > + > + /* sanity check */ > + if (bar0 & (PCI_BASE_ADDRESS_MEM_TYPE_1M | PCI_BASE_ADDRESS_SPACE_IO)) > + return CXL_RESOURCE_NONE; > + > + component_reg_phys = bar0 & PCI_BASE_ADDRESS_MEM_MASK; > + if (bar0 & PCI_BASE_ADDRESS_MEM_TYPE_64) > + component_reg_phys |= ((u64)bar1) << 32; > + > + if (!component_reg_phys) > + return CXL_RESOURCE_NONE; > + > + /* > + * Must be 8k aligned (size of combined CXL 1.1 Downstream and > + * Upstream Port RCRBs). > + */ > + if (component_reg_phys & (CXL_RCRB_SIZE - 1)) > + return CXL_RESOURCE_NONE; > + > + return component_reg_phys; > +} > + > static int add_host_bridge_dport(struct device *match, void *arg) > { > acpi_status status; > @@ -259,6 +316,7 @@ static int add_host_bridge_dport(struct device *match, void *arg) > struct cxl_port *root_port = arg; > struct device *host = root_port->dev.parent; > struct acpi_device *bridge = to_cxl_host_bridge(host, match); > + resource_size_t component_reg_phys; > > if (!bridge) > return 0; > @@ -273,19 +331,20 @@ static int add_host_bridge_dport(struct device *match, void *arg) > dev_dbg(match, "UID found: %lld\n", uid); > > ctx = (struct cxl_chbs_context) { > - .dev = host, > + .dev = match, > .uid = uid, > }; > - acpi_table_parse_cedt(ACPI_CEDT_TYPE_CHBS, cxl_get_chbcr, &ctx); > + acpi_table_parse_cedt(ACPI_CEDT_TYPE_CHBS, cxl_get_chbs, &ctx); > > - if (ctx.chbcr == 0) { > + component_reg_phys = cxl_get_chbcr(&ctx); > + if (component_reg_phys == CXL_RESOURCE_NONE) { > dev_warn(match, "No CHBS found for Host Bridge (UID %lld)\n", uid); > return 0; > } > > - dev_dbg(match, "CHBCR found: 0x%08llx\n", (u64)ctx.chbcr); > + dev_dbg(match, "CHBCR found: 0x%08llx\n", (u64)component_reg_phys); > > - dport = devm_cxl_add_dport(root_port, match, uid, ctx.chbcr); > + dport = devm_cxl_add_dport(root_port, match, uid, component_reg_phys); > if (IS_ERR(dport)) > return PTR_ERR(dport); > > -- > 2.30.2 >
On 18.10.22 15:31:16, Rafael J. Wysocki wrote: > On Tue, Oct 18, 2022 at 3:24 PM Robert Richter <rrichter@amd.com> wrote: > > > > A downstream port must be connected to a component register block. > > For restricted hosts the base address is determined from the RCRB. The > > RCRB is provided by the host's CEDT CHBS entry. Rework CEDT parser to > > get the RCRB and add code to extract the component register block from > > it. > > > > RCRB's BAR[0..1] point to the component block containing CXL subsystem > > component registers. MEMBAR extraction follows the PCI base spec here, > > esp. 64 bit extraction and memory range alignment (6.0, 7.5.1.2.1). > > > > Note: Right now the component register block is used for HDM decoder > > capability only which is optional for RCDs. If unsupported by the RCD, > > the HDM init will fail. It is future work to bypass it in this case. > > > > Signed-off-by: Terry Bowman <terry.bowman@amd.com> > > What does this S-o-B mean? If this person is your co-developer, you > need to add a Co-developed-by tag to clarify that. > > > Signed-off-by: Robert Richter <rrichter@amd.com> I picked up an early patch and modified it significantly, so I just left the S-o-B. I could change this to a Co-developed-by tag. IMO, the S-o-B is ok, but could be wrong here. -Robert
On Tue, Oct 18, 2022 at 8:42 PM Robert Richter <rrichter@amd.com> wrote: > > On 18.10.22 15:31:16, Rafael J. Wysocki wrote: > > On Tue, Oct 18, 2022 at 3:24 PM Robert Richter <rrichter@amd.com> wrote: > > > > > > A downstream port must be connected to a component register block. > > > For restricted hosts the base address is determined from the RCRB. The > > > RCRB is provided by the host's CEDT CHBS entry. Rework CEDT parser to > > > get the RCRB and add code to extract the component register block from > > > it. > > > > > > RCRB's BAR[0..1] point to the component block containing CXL subsystem > > > component registers. MEMBAR extraction follows the PCI base spec here, > > > esp. 64 bit extraction and memory range alignment (6.0, 7.5.1.2.1). > > > > > > Note: Right now the component register block is used for HDM decoder > > > capability only which is optional for RCDs. If unsupported by the RCD, > > > the HDM init will fail. It is future work to bypass it in this case. > > > > > > Signed-off-by: Terry Bowman <terry.bowman@amd.com> > > > > What does this S-o-B mean? If this person is your co-developer, you > > need to add a Co-developed-by tag to clarify that. > > > > > Signed-off-by: Robert Richter <rrichter@amd.com> > > I picked up an early patch and modified it significantly, so I just > left the S-o-B. In that case the right thing to do is to mention the original author in the changelog instead of retaining the S-o-b. > I could change this to a Co-developed-by tag. Co-developed-by should be used in addition to and not instead of S-o-b when one of the authors is sending a patch. However, all of the authors need to be familiar with the patch in the form in which it is being sent then. > IMO, the S-o-B is ok, but could be wrong here. It isn't, at least not without a Co-developed-by tag. There are 3 cases in which S-o-b is OK AFAICS: 1. When it matches the From: address. 2. When there is a matching Co-developed-by. 3. When maintainers pick up patches and add their own S-o-b. This case is none of the above.
On 18.10.22 20:57:02, Rafael J. Wysocki wrote: > On Tue, Oct 18, 2022 at 8:42 PM Robert Richter <rrichter@amd.com> wrote: > > > > On 18.10.22 15:31:16, Rafael J. Wysocki wrote: > > > On Tue, Oct 18, 2022 at 3:24 PM Robert Richter <rrichter@amd.com> wrote: > > > > > > > > A downstream port must be connected to a component register block. > > > > For restricted hosts the base address is determined from the RCRB. The > > > > RCRB is provided by the host's CEDT CHBS entry. Rework CEDT parser to > > > > get the RCRB and add code to extract the component register block from > > > > it. > > > > > > > > RCRB's BAR[0..1] point to the component block containing CXL subsystem > > > > component registers. MEMBAR extraction follows the PCI base spec here, > > > > esp. 64 bit extraction and memory range alignment (6.0, 7.5.1.2.1). > > > > > > > > Note: Right now the component register block is used for HDM decoder > > > > capability only which is optional for RCDs. If unsupported by the RCD, > > > > the HDM init will fail. It is future work to bypass it in this case. > > > > > > > > Signed-off-by: Terry Bowman <terry.bowman@amd.com> > > > > > > What does this S-o-B mean? If this person is your co-developer, you > > > need to add a Co-developed-by tag to clarify that. > > > > > > > Signed-off-by: Robert Richter <rrichter@amd.com> > > > > I picked up an early patch and modified it significantly, so I just > > left the S-o-B. > > In that case the right thing to do is to mention the original author > in the changelog instead of retaining the S-o-b. > > > I could change this to a Co-developed-by tag. > > Co-developed-by should be used in addition to and not instead of S-o-b > when one of the authors is sending a patch. However, all of the > authors need to be familiar with the patch in the form in which it is > being sent then. > > > IMO, the S-o-B is ok, but could be wrong here. > > It isn't, at least not without a Co-developed-by tag. > > There are 3 cases in which S-o-b is OK AFAICS: > > 1. When it matches the From: address. > 2. When there is a matching Co-developed-by. > 3. When maintainers pick up patches and add their own S-o-b. > > This case is none of the above. Will add a Co-developed-by tag in my next version. Thanks for pointing that out. -Robert
Robert Richter wrote: > A downstream port must be connected to a component register block. > For restricted hosts the base address is determined from the RCRB. The > RCRB is provided by the host's CEDT CHBS entry. Rework CEDT parser to > get the RCRB and add code to extract the component register block from > it. > > RCRB's BAR[0..1] point to the component block containing CXL subsystem > component registers. MEMBAR extraction follows the PCI base spec here, > esp. 64 bit extraction and memory range alignment (6.0, 7.5.1.2.1). > > Note: Right now the component register block is used for HDM decoder > capability only which is optional for RCDs. If unsupported by the RCD, > the HDM init will fail. It is future work to bypass it in this case. > > Signed-off-by: Terry Bowman <terry.bowman@amd.com> > Signed-off-by: Robert Richter <rrichter@amd.com> > --- > drivers/cxl/acpi.c | 79 ++++++++++++++++++++++++++++++++++++++++------ > 1 file changed, 69 insertions(+), 10 deletions(-) > > diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c > index fb9f72813067..a92d5d7b7a92 100644 > --- a/drivers/cxl/acpi.c > +++ b/drivers/cxl/acpi.c > @@ -9,6 +9,8 @@ > #include "cxlpci.h" > #include "cxl.h" > > +#define CXL_RCRB_SIZE SZ_8K > + > static unsigned long cfmws_to_decoder_flags(int restrictions) > { > unsigned long flags = CXL_DECODER_F_ENABLE; > @@ -229,27 +231,82 @@ static int add_host_bridge_uport(struct device *match, void *arg) > struct cxl_chbs_context { > struct device *dev; > unsigned long long uid; > - resource_size_t chbcr; > + struct acpi_cedt_chbs chbs; > }; > > -static int cxl_get_chbcr(union acpi_subtable_headers *header, void *arg, > - const unsigned long end) > +static int cxl_get_chbs(union acpi_subtable_headers *header, void *arg, > + const unsigned long end) > { > struct cxl_chbs_context *ctx = arg; > struct acpi_cedt_chbs *chbs; > > - if (ctx->chbcr) > + if (ctx->chbs.base) > return 0; > > chbs = (struct acpi_cedt_chbs *) header; > > if (ctx->uid != chbs->uid) > return 0; > - ctx->chbcr = chbs->base; > + ctx->chbs = *chbs; > > return 0; > } > > +static resource_size_t cxl_get_chbcr(struct cxl_chbs_context *ctx) > +{ The core logic of this looks good, but this wants to be shared with the upstream port component register discovery. Full disclosure I am reconciling these patches with an attempt that Dave Jiang made at this topic. Since your series hit the list first I will let it take the lead, but then fill it in with comments and learnings from Dave's effort. So in this case Dave moved this into the drivers/cxl/core/regs.c with a function signature like: enum cxl_rcrb { CXL_RCRB_DOWNSTREAM, CXL_RCRB_UPSTREAM, }; resource_size_t cxl_rcrb_to_component(struct device *dev, resource_size_t rcrb_base, int len, enum cxl_rcrb which); ...where @which alternates when called by cxl_acpi for the downstream case, or cxl_mem for the upstream case. > + struct acpi_cedt_chbs *chbs = &ctx->chbs; > + resource_size_t component_reg_phys, rcrb; > + u32 bar0, bar1; > + void *addr; > + > + if (!chbs->base) > + return CXL_RESOURCE_NONE; > + > + if (chbs->cxl_version != ACPI_CEDT_CHBS_VERSION_CXL11) > + return chbs->base; > + > + /* Extract RCRB */ > + > + if (chbs->length != CXL_RCRB_SIZE) > + return CXL_RESOURCE_NONE; > + > + rcrb = chbs->base; > + > + dev_dbg(ctx->dev, "RCRB found for UID %lld: 0x%08llx\n", > + ctx->uid, (u64)rcrb); > + > + /* > + * RCRB's BAR[0..1] point to component block containing CXL > + * subsystem component registers. MEMBAR extraction follows > + * the PCI Base spec here, esp. 64 bit extraction and memory > + * ranges alignment (6.0, 7.5.1.2.1). > + */ > + addr = ioremap(rcrb, PCI_BASE_ADDRESS_0 + SZ_8); No failure check? This also only needs to map 4K at a time. > + bar0 = readl(addr + PCI_BASE_ADDRESS_0); > + bar1 = readl(addr + PCI_BASE_ADDRESS_1); > + iounmap(addr); > + > + /* sanity check */ > + if (bar0 & (PCI_BASE_ADDRESS_MEM_TYPE_1M | PCI_BASE_ADDRESS_SPACE_IO)) > + return CXL_RESOURCE_NONE; > + > + component_reg_phys = bar0 & PCI_BASE_ADDRESS_MEM_MASK; > + if (bar0 & PCI_BASE_ADDRESS_MEM_TYPE_64) > + component_reg_phys |= ((u64)bar1) << 32; > + > + if (!component_reg_phys) > + return CXL_RESOURCE_NONE; > + > + /* > + * Must be 8k aligned (size of combined CXL 1.1 Downstream and > + * Upstream Port RCRBs). > + */ > + if (component_reg_phys & (CXL_RCRB_SIZE - 1)) > + return CXL_RESOURCE_NONE; This is open-coding the IS_ALIGNED() macro. More importantly, why is it using RCRB size for the component register block alignment? The component lock is 64K, and at least for CXL 2.0 devices it is 64K aligned (8.1.9.1 Register Block Offset Low), so I am not sure what this check is for? --- Given that there are actual CXL RCH platforms in the wild I want this topic branch to be the first thing queued for v6.2. To help us coordinate I pushed: https://git.kernel.org/pub/scm/linux/kernel/git/cxl/cxl.git/log/?h=rch ...with the patches from this set accepted so far. You can use that as the baseline for the next spin.
On 20.10.22 22:17:07, Dan Williams wrote: > Robert Richter wrote: > > A downstream port must be connected to a component register block. > > For restricted hosts the base address is determined from the RCRB. The > > RCRB is provided by the host's CEDT CHBS entry. Rework CEDT parser to > > get the RCRB and add code to extract the component register block from > > it. > > > > RCRB's BAR[0..1] point to the component block containing CXL subsystem > > component registers. MEMBAR extraction follows the PCI base spec here, > > esp. 64 bit extraction and memory range alignment (6.0, 7.5.1.2.1). > > > > Note: Right now the component register block is used for HDM decoder > > capability only which is optional for RCDs. If unsupported by the RCD, > > the HDM init will fail. It is future work to bypass it in this case. > > > > Signed-off-by: Terry Bowman <terry.bowman@amd.com> > > Signed-off-by: Robert Richter <rrichter@amd.com> > > --- > > drivers/cxl/acpi.c | 79 ++++++++++++++++++++++++++++++++++++++++------ > > 1 file changed, 69 insertions(+), 10 deletions(-) > > > > diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c > > index fb9f72813067..a92d5d7b7a92 100644 > > --- a/drivers/cxl/acpi.c > > +++ b/drivers/cxl/acpi.c > > @@ -9,6 +9,8 @@ > > #include "cxlpci.h" > > #include "cxl.h" > > > > +#define CXL_RCRB_SIZE SZ_8K > > + > > static unsigned long cfmws_to_decoder_flags(int restrictions) > > { > > unsigned long flags = CXL_DECODER_F_ENABLE; > > @@ -229,27 +231,82 @@ static int add_host_bridge_uport(struct device *match, void *arg) > > struct cxl_chbs_context { > > struct device *dev; > > unsigned long long uid; > > - resource_size_t chbcr; > > + struct acpi_cedt_chbs chbs; > > }; > > > > -static int cxl_get_chbcr(union acpi_subtable_headers *header, void *arg, > > - const unsigned long end) > > +static int cxl_get_chbs(union acpi_subtable_headers *header, void *arg, > > + const unsigned long end) > > { > > struct cxl_chbs_context *ctx = arg; > > struct acpi_cedt_chbs *chbs; > > > > - if (ctx->chbcr) > > + if (ctx->chbs.base) > > return 0; > > > > chbs = (struct acpi_cedt_chbs *) header; > > > > if (ctx->uid != chbs->uid) > > return 0; > > - ctx->chbcr = chbs->base; > > + ctx->chbs = *chbs; > > > > return 0; > > } > > > > +static resource_size_t cxl_get_chbcr(struct cxl_chbs_context *ctx) > > +{ > > The core logic of this looks good, but this wants to be shared with the > upstream port component register discovery. > > Full disclosure I am reconciling these patches with an attempt that Dave > Jiang made at this topic. Since your series hit the list first I will > let it take the lead, but then fill it in with comments and learnings > from Dave's effort. > > So in this case Dave moved this into the drivers/cxl/core/regs.c with a > function signature like: > > enum cxl_rcrb { > CXL_RCRB_DOWNSTREAM, > CXL_RCRB_UPSTREAM, > }; > > resource_size_t cxl_rcrb_to_component(struct device *dev, > resource_size_t rcrb_base, int len, > enum cxl_rcrb which); > > ...where @which alternates when called by cxl_acpi for the downstream > case, or cxl_mem for the upstream case. Ok, I see where to go here. Could you point me to Dave's postings you are referring to? I checked linux-cxl and could not find anything related to RCRB or that changes regs.c. > > > > + struct acpi_cedt_chbs *chbs = &ctx->chbs; > > + resource_size_t component_reg_phys, rcrb; > > + u32 bar0, bar1; > > + void *addr; > > + > > + if (!chbs->base) > > + return CXL_RESOURCE_NONE; > > + > > + if (chbs->cxl_version != ACPI_CEDT_CHBS_VERSION_CXL11) > > + return chbs->base; > > + > > + /* Extract RCRB */ > > + > > + if (chbs->length != CXL_RCRB_SIZE) > > + return CXL_RESOURCE_NONE; > > + > > + rcrb = chbs->base; > > + > > + dev_dbg(ctx->dev, "RCRB found for UID %lld: 0x%08llx\n", > > + ctx->uid, (u64)rcrb); > > + > > + /* > > + * RCRB's BAR[0..1] point to component block containing CXL > > + * subsystem component registers. MEMBAR extraction follows > > + * the PCI Base spec here, esp. 64 bit extraction and memory > > + * ranges alignment (6.0, 7.5.1.2.1). > > + */ > > + addr = ioremap(rcrb, PCI_BASE_ADDRESS_0 + SZ_8); > > No failure check? This also only needs to map 4K at a time. Right, will add that. > > > + bar0 = readl(addr + PCI_BASE_ADDRESS_0); > > + bar1 = readl(addr + PCI_BASE_ADDRESS_1); > > + iounmap(addr); > > + > > + /* sanity check */ > > + if (bar0 & (PCI_BASE_ADDRESS_MEM_TYPE_1M | PCI_BASE_ADDRESS_SPACE_IO)) > > + return CXL_RESOURCE_NONE; > > + > > + component_reg_phys = bar0 & PCI_BASE_ADDRESS_MEM_MASK; > > + if (bar0 & PCI_BASE_ADDRESS_MEM_TYPE_64) > > + component_reg_phys |= ((u64)bar1) << 32; > > + > > + if (!component_reg_phys) > > + return CXL_RESOURCE_NONE; > > + > > + /* > > + * Must be 8k aligned (size of combined CXL 1.1 Downstream and > > + * Upstream Port RCRBs). > > + */ > > + if (component_reg_phys & (CXL_RCRB_SIZE - 1)) > > + return CXL_RESOURCE_NONE; > > This is open-coding the IS_ALIGNED() macro. More importantly, why is it > using RCRB size for the component register block alignment? The > component lock is 64K, and at least for CXL 2.0 devices it is 64K > aligned (8.1.9.1 Register Block Offset Low), so I am not sure what this > check is for? True, this is a mistake and needs to be corrected. It is the component reg range which is 64k. Will also use IS_ALIGNED(). > > --- > > Given that there are actual CXL RCH platforms in the wild I want this > topic branch to be the first thing queued for v6.2. To help us > coordinate I pushed: > > https://git.kernel.org/pub/scm/linux/kernel/git/cxl/cxl.git/log/?h=rch > > ...with the patches from this set accepted so far. You can use that as > the baseline for the next spin. Yes, thanks for that branch and applying the first part. -Robert
Robert Richter wrote: > On 20.10.22 22:17:07, Dan Williams wrote: > > Robert Richter wrote: > > > A downstream port must be connected to a component register block. > > > For restricted hosts the base address is determined from the RCRB. The > > > RCRB is provided by the host's CEDT CHBS entry. Rework CEDT parser to > > > get the RCRB and add code to extract the component register block from > > > it. > > > > > > RCRB's BAR[0..1] point to the component block containing CXL subsystem > > > component registers. MEMBAR extraction follows the PCI base spec here, > > > esp. 64 bit extraction and memory range alignment (6.0, 7.5.1.2.1). > > > > > > Note: Right now the component register block is used for HDM decoder > > > capability only which is optional for RCDs. If unsupported by the RCD, > > > the HDM init will fail. It is future work to bypass it in this case. > > > > > > Signed-off-by: Terry Bowman <terry.bowman@amd.com> > > > Signed-off-by: Robert Richter <rrichter@amd.com> > > > --- > > > drivers/cxl/acpi.c | 79 ++++++++++++++++++++++++++++++++++++++++------ > > > 1 file changed, 69 insertions(+), 10 deletions(-) > > > > > > diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c > > > index fb9f72813067..a92d5d7b7a92 100644 > > > --- a/drivers/cxl/acpi.c > > > +++ b/drivers/cxl/acpi.c > > > @@ -9,6 +9,8 @@ > > > #include "cxlpci.h" > > > #include "cxl.h" > > > > > > +#define CXL_RCRB_SIZE SZ_8K > > > + > > > static unsigned long cfmws_to_decoder_flags(int restrictions) > > > { > > > unsigned long flags = CXL_DECODER_F_ENABLE; > > > @@ -229,27 +231,82 @@ static int add_host_bridge_uport(struct device *match, void *arg) > > > struct cxl_chbs_context { > > > struct device *dev; > > > unsigned long long uid; > > > - resource_size_t chbcr; > > > + struct acpi_cedt_chbs chbs; > > > }; > > > > > > -static int cxl_get_chbcr(union acpi_subtable_headers *header, void *arg, > > > - const unsigned long end) > > > +static int cxl_get_chbs(union acpi_subtable_headers *header, void *arg, > > > + const unsigned long end) > > > { > > > struct cxl_chbs_context *ctx = arg; > > > struct acpi_cedt_chbs *chbs; > > > > > > - if (ctx->chbcr) > > > + if (ctx->chbs.base) > > > return 0; > > > > > > chbs = (struct acpi_cedt_chbs *) header; > > > > > > if (ctx->uid != chbs->uid) > > > return 0; > > > - ctx->chbcr = chbs->base; > > > + ctx->chbs = *chbs; > > > > > > return 0; > > > } > > > > > > +static resource_size_t cxl_get_chbcr(struct cxl_chbs_context *ctx) > > > +{ > > > > The core logic of this looks good, but this wants to be shared with the > > upstream port component register discovery. > > > > Full disclosure I am reconciling these patches with an attempt that Dave > > Jiang made at this topic. Since your series hit the list first I will > > let it take the lead, but then fill it in with comments and learnings > > from Dave's effort. > > > > So in this case Dave moved this into the drivers/cxl/core/regs.c with a > > function signature like: > > > > enum cxl_rcrb { > > CXL_RCRB_DOWNSTREAM, > > CXL_RCRB_UPSTREAM, > > }; > > > > resource_size_t cxl_rcrb_to_component(struct device *dev, > > resource_size_t rcrb_base, int len, > > enum cxl_rcrb which); > > > > ...where @which alternates when called by cxl_acpi for the downstream > > case, or cxl_mem for the upstream case. > > Ok, I see where to go here. Could you point me to Dave's postings you > are referring to? I checked linux-cxl and could not find anything > related to RCRB or that changes regs.c. He was in the middle of tidying them when you posted your series, but I think it would not hurt to push them to a git tree so you can grab the bits and pieces you want. Dave?
Dan Williams wrote: > Robert Richter wrote: > > On 20.10.22 22:17:07, Dan Williams wrote: > > > Robert Richter wrote: > > > > A downstream port must be connected to a component register block. > > > > For restricted hosts the base address is determined from the RCRB. The > > > > RCRB is provided by the host's CEDT CHBS entry. Rework CEDT parser to > > > > get the RCRB and add code to extract the component register block from > > > > it. > > > > > > > > RCRB's BAR[0..1] point to the component block containing CXL subsystem > > > > component registers. MEMBAR extraction follows the PCI base spec here, > > > > esp. 64 bit extraction and memory range alignment (6.0, 7.5.1.2.1). > > > > > > > > Note: Right now the component register block is used for HDM decoder > > > > capability only which is optional for RCDs. If unsupported by the RCD, > > > > the HDM init will fail. It is future work to bypass it in this case. > > > > > > > > Signed-off-by: Terry Bowman <terry.bowman@amd.com> > > > > Signed-off-by: Robert Richter <rrichter@amd.com> > > > > --- > > > > drivers/cxl/acpi.c | 79 ++++++++++++++++++++++++++++++++++++++++------ > > > > 1 file changed, 69 insertions(+), 10 deletions(-) > > > > > > > > diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c > > > > index fb9f72813067..a92d5d7b7a92 100644 > > > > --- a/drivers/cxl/acpi.c > > > > +++ b/drivers/cxl/acpi.c > > > > @@ -9,6 +9,8 @@ > > > > #include "cxlpci.h" > > > > #include "cxl.h" > > > > > > > > +#define CXL_RCRB_SIZE SZ_8K > > > > + > > > > static unsigned long cfmws_to_decoder_flags(int restrictions) > > > > { > > > > unsigned long flags = CXL_DECODER_F_ENABLE; > > > > @@ -229,27 +231,82 @@ static int add_host_bridge_uport(struct device *match, void *arg) > > > > struct cxl_chbs_context { > > > > struct device *dev; > > > > unsigned long long uid; > > > > - resource_size_t chbcr; > > > > + struct acpi_cedt_chbs chbs; > > > > }; > > > > > > > > -static int cxl_get_chbcr(union acpi_subtable_headers *header, void *arg, > > > > - const unsigned long end) > > > > +static int cxl_get_chbs(union acpi_subtable_headers *header, void *arg, > > > > + const unsigned long end) > > > > { > > > > struct cxl_chbs_context *ctx = arg; > > > > struct acpi_cedt_chbs *chbs; > > > > > > > > - if (ctx->chbcr) > > > > + if (ctx->chbs.base) > > > > return 0; > > > > > > > > chbs = (struct acpi_cedt_chbs *) header; > > > > > > > > if (ctx->uid != chbs->uid) > > > > return 0; > > > > - ctx->chbcr = chbs->base; > > > > + ctx->chbs = *chbs; > > > > > > > > return 0; > > > > } > > > > > > > > +static resource_size_t cxl_get_chbcr(struct cxl_chbs_context *ctx) > > > > +{ > > > > > > The core logic of this looks good, but this wants to be shared with the > > > upstream port component register discovery. > > > > > > Full disclosure I am reconciling these patches with an attempt that Dave > > > Jiang made at this topic. Since your series hit the list first I will > > > let it take the lead, but then fill it in with comments and learnings > > > from Dave's effort. > > > > > > So in this case Dave moved this into the drivers/cxl/core/regs.c with a > > > function signature like: > > > > > > enum cxl_rcrb { > > > CXL_RCRB_DOWNSTREAM, > > > CXL_RCRB_UPSTREAM, > > > }; > > > > > > resource_size_t cxl_rcrb_to_component(struct device *dev, > > > resource_size_t rcrb_base, int len, > > > enum cxl_rcrb which); > > > > > > ...where @which alternates when called by cxl_acpi for the downstream > > > case, or cxl_mem for the upstream case. > > > > Ok, I see where to go here. Could you point me to Dave's postings you > > are referring to? I checked linux-cxl and could not find anything > > related to RCRB or that changes regs.c. > > He was in the middle of tidying them when you posted your series, but I > think it would not hurt to push them to a git tree so you can grab the > bits and pieces you want. > > Dave? Looks like the list delivery is backed up, so I added Dave to the Cc:. He pushed: https://git.kernel.org/pub/scm/linux/kernel/git/djiang/linux.git/log/?h=cxl-rch ...which was his original attempt and: https://git.kernel.org/pub/scm/linux/kernel/git/djiang/linux.git/log/?h=cxl-rch-robert ...which was an attempt to rebase on top of your bits. The common RCRB mapping function is here: https://git.kernel.org/pub/scm/linux/kernel/git/djiang/linux.git/commit/?h=cxl-rch-robert&id=5be44cad37972517dae6a79001080ccfbdb67c49 I think the path forward is to build on that common RCRB code, fix cxl_acpi to register the pci host bridge device instead of the APCI device as the dport device, and then rely on a flag to skip over devm_enumerate_cxl_ports() in favor of just calling cxl_mem_find_port() directly in the RCIEP / RCH case. Then we can figure out what to do about RCDs that choose not to implement the HDM decoder capability which was forbidden in CXL 2.0, but now allowed in CXL 3.0.
On 24.10.22 16:23:39, Dave Jiang wrote: > On 10/24/2022 3:37 PM, Dan Williams wrote: > Dan Williams wrote: > Robert Richter wrote: > Ok, I see where to go here. Could you point me to Dave's postings you > are referring to? I checked linux-cxl and could not find anything > related to RCRB or that changes regs.c. > > He was in the middle of tidying them when you posted your series, but I > think it would not hurt to push them to a git tree so you can grab the > bits and pieces you want. > > Dave? > > Looks like the list delivery is backed up, so I added Dave to the Cc:. > > He pushed: > > https://git.kernel.org/pub/scm/linux/kernel/git/djiang/linux.git/log/?h=cxl-rch > > ...which was his original attempt and: > > https://git.kernel.org/pub/scm/linux/kernel/git/djiang/linux.git/log/?h=cxl-rch-robert > > ...which was an attempt to rebase on top of your bits. > > The common RCRB mapping function is here: > > https://git.kernel.org/pub/scm/linux/kernel/git/djiang/linux.git/commit/?h=cxl-rch-robert&id=5be44cad37972517dae6a79001080ccfbdb67c49 Thanks for the pointers. > > I think the path forward is to build on that common RCRB code, fix > cxl_acpi to register the pci host bridge device instead of the APCI > device as the dport device, and then rely on a flag to skip over > devm_enumerate_cxl_ports() in favor of just calling cxl_mem_find_port() > directly in the RCIEP / RCH case. Yes, we can completely skip devm_enumerate_cxl_ports() now. Though, I am not convinced on using the pci host bridge as dport_dev as RCD and non-RCD mode will diverge too much then. Looking into details here. > Then we can figure out what to do > about RCDs that choose not to implement the HDM decoder capability which > was forbidden in CXL 2.0, but now allowed in CXL 3.0. > > Hi Robert. As follow on to your work, I'm also working on reworking the hdm > decoder enumeration. I have this table from Dan where rr - range register exist > but not setup, RR - range register exist and setup, hdm - HDM decoder exist but > not programmed, HDM - HDM decoders exist and programmed. And I'm trying to > refactor the current code to cover all those scenarios: > > rr RR rr hdm rr HDM RR hdm RR HDM > D2 unsupported emulate RR enable HDM keep HDM enable HDM keep HDM > D1 unsupported emulate RR enable HDM keep HDM enable HDM keep HDM > > The current test device I have that's attached to RCH host, I'm seeing the RR > has setup a single range, but none of the HDM decoders are programmed. > Right, HDM decoder init need to be changed next. Thanks, -Robert
Robert Richter wrote: > On 24.10.22 16:23:39, Dave Jiang wrote: > > On 10/24/2022 3:37 PM, Dan Williams wrote: > > Dan Williams wrote: > > Robert Richter wrote: > > > Ok, I see where to go here. Could you point me to Dave's postings you > > are referring to? I checked linux-cxl and could not find anything > > related to RCRB or that changes regs.c. > > > > He was in the middle of tidying them when you posted your series, but I > > think it would not hurt to push them to a git tree so you can grab the > > bits and pieces you want. > > > > Dave? > > > > Looks like the list delivery is backed up, so I added Dave to the Cc:. > > > > He pushed: > > > > https://git.kernel.org/pub/scm/linux/kernel/git/djiang/linux.git/log/?h=cxl-rch > > > > ...which was his original attempt and: > > > > https://git.kernel.org/pub/scm/linux/kernel/git/djiang/linux.git/log/?h=cxl-rch-robert > > > > ...which was an attempt to rebase on top of your bits. > > > > The common RCRB mapping function is here: > > > > https://git.kernel.org/pub/scm/linux/kernel/git/djiang/linux.git/commit/?h=cxl-rch-robert&id=5be44cad37972517dae6a79001080ccfbdb67c49 > > Thanks for the pointers. > > > > > I think the path forward is to build on that common RCRB code, fix > > cxl_acpi to register the pci host bridge device instead of the APCI > > device as the dport device, and then rely on a flag to skip over > > devm_enumerate_cxl_ports() in favor of just calling cxl_mem_find_port() > > directly in the RCIEP / RCH case. > > Yes, we can completely skip devm_enumerate_cxl_ports() now. Though, I > am not convinced on using the pci host bridge as dport_dev as RCD and > non-RCD mode will diverge too much then. Looking into details here. Oh, I disagree with the initial implementation Dave had here. Both cases should be specifying the bridge device as the dport. That's a fixup that can go in now even without the RCD support. As it is the tooling needs to jump through the physical_node attribute to provide the useful information in cxl list: # cxl list -BTu -b ACPI.CXL { "bus":"root0", "provider":"ACPI.CXL", "nr_dports":1, "dports":[ { "dport":"ACPI0016:00", "alias":"pci0000:34", "id":"0x34" } ] } ...and I think that should just swap to this in all cases: # cxl list -BTu -b ACPI.CXL { "bus":"root0", "provider":"ACPI.CXL", "nr_dports":1, "dports":[ { "dport":"pci0000:34", "alias":"ACPI0016:00", "id":"0x34" } ] }
diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c index fb9f72813067..a92d5d7b7a92 100644 --- a/drivers/cxl/acpi.c +++ b/drivers/cxl/acpi.c @@ -9,6 +9,8 @@ #include "cxlpci.h" #include "cxl.h" +#define CXL_RCRB_SIZE SZ_8K + static unsigned long cfmws_to_decoder_flags(int restrictions) { unsigned long flags = CXL_DECODER_F_ENABLE; @@ -229,27 +231,82 @@ static int add_host_bridge_uport(struct device *match, void *arg) struct cxl_chbs_context { struct device *dev; unsigned long long uid; - resource_size_t chbcr; + struct acpi_cedt_chbs chbs; }; -static int cxl_get_chbcr(union acpi_subtable_headers *header, void *arg, - const unsigned long end) +static int cxl_get_chbs(union acpi_subtable_headers *header, void *arg, + const unsigned long end) { struct cxl_chbs_context *ctx = arg; struct acpi_cedt_chbs *chbs; - if (ctx->chbcr) + if (ctx->chbs.base) return 0; chbs = (struct acpi_cedt_chbs *) header; if (ctx->uid != chbs->uid) return 0; - ctx->chbcr = chbs->base; + ctx->chbs = *chbs; return 0; } +static resource_size_t cxl_get_chbcr(struct cxl_chbs_context *ctx) +{ + struct acpi_cedt_chbs *chbs = &ctx->chbs; + resource_size_t component_reg_phys, rcrb; + u32 bar0, bar1; + void *addr; + + if (!chbs->base) + return CXL_RESOURCE_NONE; + + if (chbs->cxl_version != ACPI_CEDT_CHBS_VERSION_CXL11) + return chbs->base; + + /* Extract RCRB */ + + if (chbs->length != CXL_RCRB_SIZE) + return CXL_RESOURCE_NONE; + + rcrb = chbs->base; + + dev_dbg(ctx->dev, "RCRB found for UID %lld: 0x%08llx\n", + ctx->uid, (u64)rcrb); + + /* + * RCRB's BAR[0..1] point to component block containing CXL + * subsystem component registers. MEMBAR extraction follows + * the PCI Base spec here, esp. 64 bit extraction and memory + * ranges alignment (6.0, 7.5.1.2.1). + */ + addr = ioremap(rcrb, PCI_BASE_ADDRESS_0 + SZ_8); + bar0 = readl(addr + PCI_BASE_ADDRESS_0); + bar1 = readl(addr + PCI_BASE_ADDRESS_1); + iounmap(addr); + + /* sanity check */ + if (bar0 & (PCI_BASE_ADDRESS_MEM_TYPE_1M | PCI_BASE_ADDRESS_SPACE_IO)) + return CXL_RESOURCE_NONE; + + component_reg_phys = bar0 & PCI_BASE_ADDRESS_MEM_MASK; + if (bar0 & PCI_BASE_ADDRESS_MEM_TYPE_64) + component_reg_phys |= ((u64)bar1) << 32; + + if (!component_reg_phys) + return CXL_RESOURCE_NONE; + + /* + * Must be 8k aligned (size of combined CXL 1.1 Downstream and + * Upstream Port RCRBs). + */ + if (component_reg_phys & (CXL_RCRB_SIZE - 1)) + return CXL_RESOURCE_NONE; + + return component_reg_phys; +} + static int add_host_bridge_dport(struct device *match, void *arg) { acpi_status status; @@ -259,6 +316,7 @@ static int add_host_bridge_dport(struct device *match, void *arg) struct cxl_port *root_port = arg; struct device *host = root_port->dev.parent; struct acpi_device *bridge = to_cxl_host_bridge(host, match); + resource_size_t component_reg_phys; if (!bridge) return 0; @@ -273,19 +331,20 @@ static int add_host_bridge_dport(struct device *match, void *arg) dev_dbg(match, "UID found: %lld\n", uid); ctx = (struct cxl_chbs_context) { - .dev = host, + .dev = match, .uid = uid, }; - acpi_table_parse_cedt(ACPI_CEDT_TYPE_CHBS, cxl_get_chbcr, &ctx); + acpi_table_parse_cedt(ACPI_CEDT_TYPE_CHBS, cxl_get_chbs, &ctx); - if (ctx.chbcr == 0) { + component_reg_phys = cxl_get_chbcr(&ctx); + if (component_reg_phys == CXL_RESOURCE_NONE) { dev_warn(match, "No CHBS found for Host Bridge (UID %lld)\n", uid); return 0; } - dev_dbg(match, "CHBCR found: 0x%08llx\n", (u64)ctx.chbcr); + dev_dbg(match, "CHBCR found: 0x%08llx\n", (u64)component_reg_phys); - dport = devm_cxl_add_dport(root_port, match, uid, ctx.chbcr); + dport = devm_cxl_add_dport(root_port, match, uid, component_reg_phys); if (IS_ERR(dport)) return PTR_ERR(dport);