linux-kernel - Re: [PATCH] usb: dwc3: Log dwc3 instance name in traces

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: <20250911013631.hv4ibt3j452tbsex@synopsys.com>
Date: Thu, 11 Sep 2025 01:36:32 +0000
From: Thinh Nguyen <Thinh.Nguyen@...opsys.com>
To: Prashanth K <prashanth.k@....qualcomm.com>
CC: Thinh Nguyen <Thinh.Nguyen@...opsys.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] usb: dwc3: Log dwc3 instance name in traces

On Tue, Sep 09, 2025, Prashanth K wrote:
> 
> 
> On 9/5/2025 5:14 AM, Thinh Nguyen wrote:
> > On Thu, Sep 04, 2025, Prashanth K wrote:
> >>
> >>
> >> On 9/4/2025 5:30 AM, Thinh Nguyen wrote:
> >>> On Wed, Sep 03, 2025, Prashanth K wrote:
> >>>>
> >>>>
> >>>> On 9/3/2025 5:14 AM, Thinh Nguyen wrote:
> >>>>> On Mon, Sep 01, 2025, Prashanth K wrote:
> >>>>>>
> >>>>>>
> >>>>>> On 8/29/2025 4:18 AM, Thinh Nguyen wrote:
> >>>>>>> Hi,
> >>>>>>>
> >>>>>>> On Mon, Aug 25, 2025, Prashanth K wrote:
> >>>>>>>> When multiple DWC3 controllers are being used, trace events from
> >>>>>>>> different instances get mixed up making debugging difficult as
> >>>>>>>> there's no way to distinguish which instance generated the trace.
> >>>>>>>>
> >>>>>>>> Append the device name to trace events to clearly identify the
> >>>>>>>> source instance.
> >>>>>>>
> >>>>>>> Can we print the base address instead of the device name? This will be
> >>>>>>> consistent across different device names, and it will be easier to
> >>>>>>> create filter.
> >>>>>>>
> >>>>>> Did you mean to print the iomem (base address) directly?
> >>>>>> I think using device name is more readable, in most cases device name
> >>>>>> would contain the base address also. Let me know if you are pointing to
> >>>>>> something else.>>
> >>>>>
> >>>>> Yes, I mean the device base address. PCI devices won't have the base
> >>>>> address as part of the device name.
> >>>>>
> >>>> But the base address (void __iomem *base) wouldn't be helpful.
> >>>> Using the base address, i guess we would be able to differentiate the
> >>>> traces when there are multiple instances, but it wouldn't help us
> >>>> identify which controller instance generated which trace.
> >>>>
> >>>> And for PCI devices, i agree that it doesn't have  address in device
> >>>> name, but i think we should be able to identify the correct instance
> >>>> based on the bus/device numbers, right ?
> >>>>
> >>>
> >>> We may not have the PCI domain numbers if it's a child device as in the
> >>> case of dwc3-pci or dwc3-haps.
> >>>
> >>> The base address _does_ tell you exactly which device the tracepoints
> >>> correspond to. The device name is inconsistent between different device
> >>> types and only relevant if we have access to the system to know which
> >>> name belongs to which instance.
> >>
> >> Yes, I agree that device name would be inconsistent for different for
> >> PCI (and HAPS) devices. But IMO using base address (virtual) would just
> >> make it more harder to read and identify the instance.
> >>
> >> Perhaps we can cache the register addr and use it, what do you think?
> >> Here we can at least differentiate the instances based on HW addr.
> >>
> >> snprintf(dwc->inst, sizeof(dwc->inst), "0x%08llx", (unsigned long
> >> long)res->start);
> >> dev_info(dwc->dev, "addr:%s\n", dwc->inst);
> >>
> >> Output --> [    4.521746] dwc3 a600000.usb: addr:0x0a600000
> > 
> > I think there's some misunderstanding here. I refer the base address as
> > the hardware address.
> > 
> > I prefer something like this:
> > 
> >     dwc3_event: 0a600000: event (00000101): Reset [U0]
> > 
> > instead of the device name like this:
> > 
> >    dwc3_event: a600000.usb: event (00000101): Reset [U0]
> > 
> > BR,
> > Thinh
> 
> Initially I was also talking about HW address, but since we were
> discussing this under dwc3_readl/writel functions context, i also got
> confused whether you are pointing out the HW address or virtual address.
> 
> Anyways, i guess the above method using snprintf on res->start is one
> way to get base address, is there any way to do this?
> 

You're right. I forgot that we can't do virt_to_phys() for ioremapped
resource...

In that case, can we pass the dwc3 structure in the dwc3_readl/writel? I
know there are many places that this change may touch, but I feel that
it's easier to read than creating the new macro dwc3_readl/writel.

Thanks,
Thinh