GS Serial fixes.

This commit is contained in:
Scott Duensing 2026-08-20 01:22:50 -05:00
parent 30c61609e7
commit 89631006f2
3 changed files with 178 additions and 18 deletions

View file

@ -179,3 +179,37 @@ exit path. jlShutdown therefore does not return on IIgs. The root
cause still deserves a runtime fix: any app that returns from main
WITHOUT calling jlShutdown (or any non-JoeyLib program built on this
runtime) still crashes.
## 7. Volatile uint8_t loads through variable pointers widen to 16-bit bus reads (2026-08-18) -- EMPIRICALLY PROVEN
A `volatile uint8_t` LOAD through a variable (long) pointer is emitted as a
16-bit `lda [dp],y` with m=0 and the value masked afterwards (`and #$ff`) --
so the BUS performs a phantom read of address+1. For memory that is harmless;
for I/O it reads a neighbouring register, and hardware with adjacent registers
has read side effects. Concrete damage on the IIgs SCC (Z8530, four adjacent
registers $C038-$C03B): a data-register read of channel B ($C03A) phantom-pops
channel A's RX FIFO ($C03B -- the AppleTalk port), and in a printer-port
configuration a status poll of $C039 phantom-pops the MODEM port's data at
$C03A. (A per-poll RR1 harvest also correlates with MAME 0.264 SCC RX-deafness, but
that is NOT this bug: it reproduced through a guaranteed 8-bit read too, so
the phantom read is refuted as its mechanism and the harvest ships disabled
under emulation. The width violation itself stands proven by disassembly.)
The width handling is inconsistent, which is what makes it look like a bug
rather than a policy:
- STORES through the same pointers are correctly sep #$20-wrapped 8-bit.
- LOADS from CONSTANT addresses are correctly sep-wrapped 8-bit
(`sep #$20 / lda $c038 / rep #$20` -- verified via a probe TU).
- Only variable-pointer loads take the 16-bit `lda [dp],y` + mask form.
Ask: honour the declared access width for volatile loads in the long-indirect
form -- sep-wrap them exactly like the constant-address and store cases.
WORKAROUND LANDED IN JOEYLIB: src/iigs/serial.c routes every serial register
read through a hand-asm `jlpIoRead8` (global function, so no call site can be
inlined back into the widened form): `sep #$20 / lda [$e0],y / rep #$30 /
and #$ff`. ABI verified from a compiled probe: pointer lo16 in A, bank in X,
return in A, $e0-$e3 imaginary-register scratch. Any OTHER volatile I/O read
through a variable pointer (input/video HALs, future code) is still exposed
until the codegen fix lands.

View file

@ -52,9 +52,15 @@
#define SCC_RR0_RX_AVAIL 0x01
#define SCC_RR0_TX_EMPTY 0x04
// RR1 bit 5: RX overrun - the 3-byte hardware FIFO was full when another byte finished, and that byte
// is GONE. Latched until an Error Reset (WR0 command 0x30), so it survives to the next pump pass.
#define SCC_RR1_RX_OVRN 0x20
#define SCC_WR0_ERR_RESET 0x30
// 6551 status bits.
#define ACIA_ST_RX_FULL 0x08
#define ACIA_ST_TX_EMPTY 0x10
#define ACIA_ST_RX_OVRN 0x04
// SCC baud time constant: BRG output = PCLK(3.6864MHz) / (2*(TC+2)) with the
// /16 clock mode, so baud = 115200 / (TC+2) -> TC = 115200/baud - 2.
@ -76,6 +82,14 @@ static uint8_t gRxRing[RX_RING_SIZE];
// ring->caller since open. Cheap enough to keep unconditionally.
uint16_t gIigsRxPumped = 0u;
uint16_t gIigsRxRead = 0u;
// Receive-loss counters, same read-via-extern contract. Overruns = loss EVENTS the hardware latched
// (SCC RR1 overrun / 6551 status bit 2) - one latch observation may stand for SEVERAL destroyed bytes,
// so this is a loss indicator, not a byte count. Before it existed a pump-discipline gap was invisible
// except as a frame CRC drop and a retransmit stall. Dropped = bytes the SOFT ring refused because it
// was full (read from the chip, then discarded); saturating, like the overrun count.
uint16_t gIigsRxOverruns = 0u;
uint16_t gIigsRxDropped = 0u;
static uint16_t gRxHead;
static uint16_t gRxTail;
static bool gOpen;
@ -86,13 +100,44 @@ static volatile uint8_t *gData; // data register (both)
// ----- Prototypes -----
extern uint8_t jlpIoRead8(const volatile uint8_t *p);
static void serialInitAcia(volatile uint8_t *base, const jlSerialConfigT *cfg);
static void serialInitScc(volatile uint8_t *ctrl, volatile uint8_t *data, uint8_t reset, const jlSerialConfigT *cfg);
static bool serialRxReady(void);
static bool serialTxReady(void);
static uint8_t sccRead(volatile uint8_t *ctrl, uint8_t reg);
static void sccWrite(volatile uint8_t *ctrl, uint8_t reg, uint8_t val);
// ----- Guaranteed 8-bit I/O read -----
//
// llvm816 widens a volatile uint8_t LOAD through a variable long pointer to a 16-bit `lda [dp],y`
// (m=0), so every such register read also performs a phantom bus read of address+1 - and the SCC's
// four registers are adjacent, so a data read on one channel phantom-pops the OTHER channel's data
// FIFO ($C03A reads $C03B: channel A, the AppleTalk port). Constant-address loads are sep-wrapped
// correctly; only the pointer form is affected, and STORES are always sep-wrapped. Filed in
// llvm816/LLVM816-ASKS.md; until the toolchain honours volatile width, every serial register READ
// goes through this hand-asm helper - a global function so no call site can be inlined back into
// the widened form. ABI from a compiled probe: pointer lo16 in A, bank in X; return in A (high
// byte cleared), X zeroed; $e0-$e3 are call-clobbered imaginary-register scratch, D points at the
// imaginary page on entry (the same assumption every compiled function makes).
__asm__(
".section .text.jlpIoRead8,\"ax\",@progbits\n"
".globl jlpIoRead8\n"
"jlpIoRead8:\n"
"\trep #$30\n" // known width state: 16-bit A and index
"\tsta $e0\n" // pointer low 16 -> $e0-$e1
"\ttxa\n"
"\tsta $e2\n" // bank -> $e2 ($e3 catches the junk high byte - scratch)
"\tldy #0\n"
"\tsep #$20\n" // 8-bit accumulator: the actual fix
"\tlda [$e0],y\n" // ONE byte on the bus - no phantom companion read
"\trep #$30\n"
"\tand #$ff\n" // the hidden B accumulator held junk; clear it for the return
"\tldx #0\n"
"\trtl\n"
);
// ----- SCC helpers -----
// Register-pointer protocol: point at WRn (if not WR0), then write the value.
@ -104,6 +149,17 @@ static void sccWrite(volatile uint8_t *ctrl, uint8_t reg, uint8_t val) {
}
// Point at RRn, then read it. Same adjacency rule as sccWrite: nothing may touch the SCC between the
// pointer write and the read (safe here - WR1=0 means this channel never interrupts, and the HAL is
// single-threaded polled).
static uint8_t sccRead(volatile uint8_t *ctrl, uint8_t reg) {
if (reg != 0u) {
*ctrl = reg;
}
return jlpIoRead8(ctrl);
}
static void serialInitScc(volatile uint8_t *ctrl, volatile uint8_t *data, uint8_t reset, const jlSerialConfigT *cfg) {
uint8_t wr4;
uint8_t wr3;
@ -218,19 +274,11 @@ static void serialInitAcia(volatile uint8_t *base, const jlSerialConfigT *cfg) {
// ----- Status polling -----
static bool serialRxReady(void) {
if (gIsScc) {
return (*gStatus & SCC_RR0_RX_AVAIL) != 0u; // RR0 read directly
}
return (*gStatus & ACIA_ST_RX_FULL) != 0u;
}
static bool serialTxReady(void) {
if (gIsScc) {
return (*gStatus & SCC_RR0_TX_EMPTY) != 0u;
return (jlpIoRead8(gStatus) & SCC_RR0_TX_EMPTY) != 0u;
}
return (*gStatus & ACIA_ST_TX_EMPTY) != 0u;
return (jlpIoRead8(gStatus) & ACIA_ST_TX_EMPTY) != 0u;
}
@ -316,18 +364,74 @@ void jlpSerialPoll(void) {
// DBR trap that file-scope `++` lowers to on the 65816.
tail = gRxTail;
guard = RX_RING_SIZE;
while (guard != 0u && serialRxReady()) {
uint16_t next = (uint16_t)((tail + 1u) & RX_RING_MASK);
uint8_t b = *gData;
// Counter updates below use the guarded local read-modify-write from input.c's joystick counter -
// NOT because of the spelling (clang canonicalizes `x++` and `x = x + 1` identically) but because
// the saturation compare keeps the loaded value LIVE, which is what stops the backend fusing the
// update into a DBR-relative `inc abs`. Saturating is also the right behaviour for a loss counter.
while (guard != 0u) {
uint16_t next;
uint8_t st;
uint8_t b;
st = jlpIoRead8(gStatus); // RR0 (SCC) or 6551 status
if (gIsScc) {
if ((st & SCC_RR0_RX_AVAIL) == 0u) {
break;
}
} else {
// The 6551 latches overrun ONLY while RDRF is still set, and the DATA read below retires
// both bits together - so the overrun must be sampled HERE, from the same status read that
// gates the byte. (A post-loop check can never see it: the drain already cleared it.)
if ((st & ACIA_ST_RX_OVRN) != 0u) {
uint16_t ovr = gIigsRxOverruns;
if (ovr < 0xFFFFu) {
gIigsRxOverruns = (uint16_t)(ovr + 1u);
}
}
if ((st & ACIA_ST_RX_FULL) == 0u) {
break;
}
}
next = (uint16_t)((tail + 1u) & RX_RING_MASK);
b = jlpIoRead8(gData);
if (next != gRxHead) {
gRxRing[tail] = b;
tail = next;
gIigsRxPumped++;
} else {
uint16_t drop = gIigsRxDropped;
if (drop < 0xFFFFu) {
gIigsRxDropped = (uint16_t)(drop + 1u);
}
}
guard--;
}
gRxTail = tail;
// Harvest the SCC's own loss record: real silicon latches RX overrun in RR1 until an Error Reset,
// so one check per pump names a deaf-window loss even though the pump never saw the byte.
//
// DEFAULT OFF - REAL HARDWARE ONLY, on two measured grounds: (1) under MAME the counter is INERT
// (its z80scc never surfaces the flag into RR1), so enabling it under emulation buys nothing; and
// (2) with it enabled the MAME 0.264 session goes RX-deaf after HELLO - 3 of 4 runs, INCLUDING one
// through jlpIoRead8's true 8-bit reads, so the phantom companion read is NOT the mechanism (one
// green run said otherwise and the next red run retracted it - the two-greens-prove-nothing rule).
// Whatever the per-pass WR0-pointer/RR1 dance disturbs in MAME's SCC remains unpinned; it is not
// worth more rig time for a diagnostic that cannot fire there. JL_IIGS_RR1_HARVEST enables it
// for silicon validation.
#ifdef JL_IIGS_RR1_HARVEST
if (gIsScc) {
if ((sccRead(gStatus, 1u) & SCC_RR1_RX_OVRN) != 0u) {
uint16_t ovr = gIigsRxOverruns;
if (ovr < 0xFFFFu) {
gIigsRxOverruns = (uint16_t)(ovr + 1u);
}
sccWrite(gStatus, 0u, SCC_WR0_ERR_RESET);
}
}
#endif
}

View file

@ -117,18 +117,40 @@ class Xdf:
raise ValueError(f"'{name}' does not fit 8.3")
return stem.ljust(8).encode("ascii") + ext.ljust(3).encode("ascii")
@staticmethod
def encode_name_ext(name):
# Human68k EXTENDED names: the stem may run to 18 chars - the first 8 live in the classic
# name field, chars 9-18 in the dir entry's bytes 12-21 (MS-DOS's reserved area). Returns
# (main11, ext10) for matching; ext10 is all-NUL for a plain 8.3 name. RetroNet's blob names
# ('A' + 8 hex = 9 chars) need this - without it a guest-written blob reads as "not in image".
name = name.upper()
stem, _, ext = name.partition(".")
if len(stem) > 18 or len(ext) > 3:
raise ValueError(f"'{name}' does not fit Human68k 18.3")
main = stem[:8].ljust(8).encode("ascii") + ext.ljust(3).encode("ascii")
extended = stem[8:].encode("ascii").ljust(10, b"\x00")
return main, extended
@staticmethod
def entry_matches(e, main11, ext10):
if bytes(e[0:11]).upper() != main11:
return False
got = bytes(e[12:22]).rstrip(b"\x00 ").upper()
want = ext10.rstrip(b"\x00 ")
return got == want
def find(self, name):
# Human68k PRESERVES filename case in the directory entry (unlike
# MS-DOS, which upcases), so a program writing "joeylog.txt" leaves it
# lowercase on disk. Match case-insensitively or extracting a
# guest-written file fails with a confusing "not in image".
want = self.encode_name(name).upper()
main11, ext10 = self.encode_name_ext(name)
for i, off, e in self._entries():
if e[0] == FREE_MARKER:
break
if e[0] == DELETED_MARKER:
continue
if bytes(e[0:11]).upper() == want:
if self.entry_matches(e, main11, ext10):
return i, off, e
return None, None, None
@ -140,7 +162,7 @@ class Xdf:
_, _, de = self.find(dirname)
if de is None or not (de[11] & ATTR_DIR):
return None
want = self.encode_name(leaf).upper()
main11, ext10 = self.encode_name_ext(leaf)
cluster = struct.unpack("<H", de[26:28])[0]
guard = 0
while 2 <= cluster < EOC_MIN:
@ -151,7 +173,7 @@ class Xdf:
return None
if e[0] == DELETED_MARKER or (e[11] & (ATTR_VOLUME | ATTR_DIR)):
continue
if bytes(e[0:11]).upper() == want:
if self.entry_matches(e, main11, ext10):
return e
cluster = self.fat_get(cluster)
guard += 1