Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

vhci - multiple interfaces, "usbip install" feature #90

Closed

Conversation

koudis
Copy link
Collaborator

@koudis koudis commented Jan 4, 2020

PR which repair some bug and improve support for composite interfaces. Support for usbip install feature

I had problems with usb devices which has many (>10) interfaces.

There were several problems in HUB implementation:

  • IoCancelIrp - bug in Dpc release lock
  • ABORT_PIPE request was not implemented correctly.
  • RESET_PIPE_AND_CLEAR_STALL was not implemented correctly against USBIP server hosted on linux machine. I guess that RESET_PIPE_AND_CLEAR_STALL works fine for USBIP server hosted on Windows machine.
  • CONTROL_TRANSFER_EX was NOT implement

Problems in usbip.exe userspace program:

  • I updated usbip_forward routine --> no deadlock, segfaults or BSOD is now possible (I tested it for vhci not for stub driver)
  • Change process priority to HIGH for attach command

I also merge StoyanDimitrov changes to my brach (due to small update in README.md).

After the PR test and approve I will merge all commits into one.

@koudis koudis changed the title Jan kubalek/vhci interfaces vhci - multiple interfaces, usbip install feature Jan 4, 2020
@cezanne
Copy link
Owner

cezanne commented Jan 31, 2020

@koudis : sorry for my long silence. I'll investigate your great work and try to merge.

@cezanne
Copy link
Owner

cezanne commented Feb 1, 2020

@koudis : koudis_install branch is created from your commits(02cef87 .. 45d3f1d). I slightly modified your commit to remove multiple blank lines and usbip_install_usage typo error. I know that the typo error was resolved by your recent commit.

Please check a merged commit ( cefbf94 )

Now, I would like to merge remaining your work related to a multiple interfaces issue.

NOTE: I have amended the old commit (55c8591) in order to apply your improvement for README.md, usbip.c and usbip_install.c.

@koudis
Copy link
Collaborator Author

koudis commented Feb 1, 2020

Just warning: commits between f974e8e and the HEAD of my branch is not many useful :).

Just for sure - there are also some changes for Install feature which is not between 02cef87 .. 45d3f1d

But it seems that you already merged these commits into koudis_instal :).

I will test it tomorrow. (2. 2. or 3. 2.)

@cezanne cezanne closed this Feb 1, 2020
@cezanne cezanne reopened this Feb 1, 2020
@cezanne
Copy link
Owner

cezanne commented Feb 1, 2020

@koudis : I've just closed by mistake and reopened.
I have tried to upload 2 commits from your works (one for usbip.exe and another for vhci driver).
Indeed, your commits are too~ long. 😄

Just warning: commits between f974e8e and the HEAD of my branch is not many useful :).

Just for sure - there are also some changes for Install feature which is not between 02cef87 .. 45d3f1d

But it seems that you already merged these commits into koudis_instal :).

I will test it tomorrow. (2. 2. or 3. 2.)

@koudis
Copy link
Collaborator Author

koudis commented Feb 1, 2020

@cezuni I thought that we merge both features together. So I did not bother about "nice" commits.

If the problem with messy history occurred let me know and I will try it repair shortly :) .

driver/vhci/usb.ids Show resolved Hide resolved
driver/vhci/usbip_vhci.vcxproj Show resolved Hide resolved
@koudis
Copy link
Collaborator Author

koudis commented Feb 3, 2020

@cezuni i test koudis_install branch and create #98 . There was smal bugfix due to missing deps for linker.

cezanne added a commit that referenced this pull request Feb 7, 2020
The inconsitent vhci lock_urbr release routine is fixed.
Spin lock acquired by KeAcquireSpinLockAtDpcLevel() can be safely
released by KeReleaseSpinLockFromDpcLevel().
This commit got hint from #90.
@cezanne
Copy link
Owner

cezanne commented Feb 7, 2020

@koudis : Please check 5530943 in master branch. KeReleaseSpinLock is replaced with KeReleaseSpinLockFromDpcLevel. In 7990030, you gave a hint. Thanks.
I think that KeReleaseSpinLockFromDpcLevel would be more desirable than KeReleaseSpinLockForDpc. Isn't it?

@koudis
Copy link
Collaborator Author

koudis commented Feb 7, 2020

@cezuni Yes, I've read documentation once again and the doc explicitly says
"The caller should release the spin lock with KeReleaseSpinLockFromDpcLevel as quickly as possible."

Your commit looks more correct according to doc.

cezanne added a commit that referenced this pull request Feb 10, 2020
This commit is derived from #90 by @koudis
- Add debugging messages for URB FUNCTION codes
- Add useful messages for vhci read/write routine
cezanne added a commit that referenced this pull request Feb 10, 2020
This commit is inspired by @koudis(from #90)
- complete pending IRP's and remove urbr's of an aborted pipe
- resetting pipe is enhanced
@cezanne
Copy link
Owner

cezanne commented Feb 10, 2020

@koudis : I created abort_pipe branch and PR #101 in order to reviewing vhci_ioctl_abort_pipe and store_urb_reset_pipe. How about moving on #101?

@koudis
Copy link
Collaborator Author

koudis commented Feb 11, 2020

@cezuni ok :).

Are you going to merge "URB_FUNCTION_CONTROL_TRANSFER_EX" into master?

If not I suggest to test master branch agains multiple inteface devices like Logitech Unyfing device. I had problem with interface initialization without this call (but for real I am not sure if TRANFER_EX it's really necessary).

@koudis
Copy link
Collaborator Author

koudis commented Feb 11, 2020

Maybe we can consider to check URB_FUNCTION_CONTROL_TRANSFER too due to #102

@cezanne
Copy link
Owner

cezanne commented Feb 11, 2020

Are you going to merge "URB_FUNCTION_CONTROL_TRANSFER_EX" into master?

Yes, sure. But after #101.

If not I suggest to test master branch agains multiple inteface devices like Logitech Unyfing device. I had problem with interface initialization without this call (but for real I am not sure if TRANFER_EX it's really necessary).

I'll keep in mind. 😄

cezanne added a commit that referenced this pull request Feb 20, 2020
This commit is inspired by @koudis(from #90)
- complete pending IRP's and remove urbr's of an aborted pipe
- resetting pipe is enhanced
cezanne added a commit that referenced this pull request Feb 22, 2020
This commit is inspired and helped by @koudis(from #90)
- complete pending IRP's and remove urbr's of an aborted pipe
- correct a clearing pipe routine
@koudis koudis closed this Mar 13, 2020
@koudis koudis deleted the jan_kubalek/vhci_interfaces branch March 13, 2020 21:25
@koudis koudis restored the jan_kubalek/vhci_interfaces branch March 14, 2020 13:42
@koudis koudis deleted the jan_kubalek/vhci_interfaces branch July 5, 2020 14:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants