pdp11_dh.c doesn't correctly handle byte-wide access to its registers

bjorend commented 1 year ago

Context

I found this problem when trying to run Unix v7. Its DH-11 driver does do byte-wide access to the "CSR" or "SCR" register (from [https://www.tuhs.org/cgi-bin/utree.pl?file=V7/usr/sys/dev/dh.c]):
```
struct device
{
union {
    int dhcsr;
    char    dhcsrl;
} un;
int dhnxch;
int dhlpr;
char    *dhcar;
int dhbcr;
int dhbar;
int dhbreak;
int dhsilo;
};
```

the output of "sim> SHOW VERSION" while running the simulator which is having the issue

sim> show version
PDP-11 simulator Open SIMH V4.1-0 Current
Simulator Framework Capabilities:
    32b data
    32b addresses
    Threaded Ethernet Packet transports:PCAP:VDE:NAT:UDP
    Idle/Throttling support is available
    Virtual Hard Disk (VHD) support
    RAW disk and CD/DVD ROM support
    Asynchronous I/O support (Lock free asynchronous event queue)
    Asynchronous Clock support
    FrontPanel API Version 12
Host Platform:
    Compiler: GCC Apple LLVM 14.0.0 (clang-1400.0.29.202)
    Simulator Compiled as C (Release Build) on Mar 28 2023 at 20:17:21
    Build Tool: simh-makefile
    Memory Access: Little Endian
    Memory Pointer Size: 64 bits
    Large File (>2GB) support
    SDL Video support: SDL Version 2.26.3, PNG Version 1.6.39, zlib: 1.2.13
    PCRE RegEx (Version 8.45 2021-06-15) support for EXPECT commands
    OS clock resolution: 1ms
    Time taken by msleep(1): 1ms
    Ethernet packet info: libpcap version 1.10.1
    OS: Darwin titus.local 22.3.0 Darwin Kernel Version 22.3.0: Mon Jan 30 20:38:37 PST 2023; root:xnu-8792.81.3~2/RELEASE_ARM64_T6000 arm64
    Processor Name: Apple M1 Pro
    tar tool: bsdtar 3.5.3 - libarchive 3.5.3 zlib/1.2.11 liblzma/5.0.5 bz2lib/1.0.8
    curl tool: curl 7.86.0 (x86_64-apple-darwin22.0) libcurl/7.86.0 (SecureTransport) LibreSSL/3.3.6 zlib/1.2.11 nghttp2/1.47.0
    git commit id: 7a8cfcac+uncommitted-changes
    git commit time: 2023-03-11T19:38:50-1000

The "uncommitted changes" include my bugfix for this issue (see below).

how you built the simulator or that you're using prebuilt binaries

I built simh from github code on my Mac M1 running macOS 13.2.1

bkd@titus simh % make pdp11
lib paths are: /usr/lib /opt/local/lib /opt/homebrew/Cellar/binutils/2.40/lib    /opt/homebrew/Cellar/gettext/0.21.1/lib /opt/homebrew/Cellar/gmp/6.2.1_1/lib /opt/homebrew/Cellar/lftp/4.9.2_1/lib /opt/homebrew/Cellar/libidn2/2.3.4_1/lib /opt/homebrew/Cellar/libunistring/1.1/lib /opt/homebrew/Cellar/openssl@1.1/1.1.1t/lib /opt/homebrew/Cellar/openssl@3/3.0.8/lib /opt/homebrew/Cellar/readline/8.2.1/lib /opt/homebrew/Cellar/sdl2/2.26.4/lib    /usr/lib/
include paths are:  /Library/Developer/CommandLineTools/usr/lib/clang/14.0.0/include /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include /Library/Developer/CommandLineTools/usr/include /opt/local/include /opt/homebrew/Cellar/binutils/2.40/include    /opt/homebrew/Cellar/gettext/0.21.1/include /opt/homebrew/Cellar/gmp/6.2.1_1/include  /opt/homebrew/Cellar/libidn2/2.3.4_1/include /opt/homebrew/Cellar/libunistring/1.1/include /opt/homebrew/Cellar/openssl@1.1/1.1.1t/include /opt/homebrew/Cellar/openssl@3/3.0.8/include /opt/homebrew/Cellar/readline/8.2.1/include /opt/homebrew/Cellar/sdl2/2.26.4/include   
using macOS libpthread: /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/pthread.h
using libpcre: /opt/local/lib/libpcre.dylib /opt/local/include/pcre.h
using libedit: /opt/local/lib/libedit.dylib /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/editline/readline.h
using macOS dlopen with .dylib
using libpng: /opt/local/lib/libpng.dylib /opt/local/include/png.h
using zlib: /opt/local/lib/libz.dylib /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/zlib.h
using mman: /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/sys/mman.h
using libSDL2: /opt/local/include/SDL2/SDL.h
using libpcap: /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/pcap.h
using libvdeplug: /opt/local/lib/libvdeplug.dylib /opt/local/include/libvdeplug.h
***
*** pdp11 Simulator being built with:
*** - compiler optimizations and no debugging support. Apple clang version 14.0.0.
*** - Building using a single compile and link.
*** - dynamic networking support using OSX provided libpcap components.
*** - Local LAN packet transports: PCAP VDE NAT(SLiRP)
*** - video capabilities provided by libSDL2 (Simple Directmedia Layer).
*** - Per simulator tests will be run.
***
*** git commit id is 98ea251f0fa6b0cb8ce1f6bcbae5f92d98483b85+uncommitted-changes.
*** git commit time is 2023-03-10T15:32:53-1000.
***
gcc -std=c99 -U__STRICT_ANSI__  -O2 -fno-strict-overflow -finline-functions -DSIM_GIT_COMMIT_ID=98ea251f0fa6b0cb8ce1f6bcbae5f92d98483b85+uncommitted-changes -DSIM_GIT_COMMIT_TIME=2023-03-10T15:32:53-1000  -DSIM_COMPILER="Apple clang version 14.0.0" -DSIM_BUILD_OS_VERSION="Darwin titus.local 22.3.0 Darwin Kernel Version 22.3.0: Mon Jan 30 20:38:37 PST 2023; root:xnu-8792.81.3~2/RELEASE_ARM64_T6000 arm64" -DSIM_BUILD_TOOL=simh-makefile-single-compile -I . -Werror -D_GNU_SOURCE -I/opt/local/include -DUSE_READER_THREAD -DSIM_ASYNCH_IO  -DHAVE_PCRE_H -DHAVE_LIBEDIT -DHAVE_SYS_IOCTL -DSIM_HAVE_DLOPEN=dylib -DHAVE_UTIME -DHAVE_LIBPNG -DHAVE_ZLIB -DHAVE_GLOB -DHAVE_SHM_OPEN  PDP11/pdp11_fp.c PDP11/pdp11_cpu.c PDP11/pdp11_dz.c PDP11/pdp11_cis.c PDP11/pdp11_lp.c PDP11/pdp11_rk.c PDP11/pdp11_rl.c PDP11/pdp11_rp.c PDP11/pdp11_rx.c PDP11/pdp11_stddev.c PDP11/pdp11_sys.c PDP11/pdp11_tc.c PDP11/pdp11_tm.c PDP11/pdp11_ts.c PDP11/pdp11_io.c PDP11/pdp11_rq.c PDP11/pdp11_tq.c PDP11/pdp11_pclk.c PDP11/pdp11_ry.c PDP11/pdp11_pt.c PDP11/pdp11_hk.c PDP11/pdp11_xq.c PDP11/pdp11_xu.c PDP11/pdp11_vh.c PDP11/pdp11_rh.c PDP11/pdp11_tu.c PDP11/pdp11_cpumod.c PDP11/pdp11_cr.c PDP11/pdp11_rf.c PDP11/pdp11_dl.c PDP11/pdp11_ta.c PDP11/pdp11_rc.c PDP11/pdp11_kg.c PDP11/pdp11_ke.c PDP11/pdp11_dc.c PDP11/pdp11_dmc.c PDP11/pdp11_kmc.c PDP11/pdp11_dup.c PDP11/pdp11_rs.c PDP11/pdp11_vt.c PDP11/pdp11_td.c PDP11/pdp11_io_lib.c PDP11/pdp11_rom.c PDP11/pdp11_ch.c PDP11/pdp11_dh.c PDP11/pdp11_ng.c PDP11/pdp11_daz.c PDP11/pdp11_tv.c PDP11/pdp11_mb.c display/display.c display/sim_ws.c display/ng.c display/vt11.c slirp/arp_table.c slirp/bootp.c slirp/cksum.c slirp/dnssearch.c slirp/if.c slirp/ip_icmp.c slirp/ip_input.c slirp/ip_output.c slirp/mbuf.c slirp/misc.c slirp/sbuf.c slirp/slirp.c slirp/socket.c slirp/tcp_input.c slirp/tcp_output.c slirp/tcp_subr.c slirp/tcp_timer.c slirp/tftp.c slirp/udp.c slirp_glue/glib_qemu_stubs.c slirp_glue/sim_slirp.c scp.c sim_console.c sim_fio.c sim_timer.c sim_sock.c sim_tmxr.c sim_ether.c sim_tape.c sim_disk.c sim_serial.c sim_video.c sim_imd.c sim_card.c   -DVM_PDP11 -I ./PDP11 -DHAVE_PCAP_NETWORK -I/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/ -DBPF_CONST_STRING -DUSE_SHARED -DHAVE_VDE_NETWORK -I slirp -I slirp_glue -I slirp_glue/qemu -DHAVE_SLIRP_NETWORK -DUSE_SIMH_SLIRP_DEBUG  -DUSE_DISPLAY -DHAVE_LIBSDL -I/opt/local/include/SDL2 -D_THREAD_SAFE -DUSE_SIM_VIDEO  -o BIN/pdp11 -L/opt/local/lib -lpthread -lpcre -L/opt/local/lib/ -ledit -ltermcap -L/opt/local/lib/ -lpng -lz -lvdeplug -L/opt/local/lib/ `sdl2-config --libs`  
BIN/pdp11  RegisterSanityCheck  </dev/null 
Running internal register sanity checks on PDP-11 simulator.
*** Good Registers in PDP-11 simulator.

the simulator configuration file (or commands) which were used when the problem occurred.


; PDP-11 simh configuration to match the old LSRHS PDP-11/70
;
; with inspiration from http://gunkies.org/wiki/Installing_2.9_BSD_on_SIMH
; Bjoren Davis, April 24, 2022
;
set cpu 11/70
set cpu 2M
set cpu fpp
set cpu idle
set pclk ena        ; gotta do this before turning off
                ;  autoconf or it will never work.
;set pclk vector=104
echo Ignore this warning
echo -----------------------------------------------------
set noautoconf
echo -----------------------------------------------------
set ptr dis
set ptp dis
set rk dis
set rl dis
set hk dis
set rx dis
set rq dis
set tm dis
set tu ena
set tq dis
set cr ena
set cr address=17777160 vector=230
set dz lines=32
; SIMH seems to require that multiple instances of the same
;  board have contiguous addresses.  That seems really stupid.
set dz address=17760110 vector=330
;set dz0 address=17760110 vector=330
;set dz1 address=17760120 vector=340
;set dz2 address=17760130 vector=350
;set dz3 address=17760140 vector=360
set rom enable
set rom0 address=17765000
attach rom0 23-616F1-1666.bin
set rom1 address=17773000
attach rom1 23-755A9-0622.bin
set rom2 address=17773200
attach rom2 23-757A9-06F2.bin
set dh enable
set dh address=17760020 vector=320
set tu1 disable
set tu2 disable
set tu3 disable
set tu4 disable
set tu5 disable
set tu6 disable
set tu7 disable
;set rha address=17776300 vector=150    ; RP04
set rha address=17776700 vector=254     ; RM03?
set rhb address=17772440 vector=224     ; TU16
;;
;; Apparently we had 3 Massbus controllers, but I can't
;;  figure out how to get SIMH to accept more than 2.
;;
;set rhc address=17776700 vector=150    ; RM03
;set rp0 rp04
;att rp0 rp04.disk
set rp0 rm03
att rp0 rm03.disk
set rp1 dis
set rp2 dis
set rp3 dis
set rp4 dis
set rp5 dis
set rp6 dis
set rp7 dis
set tti 7b
set tto 7b
set dz 7b
attach dz -m 4429,speed=*8
attach dh -m 4430,speed=*8

;attach tu0 /Users/bkd/projects/2023/bhtapes/bh1/img/bh1_read5_rev_25ips_fixed.tap ;attach tu0 /Users/bkd/projects/2023/bhtapes/bh2/ex/bh2_bh3_unified.tap ;attach tu0 /Users/bkd/projects/2023/bhtapes/bh2/img/bh2_read2.tap ;attach tu0 /Users/bkd/projects/2023/bhtapes/bh3/img/bh3_read2.tap echo echo At the sim> prompt, type 'boot rp0' echo Then into the empty space type 'go' echo echo And then before timesharing starts, type ^E echo and issue the following commands to echo set the clock (assuming you booted echo /unix): echo ! ./time_deposit_cmds.sh echo

<!--- The simulator configuration file, and any other relatively small files can be attached to this issue now or after it is created -->
- #### the expected behavior and the actual behavior
The behavior I see is bizarre results when trying to use DH-11 serial devices. Among these results are:
- processes that write to DH-11 devices hang (as if the kernel is waiting for an interrupt on their behalf that never arrives)
- output, when it happens, can be directed to the wrong device (as seen in the debug logs)
- inability to log in when telneting into a DH-11 port that has login running on it (I never see the login: prompt)

When I make the following change:

bkd@titus simh % git diff PDP11/pdp11_dh.c | dos2unix diff --git a/PDP11/pdp11_dh.c b/PDP11/pdp11_dh.c index 030255cc..a0866d14 100644 --- a/PDP11/pdp11_dh.c +++ b/PDP11/pdp11_dh.c @@ -191,6 +191,17 @@ dh_wr(int32 data, int32 PA, int32 access) switch (PA & 017) { case 000: sim_debug (DBG_IO, &dh_dev, "WRITE DHSCR %06o\n", data);

if (access == WRITEB) {
/ Only writing one byte of "SCR" /
if ((PA & 1) == 0) {
/ Even byte offset /
data = (dh_scr & ~0377) | data;
}
else {
/ Odd byte offset /
data = (dh_scr & 0377) | (data << 8);
}
} dh_scr = data; if (data & MCLR) dh_reset (&dh_dev);
```
these problems go away.
```
you may also need to provide specific pointers to data files that may be necessary to demonstrate the problem

A couple of other points, if I may:

I found that the DH-11 simulation seems to believe that everything runs at 300 baud, and no matter what I do I can't change its mind. Should I submit that as another bug?
This exact same issue exists in [https://github.com/simh/simh.git]. Should I submit the problem there, too? Please forgive my ignorance, but I'm a little confused as to which is the canonical project. Thanks!

pkoning2 commented 1 year ago

Here is fine. Thanks Bjoren, that looks like a good fix. The DH-11 manual says clearly this register is byte writable, so this seems like an omission in the original code. It describes one of the others as only word-writable, and it doesn't say about most of the rest.

pkoning2 commented 1 year ago

On the 300 baud thing, yes, another issue would be good. Please show what the code is trying to do and what the resulting registers are.

pkoning2 commented 1 year ago

Judging from a quick test and comparing against a DHU line, it seems that the emulated speed of the DH line is 1200 baud, give or take -- and it doesn't change when I change the speed in the device CSR.

pkoning2 commented 1 year ago

Fixed by PR 204.

open-simh / simh