diff mbox series

[v2,2/5] cxl/mem: Reserve all device regions at once

Message ID 20210522001154.2680157-3-ira.weiny@intel.com
State Superseded
Headers show
Series Map register blocks individually | expand

Commit Message

Ira Weiny May 22, 2021, 12:11 a.m. UTC
From: Ira Weiny <ira.weiny@intel.com>

In order to remap individual register sets each bar region must be
reserved prior to mapping.  Because the details of individual register
sets are contained within the BARs themselves, the bar must be mapped 2
times, once to extract this information and a second time for each
register set.

Rather than attempt to reserve each BAR individually and track if that
bar has been reserved.  Open code pcim_iomap_regions() by first
reserving all memory regions on the device and then mapping the bars
individually as needed.

NOTE pci_request_mem_regions() does not need a corresponding
pci_release_mem_regions() because the pci device is managed via
pcim_enable_device().

Signed-off-by: Ira Weiny <ira.weiny@intel.com>

---
Changes for V2:
	Rebased on https://git.kernel.org/pub/scm/linux/kernel/git/cxl/cxl.git/commit/?h=pending
	Clarify why pci_release_mem_regions() does not need to be
	called.
	Adjust for the different return code between pcim_iomap_regions() and
	pcim_iomap()
	Change print specifier.
---
 drivers/cxl/pci.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

Comments

Jonathan Cameron May 25, 2021, 9:54 a.m. UTC | #1
On Fri, 21 May 2021 17:11:51 -0700
<ira.weiny@intel.com> wrote:

> From: Ira Weiny <ira.weiny@intel.com>
> 
> In order to remap individual register sets each bar region must be
> reserved prior to mapping.  Because the details of individual register
> sets are contained within the BARs themselves, the bar must be mapped 2
> times, once to extract this information and a second time for each
> register set.
> 
> Rather than attempt to reserve each BAR individually and track if that
> bar has been reserved.  Open code pcim_iomap_regions() by first
> reserving all memory regions on the device and then mapping the bars
> individually as needed.
> 
> NOTE pci_request_mem_regions() does not need a corresponding
> pci_release_mem_regions() because the pci device is managed via
> pcim_enable_device().
> 
> Signed-off-by: Ira Weiny <ira.weiny@intel.com>

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> 
> ---
> Changes for V2:
> 	Rebased on https://git.kernel.org/pub/scm/linux/kernel/git/cxl/cxl.git/commit/?h=pending
> 	Clarify why pci_release_mem_regions() does not need to be
> 	called.

Gah - I'm never keen on hidden automated cleanup.  Oh well.

> 	Adjust for the different return code between pcim_iomap_regions() and
> 	pcim_iomap()
> 	Change print specifier.
> ---
>  drivers/cxl/pci.c | 18 +++++++++++-------
>  1 file changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> index b2f978954daa..33fc6e1634e3 100644
> --- a/drivers/cxl/pci.c
> +++ b/drivers/cxl/pci.c
> @@ -927,7 +927,7 @@ static void __iomem *cxl_mem_map_regblock(struct cxl_mem *cxlm,
>  {
>  	struct pci_dev *pdev = cxlm->pdev;
>  	struct device *dev = &pdev->dev;
> -	int rc;
> +	void __iomem *addr;
>  
>  	/* Basic sanity check that BAR is big enough */
>  	if (pci_resource_len(pdev, bar) < offset) {
> @@ -936,13 +936,14 @@ static void __iomem *cxl_mem_map_regblock(struct cxl_mem *cxlm,
>  		return IOMEM_ERR_PTR(-ENXIO);
>  	}
>  
> -	rc = pcim_iomap_regions(pdev, BIT(bar), pci_name(pdev));
> -	if (rc) {
> +	addr = pcim_iomap(pdev, bar, 0);
> +	if (!addr) {
>  		dev_err(dev, "failed to map registers\n");
> -		return IOMEM_ERR_PTR(rc);
> +		return addr;
>  	}
>  
> -	dev_dbg(dev, "Mapped CXL Memory Device resource\n");
> +	dev_dbg(dev, "Mapped CXL Memory Device resource bar %u @ %#llx\n",
> +		bar, offset);
>  
>  	return pcim_iomap_table(pdev)[bar] + offset;
>  }
> @@ -1003,6 +1004,9 @@ static int cxl_mem_setup_regs(struct cxl_mem *cxlm)
>  		return -ENXIO;
>  	}
>  
> +	if (pci_request_mem_regions(pdev, pci_name(pdev)))
> +		return -ENODEV;
> +
>  	/* Get the size of the Register Locator DVSEC */
>  	pci_read_config_dword(pdev, regloc + PCI_DVSEC_HEADER1, &regloc_size);
>  	regloc_size = FIELD_GET(PCI_DVSEC_HEADER1_LENGTH_MASK, regloc_size);
> @@ -1028,8 +1032,8 @@ static int cxl_mem_setup_regs(struct cxl_mem *cxlm)
>  
>  		if (reg_type == CXL_REGLOC_RBI_MEMDEV) {
>  			base = cxl_mem_map_regblock(cxlm, bar, offset);
> -			if (IS_ERR(base))
> -				return PTR_ERR(base);
> +			if (!base)
> +				return -ENOMEM;
>  			break;
>  		}
>  	}
diff mbox series

Patch

diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
index b2f978954daa..33fc6e1634e3 100644
--- a/drivers/cxl/pci.c
+++ b/drivers/cxl/pci.c
@@ -927,7 +927,7 @@  static void __iomem *cxl_mem_map_regblock(struct cxl_mem *cxlm,
 {
 	struct pci_dev *pdev = cxlm->pdev;
 	struct device *dev = &pdev->dev;
-	int rc;
+	void __iomem *addr;
 
 	/* Basic sanity check that BAR is big enough */
 	if (pci_resource_len(pdev, bar) < offset) {
@@ -936,13 +936,14 @@  static void __iomem *cxl_mem_map_regblock(struct cxl_mem *cxlm,
 		return IOMEM_ERR_PTR(-ENXIO);
 	}
 
-	rc = pcim_iomap_regions(pdev, BIT(bar), pci_name(pdev));
-	if (rc) {
+	addr = pcim_iomap(pdev, bar, 0);
+	if (!addr) {
 		dev_err(dev, "failed to map registers\n");
-		return IOMEM_ERR_PTR(rc);
+		return addr;
 	}
 
-	dev_dbg(dev, "Mapped CXL Memory Device resource\n");
+	dev_dbg(dev, "Mapped CXL Memory Device resource bar %u @ %#llx\n",
+		bar, offset);
 
 	return pcim_iomap_table(pdev)[bar] + offset;
 }
@@ -1003,6 +1004,9 @@  static int cxl_mem_setup_regs(struct cxl_mem *cxlm)
 		return -ENXIO;
 	}
 
+	if (pci_request_mem_regions(pdev, pci_name(pdev)))
+		return -ENODEV;
+
 	/* Get the size of the Register Locator DVSEC */
 	pci_read_config_dword(pdev, regloc + PCI_DVSEC_HEADER1, &regloc_size);
 	regloc_size = FIELD_GET(PCI_DVSEC_HEADER1_LENGTH_MASK, regloc_size);
@@ -1028,8 +1032,8 @@  static int cxl_mem_setup_regs(struct cxl_mem *cxlm)
 
 		if (reg_type == CXL_REGLOC_RBI_MEMDEV) {
 			base = cxl_mem_map_regblock(cxlm, bar, offset);
-			if (IS_ERR(base))
-				return PTR_ERR(base);
+			if (!base)
+				return -ENOMEM;
 			break;
 		}
 	}