* Re: [Qemu-devel] Serial port UART emulation apparently buggy
2004-08-19 11:06 [Qemu-devel] Serial port UART emulation apparently buggy Darryl Dixon
@ 2004-08-19 14:06 ` Darryl Dixon
0 siblings, 0 replies; 2+ messages in thread
From: Darryl Dixon @ 2004-08-19 14:06 UTC (permalink / raw)
To: qemu-devel; +Cc: nile
[-- Attachment #1.1: Type: text/plain, Size: 9296 bytes --]
Hi All,
Further to the below, I've tested the serial with hardware flow
control, and I'm more confused than ever. If someone could please
verify that I'm following this dump correctly, I'd appreciate it. From
what I'm seeing here, in the case of hardware flow control it is the
guest OS (in this case Win98) that is braindead and buggy.
Attached is a patch so that you too can generate these happy dumps (the
patch also adds the one-line fix mentioned previously that makes
no-flow-control work). The patch should be applied after my previous
com2-enabling patch. The dump below is what happens when a single key
is pressed in the guest OS's terminal session to the com2 port with
hardware flow control enabled. After each event/dump I have put a
'COMMENT' showing what I think is happening:
(qemu) serial_ioport_read entry
Current state of UART is:
divider = 0x30
rbr = 0x0
ier = 0xd
iir = 0x1
lcr = 0x3
mcr = 0xb
lsr = 0x60
msr = 0x0
scr = 0x0
thr_ipending = 1
irq = 3
serial: read addr=0x01 val=0x0d
COMMENT: this looks like the guest OS is testing to see under what
conditions the UART will interrupt it (setting returned (0xd) says it
won't interrupt it the transmit buffer is empty)
serial_ioport_write entry
Current state of UART is:
divider = 0x30
rbr = 0x0
ier = 0xd
iir = 0x1
lcr = 0x3
mcr = 0xb
lsr = 0x60
msr = 0x0
scr = 0x0
thr_ipending = 1
irq = 3
serial: write addr=0x01 val=0x0f
COMMENT: following from the above, the guest OS pushes a value into
s->ier to tell it the UART to interrupt it if the transmit buffer is
empty (0xf)
serial_update_irq entry
Current state of UART is:
divider = 0x30
rbr = 0x0
ier = 0xf
iir = 0x1
lcr = 0x3
mcr = 0xb
lsr = 0x60
msr = 0x0
scr = 0x0
thr_ipending = 1
irq = 3
COMMENT: The UART processes the change, sets s->iir to 0x2 (seen below)
to tell the guest OS that the interrupt it's about to issue is for the
transmit buffer, and then issues and interrupt.
serial_ioport_write entry
Current state of UART is:
divider = 0x30
rbr = 0x0
ier = 0xf
iir = 0x2
lcr = 0x3
mcr = 0xb
lsr = 0x60
msr = 0x0
scr = 0x0
thr_ipending = 1
irq = 3
serial: write addr=0x01 val=0x0f
COMMENT: This is where the braindeadness begins; the guest OS, after
receiving the interrupt, instead of testing s->iir to see what it was
for simply pushes 0xf into s->ier again. What the heck?
serial_update_irq entry
Current state of UART is:
divider = 0x30
rbr = 0x0
ier = 0xf
iir = 0x2
lcr = 0x3
mcr = 0xb
lsr = 0x60
msr = 0x0
scr = 0x0
thr_ipending = 1
irq = 3
COMMENT: So the UART dutifully processes the change, forces s->iir to
0x2 again (seen below) and interrupts the guest OS again...
serial_ioport_read entry
Current state of UART is:
divider = 0x30
rbr = 0x0
ier = 0xf
iir = 0x2
lcr = 0x3
mcr = 0xb
lsr = 0x60
msr = 0x0
scr = 0x0
thr_ipending = 1
irq = 3
COMMENT: This time the guest OS gets the picture and reads s->iir to see
why it was interrupted (seen below)
serial_update_irq entry
Current state of UART is:
divider = 0x30
rbr = 0x0
ier = 0xf
iir = 0x2
lcr = 0x3
mcr = 0xb
lsr = 0x60
msr = 0x0
scr = 0x0
thr_ipending = 0
irq = 3
serial: read addr=0x02 val=0x02
COMMENT: Our UART, just before return 0x2 to the guest OS (telling it
the interrupt was for the transmit buffer), processes its changes, sets
s->iir back to 0x1 (no interrupt) and then doesn't interrupt any more
(just returns the 0x2).
serial_ioport_read entry
Current state of UART is:
divider = 0x30
rbr = 0x0
ier = 0xf
iir = 0x1
lcr = 0x3
mcr = 0xb
lsr = 0x60
msr = 0x0
scr = 0x0
thr_ipending = 0
irq = 3
serial: read addr=0x05 val=0x60
COMMENT: Guest OS checks the state of the line (s->lsr) and gets told
that the transmitter and transmit holding register are empty (i.e. all
good!)
serial_ioport_read entry
Current state of UART is:
divider = 0x30
rbr = 0x0
ier = 0xf
iir = 0x1
lcr = 0x3
mcr = 0xb
lsr = 0x60
msr = 0x0
scr = 0x0
thr_ipending = 0
irq = 3
serial: read addr=0x01 val=0x0f
COMMENT: Guest OS checks to see the conditions that it's interrupted
under (why isn't it trying to send its data?!?), and of course receives
0xf (all conditions)
serial_ioport_write entry
Current state of UART is:
divider = 0x30
rbr = 0x0
ier = 0xf
iir = 0x1
lcr = 0x3
mcr = 0xb
lsr = 0x60
msr = 0x0
scr = 0x0
thr_ipending = 0
irq = 3
serial: write addr=0x01 val=0x0d
COMMENT: Guest OS removes the notification of transmit buffer empty
interrupts
serial_update_irq entry
Current state of UART is:
divider = 0x30
rbr = 0x0
ier = 0xd
iir = 0x1
lcr = 0x3
mcr = 0xb
lsr = 0x60
msr = 0x0
scr = 0x0
thr_ipending = 0
irq = 3
COMMENT: UART processes that change (no interrupt).
serial_ioport_read entry
Current state of UART is:
divider = 0x30
rbr = 0x0
ier = 0xd
iir = 0x1
lcr = 0x3
mcr = 0xb
lsr = 0x60
msr = 0x0
scr = 0x0
thr_ipending = 0
irq = 3
serial_update_irq entry
COMMENT: Guest OS checks s->iir to see what the latest interrupt was
(why?!?) and of course gets told there wasn't one (see below).
Current state of UART is:
divider = 0x30
rbr = 0x0
ier = 0xd
iir = 0x1
lcr = 0x3
mcr = 0xb
lsr = 0x60
msr = 0x0
scr = 0x0
thr_ipending = 0
irq = 3
serial: read addr=0x02 val=0x01
COMMENT: And at this point guest OS just waits. If another key is
pressed it repeats the process.
I'm not sure if maybe some of the bitmask definitions are wrong or what,
but something is definitely out of whack here. Certainly the guest OS
behaviour seems somewhat myopic... Any deep serial geeks out there that
can explain or point me at a *really* detailed standards doco for how
this thing is supposed to behave? As far as I can see Win98 never
actually makes it to the point of trying to write any data into the
transmit buffer, so obviously something it sees it doesn't like; but I'm
darned if I can guess what! :)
Thanks, and sorry for the long post; enjoy the patch :)
D
On Thu, 2004-08-19 at 23:06, Darryl Dixon wrote:
> Hi All,
>
> Rather than continue one of the many previous threads I thought
> I'd start again, as I have actually made some minor progress in the
> effort to make the serial port emulation work correctly in qemu. I am
> as certain as I can be that the UART emulation coded in
> serial_ioport_write() and (maybe) serial_ioport_read() contains some
> bugs or flaws in its logic regarding which registers and which
> bitmasks get applied at any given time. After extensive debugging via
> the magic of fprintf and my own new function to dump the state of the
> SerialState object at various appropriate times, here's what I see:
> serial_ioport_read() appears to be work correctly(?)
> serial_ioport_write() appears to not set some of its bitmasks in *at
> least* the ier register correctly after the first attempt to write
> some data. This borks it for the remainder of any terminal session in
> the guest, until it is reinitialised. Specifically, I noticed that it
> seems to leave the THRI bit set in ier after having written data.
> When I add this line:
> ----------------8<-------------------
> s->thr_ipending = 1;
> s->lsr |= UART_LSR_THRE;
> s->lsr |= UART_LSR_TEMT;
> + s->ier &= ~UART_IER_THRI;
> serial_update_irq(s);
> ----------------8<-------------------
> to serial_ioport_write things start looking much more promising. A
> guest terminal session now works (I am able to get the modem on my
> host, which is connected to /dev/ttySL0 to dial another phone
> successfully from a terminal session inside the guest), but only in
> 'no flow control' mode. I assume that means that there are other
> registers not being flipped at the right time too, but I haven't found
> them yet. I will poke the code some more, and probably send a proper
> patch containing this fix(?) and some of my debug stuff ifdef'ed out
> once I've cleaned my test area up a bit. Anyhow, just thought I'd
> share :)
>
>
> Cheers,
> --
> Darryl Dixon <esrever_otua@pythonhacker.is-a-geek.net>
>
> ______________________________________________________________________
>
> _______________________________________________
> Qemu-devel mailing list
> Qemu-devel@nongnu.org
> http://lists.nongnu.org/mailman/listinfo/qemu-devel
--
Darryl Dixon <esrever_otua@pythonhacker.is-a-geek.net>
[-- Attachment #1.2: Type: text/html, Size: 16982 bytes --]
[-- Attachment #2: com2_serial_debug.patch --]
[-- Type: text/x-patch, Size: 5261 bytes --]
diff -ru qemu-snapshot-2004-08-16_23/hw/serial.c qemu-snapshot-2004-08-16_23-testserial/hw/serial.c
--- qemu-snapshot-2004-08-16_23/hw/serial.c 2004-07-15 05:28:13.000000000 +1200
+++ qemu-snapshot-2004-08-16_23-testserial/hw/serial.c 2004-08-20 01:47:43.220843707 +1200
@@ -24,6 +24,7 @@
#include "vl.h"
//#define DEBUG_SERIAL
+//#define DEBUG_SERIAL_VERBOSE
#define UART_LCR_DLAB 0x80 /* Divisor latch access bit */
@@ -87,8 +88,32 @@
CharDriverState *chr;
};
+#ifdef DEBUG_SERIAL_VERBOSE
+static void dump_uart(SerialState *s)
+{
+ fprintf(stderr, "Current state of UART is:\n\
+divider = 0x%x\n\
+rbr = 0x%x\n\
+ier = 0x%x\n\
+iir = 0x%x\n\
+lcr = 0x%x\n\
+mcr = 0x%x\n\
+lsr = 0x%x\n\
+msr = 0x%x\n\
+scr = 0x%x\n\
+thr_ipending = %d\n\
+irq = %d\n",
+s->divider, s->rbr, s->ier, s->iir, s->lcr, s->mcr, s->lsr, s->msr, s->scr, s->thr_ipending, s->irq);
+}
+#endif
+
static void serial_update_irq(SerialState *s)
{
+#ifdef DEBUG_SERIAL_VERBOSE
+ fprintf(stderr, "serial_update_irq entry\n");
+ dump_uart(s);
+#endif
+
if ((s->lsr & UART_LSR_DR) && (s->ier & UART_IER_RDI)) {
s->iir = UART_IIR_RDI;
} else if (s->thr_ipending && (s->ier & UART_IER_THRI)) {
@@ -96,10 +121,10 @@
} else {
s->iir = UART_IIR_NO_INT;
}
- if (s->iir != UART_IIR_NO_INT) {
- pic_set_irq(s->irq, 1);
- } else {
+ if (s->iir & UART_IIR_NO_INT) {
pic_set_irq(s->irq, 0);
+ } else {
+ pic_set_irq(s->irq, 1);
}
}
@@ -107,6 +132,10 @@
{
SerialState *s = opaque;
unsigned char ch;
+#ifdef DEBUG_SERIAL_VERBOSE
+ fprintf(stderr, "serial_ioport_write entry\n");
+ dump_uart(s);
+#endif
addr &= 7;
#ifdef DEBUG_SERIAL
@@ -118,6 +147,10 @@
if (s->lcr & UART_LCR_DLAB) {
s->divider = (s->divider & 0xff00) | val;
} else {
+#ifdef DEBUG_SERIAL_VERBOSE
+ fprintf(stderr, "serial_ioport_write data-to-send\n");
+ dump_uart(s);
+#endif
s->thr_ipending = 0;
s->lsr &= ~UART_LSR_THRE;
serial_update_irq(s);
@@ -126,7 +159,12 @@
s->thr_ipending = 1;
s->lsr |= UART_LSR_THRE;
s->lsr |= UART_LSR_TEMT;
+ s->ier &= ~UART_IER_THRI;
serial_update_irq(s);
+#ifdef DEBUG_SERIAL_VERBOSE
+ fprintf(stderr, "serial_ioport_write data-sent\n");
+ dump_uart(s);
+#endif
}
break;
case 1:
@@ -160,17 +198,29 @@
{
SerialState *s = opaque;
uint32_t ret;
+#ifdef DEBUG_SERIAL_VERBOSE
+ fprintf(stderr, "serial_ioport_read entry\n");
+ dump_uart(s);
+#endif
addr &= 7;
switch(addr) {
default:
case 0:
if (s->lcr & UART_LCR_DLAB) {
- ret = s->divider & 0xff;
+ ret = s->divider & 0xff;
} else {
+#ifdef DEBUG_SERIAL_VERBOSE
+ fprintf(stderr, "serial_ioport_read data-to-read\n");
+ dump_uart(s);
+#endif
ret = s->rbr;
s->lsr &= ~(UART_LSR_DR | UART_LSR_BI);
serial_update_irq(s);
+#ifdef DEBUG_SERIAL_VERBOSE
+ fprintf(stderr, "serial_ioport_read data-read\n");
+ dump_uart(s);
+#endif
}
break;
case 1:
@@ -183,7 +233,8 @@
case 2:
ret = s->iir;
/* reset THR pending bit */
- if ((ret & 0x7) == UART_IIR_THRI)
+ if ((ret & 0x7) == UART_IIR_THRI) /* Is it just me, or does this
+ evaluation make no sense? */
s->thr_ipending = 0;
serial_update_irq(s);
break;
@@ -225,8 +276,12 @@
static void serial_receive_byte(SerialState *s, int ch)
{
s->rbr = ch;
+#ifdef DEBUG_SERIAL_VERBOSE
+ fprintf(stderr, "serial_receive_byte s->rbr='%c'\n", s->rbr);
+#endif
s->lsr |= UART_LSR_DR;
serial_update_irq(s);
+
}
static void serial_receive_break(SerialState *s)
Only in qemu-snapshot-2004-08-16_23-testserial: qemu.1
Only in qemu-snapshot-2004-08-16_23-testserial: qemu-doc.html
Only in qemu-snapshot-2004-08-16_23-testserial: qemu-tech.html
diff -ru qemu-snapshot-2004-08-16_23/vl.c qemu-snapshot-2004-08-16_23-testserial/vl.c
--- qemu-snapshot-2004-08-16_23/vl.c 2004-08-20 01:41:10.804413581 +1200
+++ qemu-snapshot-2004-08-16_23-testserial/vl.c 2004-08-19 22:22:24.476670548 +1200
@@ -1029,6 +1029,9 @@
static int fd_chr_write(CharDriverState *chr, const uint8_t *buf, int len)
{
+/*
+ fprintf(stderr, "fd_chr_write wrote %c\n", *buf);
+*/
FDCharDriver *s = chr->opaque;
return write(s->fd_out, buf, len);
}
@@ -1226,10 +1229,12 @@
/* Shouldn't ever end up in here */
return NULL;
} else {
- int fdesc;
- fdesc = open(filename, O_RDWR);
- if (fdesc >= 0) {
- return qemu_chr_open_fd(fdesc, fdesc);
+ int fdesc_r;
+ int fdesc_w;
+ fdesc_w = open(filename, O_WRONLY);
+ fdesc_r = open(filename, O_RDONLY);
+ if ((fdesc_r >= 0) && (fdesc_w >= 0)) {
+ return qemu_chr_open_fd(fdesc_r, fdesc_w);
} else {
return NULL;
}
^ permalink raw reply [flat|nested] 2+ messages in thread