Paths

Table of Contentst

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

Authored by val_packett.cool on Oct 1 2019, 7:38 PM.

Details

Reviewers

• hselasky
phk

Group Reviewers

Contributor Reviews (src)

Commits

rS366877: MFC r366431:
rS366876: MFC r366431:
rS366431: Add support for Google Cr50 (GSC) Closed Case Debugging UART interfaces to

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".

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

val_packett.cool created this revision.Oct 1 2019, 7:38 PM

Herald added a subscriber: imp. · View Herald TranscriptOct 1 2019, 7:38 PM

imp added inline comments.Oct 2 2019, 3:04 AM

sys/dev/usb/serial/ugensa.c
216 ↗	(On Diff #62805)	I'd prefer a better comment here too.
217 ↗	(On Diff #62805)	This needs to be wrapped at 80 columns
244 ↗	(On Diff #62805)	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).

val_packett.cool added a reviewer: • hselasky.Oct 2 2019, 6:22 PM

• hselasky added inline comments.Oct 2 2019, 8:50 PM

sys/dev/usb/serial/ugensa.c
244 ↗	(On Diff #62805)	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 added inline comments.Oct 2 2019, 8:59 PM

sys/dev/usb/serial/ugensa.c
217 ↗	(On Diff #62805)	Code should look like this: if (uaa->info.match_flag_int_subclass && <4spaces>iface->idesc->bInterfaceSubClass != uaa->info.bInterfaceSubClass) <tab>continue;

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.

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 ↗	(On Diff #62805)	hmm, `uaa->info` is `usbd_lookup_info`, not `usb_device_id`

• hselasky added inline comments.Oct 3 2019, 9:24 AM

sys/dev/usb/serial/ugensa.c
217 ↗	(On Diff #62805)	My point is bInterfaceSubClass=0 is also valid. So you need some kind of additional flag for this, probably via the driver info.

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

Ping.

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.

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

@hselasky ping

• hselasky added inline comments.Oct 1 2020, 4:13 PM

sys/dev/usb/serial/ugensa.c
217 ↗	(On Diff #62805)	Can you add a check like I described above?

val_packett.cool added inline comments.Oct 1 2020, 4:22 PM

sys/dev/usb/serial/ugensa.c
217 ↗	(On Diff #62805)	Where? The changes above are gone?? Now there's only the `ugensa_devs` entry.

• hselasky added inline comments.Oct 1 2020, 4:34 PM

sys/dev/usb/serial/ugensa.c
217 ↗	(On Diff #62805)	In this for loop you should add: if (uaa->info.match_flag_int_subclass && <4spaces>iface->idesc->bInterfaceSubClass != uaa->info.bInterfaceSubClass) <tab>continue;