- Add synchronization on input and output buffers

- Respect timeouts on IRP_MJ_READ
- Get right buffer in read/write routines

svn path=/trunk/; revision=14297

reactos/drivers/dd/serial/devctrl.c

@@ -215,14 +215,23 @@ SerialGetCommStatus(
 	OUT PSERIAL_STATUS pSerialStatus,
 	IN PSERIAL_DEVICE_EXTENSION DeviceExtension)
 {
+	KIRQL Irql;
+	
 	RtlZeroMemory(pSerialStatus, sizeof(SERIAL_STATUS));
 	
 	pSerialStatus->Errors = 0; /* FIXME */
 	pSerialStatus->HoldReasons = 0; /* FIXME */
+	
+	KeAcquireSpinLock(&DeviceExtension->InputBufferLock, &Irql);
 	pSerialStatus->AmountInInQueue = (DeviceExtension->InputBuffer.WritePosition + DeviceExtension->InputBuffer.Length
 		- DeviceExtension->InputBuffer.ReadPosition) % DeviceExtension->InputBuffer.Length;
+	KeReleaseSpinLock(&DeviceExtension->InputBufferLock, Irql);
+	
+	KeAcquireSpinLock(&DeviceExtension->OutputBufferLock, &Irql);
 	pSerialStatus->AmountInOutQueue = (DeviceExtension->OutputBuffer.WritePosition + DeviceExtension->OutputBuffer.Length
 		- DeviceExtension->OutputBuffer.ReadPosition) % DeviceExtension->OutputBuffer.Length;
+	KeReleaseSpinLock(&DeviceExtension->OutputBufferLock, Irql);
+	
 	pSerialStatus->EofReceived = FALSE; /* FIXME */
 	pSerialStatus->WaitForImmediate = FALSE; /* FIXME */
 	
@@ -497,8 +506,10 @@ SerialDeviceControl(
 		}
 		case IOCTL_SERIAL_PURGE:
 		{
+			KIRQL Irql1, Irql2;
 			DPRINT("Serial: IOCTL_SERIAL_PURGE\n");
-			/* FIXME: lock input and output queues */
+			KeAcquireSpinLock(&DeviceExtension->InputBufferLock, &Irql1);
+			KeAcquireSpinLock(&DeviceExtension->OutputBufferLock, &Irql2);
 			DeviceExtension->InputBuffer.ReadPosition = DeviceExtension->InputBuffer.WritePosition = 0;
 			DeviceExtension->OutputBuffer.ReadPosition = DeviceExtension->OutputBuffer.WritePosition = 0;
 			/* Clear receive/transmit buffers */
@@ -507,7 +518,8 @@ SerialDeviceControl(
 				WRITE_PORT_UCHAR(SER_FCR(ComPortBase),
 					SR_FCR_CLEAR_RCVR | SR_FCR_CLEAR_XMIT);
 			}
-			/* FIXME: unlock input and output queues */
+			KeReleaseSpinLock(&DeviceExtension->OutputBufferLock, Irql2);
+			KeReleaseSpinLock(&DeviceExtension->InputBufferLock, Irql1);
 			Status = STATUS_SUCCESS;
 			break;
 		}
@@ -618,18 +630,19 @@ SerialDeviceControl(
 				return STATUS_INVALID_PARAMETER;
 			else
 			{
+				KIRQL Irql;
 				Status = STATUS_SUCCESS;
 				if (((PSERIAL_QUEUE_SIZE)Buffer)->InSize > DeviceExtension->InputBuffer.Length)
 				{
-					/* FIXME: lock input queue */
+					KeAcquireSpinLock(&DeviceExtension->InputBufferLock, &Irql);
 					Status = IncreaseCircularBufferSize(&DeviceExtension->InputBuffer, ((PSERIAL_QUEUE_SIZE)Buffer)->InSize);
-					/* FIXME: unlock input queue */
+					KeReleaseSpinLock(&DeviceExtension->InputBufferLock, Irql);
 				}
 				if (NT_SUCCESS(Status) && ((PSERIAL_QUEUE_SIZE)Buffer)->OutSize > DeviceExtension->OutputBuffer.Length)
 				{
-					/* FIXME: lock output queue */
+					KeAcquireSpinLock(&DeviceExtension->OutputBufferLock, &Irql);
 					Status = IncreaseCircularBufferSize(&DeviceExtension->OutputBuffer, ((PSERIAL_QUEUE_SIZE)Buffer)->OutSize);
-					/* FIXME: unlock output queue */
+					KeReleaseSpinLock(&DeviceExtension->OutputBufferLock, Irql);
 				}
 			}
 			break;

reactos/drivers/dd/serial/misc.c

@@ -60,6 +60,67 @@ ForwardIrpAndForget(
 	return IoCallDriver(LowerDevice, Irp);
 }
 
+VOID STDCALL
+SerialReceiveByte(
+	IN PKDPC Dpc,
+	IN PVOID pDeviceExtension, // real type PSERIAL_DEVICE_EXTENSION
+	IN PVOID pByte,            // real type UCHAR
+	IN PVOID Unused)
+{
+	PSERIAL_DEVICE_EXTENSION DeviceExtension;
+	UCHAR Byte;
+	KIRQL Irql;
+	NTSTATUS Status;
+	
+	DeviceExtension = (PSERIAL_DEVICE_EXTENSION)pDeviceExtension;
+	Byte = (UCHAR)(ULONG_PTR)pByte;
+	DPRINT1("Serial: received byte on COM%lu: 0x%02x (%c)\n",
+		DeviceExtension->ComPort, Byte, Byte);
+	
+	KeAcquireSpinLock(&DeviceExtension->InputBufferLock, &Irql);
+	Status = PushCircularBufferEntry(&DeviceExtension->InputBuffer, Byte);
+	if (!NT_SUCCESS(Status))
+	{
+		/* FIXME: count buffer overflow */
+		return;
+	}
+	DPRINT1("Serial: push to buffer done\n");
+	KeReleaseSpinLock(&DeviceExtension->InputBufferLock, Irql);
+	InterlockedIncrement(&DeviceExtension->SerialPerfStats.ReceivedCount);
+}
+
+VOID STDCALL
+SerialSendByte(
+	IN PKDPC Dpc,
+	IN PVOID pDeviceExtension, // real type PSERIAL_DEVICE_EXTENSION
+	IN PVOID Unused1,
+	IN PVOID Unused2)
+{
+	PSERIAL_DEVICE_EXTENSION DeviceExtension;
+	PUCHAR ComPortBase;
+	UCHAR Byte;
+	KIRQL Irql;
+	NTSTATUS Status;
+	
+	DeviceExtension = (PSERIAL_DEVICE_EXTENSION)pDeviceExtension;
+	ComPortBase = (PUCHAR)DeviceExtension->BaseAddress;
+	
+	DPRINT1("Serial: sending bytes (if any) on COM%lu\n",
+		DeviceExtension->ComPort);
+	
+	KeAcquireSpinLock(&DeviceExtension->OutputBufferLock, &Irql);
+	while (!IsCircularBufferEmpty(&DeviceExtension->OutputBuffer)
+		&& READ_PORT_UCHAR(SER_LSR(ComPortBase)) & SR_LSR_TBE)
+	{
+		Status = PopCircularBufferEntry(&DeviceExtension->OutputBuffer, &Byte);
+		if (!NT_SUCCESS(Status))
+			break;
+		WRITE_PORT_UCHAR(SER_THR(ComPortBase), Byte);
+		DeviceExtension->SerialPerfStats.TransmittedCount++;
+	}	
+	KeReleaseSpinLock(&DeviceExtension->OutputBufferLock, Irql);
+}
+
 BOOLEAN STDCALL
 SerialInterruptService(
 	IN PKINTERRUPT Interrupt,
@@ -70,7 +131,6 @@ SerialInterruptService(
 	UCHAR Byte;
 	PUCHAR ComPortBase;
 	UCHAR Iir;
-	NTSTATUS Status;
 	
 	DeviceObject = (PDEVICE_OBJECT)ServiceContext;
 	DeviceExtension = (PSERIAL_DEVICE_EXTENSION)DeviceObject->DeviceExtension;
@@ -96,18 +156,7 @@ SerialInterruptService(
 		case SR_IIR_THR_EMPTY:
 		{
 			DPRINT("Serial: SR_IIR_THR_EMPTY\n");
-			/* FIXME: lock OutputBuffer */
-			while (!IsCircularBufferEmpty(&DeviceExtension->OutputBuffer)
-				&& READ_PORT_UCHAR(SER_LSR(ComPortBase)) & SR_LSR_TBE)
-			{
-				Status = PopCircularBufferEntry(&DeviceExtension->OutputBuffer, &Byte);
-				if (!NT_SUCCESS(Status))
-					break;
-				WRITE_PORT_UCHAR(SER_THR(ComPortBase), Byte);
-				DeviceExtension->SerialPerfStats.TransmittedCount++;
-			}	
-			/* FIXME: unlock OutputBuffer */
-			return TRUE;
+			return KeInsertQueueDpc(&DeviceExtension->SendByteDpc, NULL, NULL);
 		}
 		case SR_IIR_DATA_RECEIVED:
 		{
@@ -116,19 +165,10 @@ SerialInterruptService(
 			{
 				Byte = READ_PORT_UCHAR(SER_RBR(ComPortBase));
 				DPRINT1("Serial: Byte received: 0x%02x (%c)\n", Byte, Byte);
-				/* FIXME: lock InputBuffer */
-				Status = PushCircularBufferEntry(&DeviceExtension->InputBuffer, Byte);
-				if (!NT_SUCCESS(Status))
-				{
-					/* FIXME: count buffer overflow */
+				if (!KeInsertQueueDpc(&DeviceExtension->ReceivedByteDpc, (PVOID)(ULONG_PTR)Byte, NULL))
 					break;
-				}
-				DPRINT1("Serial: push to buffer done\n");
-				/* FIXME: unlock InputBuffer */
-				DeviceExtension->SerialPerfStats.ReceivedCount++;
 			}
 			return TRUE;
-			break;
 		}
 		case SR_IIR_ERROR:
 		{

reactos/drivers/dd/serial/pnp.c

@@ -76,6 +76,10 @@ SerialAddDeviceInternal(
 	Status = InitializeCircularBuffer(&DeviceExtension->OutputBuffer, 16);
 	if (!NT_SUCCESS(Status)) goto ByeBye;
 	IoInitializeRemoveLock(&DeviceExtension->RemoveLock, SERIAL_TAG, 0, 0);
+	KeInitializeSpinLock(&DeviceExtension->InputBufferLock);
+	KeInitializeSpinLock(&DeviceExtension->OutputBufferLock);
+	KeInitializeDpc(&DeviceExtension->ReceivedByteDpc, SerialReceiveByte, DeviceExtension);
+	KeInitializeDpc(&DeviceExtension->SendByteDpc, SerialSendByte, DeviceExtension);
 	//Fdo->Flags |= DO_POWER_PAGEABLE (or DO_POWER_INRUSH?)
 	Status = IoAttachDeviceToDeviceStackSafe(Fdo, Pdo, &DeviceExtension->LowerDevice);
 	if (!NT_SUCCESS(Status))
@@ -152,13 +156,7 @@ SerialPnpStartDevice(
 	
 	DeviceExtension = (PSERIAL_DEVICE_EXTENSION)DeviceObject->DeviceExtension;
 	
-	/* FIXME: actually, IRP_MN_START_DEVICE is sent twice to each serial device:
-	 * - one when loading serial.sys
-	 * - one when loading attached upper filter serenum.sys
-	 * This behaviour MUST NOT exist.
-	 * As PnP handling isn't right anyway, I didn't search how to correct this.
-	 */
-	if (DeviceExtension->PnpState == dsStarted) return STATUS_SUCCESS;
+	ASSERT(DeviceExtension->PnpState == dsStopped);
 	
 	DeviceExtension->ComPort = DeviceExtension->SerialPortNumber + 1;
 	DeviceExtension->BaudRate = 19200 | SERIAL_BAUD_USER;

reactos/drivers/dd/serial/rw.c

@@ -16,12 +16,8 @@ static PVOID
 SerialGetUserBuffer(IN PIRP Irp)
 {
    ASSERT(Irp);
-
-   if (Irp->MdlAddress)
-      return MmGetSystemAddressForMdlSafe(Irp->MdlAddress, NormalPagePriority);
-   else
-   	/* FIXME: try buffer */
-      return Irp->UserBuffer;
+   
+   return Irp->AssociatedIrp.SystemBuffer;
 }
 
 NTSTATUS STDCALL
@@ -36,6 +32,7 @@ SerialRead(
 	PUCHAR Buffer;
 	PUCHAR ComPortBase;
 	UCHAR ReceivedByte;
+	KIRQL Irql;
 	NTSTATUS Status = STATUS_SUCCESS;
 	
 	DPRINT("Serial: IRP_MJ_READ\n");
@@ -64,8 +61,8 @@ SerialRead(
 	if (!NT_SUCCESS(Status))
 		goto ByeBye;
 	
-	/* FIXME: lock InputBuffer */
-	while (Length-- > 0 && !IsCircularBufferEmpty(&DeviceExtension->InputBuffer))
+	KeAcquireSpinLock(&DeviceExtension->InputBufferLock, &Irql);
+	while (Length-- > 0)
 	{
 		Status = PopCircularBufferEntry(&DeviceExtension->InputBuffer, &ReceivedByte);
 		if (!NT_SUCCESS(Status))
@@ -73,54 +70,117 @@ SerialRead(
 		DPRINT("Serial: read from buffer 0x%x (%c)\n", ReceivedByte, ReceivedByte);
 		Buffer[Information++] = ReceivedByte;
 	}
+	KeReleaseSpinLock(&DeviceExtension->InputBufferLock, Irql);
 	if (Length > 0 &&
 		!(DeviceExtension->SerialTimeOuts.ReadIntervalTimeout == INFINITE &&
 		  DeviceExtension->SerialTimeOuts.ReadTotalTimeoutConstant == 0 &&
 		  DeviceExtension->SerialTimeOuts.ReadTotalTimeoutMultiplier == 0))
 	{
-		if (DeviceExtension->SerialTimeOuts.ReadIntervalTimeout == 0
-			|| DeviceExtension->SerialTimeOuts.ReadTotalTimeoutMultiplier == 0)
+		ULONG IntervalTimeout;
+		ULONG TotalTimeout;
+		BOOLEAN UseIntervalTimeout = FALSE;
+		BOOLEAN UseTotalTimeout = FALSE;
+		ULONG ThisByteTimeout;
+		BOOLEAN IsByteReceived;
+		ULONG i;
+		/* Extract timeouts informations */
+		if (DeviceExtension->SerialTimeOuts.ReadIntervalTimeout == INFINITE &&
+			DeviceExtension->SerialTimeOuts.ReadTotalTimeoutMultiplier == INFINITE &&
+			DeviceExtension->SerialTimeOuts.ReadTotalTimeoutConstant > 0 &&
+			DeviceExtension->SerialTimeOuts.ReadTotalTimeoutConstant < INFINITE)
 		{
-			DPRINT("Serial: we must wait for %lu characters!\n", Length);
-#if 1
-			/* Disable interrupts */
-			WRITE_PORT_UCHAR(SER_IER((PUCHAR)DeviceExtension->BaseAddress), DeviceExtension->IER & ~1);
-			
-			/* Polling code */
-			while (Length > 0)
+			if (Information > 0)
 			{
-				while ((READ_PORT_UCHAR(SER_LSR(ComPortBase)) & SR_LSR_DR) == 0)
-					;
-				ReceivedByte = READ_PORT_UCHAR(SER_RBR(ComPortBase));
-				Buffer[Information++] = ReceivedByte;
-				Length--;
+				/* don't read mode bytes */
+				Length = 0;
 			}
-			/* Enable interrupts */
-			WRITE_PORT_UCHAR(SER_IER((PUCHAR)DeviceExtension->BaseAddress), DeviceExtension->IER);
-#else
-			while (Length > 0)
+			else
 			{
-				if (!IsCircularBufferEmpty(&DeviceExtension->InputBuffer))
-				{
-					Status = PopCircularBufferEntry(&DeviceExtension->InputBuffer, &ReceivedByte);
-					if (!NT_SUCCESS(Status))
-						break;
-					DPRINT1("Serial: read from buffer 0x%x (%c)\n", ReceivedByte, ReceivedByte);
-					Buffer[Information++] = ReceivedByte;
-					Length--;
-				}
+				/* read only one byte */
+				UseTotalTimeout = TRUE;
+				TotalTimeout = DeviceExtension->SerialTimeOuts.ReadTotalTimeoutConstant;
+				Length = 1;
 			}
-#endif
 		}
 		else
 		{
-			/* FIXME: use ReadTotalTimeoutMultiplier and ReadTotalTimeoutConstant */
-			DPRINT1("Serial: we must wait for %lu characters at maximum within %lu milliseconds! UNIMPLEMENTED\n",
-				Length,
-				Stack->Parameters.Read.Length * DeviceExtension->SerialTimeOuts.ReadTotalTimeoutMultiplier + DeviceExtension->SerialTimeOuts.ReadTotalTimeoutConstant);
+			if (DeviceExtension->SerialTimeOuts.ReadIntervalTimeout != 0)
+			{
+				UseIntervalTimeout = TRUE;
+				IntervalTimeout = DeviceExtension->SerialTimeOuts.ReadIntervalTimeout;
+			}
+			if (DeviceExtension->SerialTimeOuts.ReadTotalTimeoutConstant != 0 ||
+				DeviceExtension->SerialTimeOuts.ReadTotalTimeoutMultiplier != 0)
+			{
+				UseTotalTimeout = TRUE;
+				TotalTimeout = DeviceExtension->SerialTimeOuts.ReadTotalTimeoutConstant +
+					DeviceExtension->SerialTimeOuts.ReadTotalTimeoutMultiplier * Length;
+			}
 		}
+		DPRINT("Serial: UseIntervalTimeout = %ws, IntervalTimeout = %lu\n",
+			UseIntervalTimeout ? L"YES" : L"NO",
+			UseIntervalTimeout ? IntervalTimeout : 0);
+		DPRINT("Serial: UseTotalTimeout = %ws, TotalTimeout = %lu\n",
+			UseTotalTimeout ? L"YES" : L"NO",
+			UseTotalTimeout ? TotalTimeout : 0);
+		
+		/* FIXME: it should be better to use input buffer instead of
+		 * disabling interrupts, and try to directly read for port! */
+		
+		/* FIXME: NtQueryPerformanceCounter gives a more accurate
+		 * timer, but it is not available on all computers. First try
+		 * NtQueryPerformanceCounter, and current method if it is not
+		 * implemented. */
+		
+		/* FIXME: remove disabling interrupts */
+		WRITE_PORT_UCHAR(SER_IER(ComPortBase), DeviceExtension->IER & ~1);
+		while (Length > 0)
+		{
+			ThisByteTimeout = IntervalTimeout;
+			IsByteReceived = FALSE;
+			while (TRUE)
+			{
+				for (i = 0; i < 1000; i++)
+				{
+#if 1
+					if ((READ_PORT_UCHAR(SER_LSR(ComPortBase)) & SR_LSR_DR) != 0)
+					{
+						ReceivedByte = READ_PORT_UCHAR(ComPortBase);
+						DPRINT("Serial: received byte 0x%02x (%c)\n", ReceivedByte, ReceivedByte);
+						IsByteReceived = TRUE;
+						break;
+					}
+#else
+					KeAcquireSpinLock(&DeviceExtension->InputBufferLock, &Irql);
+					if (!IsCircularBufferEmpty(&DeviceExtension->InputBuffer))
+					{
+						PopCircularBufferEntry(&DeviceExtension->InputBuffer, &ReceivedByte);
+						KeReleaseSpinLock(&DeviceExtension->InputBufferLock, Irql);
+						DPRINT("Serial: reading byte from buffer 0x%02x (%c)\n", ReceivedByte, ReceivedByte);
+						IsByteReceived = TRUE;
+						break;
+					}
+					KeReleaseSpinLock(&DeviceExtension->InputBufferLock, Irql);
+#endif
+					KeStallExecutionProcessor(1);
+				}
+				if (IsByteReceived) break;
+				if (UseIntervalTimeout)
+				{
+					if (ThisByteTimeout == 0) break; else ThisByteTimeout--;
+				}
+				if (UseTotalTimeout)
+				{
+					if (TotalTimeout == 0) break; else TotalTimeout--;
+				}
+			}
+			if (!IsByteReceived) break;
+			Buffer[Information++] = ReceivedByte;
+			Length--;
+		}
+		/* FIXME: remove enabling interrupts */
+		WRITE_PORT_UCHAR(SER_IER(ComPortBase), DeviceExtension->IER);
 	}
-	/* FIXME: unlock InputBuffer */
 	Status = STATUS_SUCCESS;
 	
 	IoReleaseRemoveLock(&DeviceExtension->RemoveLock, (PVOID)DeviceExtension->ComPort);
@@ -143,6 +203,7 @@ SerialWrite(
 	ULONG Information = 0;
 	PUCHAR Buffer;
 	PUCHAR ComPortBase;
+	KIRQL Irql;
 	NTSTATUS Status = STATUS_SUCCESS;
 	
 	DPRINT("Serial: IRP_MJ_WRITE\n");
@@ -166,7 +227,7 @@ SerialWrite(
 	if (!NT_SUCCESS(Status))
 		goto ByeBye;
 	
-	/* FIXME: lock OutputBuffer */
+	KeAcquireSpinLock(&DeviceExtension->OutputBufferLock, &Irql);
 	if (IsCircularBufferEmpty(&DeviceExtension->OutputBuffer))
 	{
 		/* Put the maximum amount of data in UART output buffer */
@@ -197,7 +258,7 @@ SerialWrite(
 		DPRINT1("Serial: write to buffer 0x%02x\n", Buffer[Information]);
 		Information++;
 	}
-	/* FIXME: unlock OutputBuffer */
+	KeReleaseSpinLock(&DeviceExtension->OutputBufferLock, Irql);
 	IoReleaseRemoveLock(&DeviceExtension->RemoveLock, (PVOID)DeviceExtension->ComPort);
 
 ByeBye:

reactos/drivers/dd/serial/serial.h

@@ -81,6 +81,8 @@ typedef struct _SERIAL_DEVICE_EXTENSION
 	ULONG BaudRate;
 	ULONG BaseAddress;
 	PKINTERRUPT Interrupt;
+	KDPC ReceivedByteDpc;
+	KDPC SendByteDpc;
 	
 	SERIAL_LINE_CONTROL SerialLineControl;
 	UART_TYPE UartType;
@@ -90,7 +92,9 @@ typedef struct _SERIAL_DEVICE_EXTENSION
 	SERIAL_TIMEOUTS SerialTimeOuts;
 	BOOLEAN IsOpened;	
 	CIRCULAR_BUFFER InputBuffer;
+	KSPIN_LOCK InputBufferLock;
 	CIRCULAR_BUFFER OutputBuffer;
+	KSPIN_LOCK OutputBufferLock;
 	
 	/* Current values */
 	UCHAR IER; /* Base+1, Interrupt Enable Register */
@@ -250,6 +254,20 @@ ForwardIrpAndForget(
 	IN PDEVICE_OBJECT DeviceObject,
 	IN PIRP Irp);
 
+VOID STDCALL
+SerialReceiveByte(
+	IN PKDPC Dpc,
+	IN PVOID pDeviceExtension, // real type PSERIAL_DEVICE_EXTENSION
+	IN PVOID pByte,            // real type UCHAR
+	IN PVOID Unused);
+
+VOID STDCALL
+SerialSendByte(
+	IN PKDPC Dpc,
+	IN PVOID pDeviceExtension, // real type PSERIAL_DEVICE_EXTENSION
+	IN PVOID Unused1,
+	IN PVOID Unused2);
+
 BOOLEAN STDCALL
 SerialInterruptService(
 	IN PKINTERRUPT Interrupt,