[PATCH] ide: limit drive capacity to 137GB if host doesn't support LBA48

[PATCH] ide: limit drive capacity to 137GB if host doesn't support LBA48

[Bartlomiej Zolnierkiewicz]

Hi,

I think this simple patch is sufficient.

Block layer shouldn't issue request for sectors > capacity so
there is no need to add additional checks to __do_rw_disk().

--bartlomiej

ide: limit drive capacity to 137GB if host doesn't support LBA48

Noticed by Andries.Brouwer@cwi.nl.

Also:
- kill probe_lba_addressing() wrapper
- rename hwif->addressing to hwif->no_lba48


 drivers/ide/ide-disk.c         |   26 ++++++++++++++++++--------
 drivers/ide/ide-probe.c        |    2 +-
 drivers/ide/ide.c              |    2 +-
 drivers/ide/legacy/pdc4030.c   |    2 +-
 drivers/ide/pci/alim15x3.c     |    2 +-
 drivers/ide/pci/pdc202xx_old.c |    2 +-
 drivers/ide/pci/siimage.c      |    2 +-
 drivers/ide/pci/trm290.c       |    2 +-
 include/linux/ide.h            |    2 +-
 9 files changed, 26 insertions(+), 16 deletions(-)

diff -puN drivers/ide/ide.c~ide-disk-no-lba48-fix drivers/ide/ide.c
--- linux-2.6.0-test2-bk7/drivers/ide/ide.c~ide-disk-no-lba48-fix	2003-08-14 02:47:38.877667440 +0200
+++ linux-2.6.0-test2-bk7-root/drivers/ide/ide.c	2003-08-14 02:47:57.257873224 +0200
@@ -904,7 +904,7 @@ void ide_unregister (unsigned int index)
 
 	hwif->mmio			= old_hwif.mmio;
 	hwif->rqsize			= old_hwif.rqsize;
-	hwif->addressing		= old_hwif.addressing;
+	hwif->no_lba48			= old_hwif.no_lba48;
 #ifndef CONFIG_BLK_DEV_IDECS
 	hwif->irq			= old_hwif.irq;
 #endif /* CONFIG_BLK_DEV_IDECS */
diff -puN drivers/ide/ide-disk.c~ide-disk-no-lba48-fix drivers/ide/ide-disk.c
--- linux-2.6.0-test2-bk7/drivers/ide/ide-disk.c~ide-disk-no-lba48-fix	2003-08-14 02:32:50.571710472 +0200
+++ linux-2.6.0-test2-bk7-root/drivers/ide/ide-disk.c	2003-08-14 03:03:02.895195424 +0200
@@ -1445,11 +1445,17 @@ static int set_using_tcq(ide_drive_t *dr
 }
 #endif
 
-static int probe_lba_addressing (ide_drive_t *drive, int arg)
+/*
+ * drive->addressing:
+ *	0: 28-bit
+ *	1: 48-bit
+ *	2: 48-bit capable doing 28-bit
+ */
+static int set_lba_addressing(ide_drive_t *drive, int arg)
 {
 	drive->addressing =  0;
 
-	if (HWIF(drive)->addressing)
+	if (HWIF(drive)->no_lba48)
 		return 0;
 
 	if (!idedisk_supports_lba48(drive->id))
@@ -1458,11 +1464,6 @@ static int probe_lba_addressing (ide_dri
 	return 0;
 }
 
-static int set_lba_addressing (ide_drive_t *drive, int arg)
-{
-	return probe_lba_addressing(drive, arg);
-}
-
 static void idedisk_add_settings(ide_drive_t *drive)
 {
 	struct hd_driveid *id = drive->id;
@@ -1578,7 +1579,7 @@ static void idedisk_setup (ide_drive_t *
 		}
 	}
 
-	(void) probe_lba_addressing(drive, 1);
+	(void)set_lba_addressing(drive, 1);
 
 	if (drive->addressing == 1) {
 		ide_hwif_t *hwif = HWIF(drive);
@@ -1617,6 +1618,15 @@ static void idedisk_setup (ide_drive_t *
 	/* calculate drive capacity, and select LBA if possible */
 	init_idedisk_capacity (drive);
 
+	/* limit drive capacity to 137GB if LBA48 cannot be used */
+	if (drive->addressing == 0 && drive->capacity64 > 1ULL << 28) {
+		printk("%s: cannot use LBA48 - full capacity "
+		       "%llu sectors (%llu MB)\n",
+		       drive->name,
+		       drive->capacity64, sectors_to_MB(drive->capacity64));
+		drive->capacity64 = 1ULL << 28;
+	}
+
 	/*
 	 * if possible, give fdisk access to more of the drive,
 	 * by correcting bios_cyls:
diff -puN drivers/ide/ide-probe.c~ide-disk-no-lba48-fix drivers/ide/ide-probe.c
--- linux-2.6.0-test2-bk7/drivers/ide/ide-probe.c~ide-disk-no-lba48-fix	2003-08-14 02:48:07.696286344 +0200
+++ linux-2.6.0-test2-bk7-root/drivers/ide/ide-probe.c	2003-08-14 02:48:15.649077336 +0200
@@ -930,7 +930,7 @@ static int ide_init_queue(ide_drive_t *d
 	blk_queue_segment_boundary(q, 0xffff);
 
 	if (!hwif->rqsize)
-		hwif->rqsize = hwif->addressing ? 256 : 65536;
+		hwif->rqsize = hwif->no_lba48 ? 256 : 65536;
 	if (hwif->rqsize < max_sectors)
 		max_sectors = hwif->rqsize;
 	blk_queue_max_sectors(q, max_sectors);
diff -puN drivers/ide/legacy/pdc4030.c~ide-disk-no-lba48-fix drivers/ide/legacy/pdc4030.c
--- linux-2.6.0-test2-bk7/drivers/ide/legacy/pdc4030.c~ide-disk-no-lba48-fix	2003-08-14 02:46:06.325737472 +0200
+++ linux-2.6.0-test2-bk7-root/drivers/ide/legacy/pdc4030.c	2003-08-14 02:46:21.465435888 +0200
@@ -227,7 +227,7 @@ int __init setup_pdc4030(ide_hwif_t *hwi
 	hwif2->mate	= hwif;
 	hwif2->channel	= 1;
 	hwif->rqsize	= hwif2->rqsize = 127;
-	hwif->addressing = hwif2->addressing = 1;
+	hwif->no_lba48 = hwif2->no_lba48 = 1;
 	hwif->selectproc = hwif2->selectproc = &promise_selectproc;
 	hwif->serialized = hwif2->serialized = 1;
 	/* DC4030 hosted drives need their own identify... */
diff -puN drivers/ide/pci/alim15x3.c~ide-disk-no-lba48-fix drivers/ide/pci/alim15x3.c
--- linux-2.6.0-test2-bk7/drivers/ide/pci/alim15x3.c~ide-disk-no-lba48-fix	2003-08-14 02:44:37.468245864 +0200
+++ linux-2.6.0-test2-bk7-root/drivers/ide/pci/alim15x3.c	2003-08-14 02:44:57.401215592 +0200
@@ -745,7 +745,7 @@ static void __init init_hwif_common_ali1
 	hwif->speedproc = &ali15x3_tune_chipset;
 
 	/* Don't use LBA48 on ALi devices before rev 0xC5 */
-	hwif->addressing = (m5229_revision <= 0xC4) ? 1 : 0;
+	hwif->no_lba48 = (m5229_revision <= 0xC4) ? 1 : 0;
 
 	if (!hwif->dma_base) {
 		hwif->drives[0].autotune = 1;
diff -puN drivers/ide/pci/pdc202xx_old.c~ide-disk-no-lba48-fix drivers/ide/pci/pdc202xx_old.c
--- linux-2.6.0-test2-bk7/drivers/ide/pci/pdc202xx_old.c~ide-disk-no-lba48-fix	2003-08-14 02:45:30.043253248 +0200
+++ linux-2.6.0-test2-bk7-root/drivers/ide/pci/pdc202xx_old.c	2003-08-14 02:45:49.325321928 +0200
@@ -750,7 +750,7 @@ static void __init init_hwif_pdc202xx (i
 	hwif->quirkproc = &pdc202xx_quirkproc;
 
 	if (hwif->pci_dev->device == PCI_DEVICE_ID_PROMISE_20265)
-		hwif->addressing = (hwif->channel) ? 0 : 1;
+		hwif->no_lba48 = (hwif->channel) ? 0 : 1;
 
 	if (hwif->pci_dev->device != PCI_DEVICE_ID_PROMISE_20246) {
 		hwif->busproc   = &pdc202xx_tristate;
diff -puN drivers/ide/pci/siimage.c~ide-disk-no-lba48-fix drivers/ide/pci/siimage.c
--- linux-2.6.0-test2-bk7/drivers/ide/pci/siimage.c~ide-disk-no-lba48-fix	2003-08-14 02:45:06.437841816 +0200
+++ linux-2.6.0-test2-bk7-root/drivers/ide/pci/siimage.c	2003-08-14 02:45:20.050772336 +0200
@@ -724,7 +724,7 @@ static void __init init_mmio_iops_siimag
 	}
 
 #ifdef SIIMAGE_BUFFERED_TASKFILE
-        hwif->addressing = 1;
+	hwif->no_lba48 = 1;
 #endif /* SIIMAGE_BUFFERED_TASKFILE */
 	hwif->irq			= hw.irq;
 	hwif->hwif_data			= pci_get_drvdata(hwif->pci_dev);
diff -puN drivers/ide/pci/trm290.c~ide-disk-no-lba48-fix drivers/ide/pci/trm290.c
--- linux-2.6.0-test2-bk7/drivers/ide/pci/trm290.c~ide-disk-no-lba48-fix	2003-08-14 02:44:11.963123232 +0200
+++ linux-2.6.0-test2-bk7-root/drivers/ide/pci/trm290.c	2003-08-14 02:44:28.650586352 +0200
@@ -309,7 +309,7 @@ void __init init_hwif_trm290 (ide_hwif_t
 	u8 reg = 0;
 	struct pci_dev *dev = hwif->pci_dev;
 
-	hwif->addressing = 1;
+	hwif->no_lba48 = 1;
 	hwif->chipset = ide_trm290;
 	cfgbase = pci_resource_start(dev, 4);
 	if ((dev->class & 5) && cfgbase) {
diff -puN include/linux/ide.h~ide-disk-no-lba48-fix include/linux/ide.h
--- linux-2.6.0-test2-bk7/include/linux/ide.h~ide-disk-no-lba48-fix	2003-08-14 02:48:24.553723624 +0200
+++ linux-2.6.0-test2-bk7-root/include/linux/ide.h	2003-08-14 02:59:59.076140168 +0200
@@ -1007,7 +1007,6 @@ typedef struct hwif_s {
 
 	int		mmio;		/* hosts iomio (0), mmio (1) or custom (2) select */
 	int		rqsize;		/* max sectors per request */
-	int		addressing;	/* hosts addressing */
 	int		irq;		/* our irq number */
 	int		initializing;	/* set while initializing self */
 
@@ -1036,6 +1035,7 @@ typedef struct hwif_s {
 	unsigned	reset      : 1;	/* reset after probe */
 	unsigned	autodma    : 1;	/* auto-attempt using DMA at boot */
 	unsigned	udma_four  : 1;	/* 1=ATA-66 capable, 0=default */
+	unsigned	no_lba48   : 1; /* 1 = cannot do LBA48 */
 	unsigned	no_dsc     : 1;	/* 0 default, 1 dsc_overlap disabled */
 	unsigned	auto_poll  : 1; /* supports nop auto-poll */
 

_

Re: [PATCH] ide: limit drive capacity to 137GB if host doesn't support LBA48

[Alan Cox]

On Iau, 2003-08-14 at 02:24, Bartlomiej Zolnierkiewicz wrote:
>  	hwif->rqsize			= old_hwif.rqsize;
> -	hwif->addressing		= old_hwif.addressing;
> +	hwif->no_lba48			= old_hwif.no_lba48;

This change is a bad idea. Its called "addressing" because that is what
it is about (see SATA and ATA specs). In future SATA addressing becomes
a 0,1,2 value because 48bits isnt enough, it may get more forms beyond
that.

Might be worth defining ADDR_LBA48, ADDR_LBA28 etc to make it clearer,
but really people shouldnt be randomly hacking IDE code without having
read the specifications.

Re: [PATCH] ide: limit drive capacity to 137GB if host doesn't support LBA48

[Andries Brouwer]

On Thu, Aug 14, 2003 at 09:53:28AM +0100, Alan Cox wrote:
> On Iau, 2003-08-14 at 02:24, Bartlomiej Zolnierkiewicz wrote:
> >  	hwif->rqsize			= old_hwif.rqsize;
> > -	hwif->addressing		= old_hwif.addressing;
> > +	hwif->no_lba48			= old_hwif.no_lba48;
> 
> This change is a bad idea. Its called "addressing" because that is what
> it is about (see SATA and ATA specs). In future SATA addressing becomes
> a 0,1,2 value because 48bits isnt enough, it may get more forms beyond
> that.
> 
> Might be worth defining ADDR_LBA48, ADDR_LBA28 etc to make it clearer,
> but really people shouldnt be randomly hacking IDE code without having
> read the specifications.

Bartlomiej chose "no_lba48" (I had "cannot_do_lba48"), but otherwise
the change is mine. I will contradict you that it is a bad idea -
indeed, it is an excellent idea.
(Indeed, I made the change just for making the code more readable,
but immediately afterwards, reading this more readable code,
I understood why people had been experiencing filesystem corruption.)

Your strange remarks about reading specs - no doubt everybody who
touches IDE code knows the specs.

I don't know whether you saw my first post - but just to be sure let
me repeat a bit. There are two things that both are called addressing.
One is drive->addressing, and it is the enum that it looks like you
are thinking of. The other is hwif->addressing, and it is a boolean.
If this is ever changed then we have to visit all occurrences, and
also that is easier with the new name.

Andries

Re: [PATCH] ide: limit drive capacity to 137GB if host doesn't support LBA48

[Nick Piggin]

Alan Cox wrote:

>On Iau, 2003-08-14 at 02:24, Bartlomiej Zolnierkiewicz wrote:
>
>> 	hwif->rqsize			= old_hwif.rqsize;
>>-	hwif->addressing		= old_hwif.addressing;
>>+	hwif->no_lba48			= old_hwif.no_lba48;
>>
>
>This change is a bad idea. Its called "addressing" because that is what
>it is about (see SATA and ATA specs). In future SATA addressing becomes
>a 0,1,2 value because 48bits isnt enough, it may get more forms beyond
>that.
>

Wow! Isn't that over 100 thousand TB? ;)

Re: [PATCH] ide: limit drive capacity to 137GB if host doesn't support LBA48

[Alan Cox]

On Iau, 2003-08-14 at 10:18, Nick Piggin wrote
> >This change is a bad idea. Its called "addressing" because that is what
> >it is about (see SATA and ATA specs). In future SATA addressing becomes
> >a 0,1,2 value because 48bits isnt enough, it may get more forms beyond
> >that.
> >
> 
> Wow! Isn't that over 100 thousand TB? ;)

It depends how they are used and its more about message formats. LBA48 is
a pretty evil hack based on keeping some compatibility by writing some
registers twice and having the drive know about it.

Re: [PATCH] ide: limit drive capacity to 137GB if host doesn't support LBA48

[Bartlomiej Zolnierkiewicz]

On Thursday 14 of August 2003 10:53, Alan Cox wrote:
> On Iau, 2003-08-14 at 02:24, Bartlomiej Zolnierkiewicz wrote:
> >  	hwif->rqsize			= old_hwif.rqsize;
> > -	hwif->addressing		= old_hwif.addressing;
> > +	hwif->no_lba48			= old_hwif.no_lba48;
>
> This change is a bad idea. Its called "addressing" because that is what
> it is about (see SATA and ATA specs). In future SATA addressing becomes
> a 0,1,2 value because 48bits isnt enough, it may get more forms beyond
> that.

Its hwif->addressing not drive->addressing.  Look at the current usage.
It is 1 when host doesn't support LBA48, otherwise its 0.  We can add "real"
hwif->addressing when needed.  IDE driver is already full of unfinished,
unused "features".

--bartlomiej

Re: [PATCH] ide: limit drive capacity to 137GB if host doesn't support LBA48

[Alan Cox]

On Iau, 2003-08-14 at 13:48, Bartlomiej Zolnierkiewicz wrote:
> Its hwif->addressing not drive->addressing.  Look at the current usage.
> It is 1 when host doesn't support LBA48, otherwise its 0.  We can add "real"

Ok.

> hwif->addressing when needed.  IDE driver is already full of unfinished,
> unused "features".

Thats painfully true.

-- 
Alan Cox <alan@lxorguk.ukuu.org.uk>

Re: [PATCH] ide: limit drive capacity to 137GB if host doesn't support LBA48

[Jeff Garzik]

Alan Cox wrote:
> On Iau, 2003-08-14 at 02:24, Bartlomiej Zolnierkiewicz wrote:
> 
>> 	hwif->rqsize			= old_hwif.rqsize;
>>-	hwif->addressing		= old_hwif.addressing;
>>+	hwif->no_lba48			= old_hwif.no_lba48;
> 
> 
> This change is a bad idea. Its called "addressing" because that is what
> it is about (see SATA and ATA specs). In future SATA addressing becomes
> a 0,1,2 value because 48bits isnt enough, it may get more forms beyond
> that.
> 
> Might be worth defining ADDR_LBA48, ADDR_LBA28 etc to make it clearer,
> but really people shouldnt be randomly hacking IDE code without having
> read the specifications.


Yep.  Guess what?  Some host controller vendors are already building 
lba64 support into their chips...

	Jeff