All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] sata_mv:  Fix broken Marvell 7042 support.
@ 2007-12-01 18:07 Mark Lord
  2007-12-01 18:16 ` Alan Cox
                   ` (2 more replies)
  0 siblings, 3 replies; 54+ messages in thread
From: Mark Lord @ 2007-12-01 18:07 UTC (permalink / raw)
  To: IDE/ATA development list, Jeff Garzik
  Cc: Tejun Heo, Alan Cox, Tom Morrison, Hein-Pieter van Braam

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.

Signed-off-by: Mark Lord <mlord@pobox.com>
---

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

--- old/drivers/ata/sata_mv.c	2007-12-01 11:48:28.000000000 -0500
+++ linux/drivers/ata/sata_mv.c	2007-12-01 12:06:30.000000000 -0500
@@ -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;

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] sata_mv:  Fix broken Marvell 7042 support.
  2007-12-01 18:07 [PATCH] sata_mv: Fix broken Marvell 7042 support Mark Lord
@ 2007-12-01 18:16 ` Alan Cox
  2007-12-01 22:45 ` Jeff Garzik
  2007-12-03 12:27 ` Morrison, Tom
  2 siblings, 0 replies; 54+ messages in thread
From: Alan Cox @ 2007-12-01 18:16 UTC (permalink / raw)
  To: Mark Lord
  Cc: IDE/ATA development list, Jeff Garzik, Tejun Heo, Alan Cox,
	Tom Morrison, Hein-Pieter van Braam

On Sat, 01 Dec 2007 13:07:22 -0500
Mark Lord <liml@rtr.ca> wrote:

> 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.
> 
> Signed-off-by: Mark Lord <mlord@pobox.com>

Acked-by: Alan Cox <alan@redhat.com>

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] sata_mv:  Fix broken Marvell 7042 support.
  2007-12-01 18:07 [PATCH] sata_mv: Fix broken Marvell 7042 support Mark Lord
  2007-12-01 18:16 ` Alan Cox
@ 2007-12-01 22:45 ` Jeff Garzik
  2007-12-03 12:27 ` Morrison, Tom
  2 siblings, 0 replies; 54+ messages in thread
From: Jeff Garzik @ 2007-12-01 22:45 UTC (permalink / raw)
  To: Mark Lord
  Cc: IDE/ATA development list, Tejun Heo, Alan Cox, Tom Morrison,
	Hein-Pieter van Braam

Mark Lord wrote:
> 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.
> 
> Signed-off-by: Mark Lord <mlord@pobox.com>

applied #upstream-fixes



^ permalink raw reply	[flat|nested] 54+ messages in thread

* RE: [PATCH] sata_mv:  Fix broken Marvell 7042 support.
  2007-12-01 18:07 [PATCH] sata_mv: Fix broken Marvell 7042 support Mark Lord
  2007-12-01 18:16 ` Alan Cox
  2007-12-01 22:45 ` Jeff Garzik
@ 2007-12-03 12:27 ` Morrison, Tom
  2007-12-03 14:47   ` hp
  2 siblings, 1 reply; 54+ messages in thread
From: Morrison, Tom @ 2007-12-03 12:27 UTC (permalink / raw)
  To: Mark Lord, IDE/ATA development list, Jeff Garzik
  Cc: Tejun Heo, Alan Cox, Hein-Pieter van Braam

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

Thank you Mark!

Tom Morrison


-----Original Message-----
From: Mark Lord [mailto:liml@rtr.ca] 
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.

Signed-off-by: Mark Lord <mlord@pobox.com>
---

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

--- old/drivers/ata/sata_mv.c	2007-12-01 11:48:28.000000000 -0500
+++ linux/drivers/ata/sata_mv.c	2007-12-01 12:06:30.000000000 -0500
@@ -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;

^ permalink raw reply	[flat|nested] 54+ messages in thread

* RE: [PATCH] sata_mv:  Fix broken Marvell 7042 support.
  2007-12-03 12:27 ` Morrison, Tom
@ 2007-12-03 14:47   ` hp
  2007-12-03 14:56     ` Morrison, Tom
  2007-12-03 17:26     ` Mark Lord
  0 siblings, 2 replies; 54+ messages in thread
From: hp @ 2007-12-03 14:47 UTC (permalink / raw)
  To: Morrison, Tom
  Cc: Mark Lord, IDE/ATA development list, Jeff Garzik, Tejun Heo, Alan Cox

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" <tmorrison@empirix.com>:

> Ack - works in my 'unique' setup as well...:-)...
>
> Thank you Mark!
>
> Tom Morrison
>
>
> -----Original Message-----
> From: Mark Lord [mailto:liml@rtr.ca]
> 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.
>
> Signed-off-by: Mark Lord <mlord@pobox.com>
> ---
>
> Patch is agains 2.6.24-rc3-git5, and should go into 2.6.24.
>
> --- old/drivers/ata/sata_mv.c	2007-12-01 11:48:28.000000000 -0500
> +++ linux/drivers/ata/sata_mv.c	2007-12-01 12:06:30.000000000 -0500
> @@ -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;
> -
> To unsubscribe from this list: send the line "unsubscribe linux-ide" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>






^ permalink raw reply	[flat|nested] 54+ messages in thread

* RE: [PATCH] sata_mv:  Fix broken Marvell 7042 support.
  2007-12-03 14:47   ` hp
@ 2007-12-03 14:56     ` Morrison, Tom
  2007-12-03 17:26     ` Mark Lord
  1 sibling, 0 replies; 54+ messages in thread
From: Morrison, Tom @ 2007-12-03 14:56 UTC (permalink / raw)
  To: hp; +Cc: Mark Lord, IDE/ATA development list, Jeff Garzik, Tejun Heo, Alan Cox

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: hp@syntomax.com [mailto:hp@syntomax.com] 
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" <tmorrison@empirix.com>:

> Ack - works in my 'unique' setup as well...:-)...
>
> Thank you Mark!
>
> Tom Morrison
>
>
> -----Original Message-----
> From: Mark Lord [mailto:liml@rtr.ca]
> 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.
>
> Signed-off-by: Mark Lord <mlord@pobox.com>
> ---
>
> Patch is agains 2.6.24-rc3-git5, and should go into 2.6.24.
>
> --- old/drivers/ata/sata_mv.c	2007-12-01 11:48:28.000000000 -0500
> +++ linux/drivers/ata/sata_mv.c	2007-12-01 12:06:30.000000000
-0500
> @@ -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;
> -
> To unsubscribe from this list: send the line "unsubscribe linux-ide"
in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>






^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] sata_mv:  Fix broken Marvell 7042 support.
  2007-12-03 14:47   ` hp
  2007-12-03 14:56     ` Morrison, Tom
@ 2007-12-03 17:26     ` Mark Lord
  2007-12-03 18:14       ` Mark Lord
  1 sibling, 1 reply; 54+ messages in thread
From: Mark Lord @ 2007-12-03 17:26 UTC (permalink / raw)
  To: hp
  Cc: Morrison, Tom, IDE/ATA development list, Jeff Garzik, Tejun Heo,
	Alan Cox

hp@syntomax.com wrote:
> 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.
...

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

Specifically, you wrote:
> Now, because the chip on the thing is a Marvell 7042 I figured I just
> add the PCI ID to the driver. I tried this, and, if I do not boot from
> the device it does seem to work. I did however get the following errors
> (a lot):
> 
> 02:00.0 sata_mv PCI ERROR; PCI IRQ reason=0x00000000
> 
> I write this from memory, but the numbers are correct, sorry if they do
> not EXACTLY match.
> 
> The disk drives do work, but they are dog slow, and when I try to boot
> my ubuntu 7.10 system with this driver, it hangs during boot, right
> after it tried to enable my software raid volumes. 
> 
> I have tried both the 'stock' ubuntu kernel 2.6.22-14-generic (I guess
> that won't mean much) but I also tried vanilla kernel.org 2.6.23.1
..

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


^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] sata_mv:  Fix broken Marvell 7042 support.
  2007-12-03 17:26     ` Mark Lord
@ 2007-12-03 18:14       ` Mark Lord
  2007-12-03 18:30         ` Jeff Garzik
  2007-12-03 18:30         ` [PATCH] sata_mv: Fix broken Marvell 7042 support Mark Lord
  0 siblings, 2 replies; 54+ messages in thread
From: Mark Lord @ 2007-12-03 18:14 UTC (permalink / raw)
  To: hp
  Cc: Morrison, Tom, IDE/ATA development list, Jeff Garzik, Tejun Heo,
	Alan Cox

Mark Lord wrote:
> hp@syntomax.com wrote:
>> 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.
> ...
> 
> Before this, you were booting from some other device,
> and the sata_mv mostly worked for you there, right ?
> 
> Specifically, you wrote:
>> Now, because the chip on the thing is a Marvell 7042 I figured I just
>> add the PCI ID to the driver. I tried this, and, if I do not boot from
>> the device it does seem to work. I did however get the following errors
>> (a lot):
>>
>> 02:00.0 sata_mv PCI ERROR; PCI IRQ reason=0x00000000
>>
>> I write this from memory, but the numbers are correct, sorry if they do
>> not EXACTLY match.
>>
>> The disk drives do work, but they are dog slow, and when I try to boot
>> my ubuntu 7.10 system with this driver, it hangs during boot, right
>> after it tried to enable my software raid volumes.
>> I have tried both the 'stock' ubuntu kernel 2.6.22-14-generic (I guess
>> that won't mean much) but I also tried vanilla kernel.org 2.6.23.1
> ..
> 
> 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.
..

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

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] sata_mv:  Fix broken Marvell 7042 support.
  2007-12-03 18:14       ` Mark Lord
@ 2007-12-03 18:30         ` Jeff Garzik
  2007-12-03 18:32           ` Mark Lord
  2007-12-03 18:30         ` [PATCH] sata_mv: Fix broken Marvell 7042 support Mark Lord
  1 sibling, 1 reply; 54+ messages in thread
From: Jeff Garzik @ 2007-12-03 18:30 UTC (permalink / raw)
  To: Mark Lord
  Cc: hp, Morrison, Tom, IDE/ATA development list, Tejun Heo, Alan Cox

Mark Lord wrote:
> 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,

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



^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] sata_mv:  Fix broken Marvell 7042 support.
  2007-12-03 18:14       ` Mark Lord
  2007-12-03 18:30         ` Jeff Garzik
@ 2007-12-03 18:30         ` Mark Lord
  2007-12-03 20:11           ` Hein-Pieter van Braam
  2007-12-04  1:17           ` Mark Lord
  1 sibling, 2 replies; 54+ messages in thread
From: Mark Lord @ 2007-12-03 18:30 UTC (permalink / raw)
  To: hp
  Cc: Morrison, Tom, IDE/ATA development list, Jeff Garzik, Tejun Heo,
	Alan Cox

Mark Lord wrote:
...

> 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.
...

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 ?

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] sata_mv:  Fix broken Marvell 7042 support.
  2007-12-03 18:30         ` Jeff Garzik
@ 2007-12-03 18:32           ` Mark Lord
  2007-12-03 18:37             ` Morrison, Tom
  0 siblings, 1 reply; 54+ messages in thread
From: Mark Lord @ 2007-12-03 18:32 UTC (permalink / raw)
  To: Jeff Garzik
  Cc: hp, Morrison, Tom, IDE/ATA development list, Tejun Heo, Alan Cox

Jeff Garzik wrote:
> Mark Lord wrote:
>> 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,
> 
> 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.
..

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

^ permalink raw reply	[flat|nested] 54+ messages in thread

* RE: [PATCH] sata_mv:  Fix broken Marvell 7042 support.
  2007-12-03 18:32           ` Mark Lord
@ 2007-12-03 18:37             ` Morrison, Tom
  2007-12-03 18:40               ` Mark Lord
  0 siblings, 1 reply; 54+ messages in thread
From: Morrison, Tom @ 2007-12-03 18:37 UTC (permalink / raw)
  To: Mark Lord, Jeff Garzik; +Cc: hp, IDE/ATA development list, Tejun Heo, Alan Cox

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:liml@rtr.ca] 
Sent: Monday, December 03, 2007 1:32 PM
To: Jeff Garzik
Cc: hp@syntomax.com; Morrison, Tom; IDE/ATA development list; Tejun Heo;
Alan Cox
Subject: Re: [PATCH] sata_mv: Fix broken Marvell 7042 support.

Jeff Garzik wrote:
> Mark Lord wrote:
>> 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,
> 
> 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.
..

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

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] sata_mv:  Fix broken Marvell 7042 support.
  2007-12-03 18:37             ` Morrison, Tom
@ 2007-12-03 18:40               ` Mark Lord
  2007-12-03 18:44                 ` Mark Lord
  0 siblings, 1 reply; 54+ messages in thread
From: Mark Lord @ 2007-12-03 18:40 UTC (permalink / raw)
  To: Morrison, Tom
  Cc: Jeff Garzik, hp, IDE/ATA development list, Tejun Heo, Alan Cox

Morrison, Tom wrote:
> 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...
..

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 

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] sata_mv:  Fix broken Marvell 7042 support.
  2007-12-03 18:44                 ` Mark Lord
@ 2007-12-03 18:42                   ` Alan Cox
  2007-12-03 19:12                     ` Mark Lord
  0 siblings, 1 reply; 54+ messages in thread
From: Alan Cox @ 2007-12-03 18:42 UTC (permalink / raw)
  To: Mark Lord
  Cc: Morrison, Tom, Jeff Garzik, hp, IDE/ATA development list,
	Tejun Heo, Alan Cox

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

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

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] sata_mv:  Fix broken Marvell 7042 support.
  2007-12-03 18:40               ` Mark Lord
@ 2007-12-03 18:44                 ` Mark Lord
  2007-12-03 18:42                   ` Alan Cox
  0 siblings, 1 reply; 54+ messages in thread
From: Mark Lord @ 2007-12-03 18:44 UTC (permalink / raw)
  To: Morrison, Tom
  Cc: Jeff Garzik, hp, IDE/ATA development list, Tejun Heo, Alan Cox

Mark Lord wrote:
> Morrison, Tom wrote:
>> 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...
> ..
> 
> 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.
..

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 ?

> 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 ?
..

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

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] sata_mv:  Fix broken Marvell 7042 support.
  2007-12-03 18:42                   ` Alan Cox
@ 2007-12-03 19:12                     ` Mark Lord
  2007-12-03 20:40                       ` Mark Lord
  0 siblings, 1 reply; 54+ messages in thread
From: Mark Lord @ 2007-12-03 19:12 UTC (permalink / raw)
  To: Alan Cox
  Cc: Morrison, Tom, Jeff Garzik, hp, IDE/ATA development list,
	Tejun Heo, Alan Cox

Alan Cox wrote:
>> Confirmed.  It writes "lgcy" + stuff into the 9th sector of the drive
>> (for my "Legacy" drive).
> 
> 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 ?
..

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.


^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] sata_mv:  Fix broken Marvell 7042 support.
  2007-12-03 18:30         ` [PATCH] sata_mv: Fix broken Marvell 7042 support Mark Lord
@ 2007-12-03 20:11           ` Hein-Pieter van Braam
  2007-12-03 20:24             ` Mark Lord
  2007-12-04  1:17           ` Mark Lord
  1 sibling, 1 reply; 54+ messages in thread
From: Hein-Pieter van Braam @ 2007-12-03 20:11 UTC (permalink / raw)
  To: Mark Lord
  Cc: Morrison, Tom, IDE/ATA development list, Jeff Garzik, Tejun Heo,
	Alan Cox

On Mon, 2007-12-03 at 13:30 -0500, Mark Lord wrote:
> Mark Lord wrote:
> ...
> 
> > 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.
> ...
> 
> 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.


^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] sata_mv:  Fix broken Marvell 7042 support.
  2007-12-03 20:11           ` Hein-Pieter van Braam
@ 2007-12-03 20:24             ` Mark Lord
  2007-12-03 20:37               ` Hein-Pieter van Braam
  0 siblings, 1 reply; 54+ messages in thread
From: Mark Lord @ 2007-12-03 20:24 UTC (permalink / raw)
  To: Hein-Pieter van Braam
  Cc: Morrison, Tom, IDE/ATA development list, Jeff Garzik, Tejun Heo,
	Alan Cox

Hein-Pieter van Braam wrote:
>
> 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.
..

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

Thanks

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] sata_mv:  Fix broken Marvell 7042 support.
  2007-12-03 20:24             ` Mark Lord
@ 2007-12-03 20:37               ` Hein-Pieter van Braam
  2007-12-03 20:54                 ` Mark Lord
  0 siblings, 1 reply; 54+ messages in thread
From: Hein-Pieter van Braam @ 2007-12-03 20:37 UTC (permalink / raw)
  To: Mark Lord
  Cc: Morrison, Tom, IDE/ATA development list, Jeff Garzik, Tejun Heo,
	Alan Cox

> 
> > I'll try and make a bootable USB stick with my patched 2.6.23.9 and see
> > what the results are then.
> ..
> 
> 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.


^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] sata_mv:  Fix broken Marvell 7042 support.
  2007-12-03 19:12                     ` Mark Lord
@ 2007-12-03 20:40                       ` Mark Lord
  2007-12-03 23:59                         ` Mark Lord
  0 siblings, 1 reply; 54+ messages in thread
From: Mark Lord @ 2007-12-03 20:40 UTC (permalink / raw)
  To: Alan Cox
  Cc: Morrison, Tom, Jeff Garzik, hp, IDE/ATA development list,
	Tejun Heo, Alan Cox

Mark Lord wrote:
> Alan Cox wrote:
>>> Confirmed.  It writes "lgcy" + stuff into the 9th sector of the drive
>>> (for my "Legacy" drive).
>>
>> 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 ?
> ..
> 
> 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.
...

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).


--- old/drivers/ata/libata-core.c	2007-12-03 15:29:42.000000000 -0500
+++ linux/drivers/ata/libata-core.c	2007-12-03 15:21:15.000000000 -0500
@@ -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);
--- old/drivers/ata/libata.h	2007-12-03 15:29:37.000000000 -0500
+++ linux/drivers/ata/libata.h	2007-12-03 15:21:23.000000000 -0500
@@ -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,
--- old/drivers/ata/sata_mv.c	2007-12-03 15:29:43.000000000 -0500
+++ linux/drivers/ata/sata_mv.c	2007-12-03 15:33:17.000000000 -0500
@@ -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;
--- old/include/linux/libata.h	2007-12-03 15:29:42.000000000 -0500
+++ linux/include/linux/libata.h	2007-12-03 15:21:38.000000000 -0500
@@ -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,

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] sata_mv:  Fix broken Marvell 7042 support.
  2007-12-03 20:37               ` Hein-Pieter van Braam
@ 2007-12-03 20:54                 ` Mark Lord
  2007-12-03 22:28                   ` Hein-Pieter van Braam
  2007-12-03 22:48                   ` Hein-Pieter van Braam
  0 siblings, 2 replies; 54+ messages in thread
From: Mark Lord @ 2007-12-03 20:54 UTC (permalink / raw)
  To: Hein-Pieter van Braam
  Cc: Morrison, Tom, IDE/ATA development list, Jeff Garzik, Tejun Heo,
	Alan Cox

Hein-Pieter van Braam wrote:
>>> I'll try and make a bootable USB stick with my patched 2.6.23.9 and see
>>> what the results are then.
>> ..
>>
>> 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.
..

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.

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] sata_mv:  Fix broken Marvell 7042 support.
  2007-12-03 20:54                 ` Mark Lord
@ 2007-12-03 22:28                   ` Hein-Pieter van Braam
  2007-12-03 23:37                     ` Mark Lord
  2007-12-03 22:48                   ` Hein-Pieter van Braam
  1 sibling, 1 reply; 54+ messages in thread
From: Hein-Pieter van Braam @ 2007-12-03 22:28 UTC (permalink / raw)
  To: Mark Lord
  Cc: Morrison, Tom, IDE/ATA development list, Jeff Garzik, Tejun Heo,
	Alan Cox


> 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.


^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] sata_mv:  Fix broken Marvell 7042 support.
  2007-12-03 20:54                 ` Mark Lord
  2007-12-03 22:28                   ` Hein-Pieter van Braam
@ 2007-12-03 22:48                   ` Hein-Pieter van Braam
  2007-12-03 23:10                     ` Alan Cox
  1 sibling, 1 reply; 54+ messages in thread
From: Hein-Pieter van Braam @ 2007-12-03 22:48 UTC (permalink / raw)
  To: Mark Lord
  Cc: Morrison, Tom, IDE/ATA development list, Jeff Garzik, Tejun Heo,
	Alan Cox

> Man, what a quirky BIOS!

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.


^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] sata_mv:  Fix broken Marvell 7042 support.
  2007-12-03 22:48                   ` Hein-Pieter van Braam
@ 2007-12-03 23:10                     ` Alan Cox
  2007-12-03 23:33                       ` Mark Lord
                                         ` (2 more replies)
  0 siblings, 3 replies; 54+ messages in thread
From: Alan Cox @ 2007-12-03 23:10 UTC (permalink / raw)
  To: Hein-Pieter van Braam
  Cc: Mark Lord, Morrison, Tom, IDE/ATA development list, Jeff Garzik,
	Tejun Heo, Alan Cox

On Mon, 03 Dec 2007 23:48:44 +0100
Hein-Pieter van Braam <hp@syntomax.com> wrote:

> > Man, what a quirky BIOS!
> 
> 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*

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

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] sata_mv:  Fix broken Marvell 7042 support.
  2007-12-03 23:10                     ` Alan Cox
@ 2007-12-03 23:33                       ` Mark Lord
  2007-12-03 23:34                         ` Alan Cox
  2007-12-03 23:47                       ` Mark Lord
  2007-12-04  0:01                       ` Hein-Pieter van Braam
  2 siblings, 1 reply; 54+ messages in thread
From: Mark Lord @ 2007-12-03 23:33 UTC (permalink / raw)
  To: Alan Cox
  Cc: Hein-Pieter van Braam, Morrison, Tom, IDE/ATA development list,
	Jeff Garzik, Tejun Heo, Alan Cox

Alan Cox wrote:
> On Mon, 03 Dec 2007 23:48:44 +0100
> Hein-Pieter van Braam <hp@syntomax.com> wrote:
> 
>>> Man, what a quirky BIOS!
>> 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*
> 
> 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.
...

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?

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] sata_mv:  Fix broken Marvell 7042 support.
  2007-12-03 23:33                       ` Mark Lord
@ 2007-12-03 23:34                         ` Alan Cox
  0 siblings, 0 replies; 54+ messages in thread
From: Alan Cox @ 2007-12-03 23:34 UTC (permalink / raw)
  To: Mark Lord
  Cc: Hein-Pieter van Braam, Morrison, Tom, IDE/ATA development list,
	Jeff Garzik, Tejun Heo, Alan Cox

On Mon, 03 Dec 2007 18:33:10 -0500
Mark Lord <liml@rtr.ca> wrote:

> Alan Cox wrote:
> > On Mon, 03 Dec 2007 23:48:44 +0100
> > Hein-Pieter van Braam <hp@syntomax.com> wrote:
> > 
> >>> Man, what a quirky BIOS!
> >> 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*
> > 
> > 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.
> ...
> 
> 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.

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

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] sata_mv:  Fix broken Marvell 7042 support.
  2007-12-03 22:28                   ` Hein-Pieter van Braam
@ 2007-12-03 23:37                     ` Mark Lord
  0 siblings, 0 replies; 54+ messages in thread
From: Mark Lord @ 2007-12-03 23:37 UTC (permalink / raw)
  To: Hein-Pieter van Braam
  Cc: Morrison, Tom, IDE/ATA development list, Jeff Garzik, Tejun Heo,
	Alan Cox

Hein-Pieter van Braam wrote:
>> 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)
..

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".

> * they array works
> * the content of the array seems to have shifted 'down' a bit, that's
> probably why it won't boot. 
..

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?


^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] sata_mv:  Fix broken Marvell 7042 support.
  2007-12-03 23:10                     ` Alan Cox
  2007-12-03 23:33                       ` Mark Lord
@ 2007-12-03 23:47                       ` Mark Lord
  2007-12-03 23:47                         ` Alan Cox
  2007-12-04  0:01                       ` Hein-Pieter van Braam
  2 siblings, 1 reply; 54+ messages in thread
From: Mark Lord @ 2007-12-03 23:47 UTC (permalink / raw)
  To: Alan Cox
  Cc: Hein-Pieter van Braam, Morrison, Tom, IDE/ATA development list,
	Jeff Garzik, Tejun Heo, Alan Cox

Alan Cox wrote:
> On Mon, 03 Dec 2007 23:48:44 +0100
> Hein-Pieter van Braam <hp@syntomax.com> wrote:
> 
>>> Man, what a quirky BIOS!
>> 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... 
..

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

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] sata_mv:  Fix broken Marvell 7042 support.
  2007-12-03 23:47                       ` Mark Lord
@ 2007-12-03 23:47                         ` Alan Cox
  0 siblings, 0 replies; 54+ messages in thread
From: Alan Cox @ 2007-12-03 23:47 UTC (permalink / raw)
  To: Mark Lord
  Cc: Hein-Pieter van Braam, Morrison, Tom, IDE/ATA development list,
	Jeff Garzik, Tejun Heo, Alan Cox

On Mon, 03 Dec 2007 18:47:08 -0500
Mark Lord <liml@rtr.ca> wrote:

> Alan Cox wrote:
> > On Mon, 03 Dec 2007 23:48:44 +0100
> > Hein-Pieter van Braam <hp@syntomax.com> wrote:
> > 
> >>> Man, what a quirky BIOS!
> >> 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... 
> ..
> 
> 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 ...

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] sata_mv:  Fix broken Marvell 7042 support.
  2007-12-03 20:40                       ` Mark Lord
@ 2007-12-03 23:59                         ` Mark Lord
  2007-12-04  0:20                           ` [PATCH] sata_mv: Warn about Highpoint RocketRAID BIOS treatment of "Legacy" drives Mark Lord
  0 siblings, 1 reply; 54+ messages in thread
From: Mark Lord @ 2007-12-03 23:59 UTC (permalink / raw)
  To: Alan Cox
  Cc: Morrison, Tom, Jeff Garzik, hp, IDE/ATA development list,
	Tejun Heo, Alan Cox

Mark Lord wrote:
> Mark Lord wrote:
>> Alan Cox wrote:
>>>> Confirmed.  It writes "lgcy" + stuff into the 9th sector of the drive
>>>> (for my "Legacy" drive).
>>>
>>> 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 ?
>> ..
>>
>> 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.
> ...
> 
> Something like this, perhaps.
> 
> Comments, suggestions ?
..

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

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] sata_mv:  Fix broken Marvell 7042 support.
  2007-12-03 23:10                     ` Alan Cox
  2007-12-03 23:33                       ` Mark Lord
  2007-12-03 23:47                       ` Mark Lord
@ 2007-12-04  0:01                       ` Hein-Pieter van Braam
  2007-12-04  0:07                         ` Mark Lord
  2 siblings, 1 reply; 54+ messages in thread
From: Hein-Pieter van Braam @ 2007-12-04  0:01 UTC (permalink / raw)
  To: Alan Cox
  Cc: Mark Lord, Morrison, Tom, IDE/ATA development list, Jeff Garzik,
	Tejun Heo, Alan Cox

On Mon, 2007-12-03 at 23:10 +0000, Alan Cox wrote:
> On Mon, 03 Dec 2007 23:48:44 +0100
> Hein-Pieter van Braam <hp@syntomax.com> wrote:
> 
> > > Man, what a quirky BIOS!
> > 
> > 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*
> 
> 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

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.


^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] sata_mv:  Fix broken Marvell 7042 support.
  2007-12-04  0:01                       ` Hein-Pieter van Braam
@ 2007-12-04  0:07                         ` Mark Lord
  2007-12-04  0:17                           ` Hein-Pieter van Braam
  0 siblings, 1 reply; 54+ messages in thread
From: Mark Lord @ 2007-12-04  0:07 UTC (permalink / raw)
  To: Hein-Pieter van Braam
  Cc: Alan Cox, Morrison, Tom, IDE/ATA development list, Jeff Garzik,
	Tejun Heo, Alan Cox

Hein-Pieter van Braam wrote:
>
> 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 
..

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)?

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] sata_mv:  Fix broken Marvell 7042 support.
  2007-12-04  0:07                         ` Mark Lord
@ 2007-12-04  0:17                           ` Hein-Pieter van Braam
  2007-12-04  0:23                             ` Mark Lord
  2007-12-04 19:21                             ` Hein-Pieter van Braam
  0 siblings, 2 replies; 54+ messages in thread
From: Hein-Pieter van Braam @ 2007-12-04  0:17 UTC (permalink / raw)
  To: Mark Lord
  Cc: Alan Cox, Morrison, Tom, IDE/ATA development list, Jeff Garzik,
	Tejun Heo, Alan Cox


> 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.


^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH] sata_mv:  Warn about Highpoint RocketRAID BIOS treatment of "Legacy" drives
  2007-12-03 23:59                         ` Mark Lord
@ 2007-12-04  0:20                           ` Mark Lord
  2007-12-04 19:09                             ` Jeff Garzik
  0 siblings, 1 reply; 54+ messages in thread
From: Mark Lord @ 2007-12-04  0:20 UTC (permalink / raw)
  To: Jeff Garzik
  Cc: Alan Cox, Morrison, Tom, hp, IDE/ATA development list, Tejun Heo,
	Alan Cox

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.

Signed-off-by: Mark Lord <mlord@pobox.com>
---

Patch is for 2.6.24.

--- old/drivers/ata/sata_mv.c	2007-12-03 15:29:43.000000000 -0500
+++ linux/drivers/ata/sata_mv.c	2007-12-03 19:15:23.000000000 -0500
@@ -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;

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] sata_mv:  Fix broken Marvell 7042 support.
  2007-12-04  0:17                           ` Hein-Pieter van Braam
@ 2007-12-04  0:23                             ` Mark Lord
  2007-12-04  0:35                               ` Hein-Pieter van Braam
                                                 ` (2 more replies)
  2007-12-04 19:21                             ` Hein-Pieter van Braam
  1 sibling, 3 replies; 54+ messages in thread
From: Mark Lord @ 2007-12-04  0:23 UTC (permalink / raw)
  To: Hein-Pieter van Braam
  Cc: Alan Cox, Morrison, Tom, IDE/ATA development list, Jeff Garzik,
	Tejun Heo, Alan Cox

Hein-Pieter van Braam wrote:
..
> 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?
..

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

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] sata_mv:  Fix broken Marvell 7042 support.
  2007-12-04  0:23                             ` Mark Lord
@ 2007-12-04  0:35                               ` Hein-Pieter van Braam
  2007-12-04  0:36                               ` Mark Lord
  2007-12-04 23:56                               ` Hein-Pieter van Braam
  2 siblings, 0 replies; 54+ messages in thread
From: Hein-Pieter van Braam @ 2007-12-04  0:35 UTC (permalink / raw)
  To: Mark Lord
  Cc: Alan Cox, Morrison, Tom, IDE/ATA development list, Jeff Garzik,
	Tejun Heo, Alan Cox


> 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.
> 

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... 

> 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.
> 

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

> Ugh.
> 
> Cheers


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


^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] sata_mv:  Fix broken Marvell 7042 support.
  2007-12-04  0:23                             ` Mark Lord
  2007-12-04  0:35                               ` Hein-Pieter van Braam
@ 2007-12-04  0:36                               ` Mark Lord
  2007-12-04 23:56                               ` Hein-Pieter van Braam
  2 siblings, 0 replies; 54+ messages in thread
From: Mark Lord @ 2007-12-04  0:36 UTC (permalink / raw)
  To: Hein-Pieter van Braam
  Cc: Alan Cox, Morrison, Tom, IDE/ATA development list, Jeff Garzik,
	Tejun Heo, Alan Cox

Mark Lord wrote:
> Hein-Pieter van Braam wrote:
> ..
>> 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?
> ..
> 
> 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.
..

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 ??

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] sata_mv:  Fix broken Marvell 7042 support.
  2007-12-03 18:30         ` [PATCH] sata_mv: Fix broken Marvell 7042 support Mark Lord
  2007-12-03 20:11           ` Hein-Pieter van Braam
@ 2007-12-04  1:17           ` Mark Lord
  1 sibling, 0 replies; 54+ messages in thread
From: Mark Lord @ 2007-12-04  1:17 UTC (permalink / raw)
  To: hp
  Cc: Morrison, Tom, IDE/ATA development list, Jeff Garzik, Tejun Heo,
	Alan Cox

Mark Lord wrote:
>
> 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.
..

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

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] sata_mv:  Warn about Highpoint RocketRAID BIOS treatment of "Legacy" drives
  2007-12-04  0:20                           ` [PATCH] sata_mv: Warn about Highpoint RocketRAID BIOS treatment of "Legacy" drives Mark Lord
@ 2007-12-04 19:09                             ` Jeff Garzik
  0 siblings, 0 replies; 54+ messages in thread
From: Jeff Garzik @ 2007-12-04 19:09 UTC (permalink / raw)
  To: Mark Lord
  Cc: Alan Cox, Morrison, Tom, hp, IDE/ATA development list, Tejun Heo,
	Alan Cox

[-- Attachment #1: Type: text/plain, Size: 635 bytes --]

Mark Lord wrote:
> 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.
> 
> Signed-off-by: Mark Lord <mlord@pobox.com>

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



[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 1653 bytes --]

commit 306b30f74d37f289033c696285e07ce0158a5d7b
Author: Mark Lord <mlord@pobox.com>
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.
    
    Signed-off-by: Mark Lord <mlord@pobox.com>
    Signed-off-by: Jeff Garzik <jgarzik@redhat.com>

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

306b30f74d37f289033c696285e07ce0158a5d7b
diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
index 8d864e5..fe0105d 100644
--- a/drivers/ata/sata_mv.c
+++ b/drivers/ata/sata_mv.c
@@ -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;

^ permalink raw reply related	[flat|nested] 54+ messages in thread

* Re: [PATCH] sata_mv:  Fix broken Marvell 7042 support.
  2007-12-04  0:17                           ` Hein-Pieter van Braam
  2007-12-04  0:23                             ` Mark Lord
@ 2007-12-04 19:21                             ` Hein-Pieter van Braam
  1 sibling, 0 replies; 54+ messages in thread
From: Hein-Pieter van Braam @ 2007-12-04 19:21 UTC (permalink / raw)
  To: Mark Lord
  Cc: Alan Cox, Morrison, Tom, IDE/ATA development list, Jeff Garzik,
	Tejun Heo, Alan Cox


On Tue, 2007-12-04 at 01:17 +0100, Hein-Pieter van Braam wrote:
> I'll come up with the hdparm -g output and the results of this test with
> the two different drivers tomorrow. 

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.


^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] sata_mv:  Fix broken Marvell 7042 support.
  2007-12-04  0:23                             ` Mark Lord
  2007-12-04  0:35                               ` Hein-Pieter van Braam
  2007-12-04  0:36                               ` Mark Lord
@ 2007-12-04 23:56                               ` Hein-Pieter van Braam
  2007-12-05 22:45                                 ` Mark Lord
  2 siblings, 1 reply; 54+ messages in thread
From: Hein-Pieter van Braam @ 2007-12-04 23:56 UTC (permalink / raw)
  To: Mark Lord
  Cc: Alan Cox, Morrison, Tom, IDE/ATA development list, Jeff Garzik,
	Tejun Heo, Alan Cox

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.


^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] sata_mv:  Fix broken Marvell 7042 support.
  2007-12-04 23:56                               ` Hein-Pieter van Braam
@ 2007-12-05 22:45                                 ` Mark Lord
  2007-12-05 23:22                                   ` Mark Lord
  0 siblings, 1 reply; 54+ messages in thread
From: Mark Lord @ 2007-12-05 22:45 UTC (permalink / raw)
  To: Hein-Pieter van Braam
  Cc: Alan Cox, Morrison, Tom, IDE/ATA development list, Jeff Garzik,
	Tejun Heo, Alan Cox

Hein-Pieter van Braam wrote:
> 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.
..

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

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] sata_mv:  Fix broken Marvell 7042 support.
  2007-12-05 22:45                                 ` Mark Lord
@ 2007-12-05 23:22                                   ` Mark Lord
  2007-12-05 23:35                                     ` Mark Lord
  0 siblings, 1 reply; 54+ messages in thread
From: Mark Lord @ 2007-12-05 23:22 UTC (permalink / raw)
  To: Hein-Pieter van Braam
  Cc: Alan Cox, Morrison, Tom, IDE/ATA development list, Jeff Garzik,
	Tejun Heo, Alan Cox

Mark Lord wrote:
> Hein-Pieter van Braam wrote:
>> 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.
> ..
> 
> 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.
..

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

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] sata_mv:  Fix broken Marvell 7042 support.
  2007-12-05 23:22                                   ` Mark Lord
@ 2007-12-05 23:35                                     ` Mark Lord
  2007-12-05 23:55                                       ` Mark Lord
  0 siblings, 1 reply; 54+ messages in thread
From: Mark Lord @ 2007-12-05 23:35 UTC (permalink / raw)
  To: Hein-Pieter van Braam
  Cc: Alan Cox, Morrison, Tom, IDE/ATA development list, Jeff Garzik,
	Tejun Heo, Alan Cox

Mark Lord wrote:
> Mark Lord wrote:
>> Hein-Pieter van Braam wrote:
>>> 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.
>> ..
>>
>> 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.
> ..
> 
> 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.
..

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

> 
> 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.

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] sata_mv:  Fix broken Marvell 7042 support.
  2007-12-05 23:35                                     ` Mark Lord
@ 2007-12-05 23:55                                       ` Mark Lord
  2007-12-06  0:02                                         ` Jeff Garzik
  0 siblings, 1 reply; 54+ messages in thread
From: Mark Lord @ 2007-12-05 23:55 UTC (permalink / raw)
  To: Jeff Garzik
  Cc: Hein-Pieter van Braam, Alan Cox, Morrison, Tom,
	IDE/ATA development list, Tejun Heo, Alan Cox

Mark Lord wrote:
>...
>>> Yeah, that's my suspicion too.. they must have just simply moved
>>> the metadata to near the end instead of sector-8.
...
>> 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.
> ..
> 
> 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 (?).

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

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] sata_mv:  Fix broken Marvell 7042 support.
  2007-12-05 23:55                                       ` Mark Lord
@ 2007-12-06  0:02                                         ` Jeff Garzik
  2007-12-06  3:57                                           ` Mark Lord
                                                             ` (2 more replies)
  0 siblings, 3 replies; 54+ messages in thread
From: Jeff Garzik @ 2007-12-06  0:02 UTC (permalink / raw)
  To: Mark Lord
  Cc: Hein-Pieter van Braam, Alan Cox, Morrison, Tom,
	IDE/ATA development list, Tejun Heo, Alan Cox

Mark Lord wrote:
> 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.

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



^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] sata_mv:  Fix broken Marvell 7042 support.
  2007-12-06  0:02                                         ` Jeff Garzik
@ 2007-12-06  3:57                                           ` Mark Lord
  2007-12-06  4:45                                             ` Jeff Garzik
  2007-12-06  4:03                                           ` Mark Lord
  2007-12-06 22:32                                           ` Mark Lord
  2 siblings, 1 reply; 54+ messages in thread
From: Mark Lord @ 2007-12-06  3:57 UTC (permalink / raw)
  To: Jeff Garzik
  Cc: Hein-Pieter van Braam, Alan Cox, Morrison, Tom,
	IDE/ATA development list, Tejun Heo, Alan Cox

Jeff Garzik wrote:
> Mark Lord wrote:
>> 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.
> 
> 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].
..

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

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] sata_mv:  Fix broken Marvell 7042 support.
  2007-12-06  0:02                                         ` Jeff Garzik
  2007-12-06  3:57                                           ` Mark Lord
@ 2007-12-06  4:03                                           ` Mark Lord
  2007-12-06  4:43                                             ` Jeff Garzik
  2007-12-06 22:32                                           ` Mark Lord
  2 siblings, 1 reply; 54+ messages in thread
From: Mark Lord @ 2007-12-06  4:03 UTC (permalink / raw)
  To: Jeff Garzik
  Cc: Hein-Pieter van Braam, Alan Cox, Morrison, Tom,
	IDE/ATA development list, Tejun Heo, Alan Cox

Jeff Garzik wrote:
> Mark Lord wrote:
>> 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.
> 
> Definitely _not_.  This is a core Linux maxim:  export what the hardware 
> exports, no more no less.  We drive the "bare metal."
..

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

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] sata_mv:  Fix broken Marvell 7042 support.
  2007-12-06  4:03                                           ` Mark Lord
@ 2007-12-06  4:43                                             ` Jeff Garzik
  2007-12-06 22:23                                               ` Mark Lord
  0 siblings, 1 reply; 54+ messages in thread
From: Jeff Garzik @ 2007-12-06  4:43 UTC (permalink / raw)
  To: Mark Lord
  Cc: Hein-Pieter van Braam, Alan Cox, Morrison, Tom,
	IDE/ATA development list, Tejun Heo, Alan Cox

Mark Lord wrote:
> Jeff Garzik wrote:
>> Mark Lord wrote:
>>> 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.
>>
>> Definitely _not_.  This is a core Linux maxim:  export what the 
>> hardware exports, no more no less.  We drive the "bare metal."
> ..
> 
> The hardware limitation here is the SATA controller card:
> it corrupts data at the last GB boundary.

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




^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] sata_mv:  Fix broken Marvell 7042 support.
  2007-12-06  3:57                                           ` Mark Lord
@ 2007-12-06  4:45                                             ` Jeff Garzik
  2007-12-06 22:24                                               ` Mark Lord
  0 siblings, 1 reply; 54+ messages in thread
From: Jeff Garzik @ 2007-12-06  4:45 UTC (permalink / raw)
  To: Mark Lord
  Cc: Hein-Pieter van Braam, Alan Cox, Morrison, Tom,
	IDE/ATA development list, Tejun Heo, Alan Cox

Mark Lord wrote:
> Jeff Garzik wrote:
>> Mark Lord wrote:
>>> 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.
>>
>> 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].
> ..
> 
> 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.

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




^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] sata_mv:  Fix broken Marvell 7042 support.
  2007-12-06  4:43                                             ` Jeff Garzik
@ 2007-12-06 22:23                                               ` Mark Lord
  2007-12-07  2:22                                                 ` Jeff Garzik
  0 siblings, 1 reply; 54+ messages in thread
From: Mark Lord @ 2007-12-06 22:23 UTC (permalink / raw)
  To: Jeff Garzik
  Cc: Hein-Pieter van Braam, Alan Cox, Morrison, Tom,
	IDE/ATA development list, Tejun Heo, Alan Cox

Jeff Garzik wrote:
...
> 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.
..

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


^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] sata_mv:  Fix broken Marvell 7042 support.
  2007-12-06  4:45                                             ` Jeff Garzik
@ 2007-12-06 22:24                                               ` Mark Lord
  0 siblings, 0 replies; 54+ messages in thread
From: Mark Lord @ 2007-12-06 22:24 UTC (permalink / raw)
  To: Jeff Garzik
  Cc: Hein-Pieter van Braam, Alan Cox, Morrison, Tom,
	IDE/ATA development list, Tejun Heo, Alan Cox

Jeff Garzik wrote:
>..
> 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...
..

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

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] sata_mv:  Fix broken Marvell 7042 support.
  2007-12-06  0:02                                         ` Jeff Garzik
  2007-12-06  3:57                                           ` Mark Lord
  2007-12-06  4:03                                           ` Mark Lord
@ 2007-12-06 22:32                                           ` Mark Lord
  2 siblings, 0 replies; 54+ messages in thread
From: Mark Lord @ 2007-12-06 22:32 UTC (permalink / raw)
  To: Jeff Garzik
  Cc: Hein-Pieter van Braam, Alan Cox, Morrison, Tom,
	IDE/ATA development list, Tejun Heo, Alan Cox

Jeff Garzik wrote:
..
> 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].
..

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

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] sata_mv:  Fix broken Marvell 7042 support.
  2007-12-06 22:23                                               ` Mark Lord
@ 2007-12-07  2:22                                                 ` Jeff Garzik
  0 siblings, 0 replies; 54+ messages in thread
From: Jeff Garzik @ 2007-12-07  2:22 UTC (permalink / raw)
  To: Mark Lord
  Cc: Hein-Pieter van Braam, Alan Cox, Morrison, Tom,
	IDE/ATA development list, Tejun Heo, Alan Cox

Mark Lord wrote:
> Jeff Garzik wrote:
> ...
>> 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.
> ..

> 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.

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...

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

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



^ permalink raw reply	[flat|nested] 54+ messages in thread

end of thread, other threads:[~2007-12-07  2:22 UTC | newest]

Thread overview: 54+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-12-01 18:07 [PATCH] sata_mv: Fix broken Marvell 7042 support Mark Lord
2007-12-01 18:16 ` Alan Cox
2007-12-01 22:45 ` Jeff Garzik
2007-12-03 12:27 ` Morrison, Tom
2007-12-03 14:47   ` hp
2007-12-03 14:56     ` Morrison, Tom
2007-12-03 17:26     ` Mark Lord
2007-12-03 18:14       ` Mark Lord
2007-12-03 18:30         ` Jeff Garzik
2007-12-03 18:32           ` Mark Lord
2007-12-03 18:37             ` Morrison, Tom
2007-12-03 18:40               ` Mark Lord
2007-12-03 18:44                 ` Mark Lord
2007-12-03 18:42                   ` Alan Cox
2007-12-03 19:12                     ` Mark Lord
2007-12-03 20:40                       ` Mark Lord
2007-12-03 23:59                         ` Mark Lord
2007-12-04  0:20                           ` [PATCH] sata_mv: Warn about Highpoint RocketRAID BIOS treatment of "Legacy" drives Mark Lord
2007-12-04 19:09                             ` Jeff Garzik
2007-12-03 18:30         ` [PATCH] sata_mv: Fix broken Marvell 7042 support Mark Lord
2007-12-03 20:11           ` Hein-Pieter van Braam
2007-12-03 20:24             ` Mark Lord
2007-12-03 20:37               ` Hein-Pieter van Braam
2007-12-03 20:54                 ` Mark Lord
2007-12-03 22:28                   ` Hein-Pieter van Braam
2007-12-03 23:37                     ` Mark Lord
2007-12-03 22:48                   ` Hein-Pieter van Braam
2007-12-03 23:10                     ` Alan Cox
2007-12-03 23:33                       ` Mark Lord
2007-12-03 23:34                         ` Alan Cox
2007-12-03 23:47                       ` Mark Lord
2007-12-03 23:47                         ` Alan Cox
2007-12-04  0:01                       ` Hein-Pieter van Braam
2007-12-04  0:07                         ` Mark Lord
2007-12-04  0:17                           ` Hein-Pieter van Braam
2007-12-04  0:23                             ` Mark Lord
2007-12-04  0:35                               ` Hein-Pieter van Braam
2007-12-04  0:36                               ` Mark Lord
2007-12-04 23:56                               ` Hein-Pieter van Braam
2007-12-05 22:45                                 ` Mark Lord
2007-12-05 23:22                                   ` Mark Lord
2007-12-05 23:35                                     ` Mark Lord
2007-12-05 23:55                                       ` Mark Lord
2007-12-06  0:02                                         ` Jeff Garzik
2007-12-06  3:57                                           ` Mark Lord
2007-12-06  4:45                                             ` Jeff Garzik
2007-12-06 22:24                                               ` Mark Lord
2007-12-06  4:03                                           ` Mark Lord
2007-12-06  4:43                                             ` Jeff Garzik
2007-12-06 22:23                                               ` Mark Lord
2007-12-07  2:22                                                 ` Jeff Garzik
2007-12-06 22:32                                           ` Mark Lord
2007-12-04 19:21                             ` Hein-Pieter van Braam
2007-12-04  1:17           ` Mark Lord

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.