linux-kernel - Re: [PATCH v2 01/13] iommu/rockchip: Request irqs in rk_iommu

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 17 Jan 2018 20:46:08 +0800
From:   JeffyChen <jeffy.chen@...k-chips.com>
To:     Robin Murphy <robin.murphy@....com>, linux-kernel@...r.kernel.org
CC:     jcliang@...omium.org, xxm@...k-chips.com, tfiga@...omium.org,
        Heiko Stuebner <heiko@...ech.de>,
        linux-rockchip@...ts.infradead.org,
        iommu@...ts.linux-foundation.org, Joerg Roedel <joro@...tes.org>,
        linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v2 01/13] iommu/rockchip: Request irqs in rk_iommu_probe()

Hi Robin,

On 01/17/2018 08:18 PM, Robin Murphy wrote:
>>
>> @@ -91,7 +92,6 @@ struct rk_iommu {
>>       void __iomem **bases;
>>       int num_mmu;
>>       int *irq;
>
> Nit: irq seems to be redundant now as well.
oops, will fix it.
>
>> -    int num_irq;
>>       bool reset_disabled;
>>       struct iommu_device iommu;
>>       struct list_head node; /* entry in rk_iommu_domain.iommus */
>> @@ -830,13 +830,6 @@ static int rk_iommu_attach_device(struct
>> iommu_domain *domain,
>>       iommu->domain = domain;
>> -    for (i = 0; i < iommu->num_irq; i++) {
>> -        ret = devm_request_irq(iommu->dev, iommu->irq[i], rk_iommu_irq,
>> -                       IRQF_SHARED, dev_name(dev), iommu);
>> -        if (ret)
>> -            return ret;
>> -    }
>> -
>>       for (i = 0; i < iommu->num_mmu; i++) {
>>           rk_iommu_write(iommu->bases[i], RK_MMU_DTE_ADDR,
>>                      rk_domain->dt_dma);
>> @@ -885,9 +878,6 @@ static void rk_iommu_detach_device(struct
>> iommu_domain *domain,
>>       }
>>       rk_iommu_disable_stall(iommu);
>> -    for (i = 0; i < iommu->num_irq; i++)
>> -        devm_free_irq(iommu->dev, iommu->irq[i], iommu);
>> -
>>       iommu->domain = NULL;
>>       dev_dbg(dev, "Detached from iommu domain\n");
>> @@ -1138,7 +1128,7 @@ static int rk_iommu_probe(struct platform_device
>> *pdev)
>>       struct rk_iommu *iommu;
>>       struct resource *res;
>>       int num_res = pdev->num_resources;
>> -    int err, i;
>> +    int err, i, irq, num_irq;
>>       iommu = devm_kzalloc(dev, sizeof(*iommu), GFP_KERNEL);
>>       if (!iommu)
>> @@ -1165,23 +1155,17 @@ static int rk_iommu_probe(struct
>> platform_device *pdev)
>>       if (iommu->num_mmu == 0)
>>           return PTR_ERR(iommu->bases[0]);
>> -    iommu->num_irq = platform_irq_count(pdev);
>> -    if (iommu->num_irq < 0)
>> -        return iommu->num_irq;
>> -    if (iommu->num_irq == 0)
>> -        return -ENXIO;
>> -
>> -    iommu->irq = devm_kcalloc(dev, iommu->num_irq, sizeof(*iommu->irq),
>> -                  GFP_KERNEL);
>> -    if (!iommu->irq)
>> -        return -ENOMEM;
>> -
>> -    for (i = 0; i < iommu->num_irq; i++) {
>> -        iommu->irq[i] = platform_get_irq(pdev, i);
>> -        if (iommu->irq[i] < 0) {
>> -            dev_err(dev, "Failed to get IRQ, %d\n", iommu->irq[i]);
>> +    num_irq = of_irq_count(dev->of_node);
>
> To follow up on the other reply, I'm not sure you really need to count
> the IRQs beforehand at all - you're going to be looping through
> platform_get_irq() and handling errors anyway, so you may as well just
> start at 0 and keep going until -ENOENT (or use platform_get_resource()
> to double-check whether an index should be valid, as we do in arm_smmu).
ok, will do that.
>
> Otherwise, it looks like everything that the IRQ handler needs in the
> iommu struct (dev, num_mmu and bases) is already initialised by this
> point, so we should be OK with respect to races.
ok.
>
> Robin.