Add HID support via Patina components

[joschock]

Description

This is a significant rewrite of the UEFI HID driver from Mu HidPkg re-implemented as a Patina component.

This PR introduces two new component crates:

• uefi_hid - implements the UEFI driver binding model to produce UEFI console input protocols (SimpleTextIn/AbsolutePointer) on top of UEFI controller handles that expose the hid_io protocol.
• usb_hid - implements the UEFI driver binding model to produce hid_io instances on top of UEFI controller handles that expose the usb_io protocol and support HID input.

In addition, this PR introduces two new modules in the Patina SDK to define protocols:

• usb_io - specifies the protocol defined in UEFI spec 2.11 section 17.2.4
• hid_io - defines the HID IO vendor protocol (which could be submitted for inclusion in the appropriate spec, but is presently implemented as a "vendor proprietary" protocol).

• Impacts functionality?
• Impacts security?
• Breaking change?
• Includes tests?
• Includes documentation?

How This Was Tested

Includes extensive unit tests that cover the new component logic. Also tested on hardware platforms with a variety of input devices, confirmed keyboard, trackpad, and touch all working as expected on USB (using the above component) and other proprietary hardware interfaces that produce the HidIo protocol directly.

Integration Instructions

See component readme.

[patina-automation]

⌛ QEMU Validation Pending

QEMU validation is pending on successful CI completion.

Note: Any previous results are available in this comment's edit history.

This comment was automatically generated by the Patina QEMU PR Validation workflow.

[os-d]

I had taken a look at doing this same thing at one point, but we had decided not to because it is reliant on the driver binding dispatching flow, right?

My first thought is that we have a Patina BDS service actively being worked on (however it is not actively supporting Rust dispatch). Does it make sense for this to tie in there? Presumably it would somehow register with the BDS service for dispatch at BDS time?

We have discussed not letting Patina components depend on protocols, which it looks like this one does. The suggestion is to have an "unsafe component" that can register for various protocols and republish them as Patina services so that when whatever protocol is ported to Patina the component doesn't have to change, just the unsafe one does.

I guess what I'm getting at is this one, because it bridges into BDS, may be useful to have an RFC to discuss some of these points. Maybe I'm misremembering this and there is no BDS intersection and we just treat it as a standard DXE component.

[codecov]

Codecov Report

❌ Patch coverage is 91.74128% with 535 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
components/uefi_hid/src/keyboard/mod.rs	80.13%	238 Missing ⚠️
components/usb_hid/src/driver.rs	12.10%	109 Missing ⚠️
...ponents/uefi_hid/src/keyboard/simple_text_in_ex.rs	93.36%	37 Missing ⚠️
components/uefi_hid/src/pointer/mod.rs	93.84%	30 Missing ⚠️
components/uefi_hid/src/hid_io/mod.rs	92.67%	20 Missing ⚠️
components/uefi_hid/src/keyboard/layout.rs	95.03%	18 Missing ⚠️
components/usb_hid/src/hid_io_impl.rs	96.61%	17 Missing ⚠️
components/usb_hid/src/interrupt_transfers.rs	95.37%	17 Missing ⚠️
components/uefi_hid/src/lib.rs	79.41%	14 Missing ⚠️
components/uefi_hid/src/hid_io/protocol.rs	80.85%	9 Missing ⚠️
... and 8 more

📢 Thoughts on this report? Let us know!

[joschock]

I had taken a look at doing this same thing at one point, but we had decided not to because it is reliant on the driver binding dispatching flow, right?

My first thought is that we have a Patina BDS service actively being worked on (however it is not actively supporting Rust dispatch). Does it make sense for this to tie in there? Presumably it would somehow register with the BDS service for dispatch at BDS time?

We have discussed not letting Patina components depend on protocols, which it looks like this one does. The suggestion is to have an "unsafe component" that can register for various protocols and republish them as Patina services so that when whatever protocol is ported to Patina the component doesn't have to change, just the unsafe one does.

I guess what I'm getting at is this one, because it bridges into BDS, may be useful to have an RFC to discuss some of these points. Maybe I'm misremembering this and there is no BDS intersection and we just treat it as a standard DXE component.

There is no direct BDS relationship here. BDS does tend to use console services if it supports UI features - but that's only connected with this component insofar as that would be a client usage of the services provided by this component. This component does not know or care whether BDS exists.

One of the reasons I did this port from the existing HID driver was, in fact, to provide a ground for discussion of how components might be designed for more complicated usages involving the UEFI driver model. I would note that we are doing other forms of UEFI FFI (i.e. via ACPI and SMBIOS drivers and eventually associated protocols), so it's not clear to me why we would draw a distinction at the FFI here because it uses the driver model.

I could write an RFC, but right now I don't have a lot to hang on it as I'm not really proposing any architectural changes to Patina or the component model - more like exploring what/how we might do things with the component model. It would essentially be "We should explore using components to implement features that require UEFI Driver Binding support."

I am also okay with deciding that this isn't within the scope/remit of Patina itself; will happily carry this as an "extension" component in our repos instead. However, I do think it is a good idea to model how more complex usage can be done via the component model.

[os-d]

I had taken a look at doing this same thing at one point, but we had decided not to because it is reliant on the driver binding dispatching flow, right?
My first thought is that we have a Patina BDS service actively being worked on (however it is not actively supporting Rust dispatch). Does it make sense for this to tie in there? Presumably it would somehow register with the BDS service for dispatch at BDS time?
We have discussed not letting Patina components depend on protocols, which it looks like this one does. The suggestion is to have an "unsafe component" that can register for various protocols and republish them as Patina services so that when whatever protocol is ported to Patina the component doesn't have to change, just the unsafe one does.
I guess what I'm getting at is this one, because it bridges into BDS, may be useful to have an RFC to discuss some of these points. Maybe I'm misremembering this and there is no BDS intersection and we just treat it as a standard DXE component.

There is no direct BDS relationship here. BDS does tend to use console services if it supports UI features - but that's only connected with this component insofar as that would be a client usage of the services provided by this component. This component does not know or care whether BDS exists.

One of the reasons I did this port from the existing HID driver was, in fact, to provide a ground for discussion of how components might be designed for more complicated usages involving the UEFI driver model. I would note that we are doing other forms of UEFI FFI (i.e. via ACPI and SMBIOS drivers and eventually associated protocols), so it's not clear to me why we would draw a distinction at the FFI here because it uses the driver model.

I could write an RFC, but right now I don't have a lot to hang on it as I'm not really proposing any architectural changes to Patina or the component model - more like exploring what/how we might do things with the component model. It would essentially be "We should explore using components to implement features that require UEFI Driver Binding support."

I am also okay with deciding that this isn't within the scope/remit of Patina itself; will happily carry this as an "extension" component in our repos instead. However, I do think it is a good idea to model how more complex usage can be done via the component model.

I think it is very valuable to have more complex components and extend Patina in that way. I need to review what the component is doing, will review the PR. The part I was suggesting is new is the driver binding support, doesn’t that get launched by BdsDxe today?

[joschock]

The part I was suggesting is new is the driver binding support, doesn’t that get launched by BdsDxe today?

It's launched by gBS->ConnectController() from UEFI Boot Services table. This can be called from anywhere, but typically BDS is one of the main generators of calls to Connect as it brings up devices to prep for boot. BDS typically calls connect to bring up the conosoles and on the device path for the boot options to ensure that the hardware is fully up and running before locating and executing boot targets.

But we also (on the platform side) call Connect to light up hardware needed to get to BDS in the first place - some of our designs require the file system stack to be up and running to expose partitions needed for firmware load. Another example of non-BDS calls would be USB hotplug - it calls connect on freshly created USB devices to light up the driver stacks for them.

[os-d]

The part I was suggesting is new is the driver binding support, doesn’t that get launched by BdsDxe today?

It's launched by gBS->ConnectController() from UEFI Boot Services table. This can be called from anywhere, but typically BDS is one of the main generators of calls to Connect as it brings up devices to prep for boot. BDS typically calls connect to bring up the conosoles and on the device path for the boot options to ensure that the hardware is fully up and running before locating and executing boot targets.

But we also (on the platform side) call Connect to light up hardware needed to get to BDS in the first place - some of our designs require the file system stack to be up and running to expose partitions needed for firmware load. Another example of non-BDS calls would be USB hotplug - it calls connect on freshly created USB devices to light up the driver stacks for them.

Right, so that’s where I am suggesting that there may be intersection with the new Patina BDS here. I.e. that in Patina we likely wouldn’t say that inter-Rust calls would go through the ConnectController protocol and that Patina BDS would dispatch them differently, in a way that avoids the C FFI and perhaps has other properties that we think is desireable. Which is why when we were discussing componetizing this, we opted not to immediately because it felt like there would be additional redesign.

I don’t think that this component needs to solve those problems (the BDS one does, if we say it owns the driver binding model), but as you said, it is the first one being introduced using the driver binding protocol, so I think we should at least discuss how that may look, does it make sense to just take it as using the protocol to start (very likely), but having some idea of roadmap here.

I would still maintain that the component should not rely on any protocols directly and should use Patina services (publishing protocols is fine, but we shouldn’t consume). The unsafe component is then our bridge to get to a no-protocol environment.

[makubacki]

Maybe we can talk about this again in the upcoming Patina Dev Sync meeting.

Part of the issue in the previous discussion was prioritization for a proper UEFI Driver Model abstraction and resources available. That'd be better to gauge with the team gathered.

[joschock]

Yep! Agreed it's a good topic to discuss, and I'm not necessarily in a hurry to get this in - I mostly just wanted to take a crack at a reasonably complicated driver component to see how it might be done and practice what the approach might be like.

No objections to developing an abstraction around and/or replacement for driver binding framework amongst components; though I do think that as long as we have a heterogenous mix of patina and C drivers (which, in my view, is probably going to be the case for a long while yet) that it makes sense to interact with FFI with the existing UEFI spec abstractions. At some point we may have only components and can have pure abstractions for those - but until that point, the driver model and protocol interfaces are the spec way to interact with the rest of the world. This particular component is going to have to interact with hardware drivers from silicon vendors written in C (i.e. the HidIo abstraction it consumes eventually has to talk to USB/I2C/whatever), and it's going to need to produce services consumed by drivers written in C (i.e. AbsolutePointer and SimpleTextIn for UEFI UI and shell console, etc).

As far as safe abstractions around the drvier model - this component leverages the existing abstractions in the sdk (i.e. sdk/patina/src/driver_binding.rs) for the binding structures themselves. We don't presently have separate abstractions for HidIo/AbsolutePointer/SimpleTextIn[Ex], though I tried to write the component in such a way as those are separated out reasonably.

[makubacki]

Yep! Agreed it's a good topic to discuss, and I'm not necessarily in a hurry to get this in - I mostly just wanted to take a crack at a reasonably complicated driver component to see how it might be done and practice what the approach might be like.

The code is very helpful to review, thanks for the effort to port it over. As has been alluded to previously, we really need to articulate what ideal integration looks like. This helps provide a tangible reference for that and an actual implementation we can evolve toward that definition.

---

Can you please summarize what a platform needs from Mu to use the component?

[joschock]

Can you please summarize what a platform needs from Mu to use the component?

Aside from integrating this component (per the readme in this PR), a platform needs at least one HidIo producer.

An example of one (for USB) is here: https://github.com/microsoft/mu_plus/tree/release/202502/HidPkg/UsbHidDxe - many
of our platforms also use e.g. HID over I2C or proprietary protocols (e.g. via EC interfaces) to produce the same.

And, of course, to remove any alternative HID stack components (e.g. UsbKbDxe or similar) that might take over the controller and prevent driver attach.

[makubacki]

Can you please summarize what a platform needs from Mu to use the component?

Aside from integrating this component (per the readme in this PR), a platform needs at least one HidIo producer.

An example of one (for USB) is here: https://github.com/microsoft/mu_plus/tree/release/202502/HidPkg/UsbHidDxe - many of our platforms also use e.g. HID over I2C or proprietary protocols (e.g. via EC interfaces) to produce the same.

And, of course, to remove any alternative HID stack components (e.g. UsbKbDxe or similar) that might take over the controller and prevent driver attach.

Thanks. Ideally, we would have a set of functionality that can work without necessitating Mu code. We'll need to consider that as well.

[joschock]

Thanks. Ideally, we would have a set of functionality that can work without necessitating Mu code. We'll need to consider that as well.

Well, I could componentize UsbHidDxe into a patina component too :)

[makubacki]

Thanks. Ideally, we would have a set of functionality that can work without necessitating Mu code. We'll need to consider that as well.

Well, I could componentize UsbHidDxe into a patina component too :)

I wouldn't be opposed to it if we move forward with this component.

[joschock]

Along those lines, the formal def of the HidIo protocol is presently in the HidPkg in Mu. I purposefully duplicated it in the component here to avoid direct dependency, but ultimately it would need to move somewhere if you wanted to completely decouple.

[makubacki]

Along those lines, the formal def of the HidIo protocol is presently in the HidPkg in Mu. I purposefully duplicated it in the component here to avoid direct dependency, but ultimately it would need to move somewhere if you wanted to completely decouple.

We could either attempt to fully upstream or fall back to https://github.com/OpenDevicePartnership/patina-edk2. It looks like you're the original protocol author, so I'm interested in your thoughts on something like PI Spec.

[joschock]

We could either attempt to fully upstream or fall back to https://github.com/OpenDevicePartnership/patina-edk2. It looks like you're the original protocol author, so I'm interested in your thoughts on something like PI Spec.

I'm open to whatever approach makes sense here; I don't have a strong opinion about this. I created the protocol primarily to abstract over various hardware implementations of HID, and would be happy to extend or modify it based on community feedback from upstream or PI spec. Or - happy to keep this as an implementation-level protocol spec for interop with this particular component, if that makes more sense.

[joschock]

@makubacki / @os-d this is now updated with an implementation of the USB portion (as discussed), so is now an end-to-end input solution that doesn't require any proprietary hardware components to implement.

[os-d]

@makubacki / @os-d this is now updated with an implementation of the USB portion (as discussed), so is now an end-to-end input solution that doesn't require any proprietary hardware components to implement.

Added some reviewers. Thought 11k lines is daunting :)

[joschock]

Added some reviewers. Thought 11k lines is daunting :)

Yeah, it's big :( - but a lot of test code. Hopefully the business code is a bit more manageable.

Also, again, open to feedback on the mechanics of submitting stuff like this. Happy to rebase the commits to be more sane or split it to separate PRs.

[makubacki]

@joschock, we're thinking that the components introduced in this PR might be a good time to evaluate a separate patina component repo. Do you have any concerns with that?

If not, I can setup and prepopulate the repo so you can move the component changes there. That would result in a lighter PR here we could merge more quickly and make the changes available to the component repo.

[joschock]

@joschock, we're thinking that the components introduced in this PR might be a good time to evaluate a separate patina component repo. Do you have any concerns with that?

If not, I can setup and prepopulate the repo so you can move the component changes there. That would result in a lighter PR here we could merge more quickly and make the changes available to the component repo.

Completely aligned. One of the goals of this PR is to tease out what the right way to contribute these kinds of changes is; and I'm happy to rework the PR to split to new repo. I assume the split you are thinking is components to new repo, SDK changes to this repo?

[makubacki]

Completely aligned. One of the goals of this PR is to tease out what the right way to contribute these kinds of changes is; and I'm happy to rework the PR to split to new repo. I assume the split you are thinking is components to new repo, SDK changes to this repo?

Yes. I put more detail here #1417. Please give a couple of days for the new repo to be setup and have CI in place to accept the PR.

[apop5]

shouldn't the supported functions be checking OPEN_PROTOCOL_BY_DRIVER to make sure that someone else hasn't taken exclusive access already?

[joschock]

Good point. I think it wouldn't actually functionally break, but it would fail out of start() instead of failing out at supported() which is the correct behavior.

[makubacki]

Yes. I put more detail here #1417. Please give a couple of days for the new repo to be setup and have CI in place to accept the PR.

@joschock, patina-components is ready for your PR. You can drop the placeholder crate.

[joschock]

Yes. I put more detail here #1417. Please give a couple of days for the new repo to be setup and have CI in place to accept the PR.

@joschock, patina-components is ready for your PR. You can drop the placeholder crate.

I'm going to close this PR and submit a new one once I've reworked it to split between this repo and patina-components.