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

[Damien Le Moal <dlemoal@kernel.org>]

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
>>


-- 
Damien Le Moal
Western Digital Research