| 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: <36fdebc2-b987-40ee-abf0-624b55768e3c@amd.com>
Date: Thu, 2 Oct 2025 10:36:07 +0100
From: Alejandro Lucero Palau <alucerop@....com>
To: PJ Waskiewicz <ppwaskie@...nel.org>, alejandro.lucero-palau@....com,
linux-cxl@...r.kernel.org, netdev@...r.kernel.org, dan.j.williams@...el.com,
edward.cree@....com, davem@...emloft.net, kuba@...nel.org,
pabeni@...hat.com, edumazet@...gle.com, dave.jiang@...el.com
Cc: Jonathan Cameron <Jonathan.Cameron@...wei.com>
Subject: Re: [PATCH v18 04/20] cxl: allow Type2 drivers to map cxl component
regs
On 10/2/25 00:20, PJ Waskiewicz wrote:
> Hi Alejandro,
>
> On Thu, 2025-09-18 at 10:17 +0100, alejandro.lucero-palau@....com
> wrote:
>> From: Alejandro Lucero <alucerop@....com>
>>
>> Export cxl core functions for a Type2 driver being able to discover
>> and
>> map the device component registers.
>>
>> Use it in sfc driver cxl initialization.
>>
>> Signed-off-by: Alejandro Lucero <alucerop@....com>
>> Reviewed-by: Dan Williams <dan.j.williams@...el.com>
>> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@...wei.com>
>> ---
>> drivers/cxl/core/port.c | 1 +
>> drivers/cxl/cxl.h | 7 -------
>> drivers/cxl/cxlpci.h | 12 -----------
>> drivers/net/ethernet/sfc/efx_cxl.c | 33
>> ++++++++++++++++++++++++++++++
>> include/cxl/cxl.h | 20 ++++++++++++++++++
>> include/cxl/pci.h | 15 ++++++++++++++
>> 6 files changed, 69 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
>> index bb326dc95d5f..240c3c5bcdc8 100644
>> --- a/drivers/cxl/core/port.c
>> +++ b/drivers/cxl/core/port.c
>> @@ -11,6 +11,7 @@
>> #include <linux/idr.h>
>> #include <linux/node.h>
>> #include <cxl/einj.h>
>> +#include <cxl/pci.h>
>> #include <cxlmem.h>
>> #include <cxlpci.h>
>> #include <cxl.h>
>> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
>> index e197c36c7525..793d4dfe51a2 100644
>> --- a/drivers/cxl/cxl.h
>> +++ b/drivers/cxl/cxl.h
>> @@ -38,10 +38,6 @@ extern const struct nvdimm_security_ops
>> *cxl_security_ops;
>> #define CXL_CM_CAP_HDR_ARRAY_SIZE_MASK GENMASK(31, 24)
>> #define CXL_CM_CAP_PTR_MASK GENMASK(31, 20)
>>
>> -#define CXL_CM_CAP_CAP_ID_RAS 0x2
>> -#define CXL_CM_CAP_CAP_ID_HDM 0x5
>> -#define CXL_CM_CAP_CAP_HDM_VERSION 1
>> -
>> /* HDM decoders CXL 2.0 8.2.5.12 CXL HDM Decoder Capability
>> Structure */
>> #define CXL_HDM_DECODER_CAP_OFFSET 0x0
>> #define CXL_HDM_DECODER_COUNT_MASK GENMASK(3, 0)
>> @@ -205,9 +201,6 @@ void cxl_probe_component_regs(struct device *dev,
>> void __iomem *base,
>> struct cxl_component_reg_map *map);
>> void cxl_probe_device_regs(struct device *dev, void __iomem *base,
>> struct cxl_device_reg_map *map);
>> -int cxl_map_component_regs(const struct cxl_register_map *map,
>> - struct cxl_component_regs *regs,
>> - unsigned long map_mask);
>> int cxl_map_device_regs(const struct cxl_register_map *map,
>> struct cxl_device_regs *regs);
>> int cxl_map_pmu_regs(struct cxl_register_map *map, struct
>> cxl_pmu_regs *regs);
>> diff --git a/drivers/cxl/cxlpci.h b/drivers/cxl/cxlpci.h
>> index 4b11757a46ab..2247823acf6f 100644
>> --- a/drivers/cxl/cxlpci.h
>> +++ b/drivers/cxl/cxlpci.h
>> @@ -13,16 +13,6 @@
>> */
>> #define CXL_PCI_DEFAULT_MAX_VECTORS 16
>>
>> -/* Register Block Identifier (RBI) */
>> -enum cxl_regloc_type {
>> - CXL_REGLOC_RBI_EMPTY = 0,
>> - CXL_REGLOC_RBI_COMPONENT,
>> - CXL_REGLOC_RBI_VIRT,
>> - CXL_REGLOC_RBI_MEMDEV,
>> - CXL_REGLOC_RBI_PMU,
>> - CXL_REGLOC_RBI_TYPES
>> -};
>> -
>> /*
>> * Table Access DOE, CDAT Read Entry Response
>> *
>> @@ -90,6 +80,4 @@ struct cxl_dev_state;
>> int cxl_hdm_decode_init(struct cxl_dev_state *cxlds, struct cxl_hdm
>> *cxlhdm,
>> struct cxl_endpoint_dvsec_info *info);
>> void read_cdat_data(struct cxl_port *port);
>> -int cxl_pci_setup_regs(struct pci_dev *pdev, enum cxl_regloc_type
>> type,
>> - struct cxl_register_map *map);
>> #endif /* __CXL_PCI_H__ */
>> diff --git a/drivers/net/ethernet/sfc/efx_cxl.c
>> b/drivers/net/ethernet/sfc/efx_cxl.c
>> index 56d148318636..cdfbe546d8d8 100644
>> --- a/drivers/net/ethernet/sfc/efx_cxl.c
>> +++ b/drivers/net/ethernet/sfc/efx_cxl.c
>> @@ -5,6 +5,7 @@
>> * Copyright (C) 2025, Advanced Micro Devices, Inc.
>> */
>>
>> +#include <cxl/cxl.h>
>> #include <cxl/pci.h>
>> #include <linux/pci.h>
>>
>> @@ -19,6 +20,7 @@ int efx_cxl_init(struct efx_probe_data *probe_data)
>> struct pci_dev *pci_dev = efx->pci_dev;
>> struct efx_cxl *cxl;
>> u16 dvsec;
>> + int rc;
>>
>> probe_data->cxl_pio_initialised = false;
>>
>> @@ -45,6 +47,37 @@ int efx_cxl_init(struct efx_probe_data
>> *probe_data)
>> if (!cxl)
>> return -ENOMEM;
>>
>> + rc = cxl_pci_setup_regs(pci_dev, CXL_REGLOC_RBI_COMPONENT,
>> + &cxl->cxlds.reg_map);
>> + if (rc) {
>> + dev_err(&pci_dev->dev, "No component registers
>> (err=%d)\n", rc);
>> + return rc;
>> + }
>> +
>> + if (!cxl->cxlds.reg_map.component_map.hdm_decoder.valid) {
>> + dev_err(&pci_dev->dev, "Expected HDM component
>> register not found\n");
>> + return -ENODEV;
>> + }
>> +
>> + if (!cxl->cxlds.reg_map.component_map.ras.valid)
>> + return dev_err_probe(&pci_dev->dev, -ENODEV,
>> + "Expected RAS component
>> register not found\n");
>> +
>> + rc = cxl_map_component_regs(&cxl->cxlds.reg_map,
>> + &cxl->cxlds.regs.component,
>> + BIT(CXL_CM_CAP_CAP_ID_RAS));
>> + if (rc) {
>> + dev_err(&pci_dev->dev, "Failed to map RAS
>> capability.\n");
>> + return rc;
>> + }
> I've finally made some serious headway integrating v17 into my
> environment to better comment on this flow.
>
> I'm running into what I'm seeing as a fundamental issue of resource
> ownership between a device driver, and the CXL driver core. I'm having
> a hard time trying to resolve this.
>
> If I do the above and call cxl_map_component_regs() with a valid CAP_ID
> (RAS, HDM, etc.), that eventually calls devm_cxl_iomap_block() from
> inside the CXL core drivers. That calls devm_request_mem_region(), and
> this is where things get interesting.
>
> If my device happens to land the CXL component registers inside of a
> BAR that has other items needed by my Type 2 device's driver, then we
> have a conflict. My driver and the CXL core drivers cannot hold the
> same regions mapped. i.e. I can't call pci_request_region() on my BAR,
> and then call the above. One loses, and then we all lose.
>
> Curious if you have any ideas how we can improve this?
I ran into this issue early in my development when working with
emulation, and I had to share the mapping somehow.
It went away with newer more real emulation so I had to no worry about
it anymore. But yes, the problem does exist. I can not find my old code
but I guess the solution is to pass a pointer to the already mapped
region implying the mapping is not required. It requires changes to the
cxl core but it should not be too much work.
> Cheers,
> -PJ
Powered by blists - more mailing lists