Re: [PATCH v3 2/3] usb: xhci: Add DbC support in xHCI driver

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Mon, Nov 06, 2017 at 08:35:41AM +0800, Lu Baolu wrote:
> Hi,
> 
> On 11/03/2017 02:27 PM, Greg Kroah-Hartman wrote:
> > On Fri, Nov 03, 2017 at 08:45:46AM +0800, Lu Baolu wrote:
> >> Hi,
> >>
> >> On 11/03/2017 12:51 AM, Greg Kroah-Hartman wrote:
> >>> On Thu, Nov 02, 2017 at 12:38:57PM +0200, Felipe Balbi wrote:
> >>>>> Hi,
> >>>>>
> >>>>> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> >>>>>>>>> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> >>>>>>>>>>>>>>> xHCI compatible USB host controllers(i.e. super-speed USB3 controllers)
> >>>>>>>>>>>>>>> can be implemented with the Debug Capability(DbC). It presents a debug
> >>>>>>>>>>>>>>> device which is fully compliant with the USB framework and provides the
> >>>>>>>>>>>>>>> equivalent of a very high performance full-duplex serial link. The debug
> >>>>>>>>>>>>>>> capability operation model and registers interface are defined in 7.6.8
> >>>>>>>>>>>>>>> of the xHCI specification, revision 1.1.
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>> The DbC debug device shares a root port with the xHCI host. By default,
> >>>>>>>>>>>>>>> the debug capability is disabled and the root port is assigned to xHCI.
> >>>>>>>>>>>>>>> When the DbC is enabled, the root port will be assigned to the DbC debug
> >>>>>>>>>>>>>>> device, and the xHCI sees nothing on this port. This implementation uses
> >>>>>>>>>>>>>>> a sysfs node named <dbc> under the xHCI device to manage the enabling
> >>>>>>>>>>>>>>> and disabling of the debug capability.
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>> When the debug capability is enabled, it will present a debug device
> >>>>>>>>>>>>>>> through the debug port. This debug device is fully compliant with the
> >>>>>>>>>>>>>>> USB3 framework, and it can be enumerated by a debug host on the other
> >>>>>>>>>>>>>>> end of the USB link. As soon as the debug device is configured, a TTY
> >>>>>>>>>>>>>>> serial device named /dev/ttyDBC0 will be created.
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>> One use of this link is running a login service on the debug target.
> >>>>>>>>>>>>>>> Hence it can be remote accessed by a debug host. Another use case can
> >>>>>>>>>>>>>>> probably be found in servers. It provides a peer-to-peer USB link
> >>>>>>>>>>>>>>> between two host-only machines. This provides a reasonable out-of-band
> >>>>>>>>>>>>>>> communication method between two servers.
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> >>>>>>>>>>>>>>> ---
> >>>>>>>>>>>>>>>  .../ABI/testing/sysfs-bus-pci-drivers-xhci_hcd     |   25 +
> >>>>>>>>>>>>>>>  drivers/usb/host/Kconfig                           |    9 +
> >>>>>>>>>>>>>>>  drivers/usb/host/Makefile                          |    5 +
> >>>>>>>>>>>>>>>  drivers/usb/host/xhci-dbgcap.c                     | 1016 ++++++++++++++++++++
> >>>>>>>>>>>>>>>  drivers/usb/host/xhci-dbgcap.h                     |  247 +++++
> >>>>>>>>>>>>>>>  drivers/usb/host/xhci-dbgtty.c                     |  586 +++++++++++
> >>>>>>>>>>>>>>>  drivers/usb/host/xhci-trace.h                      |   60 ++
> >>>>>>>>>>>>>>>  drivers/usb/host/xhci.c                            |   10 +
> >>>>>>>>>>>>>>>  drivers/usb/host/xhci.h                            |    1 +
> >>>>>>>>>>>>>>>  9 files changed, 1959 insertions(+)
> >>>>>>>>>>>>>>>  create mode 100644 Documentation/ABI/testing/sysfs-bus-pci-drivers-xhci_hcd
> >>>>>>>>>>>>>>>  create mode 100644 drivers/usb/host/xhci-dbgcap.c
> >>>>>>>>>>>>>>>  create mode 100644 drivers/usb/host/xhci-dbgcap.h
> >>>>>>>>>>>>>>>  create mode 100644 drivers/usb/host/xhci-dbgtty.c
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>> [snip]
> >>>>>>>>>>>>>
> >>>>>>>>>>>>>>> +#define DBC_VENDOR_ID			0x1d6b	/* Linux Foundation 0x1d6b */
> >>>>>>>>>>>>>>> +#define DBC_PRODUCT_ID			0x0004	/* device 0004 */
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>> The DbC (xHCI DeBug Capability) is an optional functionality in
> >>>>>>>>>>>>> some xHCI host controllers. It will present a super-speed debug
> >>>>>>>>>>>>> device through the debug port after it is enabled.
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> The DbC register set defines an interface for system software
> >>>>>>>>>>>>> to specify the vendor id and product id of the debug device.
> >>>>>>>>>>>>> These two values will be presented by the debug device in its
> >>>>>>>>>>>>> device descriptor idVendor and idProduct fields.
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> Microsoft Windows have a well established protocol for
> >>>>>>>>>>>>> debugging over DbC. And it assigns below values for its use.
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> USB\VID_045E&PID_062D.DeviceDesc="Microsoft USB Debug Target"
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> I'm going to use 0x1d6b/0x0004 value pair for DbC use in
> >>>>>>>>>>>>> Linux. Do you approve me to do so?
> >>>>>>>>>>> No.  Why can't you use the same ids as Windows?  This is implementing
> >>>>>>>>>>> the same protocol, right?
> >>>>>>>>> the protocol running on top is 100% vendor specific. More than likely,
> >>>>>>>>> we would just run kgdb on top of this, right? We really don't support
> >>>>>>>>> microsoft's debug architecture.
> >>>>>>> Ah, I didn't know about the protocol specifics here, if it is
> >>>>>>> vendor-specific, then yes, we need our own id.
> >>>>> Great, thanks :-)
> >>>>>
> >>>>> Let us know which one we're allowed to use and I'm sure Baolu can respin
> >>>>> the patch in no time.
> >>> Can I get a "full" description of what string this device id will
> >>> reference?  Is it "Linux USB Debug Target" or something else?
> >>>
> >> Current manufacturer and product strings are set like this.
> >>
> >> +#define DBC_STRING_MANUFACTURER	"Linux"
> >> +#define DBC_STRING_PRODUCT		"Remote	GDB"
> >>
> >> These are also place holders. We can change them to more meaningful strings.
> > As the vendor id is assigned to "Linux Foundation" can we change that
> > string please?
> >
> > And why not match what Microsoft does here, "USB Debug Target" makes
> > more sense than "Remote GDB", right?
> >
> > If so, please use device id 0x0010, I've reserved that for the driver
> > now.
> >
> 
> I will change the strings and use the allocated product id.
> Thank you very much.
> 
> Another DbC usage is to enable DbC during early boot time,
> and run GDB protocol data or early printk message over it.
> The early printk implementation has been merged in 4.12-rc1.
> 
> commit aeb9dd1 <usb/early: Add driver for xhci debug capability>
> 
> In this code, the DbC product id is still a place holder (0x0004).
> I am really sorry. I should apply a valid product id from you during
> the code review time. But I forgot it since almost all attention was
> paid to early driver itself at that time.
> 
> We could use the same ID as you allocated here or, preferably,
> use a different ID since we're dealing with different functionality. 

A different one would be best, please give me a good device string to
use and I will assign you a new ID, probably 0011.  Also send a patch
now for this please.

thanks,

greg k-h