RE: [PATCH v5 19/26] cxl/pci: Add RCH downstream port AER register discovery

[Dan Williams]

Terry Bowman wrote:
> Restricted CXL host (RCH) downstream port AER information is not currently
> logged while in the error state. One problem preventing the error logging
> is the AER and RAS registers are not accessible. The CXL driver requires
> changes to find RCH downstream port AER and RAS registers for purpose of
> error logging.
> 
> RCH downstream ports are not enumerated during a PCI bus scan and are
> instead discovered using system firmware, ACPI in this case.[1] The
> downstream port is implemented as a Root Complex Register Block (RCRB).
> The RCRB is a 4k memory block containing PCIe registers based on the PCIe
> root port.[2] The RCRB includes AER extended capability registers used for
> reporting errors. Note, the RCH's AER Capability is located in the RCRB
> memory space instead of PCI configuration space, thus its register access
> is different. Existing kernel PCIe AER functions can not be used to manage
> the downstream port AER capabilities and RAS registers because the port was
> not enumerated during PCI scan and the registers are not PCI config
> accessible.
> 
> Discover RCH downstream port AER extended capability registers. Use MMIO
> accesses to search for extended AER capability in RCRB register space.
> 
> [1] CXL 3.0 Spec, 9.11.2 - System Firmware View of CXL 1.1 Hierarchy
> [2] CXL 3.0 Spec, 8.2.1.1 - RCH Downstream Port RCRB
> 
> Co-developed-by: Robert Richter <rrichter@xxxxxxx>
> Signed-off-by: Robert Richter <rrichter@xxxxxxx>
> Signed-off-by: Terry Bowman <terry.bowman@xxxxxxx>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
> ---
>  drivers/cxl/core/regs.c | 51 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 51 insertions(+)
> 
> diff --git a/drivers/cxl/core/regs.c b/drivers/cxl/core/regs.c
> index ba2b1763042c..dd6c3c898cff 100644
> --- a/drivers/cxl/core/regs.c
> +++ b/drivers/cxl/core/regs.c
> @@ -408,6 +408,54 @@ int cxl_setup_regs(struct cxl_register_map *map)
>  }
>  EXPORT_SYMBOL_NS_GPL(cxl_setup_regs, CXL);
>  
> +static void __iomem *cxl_map_reg(struct device *dev, resource_size_t addr,
> +				resource_size_t length)
> +{
> +	struct resource *res;
> +
> +	if (WARN_ON_ONCE(addr == CXL_RESOURCE_NONE))
> +		return NULL;
> +
> +	res = request_mem_region(addr, length, dev_name(dev));
> +	if (!res)
> +		return NULL;
> +
> +	return ioremap(addr, length);
> +}
> +
> +static void cxl_unmap_reg(void __iomem *base, resource_size_t addr,
> +			 resource_size_t length)
> +{
> +	iounmap(base);
> +	release_mem_region(addr, length);
> +}

Why redo the {request,release}_mem_region() and ioremap() vs handling
this inside of the existing mapping of the RCRB in this function?