Message ID | 20230523232214.55282-10-terry.bowman@amd.com |
---|---|
State | Superseded |
Headers | show |
Series | cxl/pci: Add support for RCH RAS error handling | expand |
On Tue, 23 May 2023 18:22:00 -0500 Terry Bowman <terry.bowman@amd.com> wrote: > From: Robert Richter <rrichter@amd.com> > > CXL RAS capabilities must be enabled and accessible as soon as the CXL > endpoint is detected in the PCI hierarchy and bound to the cxl_pci > driver. This needs to be independent of other modules such as cxl_port > or cxl_mem. > > CXL RAS capabilities reside in the Component Registers. For an RCH > this is determined by probing RCRB which is implemented very late once > the CXL Memory Device is created. > > Change this by moving the RCRB probe to the cxl_pci driver. Do this by > using a new introduced function cxl_pci_find_port() similar to > cxl_mem_find_port() to determine the involved dport by the endpoint's > PCI handle. Plug this into the existing cxl_pci_setup_regs() function > to setup Component Registers. Probe the RCRB in case the Component > Registers cannot be located through the CXL Register Locator > capability. > > This unifies code and early sets up the Component Registers at the > same time for both, VH and RCH mode. Only the cxl_pci driver is > involved for this. This allows an early mapping of the CXL RAS > capability registers. > > Signed-off-by: Robert Richter <rrichter@amd.com> > Signed-off-by: Terry Bowman <terry.bowman@amd.com> One minor wording suggestion inline. I'm don't really care that much about it though, so. Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c > index 945ca0304d68..54c486cd65dd 100644 > --- a/drivers/cxl/pci.c > +++ b/drivers/cxl/pci.c > @@ -274,13 +274,48 @@ static int cxl_pci_setup_mailbox(struct cxl_dev_state *cxlds) > return 0; > } > > +/* Extract RCRB, use same function interface as cxl_find_regblock(). */ > +static int cxl_rcrb_get_comp_regs(struct pci_dev *pdev, > + enum cxl_regloc_type type, > + struct cxl_register_map *map) > +{ > + struct cxl_dport *dport; > + resource_size_t component_reg_phys; > + > + memset(map, 0, sizeof(*map)); > + map->dev = &pdev->dev; > + map->resource = CXL_RESOURCE_NONE; > + > + if (type != CXL_REGLOC_RBI_COMPONENT) > + return -ENODEV; > + > + if (!cxl_pci_find_port(pdev, &dport) || !dport->rch) > + return -ENXIO; > + > + component_reg_phys = cxl_probe_rcrb(&pdev->dev, dport->rcrb.base, > + NULL, CXL_RCRB_UPSTREAM); > + if (component_reg_phys == CXL_RESOURCE_NONE) > + return -ENXIO; > + > + map->resource = component_reg_phys; > + map->reg_type = type; > + map->max_size = CXL_COMPONENT_REG_BLOCK_SIZE; > + > + return 0; > +} > + > static int cxl_pci_setup_regs(struct pci_dev *pdev, enum cxl_regloc_type type, > struct cxl_register_map *map) > { > int rc; > > + /* > + * If the Register Locator DVSEC does not contain the > + * Component Registers, try to extract them from the RCRB if > + * it is an RCH. My instinct here was to wonder why having said 'if it is an RCH' you don't seem to be checking that first. Perhaps change this text to something like. * Component Registers, assume it is an RCH and try to extra them * from an RCRB. */ ? > + */ > rc = cxl_find_regblock(pdev, type, map); > - if (rc) > + if (rc && cxl_rcrb_get_comp_regs(pdev, type, map)) > return rc; > > return cxl_setup_regs(map);
On 01.06.23 13:59:31, Jonathan Cameron wrote: > On Tue, 23 May 2023 18:22:00 -0500 > Terry Bowman <terry.bowman@amd.com> wrote: > > > From: Robert Richter <rrichter@amd.com> > > > > CXL RAS capabilities must be enabled and accessible as soon as the CXL > > endpoint is detected in the PCI hierarchy and bound to the cxl_pci > > driver. This needs to be independent of other modules such as cxl_port > > or cxl_mem. > > > > CXL RAS capabilities reside in the Component Registers. For an RCH > > this is determined by probing RCRB which is implemented very late once > > the CXL Memory Device is created. > > > > Change this by moving the RCRB probe to the cxl_pci driver. Do this by > > using a new introduced function cxl_pci_find_port() similar to > > cxl_mem_find_port() to determine the involved dport by the endpoint's > > PCI handle. Plug this into the existing cxl_pci_setup_regs() function > > to setup Component Registers. Probe the RCRB in case the Component > > Registers cannot be located through the CXL Register Locator > > capability. > > > > This unifies code and early sets up the Component Registers at the > > same time for both, VH and RCH mode. Only the cxl_pci driver is > > involved for this. This allows an early mapping of the CXL RAS > > capability registers. > > > > Signed-off-by: Robert Richter <rrichter@amd.com> > > Signed-off-by: Terry Bowman <terry.bowman@amd.com> > > One minor wording suggestion inline. I'm don't really care > that much about it though, so. > > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > > > > diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c > > index 945ca0304d68..54c486cd65dd 100644 > > --- a/drivers/cxl/pci.c > > +++ b/drivers/cxl/pci.c > > @@ -274,13 +274,48 @@ static int cxl_pci_setup_mailbox(struct cxl_dev_state *cxlds) > > return 0; > > } > > > > +/* Extract RCRB, use same function interface as cxl_find_regblock(). */ > > +static int cxl_rcrb_get_comp_regs(struct pci_dev *pdev, > > + enum cxl_regloc_type type, > > + struct cxl_register_map *map) > > +{ > > + struct cxl_dport *dport; > > + resource_size_t component_reg_phys; > > + > > + memset(map, 0, sizeof(*map)); > > + map->dev = &pdev->dev; > > + map->resource = CXL_RESOURCE_NONE; > > + > > + if (type != CXL_REGLOC_RBI_COMPONENT) > > + return -ENODEV; > > + > > + if (!cxl_pci_find_port(pdev, &dport) || !dport->rch) > > + return -ENXIO; > > + > > + component_reg_phys = cxl_probe_rcrb(&pdev->dev, dport->rcrb.base, > > + NULL, CXL_RCRB_UPSTREAM); > > + if (component_reg_phys == CXL_RESOURCE_NONE) > > + return -ENXIO; > > + > > + map->resource = component_reg_phys; > > + map->reg_type = type; > > + map->max_size = CXL_COMPONENT_REG_BLOCK_SIZE; > > + > > + return 0; > > +} > > + > > static int cxl_pci_setup_regs(struct pci_dev *pdev, enum cxl_regloc_type type, > > struct cxl_register_map *map) > > { > > int rc; > > > > + /* > > + * If the Register Locator DVSEC does not contain the > > + * Component Registers, try to extract them from the RCRB if > > + * it is an RCH. > > My instinct here was to wonder why having said 'if it is an RCH' > you don't seem to be checking that first. Perhaps > change this text to something like. > * Component Registers, assume it is an RCH and try to extra them > * from an RCRB. > */ > ? Will change that. Thanks for review, -Robert > > > + */ > > rc = cxl_find_regblock(pdev, type, map); > > - if (rc) > > + if (rc && cxl_rcrb_get_comp_regs(pdev, type, map)) > > return rc; > > > > return cxl_setup_regs(map); >
diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c index 66f567480238..eff91f141fde 100644 --- a/drivers/cxl/core/port.c +++ b/drivers/cxl/core/port.c @@ -1477,6 +1477,13 @@ int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd) } EXPORT_SYMBOL_NS_GPL(devm_cxl_enumerate_ports, CXL); +struct cxl_port *cxl_pci_find_port(struct pci_dev *pdev, + struct cxl_dport **dport) +{ + return find_cxl_port(pdev->dev.parent, dport); +} +EXPORT_SYMBOL_NS_GPL(cxl_pci_find_port, CXL); + struct cxl_port *cxl_mem_find_port(struct cxl_memdev *cxlmd, struct cxl_dport **dport) { diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h index 1c6fe53e9dc7..e5ae5f4e6669 100644 --- a/drivers/cxl/cxl.h +++ b/drivers/cxl/cxl.h @@ -670,6 +670,8 @@ struct cxl_port *find_cxl_root(struct cxl_port *port); int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd); void cxl_bus_rescan(void); void cxl_bus_drain(void); +struct cxl_port *cxl_pci_find_port(struct pci_dev *pdev, + struct cxl_dport **dport); struct cxl_port *cxl_mem_find_port(struct cxl_memdev *cxlmd, struct cxl_dport **dport); bool schedule_cxl_memdev_detach(struct cxl_memdev *cxlmd); diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c index 7ecdaa7f9315..0643852444f3 100644 --- a/drivers/cxl/mem.c +++ b/drivers/cxl/mem.c @@ -65,18 +65,6 @@ static int devm_cxl_add_endpoint(struct device *host, struct cxl_memdev *cxlmd, ep->next = down; } - /* - * The component registers for an RCD might come from the - * host-bridge RCRB if they are not already mapped via the - * typical register locator mechanism. - */ - if (parent_dport->rch && - cxlds->component_reg_phys == CXL_RESOURCE_NONE) { - cxlds->component_reg_phys = - cxl_probe_rcrb(&cxlmd->dev, parent_dport->rcrb.base, - NULL, CXL_RCRB_UPSTREAM); - } - endpoint = devm_cxl_add_port(host, &cxlmd->dev, cxlds->component_reg_phys, parent_dport); diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c index 945ca0304d68..54c486cd65dd 100644 --- a/drivers/cxl/pci.c +++ b/drivers/cxl/pci.c @@ -274,13 +274,48 @@ static int cxl_pci_setup_mailbox(struct cxl_dev_state *cxlds) return 0; } +/* Extract RCRB, use same function interface as cxl_find_regblock(). */ +static int cxl_rcrb_get_comp_regs(struct pci_dev *pdev, + enum cxl_regloc_type type, + struct cxl_register_map *map) +{ + struct cxl_dport *dport; + resource_size_t component_reg_phys; + + memset(map, 0, sizeof(*map)); + map->dev = &pdev->dev; + map->resource = CXL_RESOURCE_NONE; + + if (type != CXL_REGLOC_RBI_COMPONENT) + return -ENODEV; + + if (!cxl_pci_find_port(pdev, &dport) || !dport->rch) + return -ENXIO; + + component_reg_phys = cxl_probe_rcrb(&pdev->dev, dport->rcrb.base, + NULL, CXL_RCRB_UPSTREAM); + if (component_reg_phys == CXL_RESOURCE_NONE) + return -ENXIO; + + map->resource = component_reg_phys; + map->reg_type = type; + map->max_size = CXL_COMPONENT_REG_BLOCK_SIZE; + + return 0; +} + static int cxl_pci_setup_regs(struct pci_dev *pdev, enum cxl_regloc_type type, struct cxl_register_map *map) { int rc; + /* + * If the Register Locator DVSEC does not contain the + * Component Registers, try to extract them from the RCRB if + * it is an RCH. + */ rc = cxl_find_regblock(pdev, type, map); - if (rc) + if (rc && cxl_rcrb_get_comp_regs(pdev, type, map)) return rc; return cxl_setup_regs(map);