ugensa: add support for Google Cr50 (GSC) Closed Case Debugging UART interfaces

[val_packett.cool]

Summary

The Google Security Chip (its Cr50 firmware) on Chromebooks offers Closed Case Debugging via a special USB cable. usbconfig looks like: P326

The UARTs on this device are handled in Linux by usb-serial-simple. The simplest of the USB serial drivers on FreeBSD seems to be ugensa, and searching on the internet confirms that it's supposed to be generic (though sadly there's no description in the tree, even digging through git log for that file does not reveal anything interesting).

~~Unfortunately, at first I was stuck with the device resetting itself as soon as I opened the serial port. I've spent a whole evening wondering what the hell was wrong (looking at the bus in Wireshark and only finding a URB_COMPLETE Cancelled packet). Then I wrote a Python script using py36-libusb1 that simply reads the bulk endpoint… and I saw the expected console output.

Looking more carefully at what was "not simple" in ugensa revealed the "clear stall at first run" block, which turned out to be the culprit.~~

There was a bug in this device, but it was fixed in firmware 0.3.24.

---

maybe the device description should be set to something generic? currently it picks up the string for the first of the serial ports:

ugensa0: <Shell> on usbus0
ugensa0: Found 3 serial ports.

but "Shell" does not apply to the whole device, it applies to the first port cuaU0.0, while cuaU0.1 is "AP" and cuaU0.2 is "EC".

[imp]

sys/dev/usb/serial/ugensa.c

216

I'd prefer a better comment here too.

217

This needs to be wrapped at 80 columns

239

I'd be quite interested to see what hps has to say about this...
But I kinda suspect we do need it at least for some devices (I know we need it for other devices I've messed with, but those werent gensa devices).

[• hselasky]

sys/dev/usb/serial/ugensa.c

239

USB BULK packets are transferred using a data toggle of 0 or 1. If there is a mismatch when you open the connection, then the first piece of data is lost. I recommend you leave these in. If the device does not support these, then this is an indication that the device is broken! Please add a quick for this.

[• hselasky]

sys/dev/usb/serial/ugensa.c

217

Code should look like this:

if (uaa->info.match_flag_int_subclass &&
<4spaces>iface->idesc->bInterfaceSubClass != uaa->info.bInterfaceSubClass)
<tab>continue;

[• hselasky]

Likely this Google device is not USB certified. FreeBSD has a usbtest in src under tools. And others have the command verifier:
https://www.usb.org/document-library/usb20cv-x64-bit-ver-15100

See sys/dev/usb/quirk * for info how to add a new quirk.

If you can use libusb that would be the best. We also have cuse (character device in user-space) and maybe a simple tool we can add to base to wrap any bulk endpoint into a character device is possible too.

[val_packett.cool]

In D21863#477929, @hselasky wrote:

Likely this Google device is not USB certified

Yeah, it's a debug interface for developers, why would they bother.

The code on the device that complains about the thing is commented as /* Something we need to add support for? */ :D
https://chromium.googlesource.com/chromiumos/platform/ec/+/refs/tags/cr50_v4.5/chip/g/usb.c#835

sys/dev/usb/serial/ugensa.c

217

hmm, uaa->info is usbd_lookup_info, not usb_device_id

[• hselasky]

sys/dev/usb/serial/ugensa.c

217

My point is bInterfaceSubClass=0 is also valid. So you need some kind of additional flag for this, probably via the driver info.

[• hselasky]

In D21863#477983, @greg_unrelenting.technology wrote:

In D21863#477929, @hselasky wrote:

Likely this Google device is not USB certified

Yeah, it's a debug interface for developers, why would they bother.

The code on the device that complains about the thing is commented as /* Something we need to add support for? */ :D
https://chromium.googlesource.com/chromiumos/platform/ec/+/refs/tags/cr50_v4.5/chip/g/usb.c#835

Would you mind poking these guys? Can the firmware for this device be upgraded?

--HPS

[• hselasky]

Ping.

[val_packett.cool]

In D21863#511549, @hselasky wrote:

Ping.

About talking to the firmware developers? I did mention this on IRC, someone told me to report on the chromium bugtracker, I signed up for a google account and tried to sign in to there and something exploded :D

---

Also I have a quirk version of this patch, but some things are commented out. I'll try finishing it.

[val_packett.cool]

The clear stall quirk is not necessary anymore (since cr50 firmware 0.3.24), so let's just recognize the device.

[val_packett.cool]

@hselasky ping

[• hselasky]

sys/dev/usb/serial/ugensa.c

217

Can you add a check like I described above?

[val_packett.cool]

sys/dev/usb/serial/ugensa.c

217

Where? The changes above are gone?? Now there's only the ugensa_devs entry.

[• hselasky]

sys/dev/usb/serial/ugensa.c

217

In this for loop you should add:
if (uaa->info.match_flag_int_subclass &&
<4spaces>iface->idesc->bInterfaceSubClass != uaa->info.bInterfaceSubClass)
<tab>continue;

[phk]

Sorry, despite being the last person to commit to thisfile, I don't think I am qualified to review this patch.

[val_packett.cool]

sys/dev/usb/serial/ugensa.c

217

This results in:

error: no member named 'match_flag_int_subclass' in 'struct usbd_lookup_info'

[• hselasky]

sys/dev/usb/serial/ugensa.c

217

I see.

I'll fix it.