LekKit
commented
6 months ago So according to ic2-hid implementation if input_available() is called twice, read_report() should be called twice by the I2C-HID controller by holding an interrupt until the report queue is drained - which should have allowed my hid_keyboard fix to work, but it doesn't exactly work on Linux guest.
If we immediately call input_available() twice, sometimes second read_report() call is made with huge (3-4 seconds) delay, and in between those calls the guest believes that the button is still pressed and performs repeated key input, etc.
What's even more intriguing is that interrupt line of I2C-HID device is asserted all this time and not lowered until guest reads the second report. It's not like the guest is overrun with interrupts either - numbers in /proc/interrupts are around 10-100 mark, not gazzilions. So it must have ignore the raised interrupt of I2C-HID which signals there are more reports to be read, and only comes back after a huge delay.
I tried to test same VM with Haiku guest, no problems at all. Guest reads both reports immediately and registers a key click.
I went further and put repeated plic_send_irq() each 10ms into I2C OC controller to make sure that device doesn't have any interrupts that guest might wait for (There was a bug a while ago, because it seems Linux expects more interrupt cases from this device than there are documented, like an interrupt on transaction stop). It fixed the issue! But ehm, I am not sure I2C controller is at fault... Any operation it could have done already signals an interrupt, I tried to send IRQ on any CRSR write except when it actually clears interrupts and nothing changed.
So here are possible causes of this particular bug
- Something to report in I2C-HID registers to make Linux know there are more reports in queue, Haiku doesn't need that "something" tho
- A bug in I2C OC controller (Highly unlikely)
- A bug in... PLIC?
I will go have a look at how PLIC handles level-triggered IRQs as I have some second thoughts here.
@X547 can you give me any available pieces of documentation about I2C HID you have used / have access to? Seems problematic for me to find. It might be a good idea to have doc links around somewhere.
X547
Converted issue; See https://github.com/LekKit/RVVM/issues/126#issuecomment-1981278618
Older issue
### Steps to reproduce - Have a very slow guest (`rvvm_set_opt(machine, RVVM_OPT_MAX_CPU_CENT, 1)` or immediately call `hid_keyboard_release()`) - Some key presses will be lost when typing - Same possibly applies to hid_mouse ### Investigation - Press/release API calls just mark/unmark a bit in pressed keys bitmask, then mark that input is available on HID device - Guest reads input report, which enumerates pressed keys - if release() happens before this, no pressed keys are reported ### Suggested fix My initial idea is to have two key bitmasks: `keys_report` and `keys_pressed`. - On `hid_keyboard_press()`, a bit is set in both bitmasks; - On `hid_keyboard_release()`, a bit is cleared only in `keys_pressed` bitmask. - When guest is reading input report, a key should be reported as pressed if it's bit is set in **either** bitmasks. - Then, if the bit is actually cleared in `keys_pressed`, then that means we now should replay a key release event, so `input_avail()` should be called. Finally, the bit in `keys_report` is cleared. The problem is, `input_avail()` locking causes recursive deadlock inside `i2c-hid` when calling from `read_report` handler. This needs further investigation - I see there is some queueing mechanism available inside `i2c-hid`, so maybe we can actually queue input reports instead, but haven't read into that code much yet. @X547 perhaps you can point me in the right direction? I am wondering if HID reports should be just queued as packets and not call back into the device `read_report` handler, this would also allow copy-paste into `hid_keyboard` beyond just fixing this issue. In the mean time I'll be polishing network code for a new stable release, and will be back to this issue soon