sata_mv: Fix broken Marvell 7042 support.

sata_mv: Fix broken Marvell 7042 support.

The Marvell 7042 chip is more or less the same as the 6042 internally, but sports a PCIe bus. Despite having identical SATA cores, the 7042 does differ from its PCI bus counterparts in placment and layout of certain bus related registers.

This patch fixes sata_mv to distinguish between the PCI bus registers of earlier chips, and the PCIe bus registers of the 7042.

Specifically, move the offsets and bit patterns for the PCI/PCIe interrupt cause/mask registers into the struct mv_host_priv, as these values differ between the 6xxx and 7xxx series chips.

This fixes the driver to not access reserved PCI addresses, and prevents the lockups reported in linux-2.6.24 with 7042 boards.

Also add a new PCI ID for the Highpoint 2300 7042-based board that I'm using for testing this stuff here.

Tested with Marvell 6081 + 7042 chips, on x86 & x86_64.

---

Patch is agains 2.6.24-rc3-git5, and should go into 2.6.24.

@@ -164,10 +164,14 @@

	MV_PCI_ERR_ATTRIBUTE	= 0x1d48,
	MV_PCI_ERR_COMMAND	= 0x1d50,

- PCI_IRQ_CAUSE_OFS = 0x1d58,
- PCI_IRQ_MASK_OFS = 0x1d5c,
+ PCI_IRQ_CAUSE_OFS = 0x1d58,
+ PCI_IRQ_MASK_OFS = 0x1d5c,

	PCI_UNMASK_ALL_IRQS	= 0x7fffff,	/* bits 22-0 */

+ PCIE_IRQ_CAUSE_OFS = 0x1900,
+ PCIE_IRQ_MASK_OFS = 0x1910,
+ PCIE_UNMASK_ALL_IRQS = 0x70a, /* assorted bits */
+

	HC_MAIN_IRQ_CAUSE_OFS	= 0x1d60,
	HC_MAIN_IRQ_MASK_OFS	= 0x1d64,
	PORT0_ERR		= (1 << 0),	/* shift by port # */
@@ -303,6 +307,7 @@
	MV_HP_GEN_I		= (1 << 6),	/* Generation I: 50xx */
	MV_HP_GEN_II		= (1 << 7),	/* Generation II: 60xx */
	MV_HP_GEN_IIE		= (1 << 8),	/* Generation IIE: 6042/7042 */

+ MV_HP_PCIE = (1 << 9), /* PCIe bus/regs: 7042 */

	/* Port private flags (pp_flags) */
	MV_PP_FLAG_EDMA_EN	= (1 << 0),	/* is EDMA engine enabled? */
@@ -388,7 +393,15 @@
	u32			pre;
};

-struct mv_host_priv;
+struct mv_host_priv {
+ u32 hp_flags;
+ struct mv_port_signal signal[8];
+ const struct mv_hw_ops *ops;
+ u32 irq_cause_ofs;
+ u32 irq_mask_ofs;
+ u32 unmask_all_irqs;
+};
+

struct mv_hw_ops {
	void (*phy_errata)(struct mv_host_priv *hpriv, void __iomem *mmio,
			   unsigned int port);
@@ -401,12 +414,6 @@
	void (*reset_bus)(struct pci_dev *pdev, void __iomem *mmio);
};

-struct mv_host_priv {
- u32 hp_flags;
- struct mv_port_signal signal[8];
- const struct mv_hw_ops *ops;
-};
-
static void mv_irq_clear(struct ata_port *ap);
static int mv_scr_read(struct ata_port *ap, unsigned int sc_reg_in, u32 *val); static int mv_scr_write(struct ata_port *ap, unsigned int sc_reg_in, u32 val); @@ -631,11 +638,13 @@

	/* Adaptec 1430SA */
	{ PCI_VDEVICE(ADAPTEC2, 0x0243), chip_7042 },

- { PCI_VDEVICE(TTI, 0x2310), chip_7042 },
-
- /* add Marvell 7042 support */
+ /* Marvell 7042 support */

	{ PCI_VDEVICE(MARVELL, 0x7042), chip_7042 },

+ /* Highpoint RocketRAID PCIe series */
+ { PCI_VDEVICE(TTI, 0x2300), chip_7042 },
+ { PCI_VDEVICE(TTI, 0x2310), chip_7042 },
+

	{ }			/* terminate list */
};

@@ -1648,13 +1657,14 @@

static void mv_pci_error(struct ata_host *host, void __iomem *mmio) {

+	struct mv_host_priv *hpriv = host->private_data;
	struct ata_port *ap;
	struct ata_queued_cmd *qc;
	struct ata_eh_info *ehi;
	unsigned int i, err_mask, printed = 0;
	u32 err_cause;

- err_cause = readl(mmio + PCI_IRQ_CAUSE_OFS);
+ err_cause = readl(mmio + hpriv->irq_cause_ofs);

	dev_printk(KERN_ERR, host->dev, "PCI ERROR; PCI IRQ cause=0x%08x\n",
		   err_cause);
@@ -1662,7 +1672,7 @@
	DPRINTK("All regs @ PCI error\n");
	mv_dump_all_regs(mmio, -1, to_pci_dev(host->dev));

- writelfl(0, mmio + PCI_IRQ_CAUSE_OFS);
+ writelfl(0, mmio + hpriv->irq_cause_ofs);

	for (i = 0; i < host->n_ports; i++) {
		ap = host->ports[i];

@@ -1926,6 +1936,8 @@
#define ZERO(reg) writel(0, mmio + (reg))
static void mv_reset_pci_bus(struct pci_dev *pdev, void __iomem *mmio) {
+ struct ata_host *host = dev_get_drvdata(&pdev->dev);

+	struct mv_host_priv *hpriv = host->private_data;
	u32 tmp;

	tmp = readl(mmio + MV_PCI_MODE);
@@ -1937,8 +1949,8 @@
	writel(0x000100ff, mmio + MV_PCI_XBAR_TMOUT);
	ZERO(HC_MAIN_IRQ_MASK_OFS);
	ZERO(MV_PCI_SERR_MASK);
-	ZERO(PCI_IRQ_CAUSE_OFS);
-	ZERO(PCI_IRQ_MASK_OFS);
+	ZERO(hpriv->irq_cause_ofs);
+	ZERO(hpriv->irq_mask_ofs);
	ZERO(MV_PCI_ERR_LOW_ADDRESS);
	ZERO(MV_PCI_ERR_HIGH_ADDRESS);
	ZERO(MV_PCI_ERR_ATTRIBUTE);
@@ -2490,6 +2502,7 @@
		break;

	case chip_7042:
+		hp_flags |= MV_HP_PCIE;
	case chip_6042:
		hpriv->ops = &mv6xxx_ops;
		hp_flags |= MV_HP_GEN_IIE;
@@ -2516,6 +2529,15 @@
	}

	hpriv->hp_flags = hp_flags;

+ if (hp_flags & MV_HP_PCIE) {
+ hpriv->irq_cause_ofs = PCIE_IRQ_CAUSE_OFS;
+ hpriv->irq_mask_ofs = PCIE_IRQ_MASK_OFS;
+ hpriv->unmask_all_irqs = PCIE_UNMASK_ALL_IRQS;
+ } else {
+ hpriv->irq_cause_ofs = PCI_IRQ_CAUSE_OFS;
+ hpriv->irq_mask_ofs = PCI_IRQ_MASK_OFS;
+ hpriv->unmask_all_irqs = PCI_UNMASK_ALL_IRQS;
+ }

	return 0;
}
@@ -2595,10 +2617,10 @@
	}

	/* Clear any currently outstanding host interrupt conditions */

- writelfl(0, mmio + PCI_IRQ_CAUSE_OFS);
+ writelfl(0, mmio + hpriv->irq_cause_ofs);

	/* and unmask interrupt generation for host regs */

- writelfl(PCI_UNMASK_ALL_IRQS, mmio + PCI_IRQ_MASK_OFS);
+ writelfl(hpriv->unmask_all_irqs, mmio + hpriv->irq_mask_ofs);

	if (IS_GEN_I(hpriv))
		writelfl(~HC_MAIN_MASKED_IRQS_5, mmio + HC_MAIN_IRQ_MASK_OFS);

@@ -2609,8 +2631,8 @@
"PCI int cause/mask=0x%08x/0x%08x\n", readl(mmio + HC_MAIN_IRQ_CAUSE_OFS), readl(mmio + HC_MAIN_IRQ_MASK_OFS),
- readl(mmio + PCI_IRQ_CAUSE_OFS),
- readl(mmio + PCI_IRQ_MASK_OFS));
+ readl(mmio + hpriv->irq_cause_ofs),
+ readl(mmio + hpriv->irq_mask_ofs));

done:

	return rc;

applied #upstream-fixes

Ack - works in my 'unique' setup as well...:-)...

Thank you Mark!

Tom Morrison

-----Original Message-----
From: Mark Lord [mailto:[email protected]]
Sent: Saturday, December 01, 2007 1:07 PM
To: IDE/ATA development list; Jeff Garzik
Cc: Tejun Heo; Alan Cox; Morrison, Tom; Hein-Pieter van Braam
Subject: [PATCH] sata_mv: Fix broken Marvell 7042 support.

sata_mv: Fix broken Marvell 7042 support.

The Marvell 7042 chip is more or less the same as the 6042 internally, but sports a PCIe bus. Despite having identical SATA cores, the 7042 does differ from its PCI bus counterparts in placment and layout of certain bus related registers.

This patch fixes sata_mv to distinguish between the PCI bus registers of earlier chips, and the PCIe bus registers of the 7042.

Specifically, move the offsets and bit patterns for the PCI/PCIe interrupt cause/mask registers into the struct mv_host_priv, as these values differ between the 6xxx and 7xxx series chips.

This fixes the driver to not access reserved PCI addresses, and prevents the lockups reported in linux-2.6.24 with 7042 boards.

Also add a new PCI ID for the Highpoint 2300 7042-based board that I'm using for testing this stuff here.

Tested with Marvell 6081 + 7042 chips, on x86 & x86_64.

---

Patch is agains 2.6.24-rc3-git5, and should go into 2.6.24.

@@ -164,10 +164,14 @@
 	MV_PCI_ERR_ATTRIBUTE	= 0x1d48,
 	MV_PCI_ERR_COMMAND	= 0x1d50,
 
-	PCI_IRQ_CAUSE_OFS		= 0x1d58,
-	PCI_IRQ_MASK_OFS		= 0x1d5c,
+	PCI_IRQ_CAUSE_OFS	= 0x1d58,
+	PCI_IRQ_MASK_OFS	= 0x1d5c,
 	PCI_UNMASK_ALL_IRQS	= 0x7fffff,	/* bits 22-0 */
 
+	PCIE_IRQ_CAUSE_OFS	= 0x1900,
+	PCIE_IRQ_MASK_OFS	= 0x1910,
+	PCIE_UNMASK_ALL_IRQS	= 0x70a,	/* assorted bits */
+
 	HC_MAIN_IRQ_CAUSE_OFS	= 0x1d60,
 	HC_MAIN_IRQ_MASK_OFS	= 0x1d64,
 	PORT0_ERR		= (1 << 0),	/* shift by port # */

@@ -303,6 +307,7 @@
 	MV_HP_GEN_I		= (1 << 6),	/* Generation I: 50xx */
 	MV_HP_GEN_II		= (1 << 7),	/* Generation II: 60xx

*/
MV_HP_GEN_IIE = (1 << 8), /* Generation IIE:
6042/7042 */
+ MV_HP_PCIE = (1 << 9), /* PCIe bus/regs: 7042
*/

/* Port private flags (pp_flags) */
MV_PP_FLAG_EDMA_EN = (1 << 0), /* is EDMA engine
enabled? */
@@ -388,7 +393,15 @@
u32 pre;
};

-struct mv_host_priv;
+struct mv_host_priv {
+ u32 hp_flags;
+ struct mv_port_signal signal[8];
+ const struct mv_hw_ops *ops;
+ u32 irq_cause_ofs;
+ u32 irq_mask_ofs;
+ u32 unmask_all_irqs;
+};
+
struct mv_hw_ops {
void (*phy_errata)(struct mv_host_priv *hpriv, void __iomem
*mmio,
unsigned int port);
@@ -401,12 +414,6 @@
void (*reset_bus)(struct pci_dev *pdev, void __iomem *mmio);
};

-struct mv_host_priv {
- u32 hp_flags;
- struct mv_port_signal signal[8];
- const struct mv_hw_ops *ops;
-};
-
static void mv_irq_clear(struct ata_port *ap);
static int mv_scr_read(struct ata_port *ap, unsigned int sc_reg_in, u32
*val);
static int mv_scr_write(struct ata_port *ap, unsigned int sc_reg_in,
u32 val);
@@ -631,11 +638,13 @@
/* Adaptec 1430SA */
{ PCI_VDEVICE(ADAPTEC2, 0x0243), chip_7042 },

- { PCI_VDEVICE(TTI, 0x2310), chip_7042 },
-
- /* add Marvell 7042 support */
+ /* Marvell 7042 support */
{ PCI_VDEVICE(MARVELL, 0x7042), chip_7042 },

+ /* Highpoint RocketRAID PCIe series */
+ { PCI_VDEVICE(TTI, 0x2300), chip_7042 },
+ { PCI_VDEVICE(TTI, 0x2310), chip_7042 },
+
{ } /* terminate list */
};

@@ -1648,13 +1657,14 @@

static void mv_pci_error(struct ata_host *host, void __iomem *mmio) {
+ struct mv_host_priv *hpriv = host->private_data;
struct ata_port *ap;
struct ata_queued_cmd *qc;
struct ata_eh_info *ehi;
unsigned int i, err_mask, printed = 0;
u32 err_cause;

- err_cause = readl(mmio + PCI_IRQ_CAUSE_OFS);
+ err_cause = readl(mmio + hpriv->irq_cause_ofs);

dev_printk(KERN_ERR, host->dev, "PCI ERROR; PCI IRQ
cause=0x%08x\n",
err_cause);
@@ -1662,7 +1672,7 @@
DPRINTK("All regs @ PCI error\n"); mv_dump_all_regs(mmio, -1, to_pci_dev(host->dev));

- writelfl(0, mmio + PCI_IRQ_CAUSE_OFS);
+ writelfl(0, mmio + hpriv->irq_cause_ofs);

for (i = 0; i < host->n_ports; i++) {
ap = host->ports[i];
@@ -1926,6 +1936,8 @@
#define ZERO(reg) writel(0, mmio + (reg))
static void mv_reset_pci_bus(struct pci_dev *pdev, void __iomem *mmio) {
+ struct ata_host *host = dev_get_drvdata(&pdev->dev);
+ struct mv_host_priv *hpriv = host->private_data;
u32 tmp;

tmp = readl(mmio + MV_PCI_MODE);
@@ -1937,8 +1949,8 @@
writel(0x000100ff, mmio + MV_PCI_XBAR_TMOUT); ZERO(HC_MAIN_IRQ_MASK_OFS); ZERO(MV_PCI_SERR_MASK);
- ZERO(PCI_IRQ_CAUSE_OFS);
- ZERO(PCI_IRQ_MASK_OFS);
+ ZERO(hpriv->irq_cause_ofs);
+ ZERO(hpriv->irq_mask_ofs);
ZERO(MV_PCI_ERR_LOW_ADDRESS); ZERO(MV_PCI_ERR_HIGH_ADDRESS); ZERO(MV_PCI_ERR_ATTRIBUTE);
@@ -2490,6 +2502,7 @@
break;

case chip_7042:
+ hp_flags |= MV_HP_PCIE;
case chip_6042:
hpriv->ops = &mv6xxx_ops;
hp_flags |= MV_HP_GEN_IIE;
@@ -2516,6 +2529,15 @@
}

hpriv->hp_flags = hp_flags;
+ if (hp_flags & MV_HP_PCIE) {
+ hpriv->irq_cause_ofs = PCIE_IRQ_CAUSE_OFS;
+ hpriv->irq_mask_ofs = PCIE_IRQ_MASK_OFS;
+ hpriv->unmask_all_irqs = PCIE_UNMASK_ALL_IRQS;
+ } else {
+ hpriv->irq_cause_ofs = PCI_IRQ_CAUSE_OFS;
+ hpriv->irq_mask_ofs = PCI_IRQ_MASK_OFS;
+ hpriv->unmask_all_irqs = PCI_UNMASK_ALL_IRQS;
+ }

return 0;
}
@@ -2595,10 +2617,10 @@
}

/* Clear any currently outstanding host interrupt conditions */
- writelfl(0, mmio + PCI_IRQ_CAUSE_OFS);
+ writelfl(0, mmio + hpriv->irq_cause_ofs);

/* and unmask interrupt generation for host regs */
- writelfl(PCI_UNMASK_ALL_IRQS, mmio + PCI_IRQ_MASK_OFS);
+ writelfl(hpriv->unmask_all_irqs, mmio + hpriv->irq_mask_ofs);

if (IS_GEN_I(hpriv))
writelfl(~HC_MAIN_MASKED_IRQS_5, mmio +
HC_MAIN_IRQ_MASK_OFS);
@@ -2609,8 +2631,8 @@
"PCI int cause/mask=0x%08x/0x%08x\n", readl(mmio + HC_MAIN_IRQ_CAUSE_OFS), readl(mmio + HC_MAIN_IRQ_MASK_OFS),
- readl(mmio + PCI_IRQ_CAUSE_OFS),
- readl(mmio + PCI_IRQ_MASK_OFS));
+ readl(mmio + hpriv->irq_cause_ofs),
+ readl(mmio + hpriv->irq_mask_ofs));

done:
return rc;

It doesn't quite work for me, the system locks up without discernible error messages (Highpoint RocketRaid 2300 PCIe) . The drives come up, and immediately get hyper active, and I get dumped in an initrd busybox shell, when I try to tail /dev/sda it hangs, pvscan doesn't find any physical volumes for LVM, on the upside, I don't get the PCI ERROR messages anymore :) I'll try and get netconsole to work to get a proper log.

Not very useful feedback, I'll try and dig a little deeper this evening, I tried 2.6.24-rc3 and 2.6.24-rc3-git6 (couldn't find -git5 patchset, but I probably didn't look in the right place)

-git6 doesn't boot at all, complaining about not being able to execute init because it can't handle the binary format... but that's probably entirely unrelated to the sata_mv module.

Should it matter if I build as module or not? I'm now letting ubuntu hardware detection load the sata_mv.ko automatically from initrd, should I build a more stripped kernel?

Citeren "Morrison, Tom" <[email protected]>:

I should've added that I am working with a 2.6.23.x (x=8 right now) kernel/sata_mv.c. Which unless I am mistaken - has not change any until this patch - so I didn't have a problem...I need to work on the 2.6.23.x - because I am trying to stabilize my build to release a kernel for my developers to use for our next generation product...this was the last piece...and I am going through a regression to make sure everything is working correctly right now...

I don't think there should be a difference with module versus a built-in...

Tom

-----Original Message-----
From: [email protected] [mailto:[email protected]]
Sent: Monday, December 03, 2007 9:48 AM
To: Morrison, Tom
Cc: Mark Lord; IDE/ATA development list; Jeff Garzik; Tejun Heo; Alan Cox
Subject: RE: [PATCH] sata_mv: Fix broken Marvell 7042 support.

It doesn't quite work for me, the system locks up without discernible error messages (Highpoint RocketRaid 2300 PCIe) . The drives come up, and immediately get hyper active, and I get dumped in an initrd busybox shell, when I try to tail /dev/sda it hangs, pvscan doesn't find any physical volumes for LVM, on the upside, I don't get the PCI ERROR messages anymore :) I'll try and get netconsole to work to get a proper log.

Not very useful feedback, I'll try and dig a little deeper this evening, I tried 2.6.24-rc3 and 2.6.24-rc3-git6 (couldn't find -git5 patchset, but I probably didn't look in the right place)

-git6 doesn't boot at all, complaining about not being able to execute init because it can't handle the binary format... but that's probably entirely unrelated to the sata_mv module.

Should it matter if I build as module or not? I'm now letting ubuntu hardware detection load the sata_mv.ko automatically from initrd, should I build a more stripped kernel?

Citeren "Morrison, Tom" <[email protected]>:

in

...

Before this, you were booting from some other device, and the sata_mv mostly worked for you there, right ?

..

So at least that much should now be working much better for you.

I have not actually ever booted from a sata_mv controller yet, so maybe there's still something peculiar about libata::sata_mv early in the boot stages ?

I'm using sata_mv as a module here, but I *never* use initrd/initramfs images. They just complicate things needlessly once I know my hardware config. But it should work. I'll try booting from a sata_mv drive here on your board and see what's up.

Cheers

..

Okay, I've attempted to boot from the RocketRAID 2300 card using a known-good boot hard disk.

No such luck.

The BIOS on the Highpoint RR 2300 *corrupts* the GRUB image, so GRUB won't boot for me there. I actually have to re-install GRUB after each attempt so that the drive is usable again with the onboard Intel (ahci) ports.

So one then wonders exactly what the Highpoint BIOS is overwriting, and how that might affect your particular configuration..

???

I'll try booting from another (non-Highpoint) 7042 board next. After I repair GRUB again, that is.

Cheers

RAID metadata, one would assume from the 'RR'...

I've no idea if the standard solution is applicable to HPT RR: going through BIOS setup and turning off RAID completely, or if not possible, putting it into JBOD mode.

Jeff

..

It's an add-on PCIe board, with it's own onboard "RAID management" BIOS. The onboard BIOS has the drive labelled as "Legacy", but it still seems to insist upon writing it's meta data at every startup.

So to boot from it, we may need to find a way to keep our bootloader away from those same sector(s).

Cheers

Fwiw, I'm not running an add-on card or anything like it - no raid - nothing except a 7042 chip as the the front-end to 2-4 harddrives in my box) - in this case - I am not having a problem...

I write this as perhaps a hint to the 'right' direction to go looking for the trouble...

all the best~!

-----Original Message-----
From: Mark Lord [mailto:[email protected]]
Sent: Monday, December 03, 2007 1:32 PM
To: Jeff Garzik
Cc: [email protected]; Morrison, Tom; IDE/ATA development list; Tejun Heo; Alan Cox
Subject: Re: [PATCH] sata_mv: Fix broken Marvell 7042 support.

possible,

..

It's an add-on PCIe board, with it's own onboard "RAID management" BIOS. The onboard BIOS has the drive labelled as "Legacy", but it still seems to insist upon writing it's meta data at every startup.

So to boot from it, we may need to find a way to keep our bootloader away from those same sector(s).

Cheers

..

Thanks Tom.

So we know we *can* boot from a bare 7042, which is good!

I've poked a little more at this here, and I *think* the BIOS is overwriting the 9th sector on the drive.

I'll test that theory a little more.

Any GRUB experts out there? How do I get GRUB to install the stage2 stuff on a partition, instead of in the first 63 sectors ?

-ml

..

Confirmed. It writes "lgcy" + stuff into the 9th sector of the drive (for my "Legacy" drive).

Where does Linux softraid put it's stuff ?

And Hein-Pieter: what are you using for a bootloader ?

..

Technically, I suppose it's the stage 1.5 stuff that's getting nuked. Or something like that.

Thats quite nasty. Given that users putting volumes unpartitioned on drives may see actual data corruption and loss perhaps we should blacklist that controller variant with a large warning ?

Alan

..

Yeah, that's quite obnoxious of Highpoint to just arbitrarily overwrite data.

Some warnings would probably be quite useful here.

We could log a WARNING the first few times (after boot) whenever we see software writing to that sector. Do this with a hack in mv_qc_prep or mv_qc_issue ?

Or even just fail any write to that sector, so that the error gets propagated all the way back to usermode where it might be visible?

Plus some big nasty "awareness" messages at boot regardless.

If software never writes to it, then it should work fine. Except when drives are imported from somewhere else.. as I just discovered with my GRUB disk.

Then the disk is already corrupted before our kernel is even loaded.

...

Something like this, perhaps.

Comments, suggestions ?

It might be a good idea to also change qc_prep() semantics to allow the LLD to fail a request from there, rather than having to defer it to qc_issue(), (as done in this hack).

@@ -7568,6 +7568,7 @@

EXPORT_SYMBOL_GPL(sata_print_link_status);
EXPORT_SYMBOL_GPL(ata_tf_to_fis);
EXPORT_SYMBOL_GPL(ata_tf_from_fis);
+EXPORT_SYMBOL_GPL(ata_tf_read_block);
EXPORT_SYMBOL_GPL(ata_check_status);
EXPORT_SYMBOL_GPL(ata_altstatus);
EXPORT_SYMBOL_GPL(ata_exec_command);

@@ -64,7 +64,6 @@

extern int ata_build_rw_tf(struct ata_taskfile *tf, struct ata_device *dev,

			   u64 block, u32 n_block, unsigned int tf_flags,
			   unsigned int tag);

-extern u64 ata_tf_read_block(struct ata_taskfile *tf, struct ata_device *dev); extern void ata_dev_disable(struct ata_device *dev);
extern void ata_port_flush_task(struct ata_port *ap);
extern unsigned ata_exec_internal(struct ata_device *dev,

@@ -308,6 +308,7 @@

	MV_HP_GEN_II		= (1 << 7),	/* Generation II: 60xx */
	MV_HP_GEN_IIE		= (1 << 8),	/* Generation IIE: 6042/7042 */
	MV_HP_PCIE		= (1 << 9),	/* PCIe bus/regs: 7042 */

+ MV_HP_RESERVED_LBA8 = (1 << 10),

	/* Port private flags (pp_flags) */
	MV_PP_FLAG_EDMA_EN	= (1 << 0),	/* is EDMA engine enabled? */
@@ -1280,19 +1281,38 @@
{
	struct ata_port *ap = qc->ap;
	struct mv_port_priv *pp = ap->private_data;
+	struct mv_host_priv *hpriv = ap->host->private_data;
	struct mv_crqb_iie *crqb;
-	struct ata_taskfile *tf;
+	struct ata_taskfile *tf = &qc->tf;
	unsigned in_index;
	u32 flags = 0;

- if (qc->tf.protocol != ATA_PROT_DMA)
+ if (tf->protocol != ATA_PROT_DMA)

		return;

	/* Fill in Gen IIE command request block
	 */
-	if (!(qc->tf.flags & ATA_TFLAG_WRITE))
+	if (!(tf->flags & ATA_TFLAG_WRITE))
		flags |= CRQB_FLAG_READ;

+ if (tf->flags & ATA_TFLAG_WRITE) {
+ if (hpriv->hp_flags & MV_HP_RESERVED_LBA8) {
+ u64 start = ata_tf_read_block(tf, qc->dev);
+ u32 nsect = tf->nsect;
+ if (tf->flags & ATA_TFLAG_LBA48)
+ nsect |= tf->hob_nsect;
+ if (!nsect)
+ nsect = 256; /* good enough for here */
+ if (start <= 8 && (start + nsect) >= 8) {
+ printk(KERN_WARNING "ata%u: sector 8 is "
+ "reserved by Highpoint BIOS\n",
+ ap->print_id);
+ qc->err_mask |= AC_ERR_INVALID;
+ return;
+ }
+ }
+ }
+

	WARN_ON(MV_MAX_Q_DEPTH <= qc->tag);
	flags |= qc->tag << CRQB_TAG_SHIFT;
	flags |= qc->tag << CRQB_IOID_SHIFT;	/* "I/O Id" is -really-
@@ -1306,7 +1326,6 @@
	crqb->addr_hi = cpu_to_le32((pp->sg_tbl_dma >> 16) >> 16);
	crqb->flags = cpu_to_le32(flags);

-	tf = &qc->tf;
	crqb->ata_cmd[0] = cpu_to_le32(
			(tf->command << 16) |
			(tf->feature << 24)
@@ -1353,6 +1372,8 @@
	struct mv_host_priv *hpriv = ap->host->private_data;
	u32 in_index;

+ if (qc->err_mask & AC_ERR_INVALID) /* from qc_prep */

+		return AC_ERR_INVALID;
	if (qc->tf.protocol != ATA_PROT_DMA) {
		/* We're about to send a non-EDMA capable command to the
		 * port.  Turn off EDMA so there won't be problems accessing

@@ -2503,6 +2524,14 @@

	case chip_7042:
		hp_flags |= MV_HP_PCIE;

+ if (pdev->vendor == PCI_VENDOR_ID_TTI) {
+ if (pdev->device == 0x2300
+ || pdev->device == 0x2310) {
+ hp_flags |= MV_HP_RESERVED_LBA8;
+ printk(KERN_WARNING "sata_mv: RocketRAID BIOS "
+ "corrupts LBA 8 on connected drives\n");
+ }
+ }

	case chip_6042:
		hpriv->ops = &mv6xxx_ops;
		hp_flags |= MV_HP_GEN_IIE;

@@ -840,6 +840,7 @@

/*
* Default driver ops implementations
*/
+extern u64 ata_tf_read_block(struct ata_taskfile *tf, struct ata_device *dev); extern void ata_tf_load(struct ata_port *ap, const struct ata_taskfile *tf); extern void ata_tf_read(struct ata_port *ap, struct ata_taskfile *tf); extern void ata_tf_to_fis(const struct ata_taskfile *tf,

..

I'm not convinced that the big sledgehammer (previous patch which snooped for access to the sector that sometimes is overwritten by BIOS) is useful or does any good here.

The problem is, just by powering-on with a drive connected to the RocketRAID, one may have already toasted that sector, long before Linux/sata_mv ever get to see it or warn about it.

So, at a minimum, we do need to print some kind of disclaimer message to show that we're at least aware of it.

But beyond that, I don't think there's anything *really* useful to do. Comments?

I'll cook up a patch for 2.6.24 that detects and warns when a PCIe RocketRAID is found.

But do *all* HighPoint "RAID" cards do the same thing? Anyone out there got hardware other than a 2300/2310 ?

Thanks

The Highpoint RocketRAID boards using Marvell 7042 chips overwrite the 9th sector of attached drives at boot time, when those drives are configured as "Legacy" (the default) in the HighPoint BIOS.

This kills GRUB, and probably other stuff.
But it all happens *before* Linux is even loaded.

So, for now we'll log a WARNING when such boards are detected, and advise users to configure BIOS "JBOD" volumes instead, which don't appear to suffer from this problem.

---

Patch is for 2.6.24.

@@ -2503,6 +2503,15 @@
 

	case chip_7042:
		hp_flags |= MV_HP_PCIE;

+ if (pdev->vendor == PCI_VENDOR_ID_TTI
+ && (pdev->device == 0x2300 || pdev->device == 0x2310))
+ {
+ printk(KERN_WARNING "sata_mv: Highpoint RocketRAID BIOS"
+ " will corrupt attached drives when"
+ " configured as \"Legacy\". BEWARE!\n");
+ printk(KERN_WARNING "sata_mv: Use BIOS \"JBOD\" volumes"
+ " instead for safety.\n");
+ }

	case chip_6042:
		hpriv->ops = &mv6xxx_ops;
		hp_flags |= MV_HP_GEN_IIE;

applied #upstream-fixes with modifications (style, printk message), see attached.

commit 306b30f74d37f289033c696285e07ce0158a5d7b

Author: Mark Lord <[email protected]>
Date:   Tue Dec 4 14:07:52 2007 -0500

sata_mv: Warn about HPT RocketRAID BIOS treatment of "Legacy" drives

The Highpoint RocketRAID boards using Marvell 7042 chips overwrite the 9th sector of attached drives at boot time, when those drives are configured as "Legacy" (the default) in the HighPoint BIOS.

This kills GRUB, and probably other stuff. But it all happens *before* Linux is even loaded.

So, for now we'll log a WARNING when such boards are detected, and advise users to configure BIOS "JBOD" volumes instead, which don't appear to suffer from this problem.

 drivers/ata/sata_mv.c |    9 +++++++++
 1 file changed, 9 insertions(+)

306b30f74d37f289033c696285e07ce0158a5d7b

index 8d864e5..fe0105d 100644
@@ -2503,6 +2503,15 @@ static int mv_chip_id(struct ata_host *host, unsigned int board_idx)
 
 	case chip_7042:
 		hp_flags |= MV_HP_PCIE;
+		if (pdev->vendor == PCI_VENDOR_ID_TTI &&
+		    (pdev->device == 0x2300 || pdev->device == 0x2310))
+		{
+			printk(KERN_WARNING "sata_mv: Highpoint RocketRAID BIOS"
+				" will CORRUPT DATA on attached drives when"
+				" configured as \"Legacy\".  BEWARE!\n");
+			printk(KERN_WARNING "sata_mv: Use BIOS \"JBOD\" volumes"
+				" instead for safety.\n");
+		}
 	case chip_6042:
 		hpriv->ops = &mv6xxx_ops;
 		hp_flags |= MV_HP_GEN_IIE;

Mark Lord wrote:
...

...

No joy. My other 7042 and 6042 cards do not have bootable BIOSs.

So to boot from a 7042 the only theoretical choice is the Highpoint board, and for that we need to somehow coax it into not overwriting GRUB every time the onboard BIOS reinitializes.

Tricky to arrange, that.

How are you doing it ?

Personally, I put all the disks in JBOD mode, but I never had these disks connected to anything but my highpoint cards. Perhaps I should tell you what my setup's like:

* In the system I've put 3 Highpoint RocketRaid 2300 cards.
* Two of the cards each have 4 Hitachi 750GB disks
* Each disk is put into JBOD mode in the 'Raid' Bios of the 2300
* On the drives I have 2 partitions, one 200MB partition and a partition spanning the rest of the 750GB
* Both partitions are linux softraid
* /dev/md0 is a mirror of all 8 disks and serves as /boot (200MB)
* /dev/md1 contains an LVM volume that houses my other paritions (4.7TB)

Right now with 2.6.23.9 + patched sata_mv the softraid seems to come up just fine but I pvscan is unable to locate the pv located on md1.

I'll try and make a bootable USB stick with my patched 2.6.23.9 and see what the results are then.

This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.

..

How are you booting without the USB stick?
Are you booting from the Highpoint card drives?
What bootloader ?

Thanks

Right now I just boot using GRUB really, perhaps the BIOS hides the 9th sector if it's in some form of RAID mode? I haven't had any trouble using GRUB...

I haven't done anything special apart from putting the drives in JBOD mode, each disk is set to JBOD, it's set per disk, so not one large JBOD over 8 disks, but 8 separate JBOD volumes.

Hope that helps...

This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.

..

Man, what a quirky BIOS!

Okay, so if I connect a drive and don't do anything in the Highpoint BIOS setup, it then corrupts the drive by overwriting the 9th sector on every reboot.

But.. if I connect a drive and go into the Highpoint BIOS setup, and "initialize" the drive there, and then set it all as a "JBOD volume", it then leaves the drive content untouched on subsequent reboots.

Screwy to the max.

So, I might be able to boot from it by cloning the disk *after* setting it to "JBOD" in the BIOS. That will take a (longish) while to do.

I've just managed to boot with PXE and 2.6.23.9 and I have come to the following:

* softraid still works
* sata_mv is a bit slower than the proprietary highpoint drivers (I get 300MB/s from my array rather than 400MB/s)
* they array works
* the content of the array seems to have shifted 'down' a bit, that's probably why it won't boot.

it's a tad weird, but I can see the data just fine when I dump the content of the drives, but it seems to be in the wrong place.

I'll recreate my volumes with sata_mv now, see if it will work like this.

This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.

..

No worries.
That performance number will eventually come up as we develop sata_mv more.

NCQ and IRQ-coalescing together oughta do it. They're "on the list".

..

Shifted "down" which way, I wonder... ?
Maybe they're using a Host Protected Area, which would explain why I don't see them modifying the 9th sector after "initializing" (WTF?) my drive as a JBOD using their BIOS.

What does "hdparm -g /dev/sd*" show with their driver and with our driver?

Hum... I can pvcreate on the md device just fine, but after that, pvscan won't find it...

I have a suspicion that there's a BIOS doing some sector hiding or replacement of sorts...

*facepalm*

This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.

That would make sense. However we have weapons to make it surrender. See the "dmraid" tool - you should be able to use that as an example of how to set up device mapper mapped linear mappings to "unshift" partitions.

Alan

...

Ahh.. So dmraid is a way to work with using the HighPoint BIOS to define RAIDs. But not so advisable for maintaining adaptor-neutral pure Linux s/w RAID.

Right?

dmraid is a tool which knows large numbers of vendor "secret so the customer can't swap vendor easily" type disk formats. It then translates them into logical mappings using the dm layer, just as LVM2 does.

http://people.redhat.com/~heinzm/sw/dmraid/readme

..

Alan, what do we currently do (or not) with regards to HPAs in libata?

Older kernels ignore them by default, later ones report them, current one I believe reports them and disables them. However the HPA is the other end of the disk ...

I did some testing:
First I zero'd the first 80 meg of the md device, then read it back and compared, still all zero's. Then I rebooted and compared again, still all zero's

Then I got 8MB of stuff from /dev/urandom, wrote it to a file then wrote it to md device, read it back into a separate file, they still matched. I rebooted and read it back for a third time, and they still matched.

I'm not sure if this means anything or not, but I was wondering if my assertion of BIOS muckery still made sense

HP

This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.

..

I haven't seen any evidence of it here, either,
so long as BIOS "JBOD volumes" are used.

So I don't have any explanation for your data "moving around" when you switched drivers, unless *their* driver remaps things at run-time.

Or were you switching drivers across those reboots (likely, I suppose)?

No, I wasn't, I still have to try that, but I nuked my filesystem, so I have to rebuild my kernel... over NFS this time since I don't have any filesystem modules anymore, so things are slow... it's been building for an hour now.

I'll come up with the hdparm -g output and the results of this test with the two different drivers tomorrow.

also: on a side note, let me state again, that I'm extremely displeased with highpoint at this point... their whole 'open source linux drivers' thing is beginning to piss me off more and more. There must be something that can be done about this? OSI perhaps?

This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.

Hein-Pieter van Braam wrote:
..

..

Their stuff has always seemed more than a little screwy to me.

The solution for us here, is sata_mv, which will replace their drivers for this series of boards.

But.. Wow! a BIOS that just arbitrarily decides to overwrite a disk sector when one connects some other drive to it, without ever even entering their
BIOS setup screen.. And apparently doesn't even need to, since it does NOT
seem to do the same thing to drives that are configured as JBOD within their BIOS.

Ugh.

Cheers

..

Say.. the 2300 board I have here has BIOS 1.0 on it. Their website shows BIOS 2.2 available for download, along with a Linux utility to burn it.

Mmmm.. What version BIOS is on your copies of the 2300 ??

It just seriously pisses me off that because of their advertising 'Open Source drivers' they now sold ELEVEN of their boards more than they would have otherwise... I'm just bitching I guess...

Indeed, had I known any of this, I most certainly would have gone for 3-ware...

This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.

I zero'd all the disks, and now they show up as 'new' in the highpoint BIOS again... so, I guess there IS some reserved part somewhere that's accessible with the sata_mv driver.

that's a bit of a problem, because if you overwrite the RAID metadata, the raid bios won't allow booting from the devices...

This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.

..

Yeah, that's my suspicion too.. they must have just simply moved the metadata to near the end instead of sector-8.

Mmm... I'll try a smaller drive here,
and zero the entire drive, then reboot with it
on the Highpoint board, and then see what sector it modified.

This'll take a while.

Cheers

..

Actually, it doesn't take long at all,
with some creative use of fdisk.

And I haven't yet figured out their logic,
but here's what I've found.

They write their RAID metadata near-ish to the end of the drive. On my 320GB drives, it ended up at about -199853 sectors from the end of the drive. I have no idea what logic leads to them choosing such a peculiar location for it.

That's NOT an even cylinder or anything else I can figure out.

But if you leave the last 100KB of the drive unallocated for any partitions, then your data might survive.

This needs more research, and probably another patch of some kind in the driver for #upstream-fixes. I'm busy with a course all week, this won't happen (from me) until the weekend.

What a screwy BIOS.

-ml

..

Correction: Metadata begins at sector -191192 on my drive.

...

...

Okay, I figured it out.

The final sector on the drive is at 625142448,
which in hex is sector 0x2542eab0.

The metadata on this drive is at sector 624951296, which in hex is sector 0x25400000.

Note that this corresponds to a nice round giga-byte value.

So their RAID BIOS is just truncating the drive capacity down to an even gigabyte, and then putting the metadata right after that.

I don't know what they might do with a drive whose capacity is exactly some gigabyte value, though --> probably they just subtract a gigabyte from that, to make room for the metadata.

So.. I do want to make this board usable under Linux.

To do so, requires that we perhaps do a similar capacity truncation in sata_mv, but only if we see a metadata block at the expected location (because a "Legacy" mode drive will use the *real* capacity, placing the metadata in the 9th sector instead.

Jeff / Tejun / Alan:

What do you think we (I) should do here?

0. Initially, we need to further beef-up the recently added boot-time warning
about data-loss. Again, the BIOS will have *already* written metadata to
the drive before we ever get to warn about it. We cannot prevent it, but we should at least warn the user about this absurdity.
It would also be useful to print the max "usable" capacity as part of this warning (based on the calculation below).

Now we have a choice as to what else to do:

1. Just clip the drive capacity similar to how the BIOS does it.

  clipped_sectors = ((physical_sectors - 1) & ~0x0000000fffff);

OR:

2. Check for the Highpoint BIOS signature at the metadata sector first, and only clip capacity if we see their signature there.

OR:

3. Don't clip, but printk() WARNINGs whenever a write is performed to data beyond the theoretical clipped capacity.

OR:

4. Something better (?).

?????????????????????

Definitely _not_. This is a core Linux maxim: export what the hardware exports, no more no less. We drive the "bare metal."

OTOH it is quite reasonable to explore auto-loading DM on top of the bare drive, and populating a DM table, if you see that particular BIOS signature or [insert other detection method].

Jeff

..

We must remember that *data integrity* trumps all other principles here.

If we don't strongly discourage/prevent the user from installing Linux or storing data in the corruptible region, they *will lose data*, and blame us.

These are bootable cards, so people will install distro's from scratch directly to them, or to RAIDs configured onto them by the distro.

So whatever we do has to be something *safe* for those situations.

Cheers

Then auto-load a device map that gets it right.

The problem is not at the chip or device level, and this is the same problem as any number of other cards with softRAID on it. Not a new problem, not a new solution...

Jeff

..

What other cards do we support that automatically overwrite user data without confirmation or notice of any kind?

Just curious.

This card does it to any connected drive at power-on, without the user taking any action or even being told about it.

Cheers

..

The hardware limitation here is the SATA controller card:
it corrupts data at the last GB boundary.

The drives themselves are not limited --> one can unplug them and reconnect to a different controller and have full use of them.

Cheers

The BIOS does that, not the controller card.

If you pop the BIOS chip or plug the card into a non-x86 box (or any of several other alternatives), the problem is likely to go away.

Jeff

Jeff Garzik wrote:
...

..

Yeah, I was hoping for a removable BIOS chip, but it's soldered in place. And that's not a solution for most users anyway.

Cheers

That was an example, silly :) I'm not asking users to pop out chips. I'm illustrating that they are separate and distinct pieces, and you cannot assume.

Boot into a non-x86 platform, or use your x86 BIOS to disable all optional ROMs, and the BIOS-stomps-data issue goes away.

I'm not saying the _problem_ goes away; instead I am illustrating why it is incorrect to update sata_mv for this problem. The solution belongs elsewhere, because the problem is not with the chip, but the BIOS.

Continuing with the other emails...

If you use any vendor RAID (BIOS RAID / "fake RAID"), and fail to use DM+dmraid, then data corruption occurs due to lack of knowledge about the presence of underlying BIOS-created RAID metadata.

Your case is just another case of problems caused by lack of knowledge of the underlying vendor RAID that the BIOS insists upon using.

I'm pretty sure the most recently Fedora release has full dmraid support for known formats, so AFAICS the task at hand should be simply to figure out how to identify the underlying vendor RAID (on-disk signatures are greatly preferred over PCI ID matching), and update dmraid accordingly.

Welcome to the suck that is BIOS RAID :)

Jeff

Jeff Garzik wrote:
..

..

I am very interested to hear a more detailed explanation of this, as I don't really see how it addresses the problems.

Probably because I don't know much about device mapper.

But my understanding of it is that it re-exports portions of the original device as a second device.

It doesn't seem to prevent using the original device afterward, and I don't know if dm devices can have GRUB installed on them or not.

So a more full tutorial might be in order here.

Cheers

Just a little status update, I've been getting some REALLY strange results when using the disks as raid array, and I'm zeroing all the disks now to see if that will help. I'm not using the system not anyway, not as long as it requires the proprietary drivers to function.

(still using 2.6.23.9 + the patch that started this thread)

What I'm seeing now:
pvcreate works
pvscan doesn't work, MOST of the time, but if I say, put it in a watch loop after a while it'll pick it up, then lose it again, then pick it up again, sometimes complaining about corrupted meta-data
when it finally finds the pv, I get the same behavior from vgcreate, after a while it'll work, then fail again, complaining from time to time then vgscan, same, sometimes complaining that it can't find the volume group, but still NAMING the volume group
then lvcreate, same deal.

I just want to rule out any screwyness of the previous data on the disk first, and it'll take about 4 hours to wipe all 8 disks.

This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.

..

With the drive set-up as a "JBOD volume" in the HighPoint BIOS, I've now got a system that boots/runs from a single drive attached to the HighPoint 2300 PCIe board (Marvell 7042 - sata_mv).

So that much works, which is good!

I also then upgraded the card's BIOS from v1.0 to v2.2, and didn't notice anything different.

Cheers