On Thu, 01 Dec 2022 13:34:05 -0800 Dan Williams <[email protected]> wrote:
> From: Robert Richter <[email protected]> > > A downstream port must be connected to a component register block. > For restricted hosts the base address is determined from the RCRB. The > RCRB is provided by the host's CEDT CHBS entry. Rework CEDT parser to > get the RCRB and add code to extract the component register block from > it. > > RCRB's BAR[0..1] point to the component block containing CXL subsystem > component registers. MEMBAR extraction follows the PCI base spec here, > esp. 64 bit extraction and memory range alignment (6.0, 7.5.1.2.1). The > RCRB base address is cached in the cxl_dport per-host bridge so that the > upstream port component registers can be retrieved later by an RCD > (RCIEP) associated with the host bridge. > > Note: Right now the component register block is used for HDM decoder > capability only which is optional for RCDs. If unsupported by the RCD, > the HDM init will fail. It is future work to bypass it in this case. > > Co-developed-by: Terry Bowman <[email protected]> > Signed-off-by: Terry Bowman <[email protected]> > Signed-off-by: Robert Richter <[email protected]> > Link: https://lore.kernel.org/r/[email protected] > [djbw: introduce devm_cxl_add_rch_dport()] > Signed-off-by: Dan Williams <[email protected]> Trivial moans that may have something to do with it being near going home time on a Friday. Otherwise looks sensible though this was a fairly superficial look. Reviewed-by: Jonathan Cameron <[email protected]> > --- > drivers/cxl/acpi.c | 51 ++++++++++++++++++++++++++++----- > drivers/cxl/core/port.c | 53 ++++++++++++++++++++++++++++++---- > drivers/cxl/core/regs.c | 64 > +++++++++++++++++++++++++++++++++++++++++ > drivers/cxl/cxl.h | 16 ++++++++++ > tools/testing/cxl/Kbuild | 1 + > tools/testing/cxl/test/cxl.c | 10 ++++++ > tools/testing/cxl/test/mock.c | 19 ++++++++++++ > tools/testing/cxl/test/mock.h | 3 ++ > 8 files changed, 203 insertions(+), 14 deletions(-) > > diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c > index 50d82376097c..db8173f3ee10 100644 > --- a/drivers/cxl/acpi.c > +++ b/drivers/cxl/acpi.c > struct cxl_chbs_context { > - struct device *dev; > - unsigned long long uid; > - resource_size_t chbcr; > + struct device *dev; > + unsigned long long uid; > + resource_size_t rcrb; > + resource_size_t chbcr; > + u32 cxl_version; > }; I'm not keen on this style change because it slightly obscures the meaningful changes in this diff + I suspect it's not consistent with rest of the file. > diff --git a/drivers/cxl/core/regs.c b/drivers/cxl/core/regs.c > index ec178e69b18f..28ed0ec8ee3e 100644 > --- a/drivers/cxl/core/regs.c > +++ b/drivers/cxl/core/regs.c > @@ -307,3 +307,67 @@ int cxl_find_regblock(struct pci_dev *pdev, enum > cxl_regloc_type type, > return -ENODEV; > } > EXPORT_SYMBOL_NS_GPL(cxl_find_regblock, CXL); > + > +resource_size_t cxl_rcrb_to_component(struct device *dev, > + resource_size_t rcrb, > + enum cxl_rcrb which) > +{ > + resource_size_t component_reg_phys; > + u32 bar0, bar1; > + void *addr; > + u16 cmd; > + u32 id; > + > + if (which == CXL_RCRB_UPSTREAM) > + rcrb += SZ_4K; > + > + /* > + * RCRB's BAR[0..1] point to component block containing CXL > + * subsystem component registers. MEMBAR extraction follows > + * the PCI Base spec here, esp. 64 bit extraction and memory > + * ranges alignment (6.0, 7.5.1.2.1). > + */ > + if (!request_mem_region(rcrb, SZ_4K, "CXL RCRB")) > + return CXL_RESOURCE_NONE; > + addr = ioremap(rcrb, SZ_4K); > + if (!addr) { > + dev_err(dev, "Failed to map region %pr\n", addr); > + release_mem_region(rcrb, SZ_4K); > + return CXL_RESOURCE_NONE; > + } > + > + id = readl(addr + PCI_VENDOR_ID); > + cmd = readw(addr + PCI_COMMAND); > + bar0 = readl(addr + PCI_BASE_ADDRESS_0); > + bar1 = readl(addr + PCI_BASE_ADDRESS_1); > + iounmap(addr); > + release_mem_region(rcrb, SZ_4K); > + > + /* > + * Sanity check, see CXL 3.0 Figure 9-8 CXL Device that Does Not > + * Remap Upstream Port and Component Registers > + */ > + if (id == U32_MAX) { > + if (which == CXL_RCRB_DOWNSTREAM) > + dev_err(dev, "Failed to access Downstream Port RCRB\n"); > + return CXL_RESOURCE_NONE; > + } > + if (!(cmd & PCI_COMMAND_MEMORY)) > + return CXL_RESOURCE_NONE; > + if (bar0 & (PCI_BASE_ADDRESS_MEM_TYPE_1M | PCI_BASE_ADDRESS_SPACE_IO)) Trivial: A positive match on what we do want might be better... I had to got look up MEM_TYPE_1M to find out what on earth it was (marked obsolete which I guess isn't surprising.... ) Up to you though... > + return CXL_RESOURCE_NONE; > + > + component_reg_phys = bar0 & PCI_BASE_ADDRESS_MEM_MASK; > + if (bar0 & PCI_BASE_ADDRESS_MEM_TYPE_64) > + component_reg_phys |= ((u64)bar1) << 32; > + > + if (!component_reg_phys) > + return CXL_RESOURCE_NONE; > + > + /* MEMBAR is block size (64k) aligned. */ > + if (!IS_ALIGNED(component_reg_phys, CXL_COMPONENT_REG_BLOCK_SIZE)) > + return CXL_RESOURCE_NONE; > + > + return component_reg_phys; > +} > +EXPORT_SYMBOL_NS_GPL(cxl_rcrb_to_component, CXL); > diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h > index 281b1db5a271..1342e4e61537 100644 > --- a/drivers/cxl/cxl.h > +++ b/drivers/cxl/cxl.h > #define CXL_RESOURCE_NONE ((resource_size_t) -1) > #define CXL_TARGET_STRLEN 20 > > @@ -486,12 +494,16 @@ cxl_find_dport_by_dev(struct cxl_port *port, const > struct device *dport_dev) > * @dport: PCI bridge or firmware device representing the downstream link > * @port_id: unique hardware identifier for dport in decoder target list > * @component_reg_phys: downstream port component registers > + * @rcrb: base address for the Root Complex Register Block > + * @rch: Indicate whether this dport was enumerated in RCH or VH mode Clarify this as Indicate this dport was enumerated in RCH rather than VH mode. a boolean with an or in the comment is confusing! > * @port: reference to cxl_port that contains this downstream port > */ > struct cxl_dport { > struct device *dport; > int port_id; > resource_size_t component_reg_phys; > + resource_size_t rcrb; > + bool rch; > struct cxl_port *port; > };
