From: Dan Williams <dan.j.williams@intel.com>
To: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: linux-cxl@vger.kernel.org,
Alison Schofield <alison.schofield@intel.com>,
Linux PCI <linux-pci@vger.kernel.org>,
Linux ACPI <linux-acpi@vger.kernel.org>
Subject: Re: [PATCH v5 2/6] cxl/acpi: Introduce cxl_root, the root of a cxl_port topology
Date: Tue, 8 Jun 2021 13:03:34 -0700 [thread overview]
Message-ID: <CAPcyv4h27dkh4FudzhXMQv8FGwY0b1qm1eDf-vDY6aH1kNZsag@mail.gmail.com> (raw)
In-Reply-To: <20210608122836.000018fc@Huawei.com>
On Tue, Jun 8, 2021 at 4:28 AM Jonathan Cameron
<Jonathan.Cameron@huawei.com> wrote:
>
> On Sat, 5 Jun 2021 23:05:04 -0700
> Dan Williams <dan.j.williams@intel.com> 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 <alison.schofield@intel.com>
> > Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
>
> Two trivial comments inline.
>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
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 <linux/platform_device.h>
> > +#include <linux/module.h>
> > +#include <linux/device.h>
> > +#include <linux/kernel.h>
> > +#include <linux/acpi.h>
> > +#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 <linux/device.h>
> > #include <linux/module.h>
> > #include <linux/pci.h>
> > +#include <linux/slab.h>
> > +#include <linux/idr.h>
> > #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.
next prev parent reply other threads:[~2021-06-08 20:03 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-06 6:04 [PATCH v5 0/6] CXL port and decoder enumeration Dan Williams
2021-06-06 6:04 ` [PATCH v5 1/6] cxl/acpi: Local definition of ACPICA infrastructure Dan Williams
2021-06-07 12:25 ` Rafael J. Wysocki
2021-06-07 17:03 ` Dan Williams
2021-06-08 18:13 ` Dan Williams
2021-06-08 19:29 ` Rafael J. Wysocki
2021-06-06 6:05 ` [PATCH v5 2/6] cxl/acpi: Introduce cxl_root, the root of a cxl_port topology Dan Williams
2021-06-07 12:27 ` Rafael J. Wysocki
2021-06-07 22:18 ` Ben Widawsky
2021-06-08 11:28 ` Jonathan Cameron
2021-06-08 20:03 ` Dan Williams [this message]
2021-06-06 6:05 ` [PATCH v5 3/6] cxl/Kconfig: Default drivers to CONFIG_CXL_BUS Dan Williams
2021-06-06 6:05 ` [PATCH v5 4/6] cxl/acpi: Add downstream port data to cxl_port instances Dan Williams
2021-06-08 11:49 ` Jonathan Cameron
2021-06-08 23:58 ` Dan Williams
2021-06-09 11:28 ` Jonathan Cameron
2021-06-09 15:15 ` Dan Williams
2021-06-06 6:05 ` [PATCH v5 5/6] cxl/acpi: Enumerate host bridge root ports Dan Williams
2021-06-08 12:42 ` Jonathan Cameron
2021-06-06 6:05 ` [PATCH v5 6/6] cxl/acpi: Introduce cxl_decoder objects Dan Williams
2021-06-08 13:06 ` Jonathan Cameron
2021-06-08 23:48 ` Dan Williams
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAPcyv4h27dkh4FudzhXMQv8FGwY0b1qm1eDf-vDY6aH1kNZsag@mail.gmail.com \
--to=dan.j.williams@intel.com \
--cc=Jonathan.Cameron@huawei.com \
--cc=alison.schofield@intel.com \
--cc=linux-acpi@vger.kernel.org \
--cc=linux-cxl@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).