linux-kernel - Re: [PATCH 1/2] staging: fsl-mc: Allocate IRQ's before scanning DPRC objects

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] [day] [month] [year] [list]

Message-ID: <5A294826.9030805@nxp.com>
Date:   Thu, 7 Dec 2017 13:55:36 +0000
From:   Laurentiu Tudor <laurentiu.tudor@....com>
To:     Nipun Gupta <nipun.gupta@....com>,
        "stuyoder@...il.com" <stuyoder@...il.com>,
        Bharat Bhushan <bharat.bhushan@....com>,
        "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        "cakturk@...il.com" <cakturk@...il.com>,
        "bretth256@...il.com" <bretth256@...il.com>,
        "arnd@...db.de" <arnd@...db.de>
CC:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "devel@...verdev.osuosl.org" <devel@...verdev.osuosl.org>
Subject: Re: [PATCH 1/2] staging: fsl-mc: Allocate IRQ's before scanning DPRC
 objects



On 12/07/2017 03:18 PM, Nipun Gupta wrote:
>
>
>> -----Original Message-----
>> From: Laurentiu Tudor
>> Sent: Wednesday, December 06, 2017 19:00
>> To: Nipun Gupta <nipun.gupta@....com>; stuyoder@...il.com; Bharat
>> Bhushan <bharat.bhushan@....com>; gregkh@...uxfoundation.org;
>> cakturk@...il.com; bretth256@...il.com; arnd@...db.de
>> Cc: linux-kernel@...r.kernel.org; devel@...verdev.osuosl.org
>> Subject: Re: [PATCH 1/2] staging: fsl-mc: Allocate IRQ's before scanning DPRC
>> objects
>>
>> Hi Nipun,
>>
>> Can you polish a bit this commit message? It doesn't seem to explain why
>> this is needed.
>
> Sure. Ill update the message.
>
>>
>> On 12/06/2017 06:18 PM, Nipun Gupta wrote:
>>> When DPRC probing is deferred (such as where IOMMU is not probed
>>> before the fsl-mc bus), all the devices in the DPRC containers gets
>>> initialized one after another.
>>
>> Are you referring to dprc probing being deferred (do we ever do that?)
>> or devices inside the dprc deferring the probe?
>
> Yes.. Currently following is the scenario when the devices are probed
> (please correct me if I am wrong):
>
> 	FSL_MC Bus probe ---> dprc probe ---> dprc devices scan --->
> 		probe of devices in DPRC container ---> allocate IRQ's.
>
> In case the devices being probed in the DPRC container need the IRQ's;
> probing of that device will fail.
> In the current scenario DPIO device while getting probed for the first time
> gets deferred because the DPIO driver is not yet registered.
> So there is no impact seen in the current code.
>
> In case the DPRC probing gets deferred, does in case IOMMU is enabled
> (though it is not enabled till now for fsl-mc bus but we plan to add the
> support as soon as mc bus is out from staging); the DPIO gets probed
> before IRQ's being allocated. This causes DPIO probe to fail.
>
> So I think it makes sense that IRQ's are allocated before any devices
> In the DPRC container are probed.

Thanks for the details. It would be great if you could update the commit 
message with these execution flow details.

>>
>>> As IRQ's were allocated only once the
>>> DPRC scanning is completed, the devices like DPIO which uses these
>>> IRQ's at initalization fails. This patch allocates the IRQ resources
>>
>> s/initalization/initialization
>>
>>> before scanning all the objects in the DPRC container.
>>>
>>> Signed-off-by: Nipun Gupta <nipun.gupta@....com>
>>> ---
>>>    drivers/staging/fsl-mc/bus/dprc-driver.c | 49 ++++++++++++++++++------------
>> --
>>>    1 file changed, 27 insertions(+), 22 deletions(-)
>>>
>>> diff --git a/drivers/staging/fsl-mc/bus/dprc-driver.c b/drivers/staging/fsl-
>> mc/bus/dprc-driver.c
>>> index 06df528..7265431 100644
>>> --- a/drivers/staging/fsl-mc/bus/dprc-driver.c
>>> +++ b/drivers/staging/fsl-mc/bus/dprc-driver.c
>>> @@ -206,7 +206,8 @@ static void dprc_add_new_devices(struct
>> fsl_mc_device *mc_bus_dev,
>>>     * dprc_scan_objects - Discover objects in a DPRC
>>>     *
>>>     * @mc_bus_dev: pointer to the fsl-mc device that represents a DPRC object
>>> - * @total_irq_count: total number of IRQs needed by objects in the DPRC.
>>> + * @total_irq_count: If argument is provided the function populates the
>>> + * total number of IRQs created by objects in the DPRC.
>>
>> As a side node, after a cursory look i noticed that this total_irq_count
>> parameter is used only for some sanity checks. I'm thinking of dropping
>> it in a follow-up cleanup patch.
>
> Do you plan to send it very recently.
> In that case I can rebase my patch on top of it.

It's not on my short term TODO.

---
Best Regards, Laurentiu