Emulated USB Devices - Transfer Callbacks?

[hidenthusiast]

Hi all,

I've recently been inspired to attempt to Emulate USB devices for the Dolphin, and in particular the "Portal of Power" for the Skylanders games, heavily inspired by the work done in RPCS3. I've managed to make good progress on the Control Transfers in particular, where responses should occur instantaneously, however I am caught now trying to handle Interrupt transfers. As Interrupt Transfers take a while to process (approx. 22ms), these need to be handled Asynchronously, to allow other transfers to continue to occur as the game runs. RPCS3 has a 'Usb Handler Thread' where they handle responses to fake transfers made, but currently Dolphin only uses Libusb to handle real USB transfers, and these transfers have a Callback function which then processes the IPC reply. When I have tried to implement a Fake Transfer Thread, I get a segmentation faults or bus errors, presumably as I am trying to reference the TransferCommand out of scope or simply this Fake Transfer Thread cannot access the area of memory in which the Transfer Command exists.

I have an M1 Macbook Pro and a real Skylanders Trap Team portal, so I know what needs to be returned based on what protocol is being called, but any guidance on how/where to handle callbacks for Fake Transfers would be greatly appreciated! Or if anyone wants to try to implement something similar, maybe we can try to combine our powers and see what we can achieve...

The link to my fork is here, and I still need to implement a few other smarts around checking for whether a real device is connected or not, however with no real device attached the game believes that the real portal is attached, so that's a start!

[JosJuice]

Do you have a stack trace for one of these crashes you've been experiencing? If so, I could try to take a look and see if I can figure out why it's crashing.

[hidenthusiast]

Do you have a stack trace for one of these crashes you've been experiencing? If so, I could try to take a look and see if I can figure out why it's crashing.

Thanks for the quick response! I'm just playing around trying to get a proper call stack, but I also locally deleted the initial attempt I had made to handle these transfers (which is why it took me a while to reply while I restored it). The only thing I managed to pull out via LLDB was this line: 

Process 32700 stopped

* thread #38, name = 'Fake Transfer Thread', stop reason = EXC_BAD_ACCESS (code=1, address=0xbeaddc9be1d0)
    frame #0: 0x0000000100a18478 Dolphin`IOS::HLE::USB::SkylanderUsb::FakeTransferThread::Start(this=0x0000600014149e68)::$_16::operator()() const at Skylander.cpp:403:22
   400               ++iterator;
   401               continue;
   402             }
-> 403             command->OnTransferComplete(command->expected_count);
   404             iterator = m_transfers.erase(iterator);
   405           }
   406         }
Target 0: (Dolphin) stopped.

I've pushed the latest changes I made to my forked repo. I'm assuming LLDB is the correct tool to be using, but will need to learn how to get what I want a bit more. Is there a way to enable core dumping?

[JosJuice]

Your SkylanderUsb::FakeTransferThread::AddTransfer is broken.

The argument that is passed in is a unique_ptr. This unique_ptr owns the TransferCommand. When you call m_transfers.push_back, you're pushing a plain pointer to the TransferCommand. Then, once this function is done, the unique_ptr goes out of scope, which causes the TransferCommand to become deallocated. After this, you are no longer allowed to dereference that plain pointer in m_transfers.

If you want the FakeTransferThread to take ownership of the TransferCommand, you could store unique_ptrs in m_transfers instead of plain pointers.

[hidenthusiast]

Your SkylanderUsb::FakeTransferThread::AddTransfer is broken.

The argument that is passed in is a unique_ptr. This unique_ptr owns the TransferCommand. When you call m_transfers.push_back, you're pushing a plain pointer to the TransferCommand. Then, once this function is done, the unique_ptr goes out of scope, which causes the TransferCommand to become deallocated. After this, you are no longer allowed to dereference that plain pointer in m_transfers.

If you want the FakeTransferThread to take ownership of the TransferCommand, you could store unique_ptrs in m_transfers instead of plain pointers.

Gotcha, that makes sense. have made those changes and I'm getting the same message, albeit in a separate location this time.
* thread #39, name = 'Fake Transfer Thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x6b)

    frame #0: 0x0000000100a1c008 Dolphin`IOS::HLE::USB::SkylanderUsb::FakeTransferThread::Start(this=0x000060001414cda8)::$_13::operator()() const at Skylander.cpp:402:26
   399           {
   400             auto command = iterator->get();
   401             NOTICE_LOG_FMT(IOS_USB, "How many commands? {}", m_transfers.size());
-> 402             if (command->expected_time > timestamp){
   403               ++iterator;
   404               continue;
   405             }
Target 0: (Dolphin) stopped.

I've got the Start() method for the thread looking more like this now:

Code:

void SkylanderUsb::FakeTransferThread::Start()
{
 if (Core::WantsDeterminism())
   return;

 if (m_thread_running.TestAndSet())
 {
   m_thread = std::thread([this] {
     Common::SetCurrentThreadName("Fake Transfer Thread");
     while (m_thread_running.IsSet())
     {
       if (!m_transfers.empty()) {
         u64 timestamp = Common::Timer::NowUs();
         for (auto iterator = m_transfers.begin(); iterator != m_transfers.end();)
         {
           auto command = iterator->get();
           NOTICE_LOG_FMT(IOS_USB, "How many commands? {}", m_transfers.size());
           if (command->expected_time > timestamp){
             ++iterator;
             continue;
           }
           command->OnTransferComplete(command->expected_count);
           iterator = m_transfers.erase(iterator);
         }
       }
     }
   });
 }
}

Where from the SubmitTransfer methods I add the transfer as a unique pointer via std::move(cmd)

Will also upload those changes to my fork.

[hidenthusiast]

Slightly modified the iteration to now use the following:
for (auto iterator = m_transfers.begin(); iterator != m_transfers.end()
{
auto command = **iterator;
NOTICE_LOG_FMT(IOS_USB, "How many commands? {}", m_transfers.size());
if (command.expected_time > timestamp){
++iterator;
continue;
}
command.OnTransferComplete(command.expected_count);
iterator = m_transfers.erase(iterator);
NOTICE_LOG_FMT(IOS_USB, "Done");
}
And I am now getting this message:
* thread #36, name = 'Fake Transfer Thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x297)
frame #0: 0x00000001009b8e6c Dolphin`IOS::HLE::Request::Request(this=0x0000000175fc2ee8, (null)=0x000000000000028f) at Device.h:75:8
72 USB_ECANCELED = -7022, // USB OH0 insertion hook cancelled
73 };
74
-> 75 struct Request
76 {
77 u32 address = 0;
78 IPCCommandType command = IPC_CMD_OPEN;
Target 0: (Dolphin) stopped.

Presumably the ios_request field on the Request within the transfer command is out of scope?

[JosJuice]

auto command = **iterator; looks like object slicing to me. Try auto& command = **iterator; instead (or auto* command = iterator->get();, like you had in your previous post).

I'm kind of confused why the error message is pointing you to the line where the Request struct is defined, though.

[hidenthusiast]

auto command = **iterator; looks like object slicing to me. Try auto& command = **iterator; instead (or auto* command = iterator->get();, like you had in your previous post).

I'm kind of confused why the error message is pointing you to the line where the Request struct is defined, though.

Ah okay that makes sense, tried it with both options and it gave me the same issue as previous,
* thread #38, name = 'Fake Transfer Thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x2e8)

    frame #0: 0x0000000100a1bc94 Dolphin`IOS::HLE::USB::SkylanderUsb::FakeTransferThread::Start(this=0x000060001414f578)::$_13::operator()() const at Skylander.cpp:403:25
   400           {
   401              auto& command = **iterator;
   402             NOTICE_LOG_FMT(IOS_USB, "How many commands? {}", m_transfers.size());
-> 403             if (command.expected_time > timestamp)
   404             {
   405               NOTICE_LOG_FMT(IOS_USB, "Skipping");
   406               ++iterator;
Target 0: (Dolphin) stopped.
It makes it through a few loops, but when a 2nd Transfer is added to the list it shows the above error.

[JosJuice]

address=0x2e8 sounds like you dereferenced nullptr. I don't know why the command would be nullptr in this case though.

[hidenthusiast]

I'm thinking it could be how the iterator is working, as soon as it adds another request in while the iterator is looping, it crashes. It handles a couple of requests fine, but when there's an InterruptMessage as well as a ControlMessage in the list it messes up. I think when a new request is added while the iterator is within the m_transfers.begin() loop it must be deleting or something. Will look and see if I can make it smarter when it handles requests.