| 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: <20151020215714.GI8224@localhost>
Date: Tue, 20 Oct 2015 16:57:14 -0500
From: Bjorn Helgaas <helgaas@...nel.org>
To: Yinghai Lu <yinghai@...nel.org>
Cc: Bjorn Helgaas <bhelgaas@...gle.com>,
David Miller <davem@...emloft.net>,
Benjamin Herrenschmidt <benh@...nel.crashing.org>,
Wei Yang <weiyang@...ux.vnet.ibm.com>, TJ <linux@....tj>,
Yijing Wang <wangyijing@...wei.com>,
Khalid Aziz <khalid.aziz@...cle.com>,
linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v7 03/60] sparc/PCI: Unify pci_register_region()
On Thu, Oct 08, 2015 at 02:38:22PM -0700, Yinghai Lu wrote:
> We register regions for legacy and iommu and all have open code.
>
> Unify them to pci_register_region() and call it accordingly.
>
> Signed-off-by: Yinghai Lu <yinghai@...nel.org>
> ---
> arch/sparc/kernel/pci_common.c | 83 +++++++++++++++++++-----------------------
> 1 file changed, 37 insertions(+), 46 deletions(-)
>
> diff --git a/arch/sparc/kernel/pci_common.c b/arch/sparc/kernel/pci_common.c
> index 28e976a..c4b6989 100644
> --- a/arch/sparc/kernel/pci_common.c
> +++ b/arch/sparc/kernel/pci_common.c
> @@ -328,41 +328,49 @@ void pci_get_pbm_props(struct pci_pbm_info *pbm)
> }
> }
>
> -static void pci_register_legacy_regions(struct resource *io_res,
> - struct resource *mem_res)
> +static void pci_register_region(struct pci_pbm_info *pbm, const char *name,
> + resource_size_t rstart, resource_size_t size)
> {
> - struct resource *p;
> + struct resource *res, *conflict;
> + struct resource *mem_res = &pbm->mem_space;
> + resource_size_t offset = pbm->mem_offset;
> + resource_size_t mem_rstart, mem_rend;
> + resource_size_t rend = rstart + size - 1UL;
>
> - /* VGA Video RAM. */
> - p = kzalloc(sizeof(*p), GFP_KERNEL);
> - if (!p)
> + if (!mem_res->flags)
> return;
>
> - p->name = "Video RAM area";
> - p->start = mem_res->start + 0xa0000UL;
> - p->end = p->start + 0x1ffffUL;
> - p->flags = IORESOURCE_BUSY;
> - request_resource(mem_res, p);
> + mem_rstart = mem_res->start - offset;
> + mem_rend = mem_res->end - offset;
This is essentially doing pcibios_bus_to_resource(), except in an
unnecessarily arch-dependent way. Can you rework it to use
pcibios_bus_to_resource()? I think you'll have to do
pci_register_legacy_regions() in pci_scan_one_pbm() instead of in
pci_determine_mem_io_space(), so that you have a struct pci_bus to
use, but your next patch does that anyway.
>
> - p = kzalloc(sizeof(*p), GFP_KERNEL);
> - if (!p)
> + /* contain checking */
> + if (!(mem_rstart <= rstart && mem_rend >= rend))
> return;
>
> - p->name = "System ROM";
> - p->start = mem_res->start + 0xf0000UL;
> - p->end = p->start + 0xffffUL;
> - p->flags = IORESOURCE_BUSY;
> - request_resource(mem_res, p);
> -
> - p = kzalloc(sizeof(*p), GFP_KERNEL);
> - if (!p)
> + res = kzalloc(sizeof(*res), GFP_KERNEL);
> + if (!res)
> return;
>
> - p->name = "Video ROM";
> - p->start = mem_res->start + 0xc0000UL;
> - p->end = p->start + 0x7fffUL;
> - p->flags = IORESOURCE_BUSY;
> - request_resource(mem_res, p);
> + res->name = name;
> + res->flags = IORESOURCE_MEM | IORESOURCE_BUSY;
> + res->start = rstart + offset;
> + res->end = rend + offset;
> + conflict = request_resource_conflict(mem_res, res);
> + if (conflict) {
> + printk(KERN_DEBUG "PCI: %s can't claim %s %pR: address conflict with %s %pR\n",
> + pbm->name, res->name, res, conflict->name, conflict);
> + kfree(res);
> + }
> +}
> +
> +static void pci_register_legacy_regions(struct pci_pbm_info *pbm)
> +{
> + /* VGA Video RAM. */
> + pci_register_region(pbm, "Video RAM area", 0xa0000UL, 0x20000UL);
> +
> + pci_register_region(pbm, "System ROM", 0xf0000UL, 0x10000UL);
> +
> + pci_register_region(pbm, "Video ROM", 0xc0000UL, 0x8000UL);
> }
>
> static void pci_register_iommu_region(struct pci_pbm_info *pbm)
> @@ -370,24 +378,8 @@ static void pci_register_iommu_region(struct pci_pbm_info *pbm)
> const u32 *vdma = of_get_property(pbm->op->dev.of_node, "virtual-dma",
> NULL);
>
> - if (vdma) {
> - struct resource *rp = kzalloc(sizeof(*rp), GFP_KERNEL);
> -
> - if (!rp) {
> - pr_info("%s: Cannot allocate IOMMU resource.\n",
> - pbm->name);
> - return;
> - }
> - rp->name = "IOMMU";
> - rp->start = pbm->mem_space.start + (unsigned long) vdma[0];
> - rp->end = rp->start + (unsigned long) vdma[1] - 1UL;
> - rp->flags = IORESOURCE_BUSY;
> - if (request_resource(&pbm->mem_space, rp)) {
> - pr_info("%s: Unable to request IOMMU resource.\n",
> - pbm->name);
> - kfree(rp);
> - }
> - }
> + if (vdma)
> + pci_register_region(pbm, "IOMMU", vdma[0], vdma[1]);
> }
>
> void pci_determine_mem_io_space(struct pci_pbm_info *pbm)
> @@ -506,8 +498,7 @@ void pci_determine_mem_io_space(struct pci_pbm_info *pbm)
> if (pbm->mem64_space.flags)
> request_resource(&iomem_resource, &pbm->mem64_space);
>
> - pci_register_legacy_regions(&pbm->io_space,
> - &pbm->mem_space);
> + pci_register_legacy_regions(pbm);
> pci_register_iommu_region(pbm);
> }
>
> --
> 1.8.4.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists