linux-kernel - RE: [EXT] Re: [PATCH] driver core: improve cycle detection on fwnode graph

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:
 <DU2PR04MB8822693748725F85DC0CB86C8C792@DU2PR04MB8822.eurprd04.prod.outlook.com>
Date: Fri, 26 Jan 2024 09:00:39 +0000
From: Xu Yang <xu.yang_2@....com>
To: Saravana Kannan <saravanak@...gle.com>
CC: "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
	"rafael@...nel.org" <rafael@...nel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, Android Kernel Team <kernel-team@...roid.com>
Subject: RE: [EXT] Re: [PATCH] driver core: improve cycle detection on fwnode
 graph

Hi Saravana,

> 
> On Wed, Jan 24, 2024 at 8:21 PM Xu Yang <xu.yang_2@....com> wrote:
> >
> I think my confusion was because you use ----> in the opposite way to
> what I have used for all my fw_devlink and cycle detection patches.

Okay, I will follow the usage of "-->" later as yours.

> 
> The part I was referring to is related to how driver/of/property.c has
> node_not_dev set to true for pasrse_remote_endpoint.
> 
> > >
> > > 4. Has this actually caused an issue? If so, what is it? And give me
> > > an example in an upstream DT.
> >
> > Yes, there are two cycles (B.EP->A->C->B and B.EP->A/A.EP->B) in above
> > example. But only one cycle (B.EP->A->C->B) is recognized.
> >
> > My real case as below:
> 
> I think you still missed some details because usb3_phy0 seems

One line is indeed missing in usb3_phy0.

> irrelevant here. Can you just point me to the dts (not dtsi) file for
> this platform in the kernel tree?

This parts of dts is not in upstream kernel tree due to some reasons.
Allow me to show the necessary parts as below again, you can also
get the full dts file from the link I attached below:

---
ptn5110: tcpc@50 {
    compatible = "nxp,ptn5110";
    ...

    port {
        typec_dr_sw: endpoint {
            remote-endpoint = <&usb3_drd_sw>;
        };
    };
};

usb_dwc3_0: usb@...00000 {
    compatible = "snps,dwc3";
    phys = <&usb3_phy0>, <&usb3_phy0>;
    ...

    port {
        usb3_drd_sw: endpoint {
            remote-endpoint = <&typec_dr_sw>;
        };
    };
};

usb3_phy0: usb-phy@...f0040 {
    compatible = "fsl,imx8mp-usb-phy";
    vbus-power-supply = <&ptn5110>;

    ...
};

> Also, can you change all the pr_debug and dev_dbg in
> drivers/base/core.c to their info equivalent and boot up the system
> and give me the logs? That'll be a lot easier for me to understand
> your case.

Thank you for willing to debug this issue.
The boot log and dts file is under: 
https://drive.google.com/drive/folders/1hlkzg042q5_b5l59DCW2pECXRmTH4Vy_?usp=sharing

> 
> So let's say I see your logs and what you say is true, but you still
> aren't telling me what's the problem you have because of this
> incorrect cycle detection. What's breaking? Is something not allowed
> to probe? If so, which one? What's supposed to be the right order of
> probes?
> 

Let me describe the issue again based on above log and dts:

                    usb
                  +-----+
   tcpc           |     |
  +-----+         |  +--|
  |     |----------->|EP|
  |--+  |         |  +--|
  |EP|<-----------|     |
  |--+  |         |  B  |
  |     |         +-----+
  |  A  |            |
  +-----+            |
     ^     +-----+   |
     |     |     |   |
     +-----|  C  |<--+
           |     |
           +-----+
           usb-phy

Node A (tcpc) will be populated as device 1-0050.
Node B (usb) will be populated as device 38100000.usb.
Node C (usb-phy) will be populated as device 381f0040.usb-phy.

1. Node C is populated as device C. But nodes A and B are still not
   populated. When do cycle detection with device C, no cycle is found.
2. Node B is populated as device B. When do cycle detection with device
   B, it found a fwnode link cycle B-->C-->A-->B.EP. Then, fwnode link
   A-->B.EP, C-->A and B-->C are marked as cycle. The fwnode link B-->C
   is converted to device link too.
3. Node A is populated as device A. When do cycle detection with device
   A, it find C-->A is marked as cycle and convert it to device link. It
   also find A-->B.EP is marked as cycle but will not convert it to device
   link since node B.EP is not a device.

Finally, fwnode link B-->C and C-->A is removed, A-->B.EP is only marked
as cycle and B-->A.EP is neither been marked as cycle nor removed.

So there are 2 cycles and only the first cycle is detected.
1. B-->C-->A-->B.EP--B
2. B-->A.EP--A-->B.EP--B

In the end, device 38100000.usb (node B) is defered probe due to node B
still has a supplier node A.EP. 
Device 1-0050 (node A) is also defered probe due to it depends on one device
which is created by 38100000.usb.

The normal behavior is all of the devices can be successfully probed after two
cycles are detected.

Thanks,
Xu Yang