* Reworked userland buffer access functionality into two functions in the Transfer class to not duplicate code in each HCD

* Use these functions in UHCI and EHCI which greatly reduces overhead on fragmented transfers as the vector conversion and area cloning is only done once
* Use the PrepareKernelAccess() function before submitting fragments which fixes the crash when trying to access the next fragments buffer in user memory from the kernel thread

This should fix bug #1167 which could be encountered when sending data to a mass storage device with an application that uses a large enough transfer buffer size.

git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@21059 a95241bf-73f2-0310-859d-f6bbb57e9c96

src/add-ons/kernel/bus_managers/usb/Transfer.cpp

@@ -17,6 +17,8 @@ Transfer::Transfer(Pipe *pipe)
 		fBaseAddress(NULL),
 		fFragmented(false),
 		fActualLength(0),
+		fUserArea(-1),
+		fClonedArea(-1),
 		fCallback(NULL),
 		fCallbackCookie(NULL),
 		fRequestData(NULL)
@@ -32,6 +34,9 @@ Transfer::~Transfer()
 
 	if (fVector && fVector != &fData)
 		delete[] fVector;
+
+	if (fClonedArea >= B_OK)
+		delete_area(fClonedArea);
 }
 
 
@@ -101,6 +106,72 @@ Transfer::AdvanceByFragment(size_t actualLength)
 }
 
 
+status_t
+Transfer::InitKernelAccess()
+{
+#ifndef HAIKU_TARGET_PLATFORM_HAIKU
+	// we might need to access a buffer in userspace. this will not
+	// be possible in the kernel space finisher thread unless we
+	// get the proper area id for the space we need and then clone it
+	// before reading from or writing to it.
+	iovec *vector = fVector;
+	for (size_t i = 0; i < fVectorCount; i++) {
+		if (IS_USER_ADDRESS(vector[i].iov_base)) {
+			fUserArea = area_for(vector[i].iov_base);
+			if (fUserArea < B_OK)
+				return B_BAD_ADDRESS;
+			break;
+		}
+	}
+
+	// no user area required for access
+	if (fUserArea < B_OK)
+		return B_OK;
+
+	area_info areaInfo;
+	if (fUserArea < B_OK || get_area_info(fUserArea, &areaInfo) < B_OK)
+		return B_BAD_ADDRESS;
+
+	for (size_t i = 0; i < fVectorCount; i++) {
+		(uint8 *)vector[i].iov_base -= (uint8 *)areaInfo.address;
+
+		if ((size_t)vector[i].iov_base > areaInfo.size
+			|| (size_t)vector[i].iov_base + vector[i].iov_len > areaInfo.size) {
+			TRACE_ERROR(("USB Transfer: data buffer spans across multiple areas!\n"));
+			return B_BAD_ADDRESS;
+		}
+	}
+#endif // !HAIKU_TARGET_PLATFORM_HAIKU
+
+	return B_OK;
+}
+
+
+status_t
+Transfer::PrepareKernelAccess()
+{
+#ifndef HAIKU_TARGET_PLATFORM_HAIKU
+	// done if there is no userspace buffer or if we already cloned its area
+	if (fUserArea < B_OK || fClonedArea >= B_OK)
+		return B_OK;
+
+	void *clonedMemory = NULL;
+	// we got a userspace buffer, need to clone the area for that
+	// space first and map the iovecs to this cloned area.
+	fClonedArea = clone_area("userspace accessor", &clonedMemory,
+		B_ANY_ADDRESS, B_WRITE_AREA | B_KERNEL_WRITE_AREA, fUserArea);
+
+	if (fClonedArea < B_OK)
+		return fClonedArea;
+
+	for (size_t i = 0; i < fVectorCount; i++)
+		(uint8 *)fVector[i].iov_base += (addr_t)clonedMemory;
+#endif // !HAIKU_TARGET_PLATFORM_HAIKU
+
+	return B_OK;
+}
+
+
 void
 Transfer::SetCallback(usb_callback_func callback, void *cookie)
 {

src/add-ons/kernel/bus_managers/usb/usb_p.h

@@ -533,6 +533,9 @@ public:
 		bool						IsFragmented() { return fFragmented; };
 		void						AdvanceByFragment(size_t actualLength);
 
+		status_t					InitKernelAccess();
+		status_t					PrepareKernelAccess();
+
 		void						SetCallback(usb_callback_func callback,
 										void *cookie);
 
@@ -547,6 +550,8 @@ private:
 		void						*fBaseAddress;
 		bool						fFragmented;
 		size_t						fActualLength;
+		area_id						fUserArea;
+		area_id						fClonedArea;
 
 		usb_callback_func			fCallback;
 		void						*fCallbackCookie;

src/add-ons/kernel/busses/usb/ehci.cpp

@@ -835,6 +835,10 @@ EHCI::AddPendingTransfer(Transfer *transfer, ehci_qh *queueHead,
 	if (!data)
 		return B_NO_MEMORY;
 
+	status_t result = transfer->InitKernelAccess();
+	if (result < B_OK)
+		return result;
+
 	data->transfer = transfer;
 	data->queue_head = queueHead;
 	data->data_descriptor = dataDescriptor;
@@ -842,49 +846,6 @@ EHCI::AddPendingTransfer(Transfer *transfer, ehci_qh *queueHead,
 	data->incoming = directionIn;
 	data->link = NULL;
 
-#ifndef HAIKU_TARGET_PLATFORM_HAIKU
-	if (directionIn) {
-		// we might need to access a buffer in userspace. this will not
-		// be possible in the kernel space finisher thread unless we
-		// get the proper area id for the space we need and then clone it
-		// before writing to it. this is of course terribly inefficient...
-		iovec *vector = transfer->Vector();
-		size_t vectorCount = transfer->VectorCount();
-		for (size_t i = 0; i < vectorCount; i++) {
-			if (IS_USER_ADDRESS(vector[i].iov_base)) {
-				data->user_area = area_for(vector[i].iov_base);
-				if (data->user_area < B_OK) {
-					TRACE_ERROR(("usb_ehci: failed to get area of userspace buffer\n"));
-					delete data;
-					return B_BAD_ADDRESS;
-				}
-
-				break;
-			}
-		}
-
-		if (data->user_area >= B_OK) {
-			area_info areaInfo;
-			if (get_area_info(data->user_area, &areaInfo) < B_OK) {
-				TRACE_ERROR(("usb_ehci: failed to get info about user area\n"));
-				delete data;
-				return B_BAD_ADDRESS;
-			}
-
-			for (size_t i = 0; i < vectorCount; i++) {
-				(uint8 *)vector[i].iov_base -= (uint8 *)areaInfo.address;
-
-				if ((size_t)vector[i].iov_base > areaInfo.size
-					|| (size_t)vector[i].iov_base + vector[i].iov_len > areaInfo.size) {
-					TRACE_ERROR(("usb_ehci: output data buffer spans across multiple areas!\n"));
-					delete data;
-					return B_BAD_ADDRESS;
-				}
-			}
-		}
-	}
-#endif // !HAIKU_TARGET_PLATFORM_HAIKU
-
 	if (!Lock()) {
 		delete data;
 		return B_ERROR;
@@ -1053,36 +1014,11 @@ EHCI::FinishTransfers()
 						// data to read out
 						iovec *vector = transfer->transfer->Vector();
 						size_t vectorCount = transfer->transfer->VectorCount();
-
+						transfer->transfer->PrepareKernelAccess();
-#ifndef HAIKU_TARGET_PLATFORM_HAIKU
-						area_id clonedArea = -1;
-						void *clonedMemory = NULL;
-						if (transfer->user_area >= B_OK) {
-							// we got a userspace output buffer, need to clone
-							// the area for that space first and map the iovecs
-							// to this cloned area.
-							clonedArea = clone_area("userspace accessor",
-								&clonedMemory, B_ANY_ADDRESS,
-								B_WRITE_AREA | B_KERNEL_WRITE_AREA,
-								transfer->user_area);
-
-							for (size_t i = 0; i < vectorCount; i++)
-								(uint8 *)vector[i].iov_base += (addr_t)clonedMemory;
-						}
-#endif // !HAIKU_TARGET_PLATFORM_HAIKU
-
 						actualLength = ReadDescriptorChain(
 							transfer->data_descriptor,
 							vector, vectorCount,
 							&nextDataToggle);
-
-#ifndef HAIKU_TARGET_PLATFORM_HAIKU
-						if (clonedArea >= B_OK) {
-							for (size_t i = 0; i < vectorCount; i++)
-								(uint8 *)vector[i].iov_base -= (addr_t)clonedMemory;
-							delete_area(clonedArea);
-						}
-#endif // !HAIKU_TARGET_PLATFORM_HAIKU
 					} else {
 						// calculate transfered length
 						actualLength = ReadActualLength(
@@ -1096,6 +1032,7 @@ EHCI::FinishTransfers()
 						transfer->transfer->AdvanceByFragment(actualLength);
 						if (transfer->transfer->VectorLength() > 0) {
 							FreeDescriptorChain(transfer->data_descriptor);
+							transfer->transfer->PrepareKernelAccess();
 							FillQueueWithData(transfer->transfer,
 								transfer->queue_head,
 								&transfer->data_descriptor, NULL);

src/add-ons/kernel/busses/usb/uhci.cpp

@@ -668,6 +668,10 @@ UHCI::AddPendingTransfer(Transfer *transfer, Queue *queue,
 	if (!data)
 		return B_NO_MEMORY;
 
+	status_t result = transfer->InitKernelAccess();
+	if (result < B_OK)
+		return result;
+
 	data->transfer = transfer;
 	data->queue = queue;
 	data->transfer_queue = transferQueue;
@@ -677,49 +681,6 @@ UHCI::AddPendingTransfer(Transfer *transfer, Queue *queue,
 	data->incoming = directionIn;
 	data->link = NULL;
 
-#ifndef HAIKU_TARGET_PLATFORM_HAIKU
-	if (directionIn) {
-		// we might need to access a buffer in userspace. this will not
-		// be possible in the kernel space finisher thread unless we
-		// get the proper area id for the space we need and then clone it
-		// before writing to it. this is of course terribly inefficient...
-		iovec *vector = transfer->Vector();
-		size_t vectorCount = transfer->VectorCount();
-		for (size_t i = 0; i < vectorCount; i++) {
-			if (IS_USER_ADDRESS(vector[i].iov_base)) {
-				data->user_area = area_for(vector[i].iov_base);
-				if (data->user_area < B_OK) {
-					TRACE_ERROR(("usb_uhci: failed to get area of userspace buffer\n"));
-					delete data;
-					return B_BAD_ADDRESS;
-				}
-
-				break;
-			}
-		}
-
-		if (data->user_area >= B_OK) {
-			area_info areaInfo;
-			if (get_area_info(data->user_area, &areaInfo) < B_OK) {
-				TRACE_ERROR(("usb_uhci: failed to get info about user area\n"));
-				delete data;
-				return B_BAD_ADDRESS;
-			}
-
-			for (size_t i = 0; i < vectorCount; i++) {
-				(uint8 *)vector[i].iov_base -= (uint8 *)areaInfo.address;
-
-				if ((size_t)vector[i].iov_base > areaInfo.size
-					|| (size_t)vector[i].iov_base + vector[i].iov_len > areaInfo.size) {
-					TRACE_ERROR(("usb_uhci: output data buffer spans across multiple areas!\n"));
-					delete data;
-					return B_BAD_ADDRESS;
-				}
-			}
-		}
-	}
-#endif // !HAIKU_TARGET_PLATFORM_HAIKU
-
 	if (!Lock()) {
 		delete data;
 		return B_ERROR;
@@ -840,35 +801,11 @@ UHCI::FinishTransfers()
 						iovec *vector = transfer->transfer->Vector();
 						size_t vectorCount = transfer->transfer->VectorCount();
 
-#ifndef HAIKU_TARGET_PLATFORM_HAIKU
+						transfer->transfer->PrepareKernelAccess();
-						area_id clonedArea = -1;
-						void *clonedMemory = NULL;
-						if (transfer->user_area >= B_OK) {
-							// we got a userspace output buffer, need to clone
-							// the area for that space first and map the iovecs
-							// to this cloned area.
-							clonedArea = clone_area("userspace accessor",
-								&clonedMemory, B_ANY_ADDRESS,
-								B_WRITE_AREA | B_KERNEL_WRITE_AREA,
-								transfer->user_area);
-
-							for (size_t i = 0; i < vectorCount; i++)
-								(uint8 *)vector[i].iov_base += (addr_t)clonedMemory;
-						}
-#endif // !HAIKU_TARGET_PLATFORM_HAIKU
-
 						actualLength = ReadDescriptorChain(
 							transfer->data_descriptor,
 							vector, vectorCount,
 							&lastDataToggle);
-
-#ifndef HAIKU_TARGET_PLATFORM_HAIKU
-						if (clonedArea >= B_OK) {
-							for (size_t i = 0; i < vectorCount; i++)
-								(uint8 *)vector[i].iov_base -= (addr_t)clonedMemory;
-							delete_area(clonedArea);
-						}
-#endif // !HAIKU_TARGET_PLATFORM_HAIKU
 					} else {
 						// read the actual length that was sent
 						actualLength = ReadActualLength(
@@ -886,6 +823,7 @@ UHCI::FinishTransfers()
 							TRACE(("usb_uhci: still %ld bytes left on transfer\n", transfer->transfer->VectorLength()));
 							// resubmit the advanced transfer so the rest
 							// of the buffers are transmitted over the bus
+							transfer->transfer->PrepareKernelAccess();
 							status_t result = CreateFilledTransfer(transfer->transfer,
 								&transfer->first_descriptor,
 								&transfer->transfer_queue);