[PATCH] pci_try_set_mwi() in sata

linux-ide.vger.kernel.org archive mirror
 help / color / mirror / Atom feed

* [PATCH] pci_try_set_mwi() in sata_promise
@ 2008-04-22 18:26 Marin Mitov
  2008-04-23  7:57 ` Mikael Pettersson
  0 siblings, 1 reply; 7+ messages in thread
From: Marin Mitov @ 2008-04-22 18:26 UTC (permalink / raw)
  To: linux-kernel; +Cc: Jeff Garzik, Mikael Pettersson, linux-ide

Hi all,

The BIOS (Asus A8V Deluxe) is setting incorrectly PCI
Cache Line Size Register (as seen in lspci -vvv output), 
so try to correct it by pci_try_set_mwi(pdev).

Marin Mitov
----------------------------------------------------------
Enable Memory-Write-and-Invalidate in sata_promise driver and 
get rid of strange BIOS-set value for cache line size register.

According to Documentation/pci.txt:  "This enables...
...and also ensures that the cache line size register
is set correctly".

Signed-off-by: Marin Mitov <mitov@issp.bas.bg>
----------------------------------------------------------
--- a/drivers/ata/sata_promise.c	2008-04-22 13:09:15.000000000 +0300
+++ b/drivers/ata/sata_promise.c	2008-04-22 13:11:01.000000000 +0300
@@ -1114,6 +1114,7 @@ static int pdc_ata_init_one(struct pci_d

 	/* start host, request IRQ and attach */
 	pci_set_master(pdev);
+	pci_try_set_mwi(pdev);
 	return ata_host_activate(host, pdev->irq, pdc_interrupt, IRQF_SHARED,
 				 &pdc_ata_sht);
 }

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

* Re: [PATCH] pci_try_set_mwi() in sata_promise
  2008-04-22 18:26 [PATCH] pci_try_set_mwi() in sata_promise Marin Mitov
@ 2008-04-23  7:57 ` Mikael Pettersson
  2008-04-23 12:31   ` Marin Mitov
                     ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Mikael Pettersson @ 2008-04-23  7:57 UTC (permalink / raw)
  To: Marin Mitov; +Cc: linux-kernel, Jeff Garzik, Mikael Pettersson, linux-ide

Marin Mitov writes:
 > Hi all,
 > 
 > The BIOS (Asus A8V Deluxe) is setting incorrectly PCI
 > Cache Line Size Register (as seen in lspci -vvv output), 
 > so try to correct it by pci_try_set_mwi(pdev).
 > 
 > Marin Mitov
 > ----------------------------------------------------------
 > Enable Memory-Write-and-Invalidate in sata_promise driver and 
 > get rid of strange BIOS-set value for cache line size register.
 > 
 > According to Documentation/pci.txt:  "This enables...
 > ...and also ensures that the cache line size register
 > is set correctly".
 > 
 > Signed-off-by: Marin Mitov <mitov@issp.bas.bg>
 > ----------------------------------------------------------
 > --- a/drivers/ata/sata_promise.c	2008-04-22 13:09:15.000000000 +0300
 > +++ b/drivers/ata/sata_promise.c	2008-04-22 13:11:01.000000000 +0300
 > @@ -1114,6 +1114,7 @@ static int pdc_ata_init_one(struct pci_d
 > 
 >  	/* start host, request IRQ and attach */
 >  	pci_set_master(pdev);
 > +	pci_try_set_mwi(pdev);
 >  	return ata_host_activate(host, pdev->irq, pdc_interrupt, IRQF_SHARED,
 >  				 &pdc_ata_sht);
 >  }
 > 

Not enough information here.

0. Please post lspci -vvvx including all PCI devices.
1. What consequence does the incorrect cache line size setting have?
2. What improvement does that pci_try_set_mwi() cause? Speed? Fewer errors?
3. Why call pci_try_set_mwi()? Can't you set the cache line size directly?
4. You write "try to correct it". So the "fix" might not work? Then what?
5. Is the problem specific for the Promise chip? That is:
   a) are any other built-in PCI devices affected by this BIOS bug?
   b) if you add a PCI card (of any kind), does it also
      get the incorrect cache line size setting?
6. Are you running the latest BIOS?

To me it sounds like this is a generic PCI bug on that
machine, and so should be handled by the kernel's PCI layer.
But if the problem really is specific to the built-in Promise
chip on that machine, then I can see why a workaround in
sata_promise could be appropriate.

/Mikael

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

* Re: [PATCH] pci_try_set_mwi() in sata_promise
  2008-04-23  7:57 ` Mikael Pettersson
@ 2008-04-23 12:31   ` Marin Mitov
  2008-04-23 12:34   ` Marin Mitov
  2008-04-23 13:33   ` Marin Mitov
  2 siblings, 0 replies; 7+ messages in thread
From: Marin Mitov @ 2008-04-23 12:31 UTC (permalink / raw)
  To: Mikael Pettersson; +Cc: linux-kernel, Jeff Garzik, linux-ide

On 23.4.2008, Mikael Pettersson wrote:
> Marin Mitov writes:
>  > Hi all,
>  > 
>  > The BIOS (Asus A8V Deluxe) is setting incorrectly PCI
>  > Cache Line Size Register (as seen in lspci -vvv output), 
>  > so try to correct it by pci_try_set_mwi(pdev).
>  > 
>  > Marin Mitov
>  > ----------------------------------------------------------
>  > Enable Memory-Write-and-Invalidate in sata_promise driver and 
>  > get rid of strange BIOS-set value for cache line size register.
>  > 
>  > According to Documentation/pci.txt:  "This enables...
>  > ...and also ensures that the cache line size register
>  > is set correctly".
>  > 
>  > Signed-off-by: Marin Mitov <mitov@issp.bas.bg>
>  > ----------------------------------------------------------
>  > --- a/drivers/ata/sata_promise.c	2008-04-22 13:09:15.000000000 +0300
>  > +++ b/drivers/ata/sata_promise.c	2008-04-22 13:11:01.000000000 +0300
>  > @@ -1114,6 +1114,7 @@ static int pdc_ata_init_one(struct pci_d
>  > 
>  >  	/* start host, request IRQ and attach */
>  >  	pci_set_master(pdev);
>  > +	pci_try_set_mwi(pdev);
>  >  	return ata_host_activate(host, pdev->irq, pdc_interrupt, IRQF_SHARED,
>  >  				 &pdc_ata_sht);
>  >  }
>  > 
> 
> Not enough information here.
> 
> 0. Please post lspci -vvvx including all PCI devices.
Here it is:


> 1. What consequence does the incorrect cache line size setting have?

None if MWI is not enabled.

> 2. What improvement does that pci_try_set_mwi() cause? Speed? Fewer errors?

May be speed, but practicaly hardly observable. It is described in PCI specs. 
If the hardware has this property why not using it? In any case that does not hurt.

> 3. Why call pci_try_set_mwi()? Can't you set the cache line size directly?

pci_set_cacheline_size() is NOT exported, while pci_try_set_mwi() is and
sets (as a side effect) the cache line size.

> 4. You write "try to correct it". So the "fix" might not work? Then what?

It fails only if cache line size cannot be set. Then MWI is not enabled.
The device continues to work (nicely) as it did up to now.

> 5. Is the problem specific for the Promise chip? That is:
>    a) are any other built-in PCI devices affected by this BIOS bug?
>    b) if you add a PCI card (of any kind), does it also
>       get the incorrect cache line size setting?
> 6. Are you running the latest BIOS?
> 
> To me it sounds like this is a generic PCI bug on that
> machine, and so should be handled by the kernel's PCI layer.
> But if the problem really is specific to the built-in Promise
> chip on that machine, then I can see why a workaround in
> sata_promise could be appropriate.
> 
> /Mikael
> 



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

* Re: [PATCH] pci_try_set_mwi() in sata_promise
  2008-04-23  7:57 ` Mikael Pettersson
  2008-04-23 12:31   ` Marin Mitov
@ 2008-04-23 12:34   ` Marin Mitov
  2008-04-23 13:33   ` Marin Mitov
  2 siblings, 0 replies; 7+ messages in thread
From: Marin Mitov @ 2008-04-23 12:34 UTC (permalink / raw)
  To: Mikael Pettersson; +Cc: linux-kernel, Jeff Garzik, linux-ide

Please ignore my previous posting it is not finishet yet.
Sorry for the noise

Marin Mitov

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

* Re: [PATCH] pci_try_set_mwi() in sata_promise
  2008-04-23  7:57 ` Mikael Pettersson
  2008-04-23 12:31   ` Marin Mitov
  2008-04-23 12:34   ` Marin Mitov
@ 2008-04-23 13:33   ` Marin Mitov
  2008-04-23 14:46     ` Grant Grundler
  2 siblings, 1 reply; 7+ messages in thread
From: Marin Mitov @ 2008-04-23 13:33 UTC (permalink / raw)
  To: Mikael Pettersson; +Cc: linux-kernel, Jeff Garzik, linux-ide

On 23.4.2008, Mikael Pettersson wrote:
> Marin Mitov writes:
>  > Hi all,
>  > 
>  > The BIOS (Asus A8V Deluxe) is setting incorrectly PCI
>  > Cache Line Size Register (as seen in lspci -vvv output), 
>  > so try to correct it by pci_try_set_mwi(pdev).
>  > 
>  > Marin Mitov
>  > ----------------------------------------------------------
>  > Enable Memory-Write-and-Invalidate in sata_promise driver and 
>  > get rid of strange BIOS-set value for cache line size register.
>  > 
>  > According to Documentation/pci.txt:  "This enables...
>  > ...and also ensures that the cache line size register
>  > is set correctly".
>  > 
>  > Signed-off-by: Marin Mitov <mitov@issp.bas.bg>
>  > ----------------------------------------------------------
>  > --- a/drivers/ata/sata_promise.c	2008-04-22 13:09:15.000000000 +0300
>  > +++ b/drivers/ata/sata_promise.c	2008-04-22 13:11:01.000000000 +0300
>  > @@ -1114,6 +1114,7 @@ static int pdc_ata_init_one(struct pci_d
>  > 
>  >  	/* start host, request IRQ and attach */
>  >  	pci_set_master(pdev);
>  > +	pci_try_set_mwi(pdev);
>  >  	return ata_host_activate(host, pdev->irq, pdc_interrupt, IRQF_SHARED,
>  >  				 &pdc_ata_sht);
>  >  }
>  > 
> 
> Not enough information here.

Sorry.

> 
> 0. Please post lspci -vvvx including all PCI devices.
Here it is:
00:00.0 Host bridge: VIA Technologies, Inc. K8T800Pro Host Bridge
	Subsystem: ASUSTeK Computer Inc. A8V Deluxe
	Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR+ FastB2B-
	Status: Cap+ 66MHz+ UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort+ >SERR- <PERR-
	Latency: 64
	Region 0: Memory at e0000000 (32-bit, prefetchable) [size=256M]
	Capabilities: [80] AGP version 3.0
		Status: RQ=32 Iso- ArqSz=0 Cal=0 SBA+ ITACoh- GART64- HTrans- 64bit- FW+ AGP3- Rate=x1,x2,x4
		Command: RQ=1 ArqSz=0 Cal=0 SBA+ AGP+ GART64- 64bit- FW- Rate=x4
	Capabilities: [50] Power Management version 2
		Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA PME(D0-,D1-,D2-,D3hot-,D3cold-)
		Status: D0 PME-Enable- DSel=0 DScale=0 PME-
	Capabilities: [60] HyperTransport: Slave or Primary Interface
		!!! Possibly incomplete decoding
		Command: BaseUnitID=0 UnitCnt=3 MastHost- DefDir-
		Link Control 0: CFlE- CST- CFE- <LkFail- Init+ EOC- TXO- <CRCErr=0
		Link Config 0: MLWI=16bit MLWO=16bit LWI=16bit LWO=16bit
		Link Control 1: CFlE- CST- CFE- <LkFail+ Init- EOC+ TXO+ <CRCErr=0
		Link Config 1: MLWI=8bit MLWO=8bit LWI=8bit LWO=8bit
		Revision ID: 1.02
	Capabilities: [58] HyperTransport: Interrupt Discovery and Configuration
00: 06 11 82 02 06 01 30 22 00 00 00 06 00 40 80 00
10: 08 00 00 e0 00 00 00 00 00 00 00 00 00 00 00 00
20: 00 00 00 00 00 00 00 00 00 00 00 00 43 10 a3 80
30: 00 00 00 00 80 00 00 00 00 00 00 00 00 00 00 00

00:00.1 Host bridge: VIA Technologies, Inc. K8T800Pro Host Bridge
	Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B-
	Status: Cap- 66MHz- UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR-
	Latency: 0
00: 06 11 82 12 06 00 00 02 00 00 00 06 00 00 00 00
10: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
20: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
30: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00

00:00.2 Host bridge: VIA Technologies, Inc. K8T800Pro Host Bridge
	Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B-
	Status: Cap- 66MHz- UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR-
	Latency: 0
00: 06 11 82 22 06 00 00 02 00 00 00 06 00 00 00 00
10: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
20: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
30: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00

00:00.3 Host bridge: VIA Technologies, Inc. K8T800Pro Host Bridge
	Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B-
	Status: Cap- 66MHz- UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR-
	Latency: 0
00: 06 11 82 32 06 00 00 02 00 00 00 06 00 00 00 00
10: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
20: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
30: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00

00:00.4 Host bridge: VIA Technologies, Inc. K8T800Pro Host Bridge
	Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B-
	Status: Cap- 66MHz- UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR-
	Latency: 0
00: 06 11 82 42 06 00 00 02 00 00 00 06 00 00 00 00
10: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
20: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
30: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00

00:00.7 Host bridge: VIA Technologies, Inc. K8T800Pro Host Bridge
	Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B-
	Status: Cap- 66MHz- UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR-
	Latency: 0
00: 06 11 82 72 06 00 00 02 00 00 00 06 00 00 00 00
10: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
20: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
30: 00 00 00 00 a0 00 00 00 00 00 00 00 00 00 00 00

00:01.0 PCI bridge: VIA Technologies, Inc. VT8237 PCI bridge [K8T800/K8T890 South] (prog-if 00 [Normal decode])
	Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR+ FastB2B-
	Status: Cap+ 66MHz+ UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR-
	Latency: 0
	Bus: primary=00, secondary=01, subordinate=01, sec-latency=0
	I/O behind bridge: 0000f000-00000fff
	Memory behind bridge: fb000000-fbffffff
	Prefetchable memory behind bridge: f8000000-f9ffffff
	Secondary status: 66MHz+ FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort+ <SERR- <PERR-
	BridgeCtl: Parity- SERR+ NoISA- VGA+ MAbort- >Reset- FastB2B-
	Capabilities: [80] Power Management version 2
		Flags: PMEClk- DSI- D1+ D2- AuxCurrent=0mA PME(D0-,D1-,D2-,D3hot-,D3cold-)
		Status: D0 PME-Enable- DSel=0 DScale=0 PME-
00: 06 11 88 b1 07 01 30 02 00 00 04 06 00 00 01 00
10: 00 00 00 00 00 00 00 00 00 01 01 00 f0 00 20 22
20: 00 fb f0 fb 00 f8 f0 f9 00 00 00 00 00 00 00 00
30: 00 00 00 00 80 00 00 00 00 00 00 00 00 00 0a 00

00:08.0 RAID bus controller: Promise Technology, Inc. PDC20378 (FastTrak 378/SATA 378) (rev 02)
	Subsystem: ASUSTeK Computer Inc. K8V Deluxe/PC-DL Deluxe motherboard
	Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV+ VGASnoop- ParErr- Stepping- SERR+ FastB2B-
	Status: Cap+ 66MHz+ UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR-
	Latency: 96 (1000ns min, 4500ns max), Cache Line Size: 580 bytes
	Interrupt: pin A routed to IRQ 19
	Region 0: I/O ports at a800 [size=64]
	Region 1: I/O ports at a400 [size=16]
	Region 2: I/O ports at a000 [size=128]
	Region 3: Memory at faa00000 (32-bit, non-prefetchable) [size=4K]
	Region 4: Memory at fa900000 (32-bit, non-prefetchable) [size=128K]
	Capabilities: [60] Power Management version 2
		Flags: PMEClk- DSI+ D1+ D2- AuxCurrent=0mA PME(D0-,D1-,D2-,D3hot-,D3cold-)
		Status: D0 PME-Enable- DSel=0 DScale=0 PME-
00: 5a 10 73 33 17 01 30 02 02 00 04 01 91 60 00 00
10: 01 a8 00 00 01 a4 00 00 01 a0 00 00 00 00 a0 fa
20: 00 00 90 fa 00 00 00 00 00 00 00 00 43 10 f5 80
30: 00 00 00 00 60 00 00 00 00 00 00 00 05 01 04 12

00:0a.0 Ethernet controller: Marvell Technology Group Ltd. 88E8001 Gigabit Ethernet Controller (rev 13)
	Subsystem: ASUSTeK Computer Inc. Marvell 88E8001 Gigabit Ethernet Controller (Asus)
	Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV+ VGASnoop- ParErr- Stepping- SERR+ FastB2B-
	Status: Cap+ 66MHz+ UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR-
	Latency: 64 (5750ns min, 7750ns max), Cache Line Size: 64 bytes
	Interrupt: pin A routed to IRQ 17
	Region 0: Memory at fac00000 (32-bit, non-prefetchable) [size=16K]
	Region 1: I/O ports at b000 [size=256]
	Expansion ROM at fab00000 [disabled] [size=128K]
	Capabilities: [48] Power Management version 2
		Flags: PMEClk- DSI- D1+ D2+ AuxCurrent=0mA PME(D0+,D1+,D2+,D3hot+,D3cold+)
		Status: D0 PME-Enable- DSel=0 DScale=1 PME-
	Capabilities: [50] Vital Product Data
00: ab 11 20 43 17 01 b0 02 13 00 00 02 10 40 00 00
10: 00 00 c0 fa 01 b0 00 00 00 00 00 00 00 00 00 00
20: 00 00 00 00 00 00 00 00 00 00 00 00 43 10 1a 81
30: 00 00 b0 fa 48 00 00 00 00 00 00 00 0a 01 17 1f

00:0d.0 Multimedia video controller: Intel Corporation SAA7116
	Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B-
	Status: Cap- 66MHz- UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR-
	Latency: 64
	Interrupt: pin A routed to IRQ 19
	Region 0: Memory at fad00000 (32-bit, non-prefetchable) [size=4K]
00: 86 80 23 12 06 00 00 02 00 00 00 04 00 40 00 00
10: 00 00 d0 fa 00 00 00 00 00 00 00 00 00 00 00 00
20: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
30: 00 00 00 00 00 00 00 00 00 00 00 00 05 01 00 00

00:0f.0 RAID bus controller: VIA Technologies, Inc. VIA VT6420 SATA RAID Controller (rev 80)
	Subsystem: ASUSTeK Computer Inc. A7V600/K8V Deluxe/K8V-X/A8V Deluxe motherboard
	Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B-
	Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR-
	Latency: 64
	Interrupt: pin B routed to IRQ 18
	Region 0: I/O ports at d000 [size=8]
	Region 1: I/O ports at c800 [size=4]
	Region 2: I/O ports at c400 [size=8]
	Region 3: I/O ports at c000 [size=4]
	Region 4: I/O ports at b800 [size=16]
	Region 5: I/O ports at b400 [size=256]
	Capabilities: [c0] Power Management version 2
		Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA PME(D0-,D1-,D2-,D3hot-,D3cold-)
		Status: D0 PME-Enable- DSel=0 DScale=0 PME-
00: 06 11 49 31 07 00 90 02 80 00 04 01 00 40 80 00
10: 01 d0 00 00 01 c8 00 00 01 c4 00 00 01 c0 00 00
20: 01 b8 00 00 01 b4 00 00 00 00 00 00 43 10 ed 80
30: 00 00 00 00 c0 00 00 00 00 00 00 00 0a 02 00 00

00:0f.1 IDE interface: VIA Technologies, Inc. VT82C586A/B/VT82C686/A/B/VT823x/A/C PIPC Bus Master IDE (rev 06) (prog-if 8a [Master SecP PriP])
	Subsystem: ASUSTeK Computer Inc. A7V600/K8V-X/A8V Deluxe motherboard
	Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B-
	Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR-
	Latency: 32
	Interrupt: pin A routed to IRQ 18
	Region 0: [virtual] Memory at 000001f0 (32-bit, non-prefetchable) [size=8]
	Region 1: [virtual] Memory at 000003f0 (type 3, non-prefetchable) [size=1]
	Region 2: [virtual] Memory at 00000170 (32-bit, non-prefetchable) [size=8]
	Region 3: [virtual] Memory at 00000370 (type 3, non-prefetchable) [size=1]
	Region 4: I/O ports at fc00 [size=16]
	Capabilities: [c0] Power Management version 2
		Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA PME(D0-,D1-,D2-,D3hot-,D3cold-)
		Status: D0 PME-Enable- DSel=0 DScale=0 PME-
00: 06 11 71 05 07 00 90 02 06 8a 01 01 00 20 00 00
10: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
20: 01 fc 00 00 00 00 00 00 00 00 00 00 43 10 ed 80
30: 00 00 00 00 c0 00 00 00 00 00 00 00 ff 01 00 00

00:10.0 USB Controller: VIA Technologies, Inc. VT82xxxxx UHCI USB 1.1 Controller (rev 81) (prog-if 00 [UHCI])
	Subsystem: ASUSTeK Computer Inc. A7V600/K8V-X/A8V Deluxe motherboard
	Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV+ VGASnoop- ParErr- Stepping- SERR- FastB2B-
	Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR-
	Latency: 64, Cache Line Size: 64 bytes
	Interrupt: pin A routed to IRQ 20
	Region 4: I/O ports at d400 [size=32]
	Capabilities: [80] Power Management version 2
		Flags: PMEClk- DSI- D1+ D2+ AuxCurrent=375mA PME(D0+,D1+,D2+,D3hot+,D3cold+)
		Status: D0 PME-Enable- DSel=0 DScale=0 PME-
00: 06 11 38 30 17 00 10 02 81 00 03 0c 10 40 80 00
10: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
20: 01 d4 00 00 00 00 00 00 00 00 00 00 43 10 ed 80
30: 00 00 00 00 80 00 00 00 00 00 00 00 0b 01 00 00

00:10.1 USB Controller: VIA Technologies, Inc. VT82xxxxx UHCI USB 1.1 Controller (rev 81) (prog-if 00 [UHCI])
	Subsystem: ASUSTeK Computer Inc. A7V600/K8V-X/A8V Deluxe motherboard
	Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV+ VGASnoop- ParErr- Stepping- SERR- FastB2B-
	Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR-
	Latency: 64, Cache Line Size: 64 bytes
	Interrupt: pin A routed to IRQ 20
	Region 4: I/O ports at d800 [size=32]
	Capabilities: [80] Power Management version 2
		Flags: PMEClk- DSI- D1+ D2+ AuxCurrent=375mA PME(D0+,D1+,D2+,D3hot+,D3cold+)
		Status: D0 PME-Enable- DSel=0 DScale=0 PME-
00: 06 11 38 30 17 00 10 02 81 00 03 0c 10 40 80 00
10: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
20: 01 d8 00 00 00 00 00 00 00 00 00 00 43 10 ed 80
30: 00 00 00 00 80 00 00 00 00 00 00 00 0b 01 00 00

00:10.2 USB Controller: VIA Technologies, Inc. VT82xxxxx UHCI USB 1.1 Controller (rev 81) (prog-if 00 [UHCI])
	Subsystem: ASUSTeK Computer Inc. A7V600/K8V-X/A8V Deluxe motherboard
	Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV+ VGASnoop- ParErr- Stepping- SERR- FastB2B-
	Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR-
	Latency: 64, Cache Line Size: 64 bytes
	Interrupt: pin B routed to IRQ 20
	Region 4: I/O ports at e000 [size=32]
	Capabilities: [80] Power Management version 2
		Flags: PMEClk- DSI- D1+ D2+ AuxCurrent=375mA PME(D0+,D1+,D2+,D3hot+,D3cold+)
		Status: D0 PME-Enable- DSel=0 DScale=0 PME-
00: 06 11 38 30 17 00 10 02 81 00 03 0c 10 40 80 00
10: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
20: 01 e0 00 00 00 00 00 00 00 00 00 00 43 10 ed 80
30: 00 00 00 00 80 00 00 00 00 00 00 00 0a 02 00 00

00:10.3 USB Controller: VIA Technologies, Inc. VT82xxxxx UHCI USB 1.1 Controller (rev 81) (prog-if 00 [UHCI])
	Subsystem: ASUSTeK Computer Inc. A7V600/K8V-X/A8V Deluxe motherboard
	Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV+ VGASnoop- ParErr- Stepping- SERR- FastB2B-
	Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR-
	Latency: 64, Cache Line Size: 64 bytes
	Interrupt: pin B routed to IRQ 20
	Region 4: I/O ports at e400 [size=32]
	Capabilities: [80] Power Management version 2
		Flags: PMEClk- DSI- D1+ D2+ AuxCurrent=375mA PME(D0+,D1+,D2+,D3hot+,D3cold+)
		Status: D0 PME-Enable- DSel=0 DScale=0 PME-
00: 06 11 38 30 17 00 10 02 81 00 03 0c 10 40 80 00
10: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
20: 01 e4 00 00 00 00 00 00 00 00 00 00 43 10 ed 80
30: 00 00 00 00 80 00 00 00 00 00 00 00 0a 02 00 00

00:10.4 USB Controller: VIA Technologies, Inc. USB 2.0 (rev 86) (prog-if 20 [EHCI])
	Subsystem: ASUSTeK Computer Inc. A7V600/K8V-X/A8V Deluxe motherboard
	Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV+ VGASnoop- ParErr- Stepping- SERR- FastB2B-
	Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR-
	Latency: 64, Cache Line Size: 64 bytes
	Interrupt: pin C routed to IRQ 20
	Region 0: Memory at faf00000 (32-bit, non-prefetchable) [size=256]
	Capabilities: [80] Power Management version 2
		Flags: PMEClk- DSI- D1+ D2+ AuxCurrent=375mA PME(D0+,D1+,D2+,D3hot+,D3cold+)
		Status: D0 PME-Enable- DSel=0 DScale=0 PME-
00: 06 11 04 31 17 00 10 02 86 20 03 0c 10 40 80 00
10: 00 00 f0 fa 00 00 00 00 00 00 00 00 00 00 00 00
20: 00 00 00 00 00 00 00 00 00 00 00 00 43 10 ed 80
30: 00 00 00 00 80 00 00 00 00 00 00 00 05 03 00 00

00:11.0 ISA bridge: VIA Technologies, Inc. VT8237 ISA bridge [KT600/K8T800/K8T890 South]
	Subsystem: ASUSTeK Computer Inc. A7V600/K8V-X/A8V Deluxe motherboard
	Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping+ SERR- FastB2B-
	Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR-
	Latency: 0
	Capabilities: [c0] Power Management version 2
		Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA PME(D0-,D1-,D2-,D3hot-,D3cold-)
		Status: D0 PME-Enable- DSel=0 DScale=0 PME-
00: 06 11 27 32 87 00 10 02 00 00 01 06 00 00 80 00
10: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
20: 00 00 00 00 00 00 00 00 00 00 00 00 43 10 ed 80
30: 00 00 00 00 c0 00 00 00 00 00 00 00 00 00 00 00

00:11.5 Multimedia audio controller: VIA Technologies, Inc. VT8233/A/8235/8237 AC97 Audio Controller (rev 60)
	Subsystem: ASUSTeK Computer Inc. A8V Deluxe motherboard (Realtek ALC850 codec)
	Control: I/O+ Mem- BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B-
	Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR-
	Interrupt: pin C routed to IRQ 21
	Region 0: I/O ports at e800 [size=256]
	Capabilities: [c0] Power Management version 2
		Flags: PMEClk- DSI- D1+ D2+ AuxCurrent=0mA PME(D0-,D1-,D2-,D3hot-,D3cold-)
		Status: D0 PME-Enable- DSel=0 DScale=0 PME-
00: 06 11 59 30 01 00 10 02 60 00 01 04 00 00 00 00
10: 01 e8 00 00 00 00 00 00 00 00 00 00 00 00 00 00
20: 00 00 00 00 00 00 00 00 00 00 00 00 43 10 2a 81
30: 00 00 00 00 c0 00 00 00 00 00 00 00 05 03 00 00

00:11.6 Communication controller: VIA Technologies, Inc. AC'97 Modem Controller (rev 80)
	Control: I/O- Mem- BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B-
	Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR-
	Interrupt: pin C routed to IRQ 0
	Region 0: I/O ports at 1000 [disabled] [size=256]
	Capabilities: [d0] Power Management version 2
		Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA PME(D0-,D1-,D2-,D3hot-,D3cold-)
		Status: D0 PME-Enable- DSel=0 DScale=0 PME-
00: 06 11 68 30 00 00 10 02 80 00 80 07 00 00 00 00
10: 01 10 00 00 00 00 00 00 00 00 00 00 00 00 00 00
20: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
30: 00 00 00 00 d0 00 00 00 00 00 00 00 00 03 00 00

00:18.0 Host bridge: Advanced Micro Devices [AMD] K8 [Athlon64/Opteron] HyperTransport Technology Configuration
	Control: I/O- Mem- BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B-
	Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR-
	Capabilities: [80] HyperTransport: Host or Secondary Interface
		!!! Possibly incomplete decoding
		Command: WarmRst+ DblEnd-
		Link Control: CFlE- CST- CFE- <LkFail- Init+ EOC- TXO- <CRCErr=0
		Link Config: MLWI=16bit MLWO=16bit LWI=16bit LWO=16bit
		Revision ID: 1.02
00: 22 10 00 11 00 00 10 00 00 00 00 06 00 00 80 00
10: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
20: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
30: 00 00 00 00 80 00 00 00 00 00 00 00 00 00 00 00

00:18.1 Host bridge: Advanced Micro Devices [AMD] K8 [Athlon64/Opteron] Address Map
	Control: I/O- Mem- BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B-
	Status: Cap- 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR-
00: 22 10 01 11 00 00 00 00 00 00 00 06 00 00 80 00
10: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
20: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
30: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00

00:18.2 Host bridge: Advanced Micro Devices [AMD] K8 [Athlon64/Opteron] DRAM Controller
	Control: I/O- Mem- BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B-
	Status: Cap- 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR-
00: 22 10 02 11 00 00 00 00 00 00 00 06 00 00 80 00
10: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
20: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
30: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00

00:18.3 Host bridge: Advanced Micro Devices [AMD] K8 [Athlon64/Opteron] Miscellaneous Control
	Control: I/O- Mem- BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B-
	Status: Cap- 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR-
00: 22 10 03 11 00 00 00 00 00 00 00 06 00 00 80 00
10: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
20: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
30: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00

01:00.0 VGA compatible controller: Matrox Graphics, Inc. MGA G400/G450 (rev 04) (prog-if 00 [VGA])
	Subsystem: Matrox Graphics, Inc. Millennium G400 32Mb SGRAM
	Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B-
	Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR-
	Latency: 64 (4000ns min, 8000ns max), Cache Line Size: 64 bytes
	Interrupt: pin A routed to IRQ 16
	Region 0: Memory at f8000000 (32-bit, prefetchable) [size=32M]
	Region 1: Memory at fbf00000 (32-bit, non-prefetchable) [size=16K]
	Region 2: Memory at fb000000 (32-bit, non-prefetchable) [size=8M]
	Expansion ROM at fbe00000 [disabled] [size=64K]
	Capabilities: [dc] Power Management version 2
		Flags: PMEClk- DSI+ D1- D2- AuxCurrent=0mA PME(D0-,D1-,D2-,D3hot-,D3cold-)
		Status: D0 PME-Enable- DSel=0 DScale=0 PME-
	Capabilities: [f0] AGP version 2.0
		Status: RQ=32 Iso- ArqSz=0 Cal=0 SBA+ ITACoh- GART64- HTrans- 64bit- FW- AGP3- Rate=x1,x2,x4
		Command: RQ=32 ArqSz=0 Cal=0 SBA+ AGP+ GART64- 64bit- FW- Rate=x4
00: 2b 10 25 05 07 00 90 02 04 00 00 03 10 40 00 00
10: 08 00 00 f8 00 00 f0 fb 00 00 00 fb 00 00 00 00
20: 00 00 00 00 00 00 00 00 00 00 00 00 2b 10 f8 19
30: 00 00 e0 fb dc 00 00 00 00 00 00 00 0b 01 10 20

> 1. What consequence does the incorrect cache line size setting have?

None if MWI is not enabled AFAIK.

> 2. What improvement does that pci_try_set_mwi() cause? Speed? Fewer errors?

May be speed, but practically hardly observable. It is described in PCI specs. 
My point supplying the patch was: If the hardware has this property why not using it? 
In any case that does not hurt (if it is correctly implemented in the hardware)..
I am testing it since few days - it works for me.

> 3. Why call pci_try_set_mwi()? Can't you set the cache line size directly?

pci_set_cacheline_size() is NOT exported, while pci_try_set_mwi() IS and
sets (as a side effect) the cache line size.

> 4. You write "try to correct it". So the "fix" might not work? Then what?

It is not really a fix for a bug. If a bug it is in the BIOS, not in the driver.
pci_set_mwi() fails only if cache line size cannot be set. Then MWI is not enabled.
pci_try_set_mwi() just try to set it. If not the device continues to work (nicely) 
as it did up to now.

> 5. Is the problem specific for the Promise chip? That is:
>    a) are any other built-in PCI devices affected by this BIOS bug?

No any other built-in PCI devices are affected as seen from the lspci outpot.

>    b) if you add a PCI card (of any kind), does it also
>       get the incorrect cache line size setting?

There is one: 00:0d.0 Multimedia video controller: Intel Corporation SAA7116,
but it is not MWI capable. So I do not know.

> 6. Are you running the latest BIOS?

Surely not.

> 
> To me it sounds like this is a generic PCI bug on that
> machine, and so should be handled by the kernel's PCI layer.

It is not a bug, because this "strange" value for the cache line size
does not matter if MWI is not enabled, but when it is enabled by
pci_set_mwi() or pci_try_set_mwi() - it is already corrected.

> But if the problem really is specific to the built-in Promise
> chip on that machine, then I can see why a workaround in
> sata_promise could be appropriate.

I do not insist the patch to be included. It is just "nice to have" it.
The hardware has the property, theoretically it should be speedy
(otherwise it would not be implemented in the hardware, nor included 
in the PCI specs), why not using it :-)

But you are the maintainer - you decide.

Regards.

Marin Mitov
> 
> /Mikael
> 



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

* Re: [PATCH] pci_try_set_mwi() in sata_promise
  2008-04-23 13:33   ` Marin Mitov
@ 2008-04-23 14:46     ` Grant Grundler
  2008-04-23 18:15       ` Marin Mitov
  0 siblings, 1 reply; 7+ messages in thread
From: Grant Grundler @ 2008-04-23 14:46 UTC (permalink / raw)
  To: Marin Mitov; +Cc: Mikael Pettersson, linux-kernel, Jeff Garzik, linux-ide

On Wed, Apr 23, 2008 at 6:33 AM, Marin Mitov <mitov@issp.bas.bg> wrote:
...
>  > 2. What improvement does that pci_try_set_mwi() cause? Speed? Fewer errors?
>
>  May be speed, but practically hardly observable.

I expect the difference to be in available memory BW.
MWI (Memory Write Invalidate) allows the IO controller to write
partial cachlines without having to do a read/modify/write sequence.
R/W/M means the cacheline has to cross the memory bus twice.
The difference in performance will depend on how often the
Promise SATA controller was doing partial cachline
transactions with the wrong cacheline size.

This should be measurable if you run a memory test at the same time
as you stress the SATA controller with fio. Re-read the same 128KB block
repeatedly from the Promise controller (I'm expecting that to be cached)
and measure the available memory BW (e.g. see lmbench for a memtest).
Repeat with MWI enabled.

>  > 3. Why call pci_try_set_mwi()? Can't you set the cache line size directly?
>
>  pci_set_cacheline_size() is NOT exported, while pci_try_set_mwi() IS and
> sets (as a side effect) the cache line size.

Setting the cachline size only makes sense if MWI is enabled.
Drivers should not longer directly set the cacheline size since it might
not be what they think it is.

hth,
grant

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

* Re: [PATCH] pci_try_set_mwi() in sata_promise
  2008-04-23 14:46     ` Grant Grundler
@ 2008-04-23 18:15       ` Marin Mitov
  0 siblings, 0 replies; 7+ messages in thread
From: Marin Mitov @ 2008-04-23 18:15 UTC (permalink / raw)
  To: Grant Grundler; +Cc: Mikael Pettersson, linux-kernel, Jeff Garzik, linux-ide

On Wednesday 23 April 2008 05:46:49 pm Grant Grundler wrote:
> On Wed, Apr 23, 2008 at 6:33 AM, Marin Mitov <mitov@issp.bas.bg> wrote:
> ...
> >  > 2. What improvement does that pci_try_set_mwi() cause? Speed? Fewer errors?
> >
> >  May be speed, but practically hardly observable.
> 
> I expect the difference to be in available memory BW.
> MWI (Memory Write Invalidate) allows the IO controller to write
> partial cachlines without having to do a read/modify/write sequence.

My understanding of MWI is different. It concerns full cache line writes.
In that case the CPU can invalidate the cache line (without flushing it to 
RAM). While in case of partial cache line writes, the processor should 
flush the cache line before the controller writes to corresponding cached 
address and then the controller finishes its partial cache line write to RAM.
In case MWI is NOT enables the CPU should flushe every cache line  touched
by the controller's DMA machine, nevertheless it will be overwritten latter.

But, as usual, I could be wrong :-)

> R/W/M means the cacheline has to cross the memory bus twice.
> The difference in performance will depend on how often the
> Promise SATA controller was doing partial cachline
> transactions with the wrong cacheline size.
> 
> This should be measurable if you run a memory test at the same time
> as you stress the SATA controller with fio. Re-read the same 128KB block
> repeatedly from the Promise controller (I'm expecting that to be cached)
> and measure the available memory BW (e.g. see lmbench for a memtest).
> Repeat with MWI enabled.
> 
> 
> >  > 3. Why call pci_try_set_mwi()? Can't you set the cache line size directly?
> >
> >  pci_set_cacheline_size() is NOT exported, while pci_try_set_mwi() IS and
> > sets (as a side effect) the cache line size.
> 
> Setting the cachline size only makes sense if MWI is enabled.

Agreed.

> Drivers should not longer directly set the cacheline size since it might
> not be what they think it is.

That is the reason why pci_set_mwi() and pci_try_set_mwi() exist.

Marin Mitov

> 
> hth,
> grant
> 



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

end of thread, other threads:[~2008-04-23 18:14 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-04-22 18:26 [PATCH] pci_try_set_mwi() in sata_promise Marin Mitov
2008-04-23  7:57 ` Mikael Pettersson
2008-04-23 12:31   ` Marin Mitov
2008-04-23 12:34   ` Marin Mitov
2008-04-23 13:33   ` Marin Mitov
2008-04-23 14:46     ` Grant Grundler
2008-04-23 18:15       ` Marin Mitov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).