Skip to contentSkip to conversation

Poll-based IDE dump driver for LKCD

PATCH In review Srivatsa Vaddagiri 8 participants 18 messages Sep 17, 2003 – Oct 16, 2003 libata

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Poll-based IDE dump driver for LKCD

Hello ALL,
Attached patch adds poll-based dump driver support in LKCD
for IDE disks. The patch is against 2.6.0-test4. You will need the latest LKCD sources (lkcd.sourceforge.net) before you can apply this patch.

The IDE dump driver does PIO to the disk drive and is based on Rusty's OOPS dump driver and the kernel IDE disk driver.

The driver disables drive interrupts (nIEN bit) while writing to the disk. It then does a PIO of dump data to the disk and keeps polling for the write to be complete. After the write is over, it issues PIO for the next chunk of dump data and again polls for the write to get over.
This process is repeated till all the dump data is written to disk. At the end of it, drive interrupts are enabled again.

The driver makes use of the multiple sector mode feature (if any) that may be supported by the disk drive. For instance, if the drive supports writing upto 16 sectors at a time, then 8KB (16 * 512) of dump data are written at a time by the driver. This improves
the write performance.

The driver tries to minimize the dependency on kernel while writing to disk (since the state of the kernel may not
be predictable at the time of the crash). In this regard, it makes a separate copy of the ide_hwif_t data structure associated with the IDE drive and also has several ide_* functions duplicated The duplicated ide_* function have been made "dump safe" in some cases and in other cases have been merely duplicated to insulate from changes happening in the mainline IDE layer.

It is possible to introduce all this dump functionality in the existing kernel IDE disk driver itself and not have a separate IDE dump driver. We will do this based on the feedback from community.

I need inputs from IDE experts on several points:

- Would a reset of the drive be required before talking to it?
- When dump is initiated, I am assuming that the drive will be accessible at the I/O port addresses assigned to it initially during bootup. Can this assumption be wrong under some circumstances? If so, how do I deal with it?
- What if the drive is in some power-save state when dump is being initiated? How to deal with that situation?
- Range of IDE controllers supported:
Ideally I would like this driver to run on all the IDE controllers supported in Linux. To this extent, I have tried calling the I/O functions (OUTB, INB etc) associated with the IDE hwif (I have made an assumption that these I/O functions are dump safe).

Like this, are there some more measures that I need to take in order for my driver to run on all the IDE controllers supported in Linux?

Thanks in advance for any review comments on my patch!

drivers/dump/Makefile +7 -1
diff -Naur linux-2.6.0-test4.org/drivers/dump/Makefile linux-2.6.0-test4/drivers/dump/Makefile
@@ -9,7 +9,13 @@
 dump-objs				+= $(dump-y)
 
 obj-$(CONFIG_CRASH_DUMP)		+= dump.o
-obj-$(CONFIG_CRASH_DUMP_BLOCKDEV)	+= dump_blockdev.o
+
+# We link first dump_blockdev.o and then dump_ide.o.
+# This is done so that IDE driver gets a chance to claim the
+# target dump device and if it can't (because target is non-IDE disk)
+# dump_blockdev.c can claim it.
+
+obj-$(CONFIG_CRASH_DUMP_BLOCKDEV)	+= dump_blockdev.o dump_ide.o
 obj-$(CONFIG_CRASH_DUMP_NETDEV)	+= dump_netdev.o
 obj-$(CONFIG_CRASH_DUMP_COMPRESS_RLE)	+= dump_rle.o
 obj-$(CONFIG_CRASH_DUMP_COMPRESS_GZIP)	+= dump_gzip.o
drivers/dump/dump_ide.c +629 -0

629 lines not on this page · raw message

drivers/dump/dump_scheme.c +17 -5
diff -Naur linux-2.6.0-test4.org/drivers/dump/dump_scheme.c linux-2.6.0-test4/drivers/dump/dump_scheme.c
@@ -279,7 +279,7 @@
 
 int dump_generic_configure(unsigned long devid)
 {
-	struct dump_dev *dev = dump_config.dumper->dev;
+	struct dump_dev *dev;
 	struct dump_data_filter *filter;
 	void *buf;
 	int ret = 0;
@@ -299,12 +299,24 @@
 	dump_config.dumper->dump_buf = buf + DUMP_PAGE_SIZE;
 	dumper_reset();
 
-	/* Open the dump device */
-	if (!dev)
+	/* Find the appropriate block driver (IDE or SCSI) 
+	 * for this device.
+	 *
+	 * Note: dump_target_init implicitly calls the open
+	 * 	 routine of the appropriate driver to 
+	 * 	 handle this device.
+	 */
+	if (dump_target_init(devid)) {
+		dump_free_mem(buf);
 		return -ENODEV;
+	}
 
-	if ((ret = dev->ops->open(dev, devid))) {
-	       return ret;
+	dev = dump_config.dumper->dev = dump_dev;
+	
+	/* Open the dump device */
+	if (!dev) {
+		dump_free_mem(buf);
+		return -ENODEV;
 	}
 
 	/* Initialise the memory ranges in the dump filter */
drivers/dump/dump_setup.c +32 -8
diff -Naur linux-2.6.0-test4.org/drivers/dump/dump_setup.c linux-2.6.0-test4/drivers/dump/dump_setup.c
@@ -418,7 +418,6 @@
 	} else {
 		dump_config.dumper = &dumper_singlestage;
 	}	
-	dump_config.dumper->dev = dump_dev;
 
 	ret = dump_configure(devid);
 	if (!ret) {
@@ -434,12 +433,13 @@
 	return ret;
 }
 
-static int
-dump_target_init(int target)
+int
+dump_target_init(unsigned long devid)
 {
 	char type[20];
 	struct list_head *tmp;
 	struct dump_dev *dev;
+	int target = dump_config.flags & DUMP_FLAGS_TARGETMASK;
 	
 	switch (target) {
 		case DUMP_FLAGS_DISKDUMP:
@@ -458,8 +458,17 @@
 	list_for_each(tmp, &dump_target_list) {
 		dev = list_entry(tmp, struct dump_dev, list);
 		if (strcmp(type, dev->type_name) == 0) {
-			dump_dev = dev;
-			return 0;
+			int rc;
+
+			/* Essentially the open routine below
+			 * serves as a probing function.
+			 */
+			rc = dev->ops->open(dev, devid);
+			if (!rc) {
+				/* Ok ..the driver has accepted this device ..*/
+				dump_dev = dev;
+				return 0;
+			}
 		}
 	}
 	return -1;
@@ -543,9 +552,6 @@
 		if (arg < 0)
 			return -EINVAL;
 			
-		if (dump_target_init(arg & DUMP_FLAGS_TARGETMASK) < 0)
-			return -EINVAL; /* return proper error */
-
 		dump_config.flags = arg;
 		
 		pr_debug("Dump Flags 0x%lx\n", dump_config.flags);
@@ -645,6 +651,23 @@
 int
 dump_register_device(struct dump_dev *ddev)
 {
+
+	/* Allow more than one driver to register
+	 * with the same type_name. This is done
+	 * so that we can have multiple dump drivers (ex: IDE dump
+	 * driver, SCSI dump driver) of the same type
+	 * that can coexist.
+	 *
+	 * At configuration time, if the dump target is disk,
+	 * then the open routine of all registered dump drivers
+	 * having 'type_name' of "blockdev" will be invoked.
+	 * If the dump target is IDE disk, then the IDE dump
+	 * driver will claim it, if it is SCSI disk,
+	 * then the SCSI dump driver can claim it etc.
+	 *
+	 */
+
+#if 0
 	struct list_head *tmp;
 	struct dump_dev *dev;
 
@@ -656,6 +679,7 @@
 			return -1; /* return proper error */
 		}
 	}
+#endif
 	list_add(&(ddev->list), &dump_target_list);
 	
 	return 0;
Signature · 5 lines
Thanks and Regards,
Srivatsa Vaddagiri,
Linux Technology Center,
IBM Software Labs,
Bangalore, INDIA - 560033
#1 lore raw
Quoted 1 line · Srivatsa Vaddagiri

- Would a reset of the drive be required before talking to it?

We would have configured the device at boot up so it should not be. The disk may be in power saving states.

Quoted 3 lines

- When dump is initiated, I am assuming that the drive will be accessible at the I/O port addresses assigned to it initially during bootup. Can this assumption be wrong under some circumstances?

At the moment that is fairly safe because we don't handle hot unplugging and we don't switch controllers between modes

Quoted 2 lines

- What if the drive is in some power-save state when dump is being initiated? How to deal with that situation?

You must follow the ATA state diagrams.

Quoted 6 lines

- Range of IDE controllers supported:
Ideally I would like this driver to run on all the IDE controllers supported in Linux. To this extent, I have tried calling the I/O functions (OUTB, INB etc) associated with the IDE hwif (I have made an assumption that these I/O functions are dump safe).

Currently the code ought to be. We really only have three cases 'unplugged' 'pio' and 'mmio'. Pretty much all PATA controllers on Linux will behave sanely if you stick to single sector PIO and probably multi sector PIO (at least I see no reason why not). After the write you must also follow the shutdown sequence and flush the cache etc to be sure the dump is all on physical media.

Quoted 3 lines

Like this, are there some more measures that I need to take in order for my driver to run on all the IDE controllers supported in Linux?

For the old PATA stuff no - the interface is pretty standardised. For all the upcoming SATA stuff its likely to be one or more new standards that won't use the existing IDE layer at all and becomes more like the scsi dump problem.

drivers/dump/dump_ide.c line 59

Quoted 7 lines
drivers/dump/dump_ide.c +7 -0
+/* Wait for at least N usecs (1 clock per cycle, 10GHz processor = 10000) */
+/* ToDo : replace this routine based on loops_per_jiffy?? */
+static inline void dump_udelay(unsigned int num_usec)
+{
+	unsigned int i;
+	for (i = 0; i < 10000 * num_usec; i++);
+}

This routine must wait at least the right time delay. Also your delay code should be volatile. Right now i=10000*num_sec; return is a valid optimisation of your loop

drivers/dump/dump_ide.c line 227

Quoted 13 lines
drivers/dump/dump_ide.c +13 -0
+/* Same as 'ata_vlb_sync'.
+ * Has been duplicated to insulate from changes that
+ * can happen there
+ *
+ * ToDo: Remove this duplicate and call the original
+ * 'ata_tlb_sync'?
+ */
+void dump_ata_vlb_sync (ide_drive_t *drive, unsigned long port)
+{
+        (void) HWIF(drive)->INB(port);
+        (void) HWIF(drive)->INB(port);
+        (void) HWIF(drive)->INB(port);
+}

Wouldnt worry about Vesa local bus IMHO. If you drop that and io32 stuff all will be fine.

At the simplest providing you get no serious errors you could grab the port numbers and the base IN/OUT op fields and just bang the controller by hand. Basic PIO is an extremely simple sequence of operations even for LBA48, and you can obtain the mode to use CHS/LBA28/LBA48 from the IDE code before the box blows up.

Providing the IDE layer saw it you can be fairly sure the disk is programmed for a valid PIO mode and matching the controller programming, know the ports and know the command mode expected. With nIEN set you are using the polled state machine which also makes stuff really simple.

#2 lore raw

Re: [PATCH] Poll-based IDE driver

Alan,
Thanks for your review comments. I have modified
my code to take care of some of them.

I have couple of more questions:

1. Taking control of the drive
It is possible that the kernel IDE driver was very _actively_ using the dump drive when my driver takes over. For ex, there may be active DMA requests pending with the drive when my driver takes control ..

Currently all my drive does to take control of the drive is to disable drive interrupts (set nIEN bit). It then starts issuing PIO WRITE commands to write data to disk.

Do I need to do anything else in order to take control of the drive?

Do I need to explicitly program the drive for PIO transfer mode before I start issuing it PIO write commands?

2. Power Management
I found that the drive responds to my PIO write commands even when it is in Standby mode. So provided the drive is not in Sleep state, I guess I dont have to do anything special to take care of Power Management.

Tackling Sleep state may involve a drive reset followed by reinitializing drive parameters which will make my driver more complex (as I may have to take care of reset sequence of various chipsets separately). For now, I would like my driver not supporting the sleep state.

3. Shutdown sequence
You mentioned that I need to follow proper shutdown sequence when I am done writing to the drive. Apart from flushing (if the drive supports it) is there anything more to the shutdown sequence?

4. Relinquishing control of the drive
It is possible that after my dump is over system resumes working as before. This is termed as "non-disruptive" dumping in LKCD and is possible, for ex, in case of OOPS.

In such cases, I have to ensure that the kernel IDE driver continues working normally as before wrt the dump drive. Kernel IDE driver will have no knowledge of the fact that my dump driver had taken control and had done several PIO WRITEs to the drive before giving control back.

Currently all I am doing to give control back to the kernel IDE driver is to re-enable the drive interrupts (clear nIEN).

Is there anything more I need to be taking care of?

TIA for your comments ..

FYI I am attaching the modified polled IDE driver below :

~~~~~~~~~~~~~~~~~ BEGIN dump_ideops.c ~~~~~~~~~~~~~~~~~~~~~~~~~~

/*
* Low-level routines for doing polled I/O to IDE drive
* ====================================================
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation; either version 2 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program; if not, write to the Free Software
* Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. *
*

 *  Copyright (C) 1994-1998  Linus Torvalds & authors (see below)
 *  Copyright (C) 1998-2002  Linux ATA Development
 *  			     Andre Hedrick <[email protected]>
 *  Copyright (C) 2003       Red Hat <[email protected]>
 *  Copyright (C) IBM Corporation, 2003

*
* Started: August 2003 - Srivatsa Vaddagiri ([email protected])
* Based on dump_blockdev.c, IDE Disk Driver and
* Rusty's OOPS dump driver
*
*/

#include <linux/ide.h>
#include <linux/hdreg.h>
#include "dump_ide.h"

int is_valid_ide_hwif(ide_hwif_t *hwif)

{
	int i=0;

	for (i=0; i<MAX_HWIFS; ++i)
		if (hwif == &ide_hwifs[i])
			return 1;

	return 0;
}

void disable_drive_interrupts(ide_drive_t *drive)

{
	if (IDE_CONTROL_REG)
		HWIF(drive)->OUTB(drive->ctl|2, IDE_CONTROL_REG);

}

void enable_drive_interrupts(ide_drive_t *drive)

{
	if (IDE_CONTROL_REG)
		HWIF(drive)->OUTB(drive->ctl&0xfd, IDE_CONTROL_REG);

}

/* Wait for at least N usecs (1 clock per cycle, 10GHz processor = 10000) */ /* ToDo : replace this routine based on loops_per_jiffy?? */ static inline void dump_udelay(unsigned int num_usec)

{
	volatile unsigned int i;
	for (i = 0; i < 10000 * num_usec; i++);
}

/* This is essentially same as 'ide_wait_stat' but
* with these changes:
*
* - Calls to local_irq_set/local_irq_restore removed
* (Interrupts need to be kept disabled while dump
* is is in progress)
* - Dont consider max_failures for the drive
*
*/
static int
dump_ide_wait_stat (ide_startstop_t *startstop, ide_drive_t *drive, u8 good, u8 bad, unsigned long timeout)

{
	ide_hwif_t *hwif = HWIF(drive);
	u8 stat;
	int i;
 
	dump_udelay(1);	/* spec allows drive 400ns to assert "BUSY" */
	if ((stat = hwif->INB(IDE_STATUS_REG)) & BUSY_STAT) {
		i=0;
		while ((stat = hwif->INB(IDE_STATUS_REG)) & BUSY_STAT) {
			dump_udelay(1000);
			i+=1000;
			if (timeout && i>timeout) {
				printk ("dump_ide_wait_stat: Timed out waiting for drive to be ready \n");
				return -ETIMEDOUT;
			}
		}
	}
	/*
	 * Allow status to settle, then read it again.
	 * A few rare drives vastly violate the 400ns spec here,
	 * so we'll wait up to 10usec for a "good" status
	 * rather than expensively fail things immediately.
	 * This fix courtesy of Matthew Faupel & Niccolo Rigacci.
	 */
	for (i = 0; i < 10; i++) {
		dump_udelay(1);
		if (OK_STAT((stat = hwif->INB(IDE_STATUS_REG)), good, bad))
			return 0;
	}

	return -EIO;	// Invalid Status ...

}

/* Same as 'ata_vlb_sync'.
* Has been duplicated to insulate from changes that
* can happen there
*
* ToDo: Remove this duplicate and call the original
* 'ata_tlb_sync'?
*/
static void dump_ata_vlb_sync (ide_drive_t *drive, unsigned long port)

{
        (void) HWIF(drive)->INB(port);
        (void) HWIF(drive)->INB(port);
        (void) HWIF(drive)->INB(port);
}

/* Same as 'ata_output_data' but
* with these changes:
*
* - Remove calls to local_irq_save/local_irq_restore.
*
*/
static void dump_ata_output_data (ide_drive_t *drive, void *buffer, u32 wcount)

{
        ide_hwif_t *hwif        = HWIF(drive);
        u8 io_32bit             = drive->io_32bit;

        if (io_32bit) {
                if (io_32bit & 2) {
                        dump_ata_vlb_sync(drive, IDE_NSECTOR_REG);
                        hwif->OUTSL(IDE_DATA_REG, buffer, wcount);
                } else
                        hwif->OUTSL(IDE_DATA_REG, buffer, wcount);
        } else {
                hwif->OUTSW(IDE_DATA_REG, buffer, wcount<<1);
        }
}

/* Same as 'ata_bswap_data'.
* Has been duplicated to insulate from changes that
* can happen there
*
* ToDo: Remove this duplicate and call the original
* 'ata_bswap_data'?
*/
static void dump_ata_bswap_data (void *buffer, int wcount)

{
        u16 *p = buffer;

        while (wcount--) {
                *p = *p << 8 | *p >> 8; p++;
                *p = *p << 8 | *p >> 8; p++;
        }
}

/* Same as 'taskfile_output_data'.
* Has been duplicated to insulate from changes that
* can happen there
*
* ToDo: Remove this duplicate and call the original
* 'taskfile_output_data'?
*/
static void
dump_taskfile_output_data(ide_drive_t *drive, void *buffer, u32 wcount)

{
	if (drive->bswap) {
               dump_ata_bswap_data(buffer, wcount);
               dump_ata_output_data(drive, buffer, wcount);
               dump_ata_bswap_data(buffer, wcount);
        } else {
               dump_ata_output_data(drive, buffer, wcount);
        }
}

/* Highly simplified version of 'ide_multwrite' */

static int
dump_ide_multwrite(ide_drive_t *drive, void *buf, unsigned long len)

{
	int tx_words;

        tx_words = len>>2;

        dump_taskfile_output_data(drive, buf, tx_words);

        return len;
}

int dump_ide_pio_write(ide_drive_t *drive, sector_t block, char *buf, int len)

{
	ata_nsector_t	nsectors;
	u8 lba48      = (drive->addressing == 1) ? 1 : 0;
	task_ioreg_t command    = WIN_NOP;
	ide_startstop_t	ret;
        int mcount = drive->mult_count? drive->mult_count: 1;
        int burstlen;
        int actlen;
	int retval;

	/* ToDo : Should we consider making burstlen less than what device
	 * 	  supports?
	 */
	burstlen = mcount<<9;
        actlen = len < burstlen?len:burstlen;

	nsectors.all = actlen >> 9;

	/* ToDo : How safe is it to access the OUTB function below??
	 * 	  In most cases it should just be outb which shld
	 * 	  be safe enough. However, if on some platforms OUTB
	 * 	  does something more complicated, then we need to make
	 * 	  sure that it is dump "safe".
	 *
	 * 	  The same issue holds good for other I/O functions
	 * 	  (like INB, OUTSL etc) assosciated with the IDE interface.
	 *
	 */ 
	HWIF(drive)->OUTB(drive->select.all, IDE_SELECT_REG);

	if (drive->select.b.lba) {
		if (drive->addressing == 1) {
			task_ioreg_t tasklets[10];

			tasklets[0] = 0;
			tasklets[1] = 0;
			tasklets[2] = nsectors.b.low;
			tasklets[3] = nsectors.b.high;

			tasklets[4] = (task_ioreg_t) block;
			tasklets[5] = (task_ioreg_t) (block>>8);
			tasklets[6] = (task_ioreg_t) (block>>16);
			tasklets[7] = (task_ioreg_t) (block>>24);
			if (sizeof(block) == 4) {
				tasklets[8] = (task_ioreg_t) 0;
				tasklets[9] = (task_ioreg_t) 0;
			} else {
				tasklets[8] = (task_ioreg_t)((u64)block >> 32);
				tasklets[9] = (task_ioreg_t)((u64)block >> 40);
			}
			HWIF(drive)->OUTB(tasklets[1], IDE_FEATURE_REG);
			HWIF(drive)->OUTB(tasklets[3], IDE_NSECTOR_REG);
			HWIF(drive)->OUTB(tasklets[7], IDE_SECTOR_REG);
			HWIF(drive)->OUTB(tasklets[8], IDE_LCYL_REG);
			HWIF(drive)->OUTB(tasklets[9], IDE_HCYL_REG);

			HWIF(drive)->OUTB(tasklets[0], IDE_FEATURE_REG);
			HWIF(drive)->OUTB(tasklets[2], IDE_NSECTOR_REG);
			HWIF(drive)->OUTB(tasklets[4], IDE_SECTOR_REG);
			HWIF(drive)->OUTB(tasklets[5], IDE_LCYL_REG);
			HWIF(drive)->OUTB(tasklets[6], IDE_HCYL_REG);
			HWIF(drive)->OUTB(0x00|drive->select.all,IDE_SELECT_REG);
		} else {
			HWIF(drive)->OUTB(0x00, IDE_FEATURE_REG);
			HWIF(drive)->OUTB(nsectors.b.low, IDE_NSECTOR_REG);

			HWIF(drive)->OUTB(block, IDE_SECTOR_REG);
			HWIF(drive)->OUTB(block>>=8, IDE_LCYL_REG);
			HWIF(drive)->OUTB(block>>=8, IDE_HCYL_REG);
			HWIF(drive)->OUTB(((block>>8)&0x0f)|drive->select.all,IDE_SELECT_REG);
		}
	} else {
		unsigned int sect,head,cyl,track;
		track = (int)block / drive->sect;
		sect  = (int)block % drive->sect + 1;
		HWIF(drive)->OUTB(sect, IDE_SECTOR_REG);
		head  = track % drive->head;
		cyl   = track / drive->head;

		HWIF(drive)->OUTB(0x00, IDE_FEATURE_REG);
		HWIF(drive)->OUTB(nsectors.b.low, IDE_NSECTOR_REG);

		HWIF(drive)->OUTB(cyl, IDE_LCYL_REG);
		HWIF(drive)->OUTB(cyl>>8, IDE_HCYL_REG);
		HWIF(drive)->OUTB(head|drive->select.all,IDE_SELECT_REG);
	}

	command = ((drive->mult_count) ?
                           ((lba48) ? WIN_MULTWRITE_EXT : WIN_MULTWRITE) :
                           ((lba48) ? WIN_WRITE_EXT : WIN_WRITE));
        HWIF(drive)->OUTB(command, IDE_COMMAND_REG);

	retval = dump_ide_wait_stat(&ret, drive, DATA_READY, drive->bad_wstat, DUMP_WAIT_DRQ);
	if (retval)
		return retval;

	retval = dump_ide_multwrite(drive, buf, actlen);

	return retval;
}

int dump_do_idedisk_flushcache (ide_drive_t *drive)

{
	int rc;
	ide_hwif_t *hwif = HWIF(drive);
	int lba48 = drive->id->cfs_enable_2 & 0x2400;
	u8  flcmd = lba48?WIN_FLUSH_CACHE_EXT: WIN_FLUSH_CACHE;

	if (!(drive->id->cfs_enable_2 & 0x3000) && !(drive->id->cfs_enable_1 & 0x20) ) {
		printk ("Drive does not support write cache ..Hence nothing to flush ! \n");
		return 0;
	}
		
	/* ToDo : Dont we have to wait till the RDY bit is set before 
	 * 	  issuing a command?
	 */
	hwif->OUTB(drive->select.all, IDE_SELECT_REG);
	hwif->OUTBSYNC(drive, flcmd, IDE_COMMAND_REG);

	rc = dump_ide_wait_stat(NULL, drive, READY_STAT, BAD_STAT, DUMP_WAIT_WORSTCASE);

	if (rc) {
	       if (rc == -ETIMEDOUT)
			printk ("dump_do_idedisk_flushcache: Timedout waiting for response ..\n");
		rc = -EIO;
	}

	return rc;
}

int dump_get_ide_power_state(ide_drive_t *drive)

{
	int rc;
	ide_hwif_t	*hwif = HWIF(drive);
	
	/* ToDo : Dont we have to wait till the RDY bit is set before 
	 * 	  issuing a command?
	 */
	hwif->OUTB(drive->select.all, IDE_SELECT_REG);
	hwif->OUTBSYNC(drive, WIN_CHECKPOWERMODE1, IDE_COMMAND_REG);

	rc = dump_ide_wait_stat(NULL, drive, READY_STAT, BAD_STAT, DUMP_WAIT_READY);
	if (rc) {
		printk ("dump_get_ide_power_state: Bad Wait (rc = %d) \n", rc);
		return -EIO;
	}

	rc = hwif->INB(IDE_NSECTOR_REG);

	return rc;
}

~~~~~~~~~~~~~~~~~ END dump_ideops.c ~~~~~~~~~~~~~~~~~~~~~~~~~~

Quoted 83 lines · Alan Cox
Quoted 1 line · Srivatsa Vaddagiri

- Would a reset of the drive be required before talking to it?

We would have configured the device at boot up so it should not be. The disk may be in power saving states.

Quoted 3 lines

- When dump is initiated, I am assuming that the drive will be accessible at the I/O port addresses assigned to it initially during bootup. Can this assumption be wrong under some circumstances?

At the moment that is fairly safe because we don't handle hot unplugging and we don't switch controllers between modes

Quoted 2 lines

- What if the drive is in some power-save state when dump is being initiated? How to deal with that situation?

You must follow the ATA state diagrams.

Quoted 6 lines

- Range of IDE controllers supported:
Ideally I would like this driver to run on all the IDE controllers supported in Linux. To this extent, I have tried calling the I/O functions (OUTB, INB etc) associated with the IDE hwif (I have made an assumption that these I/O functions are dump safe).

Currently the code ought to be. We really only have three cases 'unplugged' 'pio' and 'mmio'. Pretty much all PATA controllers on Linux will behave sanely if you stick to single sector PIO and probably multi sector PIO (at least I see no reason why not). After the write you must also follow the shutdown sequence and flush the cache etc to be sure the dump is all on physical media.

Quoted 3 lines

Like this, are there some more measures that I need to take in order for my driver to run on all the IDE controllers supported in Linux?

For the old PATA stuff no - the interface is pretty standardised. For all the upcoming SATA stuff its likely to be one or more new standards that won't use the existing IDE layer at all and becomes more like the scsi dump problem.

drivers/dump/dump_ide.c line 59

Quoted 7 lines
drivers/dump/dump_ide.c +7 -0
+/* Wait for at least N usecs (1 clock per cycle, 10GHz processor = 10000) */
+/* ToDo : replace this routine based on loops_per_jiffy?? */
+static inline void dump_udelay(unsigned int num_usec)
+{
+	unsigned int i;
+	for (i = 0; i < 10000 * num_usec; i++);
+}

This routine must wait at least the right time delay. Also your delay code should be volatile. Right now i=10000*num_sec; return is a valid optimisation of your loop

drivers/dump/dump_ide.c line 227

Quoted 13 lines
drivers/dump/dump_ide.c +13 -0
+/* Same as 'ata_vlb_sync'.
+ * Has been duplicated to insulate from changes that
+ * can happen there
+ *
+ * ToDo: Remove this duplicate and call the original
+ * 'ata_tlb_sync'?
+ */
+void dump_ata_vlb_sync (ide_drive_t *drive, unsigned long port)
+{
+        (void) HWIF(drive)->INB(port);
+        (void) HWIF(drive)->INB(port);
+        (void) HWIF(drive)->INB(port);
+}

Wouldnt worry about Vesa local bus IMHO. If you drop that and io32 stuff all will be fine.

At the simplest providing you get no serious errors you could grab the port numbers and the base IN/OUT op fields and just bang the controller by hand. Basic PIO is an extremely simple sequence of operations even for LBA48, and you can obtain the mode to use CHS/LBA28/LBA48 from the IDE code before the box blows up.

Providing the IDE layer saw it you can be fairly sure the disk is programmed for a valid PIO mode and matching the controller programming, know the ports and know the command mode expected. With nIEN set you are using the polled state machine which also makes stuff really simple.

Signature · 5 lines
Thanks and Regards,
Srivatsa Vaddagiri,
Linux Technology Center,
IBM Software Labs,
Bangalore, INDIA - 560033
#3 lore raw

Re: [PATCH] Poll-based IDE driver

Quoted 7 lines · Srivatsa Vaddagiri

/* Wait for at least N usecs (1 clock per cycle, 10GHz processor = 10000) */ /* ToDo : replace this routine based on loops_per_jiffy?? */ static inline void dump_udelay(unsigned int num_usec)

{
	volatile unsigned int i;
	for (i = 0; i < 10000 * num_usec; i++);
}

Why not just use udelay() ? The above code cannot possibly do the right thing on all processors.

Dave

Signature · 1 line
 Dave Jones     http://www.codemonkey.org.uk
#4 lore raw

Re: [PATCH] Poll-based IDE driver

Quoted 3 lines · Dave Jones

Why not just use udelay() ? The above code cannot possibly do the right thing on all processors.

Since my code is supposed to run when system is crashing, I would like to avoid calling any function in the kernel as far as possible, since the kernel and its data structures may be in a inconsistent state and/or corrupted.

I do realize that the above code does not provide accurate delay and may not work on all platforms. In that direction I was considering using the loops_per_jiffy variable which may provide more accurate/platform-independent delay (?) ..

Signature · 5 lines
Thanks and Regards,
Srivatsa Vaddagiri,
Linux Technology Center,
IBM Software Labs,
Bangalore, INDIA - 560033
#5 lore raw

Re: [PATCH] Poll-based IDE driver

Quoted 7 lines · Srivatsa Vaddagiri
Quoted 2 lines

Why not just use udelay() ? The above code cannot possibly do the right thing on all processors.

Since my code is supposed to run when system is crashing, I would like to avoid calling any function in the kernel as far as possible, since the kernel and its data structures may be in a inconsistent state and/or corrupted.

By the same principle, your dump_udelay() is just as likely to get stomped on as the kernels udelay() function.

Quoted 4 lines

I do realize that the above code does not provide accurate delay and may not work on all platforms. In that direction I was considering using the loops_per_jiffy variable which may provide more accurate/platform-independent delay (?) ..

If you must reinvent a wheel, use a round one. Copy udelay().

Dave

Signature · 1 line
 Dave Jones     http://www.codemonkey.org.uk
#6 lore raw

Re: [PATCH] Poll-based IDE driver

In reply to Srivatsa Vaddagiri
Quoted 4 lines · Srivatsa Vaddagiri

I do realize that the above code does not provide accurate delay and may not work on all platforms. In that direction I was considering using the loops_per_jiffy variable which may provide more accurate/platform-independent delay (?) ..

loops_per_jiffy is meaningless when applied to anything other than the udelay function. It bears no resemblence in any way to the number of loops around a for loop.

Signature · 4 lines
Russell King ([email protected])	http://www.arm.linux.org.uk/personal/
      Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
      maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                      2.6 Serial core
#7 lore raw

Re: [lkcd-devel] Re: [PATCH] Poll-based IDE driver

In reply to Srivatsa Vaddagiri
Quoted 9 lines · On Wed, 8 Oct 2003 17:44:58 +0530 Srivatsa Vaddagiri <[email protected]> wrote:
Quoted 3 lines · Dave Jones

Why not just use udelay() ? The above code cannot possibly do the right thing on all processors.

Since my code is supposed to run when system is crashing, I would like to avoid calling any function in the kernel as far as possible, since the kernel and its data structures may be in a inconsistent state and/or corrupted.

If your kernel context is alive enough to take the dump, then it should be able to call things like udelay() that don't modify any data. If it can't your hosed already...

#8 lore raw

Re: [PATCH] Poll-based IDE driver

In reply to Srivatsa Vaddagiri
Quoted 4 lines · Srivatsa Vaddagiri

Since my code is supposed to run when system is crashing, I would like to avoid calling any function in the kernel as far as possible, since the kernel and its data structures may be in a inconsistent state and/or corrupted.

For x86 udelay is a tiny piece of code - you could easily inline it

#9 lore raw

Re: [PATCH] Poll-based IDE driver

Quoted 7 lines · Alan Cox
Quoted 4 lines · Srivatsa Vaddagiri

Since my code is supposed to run when system is crashing, I would like to avoid calling any function in the kernel as far as possible, since the kernel and its data structures may be in a inconsistent state and/or corrupted.

For x86 udelay is a tiny piece of code - you could easily inline it

No, because that changes the delay. Things like whether the instructions straddle a cache line or page boundary, or ar 4-byte or 16-byte aligned affect the timing on some x86 CPUs.

udelay must reside at a fixed memory location to get the nearest thing to determinism that we know how to get.

-- Jamie

#10 lore raw

Re: [PATCH] Poll-based IDE driver

In reply to Dave Jones

Does not matter, priority is to get content to platter and the hell with everything else. The reset button is your friend.

Andre Hedrick
LAD Storage Consulting Group

Quoted 22 lines · Dave Jones
Quoted 7 lines · Srivatsa Vaddagiri

/* Wait for at least N usecs (1 clock per cycle, 10GHz processor = 10000) */ /* ToDo : replace this routine based on loops_per_jiffy?? */ static inline void dump_udelay(unsigned int num_usec)

{
	volatile unsigned int i;
	for (i = 0; i < 10000 * num_usec; i++);
}

Why not just use udelay() ? The above code cannot possibly do the right thing on all processors.

Dave

Signature · 5 lines
 Dave Jones     http://www.codemonkey.org.uk
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#11 lore raw

Re: [PATCH] Poll-based IDE driver

Quoted 3 lines · Andre Hedrick

Does not matter, priority is to get content to platter and the hell with everything else.

I don't buy this. Without correct udelay()'s, how is code like this..

        for (i = 0; i < 10; i++) {
                dump_udelay(1);
                if (OK_STAT((stat = hwif->INB(IDE_STATUS_REG)), good, bad))
                        return 0;
        } 

expected to work ? It won't wait for 10usec at all, but be over almost instantly. Ramming commands at the drive before its status has settled doesn't strike me as a particularly safe thing to do.

Dave

Signature · 1 line
 Dave Jones     http://www.codemonkey.org.uk
#12 lore raw

Re: [PATCH] Poll-based IDE driver

The rules are different in POLLing.

Totally different state machine, and nothing seen here to date. You can not call any kernel services you can not depend upon.

Using altstat is actually required but that is another day.

Andre Hedrick
LAD Storage Consulting Group

Quoted 22 lines · Dave Jones
Quoted 3 lines · Andre Hedrick

Does not matter, priority is to get content to platter and the hell with everything else.

I don't buy this. Without correct udelay()'s, how is code like this..

        for (i = 0; i < 10; i++) {
                dump_udelay(1);
                if (OK_STAT((stat = hwif->INB(IDE_STATUS_REG)), good, bad))
                        return 0;
        } 

expected to work ? It won't wait for 10usec at all, but be over almost instantly. Ramming commands at the drive before its status has settled doesn't strike me as a particularly safe thing to do.

Dave

Signature · 1 line
 Dave Jones     http://www.codemonkey.org.uk
#13 lore raw

Re: [PATCH] Poll-based IDE driver

In reply to Srivatsa Vaddagiri
Quoted 5 lines · Srivatsa Vaddagiri

1. Taking control of the drive
It is possible that the kernel IDE driver was very _actively_ using the dump drive when my driver takes over. For ex, there may be active DMA requests pending with the drive when my driver takes control ..

In 2.4 we issue one command at a time in 2.6 we may have several TCQ commands queued. You may need to test if the drive is busy, wait and if neccessary reset and recover it. If possible avoid the reset because that means the drive will need some reconfiguration (see specs)

Quoted 3 lines

Currently all my drive does to take control of the drive is to disable drive interrupts (set nIEN bit). It then starts issuing PIO WRITE commands to write data to disk.

Make sure you disable interrupts before using nIEN. Not all older devices honour nIEN so you can get suprises

Quoted 5 lines

Do I need to do anything else in order to take control of the drive?

Do I need to explicitly program the drive for PIO transfer mode before I start issuing it PIO write commands?

The PIO mode on the controller and drive should have been set correctly by the base IDE driver at boot time.

Quoted 6 lines

2. Power Management
Tackling Sleep state may involve a drive reset followed by reinitializing drive parameters which will make my driver more complex (as I may have to take care of reset sequence of various chipsets separately). For now, I would like my driver not supporting the sleep state.

Makes sense. Many drives seem to come out of sleep state themselves anyway.

Quoted 5 lines

3. Shutdown sequence
You mentioned that I need to follow proper shutdown sequence when I am done writing to the drive. Apart from flushing (if the drive supports it) is there anything more to the shutdown sequence?

Flush the cache, and place the drive in a suitable standby/sleep state - follow the code in ide-disk.c for the way we do it in the main code.

Quoted 6 lines

4. Relinquishing control of the drive
Currently all I am doing to give control back to the kernel IDE driver is to re-enable the drive interrupts (clear nIEN).

Is there anything more I need to be taking care of?

Hard question. I think that providing you mark the drive busy, wait for the IDE layer to complete any pending commands and restore the state afterwards it ought to be possible. Would need a lot of thought and more time than I have right now to think about it since getting it wrong might lose requests and generate very hard to pin down errors.

#14 lore raw

Re: [PATCH] Poll-based IDE driver

Hardware reset and stomping all over the drive.
Force jam the do_request caller to fail with error_buffer. All drives have to honor polling or they fail POST on x86. nIEN is used to signal the HBA to pass the drive interrupt to the host cpu, other wise delivery does not happen period.
Assume nothing about PIO, because the drive will default to normal state and the HBA will magically match.
Sleep is not suspend, and these two are confused many times. Flush, who cares you disable write cache upon entry after the rest.

Cheers,

Andre Hedrick
LAD Storage Consulting Group

Quoted 66 lines · Alan Cox
Quoted 5 lines · Srivatsa Vaddagiri

1. Taking control of the drive
It is possible that the kernel IDE driver was very _actively_ using the dump drive when my driver takes over. For ex, there may be active DMA requests pending with the drive when my driver takes control ..

In 2.4 we issue one command at a time in 2.6 we may have several TCQ commands queued. You may need to test if the drive is busy, wait and if neccessary reset and recover it. If possible avoid the reset because that means the drive will need some reconfiguration (see specs)

Quoted 3 lines

Currently all my drive does to take control of the drive is to disable drive interrupts (set nIEN bit). It then starts issuing PIO WRITE commands to write data to disk.

Make sure you disable interrupts before using nIEN. Not all older devices honour nIEN so you can get suprises

Quoted 5 lines

Do I need to do anything else in order to take control of the drive?

Do I need to explicitly program the drive for PIO transfer mode before I start issuing it PIO write commands?

The PIO mode on the controller and drive should have been set correctly by the base IDE driver at boot time.

Quoted 6 lines

2. Power Management
Tackling Sleep state may involve a drive reset followed by reinitializing drive parameters which will make my driver more complex (as I may have to take care of reset sequence of various chipsets separately). For now, I would like my driver not supporting the sleep state.

Makes sense. Many drives seem to come out of sleep state themselves anyway.

Quoted 5 lines

3. Shutdown sequence
You mentioned that I need to follow proper shutdown sequence when I am done writing to the drive. Apart from flushing (if the drive supports it) is there anything more to the shutdown sequence?

Flush the cache, and place the drive in a suitable standby/sleep state - follow the code in ide-disk.c for the way we do it in the main code.

Quoted 6 lines

4. Relinquishing control of the drive
Currently all I am doing to give control back to the kernel IDE driver is to re-enable the drive interrupts (clear nIEN).

Is there anything more I need to be taking care of?

Hard question. I think that providing you mark the drive busy, wait for the IDE layer to complete any pending commands and restore the state afterwards it ought to be possible. Would need a lot of thought and more time than I have right now to think about it since getting it wrong might lose requests and generate very hard to pin down errors.

-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#15 lore raw

PRD Table

Hi all,

I am running into some possible DMA-related problems with my system. I am wondering, what is the easiest way to make the PRD Table region uncacheable? I appreciate any tips that will point me in the right direction.

Thanks!

__________________________________
Do you Yahoo!?
The New Yahoo! Shopping - with improved product search
http://shopping.yahoo.com

#16 lore raw

Re: PRD Table

Quoted 7 lines · id ol

Hi all,

I am running into some possible DMA-related problems with my system. I am wondering, what is the easiest way to make the PRD Table region uncacheable? I appreciate any tips that will point me in the right direction.

Hardware specific question. It depends on your platform.

#17 lore raw

Re: PRD Table

In reply to id ol

First question is why, and if you are doing lkcd, the answer is no.

Andre Hedrick
LAD Storage Consulting Group

Quoted 15 lines · id ol

Hi all,

I am running into some possible DMA-related problems with my system. I am wondering, what is the easiest way to make the PRD Table region uncacheable? I appreciate any tips that will point me in the right direction.

Thanks!

__________________________________
Do you Yahoo!?
The New Yahoo! Shopping - with improved product search
http://shopping.yahoo.com