[v5,3/6] PCI: endpoint: Introduce pci_epc_mem_map()/unmap()

Some endpoint controllers have requirements on the alignment of the
controller physical memory address that must be used to map a RC PCI
address region. For instance, the endpoint controller of the RK3399 SoC
uses at most the lower 20 bits of a physical memory address region as
the lower bits of a RC PCI address region. For mapping a PCI address
region of size bytes starting from pci_addr, the exact number of
address bits used is the number of address bits changing in the address
range [pci_addr..pci_addr + size - 1]. For this example, this creates
the following constraints:
1) The offset into the controller physical memory allocated for a
   mapping depends on the mapping size *and* the starting PCI address
   for the mapping.
2) A mapping size cannot exceed the controller windows size (1MB) minus
   the offset needed into the allocated physical memory, which can end
   up being a smaller size than the desired mapping size.

Handling these constraints independently of the controller being used
in an endpoint function driver is not possible with the current EPC
API as only the ->align field in struct pci_epc_features is provided
but used for BAR (inbound ATU mappings) mapping only. A new API is
needed for function drivers to discover mapping constraints and handle
non-static requirements based on the RC PCI address range to access.

Introduce endpoint controller operation ->get_mem_map() to allow
the EPC core functions to obtain the size and the offset into a
controller address region that must be allocated and mapped to access
a RC PCI address region. The size of the mapping provided by the
get_mem_map() operation can then be used as the size argument for the
function pci_epc_mem_alloc_addr() and the offset into the allocated
controller memory provided can be used to correctly handle data
transfers. For endpoint controllers that have PCI address alignment
constraints, get_mem_map() may indicate upon return an effective PCI
address region size that is smaller (but not 0) than the requested PCI
address region size.

The controller ->get_mem_map() operation is optional: controllers that
do not have any alignment constraints for mapping RC PCI address regions
do not need to implement this operation. For such controllers, it is
always assumed that the mapping size is equal to the requested size of
the PCI region and that the mapping offset is 0.

The function pci_epc_mem_map() is introduced to use this new controller
operation (if it is defined) to handle controller memory allocation and
mapping to a RC PCI address region in endpoint function drivers.

This function first uses the ->get_mem_map() controller operation to
determine the controller memory address size (and offset into) needed
for mapping an RC PCI address region.  The result of this function
operation is used to allocate a controller physical memory region using
pci_epc_mem_alloc_addr() and then to map that memory to the RC PCI
address space with pci_epc_map_addr().

Since ->get_mem_map() () may indicate that not all of a  RC PCI
address region can be mapped, pci_epc_mem_map() may only partially map
the RC PCI address region specified. It is the responsibility of the
caller (an endpoint function driver) to handle such smaller mapping
by repeatedly using pci_epc_mem_map() over the desried PCI address
range.

The counterpart of pci_epc_mem_map() to unmap and free the controller
memory address region is pci_epc_mem_unmap().

Both functions as well as the ->get_mem_map() controller operation
operate using the new struct pci_epc_map data structure. This new
structure represents a mapping PCI address, mapping effective size, the
size and offset into the controller memory needed for the mapping as
well as the physical and virtual CPU addresses of the mapping (phys_base
and virt_base fields). For convenience, the physical and virtual CPU
addresses within that mapping to access the target RC PCI address region
are also provided (phys_addr and virt_addr fields).

Endpoint function drivers can use struct pci_epc_map to access the
mapped RC PCI address region using the ->virt_addr and ->pci_size
fields.

Co-developed-by: Rick Wertenbroek <rick.wertenbroek@gmail.com>
Signed-off-by: Rick Wertenbroek <rick.wertenbroek@gmail.com>
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
---
 drivers/pci/endpoint/pci-epc-core.c | 126 ++++++++++++++++++++++++++++
 include/linux/pci-epc.h             |  39 +++++++++
 2 files changed, 165 insertions(+)

On Fri, Oct 11, 2024 at 09:01:12PM +0900, Damien Le Moal wrote:
> Some endpoint controllers have requirements on the alignment of the
> controller physical memory address that must be used to map a RC PCI
> address region. For instance, the endpoint controller of the RK3399 SoC
> uses at most the lower 20 bits of a physical memory address region as
> the lower bits of a RC PCI address region. For mapping a PCI address
> region of size bytes starting from pci_addr, the exact number of
> address bits used is the number of address bits changing in the address
> range [pci_addr..pci_addr + size - 1]. For this example, this creates
> the following constraints:
> 1) The offset into the controller physical memory allocated for a
>    mapping depends on the mapping size *and* the starting PCI address
>    for the mapping.
> 2) A mapping size cannot exceed the controller windows size (1MB) minus
>    the offset needed into the allocated physical memory, which can end
>    up being a smaller size than the desired mapping size.
> 
> Handling these constraints independently of the controller being used
> in an endpoint function driver is not possible with the current EPC
> API as only the ->align field in struct pci_epc_features is provided
> but used for BAR (inbound ATU mappings) mapping only. A new API is
> needed for function drivers to discover mapping constraints and handle
> non-static requirements based on the RC PCI address range to access.
> 
> Introduce endpoint controller operation ->get_mem_map() to allow
> the EPC core functions to obtain the size and the offset into a
> controller address region that must be allocated and mapped to access
> a RC PCI address region. The size of the mapping provided by the
> get_mem_map() operation can then be used as the size argument for the
> function pci_epc_mem_alloc_addr() and the offset into the allocated
> controller memory provided can be used to correctly handle data
> transfers. For endpoint controllers that have PCI address alignment
> constraints, get_mem_map() may indicate upon return an effective PCI
> address region size that is smaller (but not 0) than the requested PCI
> address region size.
> 
> The controller ->get_mem_map() operation is optional: controllers that
> do not have any alignment constraints for mapping RC PCI address regions
> do not need to implement this operation. For such controllers, it is
> always assumed that the mapping size is equal to the requested size of
> the PCI region and that the mapping offset is 0.
> 
> The function pci_epc_mem_map() is introduced to use this new controller
> operation (if it is defined) to handle controller memory allocation and
> mapping to a RC PCI address region in endpoint function drivers.
> 
> This function first uses the ->get_mem_map() controller operation to
> determine the controller memory address size (and offset into) needed
> for mapping an RC PCI address region.  The result of this function
> operation is used to allocate a controller physical memory region using
> pci_epc_mem_alloc_addr() and then to map that memory to the RC PCI
> address space with pci_epc_map_addr().
> 
> Since ->get_mem_map() () may indicate that not all of a  RC PCI
> address region can be mapped, pci_epc_mem_map() may only partially map
> the RC PCI address region specified. It is the responsibility of the
> caller (an endpoint function driver) to handle such smaller mapping
> by repeatedly using pci_epc_mem_map() over the desried PCI address
> range.
> 
> The counterpart of pci_epc_mem_map() to unmap and free the controller
> memory address region is pci_epc_mem_unmap().
> 
> Both functions as well as the ->get_mem_map() controller operation
> operate using the new struct pci_epc_map data structure. This new
> structure represents a mapping PCI address, mapping effective size, the
> size and offset into the controller memory needed for the mapping as
> well as the physical and virtual CPU addresses of the mapping (phys_base
> and virt_base fields). For convenience, the physical and virtual CPU
> addresses within that mapping to access the target RC PCI address region
> are also provided (phys_addr and virt_addr fields).
> 
> Endpoint function drivers can use struct pci_epc_map to access the
> mapped RC PCI address region using the ->virt_addr and ->pci_size
> fields.
> 
> Co-developed-by: Rick Wertenbroek <rick.wertenbroek@gmail.com>
> Signed-off-by: Rick Wertenbroek <rick.wertenbroek@gmail.com>
> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
> ---
>  drivers/pci/endpoint/pci-epc-core.c | 126 ++++++++++++++++++++++++++++
>  include/linux/pci-epc.h             |  39 +++++++++
>  2 files changed, 165 insertions(+)
> 
> diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
> index b854f1bab26f..b6bf6d9f9f85 100644
> --- a/drivers/pci/endpoint/pci-epc-core.c
> +++ b/drivers/pci/endpoint/pci-epc-core.c
> @@ -466,6 +466,132 @@ int pci_epc_map_addr(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
>  }
>  EXPORT_SYMBOL_GPL(pci_epc_map_addr);
>  
> +/*
> + * Determine the actual PCI address range that should be mapped to access
> + * @pci_size from @pci_addr. This is done using the controller get_mem_map
> + * operation if that operation is defined. Otherwise, simply assume that the
> + * controller has no mapping alignment constraint and return the requested range
> + * as-is.
> + */
> +static int pci_epc_get_mem_map(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> +			       u64 pci_addr, size_t pci_size,
> +			       struct pci_epc_map *map)
> +{
> +	int ret;
> +
> +	/*
> +	 * Initialize and remember the PCI address region to be mapped. The
> +	 * controller ->get_mem_map() operation may change the map->pci_size to a
> +	 * smaller value.
> +	 */
> +	memset(map, 0, sizeof(*map));
> +	map->pci_addr = pci_addr;
> +	map->pci_size = pci_size;
> +
> +	if (!epc->ops->get_mem_map) {
> +		/*
> +		 * Assume that the EP controller has no alignment constraint,
> +		 * that is, that the PCI address to map and the size of the
> +		 * controller memory needed for the mapping are the same as
> +		 * specified by the caller.
> +		 */
> +		map->map_pci_addr = pci_addr;
> +		map->map_size = pci_size;
> +		map->map_ofst = 0;
> +		return 0;
> +	}
> +
> +	mutex_lock(&epc->lock);
> +	ret = epc->ops->get_mem_map(epc, func_no, vfunc_no, map);
> +	mutex_unlock(&epc->lock);
> +
> +	return ret;
> +}
> +
> +/**
> + * pci_epc_mem_map() - allocate and map a PCI address to a CPU address
> + * @epc: the EPC device on which the CPU address is to be allocated and mapped
> + * @func_no: the physical endpoint function number in the EPC device
> + * @vfunc_no: the virtual endpoint function number in the physical function
> + * @pci_addr: PCI address to which the CPU address should be mapped
> + * @pci_size: the number of bytes to map starting from @pci_addr
> + * @map: where to return the mapping information
> + *
> + * Allocate a controller memory address region and map it to a RC PCI address
> + * region, taking into account the controller physical address mapping
> + * constraints using pci_epc_get_mem_map().
> + * The effective size of the PCI address range mapped from @pci_addr is
> + * indicated by @map->pci_size. This size may be less than the requested
> + * @pci_size. The local virtual CPU address for the mapping is indicated by
> + * @map->virt_addr (@map->phys_addr indicates the physical address).
> + * The size and CPU address of the controller memory allocated and mapped are
> + * respectively indicated by @map->map_size and @map->virt_base (and
> + * @map->phys_base).
> + *
> + * Returns 0 on success and a negative error code in case of error.
> + */
> +int pci_epc_mem_map(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> +		    u64 pci_addr, size_t pci_size, struct pci_epc_map *map)
> +{
> +	int ret;
> +
> +	if (!pci_epc_function_is_valid(epc, func_no, vfunc_no))
> +		return -EINVAL;
> +
> +	if (!pci_size || !map)
> +		return -EINVAL;
> +
> +	ret = pci_epc_get_mem_map(epc, func_no, vfunc_no,
> +				  pci_addr, pci_size, map);
> +	if (ret)
> +		return ret;
> +
> +	map->virt_base = pci_epc_mem_alloc_addr(epc, &map->phys_base,
> +						map->map_size);
> +	if (!map->virt_base)
> +		return -ENOMEM;
> +
> +	map->phys_addr = map->phys_base + map->map_ofst;
> +	map->virt_addr = map->virt_base + map->map_ofst;
> +
> +	ret = pci_epc_map_addr(epc, func_no, vfunc_no, map->phys_base,
> +			       map->map_pci_addr, map->map_size);
> +	if (ret) {
> +		pci_epc_mem_free_addr(epc, map->phys_base, map->virt_base,
> +				      map->map_size);
> +		map->virt_base = 0;

As reported by the kernel test robot on both v3 and v4, this should be:
map->virt_base = NULL;
otherwise you introduce a new sparse warning.


> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(pci_epc_mem_map);
> +
> +/**
> + * pci_epc_mem_unmap() - unmap and free a CPU address region
> + * @epc: the EPC device on which the CPU address is allocated and mapped
> + * @func_no: the physical endpoint function number in the EPC device
> + * @vfunc_no: the virtual endpoint function number in the physical function
> + * @map: the mapping information
> + *
> + * Unmap and free a CPU address region that was allocated and mapped with
> + * pci_epc_mem_map().
> + */
> +void pci_epc_mem_unmap(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> +		       struct pci_epc_map *map)
> +{
> +	if (!pci_epc_function_is_valid(epc, func_no, vfunc_no))
> +		return;
> +
> +	if (!map || !map->virt_base)
> +		return;
> +
> +	pci_epc_unmap_addr(epc, func_no, vfunc_no, map->phys_base);
> +	pci_epc_mem_free_addr(epc, map->phys_base, map->virt_base,
> +			      map->map_size);
> +}
> +EXPORT_SYMBOL_GPL(pci_epc_mem_unmap);
> +
>  /**
>   * pci_epc_clear_bar() - reset the BAR
>   * @epc: the EPC device for which the BAR has to be cleared
> diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
> index 42ef06136bd1..b5f5c1eb54c5 100644
> --- a/include/linux/pci-epc.h
> +++ b/include/linux/pci-epc.h
> @@ -32,11 +32,44 @@ pci_epc_interface_string(enum pci_epc_interface_type type)
>  	}
>  }
>  
> +/**
> + * struct pci_epc_map - information about EPC memory for mapping a RC PCI
> + *                      address range
> + * @pci_addr: start address of the RC PCI address range to map
> + * @pci_size: size of the RC PCI address range mapped from @pci_addr
> + * @map_pci_addr: RC PCI address used as the first address mapped (may be lower
> + *                than @pci_addr)
> + * @map_size: size of the controller memory needed for mapping the RC PCI address
> + *            range @pci_addr..@pci_addr+@pci_size
> + * @map_ofst: offset into the mapped controller memory to access @pci_addr
> + * @phys_base: base physical address of the allocated EPC memory for mapping the
> + *             RC PCI address range
> + * @phys_addr: physical address at which @pci_addr is mapped
> + * @virt_base: base virtual address of the allocated EPC memory for mapping the
> + *             RC PCI address range
> + * @virt_addr: virtual address at which @pci_addr is mapped
> + */
> +struct pci_epc_map {
> +	phys_addr_t	pci_addr;
> +	size_t		pci_size;
> +
> +	phys_addr_t	map_pci_addr;
> +	size_t		map_size;
> +	phys_addr_t	map_ofst;
> +
> +	phys_addr_t	phys_base;
> +	phys_addr_t	phys_addr;
> +	void __iomem	*virt_base;
> +	void __iomem	*virt_addr;
> +};
> +
>  /**
>   * struct pci_epc_ops - set of function pointers for performing EPC operations
>   * @write_header: ops to populate configuration space header
>   * @set_bar: ops to configure the BAR
>   * @clear_bar: ops to reset the BAR
> + * @get_mem_map: operation to get the size and offset into a controller memory
> + *               window needed to map an RC PCI address region
>   * @map_addr: ops to map CPU address to PCI address
>   * @unmap_addr: ops to unmap CPU address and PCI address
>   * @set_msi: ops to set the requested number of MSI interrupts in the MSI
> @@ -61,6 +94,8 @@ struct pci_epc_ops {
>  			   struct pci_epf_bar *epf_bar);
>  	void	(*clear_bar)(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
>  			     struct pci_epf_bar *epf_bar);
> +	int	(*get_mem_map)(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> +			       struct pci_epc_map *map);
>  	int	(*map_addr)(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
>  			    phys_addr_t addr, u64 pci_addr, size_t size);
>  	void	(*unmap_addr)(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> @@ -278,6 +313,10 @@ void __iomem *pci_epc_mem_alloc_addr(struct pci_epc *epc,
>  				     phys_addr_t *phys_addr, size_t size);
>  void pci_epc_mem_free_addr(struct pci_epc *epc, phys_addr_t phys_addr,
>  			   void __iomem *virt_addr, size_t size);
> +int pci_epc_mem_map(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> +		    u64 pci_addr, size_t pci_size, struct pci_epc_map *map);
> +void pci_epc_mem_unmap(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> +		       struct pci_epc_map *map);
>  
>  #else
>  static inline void pci_epc_init_notify(struct pci_epc *epc)
> -- 
> 2.47.0
>

On 10/11/24 21:07, Niklas Cassel wrote:
>> +int pci_epc_mem_map(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
>> +		    u64 pci_addr, size_t pci_size, struct pci_epc_map *map)
>> +{
>> +	int ret;
>> +
>> +	if (!pci_epc_function_is_valid(epc, func_no, vfunc_no))
>> +		return -EINVAL;
>> +
>> +	if (!pci_size || !map)
>> +		return -EINVAL;
>> +
>> +	ret = pci_epc_get_mem_map(epc, func_no, vfunc_no,
>> +				  pci_addr, pci_size, map);
>> +	if (ret)
>> +		return ret;
>> +
>> +	map->virt_base = pci_epc_mem_alloc_addr(epc, &map->phys_base,
>> +						map->map_size);
>> +	if (!map->virt_base)
>> +		return -ENOMEM;
>> +
>> +	map->phys_addr = map->phys_base + map->map_ofst;
>> +	map->virt_addr = map->virt_base + map->map_ofst;
>> +
>> +	ret = pci_epc_map_addr(epc, func_no, vfunc_no, map->phys_base,
>> +			       map->map_pci_addr, map->map_size);
>> +	if (ret) {
>> +		pci_epc_mem_free_addr(epc, map->phys_base, map->virt_base,
>> +				      map->map_size);
>> +		map->virt_base = 0;
> 
> As reported by the kernel test robot on both v3 and v4, this should be:
> map->virt_base = NULL;
> otherwise you introduce a new sparse warning.

Oops. Missed that... OK, sending v6 with this removed as that is not necessary
anyway.

> 
> 
>> +		return ret;
>> +	}
>> +
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(pci_epc_mem_map);
>> +
>> +/**
>> + * pci_epc_mem_unmap() - unmap and free a CPU address region
>> + * @epc: the EPC device on which the CPU address is allocated and mapped
>> + * @func_no: the physical endpoint function number in the EPC device
>> + * @vfunc_no: the virtual endpoint function number in the physical function
>> + * @map: the mapping information
>> + *
>> + * Unmap and free a CPU address region that was allocated and mapped with
>> + * pci_epc_mem_map().
>> + */
>> +void pci_epc_mem_unmap(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
>> +		       struct pci_epc_map *map)
>> +{
>> +	if (!pci_epc_function_is_valid(epc, func_no, vfunc_no))
>> +		return;
>> +
>> +	if (!map || !map->virt_base)
>> +		return;
>> +
>> +	pci_epc_unmap_addr(epc, func_no, vfunc_no, map->phys_base);
>> +	pci_epc_mem_free_addr(epc, map->phys_base, map->virt_base,
>> +			      map->map_size);
>> +}
>> +EXPORT_SYMBOL_GPL(pci_epc_mem_unmap);
>> +
>>  /**
>>   * pci_epc_clear_bar() - reset the BAR
>>   * @epc: the EPC device for which the BAR has to be cleared
>> diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
>> index 42ef06136bd1..b5f5c1eb54c5 100644
>> --- a/include/linux/pci-epc.h
>> +++ b/include/linux/pci-epc.h
>> @@ -32,11 +32,44 @@ pci_epc_interface_string(enum pci_epc_interface_type type)
>>  	}
>>  }
>>  
>> +/**
>> + * struct pci_epc_map - information about EPC memory for mapping a RC PCI
>> + *                      address range
>> + * @pci_addr: start address of the RC PCI address range to map
>> + * @pci_size: size of the RC PCI address range mapped from @pci_addr
>> + * @map_pci_addr: RC PCI address used as the first address mapped (may be lower
>> + *                than @pci_addr)
>> + * @map_size: size of the controller memory needed for mapping the RC PCI address
>> + *            range @pci_addr..@pci_addr+@pci_size
>> + * @map_ofst: offset into the mapped controller memory to access @pci_addr
>> + * @phys_base: base physical address of the allocated EPC memory for mapping the
>> + *             RC PCI address range
>> + * @phys_addr: physical address at which @pci_addr is mapped
>> + * @virt_base: base virtual address of the allocated EPC memory for mapping the
>> + *             RC PCI address range
>> + * @virt_addr: virtual address at which @pci_addr is mapped
>> + */
>> +struct pci_epc_map {
>> +	phys_addr_t	pci_addr;
>> +	size_t		pci_size;
>> +
>> +	phys_addr_t	map_pci_addr;
>> +	size_t		map_size;
>> +	phys_addr_t	map_ofst;
>> +
>> +	phys_addr_t	phys_base;
>> +	phys_addr_t	phys_addr;
>> +	void __iomem	*virt_base;
>> +	void __iomem	*virt_addr;
>> +};
>> +
>>  /**
>>   * struct pci_epc_ops - set of function pointers for performing EPC operations
>>   * @write_header: ops to populate configuration space header
>>   * @set_bar: ops to configure the BAR
>>   * @clear_bar: ops to reset the BAR
>> + * @get_mem_map: operation to get the size and offset into a controller memory
>> + *               window needed to map an RC PCI address region
>>   * @map_addr: ops to map CPU address to PCI address
>>   * @unmap_addr: ops to unmap CPU address and PCI address
>>   * @set_msi: ops to set the requested number of MSI interrupts in the MSI
>> @@ -61,6 +94,8 @@ struct pci_epc_ops {
>>  			   struct pci_epf_bar *epf_bar);
>>  	void	(*clear_bar)(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
>>  			     struct pci_epf_bar *epf_bar);
>> +	int	(*get_mem_map)(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
>> +			       struct pci_epc_map *map);
>>  	int	(*map_addr)(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
>>  			    phys_addr_t addr, u64 pci_addr, size_t size);
>>  	void	(*unmap_addr)(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
>> @@ -278,6 +313,10 @@ void __iomem *pci_epc_mem_alloc_addr(struct pci_epc *epc,
>>  				     phys_addr_t *phys_addr, size_t size);
>>  void pci_epc_mem_free_addr(struct pci_epc *epc, phys_addr_t phys_addr,
>>  			   void __iomem *virt_addr, size_t size);
>> +int pci_epc_mem_map(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
>> +		    u64 pci_addr, size_t pci_size, struct pci_epc_map *map);
>> +void pci_epc_mem_unmap(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
>> +		       struct pci_epc_map *map);
>>  
>>  #else
>>  static inline void pci_epc_init_notify(struct pci_epc *epc)
>> -- 
>> 2.47.0
>>

On Fri, Oct 11, 2024 at 02:07:07PM +0200, Niklas Cassel wrote:
> On Fri, Oct 11, 2024 at 09:01:12PM +0900, Damien Le Moal wrote:

(snip)

> > +int pci_epc_mem_map(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> > +		    u64 pci_addr, size_t pci_size, struct pci_epc_map *map)
> > +{
> > +	int ret;
> > +
> > +	if (!pci_epc_function_is_valid(epc, func_no, vfunc_no))
> > +		return -EINVAL;
> > +
> > +	if (!pci_size || !map)
> > +		return -EINVAL;
> > +
> > +	ret = pci_epc_get_mem_map(epc, func_no, vfunc_no,
> > +				  pci_addr, pci_size, map);
> > +	if (ret)
> > +		return ret;
> > +
> > +	map->virt_base = pci_epc_mem_alloc_addr(epc, &map->phys_base,
> > +						map->map_size);
> > +	if (!map->virt_base)
> > +		return -ENOMEM;
> > +
> > +	map->phys_addr = map->phys_base + map->map_ofst;
> > +	map->virt_addr = map->virt_base + map->map_ofst;
> > +
> > +	ret = pci_epc_map_addr(epc, func_no, vfunc_no, map->phys_base,
> > +			       map->map_pci_addr, map->map_size);
> > +	if (ret) {
> > +		pci_epc_mem_free_addr(epc, map->phys_base, map->virt_base,
> > +				      map->map_size);
> > +		map->virt_base = 0;
> 
> As reported by the kernel test robot on both v3 and v4, this should be:
> map->virt_base = NULL;
> otherwise you introduce a new sparse warning.

With that nit fixed:
Reviewed-by: Niklas Cassel <cassel@kernel.org>

On Fri, Oct 11, 2024 at 09:21:29PM +0900, Damien Le Moal wrote:
> On 10/11/24 21:07, Niklas Cassel wrote:
> >> +int pci_epc_mem_map(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> >> +		    u64 pci_addr, size_t pci_size, struct pci_epc_map *map)
> >> +{
> >> +	int ret;
> >> +
> >> +	if (!pci_epc_function_is_valid(epc, func_no, vfunc_no))
> >> +		return -EINVAL;
> >> +
> >> +	if (!pci_size || !map)
> >> +		return -EINVAL;
> >> +
> >> +	ret = pci_epc_get_mem_map(epc, func_no, vfunc_no,
> >> +				  pci_addr, pci_size, map);
> >> +	if (ret)
> >> +		return ret;
> >> +
> >> +	map->virt_base = pci_epc_mem_alloc_addr(epc, &map->phys_base,
> >> +						map->map_size);
> >> +	if (!map->virt_base)
> >> +		return -ENOMEM;
> >> +
> >> +	map->phys_addr = map->phys_base + map->map_ofst;
> >> +	map->virt_addr = map->virt_base + map->map_ofst;
> >> +
> >> +	ret = pci_epc_map_addr(epc, func_no, vfunc_no, map->phys_base,
> >> +			       map->map_pci_addr, map->map_size);
> >> +	if (ret) {
> >> +		pci_epc_mem_free_addr(epc, map->phys_base, map->virt_base,
> >> +				      map->map_size);
> >> +		map->virt_base = 0;
> > 
> > As reported by the kernel test robot on both v3 and v4, this should be:
> > map->virt_base = NULL;
> > otherwise you introduce a new sparse warning.
> 
> Oops. Missed that... OK, sending v6 with this removed as that is not necessary
> anyway.
> 

Please incorporate this in v6.

- Mani