| 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
| ||
|
Message-ID: <5b8c3084-1953-4058-8d2a-6234fc959aa1@amd.com>
Date: Fri, 18 Jul 2025 17:40:13 -0500
From: "Bowman, Terry" <terry.bowman@....com>
To: Dave Jiang <dave.jiang@...el.com>, dave@...olabs.net,
jonathan.cameron@...wei.com, alison.schofield@...el.com,
dan.j.williams@...el.com, bhelgaas@...gle.com, shiju.jose@...wei.com,
ming.li@...omail.com, Smita.KoralahalliChannabasappa@....com,
rrichter@....com, dan.carpenter@...aro.org,
PradeepVineshReddy.Kodamati@....com, lukas@...ner.de,
Benjamin.Cheatham@....com, sathyanarayanan.kuppuswamy@...ux.intel.com,
linux-cxl@...r.kernel.org
Cc: linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org
Subject: Re: [PATCH v10 09/17] cxl/pci: Map CXL Endpoint Port and CXL Switch
Port RAS registers
On 7/18/2025 5:01 PM, Dave Jiang wrote:
>
>
> On 7/18/25 2:55 PM, Bowman, Terry wrote:
>>
>>
>> On 7/18/2025 4:28 PM, Dave Jiang wrote:
>>>
>>> On 6/26/25 3:42 PM, Terry Bowman wrote:
>>>> CXL Endpoint (EP) Ports may include Root Ports (RP) or Downstream Switch
>>>> Ports (DSP). CXL RPs and DSPs contain RAS registers that require memory
>>>> mapping to enable RAS logging. This initialization is currently missing and
>>>> must be added for CXL RPs and DSPs.
>>>>
>>>> Update cxl_dport_init_ras_reporting() to support RP and DSP RAS mapping.
>>>> Add alongside the existing Restricted CXL Host Downstream Port RAS mapping.
>>>>
>>>> Update cxl_endpoint_port_probe() to invoke cxl_dport_init_ras_reporting().
>>>> This will initiate the RAS mapping for CXL RPs and DSPs when each CXL EP is
>>>> created and added to the EP port.
>>>>
>>>> Signed-off-by: Terry Bowman <terry.bowman@....com>
>>>> ---
>>>> drivers/cxl/cxl.h | 2 ++
>>>> drivers/cxl/mem.c | 3 ++-
>>>> drivers/cxl/port.c | 58 +++++++++++++++++++++++++++++++++++++++++++++-
>>>> 3 files changed, 61 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
>>>> index c57c160f3e5e..d696d419bd5a 100644
>>>> --- a/drivers/cxl/cxl.h
>>>> +++ b/drivers/cxl/cxl.h
>>>> @@ -590,6 +590,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
>>>> + * @uport_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
>>>> @@ -610,6 +611,7 @@ struct cxl_port {
>>>> struct cxl_dport *parent_dport;
>>>> struct ida decoder_ida;
>>>> struct cxl_register_map reg_map;
>>>> + struct cxl_component_regs uport_regs;
>>>> int nr_dports;
>>>> int hdm_end;
>>>> int commit_end;
>>>> diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
>>>> index 6e6777b7bafb..d2155f45240d 100644
>>>> --- a/drivers/cxl/mem.c
>>>> +++ b/drivers/cxl/mem.c
>>>> @@ -166,7 +166,8 @@ static int cxl_mem_probe(struct device *dev)
>>>> else
>>>> endpoint_parent = &parent_port->dev;
>>>>
>>>> - cxl_dport_init_ras_reporting(dport, dev);
>>>> + if (dport->rch)
>>>> + cxl_dport_init_ras_reporting(dport, dev);
>>>>
>>>> scoped_guard(device, endpoint_parent) {
>>>> if (!endpoint_parent->driver) {
>>>> diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c
>>>> index 021f35145c65..b52f82925891 100644
>>>> --- a/drivers/cxl/port.c
>>>> +++ b/drivers/cxl/port.c
>>>> @@ -111,6 +111,17 @@ static void cxl_disable_rch_root_ints(struct cxl_dport *dport)
>>>> writel(aer_cmd, aer_base + PCI_ERR_ROOT_COMMAND);
>>>> }
>>>>
>>>> +static void cxl_uport_init_ras_reporting(struct cxl_port *port,
>>>> + struct device *host)
>>>> +{
>>>> + struct cxl_register_map *map = &port->reg_map;
>>>> +
>>>> + map->host = host;
>>>> + if (cxl_map_component_regs(map, &port->uport_regs,
>>>> + BIT(CXL_CM_CAP_CAP_ID_RAS)))
>>>> + dev_dbg(&port->dev, "Failed to map RAS capability\n");
>>>> +}
>>>> +
>>>> /**
>>>> * cxl_dport_init_ras_reporting - Setup CXL RAS report on this dport
>>>> * @dport: the cxl_dport that needs to be initialized
>>>> @@ -119,7 +130,6 @@ static void cxl_disable_rch_root_ints(struct cxl_dport *dport)
>>>> void cxl_dport_init_ras_reporting(struct cxl_dport *dport, struct device *host)
>>>> {
>>>> dport->reg_map.host = host;
>>>> - cxl_dport_map_ras(dport);
>>>>
>>>> if (dport->rch) {
>>>> struct pci_host_bridge *host_bridge = to_pci_host_bridge(dport->dport_dev);
>>>> @@ -127,12 +137,54 @@ void cxl_dport_init_ras_reporting(struct cxl_dport *dport, struct device *host)
>>>> if (!host_bridge->native_aer)
>>>> return;
>>>>
>>>> + cxl_dport_map_ras(dport);
>>>> cxl_dport_map_rch_aer(dport);
>>>> cxl_disable_rch_root_ints(dport);
>>>> + return;
>>>> }
>>>> +
>>>> + if (cxl_map_component_regs(&dport->reg_map, &dport->regs.component,
>>>> + BIT(CXL_CM_CAP_CAP_ID_RAS)))
>>>> + dev_dbg(dport->dport_dev, "Failed to map RAS capability\n");
>>>> +
>>>> }
>>>> EXPORT_SYMBOL_NS_GPL(cxl_dport_init_ras_reporting, "CXL");
>>>>
>>>> +static void cxl_switch_port_init_ras(struct cxl_port *port)
>>>> +{
>>>> + if (is_cxl_root(to_cxl_port(port->dev.parent)))
>>>> + return;
>>>> +
>>>> + /* May have upstream DSP or RP */
>>>> + if (port->parent_dport && dev_is_pci(port->parent_dport->dport_dev)) {
>>>> + struct pci_dev *pdev = to_pci_dev(port->parent_dport->dport_dev);
>>>> +
>>>> + if ((pci_pcie_type(pdev) == PCI_EXP_TYPE_ROOT_PORT) ||
>>>> + (pci_pcie_type(pdev) == PCI_EXP_TYPE_DOWNSTREAM))
>>>> + cxl_dport_init_ras_reporting(port->parent_dport, &port->dev);
>>>> + }
>>>> +
>>>> + cxl_uport_init_ras_reporting(port, &port->dev);
>>>> +}
>>>> +
>>>> +static void cxl_endpoint_port_init_ras(struct cxl_port *port)
>>> Maybe rename 'port' to 'ep' to be explicit
>> Ok
>>>> +{
>>>> + struct cxl_dport *dport;
>>> parent_dport would be clearer. I was thinking why does the endpoint have a dport for a second there.
>> Ok
>>>> + struct cxl_memdev *cxlmd = to_cxl_memdev(port->uport_dev);
>>>> + struct cxl_port *parent_port __free(put_cxl_port) =
>>>> + cxl_mem_find_port(cxlmd, &dport);
>>>> +
>>>> + if (!dport || !dev_is_pci(dport->dport_dev)) {
>>>> + dev_err(&port->dev, "CXL port topology not found\n");> + return;
>>>> + }
>>>> +
>>>> + cxl_dport_init_ras_reporting(dport, cxlmd->cxlds->dev);
>>>> +}
>>>> +
>>>> +#else
>>>> +static void cxl_endpoint_port_init_ras(struct cxl_port *port) { }
>>>> +static void cxl_switch_port_init_ras(struct cxl_port *port) { }
>>>> #endif /* CONFIG_PCIEAER_CXL */
>>> I cc'd you on the new patch to move all the AER stuff to core/pci_aer.c. That should take care of ifdef CONFIG_PCIEAER_CXL in pci.c and port.c.
>>>
>>> DJ
>>
>> Move to core/native_ras.c introduced in "Dequeue forwarded CXL error", right? I just want to be certain.
>
> I just posted this [1] to clean up what's there. Do you prefer me to just rename the file to native_ras.c? Or maybe pci_ras.c?
>
> [1]: https://lore.kernel.org/linux-cxl/20250718212452.2100663-1-dave.jiang@intel.com/
>
> DJ
>
Hi Dave,
I think leaving as you have it is fine.
Would you like to see my v10 changes in core/native_ras.c stay as-is or move into pci_aer.c?
-Terry
>>
>> Regards,
>> Terry
>>
>>>> > static int cxl_switch_port_probe(struct cxl_port *port)
>>>> @@ -149,6 +201,8 @@ static int cxl_switch_port_probe(struct cxl_port *port)
>>>>
>>>> cxl_switch_parse_cdat(port);
>>>>
>>>> + cxl_switch_port_init_ras(port);
>>>> +
>>>> cxlhdm = devm_cxl_setup_hdm(port, NULL);
>>>> if (!IS_ERR(cxlhdm))
>>>> return devm_cxl_enumerate_decoders(cxlhdm, NULL);
>>>> @@ -203,6 +257,8 @@ static int cxl_endpoint_port_probe(struct cxl_port *port)
>>>> if (rc)
>>>> return rc;
>>>>
>>>> + cxl_endpoint_port_init_ras(port);
>>>> +
>>>> /*
>>>> * Now that all endpoint decoders are successfully enumerated, try to
>>>> * assemble regions from committed decoders
>>
>
Powered by blists - more mailing lists