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

[Lu Baolu <baolu.lu@linux.intel.com>]

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. 

Best regards,
Lu Baolu