On 02/11/2021 14:46, Peter Maydell wrote:
On Mon, 1 Nov 2021 at 20:31, Mark Cave-Ayland
<mark.cave-ayl...@ilande.co.uk> wrote:
The recent ESCC reset changes cause a regression when attemping to use a real
SS-5 Sun PROM instead of OpenBIOS. The Sun PROM doesn't send an explicit reset
command to the ESCC but gets stuck in a loop probing the keyboard waiting for
STATUS_TXEMPTY to be set in R_STATUS followed by SPEC_ALLSENT in R_SPEC.
Reading through the ESCC datasheet again indicates that SPEC_ALLSENT is updated
when a write to W_TXCTRL1 selects async mode, or remains high if sync mode is
selected. Whilst there is no explicit mention of STATUS_TXEMPTY, the ESCC device
emulation always updates these two register bits together when transmitting
data.
My reading of the spec is that this isn't the right behaviour. I think
what we ought to have is:
* in both async and sync mode, TXEMPTY tracks the tx fifo status
(which I think means "set if there is any space in it", contrary to
what the name of the bit implies)
The wording is certainly confusing but the description on p.177 does state "This bit
is set to 1 when the transmit buffer is empty". My impression from reading the
initial paragraph is that the bit is reset to 0 whenever a character is added into
the TX FIFO suggesting that if this bit is set to 1 then the TX FIFO is empty.
* in sync mode, ALLSENT is always high
Yes, agreed.
* in async mode, ALLSENT reads the same as TXEMPTY (for us, since we have
no extra delay between "data left the FIFO" and "data actually on the wire")
When I wrote the patch I was assuming that this might not be the case for escc_kbd
and escc_mouse but I think you could be right here.
* in sync mode TXEMPTY is apparently also set "when the last bit of the CRC
has cleared the transmit shift register". I don't think I really understand
what sync mode TXEMPTY does overall, but clearly it is not "always 0".
Again the wording isn't particularly helpful here but I think what it is trying to
say is that TXEMPTY remains reset to 0, even when the TX FIFO is empty, until after
the hardware-generated CRC has been shifted out onto the wire.
Add extra logic to update both transmit status bits accordingly when writing to
W_TXCTRL1 which enables the Sun PROM to initialise and boot again under QEMU.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayl...@ilande.co.uk>
---
hw/char/escc.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/hw/char/escc.c b/hw/char/escc.c
index 0fce4f6324..b33cdc229f 100644
--- a/hw/char/escc.c
+++ b/hw/char/escc.c
@@ -575,6 +575,18 @@ static void escc_mem_write(void *opaque, hwaddr addr,
s->wregs[s->reg] = val;
break;
case W_TXCTRL1:
+ s->wregs[s->reg] = val;
+ if (val & TXCTRL1_STPMSK) {
+ ESCCSERIOQueue *q = &s->queue;
+ if (s->type == escc_serial || q->count == 0) {
+ s->rregs[R_STATUS] |= STATUS_TXEMPTY;
+ s->rregs[R_SPEC] |= SPEC_ALLSENT;
+ }
...should there be an 'else' clause here which clears these
bits if we are now in async mode and the tx queue is not empty ?
+ } else {
+ s->rregs[R_STATUS] &= ~STATUS_TXEMPTY;
+ s->rregs[R_SPEC] |= SPEC_ALLSENT;
+ }
Something is a bit odd with how we currently handle both these bits
For SPEC_ALLSENT:
* it is zero on power-on
* soft-reset leaves it unchanged
* we set it on a write to SERIAL_DATA
* this new code adds two places where we set it
* but there is nowhere where we clear it
For STATUS_TXEMPTY:
* it is set on power-on and soft-reset
* it is set on write to SERIAL_DATA
* it is never cleared
Shouldn't we be clearing these bits somewhere ?
+ /* fallthrough */
case W_TXCTRL2:
s->wregs[s->reg] = val;
escc_update_parameters(s);
At this point all the "fallthrough" is doing is (1) repeat the
setting of s->wregs[s->reg] and (2) call escc_update_parameters().
So I think it would be clearer to instead make the two cases
completely separate, and have
escc_update_parameters(s);
break;
instead of the /* fallthrough */.
I'm starting to wonder if asserting the reset signal does do the same as setting the
hardware reset bit, since this would then enable the Sun PROM to pass the
STATUS_TXEMPTY check. This would just leave the SPEC_ALLSENT bit which would be
updated on a write to W_TXCTRL1 to pass the second check.
Assuming that QEMU will always accept the data then I think it would be a case of
setting SPEC_ALLSENT regardless, since if there is no delay in transmitting the data
then a transition of SPEC_ALLSENT will never be visible to the guest.
Certainly we need a solution for 6.2 since the majority of qemu-system-sparc users
who boot Solaris use the real Sun PROM (many of the online tutorials were written
when Artyom first merged his changes which was before OpenBIOS could boot Solaris).
ATB,
Mark.
