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 X-Spam-Level: X-Spam-Status: No, score=-13.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id DB079C47082 for ; Tue, 8 Jun 2021 20:04:47 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id AA10B613BD for ; Tue, 8 Jun 2021 20:04:47 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234171AbhFHUGk (ORCPT ); Tue, 8 Jun 2021 16:06:40 -0400 Received: from mail-pj1-f53.google.com ([209.85.216.53]:53810 "EHLO mail-pj1-f53.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234071AbhFHUGj (ORCPT ); Tue, 8 Jun 2021 16:06:39 -0400 Received: by mail-pj1-f53.google.com with SMTP id ei4so15456pjb.3 for ; Tue, 08 Jun 2021 13:04:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=intel-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=nhI0uZdD+bqC6HZkMksTTcPTgmv+oAlfDoNIYsPwNHQ=; b=JMNPFpGGS5lh/0K8ov8txZDzpZm+eKmL3lxLGosvfpE/O6MkoZ6FcjeMZnzhdxqTJ5 yklstTPDnA/gjDJg5+2AtiQxhNsUF/YsbmgKqRO65xaqvxAw7rGv5m7p1tDkB4T7fi07 HHoaruGFdVR7uFuX7pAgrLBxePNy4CVWFNPzGIZ8aSLmFgJGuYKrBYJgYHP8TcBLl7hy 2rKxLz31QknjybMuV53BdOy0+z9CDO3xXtiIKOO2ZsUR1h7U0j37R9U3barg8Sgsh6Ii Hcvnsja6vN/uz6rfuDbCNVGGgJNBKx4C6873XxqefJNIvcSW1GFV6QeojWLsX6FgVnaz VKLg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=nhI0uZdD+bqC6HZkMksTTcPTgmv+oAlfDoNIYsPwNHQ=; b=D+FOdWZ+OCbMAazPpDZFDm/T0cwPWLydRqQIVnOVyZ6R9jpbVu4IKLcLdq3V0aw+Dz zLKUlPXXGkatz3XI1ORwLJwMFZ2+OIf8XNZ01e3rojSzFBbW5rGCbKelZMuj5IrOpYs5 t4sBV3dpu9jDVfAK+USnqNm6e1b5Pu47DrtavHPYX/YKjGfbMEhaS1Buw5I79uWWjFxb 2ywlOGrTVQS9od2Ii/BFbSfQ6LLnc9BmJdlJ8yJMXLRgQN08d2xRAT0X3/lwLWDc2M1w vTbHIMZ4spM7JP/I+sdv/CD/qOosgRXQDfeL0ulz8RLx9G8cAjJQ2mJ8YqFgw9vSj0Ph Gb4A== X-Gm-Message-State: AOAM531e6yyjEuLMY7uVR8xs4T+kGI+JAKi21JH+toycOyksKHbFMW3I 4enTHnxz6fHgkR10I5p8xQunALIla5HDcUCRvhZC20ukADWu9Q== X-Google-Smtp-Source: ABdhPJzPDn7AoKen5PiDE/efuS/RFGNeIk19LM9QLlTquxcqqZoV1WpmhMPseVBYhHaQYA6G6aRuVOlcBz5lQgwLpVw= X-Received: by 2002:a17:90a:fc88:: with SMTP id ci8mr6734466pjb.13.1623182625985; Tue, 08 Jun 2021 13:03:45 -0700 (PDT) MIME-Version: 1.0 References: <162295949351.1109360.10329014558746500142.stgit@dwillia2-desk3.amr.corp.intel.com> <162295950449.1109360.5228772194963187441.stgit@dwillia2-desk3.amr.corp.intel.com> <20210608122836.000018fc@Huawei.com> In-Reply-To: <20210608122836.000018fc@Huawei.com> From: Dan Williams Date: Tue, 8 Jun 2021 13:03:34 -0700 Message-ID: Subject: Re: [PATCH v5 2/6] cxl/acpi: Introduce cxl_root, the root of a cxl_port topology To: Jonathan Cameron Cc: linux-cxl@vger.kernel.org, Alison Schofield , Linux PCI , Linux ACPI Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-cxl@vger.kernel.org On Tue, Jun 8, 2021 at 4:28 AM Jonathan Cameron wrote: > > On Sat, 5 Jun 2021 23:05:04 -0700 > Dan Williams wrote: > > > While CXL builds upon the PCI software model for enumeration and > > endpoint control, a static platform component is required to bootstrap > > the CXL memory layout. Similar to how ACPI identifies root-level PCI > > memory resources the ACPI identifies the address space and interleave > > configuration for CXL Memory. > > > > In addition to identifying the host bridges, ACPI is responsible for > > enumerating the CXL memory space that can be addressed by downstream > > decoders. This is similar to the requirement for ACPI to publish > > resources reported by _CRS for PCI host bridges. Specifically ACPI > > publishes a table, CXL Early Discovery Table (CEDT), which includes a > > list of CXL Memory resource, CXL Fixed Memory Window Structures (CFMWS). > > > > For now introduce the core infrastructure for a cxl_port hierarchy > > starting with a root level anchor represented by the ACPI0017 device. > > > > Follow on changes model support for the configurable decode capabilities > > of cxl_port instances. > > > > Co-developed-by: Alison Schofield > > Signed-off-by: Alison Schofield > > Signed-off-by: Dan Williams > > Two trivial comments inline. > > Reviewed-by: Jonathan Cameron > Thanks, as always, for taking a look. > > --- > > Documentation/ABI/testing/sysfs-bus-cxl | 11 ++ > > drivers/cxl/Kconfig | 15 +++ > > drivers/cxl/Makefile | 2 > > drivers/cxl/acpi.c | 39 ++++++++ > > drivers/cxl/core.c | 160 +++++++++++++++++++++++++++++++ > > drivers/cxl/cxl.h | 23 ++++ > > 6 files changed, 250 insertions(+) > > create mode 100644 drivers/cxl/acpi.c > > > > diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl > > index 2fe7490ad6a8..fb996ced7629 100644 > > --- a/Documentation/ABI/testing/sysfs-bus-cxl > > +++ b/Documentation/ABI/testing/sysfs-bus-cxl > > @@ -24,3 +24,14 @@ Description: > > (RO) "Persistent Only Capacity" as bytes. Represents the > > identically named field in the Identify Memory Device Output > > Payload in the CXL-2.0 specification. > > + > > +What: /sys/bus/cxl/devices/portX/uport > > +Date: May, 2021 > > +KernelVersion: v5.14 > > +Contact: linux-cxl@vger.kernel.org > > +Description: > > + CXL port objects are enumerated from either a platform firmware > > + device (ACPI0017 and ACPI0016) or PCIe switch upstream port with > > + CXL component registers. The 'uport' symlink connects the CXL > > + portX object to the device that published the CXL port > > + capability. > > Is this a complete list of ABI added? Looks like we also add devtype > attribute in this series. devtype is the only missing one, I'll add it. > Mind you I just realized I didn't document the proposed CDAT file either yet. Hey, we all need prompts to add documentation. I only remembered to add these here because you commented on it last time. > > diff --git a/drivers/cxl/Kconfig b/drivers/cxl/Kconfig > > index 5483ba92b6da..d2573f6aef91 100644 > > --- a/drivers/cxl/Kconfig > > +++ b/drivers/cxl/Kconfig > > @@ -45,4 +45,19 @@ config CXL_MEM_RAW_COMMANDS > > potential impact to memory currently in use by the kernel. > > > > If developing CXL hardware or the driver say Y, otherwise say N. > > + > > +config CXL_ACPI > > + tristate "CXL ACPI: Platform Support" > > + depends on ACPI > > + help > > + Enable support for host managed device memory (HDM) resources > > + published by a platform's ACPI CXL memory layout description. See > > + Chapter 9.14.1 CXL Early Discovery Table (CEDT) in the CXL 2.0 > > + specification, and CXL Fixed Memory Window Structures (CEDT.CFMWS) > > + (https://www.computeexpresslink.org/spec-landing). The CXL core > > + consumes these resource to publish the root of a cxl_port decode > > + hierarchy to map regions that represent System RAM, or Persistent > > + Memory regions to be managed by LIBNVDIMM. > > + > > + If unsure say 'm'. > > endif > > diff --git a/drivers/cxl/Makefile b/drivers/cxl/Makefile > > index d9d282dc15be..a29efb3e8ad2 100644 > > --- a/drivers/cxl/Makefile > > +++ b/drivers/cxl/Makefile > > @@ -1,7 +1,9 @@ > > # SPDX-License-Identifier: GPL-2.0 > > obj-$(CONFIG_CXL_BUS) += cxl_core.o > > obj-$(CONFIG_CXL_MEM) += cxl_pci.o > > +obj-$(CONFIG_CXL_ACPI) += cxl_acpi.o > > > > ccflags-y += -DDEFAULT_SYMBOL_NAMESPACE=CXL > > cxl_core-y := core.o > > cxl_pci-y := pci.o > > +cxl_acpi-y := acpi.o > > diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c > > new file mode 100644 > > index 000000000000..556d25ab6966 > > --- /dev/null > > +++ b/drivers/cxl/acpi.c > > @@ -0,0 +1,39 @@ > > +// SPDX-License-Identifier: GPL-2.0-only > > +/* Copyright(c) 2021 Intel Corporation. All rights reserved. */ > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include "cxl.h" > > + > > +static int cxl_acpi_probe(struct platform_device *pdev) > > +{ > > + struct cxl_port *root_port; > > + struct device *host = &pdev->dev; > > + > > + root_port = devm_cxl_add_port(host, host, CXL_RESOURCE_NONE, NULL); > > + if (IS_ERR(root_port)) > > + return PTR_ERR(root_port); > > + dev_dbg(host, "add: %s\n", dev_name(&root_port->dev)); > > + > > + return 0; > > +} > > + > > +static const struct acpi_device_id cxl_acpi_ids[] = { > > + { "ACPI0017", 0 }, > > + { "", 0 }, > > +}; > > +MODULE_DEVICE_TABLE(acpi, cxl_acpi_ids); > > + > > +static struct platform_driver cxl_acpi_driver = { > > + .probe = cxl_acpi_probe, > > + .driver = { > > + .name = KBUILD_MODNAME, > > + .acpi_match_table = cxl_acpi_ids, > > + }, > > +}; > > + > > +module_platform_driver(cxl_acpi_driver); > > +MODULE_LICENSE("GPL v2"); > > +MODULE_IMPORT_NS(CXL); > > diff --git a/drivers/cxl/core.c b/drivers/cxl/core.c > > index 853666d8a9f5..dbbb34618d7d 100644 > > --- a/drivers/cxl/core.c > > +++ b/drivers/cxl/core.c > > @@ -4,6 +4,8 @@ > > #include > > #include > > #include > > +#include > > +#include > > #include "cxl.h" > > > > /** > > @@ -13,6 +15,164 @@ > > * point for cross-device interleave coordination through cxl ports. > > */ > > > > +static DEFINE_IDA(cxl_port_ida); > > + > > +static ssize_t devtype_show(struct device *dev, struct device_attribute *attr, > > + char *buf) > > +{ > > + return sysfs_emit(buf, "%s\n", dev->type->name); > > I guess it's really small so doesn't matter that much, but not so nice > that we are gaining multiple instances of this same function. > This is only the second one unless I'm missing something. I do think it might be worth placing a common definition in the driver-core. Although it might be the case that I'm the only one that thinks having devtype as both an attribute and a uevent variable (DEVTYPE) is worthwhile. All the other variables in a dev_uevent() appear somewhere in sysfs, so that's a decent argument in my opinion to make devtype a core attribute.