On Sat, Dec 31, 2022 at 8:47 PM Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote: > > On Fri, Dec 30, 2022 at 09:11:43PM +0100, Jó Ágila Bitsch wrote: > > There is a custom (non-USB IF) extension to the USB standard: > > > > https://wicg.github.io/webusb/ > > > > This specification is published under the W3C Community Contributor > > Agreement, which in particular allows to implement the specification > > without any royalties. > > > > The specification allows USB gadgets to announce an URL to landing > > page and describes a Javascript interface for websites to interact > > with the USB gadget, if the user allows it. It is currently > > supported by Chromium-based browsers, such as Chrome, Edge and > > Opera on all major operating systems including Linux. > > > > This patch adds optional support for Linux-based USB gadgets > > wishing to expose such a landing page. > > > > During device enumeration, a host recognizes that the announced > > USB version is at least 2.01, which means, that there are BOS > > Where is this 2.01 value specified? I don't remember seeing it in the > usual USBIF documents. This is actually from the backport of BOS descriptors to USB 2 Citing Randy Aull from https://community.osr.com/discussion/comment/249742/#Comment_249742 > There is no specification called USB 2.1, however there is such a thing as a USB 2.1 device. > The USB 2.0 ECN for LPM introduced a new descriptor request to the enumeration process > for USB 2 devices (the BOS descriptor set). The problem is that software can't request a new > descriptor type to old devices that don't understand it without introducing compatibility issues > with some devices (more than you would probably expect). So, software needed a way to > identify whether a device could support the host requesting a BOS descriptor set. The solution > in this ECN was to require devices supporting the ECN to set their bcdUSB to 0x0201 (2.01). > > Now, when we created the USB 3.0 spec, we again wanted to leverage the BOS descriptor, not > only because we wanted to mandate USB 2 LPM in 3.0 devices when operating at high-speed, > but also so the device can indicate to a host that it can operate at SuperSpeed (to support > everyone's favorite "your device can perform faster" popup). Knowing that when operating at > high-speed these devices needed to report the BOS descriptor set, we knew that it couldn't > set the bcdUSB to 0x0200. We mistakenly wrote it down as 0x0210 instead of 0x0201 in the > 3.0 spec (perhaps we just said "two point one" which might have been ambiguous) when we > were trying to just be consistent with the requirement from the LPM ECN. So, rather than > changing it back to 0x0201 in the USB 3.0 spec, we just ran with it. > > So, there are USB 2.0 devices, USB 2.01 devices and USB 2.1 devices, even though the latest > revision of the USB 2 spec is USB 2.0. I have recommended that the USB-IF actually create a > USB 2.1 specification that captures all of the various errata, ECNs, etc from over the years, but > it hasn't happened yet. Btw, configfs already includes these version codes to support Link Power Management (LPM) and the associated BOS descriptor, so I didn't add that part, I only added webusb as a condition as to when to send BOS descriptors. > > > descriptors available. The device than announces WebUSB support > > using a platform device capability. This includes a vendor code > > under which the landing page URL can be retrieved using a > > vendor-specific request. > > > > Usage is modeled after os_desc descriptors: > > echo 1 > webusb/use > > echo "https://www.kernel.org"; > webusb/landingPage > > > > lsusb will report the device with the following lines: > > Platform Device Capability: > > bLength 24 > > bDescriptorType 16 > > bDevCapabilityType 5 > > bReserved 0 > > PlatformCapabilityUUID {3408b638-09a9-47a0-8bfd-a0768815b665} > > WebUSB: > > bcdVersion 1.00 > > bVendorCode 0 > > iLandingPage 1 https://www.kernel.org > > > > Signed-off-by: Jó Ágila Bitsch <jgilab@xxxxxxxxx> > > --- > > Documentation/ABI/testing/configfs-usb-gadget | 13 ++ > > drivers/usb/gadget/composite.c | 102 ++++++++++-- > > drivers/usb/gadget/configfs.c | 145 ++++++++++++++++++ > > include/linux/usb/composite.h | 6 + > > include/uapi/linux/usb/ch9.h | 33 ++++ > > 5 files changed, 289 insertions(+), 10 deletions(-) > > > > diff --git a/Documentation/ABI/testing/configfs-usb-gadget b/Documentation/ABI/testing/configfs-usb-gadget > > index b7943aa7e997..8399e0f0ed1c 100644 > > --- a/Documentation/ABI/testing/configfs-usb-gadget > > +++ b/Documentation/ABI/testing/configfs-usb-gadget > > @@ -143,3 +143,16 @@ Description: > > qw_sign an identifier to be reported as "OS String" > > proper > > ============= =============================================== > > + > > +What: /config/usb-gadget/gadget/webusb > > +Date: Dec 2022 > > +KernelVersion: 6.2 > > +Description: > > + This group contains "WebUSB" extension handling attributes. > > + > > + ============= =============================================== > > + use flag turning "WebUSB" support on/off > > + bcdVersion bcd WebUSB specification version number > > + b_vendor_code one-byte value used for custom per-device > > + landing_page UTF-8 encoded URL of the device's landing page > > + ============= =============================================== > > diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c > > index 403563c06477..937695dc5366 100644 > > --- a/drivers/usb/gadget/composite.c > > +++ b/drivers/usb/gadget/composite.c > > @@ -14,6 +14,7 @@ > > #include <linux/device.h> > > #include <linux/utsname.h> > > #include <linux/bitfield.h> > > +#include <linux/uuid.h> > > > > #include <linux/usb/composite.h> > > #include <linux/usb/otg.h> > > @@ -713,14 +714,16 @@ static int bos_desc(struct usb_composite_dev *cdev) > > * A SuperSpeed device shall include the USB2.0 extension descriptor > > * and shall support LPM when operating in USB2.0 HS mode. > > */ > > - usb_ext = cdev->req->buf + le16_to_cpu(bos->wTotalLength); > > - bos->bNumDeviceCaps++; > > - le16_add_cpu(&bos->wTotalLength, USB_DT_USB_EXT_CAP_SIZE); > > - usb_ext->bLength = USB_DT_USB_EXT_CAP_SIZE; > > - usb_ext->bDescriptorType = USB_DT_DEVICE_CAPABILITY; > > - usb_ext->bDevCapabilityType = USB_CAP_TYPE_EXT; > > - usb_ext->bmAttributes = cpu_to_le32(USB_LPM_SUPPORT | > > - USB_BESL_SUPPORT | besl); > > + if (cdev->gadget->lpm_capable) { > > This change doesn't seem to be related to the purpose of this patch. Actually, it is. Previously, BOS descriptors would only ever be sent if the device was lpm_capable. For this reason, this descriptor was previously sent unconditionally. With my patch in place, it could be that a device is not lpm_capable, but still wants to send BOS descriptors to announce its WebUSB capability, therefore I added this condition. > > > + usb_ext = cdev->req->buf + le16_to_cpu(bos->wTotalLength); > > + bos->bNumDeviceCaps++; > > + le16_add_cpu(&bos->wTotalLength, USB_DT_USB_EXT_CAP_SIZE); > > + usb_ext->bLength = USB_DT_USB_EXT_CAP_SIZE; > > + usb_ext->bDescriptorType = USB_DT_DEVICE_CAPABILITY; > > + usb_ext->bDevCapabilityType = USB_CAP_TYPE_EXT; > > + usb_ext->bmAttributes = cpu_to_le32(USB_LPM_SUPPORT | > > + USB_BESL_SUPPORT | besl); > > + } > > > > /* > > * The Superspeed USB Capability descriptor shall be implemented by all > > @@ -821,6 +824,41 @@ static int bos_desc(struct usb_composite_dev *cdev) > > } > > } > > > > + /* > > + * Following the specifaction at: > > + * https://wicg.github.io/webusb/#webusb-platform-capability-descriptor > > + */ > > + if (cdev->use_webusb) { > > + struct usb_webusb_cap_descriptor *webusb_cap; > > + /* > > + * little endian PlatformCapablityUUID for WebUSB: > > + * 3408b638-09a9-47a0-8bfd-a0768815b665 > > + */ > > +#define WEBUSB_UUID UUID_INIT(0x38b60834, 0xa909, 0xa047, \ > > + 0x8b, 0xfd, 0xa0, 0x76, 0x88, 0x15, 0xb6, 0x65) > > + uuid_t webusb_uuid = WEBUSB_UUID; > > This #define seems to be pointless. It is used nowhere but in the > immediately following line. You might as well eliminate it. > > Or you might define this UUID at the same place in the header file > where you define struct usb_webusb_cap_descriptor. I will update the patch accordingly. > > > + > > + webusb_cap = cdev->req->buf + le16_to_cpu(bos->wTotalLength); > > + bos->bNumDeviceCaps++; > > + > > + le16_add_cpu(&bos->wTotalLength, USB_DT_WEBUSB_SIZE); > > + webusb_cap->bLength = USB_DT_WEBUSB_SIZE; > > + webusb_cap->bDescriptorType = USB_DT_DEVICE_CAPABILITY; > > + webusb_cap->bDevCapabilityType = USB_PLAT_DEV_CAP_TYPE; > > + webusb_cap->bReserved = 0; > > + export_uuid(webusb_cap->UUID, &webusb_uuid); > > + if (cdev->bcd_webusb_version != 0) > > + webusb_cap->bcdVersion = cpu_to_le16(cdev->bcd_webusb_version); > > + else > > + webusb_cap->bcdVersion = cpu_to_le16(0x0100); > > + > > + webusb_cap->bVendorCode = cdev->b_webusb_vendor_code; > > + if (strnlen(cdev->landing_page, sizeof(cdev->landing_page)) > 0) > > + webusb_cap->iLandingPage = 1; > > + else > > + webusb_cap->iLandingPage = 0; > > + } > > + > > return le16_to_cpu(bos->wTotalLength); > > } > > > > @@ -1744,7 +1782,7 @@ composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl) > > cdev->desc.bcdUSB = cpu_to_le16(0x0210); > > } > > } else { > > - if (gadget->lpm_capable) > > + if (gadget->lpm_capable || cdev->use_webusb) Btw: this is the location where the USB version 2.01 and 2.1 were already in the code to support LPM. > > cdev->desc.bcdUSB = cpu_to_le16(0x0201); > > else > > cdev->desc.bcdUSB = cpu_to_le16(0x0200); > > @@ -1779,7 +1817,7 @@ composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl) > > break; > > case USB_DT_BOS: > > if (gadget_is_superspeed(gadget) || > > - gadget->lpm_capable) { > > + gadget->lpm_capable || cdev->use_webusb) { And this is the location that the BOS descriptors are sent, if the device supports webusb. Previously the code would only send these descriptors when the device was lpm_capable or superspeed. Thus requiring the additional condition above in bos_desc. > > value = bos_desc(cdev); > > value = min(w_length, (u16) value); > > } > > @@ -2013,6 +2051,50 @@ composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl) > > goto check_value; > > } > > > > + /* > > + * webusb descriptor handling, following: > > + * https://wicg.github.io/webusb/#device-requests > > + */ > > + #define WEBUSB_GET_URL 2 > > A similar comment applies to this #define. In this case it might make > sense to put all the WebUSB-related #defines in an appropriate header > file. Agreed. I'll add a comment instead. > > > + if (cdev->use_webusb && > > + (ctrl->bRequestType & USB_TYPE_VENDOR) && > > + ctrl->wIndex == WEBUSB_GET_URL && > > + ctrl->bRequest == cdev->b_webusb_vendor_code) { > > Bad indentation. Visually this makes it look like the last two tests > are part of the conditional block, because they share its indentation > level. I'll fix this. > > > + /* > > + * specification of the url descriptor in WebUSB: > > + * https://wicg.github.io/webusb/#webusb-descriptors > > + */ > > + u8 *buf; > > + unsigned int schema_http; > > *buf and schema_http should be indented by the same amount. I'll fix this. I had accidentally set my tab indentation to 4 spaces. > > > + unsigned int schema_https; > > + unsigned int landing_page_length; > > + > > + buf = cdev->req->buf; > > + #define WEBUSB_URL 3 > > + buf[1] = WEBUSB_URL; > > + > > + landing_page_length = strnlen(cdev->landing_page, 252); > > + schema_https = (strncmp("https://";, cdev->landing_page, 8) == 0); > > + schema_http = (strncmp("http://";, cdev->landing_page, 7) == 0); > > Do you really need these temporary variables? Why not put the tests > directly into the "if" conditions? Good point. > Also, should the comparisons be case-insensitive? As URI schemes are case-insensitive as per https://www.rfc-editor.org/rfc/rfc3986#section-3.1 you are right, so I will change this. > > > + if (schema_https) { > > + buf[2] = 0x01; > > + memcpy(buf+3, cdev->landing_page+8, landing_page_length-8); > > + buf[0] = landing_page_length - 8 + 3; > > + } else if (schema_http) { > > + buf[2] = 0x00; > > + memcpy(buf+3, cdev->landing_page+7, landing_page_length-7); > > + buf[0] = landing_page_length - 7 + 3; > > + } else { > > + buf[2] = 0xFF; > > + memcpy(buf+3, cdev->landing_page, landing_page_length); > > + buf[0] = landing_page_length + 3; > > + } > > + > > + value = buf[0]; > > + > > + goto check_value; > > + } > > + > > VDBG(cdev, > > "non-core control req%02x.%02x v%04x i%04x l%d\n", > > ctrl->bRequestType, ctrl->bRequest, > > diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c > > index 96121d1c8df4..da49b36f7033 100644 > > --- a/drivers/usb/gadget/configfs.c > > +++ b/drivers/usb/gadget/configfs.c > > @@ -39,6 +39,7 @@ struct gadget_info { > > struct config_group configs_group; > > struct config_group strings_group; > > struct config_group os_desc_group; > > + struct config_group webusb_group; > > > > struct mutex lock; > > struct usb_gadget_strings *gstrings[MAX_USB_STRING_LANGS + 1]; > > @@ -50,6 +51,11 @@ struct gadget_info { > > bool use_os_desc; > > char b_vendor_code; > > char qw_sign[OS_STRING_QW_SIGN_LEN]; > > + bool use_webusb; > > + u16 bcd_webusb_version; > > + u8 b_webusb_vendor_code; > > Again, improper indentation. I'll fix this. > > + char landing_page[255]; > > + > > spinlock_t spinlock; > > bool unbind; > > }; > > ... I'll fix this. > > > diff --git a/include/linux/usb/composite.h b/include/linux/usb/composite.h > > index 43ac3fa760db..44f90c37dda9 100644 > > --- a/include/linux/usb/composite.h > > +++ b/include/linux/usb/composite.h > > @@ -474,6 +474,12 @@ struct usb_composite_dev { > > struct usb_configuration *os_desc_config; > > unsigned int use_os_string:1; > > > > + /* WebUSB */ > > + u16 bcd_webusb_version; > > + u8 b_webusb_vendor_code; > > + char landing_page[255]; > > Improper indentation. I'll fix this. > > > + unsigned int use_webusb:1; > > + > > /* private: */ > > /* internals */ > > unsigned int suspended:1; > > Alan Stern Thank you for your review! I will post an updated version of the patch soon. Jó
