| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 22 Jun 2023 15:36:09 -0700
From: Dave Jiang <dave.jiang@...el.com>
To: Terry Bowman <terry.bowman@....com>, <alison.schofield@...el.com>,
<vishal.l.verma@...el.com>, <ira.weiny@...el.com>,
<bwidawsk@...nel.org>, <dan.j.williams@...el.com>,
<Jonathan.Cameron@...wei.com>, <linux-cxl@...r.kernel.org>
CC: <rrichter@....com>, <linux-kernel@...r.kernel.org>,
<bhelgaas@...gle.com>
Subject: Re: [PATCH v6 02/27] cxl/acpi: Probe RCRB later during RCH downstream
port creation
On 6/21/23 20:51, Terry Bowman wrote:
> From: Robert Richter <rrichter@....com>
>
> The RCRB is extracted already during ACPI CEDT table parsing while the
> data of this is needed not earlier than dport creation. This
> implementation comes with drawbacks: During ACPI table scan there is
> already MMIO access including mapping and unmapping, but only ACPI
> data should be collected here. The collected data must be transferred
> through a couple of interfaces until it is finally consumed when
> creating the dport. This causes complex data structures and function
> interfaces. Additionally, RCRB parsing will be extended to also
> extract AER data, it would be much easier do this at a later point
> during port and dport creation when the data structures are available
> to hold that data.
>
> To simplify all that, probe the RCRB at a later point during RCH
> downstream port creation. Change ACPI table parser to only extract the
> base address of either the component registers or the RCRB. Parse and
> extract the RCRB in devm_cxl_add_rch_dport().
>
> This is in preparation to centralize all RCRB scanning.
>
> Signed-off-by: Robert Richter <rrichter@....com>
> Signed-off-by: Terry Bowman <terry.bowman@....com>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@...wei.com>
Reviewed-by: Dave Jiang <dave.jiang@...el.com>
> ---
> drivers/cxl/acpi.c | 51 +++++++++++++++++------------------------
> drivers/cxl/core/port.c | 21 +++++++++++++----
> drivers/cxl/cxl.h | 1 -
> 3 files changed, 37 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
> index 7e1765b09e04..70cd9ac73a8b 100644
> --- a/drivers/cxl/acpi.c
> +++ b/drivers/cxl/acpi.c
> @@ -372,21 +372,21 @@ static int add_host_bridge_uport(struct device *match, void *arg)
> return 0;
> }
>
> +/* Note, @dev is used by mock_acpi_table_parse_cedt() */
> struct cxl_chbs_context {
> struct device *dev;
> unsigned long long uid;
> - resource_size_t rcrb;
> - resource_size_t chbcr;
> + resource_size_t base;
> u32 cxl_version;
> };
>
> -static int cxl_get_chbcr(union acpi_subtable_headers *header, void *arg,
> +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->base)
> return 0;
>
> chbs = (struct acpi_cedt_chbs *) header;
> @@ -395,23 +395,16 @@ static int cxl_get_chbcr(union acpi_subtable_headers *header, void *arg,
> return 0;
>
> ctx->cxl_version = chbs->cxl_version;
> - ctx->rcrb = CXL_RESOURCE_NONE;
> - ctx->chbcr = CXL_RESOURCE_NONE;
> + ctx->base = CXL_RESOURCE_NONE;
>
> if (!chbs->base)
> return 0;
>
> - if (chbs->cxl_version != ACPI_CEDT_CHBS_VERSION_CXL11) {
> - ctx->chbcr = chbs->base;
> - return 0;
> - }
> -
> - if (chbs->length != CXL_RCRB_SIZE)
> + if (chbs->cxl_version == ACPI_CEDT_CHBS_VERSION_CXL11 &&
> + chbs->length != CXL_RCRB_SIZE)
> return 0;
>
> - ctx->rcrb = chbs->base;
> - ctx->chbcr = cxl_rcrb_to_component(ctx->dev, chbs->base,
> - CXL_RCRB_DOWNSTREAM);
> + ctx->base = chbs->base;
>
> return 0;
> }
> @@ -443,33 +436,31 @@ static int add_host_bridge_dport(struct device *match, void *arg)
> .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) {
> + if (!ctx.base) {
> dev_warn(match, "No CHBS found for Host Bridge (UID %lld)\n",
> uid);
> return 0;
> }
>
> - if (ctx.rcrb != CXL_RESOURCE_NONE)
> - dev_dbg(match, "RCRB found for UID %lld: %pa\n", uid, &ctx.rcrb);
> -
> - if (ctx.chbcr == CXL_RESOURCE_NONE) {
> - dev_warn(match, "CHBCR invalid for Host Bridge (UID %lld)\n",
> + if (ctx.base == CXL_RESOURCE_NONE) {
> + dev_warn(match, "CHBS invalid for Host Bridge (UID %lld)\n",
> uid);
> return 0;
> }
>
> - dev_dbg(match, "CHBCR found: %pa\n", &ctx.chbcr);
> -
> pci_root = acpi_pci_find_root(hb->handle);
> bridge = pci_root->bus->bridge;
> - if (ctx.cxl_version == ACPI_CEDT_CHBS_VERSION_CXL11)
> - dport = devm_cxl_add_rch_dport(root_port, bridge, uid,
> - ctx.chbcr, ctx.rcrb);
> - else
> - dport = devm_cxl_add_dport(root_port, bridge, uid,
> - ctx.chbcr);
> +
> + if (ctx.cxl_version == ACPI_CEDT_CHBS_VERSION_CXL11) {
> + dev_dbg(match, "RCRB found for UID %lld: %pa\n", uid, &ctx.base);
> + dport = devm_cxl_add_rch_dport(root_port, bridge, uid, ctx.base);
> + } else {
> + dev_dbg(match, "CHBCR found for UID %lld: %pa\n", uid, &ctx.base);
> + dport = devm_cxl_add_dport(root_port, bridge, uid, ctx.base);
> + }
> +
> if (IS_ERR(dport))
> return PTR_ERR(dport);
>
> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> index e7c284c890bc..1a3f8729a616 100644
> --- a/drivers/cxl/core/port.c
> +++ b/drivers/cxl/core/port.c
> @@ -938,12 +938,25 @@ __devm_cxl_add_dport(struct cxl_port *port, struct device *dport_dev,
> if (!dport)
> return ERR_PTR(-ENOMEM);
>
> + if (rcrb != CXL_RESOURCE_NONE) {
> + component_reg_phys = cxl_rcrb_to_component(dport_dev,
> + rcrb, CXL_RCRB_DOWNSTREAM);
> + if (component_reg_phys == CXL_RESOURCE_NONE) {
> + dev_warn(dport_dev, "Invalid Component Registers in RCRB");
> + return ERR_PTR(-ENXIO);
> + }
> +
> + dport->rch = true;
> + }
> +
> + if (component_reg_phys != CXL_RESOURCE_NONE)
> + dev_dbg(dport_dev, "Component Registers found for dport: %pa\n",
> + &component_reg_phys);
> +
> dport->dport = dport_dev;
> dport->port_id = port_id;
> dport->component_reg_phys = component_reg_phys;
> dport->port = port;
> - if (rcrb != CXL_RESOURCE_NONE)
> - dport->rch = true;
> dport->rcrb = rcrb;
>
> cond_cxl_root_lock(port);
> @@ -1004,14 +1017,12 @@ EXPORT_SYMBOL_NS_GPL(devm_cxl_add_dport, CXL);
> * @port: the cxl_port that references this dport
> * @dport_dev: firmware or PCI device representing the dport
> * @port_id: identifier for this dport in a decoder's target list
> - * @component_reg_phys: optional location of CXL component registers
> * @rcrb: mandatory location of a Root Complex Register Block
> *
> * See CXL 3.0 9.11.8 CXL Devices Attached to an RCH
> */
> struct cxl_dport *devm_cxl_add_rch_dport(struct cxl_port *port,
> struct device *dport_dev, int port_id,
> - resource_size_t component_reg_phys,
> resource_size_t rcrb)
> {
> struct cxl_dport *dport;
> @@ -1022,7 +1033,7 @@ struct cxl_dport *devm_cxl_add_rch_dport(struct cxl_port *port,
> }
>
> dport = __devm_cxl_add_dport(port, dport_dev, port_id,
> - component_reg_phys, rcrb);
> + CXL_RESOURCE_NONE, rcrb);
> if (IS_ERR(dport)) {
> dev_dbg(dport_dev, "failed to add RCH dport to %s: %ld\n",
> dev_name(&port->dev), PTR_ERR(dport));
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index f93a28538962..a5cd661face2 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -671,7 +671,6 @@ struct cxl_dport *devm_cxl_add_dport(struct cxl_port *port,
> resource_size_t component_reg_phys);
> struct cxl_dport *devm_cxl_add_rch_dport(struct cxl_port *port,
> struct device *dport_dev, int port_id,
> - resource_size_t component_reg_phys,
> resource_size_t rcrb);
>
> struct cxl_decoder *to_cxl_decoder(struct device *dev);
Powered by blists - more mailing lists