| Message ID | 20241004050742.140664-5-dlemoal@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Improve PCI memory mapping API | expand |
On Fri, Oct 04, 2024 at 02:07:39PM +0900, Damien Le Moal wrote: > Introduce the function pci_epc_mem_map() to facilitate controller memory > address allocation and mapping to a RC PCI address region in endpoint > function drivers. > > This function first uses pci_epc_map_align() to determine the controller > memory address size (and offset into) depending on the controller > address alignment constraints. The result of this function is used to > allocate a controller physical memory region using > pci_epc_mem_alloc_addr() and map that memory to the RC PCI address > space with pci_epc_map_addr(). > > Since pci_epc_map_align() may indicate that the effective mapping > of a PCI address region is smaller than the user requested size, > 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. > > The counterpart of pci_epc_mem_map() to unmap and free the controller > memory address region is pci_epc_mem_unmap(). > > Both functions operate using a struct pci_epc_map data structure > 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 | 78 +++++++++++++++++++++++++++++ > include/linux/pci-epc.h | 4 ++ > 2 files changed, 82 insertions(+) > > diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c > index 48dd3c28ac4c..5f3b0a86d6fe 100644 > --- a/drivers/pci/endpoint/pci-epc-core.c > +++ b/drivers/pci/endpoint/pci-epc-core.c > @@ -522,6 +522,84 @@ int pci_epc_map_addr(struct pci_epc *epc, u8 func_no, u8 vfunc_no, > } > EXPORT_SYMBOL_GPL(pci_epc_map_addr); > > +/** > + * 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_map_align(). > + * 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; > + > + ret = pci_epc_map_align(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; > + 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); > + map->map_size = 0; > +} > +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 9df8a83e8d10..97d2fbb740fd 100644 > --- a/include/linux/pci-epc.h > +++ b/include/linux/pci-epc.h > @@ -315,6 +315,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.46.2 > Naming is one of the hardest problems in computer science :) Perhaps: s/pci_epc_mem_map()/pci_epc_mem_alloc_map()/ s/pci_epc_mem_unmap()/pci_epc_mem_free_unmap()/ is slightly more clear that this both allocates and maps. Regardless: Reviewed-by: Niklas Cassel <cassel@kernel.org>
On 10/4/24 20:47, Niklas Cassel wrote: > Naming is one of the hardest problems in computer science :) Yep. > Perhaps: > s/pci_epc_mem_map()/pci_epc_mem_alloc_map()/ > s/pci_epc_mem_unmap()/pci_epc_mem_free_unmap()/ > > is slightly more clear that this both allocates and maps. Sure, but I consider the allocation an implementation detail of the function. And I really prefer the shorter function names :) > Regardless: > Reviewed-by: Niklas Cassel <cassel@kernel.org> Thanks.
Hi Damien, kernel test robot noticed the following build warnings: [auto build test WARNING on pci/next] [also build test WARNING on pci/for-linus mani-mhi/mhi-next linus/master v6.12-rc2 next-20241004] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Damien-Le-Moal/PCI-endpoint-Introduce-pci_epc_function_is_valid/20241004-130947 base: https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git next patch link: https://lore.kernel.org/r/20241004050742.140664-5-dlemoal%40kernel.org patch subject: [PATCH v3 4/7] PCI: endpoint: Introduce pci_epc_mem_map()/unmap() config: x86_64-randconfig-122-20241007 (https://download.01.org/0day-ci/archive/20241007/202410070929.jEKAJxjG-lkp@intel.com/config) compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241007/202410070929.jEKAJxjG-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202410070929.jEKAJxjG-lkp@intel.com/ sparse warnings: (new ones prefixed by >>) >> drivers/pci/endpoint/pci-epc-core.c:569:34: sparse: sparse: Using plain integer as NULL pointer drivers/pci/endpoint/pci-epc-core.c: note: in included file (through include/linux/smp.h, include/linux/lockdep.h, include/linux/spinlock.h, ...): include/linux/list.h:83:21: sparse: sparse: self-comparison always evaluates to true vim +569 drivers/pci/endpoint/pci-epc-core.c 524 525 /** 526 * pci_epc_mem_map() - allocate and map a PCI address to a CPU address 527 * @epc: the EPC device on which the CPU address is to be allocated and mapped 528 * @func_no: the physical endpoint function number in the EPC device 529 * @vfunc_no: the virtual endpoint function number in the physical function 530 * @pci_addr: PCI address to which the CPU address should be mapped 531 * @pci_size: the number of bytes to map starting from @pci_addr 532 * @map: where to return the mapping information 533 * 534 * Allocate a controller memory address region and map it to a RC PCI address 535 * region, taking into account the controller physical address mapping 536 * constraints using pci_epc_map_align(). 537 * The effective size of the PCI address range mapped from @pci_addr is 538 * indicated by @map->pci_size. This size may be less than the requested 539 * @pci_size. The local virtual CPU address for the mapping is indicated by 540 * @map->virt_addr (@map->phys_addr indicates the physical address). 541 * The size and CPU address of the controller memory allocated and mapped are 542 * respectively indicated by @map->map_size and @map->virt_base (and 543 * @map->phys_base). 544 * 545 * Returns 0 on success and a negative error code in case of error. 546 */ 547 int pci_epc_mem_map(struct pci_epc *epc, u8 func_no, u8 vfunc_no, 548 u64 pci_addr, size_t pci_size, struct pci_epc_map *map) 549 { 550 int ret; 551 552 ret = pci_epc_map_align(epc, func_no, vfunc_no, pci_addr, pci_size, map); 553 if (ret) 554 return ret; 555 556 map->virt_base = pci_epc_mem_alloc_addr(epc, &map->phys_base, 557 map->map_size); 558 if (!map->virt_base) 559 return -ENOMEM; 560 561 map->phys_addr = map->phys_base + map->map_ofst; 562 map->virt_addr = map->virt_base + map->map_ofst; 563 564 ret = pci_epc_map_addr(epc, func_no, vfunc_no, map->phys_base, 565 map->map_pci_addr, map->map_size); 566 if (ret) { 567 pci_epc_mem_free_addr(epc, map->phys_base, map->virt_base, 568 map->map_size); > 569 map->virt_base = 0; 570 return ret; 571 } 572 573 return 0; 574 } 575 EXPORT_SYMBOL_GPL(pci_epc_mem_map); 576
diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c index 48dd3c28ac4c..5f3b0a86d6fe 100644 --- a/drivers/pci/endpoint/pci-epc-core.c +++ b/drivers/pci/endpoint/pci-epc-core.c @@ -522,6 +522,84 @@ int pci_epc_map_addr(struct pci_epc *epc, u8 func_no, u8 vfunc_no, } EXPORT_SYMBOL_GPL(pci_epc_map_addr); +/** + * 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_map_align(). + * 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; + + ret = pci_epc_map_align(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; + 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); + map->map_size = 0; +} +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 9df8a83e8d10..97d2fbb740fd 100644 --- a/include/linux/pci-epc.h +++ b/include/linux/pci-epc.h @@ -315,6 +315,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)