linux-kernel - Re: [PATCH 2/2] driver: core: add probe control driver

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: <ZulEFlDoRz4USTB8@NAB-HP-ProDesk-600sony.com>
Date: Tue, 17 Sep 2024 14:25:50 +0530
From: Nayeemahmed Badebade <nayeemahmed.badebade@...y.com>
To: Rob Herring <robh@...nel.org>
Cc: krzk+dt@...nel.org, conor+dt@...nel.org, gregkh@...uxfoundation.org,
        rafael@...nel.org, yoshihiro.toyama@...y.com,
        linux-kernel@...r.kernel.org, devicetree@...r.kernel.org
Subject: Re: [PATCH 2/2] driver: core: add probe control driver

Hi Rob,

Thank you for taking the time to check our patch and provide
valuable feedback. We appreciate your comments/suggestions.

Please find our reply to your comments.

On Thu, Sep 12, 2024 at 03:46:34PM -0500, Rob Herring wrote:
> On Wed, Sep 11, 2024 at 07:53:19PM +0530, Nayeemahmed Badebade wrote:
> > Probe control driver framework allows deferring the probes of a group of
> > devices to an arbitrary time, giving the user control to trigger the probes
> > after boot. This is useful for deferring probes from builtin drivers that
> > are not required during boot and probe when user wants after boot.
> 
> This seems like the wrong way around to me. Why not define what you want 
> to probe first or some priority order? I could see use for kernel to 
> probe whatever is the console device first. Or the rootfs device... You 
> don't need anything added to DT for those.
> 
> Of course, there's the issue that Linux probes are triggered bottom-up 
> rather than top-down.
> 

Our intention is to only postpone some driver probes not required during
boot, similar to https://elinux.org/Deferred_Initcalls. But instead of
delaying initcall execution(which requires initmem to be kept and not
freed during boot) we are trying to delay driver probes as this is much
simpler.

> 
> > This is achieved by adding a dummy device aka probe control device node
> > as provider to a group of devices(consumer nodes) in platform's device
> > tree. Consumers are the devices we want to probe after boot.
> 
> There's the obvious question of then why not make those devices modules 
> instead of built-in?
> 

Yes we can use modules for this, but there are drivers that cannot be
built as modules and this framework is specifically for such scenario.
Example: drivers/pci/controller/dwc/pci-imx6.c

> > 
> > To establish control over consumer device probes, each consumer device node
> > need to refer the probe control provider node by the phandle.
> > 'probe-control-supply' property is used for this.
> > 
> > Example:
> >     // The node below defines a probe control device/provider node
> >     prb_ctrl_dev_0: prb_ctrl_dev_0 {
> >         compatible = "linux,probe-control";
> >     };
> > 
> >     // The node below is the consumer device node that refers to provider
> >     // node by its phandle and a result will not be probed until provider
> >     // node is probed.
> >     pcie@...c000 {
> >         reg = <0x01ffc000 0x04000>, <0x01f00000 0x80000>;
> >         #address-cells = <3>;
> >         #size-cells = <2>;
> >         device_type = "pci";
> >         ranges = <0x81000000 0 0          0x01f80000 0 0x00010000>,
> >                  <0x82000000 0 0x01000000 0x01000000 0 0x00f00000>;
> > 
> >         probe-control-supply = <&prb_ctrl_dev_0>;
> >     };
> 
> Sorry, but this isn't going to happen in DT.
> 

You mean we cannot add custom properties like this to an existing
device node in DT?

> > 
> > fw_devlink ensures consumers are not probed until provider is probed
> > successfully. The provider probe during boot returns -ENXIO and is not
> > re-probed again.
> > 
> > The driver provides debug interface /sys/kernel/debug/probe_control_status
> > for checking probe control status of registered probe control devices.
> >  # cat /sys/kernel/debug/probe_control_status
> >  prb_ctrl_dev_0: [not triggered]
> >   Consumers: 1ffc000.pcie
> > 
> > Interface /sys/kernel/probe_control/trigger is provided for triggering
> > probes of the probe control devices. User can write to this interface to
> > trigger specific or all device probes managed by this driver.
> > Once the probe is triggered by user, provider probe control device is added
> > to deferred_probe_pending_list and driver_deferred_probe_trigger() is
> > triggered. This time the probe of probe control device will be
> > successful and its consumers will then be probed.
> > 
> > To trigger specific provider probe:
> >   # echo prb_ctrl_dev_0 > /sys/kernel/probe_control/trigger
> > 
> > To trigger all registered provider probes
> >   # echo all > /sys/kernel/probe_control/trigger
> > 
> > Signed-off-by: Toyama Yoshihiro <yoshihiro.toyama@...y.com>
> > Signed-off-by: Nayeemahmed Badebade <nayeemahmed.badebade@...y.com>
> 
> This is wrong. Either Toyama is the author and you need to fix the git 
> author, or you both are authors and you need a Co-developed-by tag for 
> Toyama.

Sorry about that, we will fix this.

Thanks,
Nayeem