[REBASE & FF]Add USB and HID protocol definitions to SDK.#1498
[REBASE & FF]Add USB and HID protocol definitions to SDK.#1498joschock wants to merge 2 commits into
Conversation
✅ QEMU Validation PassedAll QEMU validation jobs completed successfully.
Workflow run: https://github.com/OpenDevicePartnership/patina/actions/runs/25415455927 Boot Time to EFI Shell
Dependencies
This comment was automatically generated by the Patina QEMU PR Validation Post workflow. |
github-actions
Bot
added
the
impact:non-functional
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
joschock
force-pushed
the
usb_hid_protocol_defs
branch
from
cbbf30c to
666aceb
Compare
joschock
force-pushed
the
usb_hid_protocol_defs
branch
from
666aceb to
44f35c6
Compare
Add the EFI_USB_IO_PROTOCOL interface and associated USB descriptor types as defined in the UEFI Specification, Chapter 17.2.4 (USB I/O Protocol). All protocol functions are fully defined: - UsbControlTransfer - UsbBulkTransfer - UsbAsyncInterruptTransfer - UsbSyncInterruptTransfer - UsbIsochronousTransfer - UsbAsyncIsochronousTransfer - UsbGetDeviceDescriptor - UsbGetConfigDescriptor - UsbGetInterfaceDescriptor - UsbGetEndpointDescriptor - UsbGetStringDescriptor - UsbGetSupportedLanguages - UsbPortReset Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add the HID_IO protocol interface as a vendor-specific protocol definition. This protocol is defined by Project Mu and provides the interface between HID transport drivers (producers) and HID class drivers (consumers). Reference: https://github.com/microsoft/mu_plus/blob/release/202502/HidPkg/Include/Protocol/HidIo.h Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
joschock
force-pushed
the
usb_hid_protocol_defs
branch
from
44f35c6 to
37d6621
Compare
|
@joschock is there a reason not to take the UEFI Specification defined content to r-efi? |
| /// Manages a USB device with a control transfer pipe. A control transfer is | ||
| /// typically used to perform device initialization and configuration. | ||
| pub usb_control_transfer: unsafe extern "efiapi" fn( | ||
| this: *const EfiUsbIoProtocol, |
There was a problem hiding this comment.
With the way this is defined in the UEFI Specification, I don't think we can consider it *const here.
typedef
EFI_STATUS
(EFIAPI *EFI_USB_IO_CONTROL_TRANSFER) (
IN EFI_USB_IO_PROTOCOL *This,
IN EFI_USB_DEVICE_REQUEST *Request,
IN EFI_USB_DATA_DIRECTION Direction,
IN UINT32 Timeout,
IN OUT VOID *Data OPTIONAL,
IN UINTN DataLength OPTIONAL,
OUT UINT32 *Status
);|
|
||
| /// Common header for USB descriptors. | ||
| #[derive(Debug, Clone, Copy)] | ||
| #[repr(C, packed)] |
There was a problem hiding this comment.
Why is this structure (UsbDescHead) packed while the others are not?
There was a problem hiding this comment.
copilot added it and it was an oversight on my part not to make it consistent with the others. In this case, it should not be needed as packed will result in the same as natural alignment for this struct.
| pub struct EfiUsbDeviceDescriptor { | ||
| /// Size of this descriptor in bytes. | ||
| pub length: u8, | ||
| /// Descriptor type (USB_DESC_TYPE_DEVICE). |
There was a problem hiding this comment.
USB_DESC_TYPE_DEVICE is not defined with the other USB descriptor types above (CONFIG and INTERFACE are currrently there). Since it's referenced here, it might be worth including for clarity. Were the other types (1, 3, 5) excluded intentionally?
There was a problem hiding this comment.
initially excluded because they weren't needed for the implementation; but agree they should be added to fully define the protocol
Primarily for ease of prototyping this approach and getting feedback. I agree that it should be upstreamed. |
(definitely also needs to be more comprehensive as part of that) |
os-d
left a comment
os-d
left a comment
There was a problem hiding this comment.
Approving; agree with Michael's review. Can we file an issue to track upstream to r-efi?
makubacki
left a comment
makubacki
left a comment
There was a problem hiding this comment.
Approving with the plan to move USB IO out of this PR and take directly to r-efi (filling out all definitions).
Branch for r_efi here: https://github.com/joschock/r-efi/commits/add_usb_io/, but waiting on #1480 to facilitate proper testing. |
3 participants
Description
Add the EFI_USB_IO_PROTOCOL interface and associated USB descriptor types
as defined in the UEFI Specification, Chapter 17.2.4 (USB I/O Protocol).
Add the HID_IO protocol interface as a vendor-specific protocol definition.
This protocol is defined by Project Mu and provides the interface between
HID transport drivers (producers) and HID class drivers (consumers).
This PR consists primarily of structural definitions of the protocols themselves, and does not include any consuming logic.
How This Was Tested
Tested via work in progress component development that requires these definitions.
Integration Instructions
N/A.