| Message ID | 20240907081836.5801-3-alejandro.lucero-palau@amd.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | cxl: add Type2 device support | expand |
Hi, kernel test robot noticed the following build warnings: [auto build test WARNING on cxl/next] [also build test WARNING on linus/master v6.11-rc6 next-20240906] [cannot apply to cxl/pending horms-ipvs/master] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/alejandro-lucero-palau-amd-com/cxl-add-type2-device-basic-support/20240907-162231 base: https://git.kernel.org/pub/scm/linux/kernel/git/cxl/cxl.git next patch link: https://lore.kernel.org/r/20240907081836.5801-3-alejandro.lucero-palau%40amd.com patch subject: [PATCH v3 02/20] cxl: add capabilities field to cxl_dev_state and cxl_port config: xtensa-randconfig-r073-20240908 (https://download.01.org/0day-ci/archive/20240908/202409080140.BHrsmdob-lkp@intel.com/config) compiler: xtensa-linux-gcc (GCC) 14.1.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240908/202409080140.BHrsmdob-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202409080140.BHrsmdob-lkp@intel.com/ All warnings (new ones prefixed by >>): >> drivers/cxl/core/regs.c:41: warning: Function parameter or struct member 'caps' not described in 'cxl_probe_component_regs' >> drivers/cxl/core/regs.c:124: warning: Function parameter or struct member 'caps' not described in 'cxl_probe_device_regs' vim +41 drivers/cxl/core/regs.c fa89248e669d58 Robert Richter 2022-10-18 13 2b922a9d064f8e Dan Williams 2021-09-03 14 /** 2b922a9d064f8e Dan Williams 2021-09-03 15 * DOC: cxl registers 2b922a9d064f8e Dan Williams 2021-09-03 16 * 2b922a9d064f8e Dan Williams 2021-09-03 17 * CXL device capabilities are enumerated by PCI DVSEC (Designated 2b922a9d064f8e Dan Williams 2021-09-03 18 * Vendor-specific) and / or descriptors provided by platform firmware. 2b922a9d064f8e Dan Williams 2021-09-03 19 * They can be defined as a set like the device and component registers 2b922a9d064f8e Dan Williams 2021-09-03 20 * mandated by CXL Section 8.1.12.2 Memory Device PCIe Capabilities and 2b922a9d064f8e Dan Williams 2021-09-03 21 * Extended Capabilities, or they can be individual capabilities 2b922a9d064f8e Dan Williams 2021-09-03 22 * appended to bridged and endpoint devices. 2b922a9d064f8e Dan Williams 2021-09-03 23 * 2b922a9d064f8e Dan Williams 2021-09-03 24 * Provide common infrastructure for enumerating and mapping these 2b922a9d064f8e Dan Williams 2021-09-03 25 * discrete capabilities. 2b922a9d064f8e Dan Williams 2021-09-03 26 */ 2b922a9d064f8e Dan Williams 2021-09-03 27 0f06157e0135f5 Dan Williams 2021-08-03 28 /** 0f06157e0135f5 Dan Williams 2021-08-03 29 * cxl_probe_component_regs() - Detect CXL Component register blocks 0f06157e0135f5 Dan Williams 2021-08-03 30 * @dev: Host device of the @base mapping 0f06157e0135f5 Dan Williams 2021-08-03 31 * @base: Mapping containing the HDM Decoder Capability Header 0f06157e0135f5 Dan Williams 2021-08-03 32 * @map: Map object describing the register block information found 0f06157e0135f5 Dan Williams 2021-08-03 33 * 0f06157e0135f5 Dan Williams 2021-08-03 34 * See CXL 2.0 8.2.4 Component Register Layout and Definition 0f06157e0135f5 Dan Williams 2021-08-03 35 * See CXL 2.0 8.2.5.5 CXL Device Register Interface 0f06157e0135f5 Dan Williams 2021-08-03 36 * 0f06157e0135f5 Dan Williams 2021-08-03 37 * Probe for component register information and return it in map object. 0f06157e0135f5 Dan Williams 2021-08-03 38 */ 0f06157e0135f5 Dan Williams 2021-08-03 39 void cxl_probe_component_regs(struct device *dev, void __iomem *base, 98279f48d53f4f Alejandro Lucero 2024-09-07 40 struct cxl_component_reg_map *map, u32 *caps) 0f06157e0135f5 Dan Williams 2021-08-03 @41 { 0f06157e0135f5 Dan Williams 2021-08-03 42 int cap, cap_count; 74b0fe80409733 Jonathan Cameron 2022-02-01 43 u32 cap_array; 0f06157e0135f5 Dan Williams 2021-08-03 44 0f06157e0135f5 Dan Williams 2021-08-03 45 *map = (struct cxl_component_reg_map) { 0 }; 0f06157e0135f5 Dan Williams 2021-08-03 46 0f06157e0135f5 Dan Williams 2021-08-03 47 /* 0f06157e0135f5 Dan Williams 2021-08-03 48 * CXL.cache and CXL.mem registers are at offset 0x1000 as defined in 0f06157e0135f5 Dan Williams 2021-08-03 49 * CXL 2.0 8.2.4 Table 141. 0f06157e0135f5 Dan Williams 2021-08-03 50 */ 0f06157e0135f5 Dan Williams 2021-08-03 51 base += CXL_CM_OFFSET; 0f06157e0135f5 Dan Williams 2021-08-03 52 74b0fe80409733 Jonathan Cameron 2022-02-01 53 cap_array = readl(base + CXL_CM_CAP_HDR_OFFSET); 0f06157e0135f5 Dan Williams 2021-08-03 54 0f06157e0135f5 Dan Williams 2021-08-03 55 if (FIELD_GET(CXL_CM_CAP_HDR_ID_MASK, cap_array) != CM_CAP_HDR_CAP_ID) { 0f06157e0135f5 Dan Williams 2021-08-03 56 dev_err(dev, d621bc2e7282f9 Dan Williams 2022-01-23 57 "Couldn't locate the CXL.cache and CXL.mem capability array header.\n"); 0f06157e0135f5 Dan Williams 2021-08-03 58 return; 0f06157e0135f5 Dan Williams 2021-08-03 59 } 0f06157e0135f5 Dan Williams 2021-08-03 60 0f06157e0135f5 Dan Williams 2021-08-03 61 /* It's assumed that future versions will be backward compatible */ 0f06157e0135f5 Dan Williams 2021-08-03 62 cap_count = FIELD_GET(CXL_CM_CAP_HDR_ARRAY_SIZE_MASK, cap_array); 0f06157e0135f5 Dan Williams 2021-08-03 63 0f06157e0135f5 Dan Williams 2021-08-03 64 for (cap = 1; cap <= cap_count; cap++) { 0f06157e0135f5 Dan Williams 2021-08-03 65 void __iomem *register_block; af2dfef854aa6a Dan Williams 2022-11-29 66 struct cxl_reg_map *rmap; 0f06157e0135f5 Dan Williams 2021-08-03 67 u16 cap_id, offset; af2dfef854aa6a Dan Williams 2022-11-29 68 u32 length, hdr; 0f06157e0135f5 Dan Williams 2021-08-03 69 0f06157e0135f5 Dan Williams 2021-08-03 70 hdr = readl(base + cap * 0x4); 0f06157e0135f5 Dan Williams 2021-08-03 71 0f06157e0135f5 Dan Williams 2021-08-03 72 cap_id = FIELD_GET(CXL_CM_CAP_HDR_ID_MASK, hdr); 0f06157e0135f5 Dan Williams 2021-08-03 73 offset = FIELD_GET(CXL_CM_CAP_PTR_MASK, hdr); 0f06157e0135f5 Dan Williams 2021-08-03 74 register_block = base + offset; af2dfef854aa6a Dan Williams 2022-11-29 75 hdr = readl(register_block); 0f06157e0135f5 Dan Williams 2021-08-03 76 af2dfef854aa6a Dan Williams 2022-11-29 77 rmap = NULL; 0f06157e0135f5 Dan Williams 2021-08-03 78 switch (cap_id) { af2dfef854aa6a Dan Williams 2022-11-29 79 case CXL_CM_CAP_CAP_ID_HDM: { af2dfef854aa6a Dan Williams 2022-11-29 80 int decoder_cnt; af2dfef854aa6a Dan Williams 2022-11-29 81 0f06157e0135f5 Dan Williams 2021-08-03 82 dev_dbg(dev, "found HDM decoder capability (0x%x)\n", 0f06157e0135f5 Dan Williams 2021-08-03 83 offset); 0f06157e0135f5 Dan Williams 2021-08-03 84 0f06157e0135f5 Dan Williams 2021-08-03 85 decoder_cnt = cxl_hdm_decoder_count(hdr); 0f06157e0135f5 Dan Williams 2021-08-03 86 length = 0x20 * decoder_cnt + 0x10; af2dfef854aa6a Dan Williams 2022-11-29 87 rmap = &map->hdm_decoder; 98279f48d53f4f Alejandro Lucero 2024-09-07 88 *caps |= BIT(CXL_DEV_CAP_HDM); 0f06157e0135f5 Dan Williams 2021-08-03 89 break; af2dfef854aa6a Dan Williams 2022-11-29 90 } bd09626b39dff9 Dan Williams 2022-11-29 91 case CXL_CM_CAP_CAP_ID_RAS: bd09626b39dff9 Dan Williams 2022-11-29 92 dev_dbg(dev, "found RAS capability (0x%x)\n", bd09626b39dff9 Dan Williams 2022-11-29 93 offset); bd09626b39dff9 Dan Williams 2022-11-29 94 length = CXL_RAS_CAPABILITY_LENGTH; bd09626b39dff9 Dan Williams 2022-11-29 95 rmap = &map->ras; 98279f48d53f4f Alejandro Lucero 2024-09-07 96 *caps |= BIT(CXL_DEV_CAP_RAS); 0f06157e0135f5 Dan Williams 2021-08-03 97 break; 0f06157e0135f5 Dan Williams 2021-08-03 98 default: 0f06157e0135f5 Dan Williams 2021-08-03 99 dev_dbg(dev, "Unknown CM cap ID: %d (0x%x)\n", cap_id, 0f06157e0135f5 Dan Williams 2021-08-03 100 offset); 0f06157e0135f5 Dan Williams 2021-08-03 101 break; 0f06157e0135f5 Dan Williams 2021-08-03 102 } af2dfef854aa6a Dan Williams 2022-11-29 103 af2dfef854aa6a Dan Williams 2022-11-29 104 if (!rmap) af2dfef854aa6a Dan Williams 2022-11-29 105 continue; af2dfef854aa6a Dan Williams 2022-11-29 106 rmap->valid = true; a1554e9cac5ea0 Dan Williams 2022-11-29 107 rmap->id = cap_id; af2dfef854aa6a Dan Williams 2022-11-29 108 rmap->offset = CXL_CM_OFFSET + offset; af2dfef854aa6a Dan Williams 2022-11-29 109 rmap->size = length; 0f06157e0135f5 Dan Williams 2021-08-03 110 } 0f06157e0135f5 Dan Williams 2021-08-03 111 } affec782742e08 Dan Williams 2021-11-12 112 EXPORT_SYMBOL_NS_GPL(cxl_probe_component_regs, CXL); 0f06157e0135f5 Dan Williams 2021-08-03 113 0f06157e0135f5 Dan Williams 2021-08-03 114 /** 0f06157e0135f5 Dan Williams 2021-08-03 115 * cxl_probe_device_regs() - Detect CXL Device register blocks 0f06157e0135f5 Dan Williams 2021-08-03 116 * @dev: Host device of the @base mapping 0f06157e0135f5 Dan Williams 2021-08-03 117 * @base: Mapping of CXL 2.0 8.2.8 CXL Device Register Interface 0f06157e0135f5 Dan Williams 2021-08-03 118 * @map: Map object describing the register block information found 0f06157e0135f5 Dan Williams 2021-08-03 119 * 0f06157e0135f5 Dan Williams 2021-08-03 120 * Probe for device register information and return it in map object. 0f06157e0135f5 Dan Williams 2021-08-03 121 */ 0f06157e0135f5 Dan Williams 2021-08-03 122 void cxl_probe_device_regs(struct device *dev, void __iomem *base, 98279f48d53f4f Alejandro Lucero 2024-09-07 123 struct cxl_device_reg_map *map, u32 *caps) 0f06157e0135f5 Dan Williams 2021-08-03 @124 { 0f06157e0135f5 Dan Williams 2021-08-03 125 int cap, cap_count; 0f06157e0135f5 Dan Williams 2021-08-03 126 u64 cap_array; 0f06157e0135f5 Dan Williams 2021-08-03 127 0f06157e0135f5 Dan Williams 2021-08-03 128 *map = (struct cxl_device_reg_map){ 0 }; 0f06157e0135f5 Dan Williams 2021-08-03 129 0f06157e0135f5 Dan Williams 2021-08-03 130 cap_array = readq(base + CXLDEV_CAP_ARRAY_OFFSET); 0f06157e0135f5 Dan Williams 2021-08-03 131 if (FIELD_GET(CXLDEV_CAP_ARRAY_ID_MASK, cap_array) != 0f06157e0135f5 Dan Williams 2021-08-03 132 CXLDEV_CAP_ARRAY_CAP_ID) 0f06157e0135f5 Dan Williams 2021-08-03 133 return; 0f06157e0135f5 Dan Williams 2021-08-03 134 0f06157e0135f5 Dan Williams 2021-08-03 135 cap_count = FIELD_GET(CXLDEV_CAP_ARRAY_COUNT_MASK, cap_array); 0f06157e0135f5 Dan Williams 2021-08-03 136 0f06157e0135f5 Dan Williams 2021-08-03 137 for (cap = 1; cap <= cap_count; cap++) { af2dfef854aa6a Dan Williams 2022-11-29 138 struct cxl_reg_map *rmap; 0f06157e0135f5 Dan Williams 2021-08-03 139 u32 offset, length; 0f06157e0135f5 Dan Williams 2021-08-03 140 u16 cap_id; 0f06157e0135f5 Dan Williams 2021-08-03 141 0f06157e0135f5 Dan Williams 2021-08-03 142 cap_id = FIELD_GET(CXLDEV_CAP_HDR_CAP_ID_MASK, 0f06157e0135f5 Dan Williams 2021-08-03 143 readl(base + cap * 0x10)); 0f06157e0135f5 Dan Williams 2021-08-03 144 offset = readl(base + cap * 0x10 + 0x4); 0f06157e0135f5 Dan Williams 2021-08-03 145 length = readl(base + cap * 0x10 + 0x8); 0f06157e0135f5 Dan Williams 2021-08-03 146 af2dfef854aa6a Dan Williams 2022-11-29 147 rmap = NULL; 0f06157e0135f5 Dan Williams 2021-08-03 148 switch (cap_id) { 0f06157e0135f5 Dan Williams 2021-08-03 149 case CXLDEV_CAP_CAP_ID_DEVICE_STATUS: 0f06157e0135f5 Dan Williams 2021-08-03 150 dev_dbg(dev, "found Status capability (0x%x)\n", offset); af2dfef854aa6a Dan Williams 2022-11-29 151 rmap = &map->status; 98279f48d53f4f Alejandro Lucero 2024-09-07 152 *caps |= BIT(CXL_DEV_CAP_DEV_STATUS); 0f06157e0135f5 Dan Williams 2021-08-03 153 break; 0f06157e0135f5 Dan Williams 2021-08-03 154 case CXLDEV_CAP_CAP_ID_PRIMARY_MAILBOX: 0f06157e0135f5 Dan Williams 2021-08-03 155 dev_dbg(dev, "found Mailbox capability (0x%x)\n", offset); af2dfef854aa6a Dan Williams 2022-11-29 156 rmap = &map->mbox; 98279f48d53f4f Alejandro Lucero 2024-09-07 157 *caps |= BIT(CXL_DEV_CAP_MAILBOX_PRIMARY); 0f06157e0135f5 Dan Williams 2021-08-03 158 break; 0f06157e0135f5 Dan Williams 2021-08-03 159 case CXLDEV_CAP_CAP_ID_SECONDARY_MAILBOX: 0f06157e0135f5 Dan Williams 2021-08-03 160 dev_dbg(dev, "found Secondary Mailbox capability (0x%x)\n", offset); 0f06157e0135f5 Dan Williams 2021-08-03 161 break; 0f06157e0135f5 Dan Williams 2021-08-03 162 case CXLDEV_CAP_CAP_ID_MEMDEV: 0f06157e0135f5 Dan Williams 2021-08-03 163 dev_dbg(dev, "found Memory Device capability (0x%x)\n", offset); af2dfef854aa6a Dan Williams 2022-11-29 164 rmap = &map->memdev; 98279f48d53f4f Alejandro Lucero 2024-09-07 165 *caps |= BIT(CXL_DEV_CAP_MEMDEV); 0f06157e0135f5 Dan Williams 2021-08-03 166 break; 0f06157e0135f5 Dan Williams 2021-08-03 167 default: 0f06157e0135f5 Dan Williams 2021-08-03 168 if (cap_id >= 0x8000) 0f06157e0135f5 Dan Williams 2021-08-03 169 dev_dbg(dev, "Vendor cap ID: %#x offset: %#x\n", cap_id, offset); 0f06157e0135f5 Dan Williams 2021-08-03 170 else 0f06157e0135f5 Dan Williams 2021-08-03 171 dev_dbg(dev, "Unknown cap ID: %#x offset: %#x\n", cap_id, offset); 0f06157e0135f5 Dan Williams 2021-08-03 172 break; 0f06157e0135f5 Dan Williams 2021-08-03 173 } af2dfef854aa6a Dan Williams 2022-11-29 174 af2dfef854aa6a Dan Williams 2022-11-29 175 if (!rmap) af2dfef854aa6a Dan Williams 2022-11-29 176 continue; af2dfef854aa6a Dan Williams 2022-11-29 177 rmap->valid = true; a1554e9cac5ea0 Dan Williams 2022-11-29 178 rmap->id = cap_id; af2dfef854aa6a Dan Williams 2022-11-29 179 rmap->offset = offset; af2dfef854aa6a Dan Williams 2022-11-29 180 rmap->size = length; 0f06157e0135f5 Dan Williams 2021-08-03 181 } 0f06157e0135f5 Dan Williams 2021-08-03 182 } affec782742e08 Dan Williams 2021-11-12 183 EXPORT_SYMBOL_NS_GPL(cxl_probe_device_regs, CXL); 0f06157e0135f5 Dan Williams 2021-08-03 184
On 9/7/24 1:18 AM, alejandro.lucero-palau@amd.com wrote: > From: Alejandro Lucero <alucerop@amd.com> > > Type2 devices have some Type3 functionalities as optional like an mbox > or an hdm decoder, and CXL core needs a way to know what an CXL accelerator > implements. > > Add a new field for keeping device capabilities as discovered during > initialization. > > Add same field to cxl_port which for an endpoint will use those > capabilities discovered previously, and which will be initialized when > calling cxl_port_setup_regs for no endpoints. I don't quite understand what you are trying to say here. > > Signed-off-by: Alejandro Lucero <alucerop@amd.com> > --- > drivers/cxl/core/port.c | 9 +++++---- > drivers/cxl/core/regs.c | 20 +++++++++++++------- > drivers/cxl/cxl.h | 8 +++++--- > drivers/cxl/cxlmem.h | 2 ++ > drivers/cxl/pci.c | 9 +++++---- > include/linux/cxl/cxl.h | 30 ++++++++++++++++++++++++++++++ > 6 files changed, 60 insertions(+), 18 deletions(-) > > diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c > index 1d5007e3795a..39b20ddd0296 100644 > --- a/drivers/cxl/core/port.c > +++ b/drivers/cxl/core/port.c > @@ -749,7 +749,7 @@ static struct cxl_port *cxl_port_alloc(struct device *uport_dev, > } > > static int cxl_setup_comp_regs(struct device *host, struct cxl_register_map *map, > - resource_size_t component_reg_phys) > + resource_size_t component_reg_phys, u32 *caps) > { > *map = (struct cxl_register_map) { > .host = host, > @@ -763,7 +763,7 @@ static int cxl_setup_comp_regs(struct device *host, struct cxl_register_map *map > map->reg_type = CXL_REGLOC_RBI_COMPONENT; > map->max_size = CXL_COMPONENT_REG_BLOCK_SIZE; > > - return cxl_setup_regs(map); > + return cxl_setup_regs(map, caps); > } > > static int cxl_port_setup_regs(struct cxl_port *port, > @@ -772,7 +772,7 @@ static int cxl_port_setup_regs(struct cxl_port *port, > if (dev_is_platform(port->uport_dev)) > return 0; > return cxl_setup_comp_regs(&port->dev, &port->reg_map, > - component_reg_phys); > + component_reg_phys, &port->capabilities); > } > > static int cxl_dport_setup_regs(struct device *host, struct cxl_dport *dport, > @@ -789,7 +789,7 @@ static int cxl_dport_setup_regs(struct device *host, struct cxl_dport *dport, > * NULL. > */ > rc = cxl_setup_comp_regs(dport->dport_dev, &dport->reg_map, > - component_reg_phys); > + component_reg_phys, &dport->port->capabilities); > dport->reg_map.host = host; > return rc; > } > @@ -858,6 +858,7 @@ static struct cxl_port *__devm_cxl_add_port(struct device *host, > port->reg_map = cxlds->reg_map; > port->reg_map.host = &port->dev; > cxlmd->endpoint = port; > + port->capabilities = cxlds->capabilities; > } else if (parent_dport) { > rc = dev_set_name(dev, "port%d", port->id); > if (rc) > diff --git a/drivers/cxl/core/regs.c b/drivers/cxl/core/regs.c > index e1082e749c69..8b8abcadcb93 100644 > --- a/drivers/cxl/core/regs.c > +++ b/drivers/cxl/core/regs.c > @@ -1,6 +1,7 @@ > // SPDX-License-Identifier: GPL-2.0-only > /* Copyright(c) 2020 Intel Corporation. */ > #include <linux/io-64-nonatomic-lo-hi.h> > +#include <linux/cxl/cxl.h> > #include <linux/device.h> > #include <linux/slab.h> > #include <linux/pci.h> > @@ -36,7 +37,7 @@ > * Probe for component register information and return it in map object. > */ > void cxl_probe_component_regs(struct device *dev, void __iomem *base, > - struct cxl_component_reg_map *map) > + struct cxl_component_reg_map *map, u32 *caps) > { > int cap, cap_count; > u32 cap_array; > @@ -84,6 +85,7 @@ void cxl_probe_component_regs(struct device *dev, void __iomem *base, > decoder_cnt = cxl_hdm_decoder_count(hdr); > length = 0x20 * decoder_cnt + 0x10; > rmap = &map->hdm_decoder; > + *caps |= BIT(CXL_DEV_CAP_HDM); > break; > } > case CXL_CM_CAP_CAP_ID_RAS: > @@ -91,6 +93,7 @@ void cxl_probe_component_regs(struct device *dev, void __iomem *base, > offset); > length = CXL_RAS_CAPABILITY_LENGTH; > rmap = &map->ras; > + *caps |= BIT(CXL_DEV_CAP_RAS); > break; > default: > dev_dbg(dev, "Unknown CM cap ID: %d (0x%x)\n", cap_id, > @@ -117,7 +120,7 @@ EXPORT_SYMBOL_NS_GPL(cxl_probe_component_regs, CXL); > * Probe for device register information and return it in map object. > */ > void cxl_probe_device_regs(struct device *dev, void __iomem *base, > - struct cxl_device_reg_map *map) > + struct cxl_device_reg_map *map, u32 *caps) > { > int cap, cap_count; > u64 cap_array; > @@ -146,10 +149,12 @@ void cxl_probe_device_regs(struct device *dev, void __iomem *base, > case CXLDEV_CAP_CAP_ID_DEVICE_STATUS: > dev_dbg(dev, "found Status capability (0x%x)\n", offset); > rmap = &map->status; > + *caps |= BIT(CXL_DEV_CAP_DEV_STATUS); > break; > case CXLDEV_CAP_CAP_ID_PRIMARY_MAILBOX: > dev_dbg(dev, "found Mailbox capability (0x%x)\n", offset); > rmap = &map->mbox; > + *caps |= BIT(CXL_DEV_CAP_MAILBOX_PRIMARY); > break; > case CXLDEV_CAP_CAP_ID_SECONDARY_MAILBOX: > dev_dbg(dev, "found Secondary Mailbox capability (0x%x)\n", offset); > @@ -157,6 +162,7 @@ void cxl_probe_device_regs(struct device *dev, void __iomem *base, > case CXLDEV_CAP_CAP_ID_MEMDEV: > dev_dbg(dev, "found Memory Device capability (0x%x)\n", offset); > rmap = &map->memdev; > + *caps |= BIT(CXL_DEV_CAP_MEMDEV); > break; > default: > if (cap_id >= 0x8000) > @@ -421,7 +427,7 @@ static void cxl_unmap_regblock(struct cxl_register_map *map) > map->base = NULL; > } > > -static int cxl_probe_regs(struct cxl_register_map *map) > +static int cxl_probe_regs(struct cxl_register_map *map, u32 *caps) > { > struct cxl_component_reg_map *comp_map; > struct cxl_device_reg_map *dev_map; > @@ -431,12 +437,12 @@ static int cxl_probe_regs(struct cxl_register_map *map) > switch (map->reg_type) { > case CXL_REGLOC_RBI_COMPONENT: > comp_map = &map->component_map; > - cxl_probe_component_regs(host, base, comp_map); > + cxl_probe_component_regs(host, base, comp_map, caps); > dev_dbg(host, "Set up component registers\n"); > break; > case CXL_REGLOC_RBI_MEMDEV: > dev_map = &map->device_map; > - cxl_probe_device_regs(host, base, dev_map); > + cxl_probe_device_regs(host, base, dev_map, caps); > if (!dev_map->status.valid || !dev_map->mbox.valid || > !dev_map->memdev.valid) { > dev_err(host, "registers not found: %s%s%s\n", > @@ -455,7 +461,7 @@ static int cxl_probe_regs(struct cxl_register_map *map) > return 0; > } > > -int cxl_setup_regs(struct cxl_register_map *map) > +int cxl_setup_regs(struct cxl_register_map *map, u32 *caps) > { > int rc; > > @@ -463,7 +469,7 @@ int cxl_setup_regs(struct cxl_register_map *map) > if (rc) > return rc; > > - rc = cxl_probe_regs(map); > + rc = cxl_probe_regs(map, caps); > cxl_unmap_regblock(map); > > return rc; > diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h > index 9afb407d438f..07c153aa3d77 100644 > --- a/drivers/cxl/cxl.h > +++ b/drivers/cxl/cxl.h > @@ -284,9 +284,9 @@ struct cxl_register_map { > }; > > void cxl_probe_component_regs(struct device *dev, void __iomem *base, > - struct cxl_component_reg_map *map); > + struct cxl_component_reg_map *map, u32 *caps); > void cxl_probe_device_regs(struct device *dev, void __iomem *base, > - struct cxl_device_reg_map *map); > + struct cxl_device_reg_map *map, u32 *caps); > int cxl_map_component_regs(const struct cxl_register_map *map, > struct cxl_component_regs *regs, > unsigned long map_mask); > @@ -300,7 +300,7 @@ int cxl_find_regblock_instance(struct pci_dev *pdev, enum cxl_regloc_type type, > struct cxl_register_map *map, int index); > int cxl_find_regblock(struct pci_dev *pdev, enum cxl_regloc_type type, > struct cxl_register_map *map); > -int cxl_setup_regs(struct cxl_register_map *map); > +int cxl_setup_regs(struct cxl_register_map *map, u32 *caps); > struct cxl_dport; > resource_size_t cxl_rcd_component_reg_phys(struct device *dev, > struct cxl_dport *dport); > @@ -600,6 +600,7 @@ struct cxl_dax_region { > * @cdat: Cached CDAT data > * @cdat_available: Should a CDAT attribute be available in sysfs > * @pci_latency: Upstream latency in picoseconds > + * @capabilities: those capabilities as defined in device mapped registers > */ > struct cxl_port { > struct device dev; > @@ -623,6 +624,7 @@ struct cxl_port { > } cdat; > bool cdat_available; > long pci_latency; > + u32 capabilities; > }; > > /** > diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h > index afb53d058d62..37c043100300 100644 > --- a/drivers/cxl/cxlmem.h > +++ b/drivers/cxl/cxlmem.h > @@ -424,6 +424,7 @@ struct cxl_dpa_perf { > * @ram_res: Active Volatile memory capacity configuration > * @serial: PCIe Device Serial Number > * @type: Generic Memory Class device or Vendor Specific Memory device > + * @capabilities: those capabilities as defined in device mapped registers > */ > struct cxl_dev_state { > struct device *dev; > @@ -438,6 +439,7 @@ struct cxl_dev_state { > struct resource ram_res; > u64 serial; > enum cxl_devtype type; > + u32 capabilities; > }; > > /** > diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c > index 742a7b2a1be5..58f325019886 100644 > --- a/drivers/cxl/pci.c > +++ b/drivers/cxl/pci.c > @@ -503,7 +503,7 @@ static int cxl_rcrb_get_comp_regs(struct pci_dev *pdev, > } > > static int cxl_pci_setup_regs(struct pci_dev *pdev, enum cxl_regloc_type type, > - struct cxl_register_map *map) > + struct cxl_register_map *map, u32 *caps) > { > int rc; > > @@ -520,7 +520,7 @@ static int cxl_pci_setup_regs(struct pci_dev *pdev, enum cxl_regloc_type type, > if (rc) > return rc; > > - return cxl_setup_regs(map); > + return cxl_setup_regs(map, caps); > } > > static int cxl_pci_ras_unmask(struct pci_dev *pdev) > @@ -827,7 +827,8 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) > else > cxl_set_dvsec(cxlds, dvsec); > > - rc = cxl_pci_setup_regs(pdev, CXL_REGLOC_RBI_MEMDEV, &map); > + rc = cxl_pci_setup_regs(pdev, CXL_REGLOC_RBI_MEMDEV, &map, > + &cxlds->capabilities); > if (rc) > return rc; > > @@ -840,7 +841,7 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) > * still be useful for management functions so don't return an error. > */ > rc = cxl_pci_setup_regs(pdev, CXL_REGLOC_RBI_COMPONENT, > - &cxlds->reg_map); > + &cxlds->reg_map, &cxlds->capabilities); > if (rc) > dev_warn(&pdev->dev, "No component registers (%d)\n", rc); > else if (!cxlds->reg_map.component_map.ras.valid) > diff --git a/include/linux/cxl/cxl.h b/include/linux/cxl/cxl.h > index e78eefa82123..930b1b9c1d6a 100644 > --- a/include/linux/cxl/cxl.h > +++ b/include/linux/cxl/cxl.h > @@ -12,6 +12,36 @@ enum cxl_resource { > CXL_ACCEL_RES_PMEM, > }; > > +/* Capabilities as defined for: > + * > + * Component Registers (Table 8-22 CXL 3.0 specification) > + * Device Registers (8.2.8.2.1 CXL 3.0 specification) Should just use 3.1 since that's the latest spec. > + */ > + > +enum cxl_dev_cap { > + /* capabilities from Component Registers */ > + CXL_DEV_CAP_RAS, > + CXL_DEV_CAP_SEC, > + CXL_DEV_CAP_LINK, > + CXL_DEV_CAP_HDM, > + CXL_DEV_CAP_SEC_EXT, > + CXL_DEV_CAP_IDE, > + CXL_DEV_CAP_SNOOP_FILTER, > + CXL_DEV_CAP_TIMEOUT_AND_ISOLATION, > + CXL_DEV_CAP_CACHEMEM_EXT, > + CXL_DEV_CAP_BI_ROUTE_TABLE, > + CXL_DEV_CAP_BI_DECODER, > + CXL_DEV_CAP_CACHEID_ROUTE_TABLE, > + CXL_DEV_CAP_CACHEID_DECODER, > + CXL_DEV_CAP_HDM_EXT, > + CXL_DEV_CAP_METADATA_EXT, > + /* capabilities from Device Registers */ > + CXL_DEV_CAP_DEV_STATUS, > + CXL_DEV_CAP_MAILBOX_PRIMARY, > + CXL_DEV_CAP_MAILBOX_SECONDARY, Does the OS ever uses the SECONDARY mailbox? > + CXL_DEV_CAP_MEMDEV, > +}; > + > struct cxl_dev_state *cxl_accel_state_create(struct device *dev); > > void cxl_set_dvsec(struct cxl_dev_state *cxlds, u16 dvsec);
On Sat, 7 Sep 2024 09:18:18 +0100 alejandro.lucero-palau@amd.com wrote: > From: Alejandro Lucero <alucerop@amd.com> > > Type2 devices have some Type3 functionalities as optional like an mbox > or an hdm decoder, and CXL core needs a way to know what an CXL accelerator > implements. > > Add a new field for keeping device capabilities as discovered during > initialization. > > Add same field to cxl_port which for an endpoint will use those > capabilities discovered previously, and which will be initialized when > calling cxl_port_setup_regs for no endpoints. > > Signed-off-by: Alejandro Lucero <alucerop@amd.com> Hi, My only real suggestion on this one is to use a bitmap to make it easy to extend the capabilities as needed in future. That means passing an unsigned long pointer around. > @@ -600,6 +600,7 @@ struct cxl_dax_region { > * @cdat: Cached CDAT data > * @cdat_available: Should a CDAT attribute be available in sysfs > * @pci_latency: Upstream latency in picoseconds > + * @capabilities: those capabilities as defined in device mapped registers > */ > struct cxl_port { > struct device dev; > @@ -623,6 +624,7 @@ struct cxl_port { > } cdat; > bool cdat_available; > long pci_latency; > + u32 capabilities; Use DECLARE_BITMAP() for this to make life easy should we ever have more than 32. > }; > > /** > diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h > index afb53d058d62..37c043100300 100644 > --- a/drivers/cxl/cxlmem.h > +++ b/drivers/cxl/cxlmem.h > @@ -424,6 +424,7 @@ struct cxl_dpa_perf { > * @ram_res: Active Volatile memory capacity configuration > * @serial: PCIe Device Serial Number > * @type: Generic Memory Class device or Vendor Specific Memory device > + * @capabilities: those capabilities as defined in device mapped registers > */ > struct cxl_dev_state { > struct device *dev; > @@ -438,6 +439,7 @@ struct cxl_dev_state { > struct resource ram_res; > u64 serial; > enum cxl_devtype type; > + u32 capabilities; As above, use a bitmap and access it with the various bitmap operators so that we aren't constrained to 32 bits given half are used already. > }; > > /** > diff --git a/include/linux/cxl/cxl.h b/include/linux/cxl/cxl.h > index e78eefa82123..930b1b9c1d6a 100644 > --- a/include/linux/cxl/cxl.h > +++ b/include/linux/cxl/cxl.h > @@ -12,6 +12,36 @@ enum cxl_resource { > CXL_ACCEL_RES_PMEM, > }; > > +/* Capabilities as defined for: Trivial but cxl tends to do /* * Capabilities .. style multiline comments. > + * > + * Component Registers (Table 8-22 CXL 3.0 specification) > + * Device Registers (8.2.8.2.1 CXL 3.0 specification) > + */ > + > +enum cxl_dev_cap { > + /* capabilities from Component Registers */ > + CXL_DEV_CAP_RAS, > + CXL_DEV_CAP_SEC, > + CXL_DEV_CAP_LINK, > + CXL_DEV_CAP_HDM, > + CXL_DEV_CAP_SEC_EXT, > + CXL_DEV_CAP_IDE, > + CXL_DEV_CAP_SNOOP_FILTER, > + CXL_DEV_CAP_TIMEOUT_AND_ISOLATION, > + CXL_DEV_CAP_CACHEMEM_EXT, > + CXL_DEV_CAP_BI_ROUTE_TABLE, > + CXL_DEV_CAP_BI_DECODER, > + CXL_DEV_CAP_CACHEID_ROUTE_TABLE, > + CXL_DEV_CAP_CACHEID_DECODER, > + CXL_DEV_CAP_HDM_EXT, > + CXL_DEV_CAP_METADATA_EXT, > + /* capabilities from Device Registers */ > + CXL_DEV_CAP_DEV_STATUS, > + CXL_DEV_CAP_MAILBOX_PRIMARY, > + CXL_DEV_CAP_MAILBOX_SECONDARY, Dan raised this one already - definitely not something any driver in Linux should be aware of. Hence just drop this entry. > + CXL_DEV_CAP_MEMDEV, > +}; > + > struct cxl_dev_state *cxl_accel_state_create(struct device *dev); > > void cxl_set_dvsec(struct cxl_dev_state *cxlds, u16 dvsec);
On 9/11/24 23:17, Dave Jiang wrote: > > On 9/7/24 1:18 AM, alejandro.lucero-palau@amd.com wrote: >> From: Alejandro Lucero <alucerop@amd.com> >> >> Type2 devices have some Type3 functionalities as optional like an mbox >> or an hdm decoder, and CXL core needs a way to know what an CXL accelerator >> implements. >> >> Add a new field for keeping device capabilities as discovered during >> initialization. >> >> Add same field to cxl_port which for an endpoint will use those >> capabilities discovered previously, and which will be initialized when >> calling cxl_port_setup_regs for no endpoints. > I don't quite understand what you are trying to say here. I guess you mean the last paragraph, don't you? If so, the point is the cxl_setup_regs or the register discovery is also being used from the cxl port code, I think for CXL switches initialization. >> Signed-off-by: Alejandro Lucero <alucerop@amd.com> >> --- >> drivers/cxl/core/port.c | 9 +++++---- >> drivers/cxl/core/regs.c | 20 +++++++++++++------- >> drivers/cxl/cxl.h | 8 +++++--- >> drivers/cxl/cxlmem.h | 2 ++ >> drivers/cxl/pci.c | 9 +++++---- >> include/linux/cxl/cxl.h | 30 ++++++++++++++++++++++++++++++ >> 6 files changed, 60 insertions(+), 18 deletions(-) >> >> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c >> index 1d5007e3795a..39b20ddd0296 100644 >> --- a/drivers/cxl/core/port.c >> +++ b/drivers/cxl/core/port.c >> @@ -749,7 +749,7 @@ static struct cxl_port *cxl_port_alloc(struct device *uport_dev, >> } >> >> static int cxl_setup_comp_regs(struct device *host, struct cxl_register_map *map, >> - resource_size_t component_reg_phys) >> + resource_size_t component_reg_phys, u32 *caps) >> { >> *map = (struct cxl_register_map) { >> .host = host, >> @@ -763,7 +763,7 @@ static int cxl_setup_comp_regs(struct device *host, struct cxl_register_map *map >> map->reg_type = CXL_REGLOC_RBI_COMPONENT; >> map->max_size = CXL_COMPONENT_REG_BLOCK_SIZE; >> >> - return cxl_setup_regs(map); >> + return cxl_setup_regs(map, caps); >> } >> >> static int cxl_port_setup_regs(struct cxl_port *port, >> @@ -772,7 +772,7 @@ static int cxl_port_setup_regs(struct cxl_port *port, >> if (dev_is_platform(port->uport_dev)) >> return 0; >> return cxl_setup_comp_regs(&port->dev, &port->reg_map, >> - component_reg_phys); >> + component_reg_phys, &port->capabilities); >> } >> >> static int cxl_dport_setup_regs(struct device *host, struct cxl_dport *dport, >> @@ -789,7 +789,7 @@ static int cxl_dport_setup_regs(struct device *host, struct cxl_dport *dport, >> * NULL. >> */ >> rc = cxl_setup_comp_regs(dport->dport_dev, &dport->reg_map, >> - component_reg_phys); >> + component_reg_phys, &dport->port->capabilities); >> dport->reg_map.host = host; >> return rc; >> } >> @@ -858,6 +858,7 @@ static struct cxl_port *__devm_cxl_add_port(struct device *host, >> port->reg_map = cxlds->reg_map; >> port->reg_map.host = &port->dev; >> cxlmd->endpoint = port; >> + port->capabilities = cxlds->capabilities; >> } else if (parent_dport) { >> rc = dev_set_name(dev, "port%d", port->id); >> if (rc) >> diff --git a/drivers/cxl/core/regs.c b/drivers/cxl/core/regs.c >> index e1082e749c69..8b8abcadcb93 100644 >> --- a/drivers/cxl/core/regs.c >> +++ b/drivers/cxl/core/regs.c >> @@ -1,6 +1,7 @@ >> // SPDX-License-Identifier: GPL-2.0-only >> /* Copyright(c) 2020 Intel Corporation. */ >> #include <linux/io-64-nonatomic-lo-hi.h> >> +#include <linux/cxl/cxl.h> >> #include <linux/device.h> >> #include <linux/slab.h> >> #include <linux/pci.h> >> @@ -36,7 +37,7 @@ >> * Probe for component register information and return it in map object. >> */ >> void cxl_probe_component_regs(struct device *dev, void __iomem *base, >> - struct cxl_component_reg_map *map) >> + struct cxl_component_reg_map *map, u32 *caps) >> { >> int cap, cap_count; >> u32 cap_array; >> @@ -84,6 +85,7 @@ void cxl_probe_component_regs(struct device *dev, void __iomem *base, >> decoder_cnt = cxl_hdm_decoder_count(hdr); >> length = 0x20 * decoder_cnt + 0x10; >> rmap = &map->hdm_decoder; >> + *caps |= BIT(CXL_DEV_CAP_HDM); >> break; >> } >> case CXL_CM_CAP_CAP_ID_RAS: >> @@ -91,6 +93,7 @@ void cxl_probe_component_regs(struct device *dev, void __iomem *base, >> offset); >> length = CXL_RAS_CAPABILITY_LENGTH; >> rmap = &map->ras; >> + *caps |= BIT(CXL_DEV_CAP_RAS); >> break; >> default: >> dev_dbg(dev, "Unknown CM cap ID: %d (0x%x)\n", cap_id, >> @@ -117,7 +120,7 @@ EXPORT_SYMBOL_NS_GPL(cxl_probe_component_regs, CXL); >> * Probe for device register information and return it in map object. >> */ >> void cxl_probe_device_regs(struct device *dev, void __iomem *base, >> - struct cxl_device_reg_map *map) >> + struct cxl_device_reg_map *map, u32 *caps) >> { >> int cap, cap_count; >> u64 cap_array; >> @@ -146,10 +149,12 @@ void cxl_probe_device_regs(struct device *dev, void __iomem *base, >> case CXLDEV_CAP_CAP_ID_DEVICE_STATUS: >> dev_dbg(dev, "found Status capability (0x%x)\n", offset); >> rmap = &map->status; >> + *caps |= BIT(CXL_DEV_CAP_DEV_STATUS); >> break; >> case CXLDEV_CAP_CAP_ID_PRIMARY_MAILBOX: >> dev_dbg(dev, "found Mailbox capability (0x%x)\n", offset); >> rmap = &map->mbox; >> + *caps |= BIT(CXL_DEV_CAP_MAILBOX_PRIMARY); >> break; >> case CXLDEV_CAP_CAP_ID_SECONDARY_MAILBOX: >> dev_dbg(dev, "found Secondary Mailbox capability (0x%x)\n", offset); >> @@ -157,6 +162,7 @@ void cxl_probe_device_regs(struct device *dev, void __iomem *base, >> case CXLDEV_CAP_CAP_ID_MEMDEV: >> dev_dbg(dev, "found Memory Device capability (0x%x)\n", offset); >> rmap = &map->memdev; >> + *caps |= BIT(CXL_DEV_CAP_MEMDEV); >> break; >> default: >> if (cap_id >= 0x8000) >> @@ -421,7 +427,7 @@ static void cxl_unmap_regblock(struct cxl_register_map *map) >> map->base = NULL; >> } >> >> -static int cxl_probe_regs(struct cxl_register_map *map) >> +static int cxl_probe_regs(struct cxl_register_map *map, u32 *caps) >> { >> struct cxl_component_reg_map *comp_map; >> struct cxl_device_reg_map *dev_map; >> @@ -431,12 +437,12 @@ static int cxl_probe_regs(struct cxl_register_map *map) >> switch (map->reg_type) { >> case CXL_REGLOC_RBI_COMPONENT: >> comp_map = &map->component_map; >> - cxl_probe_component_regs(host, base, comp_map); >> + cxl_probe_component_regs(host, base, comp_map, caps); >> dev_dbg(host, "Set up component registers\n"); >> break; >> case CXL_REGLOC_RBI_MEMDEV: >> dev_map = &map->device_map; >> - cxl_probe_device_regs(host, base, dev_map); >> + cxl_probe_device_regs(host, base, dev_map, caps); >> if (!dev_map->status.valid || !dev_map->mbox.valid || >> !dev_map->memdev.valid) { >> dev_err(host, "registers not found: %s%s%s\n", >> @@ -455,7 +461,7 @@ static int cxl_probe_regs(struct cxl_register_map *map) >> return 0; >> } >> >> -int cxl_setup_regs(struct cxl_register_map *map) >> +int cxl_setup_regs(struct cxl_register_map *map, u32 *caps) >> { >> int rc; >> >> @@ -463,7 +469,7 @@ int cxl_setup_regs(struct cxl_register_map *map) >> if (rc) >> return rc; >> >> - rc = cxl_probe_regs(map); >> + rc = cxl_probe_regs(map, caps); >> cxl_unmap_regblock(map); >> >> return rc; >> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h >> index 9afb407d438f..07c153aa3d77 100644 >> --- a/drivers/cxl/cxl.h >> +++ b/drivers/cxl/cxl.h >> @@ -284,9 +284,9 @@ struct cxl_register_map { >> }; >> >> void cxl_probe_component_regs(struct device *dev, void __iomem *base, >> - struct cxl_component_reg_map *map); >> + struct cxl_component_reg_map *map, u32 *caps); >> void cxl_probe_device_regs(struct device *dev, void __iomem *base, >> - struct cxl_device_reg_map *map); >> + struct cxl_device_reg_map *map, u32 *caps); >> int cxl_map_component_regs(const struct cxl_register_map *map, >> struct cxl_component_regs *regs, >> unsigned long map_mask); >> @@ -300,7 +300,7 @@ int cxl_find_regblock_instance(struct pci_dev *pdev, enum cxl_regloc_type type, >> struct cxl_register_map *map, int index); >> int cxl_find_regblock(struct pci_dev *pdev, enum cxl_regloc_type type, >> struct cxl_register_map *map); >> -int cxl_setup_regs(struct cxl_register_map *map); >> +int cxl_setup_regs(struct cxl_register_map *map, u32 *caps); >> struct cxl_dport; >> resource_size_t cxl_rcd_component_reg_phys(struct device *dev, >> struct cxl_dport *dport); >> @@ -600,6 +600,7 @@ struct cxl_dax_region { >> * @cdat: Cached CDAT data >> * @cdat_available: Should a CDAT attribute be available in sysfs >> * @pci_latency: Upstream latency in picoseconds >> + * @capabilities: those capabilities as defined in device mapped registers >> */ >> struct cxl_port { >> struct device dev; >> @@ -623,6 +624,7 @@ struct cxl_port { >> } cdat; >> bool cdat_available; >> long pci_latency; >> + u32 capabilities; >> }; >> >> /** >> diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h >> index afb53d058d62..37c043100300 100644 >> --- a/drivers/cxl/cxlmem.h >> +++ b/drivers/cxl/cxlmem.h >> @@ -424,6 +424,7 @@ struct cxl_dpa_perf { >> * @ram_res: Active Volatile memory capacity configuration >> * @serial: PCIe Device Serial Number >> * @type: Generic Memory Class device or Vendor Specific Memory device >> + * @capabilities: those capabilities as defined in device mapped registers >> */ >> struct cxl_dev_state { >> struct device *dev; >> @@ -438,6 +439,7 @@ struct cxl_dev_state { >> struct resource ram_res; >> u64 serial; >> enum cxl_devtype type; >> + u32 capabilities; >> }; >> >> /** >> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c >> index 742a7b2a1be5..58f325019886 100644 >> --- a/drivers/cxl/pci.c >> +++ b/drivers/cxl/pci.c >> @@ -503,7 +503,7 @@ static int cxl_rcrb_get_comp_regs(struct pci_dev *pdev, >> } >> >> static int cxl_pci_setup_regs(struct pci_dev *pdev, enum cxl_regloc_type type, >> - struct cxl_register_map *map) >> + struct cxl_register_map *map, u32 *caps) >> { >> int rc; >> >> @@ -520,7 +520,7 @@ static int cxl_pci_setup_regs(struct pci_dev *pdev, enum cxl_regloc_type type, >> if (rc) >> return rc; >> >> - return cxl_setup_regs(map); >> + return cxl_setup_regs(map, caps); >> } >> >> static int cxl_pci_ras_unmask(struct pci_dev *pdev) >> @@ -827,7 +827,8 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) >> else >> cxl_set_dvsec(cxlds, dvsec); >> >> - rc = cxl_pci_setup_regs(pdev, CXL_REGLOC_RBI_MEMDEV, &map); >> + rc = cxl_pci_setup_regs(pdev, CXL_REGLOC_RBI_MEMDEV, &map, >> + &cxlds->capabilities); >> if (rc) >> return rc; >> >> @@ -840,7 +841,7 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) >> * still be useful for management functions so don't return an error. >> */ >> rc = cxl_pci_setup_regs(pdev, CXL_REGLOC_RBI_COMPONENT, >> - &cxlds->reg_map); >> + &cxlds->reg_map, &cxlds->capabilities); >> if (rc) >> dev_warn(&pdev->dev, "No component registers (%d)\n", rc); >> else if (!cxlds->reg_map.component_map.ras.valid) >> diff --git a/include/linux/cxl/cxl.h b/include/linux/cxl/cxl.h >> index e78eefa82123..930b1b9c1d6a 100644 >> --- a/include/linux/cxl/cxl.h >> +++ b/include/linux/cxl/cxl.h >> @@ -12,6 +12,36 @@ enum cxl_resource { >> CXL_ACCEL_RES_PMEM, >> }; >> >> +/* Capabilities as defined for: >> + * >> + * Component Registers (Table 8-22 CXL 3.0 specification) >> + * Device Registers (8.2.8.2.1 CXL 3.0 specification) > Should just use 3.1 since that's the latest spec. Ok. >> + */ >> + >> +enum cxl_dev_cap { >> + /* capabilities from Component Registers */ >> + CXL_DEV_CAP_RAS, >> + CXL_DEV_CAP_SEC, >> + CXL_DEV_CAP_LINK, >> + CXL_DEV_CAP_HDM, >> + CXL_DEV_CAP_SEC_EXT, >> + CXL_DEV_CAP_IDE, >> + CXL_DEV_CAP_SNOOP_FILTER, >> + CXL_DEV_CAP_TIMEOUT_AND_ISOLATION, >> + CXL_DEV_CAP_CACHEMEM_EXT, >> + CXL_DEV_CAP_BI_ROUTE_TABLE, >> + CXL_DEV_CAP_BI_DECODER, >> + CXL_DEV_CAP_CACHEID_ROUTE_TABLE, >> + CXL_DEV_CAP_CACHEID_DECODER, >> + CXL_DEV_CAP_HDM_EXT, >> + CXL_DEV_CAP_METADATA_EXT, >> + /* capabilities from Device Registers */ >> + CXL_DEV_CAP_DEV_STATUS, >> + CXL_DEV_CAP_MAILBOX_PRIMARY, >> + CXL_DEV_CAP_MAILBOX_SECONDARY, > Does the OS ever uses the SECONDARY mailbox? I have no idea. I'm just listing all the potential capabilities here as you can see for things like BI or SNOOP. Should I just add those referenced by code? >> + CXL_DEV_CAP_MEMDEV, >> +}; >> + >> struct cxl_dev_state *cxl_accel_state_create(struct device *dev); >> >> void cxl_set_dvsec(struct cxl_dev_state *cxlds, u16 dvsec);
On 9/13/24 18:25, Jonathan Cameron wrote: > On Sat, 7 Sep 2024 09:18:18 +0100 > alejandro.lucero-palau@amd.com wrote: > >> From: Alejandro Lucero <alucerop@amd.com> >> >> Type2 devices have some Type3 functionalities as optional like an mbox >> or an hdm decoder, and CXL core needs a way to know what an CXL accelerator >> implements. >> >> Add a new field for keeping device capabilities as discovered during >> initialization. >> >> Add same field to cxl_port which for an endpoint will use those >> capabilities discovered previously, and which will be initialized when >> calling cxl_port_setup_regs for no endpoints. >> >> Signed-off-by: Alejandro Lucero <alucerop@amd.com> > Hi, > > My only real suggestion on this one is to use a bitmap to make > it easy to extend the capabilities as needed in future. > That means passing an unsigned long pointer around. > It makes sense. I'll do. >> @@ -600,6 +600,7 @@ struct cxl_dax_region { >> * @cdat: Cached CDAT data >> * @cdat_available: Should a CDAT attribute be available in sysfs >> * @pci_latency: Upstream latency in picoseconds >> + * @capabilities: those capabilities as defined in device mapped registers >> */ >> struct cxl_port { >> struct device dev; >> @@ -623,6 +624,7 @@ struct cxl_port { >> } cdat; >> bool cdat_available; >> long pci_latency; >> + u32 capabilities; > Use DECLARE_BITMAP() for this to make life easy should we ever > have more than 32. > >> }; >> >> /** >> diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h >> index afb53d058d62..37c043100300 100644 >> --- a/drivers/cxl/cxlmem.h >> +++ b/drivers/cxl/cxlmem.h >> @@ -424,6 +424,7 @@ struct cxl_dpa_perf { >> * @ram_res: Active Volatile memory capacity configuration >> * @serial: PCIe Device Serial Number >> * @type: Generic Memory Class device or Vendor Specific Memory device >> + * @capabilities: those capabilities as defined in device mapped registers >> */ >> struct cxl_dev_state { >> struct device *dev; >> @@ -438,6 +439,7 @@ struct cxl_dev_state { >> struct resource ram_res; >> u64 serial; >> enum cxl_devtype type; >> + u32 capabilities; > As above, use a bitmap and access it with the various bitmap operators > so that we aren't constrained to 32 bits given half are > used already. I though maybe I should use u64 ... > >> }; >> >> /** >> diff --git a/include/linux/cxl/cxl.h b/include/linux/cxl/cxl.h >> index e78eefa82123..930b1b9c1d6a 100644 >> --- a/include/linux/cxl/cxl.h >> +++ b/include/linux/cxl/cxl.h >> @@ -12,6 +12,36 @@ enum cxl_resource { >> CXL_ACCEL_RES_PMEM, >> }; >> >> +/* Capabilities as defined for: > Trivial but cxl tends to do > /* > * Capabilities .. > > style multiline comments. Right. I'll fit it. >> + * >> + * Component Registers (Table 8-22 CXL 3.0 specification) >> + * Device Registers (8.2.8.2.1 CXL 3.0 specification) >> + */ >> + >> +enum cxl_dev_cap { >> + /* capabilities from Component Registers */ >> + CXL_DEV_CAP_RAS, >> + CXL_DEV_CAP_SEC, >> + CXL_DEV_CAP_LINK, >> + CXL_DEV_CAP_HDM, >> + CXL_DEV_CAP_SEC_EXT, >> + CXL_DEV_CAP_IDE, >> + CXL_DEV_CAP_SNOOP_FILTER, >> + CXL_DEV_CAP_TIMEOUT_AND_ISOLATION, >> + CXL_DEV_CAP_CACHEMEM_EXT, >> + CXL_DEV_CAP_BI_ROUTE_TABLE, >> + CXL_DEV_CAP_BI_DECODER, >> + CXL_DEV_CAP_CACHEID_ROUTE_TABLE, >> + CXL_DEV_CAP_CACHEID_DECODER, >> + CXL_DEV_CAP_HDM_EXT, >> + CXL_DEV_CAP_METADATA_EXT, >> + /* capabilities from Device Registers */ >> + CXL_DEV_CAP_DEV_STATUS, >> + CXL_DEV_CAP_MAILBOX_PRIMARY, >> + CXL_DEV_CAP_MAILBOX_SECONDARY, > Dan raised this one already - definitely not something any > driver in Linux should be aware of. Hence just drop this entry. You mean never ever or just by now? Maybe I'm missing something specific to the secondary mailbox, but if the rule is if none used yet, do not add it, then there are other caps I should remove as well. >> + CXL_DEV_CAP_MEMDEV, >> +}; >> + >> struct cxl_dev_state *cxl_accel_state_create(struct device *dev); >> >> void cxl_set_dvsec(struct cxl_dev_state *cxlds, u16 dvsec);
On 9/16/24 1:36 AM, Alejandro Lucero Palau wrote: > > On 9/11/24 23:17, Dave Jiang wrote: >> >> On 9/7/24 1:18 AM, alejandro.lucero-palau@amd.com wrote: >>> From: Alejandro Lucero <alucerop@amd.com> >>> >>> Type2 devices have some Type3 functionalities as optional like an mbox >>> or an hdm decoder, and CXL core needs a way to know what an CXL accelerator >>> implements. >>> >>> Add a new field for keeping device capabilities as discovered during >>> initialization. >>> >>> Add same field to cxl_port which for an endpoint will use those >>> capabilities discovered previously, and which will be initialized when >>> calling cxl_port_setup_regs for no endpoints. >> I don't quite understand what you are trying to say here. > > > I guess you mean the last paragraph, don't you? > > If so, the point is the cxl_setup_regs or the register discovery is also being used from the cxl port code, I think for CXL switches initialization. Yes. Your response clarified my confusion. I do suggest you say that in your commit log. > > >>> Signed-off-by: Alejandro Lucero <alucerop@amd.com> >>> --- >>> drivers/cxl/core/port.c | 9 +++++---- >>> drivers/cxl/core/regs.c | 20 +++++++++++++------- >>> drivers/cxl/cxl.h | 8 +++++--- >>> drivers/cxl/cxlmem.h | 2 ++ >>> drivers/cxl/pci.c | 9 +++++---- >>> include/linux/cxl/cxl.h | 30 ++++++++++++++++++++++++++++++ >>> 6 files changed, 60 insertions(+), 18 deletions(-) >>> >>> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c >>> index 1d5007e3795a..39b20ddd0296 100644 >>> --- a/drivers/cxl/core/port.c >>> +++ b/drivers/cxl/core/port.c >>> @@ -749,7 +749,7 @@ static struct cxl_port *cxl_port_alloc(struct device *uport_dev, >>> } >>> static int cxl_setup_comp_regs(struct device *host, struct cxl_register_map *map, >>> - resource_size_t component_reg_phys) >>> + resource_size_t component_reg_phys, u32 *caps) >>> { >>> *map = (struct cxl_register_map) { >>> .host = host, >>> @@ -763,7 +763,7 @@ static int cxl_setup_comp_regs(struct device *host, struct cxl_register_map *map >>> map->reg_type = CXL_REGLOC_RBI_COMPONENT; >>> map->max_size = CXL_COMPONENT_REG_BLOCK_SIZE; >>> - return cxl_setup_regs(map); >>> + return cxl_setup_regs(map, caps); >>> } >>> static int cxl_port_setup_regs(struct cxl_port *port, >>> @@ -772,7 +772,7 @@ static int cxl_port_setup_regs(struct cxl_port *port, >>> if (dev_is_platform(port->uport_dev)) >>> return 0; >>> return cxl_setup_comp_regs(&port->dev, &port->reg_map, >>> - component_reg_phys); >>> + component_reg_phys, &port->capabilities); >>> } >>> static int cxl_dport_setup_regs(struct device *host, struct cxl_dport *dport, >>> @@ -789,7 +789,7 @@ static int cxl_dport_setup_regs(struct device *host, struct cxl_dport *dport, >>> * NULL. >>> */ >>> rc = cxl_setup_comp_regs(dport->dport_dev, &dport->reg_map, >>> - component_reg_phys); >>> + component_reg_phys, &dport->port->capabilities); >>> dport->reg_map.host = host; >>> return rc; >>> } >>> @@ -858,6 +858,7 @@ static struct cxl_port *__devm_cxl_add_port(struct device *host, >>> port->reg_map = cxlds->reg_map; >>> port->reg_map.host = &port->dev; >>> cxlmd->endpoint = port; >>> + port->capabilities = cxlds->capabilities; >>> } else if (parent_dport) { >>> rc = dev_set_name(dev, "port%d", port->id); >>> if (rc) >>> diff --git a/drivers/cxl/core/regs.c b/drivers/cxl/core/regs.c >>> index e1082e749c69..8b8abcadcb93 100644 >>> --- a/drivers/cxl/core/regs.c >>> +++ b/drivers/cxl/core/regs.c >>> @@ -1,6 +1,7 @@ >>> // SPDX-License-Identifier: GPL-2.0-only >>> /* Copyright(c) 2020 Intel Corporation. */ >>> #include <linux/io-64-nonatomic-lo-hi.h> >>> +#include <linux/cxl/cxl.h> >>> #include <linux/device.h> >>> #include <linux/slab.h> >>> #include <linux/pci.h> >>> @@ -36,7 +37,7 @@ >>> * Probe for component register information and return it in map object. >>> */ >>> void cxl_probe_component_regs(struct device *dev, void __iomem *base, >>> - struct cxl_component_reg_map *map) >>> + struct cxl_component_reg_map *map, u32 *caps) >>> { >>> int cap, cap_count; >>> u32 cap_array; >>> @@ -84,6 +85,7 @@ void cxl_probe_component_regs(struct device *dev, void __iomem *base, >>> decoder_cnt = cxl_hdm_decoder_count(hdr); >>> length = 0x20 * decoder_cnt + 0x10; >>> rmap = &map->hdm_decoder; >>> + *caps |= BIT(CXL_DEV_CAP_HDM); >>> break; >>> } >>> case CXL_CM_CAP_CAP_ID_RAS: >>> @@ -91,6 +93,7 @@ void cxl_probe_component_regs(struct device *dev, void __iomem *base, >>> offset); >>> length = CXL_RAS_CAPABILITY_LENGTH; >>> rmap = &map->ras; >>> + *caps |= BIT(CXL_DEV_CAP_RAS); >>> break; >>> default: >>> dev_dbg(dev, "Unknown CM cap ID: %d (0x%x)\n", cap_id, >>> @@ -117,7 +120,7 @@ EXPORT_SYMBOL_NS_GPL(cxl_probe_component_regs, CXL); >>> * Probe for device register information and return it in map object. >>> */ >>> void cxl_probe_device_regs(struct device *dev, void __iomem *base, >>> - struct cxl_device_reg_map *map) >>> + struct cxl_device_reg_map *map, u32 *caps) >>> { >>> int cap, cap_count; >>> u64 cap_array; >>> @@ -146,10 +149,12 @@ void cxl_probe_device_regs(struct device *dev, void __iomem *base, >>> case CXLDEV_CAP_CAP_ID_DEVICE_STATUS: >>> dev_dbg(dev, "found Status capability (0x%x)\n", offset); >>> rmap = &map->status; >>> + *caps |= BIT(CXL_DEV_CAP_DEV_STATUS); >>> break; >>> case CXLDEV_CAP_CAP_ID_PRIMARY_MAILBOX: >>> dev_dbg(dev, "found Mailbox capability (0x%x)\n", offset); >>> rmap = &map->mbox; >>> + *caps |= BIT(CXL_DEV_CAP_MAILBOX_PRIMARY); >>> break; >>> case CXLDEV_CAP_CAP_ID_SECONDARY_MAILBOX: >>> dev_dbg(dev, "found Secondary Mailbox capability (0x%x)\n", offset); >>> @@ -157,6 +162,7 @@ void cxl_probe_device_regs(struct device *dev, void __iomem *base, >>> case CXLDEV_CAP_CAP_ID_MEMDEV: >>> dev_dbg(dev, "found Memory Device capability (0x%x)\n", offset); >>> rmap = &map->memdev; >>> + *caps |= BIT(CXL_DEV_CAP_MEMDEV); >>> break; >>> default: >>> if (cap_id >= 0x8000) >>> @@ -421,7 +427,7 @@ static void cxl_unmap_regblock(struct cxl_register_map *map) >>> map->base = NULL; >>> } >>> -static int cxl_probe_regs(struct cxl_register_map *map) >>> +static int cxl_probe_regs(struct cxl_register_map *map, u32 *caps) >>> { >>> struct cxl_component_reg_map *comp_map; >>> struct cxl_device_reg_map *dev_map; >>> @@ -431,12 +437,12 @@ static int cxl_probe_regs(struct cxl_register_map *map) >>> switch (map->reg_type) { >>> case CXL_REGLOC_RBI_COMPONENT: >>> comp_map = &map->component_map; >>> - cxl_probe_component_regs(host, base, comp_map); >>> + cxl_probe_component_regs(host, base, comp_map, caps); >>> dev_dbg(host, "Set up component registers\n"); >>> break; >>> case CXL_REGLOC_RBI_MEMDEV: >>> dev_map = &map->device_map; >>> - cxl_probe_device_regs(host, base, dev_map); >>> + cxl_probe_device_regs(host, base, dev_map, caps); >>> if (!dev_map->status.valid || !dev_map->mbox.valid || >>> !dev_map->memdev.valid) { >>> dev_err(host, "registers not found: %s%s%s\n", >>> @@ -455,7 +461,7 @@ static int cxl_probe_regs(struct cxl_register_map *map) >>> return 0; >>> } >>> -int cxl_setup_regs(struct cxl_register_map *map) >>> +int cxl_setup_regs(struct cxl_register_map *map, u32 *caps) >>> { >>> int rc; >>> @@ -463,7 +469,7 @@ int cxl_setup_regs(struct cxl_register_map *map) >>> if (rc) >>> return rc; >>> - rc = cxl_probe_regs(map); >>> + rc = cxl_probe_regs(map, caps); >>> cxl_unmap_regblock(map); >>> return rc; >>> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h >>> index 9afb407d438f..07c153aa3d77 100644 >>> --- a/drivers/cxl/cxl.h >>> +++ b/drivers/cxl/cxl.h >>> @@ -284,9 +284,9 @@ struct cxl_register_map { >>> }; >>> void cxl_probe_component_regs(struct device *dev, void __iomem *base, >>> - struct cxl_component_reg_map *map); >>> + struct cxl_component_reg_map *map, u32 *caps); >>> void cxl_probe_device_regs(struct device *dev, void __iomem *base, >>> - struct cxl_device_reg_map *map); >>> + struct cxl_device_reg_map *map, u32 *caps); >>> int cxl_map_component_regs(const struct cxl_register_map *map, >>> struct cxl_component_regs *regs, >>> unsigned long map_mask); >>> @@ -300,7 +300,7 @@ int cxl_find_regblock_instance(struct pci_dev *pdev, enum cxl_regloc_type type, >>> struct cxl_register_map *map, int index); >>> int cxl_find_regblock(struct pci_dev *pdev, enum cxl_regloc_type type, >>> struct cxl_register_map *map); >>> -int cxl_setup_regs(struct cxl_register_map *map); >>> +int cxl_setup_regs(struct cxl_register_map *map, u32 *caps); >>> struct cxl_dport; >>> resource_size_t cxl_rcd_component_reg_phys(struct device *dev, >>> struct cxl_dport *dport); >>> @@ -600,6 +600,7 @@ struct cxl_dax_region { >>> * @cdat: Cached CDAT data >>> * @cdat_available: Should a CDAT attribute be available in sysfs >>> * @pci_latency: Upstream latency in picoseconds >>> + * @capabilities: those capabilities as defined in device mapped registers >>> */ >>> struct cxl_port { >>> struct device dev; >>> @@ -623,6 +624,7 @@ struct cxl_port { >>> } cdat; >>> bool cdat_available; >>> long pci_latency; >>> + u32 capabilities; >>> }; >>> /** >>> diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h >>> index afb53d058d62..37c043100300 100644 >>> --- a/drivers/cxl/cxlmem.h >>> +++ b/drivers/cxl/cxlmem.h >>> @@ -424,6 +424,7 @@ struct cxl_dpa_perf { >>> * @ram_res: Active Volatile memory capacity configuration >>> * @serial: PCIe Device Serial Number >>> * @type: Generic Memory Class device or Vendor Specific Memory device >>> + * @capabilities: those capabilities as defined in device mapped registers >>> */ >>> struct cxl_dev_state { >>> struct device *dev; >>> @@ -438,6 +439,7 @@ struct cxl_dev_state { >>> struct resource ram_res; >>> u64 serial; >>> enum cxl_devtype type; >>> + u32 capabilities; >>> }; >>> /** >>> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c >>> index 742a7b2a1be5..58f325019886 100644 >>> --- a/drivers/cxl/pci.c >>> +++ b/drivers/cxl/pci.c >>> @@ -503,7 +503,7 @@ static int cxl_rcrb_get_comp_regs(struct pci_dev *pdev, >>> } >>> static int cxl_pci_setup_regs(struct pci_dev *pdev, enum cxl_regloc_type type, >>> - struct cxl_register_map *map) >>> + struct cxl_register_map *map, u32 *caps) >>> { >>> int rc; >>> @@ -520,7 +520,7 @@ static int cxl_pci_setup_regs(struct pci_dev *pdev, enum cxl_regloc_type type, >>> if (rc) >>> return rc; >>> - return cxl_setup_regs(map); >>> + return cxl_setup_regs(map, caps); >>> } >>> static int cxl_pci_ras_unmask(struct pci_dev *pdev) >>> @@ -827,7 +827,8 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) >>> else >>> cxl_set_dvsec(cxlds, dvsec); >>> - rc = cxl_pci_setup_regs(pdev, CXL_REGLOC_RBI_MEMDEV, &map); >>> + rc = cxl_pci_setup_regs(pdev, CXL_REGLOC_RBI_MEMDEV, &map, >>> + &cxlds->capabilities); >>> if (rc) >>> return rc; >>> @@ -840,7 +841,7 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) >>> * still be useful for management functions so don't return an error. >>> */ >>> rc = cxl_pci_setup_regs(pdev, CXL_REGLOC_RBI_COMPONENT, >>> - &cxlds->reg_map); >>> + &cxlds->reg_map, &cxlds->capabilities); >>> if (rc) >>> dev_warn(&pdev->dev, "No component registers (%d)\n", rc); >>> else if (!cxlds->reg_map.component_map.ras.valid) >>> diff --git a/include/linux/cxl/cxl.h b/include/linux/cxl/cxl.h >>> index e78eefa82123..930b1b9c1d6a 100644 >>> --- a/include/linux/cxl/cxl.h >>> +++ b/include/linux/cxl/cxl.h >>> @@ -12,6 +12,36 @@ enum cxl_resource { >>> CXL_ACCEL_RES_PMEM, >>> }; >>> +/* Capabilities as defined for: >>> + * >>> + * Component Registers (Table 8-22 CXL 3.0 specification) >>> + * Device Registers (8.2.8.2.1 CXL 3.0 specification) >> Should just use 3.1 since that's the latest spec. > > > Ok. > > >>> + */ >>> + >>> +enum cxl_dev_cap { >>> + /* capabilities from Component Registers */ >>> + CXL_DEV_CAP_RAS, >>> + CXL_DEV_CAP_SEC, >>> + CXL_DEV_CAP_LINK, >>> + CXL_DEV_CAP_HDM, >>> + CXL_DEV_CAP_SEC_EXT, >>> + CXL_DEV_CAP_IDE, >>> + CXL_DEV_CAP_SNOOP_FILTER, >>> + CXL_DEV_CAP_TIMEOUT_AND_ISOLATION, >>> + CXL_DEV_CAP_CACHEMEM_EXT, >>> + CXL_DEV_CAP_BI_ROUTE_TABLE, >>> + CXL_DEV_CAP_BI_DECODER, >>> + CXL_DEV_CAP_CACHEID_ROUTE_TABLE, >>> + CXL_DEV_CAP_CACHEID_DECODER, >>> + CXL_DEV_CAP_HDM_EXT, >>> + CXL_DEV_CAP_METADATA_EXT, >>> + /* capabilities from Device Registers */ >>> + CXL_DEV_CAP_DEV_STATUS, >>> + CXL_DEV_CAP_MAILBOX_PRIMARY, >>> + CXL_DEV_CAP_MAILBOX_SECONDARY, >> Does the OS ever uses the SECONDARY mailbox? > > > I have no idea. I'm just listing all the potential capabilities here as you can see for things like BI or SNOOP. > > Should I just add those referenced by code? > > >>> + CXL_DEV_CAP_MEMDEV, >>> +}; >>> + >>> struct cxl_dev_state *cxl_accel_state_create(struct device *dev); >>> void cxl_set_dvsec(struct cxl_dev_state *cxlds, u16 dvsec);
diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c index 1d5007e3795a..39b20ddd0296 100644 --- a/drivers/cxl/core/port.c +++ b/drivers/cxl/core/port.c @@ -749,7 +749,7 @@ static struct cxl_port *cxl_port_alloc(struct device *uport_dev, } static int cxl_setup_comp_regs(struct device *host, struct cxl_register_map *map, - resource_size_t component_reg_phys) + resource_size_t component_reg_phys, u32 *caps) { *map = (struct cxl_register_map) { .host = host, @@ -763,7 +763,7 @@ static int cxl_setup_comp_regs(struct device *host, struct cxl_register_map *map map->reg_type = CXL_REGLOC_RBI_COMPONENT; map->max_size = CXL_COMPONENT_REG_BLOCK_SIZE; - return cxl_setup_regs(map); + return cxl_setup_regs(map, caps); } static int cxl_port_setup_regs(struct cxl_port *port, @@ -772,7 +772,7 @@ static int cxl_port_setup_regs(struct cxl_port *port, if (dev_is_platform(port->uport_dev)) return 0; return cxl_setup_comp_regs(&port->dev, &port->reg_map, - component_reg_phys); + component_reg_phys, &port->capabilities); } static int cxl_dport_setup_regs(struct device *host, struct cxl_dport *dport, @@ -789,7 +789,7 @@ static int cxl_dport_setup_regs(struct device *host, struct cxl_dport *dport, * NULL. */ rc = cxl_setup_comp_regs(dport->dport_dev, &dport->reg_map, - component_reg_phys); + component_reg_phys, &dport->port->capabilities); dport->reg_map.host = host; return rc; } @@ -858,6 +858,7 @@ static struct cxl_port *__devm_cxl_add_port(struct device *host, port->reg_map = cxlds->reg_map; port->reg_map.host = &port->dev; cxlmd->endpoint = port; + port->capabilities = cxlds->capabilities; } else if (parent_dport) { rc = dev_set_name(dev, "port%d", port->id); if (rc) diff --git a/drivers/cxl/core/regs.c b/drivers/cxl/core/regs.c index e1082e749c69..8b8abcadcb93 100644 --- a/drivers/cxl/core/regs.c +++ b/drivers/cxl/core/regs.c @@ -1,6 +1,7 @@ // SPDX-License-Identifier: GPL-2.0-only /* Copyright(c) 2020 Intel Corporation. */ #include <linux/io-64-nonatomic-lo-hi.h> +#include <linux/cxl/cxl.h> #include <linux/device.h> #include <linux/slab.h> #include <linux/pci.h> @@ -36,7 +37,7 @@ * Probe for component register information and return it in map object. */ void cxl_probe_component_regs(struct device *dev, void __iomem *base, - struct cxl_component_reg_map *map) + struct cxl_component_reg_map *map, u32 *caps) { int cap, cap_count; u32 cap_array; @@ -84,6 +85,7 @@ void cxl_probe_component_regs(struct device *dev, void __iomem *base, decoder_cnt = cxl_hdm_decoder_count(hdr); length = 0x20 * decoder_cnt + 0x10; rmap = &map->hdm_decoder; + *caps |= BIT(CXL_DEV_CAP_HDM); break; } case CXL_CM_CAP_CAP_ID_RAS: @@ -91,6 +93,7 @@ void cxl_probe_component_regs(struct device *dev, void __iomem *base, offset); length = CXL_RAS_CAPABILITY_LENGTH; rmap = &map->ras; + *caps |= BIT(CXL_DEV_CAP_RAS); break; default: dev_dbg(dev, "Unknown CM cap ID: %d (0x%x)\n", cap_id, @@ -117,7 +120,7 @@ EXPORT_SYMBOL_NS_GPL(cxl_probe_component_regs, CXL); * Probe for device register information and return it in map object. */ void cxl_probe_device_regs(struct device *dev, void __iomem *base, - struct cxl_device_reg_map *map) + struct cxl_device_reg_map *map, u32 *caps) { int cap, cap_count; u64 cap_array; @@ -146,10 +149,12 @@ void cxl_probe_device_regs(struct device *dev, void __iomem *base, case CXLDEV_CAP_CAP_ID_DEVICE_STATUS: dev_dbg(dev, "found Status capability (0x%x)\n", offset); rmap = &map->status; + *caps |= BIT(CXL_DEV_CAP_DEV_STATUS); break; case CXLDEV_CAP_CAP_ID_PRIMARY_MAILBOX: dev_dbg(dev, "found Mailbox capability (0x%x)\n", offset); rmap = &map->mbox; + *caps |= BIT(CXL_DEV_CAP_MAILBOX_PRIMARY); break; case CXLDEV_CAP_CAP_ID_SECONDARY_MAILBOX: dev_dbg(dev, "found Secondary Mailbox capability (0x%x)\n", offset); @@ -157,6 +162,7 @@ void cxl_probe_device_regs(struct device *dev, void __iomem *base, case CXLDEV_CAP_CAP_ID_MEMDEV: dev_dbg(dev, "found Memory Device capability (0x%x)\n", offset); rmap = &map->memdev; + *caps |= BIT(CXL_DEV_CAP_MEMDEV); break; default: if (cap_id >= 0x8000) @@ -421,7 +427,7 @@ static void cxl_unmap_regblock(struct cxl_register_map *map) map->base = NULL; } -static int cxl_probe_regs(struct cxl_register_map *map) +static int cxl_probe_regs(struct cxl_register_map *map, u32 *caps) { struct cxl_component_reg_map *comp_map; struct cxl_device_reg_map *dev_map; @@ -431,12 +437,12 @@ static int cxl_probe_regs(struct cxl_register_map *map) switch (map->reg_type) { case CXL_REGLOC_RBI_COMPONENT: comp_map = &map->component_map; - cxl_probe_component_regs(host, base, comp_map); + cxl_probe_component_regs(host, base, comp_map, caps); dev_dbg(host, "Set up component registers\n"); break; case CXL_REGLOC_RBI_MEMDEV: dev_map = &map->device_map; - cxl_probe_device_regs(host, base, dev_map); + cxl_probe_device_regs(host, base, dev_map, caps); if (!dev_map->status.valid || !dev_map->mbox.valid || !dev_map->memdev.valid) { dev_err(host, "registers not found: %s%s%s\n", @@ -455,7 +461,7 @@ static int cxl_probe_regs(struct cxl_register_map *map) return 0; } -int cxl_setup_regs(struct cxl_register_map *map) +int cxl_setup_regs(struct cxl_register_map *map, u32 *caps) { int rc; @@ -463,7 +469,7 @@ int cxl_setup_regs(struct cxl_register_map *map) if (rc) return rc; - rc = cxl_probe_regs(map); + rc = cxl_probe_regs(map, caps); cxl_unmap_regblock(map); return rc; diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h index 9afb407d438f..07c153aa3d77 100644 --- a/drivers/cxl/cxl.h +++ b/drivers/cxl/cxl.h @@ -284,9 +284,9 @@ struct cxl_register_map { }; void cxl_probe_component_regs(struct device *dev, void __iomem *base, - struct cxl_component_reg_map *map); + struct cxl_component_reg_map *map, u32 *caps); void cxl_probe_device_regs(struct device *dev, void __iomem *base, - struct cxl_device_reg_map *map); + struct cxl_device_reg_map *map, u32 *caps); int cxl_map_component_regs(const struct cxl_register_map *map, struct cxl_component_regs *regs, unsigned long map_mask); @@ -300,7 +300,7 @@ int cxl_find_regblock_instance(struct pci_dev *pdev, enum cxl_regloc_type type, struct cxl_register_map *map, int index); int cxl_find_regblock(struct pci_dev *pdev, enum cxl_regloc_type type, struct cxl_register_map *map); -int cxl_setup_regs(struct cxl_register_map *map); +int cxl_setup_regs(struct cxl_register_map *map, u32 *caps); struct cxl_dport; resource_size_t cxl_rcd_component_reg_phys(struct device *dev, struct cxl_dport *dport); @@ -600,6 +600,7 @@ struct cxl_dax_region { * @cdat: Cached CDAT data * @cdat_available: Should a CDAT attribute be available in sysfs * @pci_latency: Upstream latency in picoseconds + * @capabilities: those capabilities as defined in device mapped registers */ struct cxl_port { struct device dev; @@ -623,6 +624,7 @@ struct cxl_port { } cdat; bool cdat_available; long pci_latency; + u32 capabilities; }; /** diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h index afb53d058d62..37c043100300 100644 --- a/drivers/cxl/cxlmem.h +++ b/drivers/cxl/cxlmem.h @@ -424,6 +424,7 @@ struct cxl_dpa_perf { * @ram_res: Active Volatile memory capacity configuration * @serial: PCIe Device Serial Number * @type: Generic Memory Class device or Vendor Specific Memory device + * @capabilities: those capabilities as defined in device mapped registers */ struct cxl_dev_state { struct device *dev; @@ -438,6 +439,7 @@ struct cxl_dev_state { struct resource ram_res; u64 serial; enum cxl_devtype type; + u32 capabilities; }; /** diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c index 742a7b2a1be5..58f325019886 100644 --- a/drivers/cxl/pci.c +++ b/drivers/cxl/pci.c @@ -503,7 +503,7 @@ static int cxl_rcrb_get_comp_regs(struct pci_dev *pdev, } static int cxl_pci_setup_regs(struct pci_dev *pdev, enum cxl_regloc_type type, - struct cxl_register_map *map) + struct cxl_register_map *map, u32 *caps) { int rc; @@ -520,7 +520,7 @@ static int cxl_pci_setup_regs(struct pci_dev *pdev, enum cxl_regloc_type type, if (rc) return rc; - return cxl_setup_regs(map); + return cxl_setup_regs(map, caps); } static int cxl_pci_ras_unmask(struct pci_dev *pdev) @@ -827,7 +827,8 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) else cxl_set_dvsec(cxlds, dvsec); - rc = cxl_pci_setup_regs(pdev, CXL_REGLOC_RBI_MEMDEV, &map); + rc = cxl_pci_setup_regs(pdev, CXL_REGLOC_RBI_MEMDEV, &map, + &cxlds->capabilities); if (rc) return rc; @@ -840,7 +841,7 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) * still be useful for management functions so don't return an error. */ rc = cxl_pci_setup_regs(pdev, CXL_REGLOC_RBI_COMPONENT, - &cxlds->reg_map); + &cxlds->reg_map, &cxlds->capabilities); if (rc) dev_warn(&pdev->dev, "No component registers (%d)\n", rc); else if (!cxlds->reg_map.component_map.ras.valid) diff --git a/include/linux/cxl/cxl.h b/include/linux/cxl/cxl.h index e78eefa82123..930b1b9c1d6a 100644 --- a/include/linux/cxl/cxl.h +++ b/include/linux/cxl/cxl.h @@ -12,6 +12,36 @@ enum cxl_resource { CXL_ACCEL_RES_PMEM, }; +/* Capabilities as defined for: + * + * Component Registers (Table 8-22 CXL 3.0 specification) + * Device Registers (8.2.8.2.1 CXL 3.0 specification) + */ + +enum cxl_dev_cap { + /* capabilities from Component Registers */ + CXL_DEV_CAP_RAS, + CXL_DEV_CAP_SEC, + CXL_DEV_CAP_LINK, + CXL_DEV_CAP_HDM, + CXL_DEV_CAP_SEC_EXT, + CXL_DEV_CAP_IDE, + CXL_DEV_CAP_SNOOP_FILTER, + CXL_DEV_CAP_TIMEOUT_AND_ISOLATION, + CXL_DEV_CAP_CACHEMEM_EXT, + CXL_DEV_CAP_BI_ROUTE_TABLE, + CXL_DEV_CAP_BI_DECODER, + CXL_DEV_CAP_CACHEID_ROUTE_TABLE, + CXL_DEV_CAP_CACHEID_DECODER, + CXL_DEV_CAP_HDM_EXT, + CXL_DEV_CAP_METADATA_EXT, + /* capabilities from Device Registers */ + CXL_DEV_CAP_DEV_STATUS, + CXL_DEV_CAP_MAILBOX_PRIMARY, + CXL_DEV_CAP_MAILBOX_SECONDARY, + CXL_DEV_CAP_MEMDEV, +}; + struct cxl_dev_state *cxl_accel_state_create(struct device *dev); void cxl_set_dvsec(struct cxl_dev_state *cxlds, u16 dvsec);