| 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: <52B888C2.8040303@ti.com>
Date: Mon, 23 Dec 2013 13:02:26 -0600
From: "Anna, Suman" <s-anna@...com>
To: Florian Vaussard <florian.vaussard@...l.ch>,
Joerg Roedel <joro@...tes.org>,
Tony Lindgren <tony@...mide.com>,
Benoît Cousson <bcousson@...libre.com>
CC: Rob Herring <rob.herring@...xeda.com>,
Pawel Moll <pawel.moll@....com>,
Mark Rutland <mark.rutland@....com>,
Ian Campbell <ijc+devicetree@...lion.org.uk>,
Kumar Gala <galak@...eaurora.org>,
Rob Landley <rob@...dley.net>,
Grant Likely <grant.likely@...aro.org>,
Hiroshi Doyu <hdoyu@...dia.com>,
"iommu@...ts.linux-foundation.org" <iommu@...ts.linux-foundation.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-omap@...r.kernel.org" <linux-omap@...r.kernel.org>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH 1/7] iommu/omap: Do bus_set_iommu() only if probe() succeeds
Hi Florian,
On 12/17/2013 06:53 AM, Florian Vaussard wrote:
> Currently, bus_set_iommu() is done in omap_iommu_init(). However,
> omap_iommu_probe() can fail in a number of ways, leaving the platform
> bus with a dangling reference to a non-initialized iommu. Perform
> bus_set_iommu() only if omap_iommu_probe() succeed.
Can you clarify a bit more on what kind of issues you were seeing
specifically? In general, there can be multiple instances of the iommu,
so setting it in probe may not be fixing whatever issue you were seeing.
The current OMAP3 code has only the ISP MMU enabled, but there is also
another one for the IVA MMU (currently not configured by default).
Moving the bus_set_iommu to probe makes sense if only one iommu is
present, so this patch may not be needed at all.
Also, the main change in this patch is moving the bus_set_iommu from
omap_iommu_init to omap_iommu_probe, so you should probably leave out
moving the function. The omap_iommu_probe function would anyway need
conversion to using devm_ functions.
regards
Suman
>
> Signed-off-by: Florian Vaussard <florian.vaussard@...l.ch>
> ---
> drivers/iommu/omap-iommu.c | 207 +++++++++++++++++++++++----------------------
> 1 file changed, 104 insertions(+), 103 deletions(-)
>
> diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
> index bcd78a7..ee83bcc 100644
> --- a/drivers/iommu/omap-iommu.c
> +++ b/drivers/iommu/omap-iommu.c
> @@ -930,107 +930,6 @@ static void omap_iommu_detach(struct omap_iommu *obj)
> dev_dbg(obj->dev, "%s: %s\n", __func__, obj->name);
> }
>
> -/*
> - * OMAP Device MMU(IOMMU) detection
> - */
> -static int omap_iommu_probe(struct platform_device *pdev)
> -{
> - int err = -ENODEV;
> - int irq;
> - struct omap_iommu *obj;
> - struct resource *res;
> - struct iommu_platform_data *pdata = pdev->dev.platform_data;
> -
> - obj = kzalloc(sizeof(*obj) + MMU_REG_SIZE, GFP_KERNEL);
> - if (!obj)
> - return -ENOMEM;
> -
> - obj->nr_tlb_entries = pdata->nr_tlb_entries;
> - obj->name = pdata->name;
> - obj->dev = &pdev->dev;
> - obj->ctx = (void *)obj + sizeof(*obj);
> - obj->da_start = pdata->da_start;
> - obj->da_end = pdata->da_end;
> -
> - spin_lock_init(&obj->iommu_lock);
> - mutex_init(&obj->mmap_lock);
> - spin_lock_init(&obj->page_table_lock);
> - INIT_LIST_HEAD(&obj->mmap);
> -
> - res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - if (!res) {
> - err = -ENODEV;
> - goto err_mem;
> - }
> -
> - res = request_mem_region(res->start, resource_size(res),
> - dev_name(&pdev->dev));
> - if (!res) {
> - err = -EIO;
> - goto err_mem;
> - }
> -
> - obj->regbase = ioremap(res->start, resource_size(res));
> - if (!obj->regbase) {
> - err = -ENOMEM;
> - goto err_ioremap;
> - }
> -
> - irq = platform_get_irq(pdev, 0);
> - if (irq < 0) {
> - err = -ENODEV;
> - goto err_irq;
> - }
> - err = request_irq(irq, iommu_fault_handler, IRQF_SHARED,
> - dev_name(&pdev->dev), obj);
> - if (err < 0)
> - goto err_irq;
> - platform_set_drvdata(pdev, obj);
> -
> - pm_runtime_irq_safe(obj->dev);
> - pm_runtime_enable(obj->dev);
> -
> - dev_info(&pdev->dev, "%s registered\n", obj->name);
> - return 0;
> -
> -err_irq:
> - iounmap(obj->regbase);
> -err_ioremap:
> - release_mem_region(res->start, resource_size(res));
> -err_mem:
> - kfree(obj);
> - return err;
> -}
> -
> -static int omap_iommu_remove(struct platform_device *pdev)
> -{
> - int irq;
> - struct resource *res;
> - struct omap_iommu *obj = platform_get_drvdata(pdev);
> -
> - iopgtable_clear_entry_all(obj);
> -
> - irq = platform_get_irq(pdev, 0);
> - free_irq(irq, obj);
> - res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - release_mem_region(res->start, resource_size(res));
> - iounmap(obj->regbase);
> -
> - pm_runtime_disable(obj->dev);
> -
> - dev_info(&pdev->dev, "%s removed\n", obj->name);
> - kfree(obj);
> - return 0;
> -}
> -
> -static struct platform_driver omap_iommu_driver = {
> - .probe = omap_iommu_probe,
> - .remove = omap_iommu_remove,
> - .driver = {
> - .name = "omap-iommu",
> - },
> -};
> -
> static void iopte_cachep_ctor(void *iopte)
> {
> clean_dcache_area(iopte, IOPTE_TABLE_SIZE);
> @@ -1265,6 +1164,110 @@ static struct iommu_ops omap_iommu_ops = {
> .pgsize_bitmap = OMAP_IOMMU_PGSIZES,
> };
>
> +/*
> + * OMAP Device MMU(IOMMU) detection
> + */
> +static int omap_iommu_probe(struct platform_device *pdev)
> +{
> + int err = -ENODEV;
> + int irq;
> + struct omap_iommu *obj;
> + struct resource *res;
> + struct iommu_platform_data *pdata = pdev->dev.platform_data;
> +
> + obj = kzalloc(sizeof(*obj) + MMU_REG_SIZE, GFP_KERNEL);
> + if (!obj)
> + return -ENOMEM;
> +
> + obj->nr_tlb_entries = pdata->nr_tlb_entries;
> + obj->name = pdata->name;
> + obj->dev = &pdev->dev;
> + obj->ctx = (void *)obj + sizeof(*obj);
> + obj->da_start = pdata->da_start;
> + obj->da_end = pdata->da_end;
> +
> + spin_lock_init(&obj->iommu_lock);
> + mutex_init(&obj->mmap_lock);
> + spin_lock_init(&obj->page_table_lock);
> + INIT_LIST_HEAD(&obj->mmap);
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!res) {
> + err = -ENODEV;
> + goto err_mem;
> + }
> +
> + res = request_mem_region(res->start, resource_size(res),
> + dev_name(&pdev->dev));
> + if (!res) {
> + err = -EIO;
> + goto err_mem;
> + }
> +
> + obj->regbase = ioremap(res->start, resource_size(res));
> + if (!obj->regbase) {
> + err = -ENOMEM;
> + goto err_ioremap;
> + }
> +
> + irq = platform_get_irq(pdev, 0);
> + if (irq < 0) {
> + err = -ENODEV;
> + goto err_irq;
> + }
> +
> + err = request_irq(irq, iommu_fault_handler, IRQF_SHARED,
> + dev_name(&pdev->dev), obj);
> + if (err < 0)
> + goto err_irq;
> + platform_set_drvdata(pdev, obj);
> +
> + bus_set_iommu(&platform_bus_type, &omap_iommu_ops);
> +
> + pm_runtime_irq_safe(obj->dev);
> + pm_runtime_enable(obj->dev);
> +
> + dev_info(&pdev->dev, "%s registered\n", obj->name);
> + return 0;
> +
> +err_irq:
> + iounmap(obj->regbase);
> +err_ioremap:
> + release_mem_region(res->start, resource_size(res));
> +err_mem:
> + kfree(obj);
> + return err;
> +}
> +
> +static int omap_iommu_remove(struct platform_device *pdev)
> +{
> + int irq;
> + struct resource *res;
> + struct omap_iommu *obj = platform_get_drvdata(pdev);
> +
> + iopgtable_clear_entry_all(obj);
> +
> + irq = platform_get_irq(pdev, 0);
> + free_irq(irq, obj);
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + release_mem_region(res->start, resource_size(res));
> + iounmap(obj->regbase);
> +
> + pm_runtime_disable(obj->dev);
> +
> + dev_info(&pdev->dev, "%s removed\n", obj->name);
> + kfree(obj);
> + return 0;
> +}
> +
> +static struct platform_driver omap_iommu_driver = {
> + .probe = omap_iommu_probe,
> + .remove = omap_iommu_remove,
> + .driver = {
> + .name = "omap-iommu",
> + },
> +};
> +
> static int __init omap_iommu_init(void)
> {
> struct kmem_cache *p;
> @@ -1277,8 +1280,6 @@ static int __init omap_iommu_init(void)
> return -ENOMEM;
> iopte_cachep = p;
>
> - bus_set_iommu(&platform_bus_type, &omap_iommu_ops);
> -
> return platform_driver_register(&omap_iommu_driver);
> }
> /* must be ready before omap3isp is probed */
>
--
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