USB on real hw

[Bonfra]

I've pushed it do github, here it is
https://github.com/Bonfra04/BonsOS/blob/master/serial.img

[BenLunt]

So far I have found two things, which may or may not fix your problem.

1) At line 253 in uhci.c, https://github.com/Bonfra04/BonsOS/blob ... hci.c#L253, you are reading a WORD when the register is a byte.

2) At line 17 in scsi.c, https://github.com/Bonfra04/BonsOS/blob ... scsi.c#L17, you are setting the command length to 12. However, command 0x25 (Read Capacity) expects the length to be 10. Remove line 75 from scsi_types.h, https://github.com/Bonfra04/BonsOS/blob ... ypes.h#L75, and see what happens.

The physical hardware may not like the incorrect command size and that is why it isn't working. I have added a check to BOCHS to give an error if this happens. (I will push this check with my other USB emulation additions when that time comes).

Ben

[Bonfra]

1) At line 253 in uhci.c, https://github.com/Bonfra04/BonsOS/blob ... hci.c#L253, you are reading a WORD when the register is a byte.

I mean yea this is wrong but while scanning through the code this wrong read only appears inside the logs. At line 58 for example (during the setup) the reading is performed correctly.

2) At line 17 in scsi.c, https://github.com/Bonfra04/BonsOS/blob ... scsi.c#L17, you are setting the command length to 12. However, command 0x25 (Read Capacity) expects the length to be 10. Remove line 75 from scsi_types.h, https://github.com/Bonfra04/BonsOS/blob ... ypes.h#L75, and see what happens.

That is a typical bug that would've take me months to find XD, sadly this happens later than where the code gets stuck in real HW (which is during the assignment of the USB address, even before retrieving the device descriptros)

Anyway I implemented both things and tested the code, seems nothing have changed :(

[BenLunt]

I am still convinced that it is the reset code. Since you are using the PIT and delays (somewhat much) longer than microseconds (the 8253 having a ~1.2 MHz clock), the delays may not be correct for the UHCI.

I could be completely wrong, and it is something else. However, I personally experienced the same thing you are describing and once I used the reset code I posted earlier, it worked as expected.

The image you sent me shows that you are experimenting with the HPET. Is this true? If so, you can get much more accurate delays with the HPET. You are also reading from the MSR_APICBASE. Does this mean you are experimenting with the APIC?

Code: Select all

00425899964i[APIC0 ] set timer divide factor to 16

Where you can get a very accurate delay with it as well.

The former timer tells you the frequency, so it is a simple task to get delays with a microsecond (plus) accuracy. The latter, you have to do a little more effort by having to get the CPU's frequency or using a different clock (which may not exist) to calculate the incremental timer.

Anyway, please keep us posted. If I happen to find anything else, I will let you know.
Ben
- https://www.fysnet.net/osdesign_book_series.htm

[BenLunt]

That is a typical bug that would've take me months to find XD...

Luckily, the code I am adding to Bochs found this little error. I am adding a bunch of checks to all four controller types and the devices supported to better help find bugs. The new code should be pushed to the Bochs Github as soon as I thoroughly test it.

...sadly this happens later than where the code gets stuck in real HW (which is during the assignment of the USB address, even before retrieving the device descriptros)

Something that I didn't even check before, I don't know why, I just assumed I guess. Your code tries to set the address first thing. My book clearly states that you cannot do this. You must get the first 8 bytes of the Device Descriptor, reset the device, set the address, then get the full Device Descriptor. This is a must. Some devices (more than you think) expect this, even though this is not USB compliant, and the device won't function correctly without it.

1) reset device
2) get the first 8 bytes of the Device Descriptor
3) reset device
4) set address
5) get all 18 bytes of the Device Descriptor
6) now you can do the rest in an order that pertains to the device.

Try that and see what happens. (Sorry I didn't catch this before. I simply assumed you were doing this since you have my book, and that I have always done this sequence, it is just second nature for me now).

Ben

P.S. I have found that it is more that the first request (#2 above) must be less than or equal to the max packet size of the control endpoint rather than 8 bytes, though you don't know the max packet size until after reading in (at least the first 8 bytes of) the device descriptor. So I have found if you just read in only the first 8 bytes, it will work on all devices, no matter the speed of the device. For example, on devices that are full-speed or higher, you can request 64 bytes (instead of 8) and it still works, as long as the max packet size for the Control EP is not less than 64.

[Bonfra]

1) reset device
2) get the first 8 bytes of the Device Descriptor
3) reset device
4) set address
5) get all 18 bytes of the Device Descriptor
6) now you can do the rest in an order that pertains to the device.

So something like this should do it right?

Code: Select all

   for(uint64_t i = 0; i < bus->hci.num_ports; i++)
    {
        if(!bus->hci.driver->reset_port(bus->hci.data, i))
            continue;
        if(bus->hci.driver->port_status(bus->hci.data, i) == USB_PORT_STATUS_NOT_CONNECT)
            continue;

        kernel_trace("Found connected USB device");

        uint8_t eight[8];
        if(usb_get_standard_descriptor(&(usb_device_t){.bus = bus, .addr = 0}, USB_DESCRIPTOR_DEVICE, 0, eight, 8) != USB_TRANSFER_STATUS_OK)
            return;

        kernel_trace("Got first 8 bytes of device descriptor: %x %x %x %x %x %x %x %x", eight[0], eight[1], eight[2], eight[3], eight[4], eight[5], eight[6], eight[7]);

        if(!bus->hci.driver->reset_port(bus->hci.data, i))
            continue;
        if(bus->hci.driver->port_status(bus->hci.data, i) == USB_PORT_STATUS_NOT_CONNECT)
            continue;

        kernel_log("Registering...");

        usb_register_device(bus);
    }

I don't technically need those eight bytes so I'm ignoring them for the moment.
Now it gets stuck inside this request and doesn't reach the second reset.. I've added some more logs, hopes this help.


About the HPET, Yes managed to make this code work but I'm still to implement a proper delay method with this one. I'll try to use this one for delays and update the post as soon as i get some results. I'm using LAPICs timers for the scheduler so maybe I'm going to use the HPET instead. (don't worry the schedule part happens a lot later in the code than where we are working so it doesn't interfere)

[Bonfra]

Hello there! Sorry for the long absence...
So I'm back to work on this USB UHCI driver thingy yay!
I did some sporadic tests during this period but all were unconcluded. I basically eliminated every single thing that "should work but I'm not 100% sure it will" in the code and triple-checked all code both for UCHI and for the USB protocol.

Here is a quick recap/overview of the current process I'm following:

Host controller setup:

Code: Select all

- Find any EHCI controller on the PCI bus;
- Set its op.configFlag bit to zero so that it passes control to the companion (should be done by the BIOS since It's enabled in the settings but to be sure). Nothing else is touched in the EHCI controller (not even reset);
- Find any UHCI controller on the PCI bus;
- Ensure it is port mapped
- Set its PCI privilege to MMIO | DMA | PIO
- Disable legacy support by writing 0x8F00 to the LEGSUP register
- Reset the controller
  - Set and clear 5 times the GRESET bit on USBCMD with an 11 secs delay
  - Check for default values on all registers
  - Clear the status register
  - Set the HCRESET bit on USBCMD
  - Wait for 42ms and check again for a successful reset
- Disable interrupts by writing 0 to USBINTR
- Set FRNUM to 0
- Allocate a 1024 dword buffer 0x100 aligned and set all entries to be only the TERMINATE bit
- Put this address into FRBASEADD
- Set SOFMOD to SOFMOD_64
- Clear the USBSTS again by writing 0xFF
- Check the number of available ports with bit 7a nd 1:3 of PORTSC
- Finish the setup by writing USBCMD_CF | USBCMD_RUNSTOP to USBCMD

Now for each port:

Code: Select all

- Reset the port
  - Set the PORT_RESET bit on PORTSC
  - Wait 50ms and clear the bit
  - Wait another 10ms for recovery time
  - Wait for the port to become enabled
    - Check for CONNECT_STATUS
    - Check for ENABLE_CHANGE | STATUS_CHANGE
    - Check for ENABLE
    - Set ENABLE, slep for 10ms, and repeat the process
- Discard the port if it doesn't have the CONNECT_STATUS bit set
- Get the first 8 bytes of the device descriptor

I'm interrupting the list here to inspect deeply the last point since it is the problematic one, all the process works smoothly and all of the checks returns positively till that get_descriptor thing. This is how I do it in detail: (there may be some unneeded abstraction for the POC but I want to be as transparent as possible)

Code: Select all

- Build a request packet with (0x1000 aligned):
  - type = USB_REQUEST_DIR_DEVICE_TO_HOST | USB_REQUEST_TYPE_STANDARD | USB_REQUEST_RECP_DEVICE;
  - request = USB_REQUEST_GET_DESCRIPTOR;
  - value = (USB_DESCRIPTOR_DEVICE << 8) | 0;
  - index = 0;
  - size = 8
- Use the request packet to build three USB packets (first 0x1000 aligned, subsequent follow):
  - the first one has:
    - type = USB_PACKET_TYPE_SETUP;
    - maxlen = 8;
    - buffer = <the physical address of the setup packet>;
    - toggle = 0;
  - the second one has:
    - type = USB_PACKET_TYPE_IN;
    - maxlen = 8;
    - buffer = <the physical address of the 0x1000 buffer where to save the 8 bytes>;
    - toggle = 1;
  - the third one has:
    - type = USB_PACKET_TYPE_OUT;
    - maxlen = 0x800;
    - buffer = NULL;
    - toggle = 1;
- Build the TDs starting from the above structure (first 0x1000 aligned, subsequent follow)
- Create a queue head (0x1000 aligned) that points to the TDs
- Clear all the framelist with FRAMELIST_TERMINATE
- Set index one of the framelist to point to the queue head
- Start the schedule
  - Clear USBCMD_RUNSTOP
  - Write/Clear USBSTS_INT
  - Set FRNUM to 0
  - Set USBCMD_RUNSTOP
- Wait for USBSTS_INT to be set
- Stop the schedule
  - Clear USBCMD_RUNSTOP
  - Write/Clear USBSTS_INT

This is the complete process. The problem with real HW is that the loop waiting for USBSTS_INT to be set by the last TD with the IOC bit set never stops waiting.
For precise values inside the TDs look at the screenshot in the previous posts.

I tried reimplementing this whole process numerous times every time changing some bits here and there but it always works on VMs and never in real HW. I re-read multiple times Benjamin's book but I really have no idea what I'm doing wrong.
The only thing I think is that I sent the final OUT TD before having received the data but also adding a little sleep before sending the last TD doesn't do anything...

[Klakap]

It looks like you are not reading speed of connected device. But it is vitally important thing. If you have connected full speed device, you have to request all 18 bytes at once. Otherwise there is hardware that will do all kind of strange things. For example, on one of my testing computer such piece of hardware resets whole computer just because I requested 8 bytes, not 18 bytes from descriptor. And when you will read other descriptors from full speed device, also make sure you are requesting maximum possible bytes - 64.

[Bonfra]

If you have connected full speed device, you have to request all 18 bytes at once [...] And when you will read other descriptors from full speed device, also make sure you are requesting maximum possible bytes - 64.

What if I'm requesting something like the configuration descriptor which is only 9 bytes? do i still need to allocate a buffer of 64, request all of them and then only use 9? or can i just request 9?
By "all 18 bytes at once" you mean setting the maxlen to 18 or just adding more TDs to reach 18?

Anyway your point holds up, i checked the hw and the device is indeed full speed.

[Klakap]

I mean that through every IN TD you have to transfer as much bytes as possible. Low speed devices allow you to transfer maximum of 8 bytes per one IN TD. But full speed devices allow you to transfer 64 bytes per one IN TD. So when you are requesting descriptor that return 18 bytes, for full speed devices you have to transfer them all in one IN TD. For low speed devices you need to use three IN TDs. Of course, do not forget to check how many bytes are you requesting in SETUP packet. Try to request all 18 bytes, and transfer them in one IN TD.

[Klakap]

What if I'm requesting something like the configuration descriptor which is only 9 bytes? do i still need to allocate a buffer of 64, request all of them and then only use 9? or can i just request 9?

For full speed device, you need one IN TD with maxlen 9, for low speed device, you need two IN TD, first with maxlen 8 and second with maxlen 1.

[Bonfra]

Ok so i need to always maximise the len i'm requesting, then split into multiple pieces of 8 for low speed and 64 for high speed. Can i always assume this size or is it better to read it form the device descriptor? maybe assume it for reading the descriptor the first time then use the one written there

[Klakap]

When you read device descriptor, you have to assume size by speed and then you should use value from device descriptor. For example if I recall correctly, QEMU handle mouse and keyboard in way that it emulates them as full speed device, so device descriptor should be readed in one 18 bytes IN TD, however max control endpoint size in descriptor is 8, so you should read all other descriptors by 8 bytes long IN TD.

[BenLunt]

Hi guys,

By no means do I want to sound confrontational, but I have to agree and disagree with some of these comments. Also, though I think I am quite versed in this subject, I do not know all things about it, so please take my comments as my opinion, not fact.

It looks like you are not reading speed of connected device. But it is vitally important thing. If you have connected full speed device, you have to request all 18 bytes at once. Otherwise there is hardware that will do all kind of strange things. For example, on one of my testing computer such piece of hardware resets whole computer just because I requested 8 bytes, not 18 bytes from descriptor. And when you will read other descriptors from full speed device, also make sure you are requesting maximum possible bytes - 64.

First, I agree with the speed verification. As the first request, if you detect a full-speed device, you should request 64 bytes, expecting a short packet on a short return. If you detect a low-speed device, you should request only 8 bytes, not necessarily expecting a short packet. The port's register set will indicate the speed.

However, I disagree with the "make sure you are requesting maximum possible bytes - 64". A device must adhere to your request. If you request only 8 bytes, the device must not send more than 8 bytes. It can send less, but it must not send more. A TD that requests only one byte is a valid request. If a TD requests more than the max_packet_size previously retrieved, all but the last DATA packet of the TD (which the last could be the one and only) should be of max_packet_size, while the last can and usually is less than the max_packet_size. However, again, a TD containing only one DATA packet, can request less than max_packet_size. It is allowed.

With that being said, I again agree, that when requesting the Device Descriptor for the first time, you should request the assumed Max Packet Size, 8 bytes for a low-speed device, and 64 bytes for a full-speed device. However, after that, the device must respond with any size packet requested, from a single byte to the max allowed by the hardware (8, 64, 512, 4096, etc.), per TD.

A note: When I use the term TD here, this is considered a Transfer Descriptor. A Transfer Descriptor is a count of one or more packets requesting a transfer. A Control TD is a SETUP packet, zero or more DATA packets, and a single STATUS packet. Unfortunately, the UHCI specs also name a single packet transfer as a TD. Therefore, don't confuse the term TD here with a single UHCI transfer TD. In all USB terms, a TD (a Transfer Descriptor) is considered one or more packet requests consisting of a complete transfer.

USB 2.0, section 5.3.2 states:

The data payloads for such a multiple data payload IRP are expected to be of the maximum packet size until the last data payload that contains the remainder of the overall IRP.

You must first get the max packet size from the Device Descriptor, which may be much less than 64 bytes on a full-speed device. Once retrieved, you should use this value for every sequential packet, expecting a possible short-packet on the last request.

If the hardware does "all kind of strange things", this is not due to your request, this is due to something else. Again, please don't take this as confrontational, but if your computer reset the whole system because you requested only 8 bytes instead of 18, I believe you have more problems than you think. Either your USB stack is faulty, the HC is faulty, the device attached is faulty, or any or all the above.

I mean that through every IN TD you have to transfer as much bytes as possible. Low speed devices allow you to transfer maximum of 8 bytes per one IN TD. But full speed devices allow you to transfer 64 bytes per one IN TD. So when you are requesting descriptor that return 18 bytes, for full speed devices you have to transfer them all in one IN TD. For low speed devices you need to use three IN TDs. Of course, do not forget to check how many bytes are you requesting in SETUP packet. Try to request all 18 bytes, and transfer them in one IN TD.

Because of bad choices by manufacturers testing their products only on a well-known well-used operating system that incorrectly interpreted a statement about a device in early development, the first request after a reset must be for the Device Descriptor *and* it must be a request for 8 bytes on low-speed devices, and 64-bytes on full speed devices. However, after that, this is no longer the case. You do not have to request as much as possible. It is preferred that you do, but it is not a requirement. Also, not all full-speed devices allow you to transfer 64-bytes per IN request. A full-speed mouse may only allow 8 bytes at a time, or less. This is why you request the Device Descriptor as the first descriptor so that you can retrieve the max packet size the device can transfer. From there, you only request a max size of this retrieved value, which can be less than 64.

For example, what happens when you plug a high-speed device into a full-speed Host Controller (HC)? Even though the high-speed device, when connected to a high-speed capable HC, might transfer more than 64-bytes at a time, the device *must* act as a full-speed device transferring no more than 64-bytes at a time. A device must adhere to the HC it is attached to as well as never sending more than requested. On the other side of that, a high-speed device, when plugged into a full-speed only HC, is only required to enumerate the Control pipe. It is not required to enumerate further. i.e.: it only has to respond to the Control pipe. Most high-speed devices will enumerate as a full-speed device, functioning as a full-speed device, but this is not a requirement.

Again, it is recommended and preferred that you transfer MAX_PACKET sized packets, usually 64 bytes at a time for full-speed devices, but it is not a requirement. A properly functioning device must respect the size requested. If the size a TD requests is a single byte, it must respect that and only send one byte. i.e.: It must only write one byte to the buffer you specify. If it writes more, you have a faulty Host Controller and/or Device. Remember that a TD is one or more max_packet_size packets. A Control TD is a single SETUP, zero or more DATA, and a single STATUS packet. It is perfectly acceptable to request a single byte of data, which would consist of one and only one DATA packet. It is all DATA packets before the last that should (in most cases *must*) contain max_packet_size'd packets.

For full speed device, you need one IN TD with maxlen 9, for low speed device, you need two IN TD, first with maxlen 8 and second with maxlen 1.

This is correct. For (most) full-speed devices, you only need one IN TD. For low-speed devices, you need at least two. However, again, you must adhere to the max_packet_size, not the speed of the device. A full-speed device can have a max_packet_size of 8 bytes.

When you read device descriptor, you have to assume size by speed and then you should use value from device descriptor. For example if I recall correctly, QEMU handle mouse and keyboard in way that it emulates them as full speed device, so device descriptor should be readed in one 18 bytes IN TD, however max control endpoint size in descriptor is 8, so you should read all other descriptors by 8 bytes long IN TD.

My notes agree that QEMU does emulate a mouse at both full- and high-speed. However, with that being said, may I reiterate, only the first request of the Device Descriptor should be a 64-byte request. Once you know the max_packet_size due to this request, all further requests must not be more than this max_packet_size, which in this example you give, must not be more than 8 bytes for this particular full speed device. (https://gitlab.com/qemu-project/qemu/-/ ... hid.c#L265)

To reiterate, on the UHCI/OHCI, the first request after the first reset must be an 8-byte request for low-speed devices and a 64-byte request for full-speed devices. All residual requests, even the request after the second reset, must be the size of the max_packet_size found, not including the last packet of the TD. (On a super-speed controller, when a super-speed device is found, you initially request 512 bytes. However, the super-speed controller will indicate if the device is high-speed or less, or a super-speed device before you request the Device Descriptor.)

1) send reset
2) send GET_DESCRIPTOR(USB_DT_DEVICE) request. (8 bytes for low-speed, 64-bytes for full-speed. There *must* be only one DATA packet within the TD. No more.)
3) send reset
4) send GET_DESCRIPTOR(USB_DT_DEVICE) request. (max_packet_size requests only. There can be more than one DATA packet in the TD.)
5) continue with retrieving requests and further enumeration

Hope this helps,
Ben
- https://www.fysnet.net/osdesign_book_series.htm

[Bonfra]

Thanks a lot to both of you for the help, mering all of what you said I managed to do some progress!
Now, I'm stuck a bit further on the negotiation process during the discovery of the device.
Expanding the list from before:

Code: Select all

- Reset the port
  - Set the PORT_RESET bit on PORTSC
  - Wait 50ms and clear the bit
  - Wait another 10ms for recovery time
  - Wait for the port to become enabled
    - Check for CONNECT_STATUS
    - Check for ENABLE_CHANGE | STATUS_CHANGE
    - Check for ENABLE
    - Set ENABLE, slep for 10ms, and repeat the process
- Discard the port if it doesn't have the CONNECT_STATUS bit set
- Get the first 8/64 bytes of the device descriptor (based on the speed) // It was stuck here before
- Reset the port again
- Get the full device descriptor
- Request to set the address

I'm stopping here since after setting the new address the device doesn't respond anymore to anything, for example, if I try to read again the standard descriptor now using the newly assigned address it never answers (seems like the same behavior I had before where it didn't answer to the first get descriptor request). Any other request doesn't work like reading the configuration descriptor and so on.
Just to be sure I checked if it still answered on address zero even if it should've just received a new address and no, it doesn't answer either.

Still, this is a perfectly working code on emulators... It's just real hw that reacts;(

This is the precise structure I'm sending to the device to set the address:

Code: Select all

setup packet:
  - type = USB_REQUEST_DIR_HOST_TO_DEVICE | USB_REQUEST_TYPE_STANDARD;
  - request = USB_REQUEST_SET_ADDRESS;
  - value = <new address>;
  - index = 0;
  - size = 0;

which creates two usb packets
first one:
  - type = USB_PACKET_TYPE_SETUP;
  - maxlen = 8;
  - buffer = <the 0x1000 physical address of the setup packet>;
  - toggle = 0;
second one:
  - type = USB_PACKET_TYPE_IN
  - maxlen = 0x800;
  - buffer = NULL;
  - toggle = 1;

UHCI TDs are then constructed based on the USB packets as discussed before. Should be alright reading various implementations online and looking at the specs... maybe I'm hitting some dumb edge case