Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

usb session renewal: fully asynchronous & interface level access #5021

Closed
skalk opened this issue Oct 11, 2023 · 17 comments
Closed

usb session renewal: fully asynchronous & interface level access #5021

skalk opened this issue Oct 11, 2023 · 17 comments
Labels

Comments

@skalk
Copy link
Member

skalk commented Oct 11, 2023

Within the past years we've observed certain limitations of the adhoc developed original USB session RPC API. First and foremost there were still several synchronous RPC calls in place to e.g. ask for USB descriptors. The problem here is the server side - an USB multiplexer, which interacts with multiple clients and serves different requests at once should not block during individual RPC calls. The other limitation regards USB devices with multiple interfaces of different classes, e.g. an USB headset with audio interfaces and HID interface to realize volume, play/stop buttons. Here, we would like to give access to the different interfaces to different USB clients resp. drivers.

The new design is inspired by the platform session API. An USB session now only provides a ROM dataspace, which reports all devices this USB session has access to. The USB device information includes its interfaces and endpoint descriptions. Therefore, an USB client does not necessarily need to access different USB descriptors during initialiyation, but can extract most information by the XML representation of its devices ROM. Morover, the USB session itself only provides two RPC calls to acquire and release an USB device. The acquire call returns a capability to a new USB device session.

The device session includes RPC calls to register a signal context and request a dataspace. Both together form a packet stream, wereby packets here are always interpreted as USB control transfers. If a client has full control over the device, it can send any form of control transfer here. If the client has only limited access to a certain interface, it can only send control transfers to get information over the device, and to set an alternate interface regarding the interface it has access to. Moreover, the device session includes two RPC call to acquire or release an interface of the device. The acquire call returns a capability to a new USB interface session.

The interface session again includes RPC calls to register signal context and retrieve a dataspace for its packet stream access. The packets of this stream represent either interrupt, bulk, or isochronous transfers to an endpoint of this interface, or a flush command to cancel all ongoing transfers regarding this interface.

Beside the pure USB session renewal, a client API for device and interface access is envisioned that loosely corresponds to the existent block job API. Thereby, we hide the ancient packet stream API from developers, and enforce that signaling is done on a less eagerly base to increase throughput.

skalk added a commit to skalk/genode that referenced this issue Oct 11, 2023
skalk added a commit to skalk/genode that referenced this issue Oct 11, 2023
@skalk
Copy link
Member Author

skalk commented Oct 11, 2023

Commit 7b05ce4 shows the current state of the new USB session & client API. It moreover adapts the Genode C-API for USB server side, and the implementation of the DDE Linux USB host controller driver.
Commit 8278075 adapts the USB block driver to use te new USB client API. The whole initialization is now done asynchronously in form of a state-machine. I've marked both commits as work-in-progress, because not all files that got deprecated were removed, and several other USB clients are not adapted yet. Also some places might need some explanations. Anyway, I've pused the status quo, so that you can get a first impression and leave comments before all other clients get adapted.

@chelmuth
Copy link
Member

chelmuth commented Oct 24, 2023

Today I took the time to dive into 7b05ce4 and very much enjoyed the ride. I'm impressed by the clearness of the (hierarchical) session interfaces for USB bus, devices, and interfaces as well as the C API. It's has become quite tangible how the implementation will evolve and solves the issues stated above.

Nevertheless, I have a small collection of questions and remarks.

GENERAL Can usb_session/urb_handler.h be removed? The file does not contain any code.
GENERAL Do you plan to deprecate usb/usb.h and usb/packet_handler.h entirely or may we lose essential functionality?
GENERAL I expect genode_c_api/usb_client.cc will be added next?
usb/types.h I suggest to move the file to usb_session/ or do you have other plans?
usb/types.h I suggest to replace genode_usb_isoc_transfer_header::data_offset by a calculation.
usb_session/usb_session.h The recipients PORT and RPIPE should be removed as Wireless USB is no more. Promise! It was removed from the Linux kernel before 6.0.
usb_session/device.h The use of read and write in Update_urbs_policy feels strange. How about renaming the methods to produce_out_content and consume_in_content analogous to the USB specification for IN and OUT endpoints?
usb_session/device.h Usb::Interface has virtual destructor but is not used as a base class anywhere. If this is not planned, I suggest to remove virtual.
usb_session/device.h _try_process_ack and _try_submit_pending_urb mention split read/write operations. What are these operations and why do they complicate the packet-stream handling?
genode_c_api/usb.h,usb.cc What's the rationale behind the removal of struct genode_usb_buffer payload? I changed this with Genode::Byte_range_ptr in the back of my head.
genode_c_api/usb.h,usb.cc I suggest to unify void * data, void *callback_data, void *opaque_data, and void *opaque_callback_data.
genode_c_api/usb.cc,lx_emul/usb.c You removed class-based access policies incl. interface-to-device class quirk. How is the plan to address this use case in the future?

@skalk
Copy link
Member Author

skalk commented Oct 25, 2023

Thanks @chelmuth for your valueable review!

Nevertheless, I have a small collection of questions and remarks.

GENERAL Can usb_session/urb_handler.h be removed? The file does not contain any code.

Sure, it is a development artefact.

GENERAL Do you plan to deprecate usb/usb.h and usb/packet_handler.h entirely or may we lose essential functionality?

Yes

GENERAL I expect genode_c_api/usb_client.cc will be added next?

Yes

usb/types.h I suggest to move the file to usb_session/ or do you have other plans?

I agree.

usb/types.h I suggest to replace genode_usb_isoc_transfer_header::data_offset by a calculation.

Thanks for the pointer, I'll have a look into this.

usb_session/usb_session.h The recipients PORT and RPIPE should be removed as Wireless USB is no more. Promise! It was removed from the Linux kernel before 6.0.

Okay.

usb_session/device.h The use of read and write in Update_urbs_policy feels strange. How about renaming the methods to produce_out_content and consume_in_content analogous to the USB specification for IN and OUT endpoints?

Yes, this was probably too much influenced by the Block Job API.

usb_session/device.h Usb::Interface has virtual destructor but is not used as a base class anywhere. If this is not planned, I suggest to remove virtual.

I have to proof this.

usb_session/device.h _try_process_ack and _try_submit_pending_urb mention split read/write operations. What are these operations and why do they complicate the packet-stream handling?

Good point! Yet another artefact from the Job API. In the first place, I tried to keep it as similar as possible to potentially extract a common pattern. But during development it was no realistic option anymore.

genode_c_api/usb.h,usb.cc What's the rationale behind the removal of struct genode_usb_buffer payload? I changed this with Genode::Byte_range_ptr in the back of my head.

There is no good rationale behind it. I was trying to simplify the callbacks, and during different iterations, I assume` it got removed without need. I can re-insert it.

genode_c_api/usb.h,usb.cc I suggest to unify void * data, void *callback_data, void *opaque_data, and void *opaque_callback_data.

Okay.

genode_c_api/usb.cc,lx_emul/usb.c You removed class-based access policies incl. interface-to-device class quirk. How is the plan to address this use case in the future?

We had an offline discussion about this in preparation of this issue's re-design ambition, and my understanding of the outcome was that we only support per-device (named), and at maximum per-vendor-product (for development run-scripts) assigments, because the current approach is too inflexible.
If in contrast to the discussion result, as I've understood it, a class-based approach is still desired, we have to distinguish in between different interfaces now anyway. Overwriting the class of a device as the current quirk, would not be an option anymore.

@chelmuth
Copy link
Member

genode_c_api/usb.cc,lx_emul/usb.c You removed class-based access policies incl. interface-to-device class quirk. How is the plan to address this use case in the future?

We had an offline discussion about this in preparation of this issue's re-design ambition, and my understanding of the outcome was that we only support per-device (named), and at maximum per-vendor-product (for development run-scripts) assignments, because the current approach is too inflexible. If in contrast to the discussion result, as I've understood it, a class-based approach is still desired, we have to distinguish in between different interfaces now anyway. Overwriting the class of a device as the current quirk, would not be an option anymore.

Don't get me wrong, I could remember our discussion but also that we had no concrete solution for Sculpt (or a USB HID test that supports not yet known mice). Thus, I added this open question to the list as we have to find a solution before a final merge.

skalk added a commit to skalk/genode that referenced this issue Oct 25, 2023
skalk added a commit to skalk/genode that referenced this issue Oct 25, 2023
skalk added a commit to skalk/genode that referenced this issue Oct 25, 2023
skalk added a commit to skalk/genode that referenced this issue Oct 25, 2023
@skalk
Copy link
Member Author

skalk commented Oct 25, 2023

@chelmuth I've realized all your comments within two additional fixup commits, and rebased it to staging. It is still work-in-progress, but so you can have a separate look at the changes, if you like to.
The only open discussion point is the last one. For now, I would proceed with the work on the Genode C API USB client part.

skalk added a commit to skalk/genode that referenced this issue Jan 4, 2024
skalk added a commit to skalk/genode that referenced this issue Jan 4, 2024
skalk added a commit to skalk/genode that referenced this issue Jan 17, 2024
skalk added a commit to skalk/genode that referenced this issue Jan 17, 2024
skalk added a commit to skalk/genode that referenced this issue Jan 30, 2024
skalk added a commit to skalk/genode that referenced this issue Jan 30, 2024
skalk added a commit to skalk/genode that referenced this issue Feb 20, 2024
skalk added a commit to skalk/genode that referenced this issue Feb 20, 2024
skalk added a commit to skalk/genode that referenced this issue Feb 20, 2024
skalk added a commit to skalk/genode that referenced this issue Feb 20, 2024
skalk added a commit to skalk/genode that referenced this issue Feb 20, 2024
skalk added a commit to skalk/genode that referenced this issue Feb 20, 2024
skalk added a commit to skalk/genode that referenced this issue Feb 21, 2024
skalk added a commit to skalk/genode that referenced this issue Feb 21, 2024
Newer Qemu variants quit with an error about already existing devices
if the same device-id is add and removed in a loop fast. To circumvent
this strange behaviour, simply use consecutive device id numbers.

Ref genodelabs#5021
skalk added a commit to skalk/genode that referenced this issue Feb 21, 2024
skalk added a commit to skalk/genode that referenced this issue Feb 21, 2024
skalk added a commit to skalk/genode that referenced this issue Feb 21, 2024
skalk added a commit to skalk/genode that referenced this issue Feb 21, 2024
skalk added a commit to skalk/genode that referenced this issue Feb 21, 2024
skalk added a commit to skalk/genode that referenced this issue Mar 7, 2024
Replace the USB session API by one that provides a devices ROM only,
which contains information about all USB devices available for this client,
as well as methods to acquire and release a single device.

The acquisition of an USB device returns the capability to a device session
that includes a packet stream buffer to communicate control transfers
in between the client and the USB host controller driver. Moreover,
additional methods to acquire and release an USB interface can be used.

The acquisition of an USB interface returns the capability to an interface
session that includes a packet stream buffer to communicate either
bulk, interrupt, or isochronous transfers in between the client and the
USB host controller driver.

This commit implements the API changes in behalf of the Genode C API's
USB server and client side. Addtionally, it provides Usb::Device,
Usb::Interface, and Usb::Endpoint utilities that can be used by native
C++ clients to use the new API and hide the sophisticated packet stream API.

The adaptations necessary target the following areas:

* lx_emul layer for USB host and client side
* Linux USB host controller driver port for PC
* Linux USB client ports: usb_hid_drv and usb_net_drv, additionally
  reduce the Linux tasks used inside these drivers
* Native usb_block_drv
* black_hole component
* Port of libusb, including smartcard and usb_webcam driver depending on it
* Port of Qemu XHCI model library, including vbox5 & vbox6 depending on it
* Adapt all run-scripts and drivers_interactive recipes to work
  with the new policy rules of the USB host controller driver

Fix genodelabs#5021
skalk added a commit to skalk/genode that referenced this issue Mar 11, 2024
Replace the USB session API by one that provides a devices ROM only,
which contains information about all USB devices available for this client,
as well as methods to acquire and release a single device.

The acquisition of an USB device returns the capability to a device session
that includes a packet stream buffer to communicate control transfers
in between the client and the USB host controller driver. Moreover,
additional methods to acquire and release an USB interface can be used.

The acquisition of an USB interface returns the capability to an interface
session that includes a packet stream buffer to communicate either
bulk, interrupt, or isochronous transfers in between the client and the
USB host controller driver.

This commit implements the API changes in behalf of the Genode C API's
USB server and client side. Addtionally, it provides Usb::Device,
Usb::Interface, and Usb::Endpoint utilities that can be used by native
C++ clients to use the new API and hide the sophisticated packet stream API.

The adaptations necessary target the following areas:

* lx_emul layer for USB host and client side
* Linux USB host controller driver port for PC
* Linux USB client ports: usb_hid_drv and usb_net_drv, additionally
  reduce the Linux tasks used inside these drivers
* Native usb_block_drv
* black_hole component
* Port of libusb, including smartcard and usb_webcam driver depending on it
* Port of Qemu XHCI model library, including vbox5 & vbox6 depending on it
* Adapt all run-scripts and drivers_interactive recipes to work
  with the new policy rules of the USB host controller driver

Fix genodelabs#5021
skalk added a commit to skalk/genode that referenced this issue Mar 12, 2024
Replace the USB session API by one that provides a devices ROM only,
which contains information about all USB devices available for this client,
as well as methods to acquire and release a single device.

The acquisition of an USB device returns the capability to a device session
that includes a packet stream buffer to communicate control transfers
in between the client and the USB host controller driver. Moreover,
additional methods to acquire and release an USB interface can be used.

The acquisition of an USB interface returns the capability to an interface
session that includes a packet stream buffer to communicate either
bulk, interrupt, or isochronous transfers in between the client and the
USB host controller driver.

This commit implements the API changes in behalf of the Genode C API's
USB server and client side. Addtionally, it provides Usb::Device,
Usb::Interface, and Usb::Endpoint utilities that can be used by native
C++ clients to use the new API and hide the sophisticated packet stream API.

The adaptations necessary target the following areas:

* lx_emul layer for USB host and client side
* Linux USB host controller driver port for PC
* Linux USB client ports: usb_hid_drv and usb_net_drv, additionally
  reduce the Linux tasks used inside these drivers
* Native usb_block_drv
* black_hole component
* Port of libusb, including smartcard and usb_webcam driver depending on it
* Port of Qemu XHCI model library, including vbox5 & vbox6 depending on it
* Adapt all run-scripts and drivers_interactive recipes to work
  with the new policy rules of the USB host controller driver

Fix genodelabs#5021
skalk added a commit to skalk/genode that referenced this issue Mar 12, 2024
Replace the USB session API by one that provides a devices ROM only,
which contains information about all USB devices available for this client,
as well as methods to acquire and release a single device.

The acquisition of an USB device returns the capability to a device session
that includes a packet stream buffer to communicate control transfers
in between the client and the USB host controller driver. Moreover,
additional methods to acquire and release an USB interface can be used.

The acquisition of an USB interface returns the capability to an interface
session that includes a packet stream buffer to communicate either
bulk, interrupt, or isochronous transfers in between the client and the
USB host controller driver.

This commit implements the API changes in behalf of the Genode C API's
USB server and client side. Addtionally, it provides Usb::Device,
Usb::Interface, and Usb::Endpoint utilities that can be used by native
C++ clients to use the new API and hide the sophisticated packet stream API.

The adaptations necessary target the following areas:

* lx_emul layer for USB host and client side
* Linux USB host controller driver port for PC
* Linux USB client ports: usb_hid_drv and usb_net_drv, additionally
  reduce the Linux tasks used inside these drivers
* Native usb_block_drv
* black_hole component
* Port of libusb, including smartcard and usb_webcam driver depending on it
* Port of Qemu XHCI model library, including vbox5 & vbox6 depending on it
* Adapt all run-scripts and drivers_interactive recipes to work
  with the new policy rules of the USB host controller driver

Fix genodelabs#5021
skalk added a commit to skalk/genode that referenced this issue Mar 13, 2024
Replace the USB session API by one that provides a devices ROM only,
which contains information about all USB devices available for this client,
as well as methods to acquire and release a single device.

The acquisition of an USB device returns the capability to a device session
that includes a packet stream buffer to communicate control transfers
in between the client and the USB host controller driver. Moreover,
additional methods to acquire and release an USB interface can be used.

The acquisition of an USB interface returns the capability to an interface
session that includes a packet stream buffer to communicate either
bulk, interrupt, or isochronous transfers in between the client and the
USB host controller driver.

This commit implements the API changes in behalf of the Genode C API's
USB server and client side. Addtionally, it provides Usb::Device,
Usb::Interface, and Usb::Endpoint utilities that can be used by native
C++ clients to use the new API and hide the sophisticated packet stream API.

The adaptations necessary target the following areas:

* lx_emul layer for USB host and client side
* Linux USB host controller driver port for PC
* Linux USB client ports: usb_hid_drv and usb_net_drv, additionally
  reduce the Linux tasks used inside these drivers
* Native usb_block_drv
* black_hole component
* Port of libusb, including smartcard and usb_webcam driver depending on it
* Port of Qemu XHCI model library, including vbox5 & vbox6 depending on it
* Adapt all run-scripts and drivers_interactive recipes to work
  with the new policy rules of the USB host controller driver

Fix genodelabs#5021
skalk added a commit to skalk/genode that referenced this issue Mar 13, 2024
Replace the USB session API by one that provides a devices ROM only,
which contains information about all USB devices available for this client,
as well as methods to acquire and release a single device.

The acquisition of an USB device returns the capability to a device session
that includes a packet stream buffer to communicate control transfers
in between the client and the USB host controller driver. Moreover,
additional methods to acquire and release an USB interface can be used.

The acquisition of an USB interface returns the capability to an interface
session that includes a packet stream buffer to communicate either
bulk, interrupt, or isochronous transfers in between the client and the
USB host controller driver.

This commit implements the API changes in behalf of the Genode C API's
USB server and client side. Addtionally, it provides Usb::Device,
Usb::Interface, and Usb::Endpoint utilities that can be used by native
C++ clients to use the new API and hide the sophisticated packet stream API.

The adaptations necessary target the following areas:

* lx_emul layer for USB host and client side
* Linux USB host controller driver port for PC
* Linux USB client ports: usb_hid_drv and usb_net_drv, additionally
  reduce the Linux tasks used inside these drivers
* Native usb_block_drv
* black_hole component
* Port of libusb, including smartcard and usb_webcam driver depending on it
* Port of Qemu XHCI model library, including vbox5 & vbox6 depending on it
* Adapt all run-scripts and drivers_interactive recipes to work
  with the new policy rules of the USB host controller driver

Fix genodelabs#5021
@skalk skalk added the fixed label Mar 13, 2024
@skalk
Copy link
Member Author

skalk commented Mar 13, 2024

@chelmuth commit 6c44bb5 shall accomplish this line of work.

skalk added a commit to genodelabs/genode-allwinner that referenced this issue Mar 13, 2024
skalk added a commit to genodelabs/genode-imx that referenced this issue Mar 13, 2024
skalk added a commit to genodelabs/genode-imx that referenced this issue Mar 13, 2024
skalk added a commit to genodelabs/genode-rpi that referenced this issue Mar 13, 2024
skalk added a commit to genodelabs/genode-rpi that referenced this issue Mar 13, 2024
chelmuth added a commit that referenced this issue Mar 20, 2024
@chelmuth
Copy link
Member

chelmuth commented Mar 20, 2024

Commit 451a726 implements reporting of device manufacturer and product strings as well as interface description string. Output like follows.

<device name="usb-1-2" class="0x0" manufacturer="Logitech" product="USB Laser Mouse" vendor_id="0x46d" product_id="0xc062" speed="low" acquired="true">
  ...
</device>
<device name="usb-1-7" class="0x0" manufacturer="Apple Inc." product="Magic Trackpad" vendor_id="0x5ac" product_id="0x265" speed="full">
  <config active="true" value="0x1">
    <interface active="true" number="0x0" info="Device Management" alt_setting="0x0" class="0x3" subclass="0x0" protocol="0x0">
      <endpoint address="0x81" attributes="0x3" max_packet_size="0x10"/>
    </interface>
    <interface active="true" number="0x1" info="Trackpad / Boot" alt_setting="0x0" class="0x3" subclass="0x1" protocol="0x2">
      <endpoint address="0x83" attributes="0x3" max_packet_size="0x40"/>
    </interface>
    <interface active="true" number="0x2" info="Actuator" alt_setting="0x0" class="0x3" subclass="0x0" protocol="0x0">
      <endpoint address="0x84" attributes="0x3" max_packet_size="0x10"/>
      <endpoint address="0x4" attributes="0x3" max_packet_size="0x40"/>
    </interface>
    <interface active="true" number="0x3" info="Accelerometer" alt_setting="0x0" class="0x3" subclass="0x0" protocol="0x0">
      <endpoint address="0x85" attributes="0x3" max_packet_size="0x40"/>
    </interface>
  </config>
</device>

@skalk
Copy link
Member Author

skalk commented Mar 20, 2024

@chelmuth a feature, which is very welcome!

alex-ab added a commit to alex-ab/genode that referenced this issue Mar 20, 2024
chelmuth pushed a commit that referenced this issue Mar 20, 2024
nfeske added a commit to genodelabs/genode-allwinner that referenced this issue Mar 21, 2024
nfeske added a commit to nfeske/genode-allwinner that referenced this issue Mar 25, 2024
skalk added a commit to skalk/genode that referenced this issue Apr 16, 2024
Instead of returning an invalid capability whenever an interface is
requested that does not exist, create a disconnected interface component.
It is also possible that a client requests an interface that got removed
at the same time. When an invalid capability gets returned, a client
can stumble about invoking it.
Moreover, this commit marks either invalid interface or device components
as disconnected objects to optimize their handling.

Ref genodelabs#5021
chelmuth pushed a commit that referenced this issue Apr 16, 2024
Instead of returning an invalid capability whenever an interface is
requested that does not exist, create a disconnected interface component.
It is also possible that a client requests an interface that got removed
at the same time. When an invalid capability gets returned, a client
can stumble about invoking it.
Moreover, this commit marks either invalid interface or device components
as disconnected objects to optimize their handling.

Ref #5021
chelmuth pushed a commit that referenced this issue Apr 19, 2024
Instead of returning an invalid capability whenever an interface is
requested that does not exist, create a disconnected interface component.
It is also possible that a client requests an interface that got removed
at the same time. When an invalid capability gets returned, a client
can stumble about invoking it.
Moreover, this commit marks either invalid interface or device components
as disconnected objects to optimize their handling.

Ref #5021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants