hrev52918 adds 3 changesets to branch 'master'
old head: 14263044aac31fb9b570850708a3c29a4f44f95d
new head: da8c1a9a403aed0fa06beeeb100c3c0c50b8362c
overview:
https://git.haiku-os.org/haiku/log/?qt=range&q=da8c1a9a403a+%5E14263044aac3
----------------------------------------------------------------------------
c409803d6f4a: XHCI: Refactoring and fixes to SubmitControlRequest.
* Rearrange some basic setup to be more towards the top of the function,
so we have less things to tear down upon failures.
* Don't double-initialize qwtrb0 of the Setup Stage (no functional change.)
* Rework Data Stage initialization considerably:
- TD_SIZE refers to the number of remaining TRBs in this TD. As we only
use 1 TRB for the Data Stage TD at present, this should be 0, not 1.
- Actually copy data for outbound transfers. (This code does get hit,
so I'm not sure how it wasn't a problem previously.)
* Initialize the first quadbit of the Status Stage. (Potentially a
functional change.)
* Set the TRB_3_DIR bit on the Status Stage correctly as per the spec.
(See inline comment.)
Device initialization seems to behave much more smoothly now; at least on
my hardware, the "error Parameter" doesn't happen anymore, and of course
anything depending on outbound Control transfers will now work correctly.
I now get much better speeds from usb_disk, but I still see usb_hid stalls
after this patch.
7b53b3c2e490: XHCI: Don't compute the error code twice.
No functional change.
da8c1a9a403a: USB: Don't loop endlessly waiting for a physical buffer.
This is used quite a lot in critical transfer paths, so we don't
want to lock things up if no buffers are available for whatever reason.
Wait 2 seconds, and if we didn't get anything by then, return B_NO_MEMORY.
Possibly fixes or helps with certain USB-related lockups.
[ Augustin Cavalier <waddlesplash@xxxxxxxxx> ]
----------------------------------------------------------------------------
2 files changed, 45 insertions(+), 35 deletions(-)
.../bus_managers/usb/PhysicalMemoryAllocator.cpp | 5 +-
src/add-ons/kernel/busses/usb/xhci.cpp | 75 +++++++++++---------
############################################################################
Commit: c409803d6f4a0577547c05938c6fecd1a43aec4a
URL: https://git.haiku-os.org/haiku/commit/?id=c409803d6f4a
Author: Augustin Cavalier <waddlesplash@xxxxxxxxx>
Date: Fri Feb 22 18:07:15 2019 UTC
XHCI: Refactoring and fixes to SubmitControlRequest.
* Rearrange some basic setup to be more towards the top of the function,
so we have less things to tear down upon failures.
* Don't double-initialize qwtrb0 of the Setup Stage (no functional change.)
* Rework Data Stage initialization considerably:
- TD_SIZE refers to the number of remaining TRBs in this TD. As we only
use 1 TRB for the Data Stage TD at present, this should be 0, not 1.
- Actually copy data for outbound transfers. (This code does get hit,
so I'm not sure how it wasn't a problem previously.)
* Initialize the first quadbit of the Status Stage. (Potentially a
functional change.)
* Set the TRB_3_DIR bit on the Status Stage correctly as per the spec.
(See inline comment.)
Device initialization seems to behave much more smoothly now; at least on
my hardware, the "error Parameter" doesn't happen anymore, and of course
anything depending on outbound Control transfers will now work correctly.
I now get much better speeds from usb_disk, but I still see usb_hid stalls
after this patch.
----------------------------------------------------------------------------
diff --git a/src/add-ons/kernel/busses/usb/xhci.cpp
b/src/add-ons/kernel/busses/usb/xhci.cpp
index 3831098c57..ad8f0014f5 100644
--- a/src/add-ons/kernel/busses/usb/xhci.cpp
+++ b/src/add-ons/kernel/busses/usb/xhci.cpp
@@ -618,62 +618,69 @@ XHCI::SubmitControlRequest(Transfer *transfer)
usb_request_data *requestData = transfer->RequestData();
bool directionIn = (requestData->RequestType & USB_REQTYPE_DEVICE_IN)
!= 0;
+ xhci_endpoint *endpoint = (xhci_endpoint *)pipe->ControllerCookie();
+ uint8 id = XHCI_ENDPOINT_ID(pipe);
+ if (id >= XHCI_MAX_ENDPOINTS) {
+ TRACE_ERROR("Invalid Endpoint");
+ return B_BAD_VALUE;
+ }
+ status_t status = transfer->InitKernelAccess();
+ if (status != B_OK)
+ return status;
+
TRACE("SubmitControlRequest() length %d\n", requestData->Length);
- xhci_td *setupDescriptor = CreateDescriptor(requestData->Length);
+ xhci_td *descriptor = CreateDescriptor(requestData->Length);
+ descriptor->transfer = transfer;
- // set SetupStage
+ // Setup Stage
uint8 index = 0;
- setupDescriptor->trbs[index].qwtrb0 = 0;
- setupDescriptor->trbs[index].dwtrb2 = TRB_2_IRQ(0) | TRB_2_BYTES(8);
- setupDescriptor->trbs[index].dwtrb3
+ memcpy(&descriptor->trbs[index].qwtrb0, requestData,
+ sizeof(usb_request_data));
+ descriptor->trbs[index].dwtrb2 = TRB_2_IRQ(0) | TRB_2_BYTES(8);
+ descriptor->trbs[index].dwtrb3
= B_HOST_TO_LENDIAN_INT32(TRB_3_TYPE(TRB_TYPE_SETUP_STAGE)
| TRB_3_IDT_BIT | TRB_3_CYCLE_BIT);
if (requestData->Length > 0) {
- setupDescriptor->trbs[index].dwtrb3 |= B_HOST_TO_LENDIAN_INT32(
+ descriptor->trbs[index].dwtrb3 |= B_HOST_TO_LENDIAN_INT32(
directionIn ? TRB_3_TRT_IN : TRB_3_TRT_OUT);
}
- memcpy(&setupDescriptor->trbs[index].qwtrb0, requestData,
- sizeof(usb_request_data));
index++;
+ // Data Stage (if any)
if (requestData->Length > 0) {
- // set DataStage if any
- setupDescriptor->trbs[index].qwtrb0 =
setupDescriptor->buffer_phy[0];
- setupDescriptor->trbs[index].dwtrb2 = TRB_2_IRQ(0)
+ descriptor->trbs[index].qwtrb0 = descriptor->buffer_phy[0];
+ descriptor->trbs[index].dwtrb2 = TRB_2_IRQ(0)
| TRB_2_BYTES(requestData->Length)
- | TRB_2_TD_SIZE(transfer->VectorCount());
- setupDescriptor->trbs[index].dwtrb3 = B_HOST_TO_LENDIAN_INT32(
+ | TRB_2_TD_SIZE(0);
+ descriptor->trbs[index].dwtrb3 = B_HOST_TO_LENDIAN_INT32(
TRB_3_TYPE(TRB_TYPE_DATA_STAGE)
| (directionIn ? (TRB_3_DIR_IN | TRB_3_ISP_BIT)
: 0)
| TRB_3_CYCLE_BIT);
- // TODO copy data for out transfers
+ if (!directionIn) {
+ transfer->PrepareKernelAccess();
+ memcpy(descriptor->buffer_log[0],
+ (uint8 *)transfer->Vector()[0].iov_base,
requestData->Length);
+ }
+
index++;
}
- // set StatusStage
- setupDescriptor->trbs[index].dwtrb2 = TRB_2_IRQ(0);
- setupDescriptor->trbs[index].dwtrb3 = B_HOST_TO_LENDIAN_INT32(
+ // Status Stage
+ descriptor->trbs[index].qwtrb0 = 0;
+ descriptor->trbs[index].dwtrb2 = TRB_2_IRQ(0);
+ descriptor->trbs[index].dwtrb3 = B_HOST_TO_LENDIAN_INT32(
TRB_3_TYPE(TRB_TYPE_STATUS_STAGE)
- | ((directionIn && requestData->Length > 0) ? 0 :
TRB_3_DIR_IN)
- | TRB_3_IOC_BIT | TRB_3_CYCLE_BIT);
+ | TRB_3_DIR_IN | TRB_3_IOC_BIT | TRB_3_CYCLE_BIT);
+ // Note that TRB_3_DIR on a TRB_TYPE_STATUS_STAGE does not
refer to the
+ // Data Stage, but rather to which way the status notification
should
+ // go, i.e. device to host, or host to device. (XHCI 1.1 §
4.11.2.2 p205.)
- setupDescriptor->trb_count = index + 1;
+ descriptor->trb_count = index + 1;
- xhci_endpoint *endpoint = (xhci_endpoint *)pipe->ControllerCookie();
- uint8 id = XHCI_ENDPOINT_ID(pipe);
- if (id >= XHCI_MAX_ENDPOINTS) {
- TRACE_ERROR("Invalid Endpoint");
- return B_BAD_VALUE;
- }
- status_t status = transfer->InitKernelAccess();
- if (status != B_OK)
- return status;
-
- setupDescriptor->transfer = transfer;
- status = _LinkDescriptorForPipe(setupDescriptor, endpoint);
+ status = _LinkDescriptorForPipe(descriptor, endpoint);
if (status != B_OK)
return status;
TRACE("SubmitControlRequest() request linked\n");
############################################################################
Commit: 7b53b3c2e49005cd02ca61c1f39d340267d52760
URL: https://git.haiku-os.org/haiku/commit/?id=7b53b3c2e490
Author: Augustin Cavalier <waddlesplash@xxxxxxxxx>
Date: Fri Feb 22 18:26:10 2019 UTC
XHCI: Don't compute the error code twice.
No functional change.
----------------------------------------------------------------------------
diff --git a/src/add-ons/kernel/busses/usb/xhci.cpp
b/src/add-ons/kernel/busses/usb/xhci.cpp
index ad8f0014f5..efbd063dcb 100644
--- a/src/add-ons/kernel/busses/usb/xhci.cpp
+++ b/src/add-ons/kernel/busses/usb/xhci.cpp
@@ -2153,9 +2153,9 @@ XHCI::DoCommand(xhci_trb* trb)
uint32 completionCode = TRB_2_COMP_CODE_GET(fCmdResult[0]);
TRACE("Command Complete. Result: %" B_PRId32 "\n", completionCode);
if (completionCode != COMP_SUCCESS) {
- uint32 errorCode = TRB_2_COMP_CODE_GET(fCmdResult[0]);
TRACE_ERROR("unsuccessful command %" B_PRId32 ", error %s (%"
B_PRId32 ")\n",
- TRB_3_TYPE_GET(trb->dwtrb3),
xhci_error_string(errorCode), errorCode);
+ TRB_3_TYPE_GET(trb->dwtrb3),
xhci_error_string(completionCode),
+ completionCode);
status = B_IO_ERROR;
}
############################################################################
Revision: hrev52918
Commit: da8c1a9a403aed0fa06beeeb100c3c0c50b8362c
URL: https://git.haiku-os.org/haiku/commit/?id=da8c1a9a403a
Author: Augustin Cavalier <waddlesplash@xxxxxxxxx>
Date: Fri Feb 22 18:29:40 2019 UTC
USB: Don't loop endlessly waiting for a physical buffer.
This is used quite a lot in critical transfer paths, so we don't
want to lock things up if no buffers are available for whatever reason.
Wait 2 seconds, and if we didn't get anything by then, return B_NO_MEMORY.
Possibly fixes or helps with certain USB-related lockups.
----------------------------------------------------------------------------
diff --git a/src/add-ons/kernel/bus_managers/usb/PhysicalMemoryAllocator.cpp
b/src/add-ons/kernel/bus_managers/usb/PhysicalMemoryAllocator.cpp
index 2bc3db2402..a1f09e0167 100644
--- a/src/add-ons/kernel/bus_managers/usb/PhysicalMemoryAllocator.cpp
+++ b/src/add-ons/kernel/bus_managers/usb/PhysicalMemoryAllocator.cpp
@@ -222,7 +222,10 @@ PhysicalMemoryAllocator::Allocate(size_t size, void
**logicalAddress,
TRACE_ERROR(("PMA: found no free slot to store %ld bytes,
waiting\n",
size));
- entry.Wait();
+ if (entry.Wait(B_RELATIVE_TIMEOUT, 2 * 1000 * 1000) ==
B_TIMED_OUT) {
+ TRACE_ERROR(("PMA: timed out waiting for a free slot,
giving up\n"));
+ break;
+ }
if (!_Lock())
return B_ERROR;
