| 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: Wed, 26 Jun 2024 11:39:50 +0800
From: "Li, Ming4" <ming4.li@...el.com>
To: Terry Bowman <terry.bowman@....com>, "Williams, Dan J"
<dan.j.williams@...el.com>, "Weiny, Ira" <ira.weiny@...el.com>,
"dave@...olabs.net" <dave@...olabs.net>, "Jiang, Dave"
<dave.jiang@...el.com>, "Schofield, Alison" <Alison.Schofield@...el.com>,
"Verma, Vishal L" <vishal.l.verma@...el.com>, "jim.harris@...sung.com"
<jim.harris@...sung.com>, "ilpo.jarvinen@...ux.intel.com"
<ilpo.jarvinen@...ux.intel.com>, "ardb@...nel.org" <ardb@...nel.org>,
"sathyanarayanan.kuppuswamy@...ux.intel.com"
<sathyanarayanan.kuppuswamy@...ux.intel.com>, "linux-cxl@...r.kernel.org"
<linux-cxl@...r.kernel.org>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>, "Yazen.Ghannam@....com"
<Yazen.Ghannam@....com>, "Robert.Richter@....com" <Robert.Richter@....com>
Subject: Re: [RFC PATCH 4/9] cxl/pci: Map CXL PCIe ports' RAS registers
On 6/18/2024 4:04 AM, Terry Bowman wrote:
> RAS registers are not currently mapped for CXL root ports, CXL downstream
> switch ports, and CXL upstream switch ports. Update the driver to map the
> ports' RAS registers in preparation for RAS logging and handling to be
> added in the future.
>
> Add a 'struct cxl_regs' variable to 'struct cxl_port'. This will be used
> to store a pointer to the upstream port's mapped RAS registers.
>
> Invoke the RAS mapping logic from the CXL memory device probe routine
> after the endpoint is added. This ensures the ports have completed
> training and the RAS registers are present in CXL.cachemem.
>
> Refactor the cxl_dport_map_regs() function to support mapping the CXL
> PCIe ports. Also, check for previously mapped registers in the topology
> including CXL switch. Endpoints under a CXL switch share a CXL root port
> and will be iterated for each endpoint. Only map once.
>
> Signed-off-by: Terry Bowman <terry.bowman@....com>
> ---
> drivers/cxl/core/pci.c | 30 +++++++++++++++++++++++++-----
> drivers/cxl/cxl.h | 5 +++++
> drivers/cxl/mem.c | 32 ++++++++++++++++++++++++++++++--
> 3 files changed, 60 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
> index 0df09bd79408..e6c91b3dfccf 100644
> --- a/drivers/cxl/core/pci.c
> +++ b/drivers/cxl/core/pci.c
> @@ -787,16 +787,26 @@ static void cxl_dport_map_rch_aer(struct cxl_dport *dport)
> dport->regs.dport_aer = dport_aer;
> }
>
> -static void cxl_dport_map_regs(struct cxl_dport *dport)
> +static void cxl_port_map_regs(struct device *dev,
> + struct cxl_register_map *map,
> + struct cxl_regs *regs)
> {
> - struct cxl_register_map *map = &dport->reg_map;
> - struct device *dev = dport->dport_dev;
> -
> if (!map->component_map.ras.valid)
> dev_dbg(dev, "RAS registers not found\n");
> - else if (cxl_map_component_regs(map, &dport->regs.component,
> + else if (regs->ras)
> + dev_dbg(dev, "RAS registers already initialized\n");
> + else if (cxl_map_component_regs(map, ®s->component,
> BIT(CXL_CM_CAP_CAP_ID_RAS)))
> dev_dbg(dev, "Failed to map RAS capability.\n");
> +}
> +
> +static void cxl_dport_map_regs(struct cxl_dport *dport)
> +{
> + struct cxl_register_map *map = &dport->reg_map;
> + struct cxl_regs *regs = &dport->regs;
> + struct device *dev = dport->dport_dev;
> +
> + cxl_port_map_regs(dev, map, regs);
>
> if (dport->rch)
> cxl_dport_map_rch_aer(dport);
> @@ -831,6 +841,16 @@ static void cxl_disable_rch_root_ints(struct cxl_dport *dport)
> }
> }
>
> +void cxl_setup_parent_uport(struct device *host, struct cxl_port *port)
> +{
> + struct cxl_register_map *map = &port->reg_map;
> + struct cxl_regs *regs = &port->regs;
> + struct device *uport_dev = port->uport_dev;
> +
> + cxl_port_map_regs(uport_dev, map, regs);
> +}
> +EXPORT_SYMBOL_NS_GPL(cxl_setup_parent_uport, CXL);
> +
> void cxl_setup_parent_dport(struct device *host, struct cxl_dport *dport)
> {
> struct device *dport_dev = dport->dport_dev;
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index 036d17db68e0..7cee678fdb75 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -587,6 +587,7 @@ struct cxl_dax_region {
> * @parent_dport: dport that points to this port in the parent
> * @decoder_ida: allocator for decoder ids
> * @reg_map: component and ras register mapping parameters
> + * @regs: mapped component registers
> * @nr_dports: number of entries in @dports
> * @hdm_end: track last allocated HDM decoder instance for allocation ordering
> * @commit_end: cursor to track highest committed decoder for commit ordering
> @@ -607,6 +608,7 @@ struct cxl_port {
> struct cxl_dport *parent_dport;
> struct ida decoder_ida;
> struct cxl_register_map reg_map;
> + struct cxl_regs regs;
> int nr_dports;
> int hdm_end;
> int commit_end;
> @@ -757,9 +759,12 @@ struct cxl_dport *devm_cxl_add_rch_dport(struct cxl_port *port,
>
> #ifdef CONFIG_PCIEAER_CXL
> void cxl_setup_parent_dport(struct device *host, struct cxl_dport *dport);
> +void cxl_setup_parent_uport(struct device *host, struct cxl_port *port);
> #else
> static inline void cxl_setup_parent_dport(struct device *host,
> struct cxl_dport *dport) { }
> +static inline void cxl_setup_parent_uport(struct device *host,
> + struct cxl_port *port) { }
> #endif
>
> struct cxl_decoder *to_cxl_decoder(struct device *dev);
> diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
> index 0c79d9ce877c..51a4641fc9a6 100644
> --- a/drivers/cxl/mem.c
> +++ b/drivers/cxl/mem.c
> @@ -45,10 +45,39 @@ static int cxl_mem_dpa_show(struct seq_file *file, void *data)
> return 0;
> }
>
> +static bool cxl_dev_is_pci_type(struct device *dev, u32 pcie_type)
> +{
> + struct pci_dev *pdev;
> +
> + if (!dev_is_pci(dev))
> + return false;
> +
> + pdev = to_pci_dev(dev);
> + if (pci_pcie_type(pdev) != pcie_type)
> + return false;
> +
> + return pci_find_dvsec_capability(pdev, PCI_DVSEC_VENDOR_ID_CXL,
> + CXL_DVSEC_REG_LOCATOR);
> +}
> +
> +static void cxl_setup_ep_parent_ports(struct cxl_ep *ep, struct device *host)
> +{
> + struct cxl_dport *dport = ep->dport;
> +
> + if (cxl_dev_is_pci_type(dport->dport_dev, PCI_EXP_TYPE_DOWNSTREAM) ||
> + cxl_dev_is_pci_type(dport->dport_dev, PCI_EXP_TYPE_ROOT_PORT))
> + cxl_setup_parent_dport(host, ep->dport);
you reuse cxl_setup_parent_dport() for root ports in this case, and I noticed that cxl_setup_parent_dport() will update dport->reg_map.host. So the host of dport's reg_map is the first cxl device trying to map the registers on the dport, the mapping of registers will be released during the device removal, but the mapping should still be available for other devices/switches under the root port after the device removal.
> +
> + if (cxl_dev_is_pci_type(dport->port->uport_dev, PCI_EXP_TYPE_UPSTREAM))
> + cxl_setup_parent_uport(host, ep->dport->port);
> +}
> +
> static int devm_cxl_add_endpoint(struct device *host, struct cxl_memdev *cxlmd,
> struct cxl_dport *parent_dport)
> {
> struct cxl_port *parent_port = parent_dport->port;
> + struct cxl_dev_state *cxlds = cxlmd->cxlds;
> + struct pci_dev *pdev = to_pci_dev(cxlds->dev);
> struct cxl_port *endpoint, *iter, *down;
> int rc;
>
> @@ -62,6 +91,7 @@ static int devm_cxl_add_endpoint(struct device *host, struct cxl_memdev *cxlmd,
>
> ep = cxl_ep_load(iter, cxlmd);
> ep->next = down;
> + cxl_setup_ep_parent_ports(ep, &pdev->dev);
> }
>
> /* Note: endpoint port component registers are derived from @cxlds */
> @@ -157,8 +187,6 @@ static int cxl_mem_probe(struct device *dev)
> else
> endpoint_parent = &parent_port->dev;
>
> - cxl_setup_parent_dport(dev, dport);
> -
> device_lock(endpoint_parent);
> if (!endpoint_parent->driver) {
> dev_err(dev, "CXL port topology %s not enabled\n",
Powered by blists - more mailing lists