Re: [hatari-devel] Re: IDE byte swapping options (was: Problems with GEMDOS drive support)

[ Thread Index | Date Index | More lists.tuxfamily.org/hatari-devel Archives ]


Hi,

On 9/21/18 7:17 PM, Roger Burrows wrote:
On 21 Sep 2018 at 10:33, Vincent Rivière wrote:
Hatari should have an option to disable its automatic byteswapping for
emulated IDE drives, then everything will be fine.

There still has to be a fix in IDE_Init()/HDC_Partition_Count(), because Hatari
now reads the disk in two different ways: when passing the data to the user, it
byteswaps, but when analyzing the boot sector for its own purposes, it doesn't.
It needs to do the same thing for both.  An extra option not to byteswap might
be nice, but it would have to apply to both situations (and it runs the risk of
being confusing to users).

I looked at Aranym code quickly, and it would seem to do 16-bit
swaps for the whole buffer.

So, is that what "byteswap" means?


If yes, attached patch "should" implement such option for IDE
partition counting.  If it works fine, same options can be used
to toggle swapping for rest of IDE handling too.


To whichever Hatari developer is going to fix this:

When I looked at the code, I did it with Atari-style images in mind.  But I
think there might also be the same issue with DOS-style images.  Of course,
this would not apply to plain Atari TOS, which doesn't understand such things,
but only to EmuTOS (or Atari TOS + BigDOS?).  So this could in fact be even
worse, because to assign the GEMDOS letter properly, Hatari needs to know how a
DOS-style image will be interpreted by the OS/hard disk driver.
>
Thinking about this from a different direction: when the 'assign GEMDOS drive
after image drives' option is selected, would it be possible for Hatari to only
hook in the GEMDOS drive after the hard disk driver has been run?  That way it
doesn't need to worry about analyzing the boot sector, it just looks at
_drivebits or whatever it's called.  I realize that this might not be easy (or
perhaps even possible).

No, because GEMDOS HD needs to be assigned at start, and HD driver
may be run at any moment, e.g. by user manually from a floppy.


	- Eero
Add partition swap config & cli options for IDE

NOTE: currently affects only partition count parsing for GEMDOS HD
partition skipping!

To enable byte swapping for partition counting, use:
	--byteswap master --byteswap slave

diff -r 74289ac5e3d6 src/configuration.c
--- a/src/configuration.c	Thu Sep 20 01:07:13 2018 +0300
+++ b/src/configuration.c	Sun Sep 23 20:09:07 2018 +0300
@@ -445,6 +445,8 @@
 	{ "szHardDiskImage", String_Tag, ConfigureParams.Acsi[0].sDeviceFile },
 	{ "bUseIdeMasterHardDiskImage", Bool_Tag, &ConfigureParams.HardDisk.bUseIdeMasterHardDiskImage },
 	{ "bUseIdeSlaveHardDiskImage", Bool_Tag, &ConfigureParams.HardDisk.bUseIdeSlaveHardDiskImage },
+	{ "bIdeMasterByteSwap", Bool_Tag, &ConfigureParams.HardDisk.bIdeMasterByteSwap },
+	{ "bUIdeSlaveByteSwap", Bool_Tag, &ConfigureParams.HardDisk.bIdeSlaveByteSwap },
 	{ "szIdeMasterHardDiskImage", String_Tag, ConfigureParams.HardDisk.szIdeMasterHardDiskImage },
 	{ "szIdeSlaveHardDiskImage", String_Tag, ConfigureParams.HardDisk.szIdeSlaveHardDiskImage },
 	{ NULL , Error_Tag, NULL }
@@ -646,8 +648,10 @@
 		strcpy(ConfigureParams.HardDisk.szHardDiskDirectories[i], psWorkingDir);
 		File_CleanFileName(ConfigureParams.HardDisk.szHardDiskDirectories[i]);
 	}
+	ConfigureParams.HardDisk.bIdeMasterByteSwap = false;
 	ConfigureParams.HardDisk.bUseIdeMasterHardDiskImage = false;
 	strcpy(ConfigureParams.HardDisk.szIdeMasterHardDiskImage, psWorkingDir);
+	ConfigureParams.HardDisk.bIdeSlaveByteSwap = false;
 	ConfigureParams.HardDisk.bUseIdeSlaveHardDiskImage = false;
 	strcpy(ConfigureParams.HardDisk.szIdeSlaveHardDiskImage, psWorkingDir);
 
diff -r 74289ac5e3d6 src/hdc.c
--- a/src/hdc.c	Thu Sep 20 01:07:13 2018 +0300
+++ b/src/hdc.c	Sun Sep 23 20:09:07 2018 +0300
@@ -570,7 +570,7 @@
  * print also partition table.
  *
  * Supports both DOS and Atari master boot record partition tables
- * (with 4 entries).
+ * (with 4 entries), and option for 16-bit drive byte swap.
  *
  * TODO:
  * - Support also Atari ICD (12 entries, at offset 0x156) and
@@ -578,13 +578,12 @@
  *   http://lxr.free-electrons.com/source/block/partitions/atari.c
  *   Normal and extended partition tables are described in:
  *   http://dev-docs.atariforge.org/files/AHDI_3_RN_4-18-1990.pdf
- * - Support partition tables with other endianness
  */
-int HDC_PartitionCount(FILE *fp, const Uint64 tracelevel)
+int HDC_PartitionCount(FILE *fp, Uint64 tracelevel, bool byteswap)
 {
 	unsigned char *pinfo, bootsector[512];
 	Uint32 start, sectors, total = 0;
-	int i, parts = 0;
+	unsigned int i, parts = 0;
 	off_t offset;
 
 	if (!fp)
@@ -598,6 +597,16 @@
 		return 0;
 	}
 
+	if (byteswap)
+	{
+		Uint16 *boots = (Uint16*)bootsector;
+		for (i = 0; i < sizeof(bootsector)/sizeof(*boots); i++)
+		{
+			*boots = SDL_Swap16(*boots);
+			boots++;
+		}
+	}
+
 	if (bootsector[0x1FE] == 0x55 && bootsector[0x1FF] == 0xAA)
 	{
 		int ptype, boot;
@@ -628,7 +637,7 @@
 		 * there's the boot code.
 		 */
 		char c, pid[4];
-		int j, flags;
+		unsigned int j, flags;
 		bool extended;
 
 		LOG_TRACE(tracelevel, "ATARI MBR:\n");
@@ -769,7 +778,7 @@
 		if (HDC_InitDevice(&AcsiBus.devs[i], ConfigureParams.Acsi[i].sDeviceFile) == 0)
 		{
 			bAcsiEmuOn = true;
-			nAcsiPartitions += HDC_PartitionCount(AcsiBus.devs[i].image_file, TRACE_SCSI_CMD);
+			nAcsiPartitions += HDC_PartitionCount(AcsiBus.devs[i].image_file, TRACE_SCSI_CMD, false);
 		}
 	}
 
diff -r 74289ac5e3d6 src/ide.c
--- a/src/ide.c	Thu Sep 20 01:07:13 2018 +0300
+++ b/src/ide.c	Sun Sep 23 20:09:07 2018 +0300
@@ -2665,12 +2665,12 @@
 	memset(hd_table[1], 0, sizeof(BlockDriverState));
 
 	bdrv_open(hd_table[0], ConfigureParams.HardDisk.szIdeMasterHardDiskImage, 0);
-	nIDEPartitions += HDC_PartitionCount(hd_table[0]->fhndl, TRACE_IDE);
+	nIDEPartitions += HDC_PartitionCount(hd_table[0]->fhndl, TRACE_IDE, ConfigureParams.HardDisk.bIdeMasterByteSwap);
 
 	if (ConfigureParams.HardDisk.bUseIdeSlaveHardDiskImage)
 	{
 		bdrv_open(hd_table[1], ConfigureParams.HardDisk.szIdeSlaveHardDiskImage, 0);
-		nIDEPartitions += HDC_PartitionCount(hd_table[1]->fhndl, TRACE_IDE);
+		nIDEPartitions += HDC_PartitionCount(hd_table[1]->fhndl, TRACE_IDE, ConfigureParams.HardDisk.bIdeSlaveByteSwap);
 
 		ide_init2(&opaque_ide_if[0], hd_table[0], hd_table[1]);
 	}
diff -r 74289ac5e3d6 src/includes/configuration.h
--- a/src/includes/configuration.h	Thu Sep 20 01:07:13 2018 +0300
+++ b/src/includes/configuration.h	Sun Sep 23 20:09:07 2018 +0300
@@ -213,6 +213,8 @@
   bool bUseHardDiskDirectories;
   bool bUseIdeMasterHardDiskImage;
   bool bUseIdeSlaveHardDiskImage;
+  bool bIdeMasterByteSwap;
+  bool bIdeSlaveByteSwap;
   WRITEPROTECTION nWriteProtection;
   GEMDOS_CHR_CONV nGemdosCase;
   bool bFilenameConversion;
diff -r 74289ac5e3d6 src/includes/hdc.h
--- a/src/includes/hdc.h	Thu Sep 20 01:07:13 2018 +0300
+++ b/src/includes/hdc.h	Sun Sep 23 20:09:07 2018 +0300
@@ -97,7 +97,7 @@
 extern void HDC_ResetCommandStatus(void);
 extern short int HDC_ReadCommandByte(int addr);
 extern void HDC_WriteCommandByte(int addr, Uint8 byte);
-extern int HDC_PartitionCount(FILE *fp, const Uint64 tracelevel);
+extern int HDC_PartitionCount(FILE *fp, const Uint64 tracelevel, bool byteswap);
 extern off_t HDC_CheckAndGetSize(const char *filename);
 extern bool HDC_WriteCommandPacket(SCSI_CTRLR *ctr, Uint8 b);
 extern void HDC_DmaTransfer(void);
diff -r 74289ac5e3d6 src/ncr5380.c
--- a/src/ncr5380.c	Thu Sep 20 01:07:13 2018 +0300
+++ b/src/ncr5380.c	Sun Sep 23 20:09:07 2018 +0300
@@ -1020,7 +1020,7 @@
 		if (!ConfigureParams.Scsi[i].bUseDevice)
 			continue;
 		if (HDC_InitDevice(&ScsiBus.devs[i], ConfigureParams.Scsi[i].sDeviceFile) == 0)
-			partitions += HDC_PartitionCount(ScsiBus.devs[i].image_file, TRACE_SCSI_CMD);
+			partitions += HDC_PartitionCount(ScsiBus.devs[i].image_file, TRACE_SCSI_CMD, false);
 	}
 	return partitions;
 #else
diff -r 74289ac5e3d6 src/options.c
--- a/src/options.c	Thu Sep 20 01:07:13 2018 +0300
+++ b/src/options.c	Sun Sep 23 20:09:07 2018 +0300
@@ -134,6 +134,7 @@
 	OPT_SCSIHDIMAGE,
 	OPT_IDEMASTERHDIMAGE,
 	OPT_IDESLAVEHDIMAGE,
+	OPT_IMAGEBYTESWAP,
 	OPT_MEMSIZE,		/* memory options */
 #if ENABLE_WINUAE_CPU
 	OPT_TT_RAM,
@@ -368,6 +369,8 @@
 	  "<file>", "Emulate an IDE master harddrive with an image <file>" },
 	{ OPT_IDESLAVEHDIMAGE,   NULL, "--ide-slave",
 	  "<file>", "Emulate an IDE slave harddrive with an image <file>" },
+	{ OPT_IMAGEBYTESWAP,   NULL, "--byteswap",
+	  "<drive>", "Enable byte swapping for given IDE <drive> (master|slave)" },
 	
 	{ OPT_HEADER, NULL, NULL, NULL, "Memory" },
 	{ OPT_MEMSIZE,   "-s", "--memsize",
@@ -1652,6 +1655,22 @@
 			}
 			break;
 
+		case OPT_IMAGEBYTESWAP:
+			i += 1;
+			if (strcasecmp(argv[i], "master") == 0)
+			{
+				ConfigureParams.HardDisk.bIdeMasterByteSwap = true;
+			}
+			else if (strcasecmp(argv[i], "slave") == 0)
+			{
+				ConfigureParams.HardDisk.bIdeSlaveByteSwap = true;
+			}
+			else
+			{
+				return Opt_ShowError(OPT_IMAGEBYTESWAP, argv[i], "Invalid <drive>");
+			}
+			break;
+
 			/* Memory options */
 		case OPT_MEMSIZE:
 			memsize = atoi(argv[++i]);


Mail converted by MHonArc 2.6.19+ http://listengine.tuxfamily.org/