From: Dan Williams <dan.j.williams@intel.com>
To: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: Ben Widawsky <ben.widawsky@intel.com>,
linux-cxl@vger.kernel.org,
Vishal Verma <vishal.l.verma@intel.com>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Linux PCI <linux-pci@vger.kernel.org>,
"linux-acpi@vger.kernel.org, Ira Weiny" <ira.weiny@intel.com>,
"Kelley, Sean V" <sean.v.kelley@intel.com>,
Rafael Wysocki <rafael.j.wysocki@intel.com>,
Bjorn Helgaas <helgaas@kernel.org>,
Jon Masters <jcm@jonmasters.org>,
Chris Browy <cbrowy@avery-design.com>,
Randy Dunlap <rdunlap@infradead.org>,
Christoph Hellwig <hch@infradead.org>,
daniel.lll@alibaba-inc.com, "Moore,
Robert" <robert.moore@intel.com>,
"Kaneda, Erik" <erik.kaneda@intel.com>
Subject: Re: [RFC PATCH v3 02/16] cxl/acpi: Add an acpi_cxl module for the CXL interconnect
Date: Tue, 12 Jan 2021 11:43:31 -0800 [thread overview]
Message-ID: <CAPcyv4hcppMZ2L8W8arUKmbCo0r=_yZggrnsj3w-Jgszjn=ZoA@mail.gmail.com> (raw)
In-Reply-To: <20210112184355.00007632@Huawei.com>
On Tue, Jan 12, 2021 at 10:44 AM Jonathan Cameron
<Jonathan.Cameron@huawei.com> wrote:
>
> On Mon, 11 Jan 2021 14:51:06 -0800
> Ben Widawsky <ben.widawsky@intel.com> wrote:
>
> > From: Vishal Verma <vishal.l.verma@intel.com>
> >
> > Add an acpi_cxl module to coordinate the ACPI portions of the CXL
> > (Compute eXpress Link) interconnect. This driver binds to ACPI0017
> > objects in the ACPI tree, and coordinates access to the resources
> > provided by the ACPI CEDT (CXL Early Discovery Table).
> >
> > It also coordinates operations of the root port _OSC object to notify
> > platform firmware that the OS has native support for the CXL
> > capabilities of endpoints.
> >
> > Note: the actbl1.h changes are speculative. The expectation is that they
> > will arrive through the ACPICA tree in due time.
>
> I would pull the ACPICA changes out into a precursor patch.
>
> >
> > Cc: Ben Widawsky <ben.widawsky@intel.com>
> > Cc: Dan Williams <dan.j.williams@intel.com>
> > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
> > Signed-off-by: Ben Widawsky <ben.widawsky@intel.com>
>
> Hi,
>
> I think it would be good to also add CEDT to the list in drivers/acpi/tables.c
> so that we can dump it from /sys/firmware/acpi/tables/ and potentially
> override it from an initrd.
ACPICA changes will eventually come through the ACPI tree not this patch set.
>
> https://elixir.bootlin.com/linux/v5.11-rc3/source/drivers/acpi/tables.c#L482
> Can be very helpful whilst debugging. Related to that, anyone know if anyone
> has acpica patches so we can have iasl -d work on the table? Would probably
> be useful but I'd rather not duplicate work if it's already done.
>
The supplemental tables described here:
https://www.uefi.org/acpi
...do eventually make there way into ACPICA. Added Bob and Erik in
case they can comment on when CEDT and CDAT support will be picked up.
> A few minor things inline
>
> Jonathan
>
> > ---
> > drivers/Kconfig | 1 +
> > drivers/Makefile | 1 +
> > drivers/cxl/Kconfig | 36 ++++++++++++++++
> > drivers/cxl/Makefile | 5 +++
> > drivers/cxl/acpi.c | 97 +++++++++++++++++++++++++++++++++++++++++++
> > drivers/cxl/acpi.h | 15 +++++++
> > include/acpi/actbl1.h | 50 ++++++++++++++++++++++
> > 7 files changed, 205 insertions(+)
> > create mode 100644 drivers/cxl/Kconfig
> > create mode 100644 drivers/cxl/Makefile
> > create mode 100644 drivers/cxl/acpi.c
> > create mode 100644 drivers/cxl/acpi.h
> >
> > diff --git a/drivers/Kconfig b/drivers/Kconfig
> > index dcecc9f6e33f..62c753a73651 100644
> > --- a/drivers/Kconfig
> > +++ b/drivers/Kconfig
> > @@ -6,6 +6,7 @@ menu "Device Drivers"
> > source "drivers/amba/Kconfig"
> > source "drivers/eisa/Kconfig"
> > source "drivers/pci/Kconfig"
> > +source "drivers/cxl/Kconfig"
> > source "drivers/pcmcia/Kconfig"
> > source "drivers/rapidio/Kconfig"
> >
> > diff --git a/drivers/Makefile b/drivers/Makefile
> > index fd11b9ac4cc3..678ea810410f 100644
> > --- a/drivers/Makefile
> > +++ b/drivers/Makefile
> > @@ -73,6 +73,7 @@ obj-$(CONFIG_NVM) += lightnvm/
> > obj-y += base/ block/ misc/ mfd/ nfc/
> > obj-$(CONFIG_LIBNVDIMM) += nvdimm/
> > obj-$(CONFIG_DAX) += dax/
> > +obj-$(CONFIG_CXL_BUS) += cxl/
> > obj-$(CONFIG_DMA_SHARED_BUFFER) += dma-buf/
> > obj-$(CONFIG_NUBUS) += nubus/
> > obj-y += macintosh/
> > diff --git a/drivers/cxl/Kconfig b/drivers/cxl/Kconfig
> > new file mode 100644
> > index 000000000000..68da926ba5b1
> > --- /dev/null
> > +++ b/drivers/cxl/Kconfig
> > @@ -0,0 +1,36 @@
> > +# SPDX-License-Identifier: GPL-2.0-only
> > +menuconfig CXL_BUS
> > + tristate "CXL (Compute Express Link) Devices Support"
> > + help
> > + CXL is a bus that is electrically compatible with PCI-E, but layers
>
> For consistency with Kconfig in driver/pci probably better to spell PCI Express
> throughout.
Sure.
>
> > + three protocols on that signalling (CXL.io, CXL.cache, and CXL.mem).
> > + The CXL.cache protocol allows devices to hold cachelines locally, the
> > + CXL.mem protocol allows devices to be fully coherent memory targets,
> > + the CXL.io protocol is equivalent to PCI-E. Say 'y' to enable support
> > + for the configuration and management of devices supporting these
> > + protocols.
> > +
> > +if CXL_BUS
> > +
> > +config CXL_BUS_PROVIDER
> > + tristate
> > +
> > +config CXL_ACPI
> > + tristate "CXL ACPI: Platform Support"
> > + depends on ACPI
> > + default CXL_BUS
> > + select CXL_BUS_PROVIDER
> > + help
> > + Say 'y/m' to enable a driver (named "cxl_acpi.ko" when built
> > + as a module) that will enable support for CXL.mem endpoint
> > + devices. In general, CXL Platform Support is a prerequisite
> > + for any CXL device driver that wants to claim ownership of a
> > + component register space. By default platform firmware assumes
> > + Linux is unaware of CXL capabilities and requires explicit
> > + opt-in. This platform component also mediates resources
> > + described by the CEDT (CXL Early Discovery Table). See
> > + Chapter 9.14.1 CXL Early Discovery Table (CEDT) in the CXL 2.0
> > + specification.
> > +
> > + If unsure say 'm'
> > +endif
> > diff --git a/drivers/cxl/Makefile b/drivers/cxl/Makefile
> > new file mode 100644
> > index 000000000000..d38cd34a2582
> > --- /dev/null
> > +++ b/drivers/cxl/Makefile
> > @@ -0,0 +1,5 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +obj-$(CONFIG_CXL_ACPI) += cxl_acpi.o
> > +
> > +ccflags-y += -DDEFAULT_SYMBOL_NAMESPACE=CXL
> > +cxl_acpi-y := acpi.o
> > diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
> > new file mode 100644
> > index 000000000000..0f1ba9b3f1ed
> > --- /dev/null
> > +++ b/drivers/cxl/acpi.c
> > @@ -0,0 +1,97 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/* Copyright(c) 2020 Intel Corporation. All rights reserved. */
> > +#include <linux/platform_device.h>
> > +#include <linux/list_sort.h>
> > +#include <linux/module.h>
> > +#include <linux/mutex.h>
> > +#include <linux/sysfs.h>
> > +#include <linux/list.h>
> > +#include <linux/acpi.h>
> > +#include <linux/sort.h>
> > +#include <linux/pci.h>
>
> Trivial: For includes I'm fairly sure general convention (as much as there
> is one is to do alphabetical order, not reverse xmas tree).
No, there's no guidance for this in coding-style so it's one of those
maintainer preference items.
>
> > +#include "acpi.h"
> > +
> > +/*
> > + * If/when CXL support is defined by other platform firmware the kernel
> > + * will need a mechanism to select between the platform specific version
> > + * of this routine, until then, hard-code ACPI assumptions
> > + */
> > +int cxl_bus_acquire(struct pci_dev *pdev)
> > +{
> > + struct acpi_device *adev;
> > + struct pci_dev *root_port;
> > + struct device *root;
> > +
> > + root_port = pcie_find_root_port(pdev);
> > + if (!root_port)
> > + return -ENXIO;
> > +
> > + root = root_port->dev.parent;
> > + if (!root)
> > + return -ENXIO;
> > +
> > + adev = ACPI_COMPANION(root);
> > + if (!adev)
> > + return -ENXIO;
> > +
> > + /* TODO: OSC enabling */
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(cxl_bus_acquire);
> > +
> > +static void acpi_cedt_put_table(void *table)
> > +{
> > + acpi_put_table(table);
> > +}
> > +
> > +static int cxl_acpi_probe(struct platform_device *pdev)
> > +{
> > + struct acpi_device *adev = ACPI_COMPANION(&pdev->dev);
> > + struct cxl_acpi_desc *acpi_desc;
> > + struct device *dev = &adev->dev;
> > + struct acpi_table_header *tbl;
> > + acpi_status status;
> > + acpi_size sz;
> > + int rc;
> > +
> > + status = acpi_get_table(ACPI_SIG_CEDT, 0, &tbl);
> > + if (ACPI_FAILURE(status)) {
> > + dev_err(dev, "failed to find CEDT at startup\n");
> > + return 0;
> > + }
> > +
> > + rc = devm_add_action_or_reset(dev, acpi_cedt_put_table, tbl);
> > + if (rc)
> > + return rc;
> > +
> > + sz = tbl->length;
> > + dev_info(dev, "found CEDT at startup: %lld bytes\n", sz);
>
> Is this useful? At least for my normal acpi boot I'll see CEDT in the list of
> detected ACPI tables early in boot. This provides less info than that
> print already has.
Agree.
next prev parent reply other threads:[~2021-01-12 19:44 UTC|newest]
Thread overview: 61+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-11 22:51 [RFC PATCH v3 00/16] CXL 2.0 Support Ben Widawsky
2021-01-11 22:51 ` [RFC PATCH v3 01/16] docs: cxl: Add basic documentation Ben Widawsky
2021-01-11 22:51 ` [RFC PATCH v3 02/16] cxl/acpi: Add an acpi_cxl module for the CXL interconnect Ben Widawsky
2021-01-12 7:08 ` Randy Dunlap
2021-01-12 18:43 ` Jonathan Cameron
2021-01-12 19:43 ` Dan Williams [this message]
2021-01-12 22:06 ` Jonathan Cameron
2021-01-13 17:55 ` Kaneda, Erik
2021-01-20 19:27 ` Dan Williams
2021-01-20 19:18 ` Verma, Vishal L
2021-01-13 12:40 ` Rafael J. Wysocki
2021-01-20 19:21 ` Verma, Vishal L
2021-01-11 22:51 ` [RFC PATCH v3 03/16] cxl/acpi: add OSC support Ben Widawsky
2021-01-12 15:09 ` Rafael J. Wysocki
2021-01-12 18:48 ` Jonathan Cameron
2021-01-11 22:51 ` [RFC PATCH v3 04/16] cxl/mem: Introduce a driver for CXL-2.0-Type-3 endpoints Ben Widawsky
2021-01-12 7:08 ` Randy Dunlap
2021-01-12 19:01 ` Jonathan Cameron
2021-01-12 20:06 ` Dan Williams
2021-01-11 22:51 ` [RFC PATCH v3 05/16] cxl/mem: Map memory device registers Ben Widawsky
2021-01-12 19:13 ` Jonathan Cameron
2021-01-12 19:21 ` Ben Widawsky
2021-01-12 20:40 ` Dan Williams
2021-01-11 22:51 ` [RFC PATCH v3 06/16] cxl/mem: Find device capabilities Ben Widawsky
2021-01-12 19:17 ` Jonathan Cameron
2021-01-12 19:22 ` Ben Widawsky
2021-01-11 22:51 ` [RFC PATCH v3 07/16] cxl/mem: Implement polled mode mailbox Ben Widawsky
2021-01-13 18:26 ` Jonathan Cameron
2021-01-14 17:40 ` Jonathan Cameron
2021-01-14 17:50 ` Ben Widawsky
2021-01-14 18:13 ` Jonathan Cameron
2021-01-11 22:51 ` [RFC PATCH v3 08/16] cxl/mem: Register CXL memX devices Ben Widawsky
2021-01-14 16:28 ` Jonathan Cameron
2021-01-11 22:51 ` [RFC PATCH v3 09/16] cxl/mem: Add basic IOCTL interface Ben Widawsky
2021-01-14 16:19 ` Jonathan Cameron
2021-01-11 22:51 ` [RFC PATCH v3 10/16] cxl/mem: Add send command Ben Widawsky
2021-01-14 17:10 ` Jonathan Cameron
2021-01-21 18:15 ` Ben Widawsky
2021-01-22 11:43 ` Jonathan Cameron
2021-01-22 17:08 ` Ben Widawsky
2021-01-11 22:51 ` [RFC PATCH v3 11/16] taint: add taint for direct hardware access Ben Widawsky
2021-01-11 22:51 ` [RFC PATCH v3 11/16] taint: add taint for unfettered " Ben Widawsky
2021-01-12 3:31 ` Ben Widawsky
2021-01-11 22:51 ` [RFC PATCH v3 12/16] cxl/mem: Add a "RAW" send command Ben Widawsky
2021-01-11 22:51 ` [RFC PATCH v3 13/16] cxl/mem: Create concept of enabled commands Ben Widawsky
2021-01-14 17:25 ` Jonathan Cameron
2021-01-21 18:40 ` Ben Widawsky
2021-01-22 11:28 ` Jonathan Cameron
2021-01-11 22:51 ` [RFC PATCH v3 14/16] cxl/mem: Use CEL for enabling commands Ben Widawsky
2021-01-14 18:02 ` Jonathan Cameron
2021-01-14 18:13 ` Ben Widawsky
2021-01-14 18:32 ` Jonathan Cameron
2021-01-14 19:04 ` Ben Widawsky
2021-01-14 19:24 ` Jonathan Cameron
2021-01-11 22:51 ` [RFC PATCH v3 15/16] cxl/mem: Add limited Get Log command (0401h) Ben Widawsky
2021-01-14 18:08 ` Jonathan Cameron
2021-01-23 0:14 ` Ben Widawsky
2021-01-11 22:51 ` [RFC PATCH v3 16/16] MAINTAINERS: Add maintainers of the CXL driver Ben Widawsky
2021-01-12 1:12 ` Joe Perches
[not found] ` <0f2a6d62-09d8-416f-e972-3e9869c3e1a6@alibaba-inc.com>
2021-01-12 15:17 ` [RFC PATCH v3 00/16] CXL 2.0 Support Ben Widawsky
2021-01-12 16:19 ` Bjorn Helgaas
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='CAPcyv4hcppMZ2L8W8arUKmbCo0r=_yZggrnsj3w-Jgszjn=ZoA@mail.gmail.com' \
--to=dan.j.williams@intel.com \
--cc=Jonathan.Cameron@huawei.com \
--cc=ben.widawsky@intel.com \
--cc=cbrowy@avery-design.com \
--cc=daniel.lll@alibaba-inc.com \
--cc=erik.kaneda@intel.com \
--cc=hch@infradead.org \
--cc=helgaas@kernel.org \
--cc=ira.weiny@intel.com \
--cc=jcm@jonmasters.org \
--cc=linux-cxl@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=rafael.j.wysocki@intel.com \
--cc=rdunlap@infradead.org \
--cc=robert.moore@intel.com \
--cc=sean.v.kelley@intel.com \
--cc=vishal.l.verma@intel.com \
/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).