Re: [PATCH] Experimental IDE oops dumper v0.1

[Rusty Russell <rusty@rustcorp.com.au>]

In message <200209161218.g8GCI7301692@devserv.devel.redhat.com> you write:
> > +	/* Pause (at least 400ns) in case we just issued a command */
> > +	oopser_usec(1);
> > +	for (i = 0; i < 1000; i++) {
> > +		b = inb(HD_STATUS);
> > +		if (!(b & BUSY_STAT)) {
> > +			if (b & ERR_STAT) return 0;
> > +			if (b & flag) return 1;
> > +		}
> > +		oopser_usec(1000);
> 
> This will stop working on SATA when VDMA goes into newer controllers btw.

My ignorance of ATA is incredibly deep.  I read an old IDE spec to get
this to work.  So thanks for checking it.

There are several ways around this: we can detect in userspace and
refuse to arm, we can detect in the kernel and refuse to arm, we can
detect in kernel or userspace and set an "use LBA48" flag.  For
example, my userspace code currently does:

	if (ioctl(devfd, HDIO_GET_IDENTITY, &hdid) < 0) {
		perror("Getting identity of drive");
		exit(1);
	}
	if (!(hdid.capability & (1 << 1))) {
		fprintf(stderr, "Drive does not support LBA\n");
		exit(1);
	}


> > +	/* Bits 24-27, 0x40=LBA, 0x10=slave */
> > +	if (!wait_before_command()) return -EIO;
> > +	outb(0x40 | (master ? 0 : 0x10) | ((lba >> 24) & 0x0F), HD_CURRENT);
> 
> Doesn't work for LBA48
> 
> > +	if (!wait_before_command()) return -EIO;
> > +	outb(0x40 | (master ? 0 : 0x10) | ((lba >> 24) & 0x0F), HD_CURRENT);
> 
> Ditto

OK, what's the codepath for LBA48 look like?  And how do I detect it?

> > +/* Called with interrupts off */
> > +int oopser_read_ide(char dump[512], unsigned int block)
> > +{
> > +	/* Wait for non-busy: if not, reset anyway */
> > +	wait_before_command();
> > +	/* Soft reset of drive (set nIEN as well) */
> > +	outb(0x0e, HD_CMD);
> 
> Be careful here - one or two drives get nIEN backwards, you might just
> want to turn off interrupts and be done with it

Hmm... I have interrupts disabled so I don't really care: should be OK
I think.  Or were you thinking of something else?

> > +	oopser_usec(1); /* 400ns according to spec */
> > +	outb(0x0a, HD_CMD);
> 
> You really need to reset and reprogram/retune the controller as well.

OK... How?

> I like the infrastructure but the IDE dumper code is wishful thinking
> in one or two spots. You don't know f the controller is in DMA modes,
> tuned for different things to the drives or legacy free. Im not sure what
> to do for legacy free cases but the other bits like LBA48 and retuning
> probably can be handled with some small chipset specific hooks

The code always does a read sector then a write: if the sector doesn't
contain the right magic, it stops.  But the more robust the better.

Patches appreciated 8)
Rusty.
--
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.