[kernel] Introduce block device driver overlays, move ATA CF to /dev/cf0

[ghaerr]

Adds block device overlay concept, described in #2305 (comment).

For now, the driver tries to set 8-bit read/write ATA transfers if the ATA chip supports it. This is untested since QEMU doesn't support 8-bit transfers.

@fhendrikx, if you could test this on Solo/86, that would be great. This could possibly also work on @toncho11's Amstradt, but the 8-bit code is completely untested. The boot screen now shows more data from the ATA identify information buffer, for debugging purposes.

Currently, the ATA-CF (now /dev/cf0) driver only works with CF cards inserted at boot time; the at_init() code will be moved in a later commit to the driver open code so that CF cards can be swapped when not mounted.

[fhendrikx]

@ghaerr I did some initial testing as requested. I used a 4096MB Verbatim (CF0) and a 16MB Nikon card (CF1).

• "sanity checks" fail for both of these cards. We need to rethink this, as these are not large cards. I temporarily removed these checks.
• 8-bit mode working nicely under Solo/86 (for the 4096MB CF card; the 16MB card not tested, see below).

• On the 16MB CF card, the "capability word" reports no LBA in 8-bit mode (but it does in 16-bit mode), and changes the number of reported heads/cylinders.

Both cards in 16-bit mode (notice the CAPability words and CHS):

Both cards in 8-bit mode (notice the CAPability words and CHS):

If you're happy, I can do some further work to try and get both working, maybe detect 8-bit transfers (still thinking about this) and push to this PR?

[toncho11]

Please comment out total=0 in the sanity checks, so that we can see the messages, but still execute the code. I am unable to compile at this moment. I will use the github actions builds. I found my xtide+cf card.

[toncho11]

@fhendrikx you use mount /dev/hda mnt for cf-card, but I though it should be: mount /dev/cf0 mnt? How does this work?

[fhendrikx]

@fhendrikx you use mount /dev/hda mnt for cf-card, but I though it should be: mount /dev/cf0 mnt? How does this work?

That's the new remapping code, presumably.

[ghaerr]

Hello @fhendrikx,

Thanks for your testing! Very strange results with CAP word changing in 8-bit mode, but happy to hear initial reports of 8-bit mode working. I've been unable to test anything in 8-bit or real hardware since I'm only using QEMU at the moment, and its ATA emulation seems a bit incomplete.

I went backwards to the ATA ACS-1 spec where the CHS capability words were valid, they're technically obsolete in ACS-3+, but I added them because QEMU allows communication with the ATA interface even when no hard drive is attached, which then seems to return random data. I have no problem with deleting most of the sanity checks, but it would be nice if we found one that worked for both hard drives attached with QEMU (for my continued testing and maintenance) as well as the more modern CF cards. Perhaps look at the sector size ATA_INFO_SECTSIZE, is that 512 on your device? Without additional checks, we're dependent on the single 0x200 bit in word 49 ATA_INFO_CAPS as to whether to accept a device or not, which I felt a bit lacking.

I can do some further work to try and get both working, maybe detect 8-bit transfers (still thinking about this) and push to this PR?

That'd be great, thanks! I'm unable to test any 8-bit modes, so that will help a lot. Please push any improvements you can make and we'll commit before I make any other changes (such as working when XMS enabled, etc).

you use mount /dev/hda mnt for cf-card, but I though it should be: mount /dev/cf0 mnt? How does this work?

That's the new remapping code, presumably.

Yes, @toncho11, that's the new "overlay" capability I discussed here. While the actual ATA CF device is /dev/cf0, for Solo/86 /dev/hda is overlaid on top of /dev/cf0, which allows for Solo/86 users for which CF is their internal mass media to use /dev/hda much like modern laptop users use /dev/hda for internal flash drives. This feature can be turned on later for other ELKS systems/users that may want to do the same thing. It's not on by default since many ELKS users will want to use /dev/hda for BIOS access to an actual hard drive.

[toncho11]

@ghaerr Can we make the sanity checks non blocking for now?

[ghaerr]

Can we make the sanity checks non blocking for now?

Sure - I just pushed a commit with total=0 commented out for your testing.

[toncho11]

I used mount /dev/cf0 /mnt and it failed with mounf failed: no such device
If I use mount /dev/hda1 /mnt it does work. It is able to mount a fat partition.
I have partitions on this card. The first one is fat.
My I/O address is 0x300
My ROM address is: 0xCE000 (which should not matter because we do not want to use the ROM)

[ghaerr]

I used mount /dev/cf0 /mnt and it failed with mounf failed: no such device

Can you take a screenshot of the boot screen? We need to see the various cf: messages at startup to determine what is happening. It sounds like both ATA-CF drives failed the current init code.

[ghaerr]

If I use mount /dev/hda1 /mnt it does work. It is able to mount a fat partition.

On your system (and all systems other than Solo/86 at the moment), /dev/hda1 is the BIOS-addressed first partition of the first hard drive. Is that also the same as the XT-IDE-addressable CF card on your system?

I have partitions on this card. The first one is fat.

Partitions aren't supported with the ATA CF driver yet. If you try to mount a partition, it won't say "no such device", but the mount will fail with another message. So that's not the problem yet.

My I/O address is 0x300

The ATA CF driver is defaulting to address 0x1F0. This would need to be changed in include/arch/ata.h. 0x300 is normally the I/O address for an NE2K network card.

[toncho11]

So it should be like that:

//#define BASE_ADDR    0
#define BASE_ADDR    0x300

#define ATA_PORT_DATA      (BASE_ADDR + 0)  // Data Register (R/W)
#define ATA_PORT_ERR       (BASE_ADDR + 1)  // Error Register (R)
#define ATA_PORT_FEAT      (BASE_ADDR + 1)  // Features Register (W)
#define ATA_PORT_CNT       (BASE_ADDR + 2)  // Sector Count Register
#define ATA_PORT_LBA_LO    (BASE_ADDR + 3)  // LBA Low Byte
#define ATA_PORT_LBA_MD    (BASE_ADDR + 4)  // LBA Mid Byte
#define ATA_PORT_LBA_HI    (BASE_ADDR + 5)  // LBA High Byte
#define ATA_PORT_DRVH      (BASE_ADDR + 6)  // Drive/Head Register
#define ATA_PORT_CMD       (BASE_ADDR + 7)  // Command Register (W)
#define ATA_PORT_STATUS    (BASE_ADDR + 7)  // Status Register (R)
#define ATA_PORT_CTRL      0x3F6                  // Optional, not used on XT-IDE

[toncho11]

@ghaerr Can you please add the above code with base address = 0x300. This will allow me to test.
And partitions are a problem?

[ghaerr]

#define BASE_ADDR 0x300

Yes, if you need 0x300 instead of 0x1F0 for your hardware. Is that standard for XT-IDE?

@fhendrikx or I can make the change and add ATA_BASE_PORT to ata.h after getting the current PR working on Solo/86 hardware in 8-bit mode.

[toncho11]

XT-IDE has switches on the card where you can configure 8 port addresses.
0x300 is standard I think.
I need to remove my network card for this test. Good point!

[ghaerr]

And partitions are a problem?

Partitions definitely won't work.

0x300 is standard I think.

Let's check more into this before changing the base address, since the standard ATA driver address is 0x1F0. We need to understand why yours is set to 0x300, which of course is also in the way of your NIC. We will update ata.h as described so that it is easy to change the base address however.

[toncho11]

It is clear why. The creators of XT-IDE wanted to use another port address that is not in conflict with the standard IDE 0x1F0 for compatibility reasons. Hopefully this is the only required change.

Register-Level Mapping (Base + N Style)

Define Name	Standard IDE (Base = 0x1F0)	XT-IDE (Base = 0x300)	Notes
ATA_PORT_DATA	0x1F0 = Base + 0	0x300 = Base + 0	Data Register (16-bit standard, 8-bit XT)
ATA_PORT_ERR	0x1F1 = Base + 1	0x301 = Base + 1	Error Register (read-only)
ATA_PORT_FEAT	0x1F1 = Base + 1	0x301 = Base + 1	Features Register (write-only)
ATA_PORT_CNT	0x1F2 = Base + 2	0x302 = Base + 2	Sector Count Register
ATA_PORT_LBA_LO	0x1F3 = Base + 3	0x303 = Base + 3	LBA Low Byte
ATA_PORT_LBA_MD	0x1F4 = Base + 4	0x304 = Base + 4	LBA Mid Byte
ATA_PORT_LBA_HI	0x1F5 = Base + 5	0x305 = Base + 5	LBA High Byte
ATA_PORT_DRVH	0x1F6 = Base + 6	0x306 = Base + 6	Drive/Head Register (also LBA bits 24–27)
ATA_PORT_CMD	0x1F7 = Base + 7	0x307 = Base + 7	Command Register (write-only)
ATA_PORT_STATUS	0x1F7 = Base + 7	0x307 = Base + 7	Status Register (read-only)
ATA_PORT_CTRL	0x3F6 (not Base-relative)	Not used	Control / Alternate Status (optional)

[fhendrikx]

#define BASE_ADDR 0x300

Yes, if you need 0x300 instead of 0x1F0 for your hardware. Is that standard for XT-IDE?

@fhendrikx or I can make the change and add ATA_BASE_PORT to ata.h after getting the current PR working on Solo/86 hardware in 8-bit mode.

I've got the hood open... so I'll put that in shortly.

[fhendrikx]

@ghaerr That change has been made, but github won't let me push to your repo directly (since that PR only exists on this copy of the elks repo).

[ghaerr]

Shall I commit this right now, and then then have you make changes from there with a new PR of your own?

[fhendrikx]

Sure, that works too...

[ghaerr]

github won't let me push to your repo directly (since that PR only exists on this copy of the elks repo).

There's only a single GitHub master repo - how is it that is in fact the problem? I'm not very git-knowledgable, are there other ways to enable pushing to an open Github PR?

[fhendrikx]

On your project, Settings > Collaborators (I believe).

[ghaerr]

In any case @fhendrikx, thanks for your testing. Please feel free to open another PR and add whatever fixes/enhancements you'd like for an improved ATA CF driver that hopefully will also work on @toncho11's Amstradt. I'll postpone any other additions until after that. I'd like to see the ATA identify code essentially duplicated in ata_cf_open, so that CF cards can be hot-swapped when not mounted. You're welcome to add that too, if you like, otherwise I'll take a pass at it afterwards.

[fhendrikx]

In any case @fhendrikx, thanks for your testing. Please feel free to open another PR and add whatever fixes/enhancements you'd like for an improved ATA CF driver that hopefully will also work on @toncho11's Amstradt. I'll postpone any other additions until after that. I'd like to see the ATA identify code essentially duplicated in ata_cf_open, so that CF cards can be hot-swapped when not mounted. You're welcome to add that too, if you like, otherwise I'll take a pass at it afterwards.

Thank you... will have a crack at all of that.

[toncho11]

I made the change in: #2349

[fhendrikx]

Right, a PR with the requested changes is ready.

• Move ATA detection to ata_cf_open function
• Update ATA sanity checks
• Fix outw() usage bug in 8bit write

Probably worth noting that my 8-bit tests on Solo/86 were successful... creating new FAT32 F/S, copying files, umount/remount, executing stuff, etc.

I did however spot two possible bugs, where mount and df seem to think in 1K blocks. For example, mount claims we have 7806M of space, when in reality we only have half of that:

Ditto df:

[ghaerr]

I did however spot two possible bugs, where mount and df seem to think in 1K blocks. For example, mount claims we have 7806M of space, when in reality we only have half of that:

Hmmm, mkfat's argument is supposed to be in 1K blocks, so it would seem that the FAT mount and df displays are correct, but possibly the cf0: size= value is incorrect. We don't really need to report size in sectors in the cf0: display. And since the number of sectors would be twice that of the number of blocks, the real culprit is "total >> 1" should actually be "total << 1" in the ATA CF driver. I would suggest remove sectors= completely and just showing blocks.

BTW, the boot screen kernel messages generally don't use '=' signs when showing numbers, but just a space, trying to make the display more easily readable. Perhaps just change the display to something like:

cf0: 7806960 blocks, CHS 7745,16,63, version 30

[fhendrikx]

Hmmm, mkfat's argument is supposed to be in 1K blocks, so it would seem that the FAT mount and df displays are correct, but possibly the cf0: size= value is incorrect. We don't really need to report size in sectors in the cf0: display. And since the number of sectors would be twice that of the number of blocks, the real culprit is "total >> 1" should actually be "total << 1" in the ATA CF driver. I would suggest remove sectors= completely and just showing blocks.

Ok, will have a look tomorrow. Will fix it in the PR.

BTW, the boot screen kernel messages generally don't use '=' signs when showing numbers, but just a space, trying to make the display more easily readable. Perhaps just change the display to something like:

Ok, no worries. Will fix that too

[ghaerr]

Perhaps just change the display to something like:
cf0: 7806960 blocks, CHS 7745,16,63, version 30

Oops - I now remember why I don't use the term "blocks" anymore - it's potentially ambiguous between meaning 512 vs 1024 bytes. I recommend changing the cf0: display to something like:

cf0: 7806960K CHS 7745,16,63 (version 30)

This is pretty much what the display was originally.

the real culprit is "total >> 1" should actually be "total << 1" in the ATA CF driver.

I think the calculations were also correct in the version prior to your PR also. Of course, the ATA driver keeps track of sectors internally since block requests are sector-numbered, but we report sizes in 1K increments, not sectors or "blocks".