PDP11: RP11: Implement delayed CS_DONE for "initiation" commands (SEE…

…K/HOME)

Running earlier XXDP tests revealed that a technique of concurrent command
initiation and continued housekeeping for the command completion was used in
the old code.

For example, code could initiate a SEEK command for a drive, and knowing that
CS_DONE (and thus, an interrupt) is coming in about 16us, it would then go
ahead and clear a flag, which registers that the interrupt has occurred
(expected to be set to 1 by the ISR).  If CS_DONE is set by the implementation
at the function initiation immediately, that would mean that the interrupt
could be triggered before the next instruction, and the flag would be set by
the ISR right away.  The main code, however, would proceed with the the flag
clear as the following instruction, thus, never detecting the interrupt.

Since this technique was in existence, it is better to introduce a delay for
setting CS_DONE in the "fast" initiation commands like SEEK and HOME, to
accommodate the software that was relying on it.

So far, however, no issues were encountered in testing (except one), where
this delay mattered, but it's hard to tell if it would not be needed at all.

All I/O commands always delay CS_DONE already because they were never supposed
to be immediate.

Since the time for CS_DONE in initiation commands was documented at 16us, the
introduced delay is set to 10 instructions, which usually took more than that
to execute.  But the interrupt flag clear case would be covered, as well as
the counted waits, which used some 25+-iteration tight loops for "drive ready",
before flagging a time-out (so the delay cannot longer, either).

It also looks like more modern code never used any such tricks, so for it, it
should not matter if CS_DONE was delayed or not.

PDP11/pdp11_rr.c

@@ -33,6 +33,14 @@
 #include "pdp11_defs.h"
 #include "sim_disk.h"
 
+/* const_assert() */
+
+#define _Cx(a, b)        a ## b
+#define _Ax(x)           _Cx(_assert, x)
+#define _Sx(n)           (((n) << 1) - 1)
+#define const_assert(c)  do { static const char _Ax(__LINE__)[-_Sx(!(c))] \
+                              = {!(c)};  (void) _Ax(__LINE__); } while (0)
+
 /* Constants */
 
 #define RPCONTR         uint16
@@ -69,9 +77,9 @@
 /* Parameters in the unit descriptor */
 #define CYL             u3                              /* current cylinder */
 #define HEAD            u4                              /* current track */
-#define FUNC            u5                              /* current func */
-#define STATUS          u6                              /* RPDS bits for the unit */
-#define SEEKING         us9                             /* seek cmd underway */
+#define FUNC            us9                             /* current func */
+#define STATUS          u5                              /* RPDS bits for the unit */
+#define SEEKING         u6                              /* seek cmd underway */
 
 /* 4 dummy UNIBUS registers followed by... */
 #define RP_IOFF         4
@@ -346,7 +354,7 @@ static t_stat rr_wr (int32 data, int32 PA, int32 access);
 static int32  rr_inta (void);
 static t_stat rr_svc (UNIT *uptr);
 static t_stat rr_reset (DEVICE *dptr);
-static void   rr_go (int32 func);
+static void   rr_go (int16 func);
 static void   rr_set_done (int32 error);
 static void   rr_clr_done (void);
 static t_stat rr_boot (int32 unitno, DEVICE *dptr);
@@ -604,7 +612,8 @@ static t_stat rr_wr (int32 data, int32 PA, int32 access)
 {
     /* offset by base then decode <4:1> */
     int32 rn = (((PA - rr_dib.ba) >> 1) & 017) - RP_IOFF;
-    int32 n, func, oval = rn < 0 ? 0 : *rr_regs[rn].valp;
+    int32 n, oval = rn < 0 ? 0 : *rr_regs[rn].valp;
+    int16 func;
 
     if (access == WRITEB  &&  2 <= rn  &&  rn <= 6)
         data = RR_DATOB(oval, data);
@@ -682,9 +691,20 @@ static t_stat rr_wr (int32 data, int32 PA, int32 access)
     return SCPE_OK;
 }
 
+/* Complete seek initiation */
+
+static t_stat rr_seek_init (UNIT *uptr)
+{
+    rr_set_done(0);                                     /* set done */
+    assert(uptr->SEEKING);
+    uptr->action = rr_svc;
+    sim_activate(uptr, uptr->SEEKING);                  /* seek ends then */
+    return SCPE_OK;
+}
+
 /* Initiate new function */
 
-static void rr_go (int32 func)
+static void rr_go (int16 func)
 {
     int32 i, type, cyl, head;
     t_bool rd, wr;
@@ -704,6 +724,7 @@ static void rr_go (int32 func)
         for (i = 0;  i < RP_NUMDR;  ++i) {
             uptr = rr_dev.units + i;
             sim_cancel(uptr);
+            uptr->action = rr_svc;
             uptr->SEEKING = 0;
             uptr->STATUS = 0;
             uptr->FUNC = 0;
@@ -720,6 +741,7 @@ static void rr_go (int32 func)
     rpcs &= ~(CSR_ERR | RPCS_HERR);                     /* clear summary */
     i = GET_DRIVE(rpcs);                                /* get drive no */
     uptr = rr_dev.units + i;                            /* selected unit */
+    assert(uptr->action == rr_svc);
     assert(uptr->SEEKING  ||  !uptr->FUNC);             /* SEEK underway or idle */
     uptr->STATUS &= ~(RPDS_DKER | RPDS_WLK);            /* clear drive errors */
 
@@ -815,20 +837,30 @@ static void rr_go (int32 func)
     else
         i  = drv_tab[type].seek_max;
     if (func == RPCS_HOME  ||  func == RPCS_SEEK) {     /* seek? */
-        uptr->SEEKING = 1;                              /* drive is seeking */
-        rr_set_done(0);                                 /* set done */
-        /* XXDP ZRPF-B test 0 fails because of the data race on INTFLG cleared
-         * _after_ initiating the SEEK.  This code interrupts right away (as so
-         * could the device) causing INTFLG set by ISR to 1, to be lost.  Hence,
-         * the test always sees INTFLG as 0, and cannot confirm the interrupt.
-         * INTFLG should have been cleared prior to SEEK.  Fixed in ZRPB-E. */
+        uptr->action = rr_seek_init;
+        uptr->SEEKING = i;                              /* drive is seeking */
+        i = 10;                                         /* enough for 16us */
+        /* XXDP ZRPF-B, Test 0 (cylinder positioning) fails because, even though
+         * it initializes INTFLG late (_after_ initiating the SEEK), the delayed
+         * CS_DONE here helps avoid the race (and an error) but only for the
+         * first iteration (cylinder 0).  The next and all subsequent iterations
+         * begin with clearing attention bits in RDPS with a BIC instruction,
+         * like so: "BIC @#RPDS, @#RPDS", resulting in bare 0 actually sent by
+         * ALU to the UNIBUS in DATO cycle to RPDS, and causing no bits reset.
+         * That, in turn, causes the interrupt to occur immediately when RPCS is
+         * written with the new SEEK/AIE function (the attention bit still set)
+         * for the next cylinder, and that causes INTFLG to get set by the ISR
+         * just prior to when the test clears it.  Thus, the test is unable to
+         * confirm the interrupt for all remaining cylinders.  ZRPB-E fixes both
+         * bugs by initializing INTFLG before firing up SEEK/AIE, and also by
+         * using the more correct "MOVB @#RPDS, @#RPDS", instead of BIC. */
     } else {
         if (cyl != uptr->CYL  ||  head != uptr->HEAD) {
             assert(func != RPCS_RD_NOSEEK  &&  func != RPCS_WR_NOSEEK);
             uptr->STATUS |= RPDS_SEEK;                  /* drive is seeking */
         }
         i += RP_ROT_12;                                 /* I/O takes longer */
-        /* XXDP ZRPB-E / ZRPF-B have two data race conditions in test 5 (data
+        /* XXDP ZRPB-E / ZRPF-B have two data race conditions in Test 5 (data
          * reliability), which in ZRPB-E can be worked around with the following
          * multiplier for all 15 steady patterns, but it does not help eliminate
          * the second race in the last (random) pattern test, despite showing no
@@ -881,7 +913,8 @@ static void rr_seek_done (UNIT *uptr, t_bool cancel)
 
 static t_stat rr_svc (UNIT *uptr)
 {
-    int32 func = uptr->FUNC, cyl, head, sect, da, wc, n;
+    int32 cyl, head, sect, da, wc, n;
+    int16 func = uptr->FUNC;
     t_seccnt todo, done;
     t_stat ioerr;
     uint32 ma;
@@ -1179,7 +1212,7 @@ static t_stat rr_reset (DEVICE *dptr)
     int32 i;
 
     /* compile-time sanity check first */
-    assert(sizeof(rr_regs)/sizeof(rr_regs[0]) == RP_IOLN/2 - RP_IOFF);
+    const_assert(sizeof(rr_regs)/sizeof(rr_regs[0]) == RP_IOLN/2 - RP_IOFF);
 
     /* clear everything now */
     rpds = 0;
@@ -1193,6 +1226,7 @@ static t_stat rr_reset (DEVICE *dptr)
     for (i = 0;  i < RP_NUMDR;  ++i) {
         UNIT* uptr = rr_dev.units + i;
         sim_cancel(uptr);
+        uptr->action = rr_svc;
         uptr->SEEKING = 0;
         uptr->STATUS = 0;
         uptr->FUNC = 0;
@@ -1226,7 +1260,7 @@ static t_stat rr_attach (UNIT *uptr, CONST char *cptr)
 
 static t_stat rr_detach (UNIT *uptr)
 {
-    int32 func = uptr->FUNC;
+    int16 func = uptr->FUNC;
     rr_seek_done(uptr, 1/*cancel*/);
     if (func) {
         uptr->FUNC = 0;                                 /* idle now */
@@ -1241,6 +1275,7 @@ static t_stat rr_detach (UNIT *uptr)
     uptr->STATUS |= RPDS_UNSAFE;                        /* must reset before use */
     assert(!sim_is_active(uptr));
     assert(!uptr->SEEKING);
+    uptr->action = rr_svc;
     uptr->HEAD = 0;
     uptr->CYL = 0;
     return sim_disk_detach(uptr);