| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <ea5fa2ec-3c0c-0343-0b14-9bbfa93feaa3@ti.com>
Date: Fri, 21 Feb 2020 17:10:15 +0530
From: Kishon Vijay Abraham I <kishon@...com>
To: Lad Prabhakar <prabhakar.csengg@...il.com>,
Bjorn Helgaas <bhelgaas@...gle.com>,
Rob Herring <robh+dt@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Catalin Marinas <catalin.marinas@....com>,
Will Deacon <will@...nel.org>,
Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
Arnd Bergmann <arnd@...db.de>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Jingoo Han <jingoohan1@...il.com>,
Gustavo Pimentel <gustavo.pimentel@...opsys.com>,
Marek Vasut <marek.vasut+renesas@...il.com>,
Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com>,
Shawn Lin <shawn.lin@...k-chips.com>,
Heiko Stuebner <heiko@...ech.de>
CC: Andrew Murray <andrew.murray@....com>, <linux-pci@...r.kernel.org>,
<linux-arm-kernel@...ts.infradead.org>,
<linux-renesas-soc@...r.kernel.org>,
<linux-rockchip@...ts.infradead.org>,
<linux-kernel@...r.kernel.org>, <devicetree@...r.kernel.org>,
Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>
Subject: Re: [PATCH v4 3/6] PCI: endpoint: Add support to handle multiple base
for mapping outbound memory
Hi Prabhakar,
On 09/02/20 12:06 am, Lad Prabhakar wrote:
> R-Car PCIe controller has support to map multiple memory regions for
> mapping the outbound memory in local system also the controller limits
> single allocation for each region (that is, once a chunk is used from the
> region it cannot be used to allocate a new one). This features inspires to
> add support for handling multiple memory bases in endpoint framework.
>
> With this patch pci_epc_mem_init() now accepts multiple regions, also
> page_size for each memory region is passed during initialization so as
> to handle single allocation for each region by setting the page_size to
> window_size.
This patch looks much better now except for one comment below..
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>
> ---
> drivers/pci/controller/cadence/pcie-cadence-ep.c | 7 +-
> drivers/pci/controller/dwc/pcie-designware-ep.c | 29 ++--
> drivers/pci/controller/pcie-rockchip-ep.c | 7 +-
> drivers/pci/endpoint/pci-epc-mem.c | 166 ++++++++++++++++-------
> include/linux/pci-epc.h | 39 ++++--
> 5 files changed, 168 insertions(+), 80 deletions(-)
>
.
.
<snip>
.
.
> diff --git a/drivers/pci/endpoint/pci-epc-mem.c b/drivers/pci/endpoint/pci-epc-mem.c
> index d2b174c..b3eedee 100644
> --- a/drivers/pci/endpoint/pci-epc-mem.c
> +++ b/drivers/pci/endpoint/pci-epc-mem.c
> @@ -38,57 +38,76 @@ static int pci_epc_mem_get_order(struct pci_epc_mem *mem, size_t size)
> /**
> * __pci_epc_mem_init() - initialize the pci_epc_mem structure
> * @epc: the EPC device that invoked pci_epc_mem_init
> - * @phys_base: the physical address of the base
> - * @size: the size of the address space
> - * @page_size: size of each page
> + * @windows: pointer to windows supported by the device
> + * @num_windows: number of windows device supports
> *
> * Invoke to initialize the pci_epc_mem structure used by the
> * endpoint functions to allocate mapped PCI address.
> */
> -int __pci_epc_mem_init(struct pci_epc *epc, phys_addr_t phys_base, size_t size,
> - size_t page_size)
> +int __pci_epc_mem_init(struct pci_epc *epc, struct pci_epc_mem_window *windows,
> + int num_windows)
> {
> - int ret;
> - struct pci_epc_mem *mem;
> - unsigned long *bitmap;
> + struct pci_epc_mem *mem = NULL;
> + unsigned long *bitmap = NULL;
> unsigned int page_shift;
> - int pages;
> + size_t page_size;
> int bitmap_size;
> -
> - if (page_size < PAGE_SIZE)
> - page_size = PAGE_SIZE;
> -
> - page_shift = ilog2(page_size);
> - pages = size >> page_shift;
> - bitmap_size = BITS_TO_LONGS(pages) * sizeof(long);
> -
> - mem = kzalloc(sizeof(*mem), GFP_KERNEL);
> - if (!mem) {
> - ret = -ENOMEM;
> - goto err;
> - }
> -
> - bitmap = kzalloc(bitmap_size, GFP_KERNEL);
> - if (!bitmap) {
> - ret = -ENOMEM;
> - goto err_mem;
> + int pages;
> + int ret;
> + int i;
> +
> + epc->mem_windows = 0;
> +
> + if (!windows)
> + return -EINVAL;
> +
> + if (num_windows <= 0)
> + return -EINVAL;
> +
> + epc->mem = kcalloc(num_windows, sizeof(*mem), GFP_KERNEL);
> + if (!epc->mem)
> + return -EINVAL;
> +
> + for (i = 0; i < num_windows; i++) {
> + page_size = windows[i].page_size;
> + if (page_size < PAGE_SIZE)
> + page_size = PAGE_SIZE;
> + page_shift = ilog2(page_size);
> + pages = windows[i].size >> page_shift;
> + bitmap_size = BITS_TO_LONGS(pages) * sizeof(long);
> +
> + mem = kzalloc(sizeof(*mem), GFP_KERNEL);
> + if (!mem) {
> + ret = -ENOMEM;
> + goto err_mem;
> + }
> +
> + bitmap = kzalloc(bitmap_size, GFP_KERNEL);
> + if (!bitmap) {
> + ret = -ENOMEM;
> + goto err_mem;
> + }
> +
> + mem->bitmap = bitmap;
> + mem->window.phys_base = windows[i].phys_base;
> + mem->page_size = page_size;
> + mem->pages = pages;
> + mem->window.size = windows[i].size;
> + epc->mem[i] = mem;
> }
> -
> - mem->bitmap = bitmap;
> - mem->phys_base = phys_base;
> - mem->page_size = page_size;
> - mem->pages = pages;
> - mem->size = size;
> -
> - epc->mem = mem;
> + epc->mem_windows = num_windows;
>
> return 0;
>
> err_mem:
> - kfree(mem);
> + for (; i >= 0; i--) {
> + mem = epc->mem[i];
> + kfree(mem->bitmap);
> + kfree(mem);
> + }
> + kfree(epc->mem);
>
> -err:
> -return ret;
> + return ret;
> }
> EXPORT_SYMBOL_GPL(__pci_epc_mem_init);
>
> @@ -101,11 +120,21 @@ EXPORT_SYMBOL_GPL(__pci_epc_mem_init);
> */
> void pci_epc_mem_exit(struct pci_epc *epc)
> {
> - struct pci_epc_mem *mem = epc->mem;
> + struct pci_epc_mem *mem;
> + int i;
> +
> + if (!epc->mem_windows)
> + return;
> +
> + for (i = 0; i <= epc->mem_windows; i++) {
> + mem = epc->mem[i];
> + kfree(mem->bitmap);
> + kfree(mem);
> + }
> + kfree(epc->mem);
>
> epc->mem = NULL;
> - kfree(mem->bitmap);
> - kfree(mem);
> + epc->mem_windows = 0;
> }
> EXPORT_SYMBOL_GPL(pci_epc_mem_exit);
>
> @@ -121,20 +150,30 @@ EXPORT_SYMBOL_GPL(pci_epc_mem_exit);
> void __iomem *pci_epc_mem_alloc_addr(struct pci_epc *epc,
> phys_addr_t *phys_addr, size_t size)
> {
> - int pageno;
> - void __iomem *virt_addr;
> - struct pci_epc_mem *mem = epc->mem;
> - unsigned int page_shift = ilog2(mem->page_size);
> + void __iomem *virt_addr = NULL;
> + struct pci_epc_mem *mem;
> + unsigned int page_shift;
> + int pageno = -EINVAL;
> int order;
> + int i;
>
> - size = ALIGN(size, mem->page_size);
> - order = pci_epc_mem_get_order(mem, size);
> + for (i = 0; i < epc->mem_windows; i++) {
> + mem = epc->mem[i];
> + size = ALIGN(size, mem->page_size);
> + order = pci_epc_mem_get_order(mem, size);
> +
> + pageno = bitmap_find_free_region(mem->bitmap, mem->pages,
> + order);
> + if (pageno >= 0)
> + break;
> + }
>
> - pageno = bitmap_find_free_region(mem->bitmap, mem->pages, order);
> if (pageno < 0)
> return NULL;
>
> - *phys_addr = mem->phys_base + ((phys_addr_t)pageno << page_shift);
> + page_shift = ilog2(mem->page_size);
> + *phys_addr = mem->window.phys_base +
> + ((phys_addr_t)pageno << page_shift);
> virt_addr = ioremap(*phys_addr, size);
> if (!virt_addr)
> bitmap_release_region(mem->bitmap, pageno, order);
> @@ -143,6 +182,22 @@ void __iomem *pci_epc_mem_alloc_addr(struct pci_epc *epc,
> }
> EXPORT_SYMBOL_GPL(pci_epc_mem_alloc_addr);
>
> +struct pci_epc_mem *pci_epc_get_matching_window(struct pci_epc *epc,
> + phys_addr_t phys_addr)
> +{
> + struct pci_epc_mem *mem;
> + int i;
> +
> + for (i = 0; i < epc->mem_windows; i++) {
> + mem = epc->mem[i];
> +
> + if (mem->window.phys_base == phys_addr)
> + return mem;
This will work only if the phys_addr is same as start of windows base.
This need not be true for all the platforms and will fail for all the
allocations except the first allocation.
Thanks
Kishon
Powered by blists - more mailing lists