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:   Thu, 7 Dec 2017 13:18:48 +0000
From:   Nipun Gupta <nipun.gupta@....com>
To:     Laurentiu Tudor <laurentiu.tudor@....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



> -----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.

> 
> > 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.

Regards,
Nipun

> 
> ---
> Best Regards, Laurentiu

Powered by blists - more mailing lists