Re: [PATCH net-next v3 02/14] libie: add PCI device initialization helpers to libie

[Bjorn Helgaas <helgaas@kernel.org>]

[+cc Philipp]

On Fri, May 15, 2026 at 03:44:26PM -0700, Tony Nguyen wrote:
> From: Phani R Burra <phani.r.burra@intel.com>
> 
> Add support functions for drivers to configure PCI functionality and access
> MMIO space.

This looks kind of like what pcim_iomap_range() does, i.e., a way to
ioremap (BAR-idx, offset, size) pieces of PCI BARs.  That sounds like
useful functionality.

Is there something Intel-specific or even ethernet-specific about
this?  If devm_* and pcim_* don't do what you need, maybe they should
be extended or this could be made generic so any drivers could use it?

This looks like a mix of managed (pcim_enable_device(),
pcim_request_region()), and unmanaged (ioremap(), iounmap()) things.
I haven't looked at how all this is used, but it's pretty easy to get
things wrong when mixing models.

> +++ b/drivers/net/ethernet/intel/libie/pci.c
> @@ -0,0 +1,208 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/* Copyright (C) 2025 Intel Corporation */
> +
> +#include <linux/intel/libie/pci.h>
> +
> +/**
> + * libie_find_mmio_region - find MMIO region containing a range
> + * @mmio_list: list that contains MMIO region info
> + * @offset: range start offset
> + * @size: range size
> + * @bar_idx: BAR index containing the range to search
> + *
> + * Return: pointer to a MMIO region overlapping with the range in any way or
> + *	   NULL if no such region is mapped.
> + */
> +static struct libie_pci_mmio_region *
> +libie_find_mmio_region(const struct list_head *mmio_list,
> +		       resource_size_t offset, resource_size_t size,
> +		       int bar_idx)
> +{
> +	resource_size_t end_offset = offset + size;
> +	struct libie_pci_mmio_region *mr;
> +
> +	list_for_each_entry(mr, mmio_list, list) {
> +		resource_size_t mr_end = mr->offset + mr->size;
> +		resource_size_t mr_start = mr->offset;
> +
> +		if (mr->bar_idx != bar_idx)
> +			continue;
> +		if (offset < mr_end && end_offset > mr_start)
> +			return mr;
> +	}
> +
> +	return NULL;
> +}
> +
> +/**
> + * __libie_pci_get_mmio_addr - get the MMIO virtual address
> + * @mmio_info: contains list of MMIO regions
> + * @offset: register offset to find
> + * @num_args: number of additional arguments present
> + *
> + * This function finds the virtual address of a register offset by iterating
> + * through the non-linear MMIO regions that are mapped by the driver.
> + *
> + * Return: valid MMIO virtual address or NULL.
> + */
> +void __iomem *__libie_pci_get_mmio_addr(struct libie_mmio_info *mmio_info,
> +					resource_size_t offset,
> +					int num_args, ...)
> +{
> +	struct libie_pci_mmio_region *mr;
> +	int bar_idx = 0;
> +	va_list args;
> +
> +	if (num_args) {
> +		va_start(args, num_args);
> +		bar_idx = va_arg(args, int);
> +		va_end(args);
> +	}
> +
> +	list_for_each_entry(mr, &mmio_info->mmio_list, list)
> +		if (bar_idx == mr->bar_idx && offset >= mr->offset &&
> +		    offset < mr->offset + mr->size) {
> +			offset -= mr->offset;
> +
> +			return mr->addr + offset;
> +		}
> +
> +	return NULL;
> +}
> +EXPORT_SYMBOL_NS_GPL(__libie_pci_get_mmio_addr, "LIBIE_PCI");
> +
> +/**
> + * __libie_pci_map_mmio_region - map PCI device MMIO region
> + * @mmio_info: struct to store the mapped MMIO region
> + * @offset: MMIO region start offset
> + * @size: MMIO region size
> + * @num_args: number of additional arguments present
> + *
> + * Return: true on success, false on memory map failure.
> + */
> +bool __libie_pci_map_mmio_region(struct libie_mmio_info *mmio_info,
> +				 resource_size_t offset,
> +				 resource_size_t size, int num_args, ...)
> +{
> +	struct pci_dev *pdev = mmio_info->pdev;
> +	struct libie_pci_mmio_region *mr;
> +	resource_size_t pa;
> +	void __iomem *va;
> +	int bar_idx = 0;
> +	va_list args;
> +
> +	if (num_args) {
> +		va_start(args, num_args);
> +		bar_idx = va_arg(args, int);
> +		va_end(args);
> +	}
> +
> +	if (offset + size > pci_resource_len(pdev, bar_idx))
> +		return false;
> +
> +	mr = libie_find_mmio_region(&mmio_info->mmio_list, offset, size,
> +				    bar_idx);
> +	if (mr) {
> +		pci_warn(pdev,
> +			 "Mapping of BAR%u (offset=%llu, size=%llu) intersecting region (offset=%llu, size=%llu) already exists\n",
> +			 bar_idx, (unsigned long long)mr->offset,
> +			 (unsigned long long)mr->size,
> +			 (unsigned long long)offset, (unsigned long long)size);
> +		return mr->offset <= offset &&
> +		       mr->offset + mr->size >= offset + size;
> +	}
> +
> +	pa = pci_resource_start(pdev, bar_idx) + offset;
> +	va = ioremap(pa, size);
> +	if (!va) {
> +		pci_err(pdev, "Failed to map BAR%u region\n", bar_idx);
> +		return false;
> +	}
> +
> +	mr = kvzalloc_obj(*mr);
> +	if (!mr) {
> +		iounmap(va);
> +		return false;
> +	}
> +
> +	mr->addr = va;
> +	mr->offset = offset;
> +	mr->size = size;
> +	mr->bar_idx = bar_idx;
> +
> +	list_add_tail(&mr->list, &mmio_info->mmio_list);
> +
> +	return true;
> +}
> +EXPORT_SYMBOL_NS_GPL(__libie_pci_map_mmio_region, "LIBIE_PCI");
> +
> +/**
> + * libie_pci_unmap_fltr_regs - unmap selected PCI device MMIO regions
> + * @mmio_info: contains list of MMIO regions to unmap
> + * @fltr: returns true, if region is to be unmapped
> + */
> +void libie_pci_unmap_fltr_regs(struct libie_mmio_info *mmio_info,
> +			       bool (*fltr)(struct libie_mmio_info *mmio_info,
> +					    struct libie_pci_mmio_region *reg))
> +{
> +	struct libie_pci_mmio_region *mr, *tmp;
> +
> +	list_for_each_entry_safe(mr, tmp, &mmio_info->mmio_list, list) {
> +		if (!fltr(mmio_info, mr))
> +			continue;
> +		iounmap(mr->addr);
> +		list_del(&mr->list);
> +		kvfree(mr);
> +	}
> +}
> +EXPORT_SYMBOL_NS_GPL(libie_pci_unmap_fltr_regs, "LIBIE_PCI");
> +
> +/**
> + * libie_pci_unmap_all_mmio_regions - unmap all PCI device MMIO regions
> + * @mmio_info: contains list of MMIO regions to unmap
> + */
> +void libie_pci_unmap_all_mmio_regions(struct libie_mmio_info *mmio_info)
> +{
> +	struct libie_pci_mmio_region *mr, *tmp;
> +
> +	list_for_each_entry_safe(mr, tmp, &mmio_info->mmio_list, list) {
> +		iounmap(mr->addr);
> +		list_del(&mr->list);
> +		kvfree(mr);
> +	}
> +}
> +EXPORT_SYMBOL_NS_GPL(libie_pci_unmap_all_mmio_regions, "LIBIE_PCI");
> +
> +/**
> + * libie_pci_init_dev - enable and reserve PCI regions of the device
> + * @pdev: PCI device information
> + *
> + * Return: %0 on success, -%errno on failure.
> + */
> +int libie_pci_init_dev(struct pci_dev *pdev)
> +{
> +	int err;
> +
> +	err = pcim_enable_device(pdev);
> +	if (err)
> +		return err;
> +
> +	for (int bar = 0; bar < PCI_STD_NUM_BARS; bar++)
> +		if (pci_resource_flags(pdev, bar) & IORESOURCE_MEM) {
> +			err = pcim_request_region(pdev, bar, pci_name(pdev));
> +			if (err)
> +				return err;
> +		}
> +
> +	err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
> +	if (err)
> +		return err;
> +
> +	pci_set_master(pdev);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_NS_GPL(libie_pci_init_dev, "LIBIE_PCI");
> +
> +MODULE_DESCRIPTION("Common Ethernet PCI library");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/intel/libie/pci.h b/include/linux/intel/libie/pci.h
> new file mode 100644
> index 000000000000..effd072c55c8
> --- /dev/null
> +++ b/include/linux/intel/libie/pci.h
> @@ -0,0 +1,56 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/* Copyright (C) 2025 Intel Corporation */
> +
> +#ifndef __LIBIE_PCI_H
> +#define __LIBIE_PCI_H
> +
> +#include <linux/pci.h>
> +
> +/**
> + * struct libie_pci_mmio_region - structure for MMIO region info
> + * @list: used to add a MMIO region to the list of MMIO regions in
> + *	  libie_mmio_info
> + * @addr: virtual address of MMIO region start
> + * @offset: start offset of the MMIO region
> + * @size: size of the MMIO region
> + * @bar_idx: BAR index to which the MMIO region belongs to
> + */
> +struct libie_pci_mmio_region {
> +	struct list_head	list;
> +	void __iomem		*addr;
> +	resource_size_t		offset;
> +	resource_size_t		size;
> +	u16			bar_idx;
> +};
> +
> +/**
> + * struct libie_mmio_info - contains list of MMIO regions
> + * @pdev: PCI device pointer
> + * @mmio_list: list of MMIO regions
> + */
> +struct libie_mmio_info {
> +	struct pci_dev		*pdev;
> +	struct list_head	mmio_list;
> +};
> +
> +#define libie_pci_map_mmio_region(mmio_info, offset, size, ...)	\
> +	__libie_pci_map_mmio_region(mmio_info, offset, size,		\
> +				     COUNT_ARGS(__VA_ARGS__), ##__VA_ARGS__)
> +
> +#define libie_pci_get_mmio_addr(mmio_info, offset, ...)		\
> +	__libie_pci_get_mmio_addr(mmio_info, offset,			\
> +				   COUNT_ARGS(__VA_ARGS__), ##__VA_ARGS__)
> +
> +bool __libie_pci_map_mmio_region(struct libie_mmio_info *mmio_info,
> +				 resource_size_t offset, resource_size_t size,
> +				 int num_args, ...);
> +void __iomem *__libie_pci_get_mmio_addr(struct libie_mmio_info *mmio_info,
> +					resource_size_t offset,
> +					int num_args, ...);
> +void libie_pci_unmap_all_mmio_regions(struct libie_mmio_info *mmio_info);
> +void libie_pci_unmap_fltr_regs(struct libie_mmio_info *mmio_info,
> +			       bool (*fltr)(struct libie_mmio_info *mmio_info,
> +					    struct libie_pci_mmio_region *reg));
> +int libie_pci_init_dev(struct pci_dev *pdev);
> +
> +#endif /* __LIBIE_PCI_H */
> -- 
> 2.47.1
>