Re: [hatari-devel] Patch: IDE support for sector sizes > 512 bytes

[Uwe Seimet <Uwe.Seimet@xxxxxxxxx>]

Hi,

There was still a problem with calculating the capacity of IDE drives
with sectors > 512 bytes. The updated attached patch completely replaces
the previous patch and fixes this issue.

Best regards

Uwe

> Hi,
> 
> The attached patch adds support for IDE/SATA hard disks with physical
> sector sizes of up to 4096 bytes.
> Just like with ACSI/SCSI there is a new configuration property nBlockSize.
> 
> The patch was successfully tested with emulated drives with 512 and 4096
> physical bytes per sector. No backwards compatibility issues were found.
> 
> Notes:
> 
> - With 4096 bytes per physical sector a real Atari blocks the IDE port
>   when trying to boot from such a drive. This is because the TOS boot
>   code does not support sectors > 512 bytes.
> - A hard disk driver with support for IDE/SATA drives with 4096 bytes per
>   sector (4KN) is required to make use of this feature with Hatari.
> 
> Best regards
> 
> Uwe

> diff -r 4f4871bb49f0 src/configuration.c
> --- a/src/configuration.c	Tue Oct 16 06:49:32 2018 +0200
> +++ b/src/configuration.c	Tue Oct 16 19:41:40 2018 +0200
> @@ -521,9 +521,11 @@
>  	{ "bUseDevice0", Bool_Tag, &ConfigureParams.Ide[0].bUseDevice },
>  	{ "nByteSwap0", Int_Tag, &ConfigureParams.Ide[0].nByteSwap },
>  	{ "sDeviceFile0", String_Tag, ConfigureParams.Ide[0].sDeviceFile },
> +	{ "nBlockSize0", Int_Tag, &ConfigureParams.Ide[0].nBlockSize },
>  	{ "bUseDevice1", Bool_Tag, &ConfigureParams.Ide[1].bUseDevice },
>  	{ "nByteSwap1", Int_Tag, &ConfigureParams.Ide[1].nByteSwap },
>  	{ "sDeviceFile1", String_Tag, ConfigureParams.Ide[1].sDeviceFile },
> +	{ "nBlockSize1", Int_Tag, &ConfigureParams.Ide[1].nBlockSize },
>  	{ NULL , Error_Tag, NULL }
>  };
>  
> @@ -702,6 +704,7 @@
>  		ConfigureParams.Ide[i].bUseDevice = false;
>  		ConfigureParams.Ide[i].nByteSwap = BYTESWAP_AUTO;
>  		strcpy(ConfigureParams.Ide[i].sDeviceFile, psWorkingDir);
> +		ConfigureParams.Ide[i].nBlockSize = 512;
>  	}
>  
>  	/* Set defaults for Joysticks */
> diff -r 4f4871bb49f0 src/ide.c
> --- a/src/ide.c	Tue Oct 16 06:49:32 2018 +0200
> +++ b/src/ide.c	Tue Oct 16 19:41:40 2018 +0200
> @@ -334,6 +334,7 @@
>      off_t file_size;
>      int media_changed;
>      int byteswap;
> +    int sector_size;
>  
>      /* I/O stats (display with "info blockstats"). */
>      uint64_t rd_bytes;
> @@ -366,7 +367,6 @@
>  #define MIN(a, b) (((a) < (b)) ? (a) : (b))
>  
>  #define SECTOR_BITS 9
> -#define SECTOR_SIZE (1 << SECTOR_BITS)
>  
>  
>  /**
> @@ -455,9 +455,9 @@
>  	if (!bs->fhndl)
>  		return -ENOMEDIUM;
>  
> -	len = nb_sectors * SECTOR_SIZE;
> +	len = nb_sectors * bs->sector_size;
>  
> -	if (fseeko(bs->fhndl, sector_num * SECTOR_SIZE, SEEK_SET) != 0)
> +	if (fseeko(bs->fhndl, sector_num * bs->sector_size, SEEK_SET) != 0)
>  	{
>  		perror("bdrv_read");
>  		return -errno;
> @@ -503,9 +503,9 @@
>  	if (bs->read_only)
>  		return -EACCES;
>  
> -	len = nb_sectors * SECTOR_SIZE;
> +	len = nb_sectors * bs->sector_size;
>  
> -	if (fseeko(bs->fhndl, sector_num * SECTOR_SIZE, SEEK_SET) != 0)
> +	if (fseeko(bs->fhndl, sector_num * bs->sector_size, SEEK_SET) != 0)
>  	{
>  		perror("bdrv_write");
>  		return -errno;
> @@ -549,7 +549,7 @@
>  	bs->file_size = HDC_CheckAndGetSize(filename, blockSize);
>  	if (bs->file_size <= 0)
>  		return -1;
> -	if (bs->file_size < 2 * 16 * 63 * SECTOR_SIZE)
> +	if (bs->file_size < 2 * 16 * 63 * bs->sector_size)
>  	{
>  		Log_AlertDlg(LOG_ERROR, "IDE disk image size (%"PRId64" bytes) is "
>  		                        "too small for an IDE disk image "
> @@ -768,6 +768,9 @@
>  /* set to 1 set disable mult support */
>  #define MAX_MULT_SECTORS 16
>  
> +/* maximum physical IDE hard disk drive sector size */
> +#define MAX_SECTOR_SIZE 4096
> +
>  /* ATAPI defines */
>  
>  #define ATAPI_PACKET_SIZE 12
> @@ -1031,8 +1034,8 @@
>  	/* ratio logical/physical: 0, logicalSectorSizeSupported */
>  	put_le16(p + 106, 1 << 12);
>  	/* words per logical sector */
> -	put_le16(p + 117, 512 >> 1);
> -	put_le16(p + 118, 512 >> 17);
> +	put_le16(p + 117, s->bs->sector_size >> 1);
> +	put_le16(p + 118, s->bs->sector_size >> 17);
>  
>  	memcpy(s->identify_data, p, sizeof(s->identify_data));
>  	s->identify_set = 1;
> @@ -1234,7 +1237,7 @@
>  			ide_set_irq(s);
>  			return;
>  		}
> -		ide_transfer_start(s, s->io_buffer, 512 * n, ide_sector_read);
> +		ide_transfer_start(s, s->io_buffer, s->bs->sector_size * n, ide_sector_read);
>  		ide_set_irq(s);
>  		ide_set_sector(s, sector_num + n);
>  		s->nsector -= n;
> @@ -1272,7 +1275,7 @@
>  		n1 = s->nsector;
>  		if (n1 > s->req_nb_sectors)
>  			n1 = s->req_nb_sectors;
> -		ide_transfer_start(s, s->io_buffer, 512 * n1, ide_sector_write);
> +		ide_transfer_start(s, s->io_buffer, s->bs->sector_size * n1, ide_sector_write);
>  	}
>  	ide_set_sector(s, sector_num + n);
>  
> @@ -2086,7 +2089,7 @@
>  			n = s->nsector;
>  			if (n > s->req_nb_sectors)
>  				n = s->req_nb_sectors;
> -			ide_transfer_start(s, s->io_buffer, 512 * n, ide_sector_write);
> +			ide_transfer_start(s, s->io_buffer, s->bs->sector_size * n, ide_sector_write);
>  			s->media_changed = 1;
>  			break;
>  		case WIN_READ_EXT:
> @@ -2111,7 +2114,7 @@
>  			s->error = 0;
>  			s->status = SEEK_STAT | READY_STAT;
>  			s->req_nb_sectors = 1;
> -			ide_transfer_start(s, s->io_buffer, 512, ide_sector_write);
> +			ide_transfer_start(s, s->io_buffer, s->bs->sector_size, ide_sector_write);
>  			s->media_changed = 1;
>  			break;
>  		case WIN_MULTREAD_EXT:
> @@ -2138,7 +2141,7 @@
>  			n = s->nsector;
>  			if (n > s->req_nb_sectors)
>  				n = s->req_nb_sectors;
> -			ide_transfer_start(s, s->io_buffer, 512 * n, ide_sector_write);
> +			ide_transfer_start(s, s->io_buffer, s->bs->sector_size * n, ide_sector_write);
>  			s->media_changed = 1;
>  			break;
>  		case WIN_READDMA_EXT:
> @@ -2537,7 +2540,7 @@
>  	struct partition *p;
>  	uint32_t nr_sects;
>  
> -	buf = malloc(SECTOR_SIZE);
> +	buf = malloc(MAX_SECTOR_SIZE);
>  	if (buf == NULL)
>  		return -1;
>  	ret = bdrv_read(s->bs, 0, buf, 1);
> @@ -2591,7 +2594,7 @@
>  	for (i = 0; i < 2; i++)
>  	{
>  		s = ide_state + i;
> -		s->io_buffer = malloc(MAX_MULT_SECTORS * 512 + 4);
> +		s->io_buffer = malloc(MAX_MULT_SECTORS * MAX_SECTOR_SIZE + 4);
>  		assert(s->io_buffer);
>  		if (i == 0)
>  			s->bs = hd0;
> @@ -2704,7 +2707,7 @@
>  		if (ConfigureParams.Ide[i].bUseDevice)
>  		{
>  			int is_byteswap;
> -			bdrv_open(hd_table[i], ConfigureParams.Ide[i].sDeviceFile, 512, 0);
> +			bdrv_open(hd_table[i], ConfigureParams.Ide[i].sDeviceFile, ConfigureParams.Ide[i].nBlockSize, 0);
>  			nIDEPartitions += HDC_PartitionCount(hd_table[i]->fhndl, TRACE_IDE, &is_byteswap);
>  			/* Our IDE implementation is little endian by default,
>  			 * so we need to byteswap if the image is not swapped! */
> @@ -2712,6 +2715,8 @@
>  				hd_table[i]->byteswap = !is_byteswap;
>  			else
>  				hd_table[i]->byteswap = !ConfigureParams.Ide[i].nByteSwap;
> +
> +			hd_table[i]->sector_size = ConfigureParams.Ide[i].nBlockSize;
>  		}
>  	}
>  
> diff -r 4f4871bb49f0 src/includes/configuration.h
> --- a/src/includes/configuration.h	Tue Oct 16 06:49:32 2018 +0200
> +++ b/src/includes/configuration.h	Tue Oct 16 19:41:40 2018 +0200
> @@ -243,6 +243,7 @@
>    bool bUseDevice;
>    BYTESWAPPING nByteSwap;
>    char sDeviceFile[FILENAME_MAX];
> +  int nBlockSize;
>  } CNF_IDEDEV;
>  
>  /* Falcon register $FFFF8006 bits 6 & 7 (mirrored in $FFFF82C0 bits 0 & 1):

diff -r 4f4871bb49f0 src/configuration.c
--- a/src/configuration.c	Tue Oct 16 06:49:32 2018 +0200
+++ b/src/configuration.c	Wed Oct 17 19:33:24 2018 +0200
@@ -521,9 +521,11 @@
 	{ "bUseDevice0", Bool_Tag, &ConfigureParams.Ide[0].bUseDevice },
 	{ "nByteSwap0", Int_Tag, &ConfigureParams.Ide[0].nByteSwap },
 	{ "sDeviceFile0", String_Tag, ConfigureParams.Ide[0].sDeviceFile },
+	{ "nBlockSize0", Int_Tag, &ConfigureParams.Ide[0].nBlockSize },
 	{ "bUseDevice1", Bool_Tag, &ConfigureParams.Ide[1].bUseDevice },
 	{ "nByteSwap1", Int_Tag, &ConfigureParams.Ide[1].nByteSwap },
 	{ "sDeviceFile1", String_Tag, ConfigureParams.Ide[1].sDeviceFile },
+	{ "nBlockSize1", Int_Tag, &ConfigureParams.Ide[1].nBlockSize },
 	{ NULL , Error_Tag, NULL }
 };
 
@@ -702,6 +704,7 @@
 		ConfigureParams.Ide[i].bUseDevice = false;
 		ConfigureParams.Ide[i].nByteSwap = BYTESWAP_AUTO;
 		strcpy(ConfigureParams.Ide[i].sDeviceFile, psWorkingDir);
+		ConfigureParams.Ide[i].nBlockSize = 512;
 	}
 
 	/* Set defaults for Joysticks */
diff -r 4f4871bb49f0 src/ide.c
--- a/src/ide.c	Tue Oct 16 06:49:32 2018 +0200
+++ b/src/ide.c	Wed Oct 17 19:33:24 2018 +0200
@@ -334,6 +334,7 @@
     off_t file_size;
     int media_changed;
     int byteswap;
+    int sector_size;
 
     /* I/O stats (display with "info blockstats"). */
     uint64_t rd_bytes;
@@ -365,9 +366,6 @@
 
 #define MIN(a, b) (((a) < (b)) ? (a) : (b))
 
-#define SECTOR_BITS 9
-#define SECTOR_SIZE (1 << SECTOR_BITS)
-
 
 /**
  * return 0 as number of sectors if no device present or error
@@ -379,7 +377,7 @@
 	if (length < 0)
 		length = 0;
 	else
-		length = length >> SECTOR_BITS;
+		length = length / bs->sector_size;
 	*nb_sectors_ptr = length;
 }
 
@@ -455,9 +453,9 @@
 	if (!bs->fhndl)
 		return -ENOMEDIUM;
 
-	len = nb_sectors * SECTOR_SIZE;
+	len = nb_sectors * bs->sector_size;
 
-	if (fseeko(bs->fhndl, sector_num * SECTOR_SIZE, SEEK_SET) != 0)
+	if (fseeko(bs->fhndl, sector_num * bs->sector_size, SEEK_SET) != 0)
 	{
 		perror("bdrv_read");
 		return -errno;
@@ -503,9 +501,9 @@
 	if (bs->read_only)
 		return -EACCES;
 
-	len = nb_sectors * SECTOR_SIZE;
+	len = nb_sectors * bs->sector_size;
 
-	if (fseeko(bs->fhndl, sector_num * SECTOR_SIZE, SEEK_SET) != 0)
+	if (fseeko(bs->fhndl, sector_num * bs->sector_size, SEEK_SET) != 0)
 	{
 		perror("bdrv_write");
 		return -errno;
@@ -549,7 +547,7 @@
 	bs->file_size = HDC_CheckAndGetSize(filename, blockSize);
 	if (bs->file_size <= 0)
 		return -1;
-	if (bs->file_size < 2 * 16 * 63 * SECTOR_SIZE)
+	if (bs->file_size < 2 * 16 * 63 * bs->sector_size)
 	{
 		Log_AlertDlg(LOG_ERROR, "IDE disk image size (%"PRId64" bytes) is "
 		                        "too small for an IDE disk image "
@@ -768,6 +766,9 @@
 /* set to 1 set disable mult support */
 #define MAX_MULT_SECTORS 16
 
+/* maximum physical IDE hard disk drive sector size */
+#define MAX_SECTOR_SIZE 4096
+
 /* ATAPI defines */
 
 #define ATAPI_PACKET_SIZE 12
@@ -1031,8 +1032,8 @@
 	/* ratio logical/physical: 0, logicalSectorSizeSupported */
 	put_le16(p + 106, 1 << 12);
 	/* words per logical sector */
-	put_le16(p + 117, 512 >> 1);
-	put_le16(p + 118, 512 >> 17);
+	put_le16(p + 117, s->bs->sector_size >> 1);
+	put_le16(p + 118, s->bs->sector_size >> 17);
 
 	memcpy(s->identify_data, p, sizeof(s->identify_data));
 	s->identify_set = 1;
@@ -1234,7 +1235,7 @@
 			ide_set_irq(s);
 			return;
 		}
-		ide_transfer_start(s, s->io_buffer, 512 * n, ide_sector_read);
+		ide_transfer_start(s, s->io_buffer, s->bs->sector_size * n, ide_sector_read);
 		ide_set_irq(s);
 		ide_set_sector(s, sector_num + n);
 		s->nsector -= n;
@@ -1272,7 +1273,7 @@
 		n1 = s->nsector;
 		if (n1 > s->req_nb_sectors)
 			n1 = s->req_nb_sectors;
-		ide_transfer_start(s, s->io_buffer, 512 * n1, ide_sector_write);
+		ide_transfer_start(s, s->io_buffer, s->bs->sector_size * n1, ide_sector_write);
 	}
 	ide_set_sector(s, sector_num + n);
 
@@ -2086,7 +2087,7 @@
 			n = s->nsector;
 			if (n > s->req_nb_sectors)
 				n = s->req_nb_sectors;
-			ide_transfer_start(s, s->io_buffer, 512 * n, ide_sector_write);
+			ide_transfer_start(s, s->io_buffer, s->bs->sector_size * n, ide_sector_write);
 			s->media_changed = 1;
 			break;
 		case WIN_READ_EXT:
@@ -2111,7 +2112,7 @@
 			s->error = 0;
 			s->status = SEEK_STAT | READY_STAT;
 			s->req_nb_sectors = 1;
-			ide_transfer_start(s, s->io_buffer, 512, ide_sector_write);
+			ide_transfer_start(s, s->io_buffer, s->bs->sector_size, ide_sector_write);
 			s->media_changed = 1;
 			break;
 		case WIN_MULTREAD_EXT:
@@ -2138,7 +2139,7 @@
 			n = s->nsector;
 			if (n > s->req_nb_sectors)
 				n = s->req_nb_sectors;
-			ide_transfer_start(s, s->io_buffer, 512 * n, ide_sector_write);
+			ide_transfer_start(s, s->io_buffer, s->bs->sector_size * n, ide_sector_write);
 			s->media_changed = 1;
 			break;
 		case WIN_READDMA_EXT:
@@ -2537,7 +2538,7 @@
 	struct partition *p;
 	uint32_t nr_sects;
 
-	buf = malloc(SECTOR_SIZE);
+	buf = malloc(MAX_SECTOR_SIZE);
 	if (buf == NULL)
 		return -1;
 	ret = bdrv_read(s->bs, 0, buf, 1);
@@ -2591,7 +2592,7 @@
 	for (i = 0; i < 2; i++)
 	{
 		s = ide_state + i;
-		s->io_buffer = malloc(MAX_MULT_SECTORS * 512 + 4);
+		s->io_buffer = malloc(MAX_MULT_SECTORS * MAX_SECTOR_SIZE + 4);
 		assert(s->io_buffer);
 		if (i == 0)
 			s->bs = hd0;
@@ -2704,7 +2705,7 @@
 		if (ConfigureParams.Ide[i].bUseDevice)
 		{
 			int is_byteswap;
-			bdrv_open(hd_table[i], ConfigureParams.Ide[i].sDeviceFile, 512, 0);
+			bdrv_open(hd_table[i], ConfigureParams.Ide[i].sDeviceFile, ConfigureParams.Ide[i].nBlockSize, 0);
 			nIDEPartitions += HDC_PartitionCount(hd_table[i]->fhndl, TRACE_IDE, &is_byteswap);
 			/* Our IDE implementation is little endian by default,
 			 * so we need to byteswap if the image is not swapped! */
@@ -2712,6 +2713,8 @@
 				hd_table[i]->byteswap = !is_byteswap;
 			else
 				hd_table[i]->byteswap = !ConfigureParams.Ide[i].nByteSwap;
+
+			hd_table[i]->sector_size = ConfigureParams.Ide[i].nBlockSize;
 		}
 	}
 
diff -r 4f4871bb49f0 src/includes/configuration.h
--- a/src/includes/configuration.h	Tue Oct 16 06:49:32 2018 +0200
+++ b/src/includes/configuration.h	Wed Oct 17 19:33:24 2018 +0200
@@ -243,6 +243,7 @@
   bool bUseDevice;
   BYTESWAPPING nByteSwap;
   char sDeviceFile[FILENAME_MAX];
+  int nBlockSize;
 } CNF_IDEDEV;
 
 /* Falcon register $FFFF8006 bits 6 & 7 (mirrored in $FFFF82C0 bits 0 & 1):