On 3/6/19 12:01 PM, Paolo Bonzini wrote:On 05/03/19 06:10, Stephen Checkoway wrote:The SCC/ESCC will briefly stop asserting an interrupt when the
transmit FIFO is filled.
This code doesn't model the transmit FIFO/shift register so the
pending transmit interrupt is never deasserted which means that an
edge-triggered interrupt controller will never see the low-to-high
transition it needs to raise another interrupt. The practical
consequence of this is that guest firmware with an interrupt service
routine for the ESCC that does not send all of the data it has
immediately will stop sending data if the following sequence of
events occurs:
1. Disable processor interrupts
2. Write a character to the ESCC
3. Add additional characters to a buffer which is drained by the ISR
4. Enable processor interrupts
In this case, the first character will be sent, the interrupt will
fire and the ISR will output the second character. Since the pending
transmit interrupt remains asserted, no additional interrupts will
ever fire.
This fixes that situation by explicitly lowering the IRQ when a
character is written to the buffer.
Signed-off-by: Stephen Checkoway <address@hidden>
Looks good but I would like Mark to give his ack as well.
Mark, could you also add hw/char/escc.c to both SPARC and Mac sections
of MAINTAINERS?
Cc'ing Artyom who also made some changes in this file, and Laurent.
Stephen, which particular chipset are you using?
I'm away from the hardware, but my notes say Z80C30. I've included the image from the board if you want to try to decipher the rest of the part number. My best guess is it's a Z85C3008VEC. If it's important, I can ask someone who is a few thousand km closer to the hardware than I currently am to take a look.
This file models various types. I had a talk with Mark or Laurent at
last KVM forum about this device. IIRC, while the sun4m machines use a
real chipset, the MacIO embeds an slighly modified IP core.
I can't find what you describe in the Z85C30 doc, however in the ESCC
datasheet referenced in escc.c (which happens to be a 85MiB pdf!!!) I found:
(This chip is ridiculously configurable, I honestly found it much easier to write a driver by reverse engineering existing firmware than by reading the part sheet.)
Transmit Interrupts and Transmit Buffer Empty Bit on the NMOS/CMOS
The TxIP is reset either by writing data to the transmit buffer or
by issuing the Reset Tx Int command in WR0.
I understand the NMOS/CMOS desc. matches the Z85C30 (no FIFO used).
It has been a little while since I was last looking at this, but my recollection was the SCC has a 1-byte transmit buffer and the ESCC has a small (4 maybe?) byte transmit buffer. But in either case writing data to the transmit buffer should clear the interrupt.
So your description and patch makes sens.
What worries me is the controller could have other pending IRQs to
deliver and you are clearing them. Shouldn't we only clear the
INTR_TXINT bit, and call escc_update_irq() which should lower the IRQ if
no bits are pending?
Maybe as:
s->wregs[W_INTR] &= ~INTR_TXINT;
escc_update_irq(s);
Ah, I think I see. In this case, escc_update_irq() will lower the IRQ if no other interrupts are pending and the set_txint() will raise it again. Otherwise, it'll remain raised. I can give this a shot and see how it goes. (I think this should be R_INTR instead of W_INTR, but I'll double check once I have an opportunity to dig into this again.)
Thanks for looking at this!
Steve
--
Stephen Checkoway
Stephen Checkoway