Turn on RAW_IO on Windows

[wangxiaochuTHU]

For queue transfers on IN endpoints

[kevinmehall]

As I noted on your prior PR, I'm not sure if this is safe in edge cases.

While users shouldn't, and have no reason to, create multiple Queues for the same endpoint, or create a Queue while individual TransferFutures are pending, there's nothing stopping one from doing so with the current API, so it's possible for this to be called with other transfers pending on the endpoint.

@martinling In the libusb discussions around RAW_IO everyone seemed to think that WinUsb_SetPipePolicy was not allowed while there are pending transfers. As far as I've seen, the Microsoft documentation doesn't say one way or another. Do you know of any documentation on this, or have you encountered problems doing it? I am specifically looking for anything suggesting that it is forbidden as in Rust unsafe -- even returning an error or causing pending transfers to fail would be fine here. So it requires a much weaker guarantee than libusb was looking for when wanting to set it on a transfer-by-transfer basis.

In my limited testing of this branch on Windows 10, it seems to work fine:

    let bulk_in_pending = interface.bulk_in(0x81, RequestBuffer::new(16001));
    let mut queue = interface.bulk_in_queue(0x81);
    queue.submit(RequestBuffer::new(64));
    block_on(bulk_in_pending).into_result().unwrap();
    block_on(queue.next_complete()).into_result().unwrap();

[2024-06-01T20:27:12Z DEBUG nusb::platform::windows_winusb::transfer] Submit transfer 0x1ea3881f9e0 on endpoint 81 for 16001 bytes IN
[2024-06-01T20:27:12Z DEBUG nusb::platform::windows_winusb::device] WinUsb_SetPipePolicy succeeded to enable RAW_IO on endpoint 0x81
[2024-06-01T20:27:12Z DEBUG nusb::platform::windows_winusb::transfer] Submit transfer 0x1ea3881fd70 on endpoint 81 for 64 bytes IN
[2024-06-01T20:27:12Z DEBUG nusb::platform::windows_winusb::transfer] Handling completion for transfer 0x1ea3881fd70
[2024-06-01T20:27:12Z DEBUG nusb::platform::windows_winusb::transfer] Handling completion for transfer 0x1ea3881f9e0
[2024-06-01T20:27:12Z DEBUG nusb::platform::windows_winusb::transfer] Transfer 0x1ea3881f9e0 on endpoint 81 complete: 16001 bytes transferred 
[2024-06-01T20:27:12Z DEBUG nusb::platform::windows_winusb::transfer] Transfer 0x1ea3881fd70 on endpoint 81 complete: 64 bytes transferred

I also considered just unconditionally enabling RAW_IO for all IN endpoints when opening the interface, before anyone could have submitted transfers. The documentation already notes "The requested length must be a multiple of the endpoint’s maximum packet size" with the intention of avoiding overflows, so this should be fine, however I should have enforced that in code. Probe-rs is submitting odd-sized reads via the TransferFuture API in at least one place.

Really, I should just go ahead with the breaking change and design the v0.2 transfer interface that guarantees exclusive access, but if this is reasonably safe in that edge case I don't want to block this indefinitely like libusb.

[martinling]

@kevinmehall

As far as I've seen, the Microsoft documentation doesn't say one way or another. Do you know of any documentation on this, or have you encountered problems doing it?

I don't have anything specific. It was more a case of thinking that behind the scenes, this clearly switches on or off some DMA stuff in the driver, and it would be very tricky for the OS to do that safely whilst existing transfers are in flight. What happens when you have an odd-size transfer in flight when the policy is changed? Maybe it just defers the change until the next transfer starts - but what if there is more than one odd-size transfer already queued up? So it'd need to do something like putting a special entry in the transfer queue saying "when you get to here, turn the DMA stuff on" and defer that work to that point.

And that all just seemed like a lot of work, that nobody would bother to do unless there was a very specific need for it, and surely if they had gone to all that trouble someone would at least note in the documentation that this was allowed and handled properly.

From experience, it seemed much more Microsoft-like to me that they would just implicitly expect programs to only call SetPipePolicy before making transfers, and not really document that assumption.

What also came up in the libusb discussions was that there is already this comment in the libusb backend code, noting that calling WinUsb_SetPipePolicy to change the SHORT_PACKET_TERMINATE option can cause problems with transfers in flight.

That comment was added by @hjelmn in this commit; perhaps he might be able to comment on his findings.

In general the feeling was that in the absence of information confirming it was safe to change policy with transfers in flight, it was better to just assume that it isn't. I certainly wouldn't want to guarantee it was safe in the Rust sense of the word, particularly for something that's messing with DMA.