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]
Message-ID: <OS0PR01MB59227E69FF1DE67327BDEF77864A9@OS0PR01MB5922.jpnprd01.prod.outlook.com>
Date:   Mon, 29 May 2023 09:39:50 +0000
From:   Biju Das <biju.das.jz@...renesas.com>
To:     Marc Zyngier <maz@...nel.org>
CC:     Laurent Pinchart <laurent.pinchart@...asonboard.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Zheng Wang <zyytlz.wz@....com>,
        Geert Uytterhoeven <geert+renesas@...der.be>,
        Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com>,
        Wolfram Sang <wsa+renesas@...g-engineering.com>,
        Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
        Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@...renesas.com>,
        "linux-renesas-soc@...r.kernel.org" 
        <linux-renesas-soc@...r.kernel.org>
Subject: RE: [PATCH] usb: gadget: udc: renesas_usb3: Fix RZ/V2M
 {modprobe,bind} error

Hi Marc Zyngier,

Thanks for the feedback.

> Subject: Re: [PATCH] usb: gadget: udc: renesas_usb3: Fix RZ/V2M
> {modprobe,bind} error
> 
> On Mon, 29 May 2023 09:42:34 +0100,
> Biju Das <biju.das.jz@...renesas.com> wrote:
> >
> > Hi Laurent,
> >
> > Thanks for the feedback.
> >
> > > Subject: Re: [PATCH] usb: gadget: udc: renesas_usb3: Fix RZ/V2M
> > > {modprobe,bind} error
> > >
> > > Hi Biju,
> > >
> > > Thank you for the patch.
> > >
> > > On Fri, May 26, 2023 at 03:36:15PM +0100, Biju Das wrote:
> > > > Currently {modprobe, bind} after {rmmod, unbind} results in probe
> > > failure.
> > > >
> > > > genirq: Flags mismatch irq 22. 00000004 (85070400.usb3drd) vs.
> > > > 00000004 (85070400.usb3drd)
> > > > renesas_usb3: probe of 85070000.usb3peri failed with error -16
> > > >
> > > > Fix this issue by replacing "parent dev"->"dev" as the irq
> > > > resource is managed by this driver.
> > >
> > > If the dev pointer passed to devm_request_irq() is not the correct
> > > one, how does it work the first time the driver is loaded ?
> >
> > + Marc/ Kernel.org to give some feedback on this issue
> >
> > I believe there may be a bug in the genirq (kernel/irq) driver.
> > first time it works ok. Maybe this driver is caching on unload with
> > null value and comparing with actual one (irq 22) during reload??
> >
> > Maybe genirq expert can comment what went wrong here??
> 
> You get shouted at because you are registering an interrupt handler for
> the same IRQ twice,

This not true. It is registering only one IRQ, but with parent device handle.

 and the interrupt is not configured with the SHARED
> flag.

I haven't added SHARED flag as there is only one IRQ registration.

 If, as I understand it, you only have a single device using this
> interrupt, then it means your driver is not freeing its interrupt on
> unload.

You mean devm_request_irq(ddata->dev..)  doesn't free the resource as
we have unloaded only child device rather than parent.

But while parent is active, why genirq is giving error during reload?
It should show same behaviour like initial probe.

> 
> And that's probably because the device object used when requesting the
> interrupt isn't the one you load/unload, as indicated by the message.
> On the first load of "usb3peri", you register an interrupt with
> "usb3drd" as the requester device. You then unload "usb3peri", which of
> course has no effect whatsoever on the interrupt.
> 
> You could simply have done a "cat /proc/interrupt" and see that
> interrupt was still there after unload.

Yes, interrupt still there after unload.

With devm_request_irq(ddata->dev..), after unload
=================================================

root@...2m:~# cat /proc/interrupts | grep usb
 22:          0     GICv2 274 Level     85070400.usb3drd
 28:          0     GICv2 278 Level     85070000.usb3peri
root@...2m:~# lsmod
Module                  Size  Used by
hd3ss3220              12288  0
typec                  73728  1 hd3ss3220
renesas_usb3           32768  1
i2c_rzv2m              12288  0
crct10dif_ce           12288  1
ipv6                  450560  16
root@...2m:~# rmmod hd3ss3220
root@...2m:~# rmmod renesas_usb3
root@...2m:~# cat /proc/interrupts | grep usb
 22:          0     GICv2 274 Level     85070400.usb3drd
root@...2m:~#

With devm_request_irq(&pdev->dev..), after unload
================================================

root@...2m:~# cat /proc/interrupts | grep usb
 22:          0     GICv2 274 Level     85070400.usb3drd
 28:          0     GICv2 278 Level     85070000.usb3peri
root@...2m:~# lsmod
Module                  Size  Used by
hd3ss3220              12288  0
typec                  73728  1 hd3ss3220
renesas_usb3           32768  1
crct10dif_ce           12288  1
i2c_rzv2m              12288  0
ipv6                  450560  16
root@...2m:~# rmmod hd3ss3220
root@...2m:~# rmmod renesas_usb3
root@...2m:~# cat /proc/interrupts | grep usb
root@...2m:~#

> 
> So the only bug here is in the handling of the interrupt request. And
> that bug firmly lies in your code. My "expert" advise is to debug the
> problem rather than suspecting some random failure modes.

With devm_request_irq(&pdev->dev..) the above issue is fixed.

Or

the correct way is passing SHARED flag with devm_request_irq(ddata ->dev..), 
as the resource is owned by the parent??

Cheers,
Biju

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ