search

open-simh / simh

The Open SIMH simulators package
https://opensimh.org/
Other
469 stars 89 forks source link

RA92 disk VHD files created too small, correctly size ones fail to attach. #287

Closed rdebath closed 1 year ago

rdebath commented 1 year ago

The error is given %SIM-ERROR: RQ2: 'd2.vhd' can only be attached Read Only

Layout: 73*13*3099*512, sectors 2940951
Required disk size: 1505766912 
Created by old version of Simh
file format: raw
virtual size: 1.4 GiB (1505766912 bytes)
Created by Simh (with or without "set noautosize")
file format: raw
virtual size: 1.4 GiB (1505767424 bytes)
Created by Simh
file format: vpc
virtual size: 1.4 GiB (1505452032 bytes)
Created by qemu-img
file format: vpc
virtual size: 1.4 GiB (1505968128 bytes)

MicroVAX 3900 simulator Open SIMH V4.1-0 Current git commit id: 348f5f29 Running Hardware Core Test (EHKAA)

*** PASSED - MicroVAX 3900 Hardware Core Instruction test EHKAA

- #### the simulator configuration file (or commands) which were used when the problem occurred.
<!--- The simulator configuration file, and any other relatively small files can be attached to this issue now or after it is created -->

set rq2 ra92 set rq2 format=vhd attach rq2 d2.vhd


- #### the expected behavior and the actual behavior
<!--- Please provide the output the simulator produced when you experienced the problem -->
`%SIM-ERROR: RQ2: 'd2.vhd' can only be attached Read Only`
Expected: `sim>`

- #### you may also need to provide specific pointers to data files that may be necessary to demonstrate the problem
For other files; create a 1505766912 byte file using an old version or just: `truncate -s 1505766912 d2.dsk`
convert it to vpc/vhd `qemu-img convert -f raw -O vpc d2.dsk d2.vhd`
Run the ra92 script above.

<!--- If you haven't already, please be sure that your full name is visible in your Github profile (no email address is needed) -->
pkoning2 commented 1 year ago

That's odd, it works fine for me, the exact same build, running on a Mac. Out of curiosity, why are you using VHD rather than plain (raw) containers?

pkoning2 commented 1 year ago

Also, are you really running this on a 32 bit machine?

pkoning2 commented 1 year ago

32 bit isn't the explanation: I just tried this on a 32 bit ARM machine (Linux) and it works fine.

rdebath commented 1 year ago

I'm switching to VHDs because simh is breaking it's raw images, my other tools can deal with VHD but not corrupted raw disk files.

Also my root disk is the first 17Mb partition of a small drive, the rest of the drive isn't touched so never allocated in the old simh, now it gets allocated and debris put on the end.

That is a 32bit user space, yes. The kernel is 64bit ... are you mis-identifying something? Though I really don't see how you could break it in this small a fashion if you are identifying the kernel rather than the userspace.

Which version of gcc did you use on your ARM test? Would you like me to make a i386 kernel test in an x86 VM, I have a Debian bullseye that I can copy and quickly turn into a build VM. (or even making a "bookworm" one wouldn't take long)

pkoning2 commented 1 year ago

We'll fix the raw thing at some point. You can cure it manually with the "zap" command. I don't know of any reason why things wouldn't work with 32 bit, but I wanted to understand the specifics of what you built. Could you supply the command and the log from the build? The ARM compiler was gcc 8.3, so I will go back and try a newer one. I'll want to match what you have so please supply all the details.

pkoning2 commented 1 year ago

Ok, I should have read more carefully, I see your build procedure in the original note. But you said it's a 64 bit kernel, so how does it manage to have a 32 bit user environment? I suppose that's possible, but 32 bit support is really just for running old binaries, not as a normal user environment.

rdebath commented 1 year ago

I think the 64bit kernel is a red herring; I remembered there's a command called setarch that hides the kernel difference from the running program. There's no difference.

I also have a Debian Stretch i386 and amd64 chroots ( GCC Version: 6.3.0.) -- both still too small.

I'll get an arm cooking... it's only a PI so it'll take a while.

Meanwhile...

To use a 64bit kernel with a 32 bit userspace all you have to do with Debian i386 bookworm is run apt install linux-image-6.1.0-11-amd64 and reboot. With that kernel you can run chroot environments for any of the ELF based Debian distributions (that's back to Debian potato for i386, x64 and x64_32). The main userspace usually has to be a pretty close variant so that all the executables are runnable so you probably need a userspace of +/- two or three major distribution releases. Though a full i386 userspace on a 64bit kernel has worked basically since day one; I think the first iteration had minor issues with iptables.

The i386/amd64 ports, mutli-arch and mixed operation support with amd64. i386 and x32 is fully supported by Debian (except x32 isn't a full port). Using the multiarch you can install packages from any architecture your kernel (or binfmtmisc) can run. Once you add qemu into the mix you get a lot of emulated chroots too, reading off my list I've got i386, amd64, x64_32, m68k, ppc64, riscv64, s390x, sh4, arm, ppc, and sparc.

Working mixed architecture installs are more limited because you can't install multiple copies of one package and sometimes the architecture between packages has to match.

tmp$ which hello
hello runs: /usr/bin/hello
/usr/bin/hello: ELF 32-bit LSB pie executable, ARM, EABI5 version 1 (SYSV), dynamically linked, interpreter /lib/ld-linux-armhf.so.3, BuildID[sha1]=7909bfeae73fe9498326c4c136dde55f811493b4, for GNU/Linux 3.2.0, stripped
hello is /usr/bin/hello
hello is /bin/hello
tmp$ uname -a
Linux mayday 6.1.0-11-amd64 #1 SMP PREEMPT_DYNAMIC Debian 6.1.38-4 (2023-08-08) x86_64 GNU/Linux
tmp$ getconf LONG_BIT
32
tmp$ file /usr/bin/qemu-arm-static
/usr/bin/qemu-arm-static: ELF 64-bit LSB executable, x86-64, version 1 (GNU/Linux), statically linked, BuildID[sha1]=26040003e479631388c56805942ea99b7fcf8e1f, for GNU/Linux 3.2.0, stripped
tmp$ file /usr/bin/file
/usr/bin/file: ELF 32-bit LSB pie executable, Intel 80386, version 1 (SYSV), dynamically linked, interpreter /lib/ld-linux.so.2, BuildID[sha1]=51de4a7fa64d0f5eb63bb026eca39b9617038653, for GNU/Linux 3.2.0, stripped
tmp$
pkoning2 commented 1 year ago

I try to avoid Debian stuff, but I'll see how best to try to reproduce this. I'm wondering though: if you have a 64 bit machine, why would you run 32 bit code? I can't see the point of that, other than as a dancing bear experiment. It's not quite as bad for VAX as it is for some of the other emulators which really need 64 bit arithmetic, but it still seems unusual. That's not to say it shouldn't work, but the most natural thing to do is to build and run the ISA that's native to your hardware.

rdebath commented 1 year ago

All three variations are native to the machines, and 32bit code is smaller and rarely slower than even the fake 64bit that amd64 uses let alone real 64bit (ILP64) which no one has used since the Cray days (more or less) .

But like you said; don't worry about it, it's not a problem here.

Below is a script running on a raspberry pi (32bit). The ra92 vhd's virtual size is several hundred kB smaller than the raw image size.

Script started on 2023-08-29 21:11:23+01:00 [TERM="putty" TTY="/dev/pts/0" COLUMNS="80" LINES="25"]
$ make vax 
lib paths are: /lib/ /lib/arm-linux-gnueabihf/ /opt/vc/lib/ /usr/lib/ /usr/lib/arm-linux-gnueabihf/ /usr/lib/arm-linux-gnueabihf/libfakeroot/
include paths are:  /usr/lib/gcc/arm-linux-gnueabihf/8/include /usr/local/include /usr/lib/gcc/arm-linux-gnueabihf/8/include-fixed /usr/include/arm-linux-gnueabihf /usr/include
using libm: /usr/lib/arm-linux-gnueabihf/libm.so
using librt: /usr/lib/arm-linux-gnueabihf/librt.so
using libpthread: /usr/lib/arm-linux-gnueabihf/libpthread.so /usr/include/pthread.h
using libpcre: /usr/lib/arm-linux-gnueabihf/libpcre.so /usr/include/pcre.h
using semaphore: /usr/include/semaphore.h
using libdl: /usr/lib/arm-linux-gnueabihf/libdl.so /usr/include/dlfcn.h
using libpng: /usr/lib/arm-linux-gnueabihf/libpng.so /usr/include/png.h
using zlib: /usr/lib/arm-linux-gnueabihf/libz.so /usr/include/zlib.h
using mman: /usr/include/arm-linux-gnueabihf/sys/mman.h
*** Info ***
*** Info *** The simulator you are building could provide more functionality
*** Info *** if video support was available on your system.
*** Info *** To gain this functionality:
*** Info *** Install the development components of libSDL2 packaged for
*** Info *** your operating system distribution for your Linux
*** Info *** system:
*** Info ***        $ sudo apt-get install libsdl2-dev libpng-dev
*** Info ***
*** Warning ***
*** Warning *** vax Simulator is being built WITHOUT
*** Warning *** libpcap networking support
*** Warning ***
*** Warning *** To build simulator(s) with libpcap networking support you
*** Warning *** should install the libpcap development components for
*** Warning *** for your Linux system:
*** Warning ***        $ sudo apt-get install libpcap-dev
*** Warning ***
*** Info ***
*** Info *** Simulators on your Linux platform can also be built with
*** Info *** extended LAN Ethernet networking support by using VDE Ethernet.
*** Info ***
*** Info *** To build simulator(s) with extended networking support you
*** Info *** should install the vde2 package to provide this
*** Info *** functionality for your Linux system:
*** Info ***        $ sudo apt-get install vde2
*** Info ***
***
*** vax Simulator being built with:
*** - compiler optimizations and no debugging support. GCC Version: 8.3.0.
*** - Local LAN packet transports: TAP NAT(SLiRP)
*** - Per simulator tests will be run.
***
*** git commit id is 348f5f294415202350a2fa80a839f3cd8e4a6e3f.
*** git commit time is 2023-07-31T10:34:33-0700.
***
gcc -std=gnu99 -U__STRICT_ANSI__  -O2 -DNDEBUG=1 -finline-functions -fgcse-after-reload -fpredictive-commoning -fipa-cp-clone -fno-unsafe-loop-optimizations -fno-strict-overflow -DSIM_GIT_COMMIT_ID=348f5f294415202350a2fa80a839f3cd8e4a6e3f -DSIM_GIT_COMMIT_TIME=2023-07-31T10:34:33-0700  -DSIM_COMPILER="GCC Version: 8.3.0" -DSIM_BUILD_TOOL=simh-makefile -I . -Werror -D_GNU_SOURCE -DUSE_READER_THREAD -DSIM_ASYNCH_IO  -DHAVE_PCRE_H -DHAVE_SEMAPHORE -DHAVE_SYS_IOCTL -DHAVE_LINUX_CDROM -DSIM_HAVE_DLOPEN=so -DHAVE_UTIME -DHAVE_LIBPNG -DHAVE_ZLIB -DHAVE_GLOB -DHAVE_SHM_OPEN  ./VAX/vax_cpu.c ./VAX/vax_cpu1.c ./VAX/vax_fpa.c ./VAX/vax_io.c ./VAX/vax_cis.c ./VAX/vax_octa.c  ./VAX/vax_cmode.c ./VAX/vax_mmu.c ./VAX/vax_stddev.c ./VAX/vax_sysdev.c ./VAX/vax_sys.c  ./VAX/vax_syscm.c ./VAX/vax_syslist.c ./VAX/vax_vc.c ./VAX/vax_lk.c ./VAX/vax_vs.c ./VAX/vax_2681.c ./PDP11/pdp11_rl.c ./PDP11/pdp11_rq.c ./PDP11/pdp11_ts.c ./PDP11/pdp11_dz.c ./PDP11/pdp11_lp.c ./PDP11/pdp11_tq.c ./PDP11/pdp11_xq.c ./PDP11/pdp11_vh.c ./PDP11/pdp11_cr.c ./PDP11/pdp11_td.c ./PDP11/pdp11_io_lib.c ./PDP11/pdp11_dup.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_VAX -DUSE_INT64 -DUSE_ADDR64 -DUSE_SIM_VIDEO -I ./VAX -I ./PDP11 -DHAVE_TAP_NETWORK -DUSE_NETWORK -Islirp -Islirp_glue -Islirp_glue/qemu -DHAVE_SLIRP_NETWORK -DUSE_SIMH_SLIRP_DEBUG slirp/*.c slirp_glue/*.c   -o BIN/vax -lm -lrt -lpthread -lpcre -ldl -lpng -lz  
cp BIN/vax BIN/microvax3900
BIN/vax RegisterSanityCheck /home/pi/simh/VAX/tests/vax-diag_test.ini </dev/null 
 Running internal register sanity checks on MicroVAX 3900 simulator.
*** Good Registers in MicroVAX 3900 simulator.

MicroVAX 3900 simulator Open SIMH V4.1-0 Current        git commit id: 348f5f29
Running Hardware Core Test (EHKAA)

*** PASSED - MicroVAX 3900 Hardware Core Instruction test EHKAA

$ BIN/vax

MicroVAX 3900 simulator Open SIMH V4.1-0 Current        git commit id: 348f5f29
sim> set rq0 rd53
set rq1 ra81
set rq2 ra92
sim> sim> sim> 
sim> set rq0 format=vhd
set rq1 format=vhd
set rq2 format=vhd
sim> sim> sim> 
sim> attach rq0 d0.vhd
attach rq1 d1.vhd
attach rq2 d2.vhd
%SIM-INFO: RQ0: Creating new file: d0.vhd
sim> %SIM-INFO: RQ1: Creating new file: d1.vhd
sim> %SIM-INFO: RQ2: Creating new file: d2.vhd

sim> sim> 
sim> 
sim> exit
Goodbye
$ ls -l *vhd
-rw-r--r-- 1 pi pi 2560 Aug 29 21:16 d0.vhd
-rw-r--r-- 1 pi pi 3072 Aug 29 21:16 d1.vhd
-rw-r--r-- 1 pi pi 5120 Aug 29 21:16 d2.vhd
$ qemu-img info d0.vhd
image: d0.vhd
file format: vpc
virtual size: 68M (70955008 bytes)
disk size: 4.0K
cluster_size: 2097152
$ qemu-img info d1.vhd
image: d1.vhd
file format: vpc
virtual size: 435M (456228864 bytes)
disk size: 4.0K
cluster_size: 2097152
$ qemu-img info d2.vhd
image: d2.vhd
file format: vpc
virtual size: 1.4G (1505452032 bytes)
disk size: 8.0K
cluster_size: 2097152
$ BIN/vax

MicroVAX 3900 simulator Open SIMH V4.1-0 Current        git commit id: 348f5f29
sim> set rq0 rd53
set rq1 ra81
set rq2 ra92
sim> sim> sim> 
sim> attach rq0 d0.dsk
%SIM-INFO: RQ0: Creating new file: d0.dsk
sim> attach rq1 d1.dsk
%SIM-INFO: RQ1: Creating new file: d1.dsk
attach rq2 d2.dsk

sim> %SIM-INFO: RQ2: Creating new file: d2.dsk
sim> sim> 
sim> exit
Goodbye
$ ls -l d?.dsk
-rw-r--r-- 1 pi pi   71000576 Jan  1  1970 d0.dsk
-rw-r--r-- 1 pi pi  456229376 Jan  1  1970 d1.dsk
-rw-r--r-- 1 pi pi 1505767424 Jan  1  1970 d2.dsk
$ date
Tue 29 Aug 21:40:23 BST 2023
$ exit

Script done on 2023-08-29 21:40:30+01:00 [COMMAND_EXIT_CODE="0"]
pkoning2 commented 1 year ago

Could you do a SIMH "DISKINFO" command on each of those container files?

rdebath commented 1 year ago

So can simh create fixed size VHD files ? They'd look exactly like the new fake raw files, down to the exact size.

Empty files created by vax on the pi.

sim>
sim> diskinfo d0.dsk
Container:              ~/simh/d0.dsk
   Simulator:           MicroVAX 3900
   DriveType:           RD53
   SectorSize:          512
   SectorCount:         138672
   TransferElementSize: 2
   AccessFormat:        SIMH
   CreationTime:        Wed Aug 30 06:52:37 2023
   DeviceName:          RQ
   HighwaterSector:     138672
Container Size: 71,000,064 bytes
sim> diskinfo d0.vhd
Container:              ~/simh/d0.vhd
   Simulator:           MicroVAX 3900
   DriveType:           RD53
   SectorSize:          512
   SectorCount:         138672
   TransferElementSize: 2
   AccessFormat:        VHD
   CreationTime:        Wed Aug 30 06:50:25 2023
Container Size: 71,000,064 bytes
sim> diskinfo d1.dsk
Container:              ~/simh/d1.dsk
   Simulator:           MicroVAX 3900
   DriveType:           RA81
   SectorSize:          512
   SectorCount:         891072
   TransferElementSize: 2
   AccessFormat:        SIMH
   CreationTime:        Wed Aug 30 06:53:26 2023
   DeviceName:          RQ
   HighwaterSector:     891072
Container Size: 456,228,864 bytes
sim> diskinfo d1.vhd
Container:              ~/simh/d1.vhd
   Simulator:           MicroVAX 3900
   DriveType:           RA81
   SectorSize:          512
   SectorCount:         891072
   TransferElementSize: 2
   AccessFormat:        VHD
   CreationTime:        Wed Aug 30 06:50:25 2023
Container Size: 456,228,864 bytes
sim> diskinfo d2.dsk
Container:              ~/simh/d2.dsk
   Simulator:           MicroVAX 3900
   DriveType:           RA92
   SectorSize:          512
   SectorCount:         2940951
   TransferElementSize: 2
   AccessFormat:        SIMH
   CreationTime:        Wed Aug 30 06:56:36 2023
   DeviceName:          RQ
   HighwaterSector:     2940951
Container Size: 1,505,766,912 bytes
sim> diskinfo d2.vhd
Container:              ~/simh/d2.vhd
   Simulator:           MicroVAX 3900
   DriveType:           RA92
   SectorSize:          512
   SectorCount:         2940951
   TransferElementSize: 2
   AccessFormat:        VHD
   CreationTime:        Wed Aug 30 06:50:27 2023
Container Size: 1,505,766,912 bytes
sim> exit
Goodbye

Unconverted files old version dsk and qemu vhd.

$ ../open-simh/BIN/vax

MicroVAX 3900 simulator Open SIMH V4.1-0 Current        git commit id: 348f5f29
sim> diskinfo d2.dsk
Container Info for '~/Ultrix/CurrentSetup/d2.dsk' unavailable
Container Size: 1,505,766,912 bytes
sim> diskinfo d2.vhd
Container:              ~/Ultrix/CurrentSetup/d2.vhd
   Simulator:           MicroVAX 3900
   DriveType:
   SectorSize:          0
   SectorCount:         0
   TransferElementSize: 0
   AccessFormat:        VHD
   CreationTime:        Sat Aug 26 20:00:14 2023
Container Size: 1,505,968,128 bytes
sim>
$ qemu-img info ~/Ultrix/CurrentSetup/d2.vhd
image: /home/robert/Ultrix/CurrentSetup/d2.vhd
file format: vpc
virtual size: 1.4 GiB (1505968128 bytes)
disk size: 842 MiB
cluster_size: 2097152
pkoning2 commented 1 year ago

The reported size, for VHD files, is the virtual device capacity. If you do ls -l you'll see the actual container is much smaller. So it should match the other format, after all they are both containers for RA92 disks and that's the sector count for an RA92. The fact that they match (and are the right number) says they were created correctly.

markpizz commented 1 year ago

@rdebath I suspect you'll get different results if you try the vax simulator built from https://github.com/simh/simh

Try DISKINFO and see HELP RQ ATTACH and HELP ZAP for potentially useful information.

pkoning2 commented 1 year ago

I don't see why, and in any case that's off topic.

markpizz commented 1 year ago

I don't see why, and in any case that's off topic.

If you don't look, you never will see the relevance, and it certainly isn't off topic.

The disk and VHD functionality in that repository has been extensively modified during the past 15 months. Bugs fixed, functionality added and enhanced.

@rdebath's issues may just work fine there as he expected.

I suggested looking at HELP since his various comments suggested he wasn't available about functionality that was already available in open-simh and has been enriched since.

BTW: Running in circles about 32bit vs 64bit was off topic.

pkoning2 commented 1 year ago

Looking at the closed "simh" repository is not an option given the license. If you're willing to port your fixed here, great. If not, it is absolutely off topic. Maybe the best answer is to remove the VHD format.

rdebath commented 1 year ago

@markpizz Actually git blame says the problem is your code; you copied Microsoft's "pseudo code" to C without thinking about it.

This line is copied from Microsoft's DOC file describing the VHD format. But it only works if the total sectors variable is already generated from a CHS combination. If not C will round the number of cylinders down to the next integer making the size according to the CHS specification too small.

After rounding up the cylinders qemu-img appears to copy that value back into the originalSize field to round that up too and make it consistent with the exact value of the CHS with 512 byte sectors. Presumably because VirtualPC does the same.

Also the size qemu-img info shows is based on the CHS not the originalSize.

Obviously this doesn't work if the CHS is "65535,16,255", the maximum value (Geometry field =0xffff10ff ).

Okay, looks like it's even more confusing :grin:

pkoning2 commented 1 year ago

I haven't looked at the VHD spec. Are you saying that it describes the device size by a C/H/S triple? That can't work if the actual size can't be factored into valid C/H/S values.

rdebath commented 1 year ago

It has both. There's an originalSize in bytes, a currentSize in bytes and a diskGeometry field containing CHS. Qemu's vpc driver seems to assume the CHS is the master unless it's got good reason to note otherwise. If the drive is too large for CHS the currentSize must be right. If it's a VirtualPC file (by the creator APP) then it's CHS. If the creator APP is Hyper-V then the CurrentSize in bytes is correct. ... Microsoft is inconsistent.

Looks like qemu has basically given up and decided that rounding up a small disk to a valid CHS is the only safe option.

markpizz commented 1 year ago

@pkoning2

Looking at the closed "simh" repository is not an option given the license. If you're willing to port your fixed here, great. If not, it is absolutely off topic.

I didn't say you or anyone should look there. I suggested he try to use what's there. The topic, as stated, and the conversation was all about the specific problem(s) he was seeing. I'm sorry that you think a potential solution to the user's actual problem is off topic. I don't see how.

Maybe the best answer is to remove the VHD format.

Feel free to rip it out if you want. Somehow I don't think that will help with the stated problem.

pkoning2 commented 1 year ago

The issues section of this repository, as with any other repository, is for issues in this code. So "look somewhere else" is by definition off topic, unless that somewhere else can be used to provide a fix for this repository, which as we know is not the case.

pkoning2 commented 1 year ago

It has both. There's an originalSize in bytes, a currentSize in bytes and a diskGeometry field containing CHS. Qemu's vpc driver seems to assume the CHS is the master unless it's got good reason to note otherwise. If the drive is too large for CHS the currentSize must be right. If it's a VirtualPC file (by the creator APP) then it's CHS. If the creator APP is Hyper-V then the CurrentSize in bytes is correct. ... Microsoft is inconsistent.

Looks like qemu has basically given up and decided that rounding up a small disk to a valid CHS is the only safe option.

If qemu is using CHS for the size rather than the size fields, I would argue they are wrong because the words of the spec doesn't support that. It says CHS is for ATA emulation, which of course isn't what we're doing in SIMH.

Then again, it would also make sense to argue that the Microsoft spec is severely broken because the "algorithm" for calculating CHS only makes sense if the size is in fact a product of C, H, and S values in the specified ranges. For any other size it produces an outcome that describes a smaller device, as you pointed out. But rounding up doesn't make sense either.

So what is the actual problem? Let's ignore QEMU since its apparent reliance on the CHS value makes it unsupportable. Do you have a case where a VHD file created by SIMH is mishandled by SIMH? I thought that's what you originally reported but I can't reproduce that.

rdebath commented 1 year ago

My problem is simh no longer treating raw files as raw files. I want to fix that by converting the disk files to vhd. So I need something to convert the files ... qemu-img is obvious. But it gave results that strongly implied simh was doing something wrong with VHD files. I have now successfully converted the files to something that simh likes.

Nevertheless, I think you should make a minor change or two as below.

I've been digging; we don't want to ignore qemu 'cause they've been here. Nevertheless, I think you're right that simh shouldn't round up the CurrentSize field just because the AT controller had limitations.

But I do think the cylinders field should be fixed so that qemu-img and other tools don't truncate the image by accident. Doing this should make that safe; it will mean that those tools extend the file, but at least that can be fixed.

diff --git a/sim_disk.c b/sim_disk.c
index 4b9d0dfa..d252f158 100644
--- a/sim_disk.c
+++ b/sim_disk.c
@@ -5758,7 +5758,7 @@ if (1) { /* CHS Calculation */
             cylinderTimesHeads = totalSectors / sectorsPerTrack;
             }
         }
-    cylinders = cylinderTimesHeads / heads;
+    cylinders = (cylinderTimesHeads+heads-1) / heads;
     Footer.DiskGeometry = NtoHl ((cylinders<<16)|(heads<<8)|sectorsPerTrack);
     }
 Footer.Checksum = NtoHl (CalculateVhdFooterChecksum(&Footer, sizeof(Footer)));

But otherwise simh doesn't need to make any other changes because qemu has been there.

Specifically, qemu has some ugly syntax to read simh VHD files as intended ...

qemu-img info --image-opts driver=vpc,force_size_calc=current_size,file.driver=file,file.filename=disk.vhd

And a somewhat less ugly way of converting raw files to vhd files in the way that simh (and Amazon) likes.

qemu-img convert -f raw -O vpc -o subformat=dynamic,force_size disk.dsk disk.vhd

Then back to the ugly for converting from vhd.

qemu-img convert --image-opts driver=vpc,force_size_calc=current_size,file.driver=file,file.filename=disk.vhd -O qcow2 disk.qcow2

It might be reasonable to include those command lines at the back of some notebook.

markpizz commented 1 year ago

But I do think the cylinders field should be fixed so that qemu-img and other tools don't truncate the image by accident. Doing this should make that safe; it will mean that those tools extend the file, but at least that can be fixed.

diff --git a/sim_disk.c b/sim_disk.c
index 4b9d0dfa..d252f158 100644
--- a/sim_disk.c
+++ b/sim_disk.c
@@ -5758,7 +5758,7 @@ if (1) { /* CHS Calculation */
             cylinderTimesHeads = totalSectors / sectorsPerTrack;
             }
         }
-    cylinders = cylinderTimesHeads / heads;
+    cylinders = (cylinderTimesHeads+heads-1) / heads;
     Footer.DiskGeometry = NtoHl ((cylinders<<16)|(heads<<8)|sectorsPerTrack);
     }
 Footer.Checksum = NtoHl (CalculateVhdFooterChecksum(&Footer, sizeof(Footer)));

But otherwise simh doesn't need to make any other changes because qemu has been there.

Interesting.

CHS isn't used in any way in simh, I just followed the spec when I implemented VHD support. Clearly, qemu provides simulation of ATA devices with both VHD and any file based disk container, so getting that computation right must have been solved long ago and it was likewise info that the ATA hardware implementations must have needed as well.

Your suggested change is certainly reasonable. Historically, we've taken a small patch like this as a change that would be committed with the person who supplied it as author. Any more detail than rdebath would help with that...

rdebath commented 1 year ago

Good point, this git am file has been fixed; the previous patch was still wrong (for some simh drives) due to Microsoft doing yet another weird thing calculating the CHS value.

I release this under the MIT License as described in "Contributions will be covered by the project license"

Note: though this prevents other tools damaging the disk badly they will still extend the disk to the CHS length. Because of this I suggest that the disk is created by qemu like this: qemu-img create -f vpc -o subformat=dynamic,force_size disk.vpc 1505766912 Or labeled with one of the other creators (eg: Hyper-V) that are known to use the currentSize rather than the CHS fields.

From 9eab728298dac5cc8ff9c0f6f9fbb6febb7429bf Mon Sep 17 00:00:00 2001
From: Robert de Bath <rdebath@tvisiontech.co.uk>
Date: Fri, 1 Sep 2023 18:39:38 +0100
Subject: [PATCH] Ensure that the CHS capacity exceeds the disk length

If the total sectors exceeds 127Gb this is not possible, but simh
disks are smaller.  For these disks this change prevents "qemu-img"
and any other application that assumes the CHS size is the priority
from truncating the disk.
---
 sim_disk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sim_disk.c b/sim_disk.c
index 4b9d0dfa..ba040ef7 100644
--- a/sim_disk.c
+++ b/sim_disk.c
@@ -5758,7 +5758,7 @@ if (1) { /* CHS Calculation */
             cylinderTimesHeads = totalSectors / sectorsPerTrack;
             }
         }
-    cylinders = cylinderTimesHeads / heads;
+    cylinders = (totalSectors+sectorsPerTrack*heads-1) / (sectorsPerTrack*heads);
     Footer.DiskGeometry = NtoHl ((cylinders<<16)|(heads<<8)|sectorsPerTrack);
     }
 Footer.Checksum = NtoHl (CalculateVhdFooterChecksum(&Footer, sizeof(Footer)));
--
2.39.2
markpizz commented 1 year ago

@rdebath you say:

If the total sectors exceeds 127Gb this is not possible, but simh disks are smaller. For these disks this change prevents "qemu-img" and any other application that assumes the CHS size is the priority from truncating the disk.

Which is true for all of the normal disks which emulate what was sold by DEC, however the MSCP RQ devices have long allowed for user a settable sized disk type, and the size limit here is just under a 1 TB.

 sim> set rq0 -b RAUSER=1048575
 sim> show -b rq0
 RQ0     1048575MB, not attached, write enabled
             RA8U, UNIT=0, autosize
             AUTO detect format

There weren't any operating systems sold by DEC which could handle a file system this large, but I wouldn't be surprised if a recent version of NetBSD could actually make use of such a device (in theory anyway).

How would your external VHD tools handle such a device? The CHS would then be 0XFFFF10FF for all larger disks, right?

pkoning2 commented 1 year ago

Thanks, the fix is merged.

rdebath commented 1 year ago

@markpizz If the CHS is set to the maximum this means that the drive is a big IDE using LBA so yes, linear sectors for everything.