Good day Patrick, On Wed, 24 Feb 2021 07:46:48 +0100 Patrick Rudolph <patrick.rudo...@9elements.com> wrote:
> Hi Glenn, > yes it's the same patch series, but has been cleaned and improved a > lot. I've addressed most of the comments received earlier. I see that I implied that you hadn't and I didn't mean to. I only meant to suggest that there were possible unaddressed issues. Glad to hear those were already taken care of. > As stated in the commit message, USB 3.1 and USB 3.2 are not > tested, as I lack hardware to test this. I'm not going to look into > this any further into this as it works fine with USB 3.0 compliant > xhci controllers. > > I'l fix the remaining issues and publish a new patch series. Great. Are you thinking of adding in some tests? If not, I would appreciate knowing the qemu opts used to create the virtual machine (I'm unsure how to turn on xHCI in qemu). Glenn > > On Fri, Feb 19, 2021 at 7:11 PM Glenn Washburn > <developm...@efficientek.com> wrote: > > > > Hi Patrick, > > > > Thanks for the contribution. I think this would be a great addition > > to GRUB. However, there are a few issues I see at the moment. > > > > On Mon, 7 Dec 2020 08:41:20 +0100 > > Patrick Rudolph <patrick.rudo...@9elements.com> wrote: > > > > > Add basic support for xHCI USB controllers and non root xHCI hubs. > > > The motivation is to use this code on platforms that do not > > > provide user input by runtime services (like BIOS or UEFI > > > platform) do. This is the case when GRUB is used as coreboot > > > payload for example. > > > > > > The code is based on seabios implementation, but has been heavily > > > modified to match grubs internals. > > > > Are these the same changes as in > > https://github.com/Nitrokey/grub.git referenced in a previous > > email? I'm asking because there were some comments that were > > unaddressed. So, if it is, could you please address Thomas's email > > (I haven't checked to see if they are still relevant if so)? (see: > > https://marc.info/?l=grub-devel&m=159769164332293) > > > > Searching in the mailing list, I found this attempt in early 2017. > > Having not looked at the code at all, could this help in addressing > > some of Thomas's concerns? > > https://github.com/bjornfor/grub/tree/add-coreboot-xhci-driver-2nd-attempt-v2 > > > > I've run your code through the GitLab CI I've configured, and there > > was a build failure on x86_64-efi. Looks like some implicit type > > casting issues. I believe it failed for both gcc 8.1 and 10.1. Here > > is the error log when using gcc 10.1: > > https://gitlab.com/gwashburn/grub/-/jobs/1025317154#L594 > > > > Also, I think that any xhci support should be accompanied by passing > > make check tests. See uhci_test for an example of how this might be > > done. Since you've already tested on qemu, I think you're 90% of the > > way there in making some tests. Both of the ones listed before > > would be nice. If you'd like some guidance or help, let me know. > > > > Glenn > > > > > Changes done in version 2: > > > * Code cleanup > > > * Code style fixes > > > * Don't leak memory buffers > > > * Compile without warnings > > > * Add more defines > > > * Add more helper functions > > > * Don't assume a 1:1 virtual to physical mapping > > > * Flush cachelines after writing buffers > > > * Don't use hardcoded page size > > > * Proper scratchpad register setup > > > * xHCI bios ownership handoff > > > > > > Changes done in version 3: > > > * Several bug fixes for real hardware > > > * Fixed a race condition detecting events, which doesn't appear > > > on qemu based xHCI controllers > > > * Don't accidently disable USB3.0 devices after first command > > > * Support arbitrary protocol speed IDs > > > * Coding style cleanup > > > * Support for non root SuperSpeed hubs > > > > > > Changes Tested: > > > * Qemu system x86_64 > > > * virtual USB HID keyboard (usb-kbd) > > > * virtual USB HID mass storage (usb-storage) > > > * Intel C246 integrated xHCI (Supermicro X11SSH-F) > > > * iKVM HID keyboard > > > * USB3 HID mass storage (controller root port) > > > * External USB HID keyboard > > > > > > TODO: > > > * Test on more hardware > > > * Test on USB3 hubs > > > * Support for USB 3.1 and USB 3.2 controllers > > > > > > Patrick Rudolph (7): > > > grub-core/bus/usb: Parse SuperSpeed companion descriptors > > > usb: Add enum for xHCI > > > usbtrans: Set default maximum packet size > > > grub-core/bus/usb: Add function pointer for attach/detach events > > > grub-core/bus/usb/usbhub: Add new private fields for xHCI > > > controller grub-core/bus/usb: Add xhci support > > > grub-core/bus/usb/usbhub: Add xHCI non root hub support > > > > > > Makefile.am | 2 +- > > > grub-core/Makefile.core.def | 7 + > > > grub-core/bus/usb/ehci.c | 2 + > > > grub-core/bus/usb/ohci.c | 2 + > > > grub-core/bus/usb/serial/common.c | 2 +- > > > grub-core/bus/usb/uhci.c | 2 + > > > grub-core/bus/usb/usb.c | 44 +- > > > grub-core/bus/usb/usbhub.c | 95 +- > > > grub-core/bus/usb/usbtrans.c | 6 +- > > > grub-core/bus/usb/xhci-pci.c | 195 +++ > > > grub-core/bus/usb/xhci.c | 2496 > > > +++++++++++++++++++++++++++++ grub-core/commands/usbtest.c | > > > 2 +- grub-core/disk/usbms.c | 2 +- > > > grub-core/term/usb_keyboard.c | 2 +- > > > include/grub/usb.h | 18 +- > > > include/grub/usbdesc.h | 12 +- > > > include/grub/usbtrans.h | 4 + > > > 17 files changed, 2852 insertions(+), 41 deletions(-) > > > create mode 100644 grub-core/bus/usb/xhci-pci.c > > > create mode 100644 grub-core/bus/usb/xhci.c > > > _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel