LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [Patch 2/2] Kexec/Kdump support POWER6
From: Sachin P. Sant @ 2007-05-23  5:14 UTC (permalink / raw)
  To: Olof Johansson; +Cc: linuxppc-dev, ellerman, Milton Miller II
In-Reply-To: <20070522153419.GA22047@lixom.net>

Hi Olof, thanks for the review. 

>> +	if (have_vrma)
>> +		pSeries_find_hpte_vrma();
>> +
>>     
>
> This will break kexec builds on non-pseries. It's referring to platform
> code that might not be built.
>
>   
Ok. Will call this function from lpar.c instead of
default_machine_kexec().

> Does this function find the vrma, or save it away? Seems like the name
> is misleading.
>
>   
Well it finds a vrma entry and saves it. I thought of
pSeries_find_save_hpte_vrma(), but decided against it. I could
change it to pSeries_save_hpte_vrma().

> Is ppc64_vrma_page_size really the size, or the shift? Above would
> indicate that it's really a shift value.
>
>   
It is a shift. I will change it to ppc64_vrma_page_shift.

> Why is 16M hardcoded here, when you're taking such great care to read
> out the pagesize earlier?
>
>   
Hrmm. Ok will use the vrma_page_shift value.

>> +			((dword0 & HPTE_V_MASK) == MAGIC_SKIP_HPTE)) {
>>     
> Indentation
>   
Done.

>> +				/* store the hpte */
>> +				hpte_vrma_slots[num_hpte_vrma_slots++] = slot;
>>     
>
> Here you rely on global exported state (num_hpte_vrma_slots), increasing it without
> checking for limits. What happens if this function is ever called twice? Should you
> set it to 0 in the beginning of the function and check it against the size of the
> hpte_vrma_slots array instead?
>   
Will add proper checks for num_hpte_vrma_slots variable value.

>> +extern void pSeries_find_hpte_vrma(void);
>>
>>     
> Same comment as above: This isn't a kexec function as much as a pseries function, so
> it should be defined in some other header instead.
>   
>>  
>> +#define HPTE_V_RMA_VPN         ASM_CONST(0x001FFFFFF0000000)
>> +#define HPTE_V_MASK            ASM_CONST(0xc000000000000000)
>> +#define MAGIC_SKIP_HPTE        ASM_CONST(0x4000000000000000)
>> +#define HPTE_V_RMA_NUM         16
>>     
>
> "MAGIC_SKIP_HPTE"? I'm sure there's a proper name for this field in the
> PAPR, isn't there? Also, HPTE_V_RMA_NUM isn't a HPTE_V field, it shouldn't
> have that prefix. It's not a property of the mmu in the first place.
>
> These should maybe be local defines in the pseries lpar code instead, since it's
> more of a lpar<->phyp interface than mmu programming interface.
>   
Will move them to pseries lpar code.
Updated patch on its way.

Thanks
-Sachin

^ permalink raw reply

* Re: [Patch 2/2] Kexec/Kdump support POWER6
From: Sachin P. Sant @ 2007-05-23  5:13 UTC (permalink / raw)
  To: Olof Johansson; +Cc: linuxppc-dev, ellerman, Milton Miller II
In-Reply-To: <20070522153419.GA22047@lixom.net>

Hi Olaf, thanks for the review. 

>> +	if (have_vrma)
>> +		pSeries_find_hpte_vrma();
>> +
>>     
>
> This will break kexec builds on non-pseries. It's referring to platform
> code that might not be built.
>
>   
Ok. Will call this function from lpar.c instead of
default_machine_kexec().

> Does this function find the vrma, or save it away? Seems like the name
> is misleading.
>
>   
Well it finds a vrma entry and saves it. I thought of
pSeries_find_save_hpte_vrma(), but decided against it. I could
change it to pSeries_save_hpte_vrma().

> Is ppc64_vrma_page_size really the size, or the shift? Above would
> indicate that it's really a shift value.
>
>   
It is a shift. I will change it to ppc64_vrma_page_shift.

> Why is 16M hardcoded here, when you're taking such great care to read
> out the pagesize earlier?
>
>   
Hrmm. Ok will use the vrma_page_shift value.

>> +			((dword0 & HPTE_V_MASK) == MAGIC_SKIP_HPTE)) {
>>     
> Indentation
>   
Done.

>> +				/* store the hpte */
>> +				hpte_vrma_slots[num_hpte_vrma_slots++] = slot;
>>     
>
> Here you rely on global exported state (num_hpte_vrma_slots), increasing it without
> checking for limits. What happens if this function is ever called twice? Should you
> set it to 0 in the beginning of the function and check it against the size of the
> hpte_vrma_slots array instead?
>   
Will add proper checks for num_hpte_vrma_slots variable value.

>> +extern void pSeries_find_hpte_vrma(void);
>>
>>     
> Same comment as above: This isn't a kexec function as much as a pseries function, so
> it should be defined in some other header instead.
>   
>>  
>> +#define HPTE_V_RMA_VPN         ASM_CONST(0x001FFFFFF0000000)
>> +#define HPTE_V_MASK            ASM_CONST(0xc000000000000000)
>> +#define MAGIC_SKIP_HPTE        ASM_CONST(0x4000000000000000)
>> +#define HPTE_V_RMA_NUM         16
>>     
>
> "MAGIC_SKIP_HPTE"? I'm sure there's a proper name for this field in the
> PAPR, isn't there? Also, HPTE_V_RMA_NUM isn't a HPTE_V field, it shouldn't
> have that prefix. It's not a property of the mmu in the first place.
>
> These should maybe be local defines in the pseries lpar code instead, since it's
> more of a lpar<->phyp interface than mmu programming interface.
>   
Will move them to pseries lpar code.
Updated patch on its way.

Thanks
-Sachin

^ permalink raw reply

* Re: [PATCH/RFC] Rework ptep_set_access_flags and fix sun4c
From: Benjamin Herrenschmidt @ 2007-05-23  4:21 UTC (permalink / raw)
  To: Hugh Dickins
  Cc: mark, linux-mm, wli, linuxppc-dev, andrea,
	Tom "spot" Callaway, sparclinux, akpm, David Miller
In-Reply-To: <Pine.LNX.4.64.0705230438490.14106@blonde.wat.veritas.com>

On Wed, 2007-05-23 at 05:03 +0100, Hugh Dickins wrote:
> 
> No, I wasn't meaning the optimization, but the significance of the
> boolean __changed that's returned.  If ptep_set_access_flags does
> not change the pte (because !dirty or !safely_writable or whatever
> that arch calls it), then ideally it ought to return false.

Hrm... I prefer keeping the existing semantics. The old code used to
always update_mmu_cache() on those archs and I'd rather let it continue
do so unless the arch maintainer who knows better changes it :-)
 
> But it doesn't affect correctness if it sometimes says true not
> false, and these arches happen to have an empty update_mmu_cache
> (with lazy_mmu_prot_update currently under separate review), and
> what you have follows what was already being done, and sun4c
> already has to "lie": so it's rather theoretical. 

Ok.

Cheers,
Ben.

^ permalink raw reply

* [PATCH] Rework ptep_set_access_flags and fix sun4c
From: Benjamin Herrenschmidt @ 2007-05-23  4:20 UTC (permalink / raw)
  To: Andrew Morton
  Cc: mark, linux-mm, wli, linuxppc-dev, tcallawa, sparclinux,
	Hugh Dickins, David S. Miller, andea

Some changes done a while ago to avoid pounding on ptep_set_access_flags
and update_mmu_cache in some race situations break sun4c which requires
update_mmu_cache() to always be called on minor faults.

This patch reworks ptep_set_access_flags() semantics, implementations
and callers so that it's now responsible for returning whether an update
is necessary or not (basically whether the PTE actually changed). This
allow fixing the sparc implementation to always return 1 on sun4c.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---

This version adds the missing ia64 bits and fixes the i386 bit (compile
tested this time) according to Hugh's comments. I also updated
set_huge_ptep_writable() to do the same as do_wp_page().

---

 include/asm-generic/pgtable.h       |   17 ++++++++++++-----
 include/asm-i386/pgtable.h          |    8 +++++---
 include/asm-ia64/pgtable.h          |   25 ++++++++++++++++---------
 include/asm-powerpc/pgtable-ppc32.h |   12 ++++++++----
 include/asm-powerpc/pgtable-ppc64.h |   12 ++++++++----
 include/asm-ppc/pgtable.h           |   12 ++++++++----
 include/asm-s390/pgtable.h          |    7 ++++++-
 include/asm-sparc/pgtable.h         |   11 +++++++++++
 include/asm-x86_64/pgtable.h        |   14 ++++++++------
 mm/hugetlb.c                        |    7 ++++---
 mm/memory.c                         |   13 ++++++-------
 11 files changed, 92 insertions(+), 46 deletions(-)

Index: linux-work/include/asm-generic/pgtable.h
===================================================================
--- linux-work.orig/include/asm-generic/pgtable.h	2007-05-22 15:41:38.000000000 +1000
+++ linux-work/include/asm-generic/pgtable.h	2007-05-23 14:00:32.000000000 +1000
@@ -27,13 +27,20 @@ do {				  					\
  * Largely same as above, but only sets the access flags (dirty,
  * accessed, and writable). Furthermore, we know it always gets set
  * to a "more permissive" setting, which allows most architectures
- * to optimize this.
+ * to optimize this. We return whether the PTE actually changed, which
+ * in turn instructs the caller to do things like update__mmu_cache.
+ * This used to be done in the caller, but sparc needs minor faults to
+ * force that call on sun4c so we changed this macro slightly
  */
 #define ptep_set_access_flags(__vma, __address, __ptep, __entry, __dirty) \
-do {				  					  \
-	set_pte_at((__vma)->vm_mm, (__address), __ptep, __entry);	  \
-	flush_tlb_page(__vma, __address);				  \
-} while (0)
+({									  \
+	int __changed = !pte_same(*(__ptep), __entry);			  \
+	if (__changed) {						  \
+		set_pte_at((__vma)->vm_mm, (__address), __ptep, __entry); \
+		flush_tlb_page(__vma, __address);			  \
+	}								  \
+	__changed;							  \
+})
 #endif
 
 #ifndef __HAVE_ARCH_PTEP_TEST_AND_CLEAR_YOUNG
Index: linux-work/include/asm-powerpc/pgtable-ppc64.h
===================================================================
--- linux-work.orig/include/asm-powerpc/pgtable-ppc64.h	2007-05-22 15:41:38.000000000 +1000
+++ linux-work/include/asm-powerpc/pgtable-ppc64.h	2007-05-22 16:03:09.000000000 +1000
@@ -413,10 +413,14 @@ static inline void __ptep_set_access_fla
 	:"cc");
 }
 #define  ptep_set_access_flags(__vma, __address, __ptep, __entry, __dirty) \
-	do {								   \
-		__ptep_set_access_flags(__ptep, __entry, __dirty);	   \
-		flush_tlb_page_nohash(__vma, __address);	       	   \
-	} while(0)
+({									   \
+	int __changed = !pte_same(*(__ptep), __entry);			   \
+	if (__changed) {						   \
+		__ptep_set_access_flags(__ptep, __entry, __dirty);    	   \
+		flush_tlb_page_nohash(__vma, __address);		   \
+	}								   \
+	__changed;							   \
+})
 
 /*
  * Macro to mark a page protection value as "uncacheable".
Index: linux-work/mm/memory.c
===================================================================
--- linux-work.orig/mm/memory.c	2007-05-22 15:41:38.000000000 +1000
+++ linux-work/mm/memory.c	2007-05-22 16:03:09.000000000 +1000
@@ -1691,9 +1691,10 @@ static int do_wp_page(struct mm_struct *
 		flush_cache_page(vma, address, pte_pfn(orig_pte));
 		entry = pte_mkyoung(orig_pte);
 		entry = maybe_mkwrite(pte_mkdirty(entry), vma);
-		ptep_set_access_flags(vma, address, page_table, entry, 1);
-		update_mmu_cache(vma, address, entry);
-		lazy_mmu_prot_update(entry);
+		if (ptep_set_access_flags(vma, address, page_table, entry,1)) {
+			update_mmu_cache(vma, address, entry);
+			lazy_mmu_prot_update(entry);
+		}
 		ret |= VM_FAULT_WRITE;
 		goto unlock;
 	}
@@ -2525,10 +2526,9 @@ static inline int handle_pte_fault(struc
 		pte_t *pte, pmd_t *pmd, int write_access)
 {
 	pte_t entry;
-	pte_t old_entry;
 	spinlock_t *ptl;
 
-	old_entry = entry = *pte;
+	entry = *pte;
 	if (!pte_present(entry)) {
 		if (pte_none(entry)) {
 			if (vma->vm_ops) {
@@ -2561,8 +2561,7 @@ static inline int handle_pte_fault(struc
 		entry = pte_mkdirty(entry);
 	}
 	entry = pte_mkyoung(entry);
-	if (!pte_same(old_entry, entry)) {
-		ptep_set_access_flags(vma, address, pte, entry, write_access);
+	if (ptep_set_access_flags(vma, address, pte, entry, write_access)) {
 		update_mmu_cache(vma, address, entry);
 		lazy_mmu_prot_update(entry);
 	} else {
Index: linux-work/include/asm-powerpc/pgtable-ppc32.h
===================================================================
--- linux-work.orig/include/asm-powerpc/pgtable-ppc32.h	2007-05-22 15:41:38.000000000 +1000
+++ linux-work/include/asm-powerpc/pgtable-ppc32.h	2007-05-22 16:03:09.000000000 +1000
@@ -673,10 +673,14 @@ static inline void __ptep_set_access_fla
 }
 
 #define  ptep_set_access_flags(__vma, __address, __ptep, __entry, __dirty) \
-	do {								   \
-		__ptep_set_access_flags(__ptep, __entry, __dirty);	   \
-		flush_tlb_page_nohash(__vma, __address);	       	   \
-	} while(0)
+({									   \
+	int __changed = !pte_same(*(__ptep), __entry);			   \
+	if (__changed) {						   \
+		__ptep_set_access_flags(__ptep, __entry, __dirty);    	   \
+		flush_tlb_page_nohash(__vma, __address);		   \
+	}								   \
+	__changed;							   \
+})
 
 /*
  * Macro to mark a page protection value as "uncacheable".
Index: linux-work/include/asm-i386/pgtable.h
===================================================================
--- linux-work.orig/include/asm-i386/pgtable.h	2007-05-22 15:41:38.000000000 +1000
+++ linux-work/include/asm-i386/pgtable.h	2007-05-23 11:54:33.000000000 +1000
@@ -285,13 +285,15 @@ static inline pte_t native_local_ptep_ge
  */
 #define  __HAVE_ARCH_PTEP_SET_ACCESS_FLAGS
 #define ptep_set_access_flags(vma, address, ptep, entry, dirty)		\
-do {									\
-	if (dirty) {							\
+({									\
+	int __changed = !pte_same(*(ptep), entry);			\
+	if (__changed && dirty) {					\
 		(ptep)->pte_low = (entry).pte_low;			\
 		pte_update_defer((vma)->vm_mm, (address), (ptep));	\
 		flush_tlb_page(vma, address);				\
 	}								\
-} while (0)
+	__changed;							\
+})
 
 #define __HAVE_ARCH_PTEP_TEST_AND_CLEAR_DIRTY
 #define ptep_test_and_clear_dirty(vma, addr, ptep) ({			\
Index: linux-work/include/asm-ppc/pgtable.h
===================================================================
--- linux-work.orig/include/asm-ppc/pgtable.h	2007-05-22 15:41:38.000000000 +1000
+++ linux-work/include/asm-ppc/pgtable.h	2007-05-22 16:03:09.000000000 +1000
@@ -694,10 +694,14 @@ static inline void __ptep_set_access_fla
 }
 
 #define  ptep_set_access_flags(__vma, __address, __ptep, __entry, __dirty) \
-	do {								   \
-		__ptep_set_access_flags(__ptep, __entry, __dirty);	   \
-		flush_tlb_page_nohash(__vma, __address);	       	   \
-	} while(0)
+({									   \
+	int __changed = !pte_same(*(__ptep), __entry);			   \
+	if (__changed) {						   \
+		__ptep_set_access_flags(__ptep, __entry, __dirty);    	   \
+		flush_tlb_page_nohash(__vma, __address);		   \
+	}								   \
+	__changed;							   \
+})
 
 /*
  * Macro to mark a page protection value as "uncacheable".
Index: linux-work/include/asm-s390/pgtable.h
===================================================================
--- linux-work.orig/include/asm-s390/pgtable.h	2007-05-22 15:41:38.000000000 +1000
+++ linux-work/include/asm-s390/pgtable.h	2007-05-22 16:03:09.000000000 +1000
@@ -744,7 +744,12 @@ ptep_establish(struct vm_area_struct *vm
 }
 
 #define ptep_set_access_flags(__vma, __address, __ptep, __entry, __dirty) \
-	ptep_establish(__vma, __address, __ptep, __entry)
+({									  \
+	int __changed = !pte_same(*(__ptep), __entry);			  \
+	if (__changed)							  \
+		ptep_establish(__vma, __address, __ptep, __entry);	  \
+	__changed;							  \
+})
 
 /*
  * Test and clear dirty bit in storage key.
Index: linux-work/include/asm-sparc/pgtable.h
===================================================================
--- linux-work.orig/include/asm-sparc/pgtable.h	2007-05-22 15:41:38.000000000 +1000
+++ linux-work/include/asm-sparc/pgtable.h	2007-05-22 16:03:09.000000000 +1000
@@ -446,6 +446,17 @@ extern int io_remap_pfn_range(struct vm_
 #define GET_IOSPACE(pfn)		(pfn >> (BITS_PER_LONG - 4))
 #define GET_PFN(pfn)			(pfn & 0x0fffffffUL)
 
+#define __HAVE_ARCH_PTEP_SET_ACCESS_FLAGS
+#define ptep_set_access_flags(__vma, __address, __ptep, __entry, __dirty) \
+({									  \
+	int __changed = !pte_same(*(__ptep), __entry);			  \
+	if (__changed) {						  \
+		set_pte_at((__vma)->vm_mm, (__address), __ptep, __entry); \
+		flush_tlb_page(__vma, __address);			  \
+	}								  \
+	(sparc_cpu_model == sun4c) || __changed;			  \
+})
+
 #include <asm-generic/pgtable.h>
 
 #endif /* !(__ASSEMBLY__) */
Index: linux-work/include/asm-x86_64/pgtable.h
===================================================================
--- linux-work.orig/include/asm-x86_64/pgtable.h	2007-05-22 15:41:38.000000000 +1000
+++ linux-work/include/asm-x86_64/pgtable.h	2007-05-22 16:03:09.000000000 +1000
@@ -395,12 +395,14 @@ static inline pte_t pte_modify(pte_t pte
  * bit at the same time. */
 #define  __HAVE_ARCH_PTEP_SET_ACCESS_FLAGS
 #define ptep_set_access_flags(__vma, __address, __ptep, __entry, __dirty) \
-	do {								  \
-		if (__dirty) {						  \
-			set_pte(__ptep, __entry);			  \
-			flush_tlb_page(__vma, __address);		  \
-		}							  \
-	} while (0)
+({									  \
+	int __changed = !pte_same(*(__ptep), __entry);			  \
+	if (__changed && __dirty) {					  \
+		set_pte(__ptep, __entry);			  	  \
+		flush_tlb_page(__vma, __address);		  	  \
+	}								  \
+	__changed;							  \
+})
 
 /* Encode and de-code a swap entry */
 #define __swp_type(x)			(((x).val >> 1) & 0x3f)
Index: linux-work/include/asm-ia64/pgtable.h
===================================================================
--- linux-work.orig/include/asm-ia64/pgtable.h	2007-05-23 11:55:13.000000000 +1000
+++ linux-work/include/asm-ia64/pgtable.h	2007-05-23 11:57:23.000000000 +1000
@@ -533,16 +533,23 @@ extern void lazy_mmu_prot_update (pte_t 
  * daccess_bit in ivt.S).
  */
 #ifdef CONFIG_SMP
-# define ptep_set_access_flags(__vma, __addr, __ptep, __entry, __safely_writable)	\
-do {											\
-	if (__safely_writable) {							\
-		set_pte(__ptep, __entry);						\
-		flush_tlb_page(__vma, __addr);						\
-	}										\
-} while (0)
+# define ptep_set_access_flags(__vma, __addr, __ptep, __entry, __safely_writable) \
+({										\
+	int __changed = !pte_same(*(__ptep), __entry);				\
+	if (__changed && __safely_writable) {					\
+		set_pte(__ptep, __entry);					\
+		flush_tlb_page(__vma, __addr);					\
+	}									\
+	__changed;								\
+})
 #else
-# define ptep_set_access_flags(__vma, __addr, __ptep, __entry, __safely_writable)	\
-	ptep_establish(__vma, __addr, __ptep, __entry)
+# define ptep_set_access_flags(__vma, __addr, __ptep, __entry, __safely_writable) \
+({										\
+	int __changed = !pte_same(*(__ptep), __entry);				\
+	if (__changed) {							\
+		ptep_establish(__vma, __addr, __ptep, __entry)			\
+	__changed;								\
+})
 #endif
 
 #  ifdef CONFIG_VIRTUAL_MEM_MAP
Index: linux-work/mm/hugetlb.c
===================================================================
--- linux-work.orig/mm/hugetlb.c	2007-05-23 13:59:25.000000000 +1000
+++ linux-work/mm/hugetlb.c	2007-05-23 13:59:32.000000000 +1000
@@ -326,9 +326,10 @@ static void set_huge_ptep_writable(struc
 	pte_t entry;
 
 	entry = pte_mkwrite(pte_mkdirty(*ptep));
-	ptep_set_access_flags(vma, address, ptep, entry, 1);
-	update_mmu_cache(vma, address, entry);
-	lazy_mmu_prot_update(entry);
+	if (ptep_set_access_flags(vma, address, ptep, entry, 1)) {
+		update_mmu_cache(vma, address, entry);
+		lazy_mmu_prot_update(entry);
+	}
 }
 
 

^ permalink raw reply

* Re: [PATCH/RFC] Rework ptep_set_access_flags and fix sun4c
From: Hugh Dickins @ 2007-05-23  4:03 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: mark, linux-mm, wli, linuxppc-dev, andrea,
	Tom "spot" Callaway, sparclinux, akpm, David Miller
In-Reply-To: <1179874748.32247.868.camel@localhost.localdomain>

On Wed, 23 May 2007, Benjamin Herrenschmidt wrote:
> 
> > Would the "__changed && __dirty" architectures (x86, x86_64, ia64)
> > be better off saying __changed = __dirty && pte_same?  I doubt it's
> > worth bothering about.
> 
> I'd say let gcc figure it out :-)

No, I wasn't meaning the optimization, but the significance of the
boolean __changed that's returned.  If ptep_set_access_flags does
not change the pte (because !dirty or !safely_writable or whatever
that arch calls it), then ideally it ought to return false.

But it doesn't affect correctness if it sometimes says true not
false, and these arches happen to have an empty update_mmu_cache
(with lazy_mmu_prot_update currently under separate review), and
what you have follows what was already being done, and sun4c
already has to "lie": so it's rather theoretical.

Hugh

^ permalink raw reply

* RE: [PATCH 2/5] Add uli1575 pci-bridge sector to MPC8641HPCN dts file.
From: Zhang Wei-r63237 @ 2007-05-23  3:38 UTC (permalink / raw)
  To: Xiao Xianghua-R6AADS; +Cc: linuxppc-dev, paulus
In-Reply-To: <465354A0.5020502@freescale.com>

Hi, Xianghua,=20

> -----Original Message-----
> From: xianghua xiao [mailto:x.xiao@freescale.com]=20
> Subject: Re: [PATCH 2/5] Add uli1575 pci-bridge sector to=20
> MPC8641HPCN dts file.
>=20
> where is the new uli1575 node referred except for this added snippet?
> thanks,
> xianghua

There're no other places now. You means the 'uli1757:' must be removed?

Thanks!
Wei

>=20
> Zhang Wei wrote:
>=20
> >Add uli1575 pci-bridge sector. It fixes the issue of ULI1575=20
> not found on rev.2 board.
> >
> >Signed-off-by: Zhang Wei <wei.zhang@freescale.com>
> >---
> > arch/powerpc/boot/dts/mpc8641_hpcn.dts      |    6 ++++++
> > arch/powerpc/boot/dts/mpc8641_hpcn_srio.dts |    6 ++++++
> > 2 files changed, 12 insertions(+), 0 deletions(-)
> >
> >diff --git a/arch/powerpc/boot/dts/mpc8641_hpcn.dts=20
> b/arch/powerpc/boot/dts/mpc8641_hpcn.dts
> >index 260b264..04626b1 100644
> >--- a/arch/powerpc/boot/dts/mpc8641_hpcn.dts
> >+++ b/arch/powerpc/boot/dts/mpc8641_hpcn.dts
> >@@ -297,6 +297,12 @@
> > 				interrupts =3D <49 2>;
> > 				interrupt-parent =3D <&mpic>;
> > 			};
> >+			uli1575: uli1575@100 {
> >+				reg =3D <0 0 0 0 0>;
> >+				pci_bridge@200 {
> >+					reg =3D <0 0 0 0 0>;
> >+				};
> >+			};
> >=20
> > 		};
> >=20
> > =20
> >
>=20
>=20

^ permalink raw reply

* Re: [RFC] BOOKE watchdog and kexec
From: Michael Ellerman @ 2007-05-23  3:36 UTC (permalink / raw)
  To: Dave Jiang; +Cc: linuxppc-dev
In-Reply-To: <46538264.2050000@mvista.com>

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

On Tue, 2007-05-22 at 16:53 -0700, Dave Jiang wrote:
> What would be the appropriate way to deal with the BOOKE watchdog in order to
> properly kexec? The BOOKE watchdog cannot be disabled. With the current
> implementation, a watchdog daemon in userland is required to poke the
> /dev/watchdog continously in order to keep it from going off. In the kexec
> situation, the watchdog daemon in userland goes away when the new kernel is
> executed. It is very possible that the new kernel can potentially timeout on a
> certain hardware device initialization (i.e. SCSI discovery/timeout) and causes
> the watchdog to go off and reset the hardware. The reset is of course not
> wanted in this situation.
> 
> Several solutions comes into mind:
> 1. Have the kernel timer poke the watchdog. This would ensure situation
> described above would never happen. I think x86 does this with NMI watchdog.
> 
> 2. Have the watchdog driver spawn a kernel thread to poke the watchdog at a
> periodic time. Or perhaps use the delayed-work mechanism to do that.
> 
> 3. Set the highest bit of the watchdog register so that it does not expire for
> 2^32 ticks.

#3 sounds the easiest. You'd set it in machine_kexec_prepare() and then
have the second kernel restore a sane value. I assume 2^32 ticks is long
enough to boot?

cheers

-- 
Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* RE: [PATCH 3/5] Float the pci bus number on MPC8641HPCN board.
From: Zhang Wei-r63237 @ 2007-05-23  3:35 UTC (permalink / raw)
  To: Wade Farnsworth; +Cc: linuxppc-dev, paulus
In-Reply-To: <1179856792.27985.22.camel@rhino>

Hi, Wade,

That's a good question.

The ppc_md.exclude_device() function will be called before PCI config
access in indirect_read/write_config() of the file
arch/powerpc/sysdev/indirect_pci.c. If the hose->bus_offset value is 0,
the primary bus number register of the host must be hose->first_busno.
Otherwise, if the host primary bus number register is 0, the
hose->bus_offset must be the same value of hose->first_buseno.

When the pci scan bus, the bridge will be scaned twice. When the second
scan is finished, the host primary bus number register will be write to
hose->first_busno. The PCI bus(not the first PCI host) access will be
wrong in the later access. I'll check it and correct the
hose->bus_offset value in these codes.

Thanks!
Wei

> -----Original Message-----
> From: Wade Farnsworth [mailto:wfarnsworth@mvista.com]=20
> Subject: Re: [PATCH 3/5] Float the pci bus number on=20
> MPC8641HPCN board.
>=20
> On Tue, 2007-05-22 at 11:38 +0800, Zhang Wei wrote:
> >  int mpc86xx_exclude_device(u_char bus, u_char devfn)
> >  {
> > +	struct pci_controller *hose;
> > +
> > +	hose =3D pci_bus_to_hose(bus);
> > +	if (unlikely(!hose))
> > +		return PCIBIOS_DEVICE_NOT_FOUND;
> > +
> > +	/* Correcting the hose->bus_offset value. */
> > +	out_be32(hose->cfg_addr, 0x80000000 | ((hose->first_busno
> > +					- hose->bus_offset) << 16));
> > +	if (unlikely(in_le32(hose->cfg_data) =3D=3D 0xffffffff))
> > +		hose->bus_offset =3D hose->bus_offset ? 0 :=20
> hose->first_busno;
> > +
> >  	return PCIBIOS_SUCCESSFUL;
> >  }
>=20
> What is the purpose of this code and why put it in
> mpc86xx_exclude_device?
>=20
> --Wade
>=20
>=20

^ permalink raw reply

* [PATCH] powerpc: Fix Section mismatch warnings
From: Li Yang @ 2007-05-23  3:28 UTC (permalink / raw)
  To: Paul; +Cc: sl, Linux Kernel, linuxppc-dev

This patch fix the following Section mismatch warnings
in powerpc code.

WARNING: arch/powerpc/platforms/built-in.o - Section mismatch: reference to .init.data:mv643xx_eth_pd_devs from .text between 'mv643xx_eth_add_pds' (at offset 0x9ed2) and 'gg2_read_config'
WARNING: arch/powerpc/platforms/built-in.o - Section mismatch: reference to .init.data:mv643xx_eth_pd_devs from .text between 'mv643xx_eth_add_pds' (at offset 0x9ed6) and 'gg2_read_config'
WARNING: arch/powerpc/platforms/built-in.o - Section mismatch: reference to .init.text:note_scsi_host from __ksymtab between '__ksymtab_note_scsi_host' (at offset 0x8) and '__ksymtab_sys_ctrler'

Signed-off-by: Li Yang <leoli@freescale.com>
---
 arch/powerpc/platforms/chrp/pegasos_eth.c |    2 +-
 arch/powerpc/platforms/powermac/setup.c   |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/chrp/pegasos_eth.c b/arch/powerpc/platforms/chrp/pegasos_eth.c
index 7104567..5bcc58d 100644
--- a/arch/powerpc/platforms/chrp/pegasos_eth.c
+++ b/arch/powerpc/platforms/chrp/pegasos_eth.c
@@ -169,7 +169,7 @@ static int Enable_SRAM(void)
 
 /***********/
 /***********/
-int mv643xx_eth_add_pds(void)
+static int __init mv643xx_eth_add_pds(void)
 {
 	int ret = 0;
 	static struct pci_device_id pci_marvell_mv64360[] = {
diff --git a/arch/powerpc/platforms/powermac/setup.c b/arch/powerpc/platforms/powermac/setup.c
index a410bc7..07b1c4e 100644
--- a/arch/powerpc/platforms/powermac/setup.c
+++ b/arch/powerpc/platforms/powermac/setup.c
@@ -384,7 +384,7 @@ int boot_part;
 static dev_t boot_dev;
 
 #ifdef CONFIG_SCSI
-void __init note_scsi_host(struct device_node *node, void *host)
+void note_scsi_host(struct device_node *node, void *host)
 {
 	int l;
 	char *p;

^ permalink raw reply related

* Re: fsl booke MM vs. SMP questions
From: Benjamin Herrenschmidt @ 2007-05-23  3:08 UTC (permalink / raw)
  To: Dave Liu; +Cc: ppc-dev, Paul Mackerras, Kumar Gala
In-Reply-To: <1179887929.3656.2.camel@localhost.localdomain>

On Wed, 2007-05-23 at 10:38 +0800, Dave Liu wrote:
> On Wed, 2007-05-23 at 08:42 +1000, Benjamin Herrenschmidt wrote:
> 
> > The IPIs should be buffered by the PIC ... delivered only once but
> 
> but what is the buffer depth for IPIs in the PIC?

One :-)

You never "loose" IPIs in the sense that you always get at least 1
interrupt for N IPIs and it's up to software to make sure not to lose
any event. The linux kernel arch code usually handles that with a
synchronous IPI mecanism.

Ben.

^ permalink raw reply

* RE: [PATCH 0/5] Add the support for MPC8641 silicon rev 2.0 andMPC8641HPCN board 2.0
From: Zhang Wei-r63237 @ 2007-05-23  2:48 UTC (permalink / raw)
  To: Wade Farnsworth; +Cc: linuxppc-dev, paulus
In-Reply-To: <1179856039.27985.17.camel@rhino>

Hi,

They are against to Paul's git tree and I've verified these patches
again today.

They can be applied to Paul's top git tree successfully.

Thanks!
Wei=20

> -----Original Message-----
> From: Wade Farnsworth [mailto:wfarnsworth@mvista.com]=20
> Sent: Wednesday, May 23, 2007 1:47 AM
> To: Zhang Wei-r63237
> Cc: paulus@samba.org; linuxppc-dev@ozlabs.org
> Subject: Re: [PATCH 0/5] Add the support for MPC8641 silicon=20
> rev 2.0 andMPC8641HPCN board 2.0
>=20
> On Tue, 2007-05-22 at 11:38 +0800, Zhang Wei wrote:
> > Hi, Paul,
> >=20
> > The below 5 patches add the support for MPC8641 rev 2.0=20
> silicon and MPC8641HPCN board 2.0.
> >=20
> >   [PATCH 1/5] Remove the errata fix codes for MPC8641=20
> silicon ver 1.0 which is end of life.
> >   [PATCH 2/5] Add uli1575 pci-bridge sector to MPC8641HPCN dts file.
> >   [PATCH 3/5] Float the pci bus number on MPC8641HPCN board.
> >   [PATCH 4/5] Set RC of mpc8641 to transparent bridge for=20
> transfer legacy I/O access.
> >   [PATCH 5/5] Set IDE in ULI1575 to not native mode.
> >=20
> > The MPC8641 rev 2.0 silicon fix the PCI/PCI-Express errata=20
> in rev 1.0. And the rev 1.0 silicon is end of life. So the=20
> dts tree and codes should be changes. Codes which are not=20
> compatible with PCI/PCI-Express specifications are removed.=20
> Some minor issues are fixed.
> >=20
> > Thanks!
> >=20
> > Best Regards,
> > Zhang Wei
> >=20
>=20
> I can't get your patches to apply to Paulus' tree.  Which=20
> tree are these
> against?
>=20
> --Wade
>=20
>=20

^ permalink raw reply

* Re: fsl booke MM vs. SMP questions
From: Dave Liu @ 2007-05-23  2:38 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: ppc-dev, Paul Mackerras, Kumar Gala
In-Reply-To: <1179873742.32247.864.camel@localhost.localdomain>

On Wed, 2007-05-23 at 08:42 +1000, Benjamin Herrenschmidt wrote:

> The IPIs should be buffered by the PIC ... delivered only once but

but what is the buffer depth for IPIs in the PIC?

> still. Also, IPI handling in linux is synchronous, there is an ack to
> wait for the remote function to complete.

ya. I got it.

^ permalink raw reply

* Re: [PATCH] pseries: asm/pci-bridge.h CONFIG_ minor cleanup
From: Michael Ellerman @ 2007-05-23  1:11 UTC (permalink / raw)
  To: Linas Vepstas; +Cc: linuxppc-dev, Paul Mackerras
In-Reply-To: <20070522170058.GJ5921@austin.ibm.com>

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

On Tue, 2007-05-22 at 12:00 -0500, Linas Vepstas wrote:
> On Tue, May 22, 2007 at 10:40:59AM +1000, Michael Ellerman wrote:
> > On Mon, 2007-05-21 at 18:18 -0500, Linas Vepstas wrote:
> > > Use the correct CONFIG_ option to mark off the EEH bits.
> > 
> > You're making the struct 4 bytes larger by creating a hole here :(
> 
> Hmm. Perhaps I could shuffle one of the ints down ...  
> 
> I figured there might be some vague cache-line benefits to
> getting the eeh stuff out of the way. 

Yeah who knows. The hole doesn't really matter unless the struct gets
bigger, having the EEH stuff at the bottom makes it read better which is
probably more important.


> > >  	u32	config_space[16];	/* saved PCI config space */
> > > +#endif
> > 
> > It looks correct, but I think it's worth mentioning in the changelog
> > that config_space was previously unconditionally defined, but is now
> > within CONFIG_EEH.
> 
> I could just tell that someone would remark on this; why I didn't
> mention it, I don't know.

Me neither :)

cheers

-- 
Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* [PATCH] remove spinlock from struct cpu_purr_data
From: Nathan Lynch @ 2007-05-23  0:51 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: linuxppc-dev

cpu_purr_data is a per-cpu array used to account for stolen time on
partitioned systems.  It used to be the case that cpus accessed each
others' cpu_purr_data, so each entry was protected by a spinlock.

However, the code was reworked ("Simplify stolen time calculation")
with the result that each cpu accesses its own cpu_purr_data and not
those of other cpus.  This means we can get rid of the spinlock as
long as we're careful to disable interrupts when accessing
cpu_purr_data in process context.

Signed-off-by: Nathan Lynch <ntl@pobox.com>
---
 arch/powerpc/kernel/time.c |   24 +++++++++++++++---------
 1 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index 2c8564d..0a8a820 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -222,19 +222,28 @@ struct cpu_purr_data {
 	int	initialized;			/* thread is running */
 	u64	tb;			/* last TB value read */
 	u64	purr;			/* last PURR value read */
-	spinlock_t lock;
 };
 
+/*
+ * Each entry in the cpu_purr_data array is manipulated only by its
+ * "owner" cpu -- usually in the timer interrupt but also occasionally
+ * in process context for cpu online.  As long as cpus do not touch
+ * each others' cpu_purr_data, disabling local interrupts is
+ * sufficient to serialize accesses.
+ */
 static DEFINE_PER_CPU(struct cpu_purr_data, cpu_purr_data);
 
 static void snapshot_tb_and_purr(void *data)
 {
+	unsigned long flags;
 	struct cpu_purr_data *p = &__get_cpu_var(cpu_purr_data);
 
+	local_irq_save(flags);
 	p->tb = mftb();
 	p->purr = mfspr(SPRN_PURR);
 	wmb();
 	p->initialized = 1;
+	local_irq_restore(flags);
 }
 
 /*
@@ -242,15 +251,14 @@ static void snapshot_tb_and_purr(void *data)
  */
 void snapshot_timebases(void)
 {
-	int cpu;
-
 	if (!cpu_has_feature(CPU_FTR_PURR))
 		return;
-	for_each_possible_cpu(cpu)
-		spin_lock_init(&per_cpu(cpu_purr_data, cpu).lock);
 	on_each_cpu(snapshot_tb_and_purr, NULL, 0, 1);
 }
 
+/*
+ * Must be called with interrupts disabled.
+ */
 void calculate_steal_time(void)
 {
 	u64 tb, purr;
@@ -262,7 +270,6 @@ void calculate_steal_time(void)
 	pme = &per_cpu(cpu_purr_data, smp_processor_id());
 	if (!pme->initialized)
 		return;		/* this can happen in early boot */
-	spin_lock(&pme->lock);
 	tb = mftb();
 	purr = mfspr(SPRN_PURR);
 	stolen = (tb - pme->tb) - (purr - pme->purr);
@@ -270,7 +277,6 @@ void calculate_steal_time(void)
 		account_steal_time(current, stolen);
 	pme->tb = tb;
 	pme->purr = purr;
-	spin_unlock(&pme->lock);
 }
 
 /*
@@ -284,12 +290,12 @@ static void snapshot_purr(void)
 
 	if (!cpu_has_feature(CPU_FTR_PURR))
 		return;
+	local_irq_save(flags);
 	pme = &per_cpu(cpu_purr_data, smp_processor_id());
-	spin_lock_irqsave(&pme->lock, flags);
 	pme->tb = mftb();
 	pme->purr = mfspr(SPRN_PURR);
 	pme->initialized = 1;
-	spin_unlock_irqrestore(&pme->lock, flags);
+	local_irq_restore(flags);
 }
 
 #endif /* CONFIG_PPC_SPLPAR */
-- 
1.5.2

^ permalink raw reply related

* Re: [RFC] BOOKE watchdog and kexec
From: Dave Jiang @ 2007-05-23  0:29 UTC (permalink / raw)
  To: Geoff Levand; +Cc: linuxppc-dev
In-Reply-To: <4653886A.30800@am.sony.com>

Geoff Levand wrote:
> Dave Jiang wrote:
>> What would be the appropriate way to deal with the BOOKE watchdog in order to
>> properly kexec? The BOOKE watchdog cannot be disabled. With the current
>> implementation, a watchdog daemon in userland is required to poke the
>> /dev/watchdog continously in order to keep it from going off. In the kexec
>> situation, the watchdog daemon in userland goes away when the new kernel is
>> executed. It is very possible that the new kernel can potentially timeout on a
>> certain hardware device initialization (i.e. SCSI discovery/timeout) and causes
>> the watchdog to go off and reset the hardware. The reset is of course not
>> wanted in this situation.
> 
> I would think the same situation exists when the bootloader loads the first
> kernel.  If that works, then you should be able to use the same mechanism to
> get the second kernel up.
> 
> -Geoff
> 

Not really. The bootloader starts from a hardware reset. The watchdog is off
from a hardware reset. The kernel driver has to specifically turn the watchdog
on either via kernel command line or by opening the watchdog device
/dev/watchdog right now. So technically this issue already exists even without
kexec. If the watchdog is turned on via kernel parameter and we hit a device
initialization timeout that takes too long, then we will get a watchdog reset.
There is a period of uncertainty between the watchdog turning on and when the
userland watchdog daemon is started with the current implementation.

-- 

------------------------------------------------------
Dave Jiang
Software Engineer
MontaVista Software, Inc.
http://www.mvista.com
------------------------------------------------------

^ permalink raw reply

* Re: [RFC] BOOKE watchdog and kexec
From: Geoff Levand @ 2007-05-23  0:18 UTC (permalink / raw)
  To: Dave Jiang; +Cc: linuxppc-dev
In-Reply-To: <46538264.2050000@mvista.com>

Dave Jiang wrote:
> What would be the appropriate way to deal with the BOOKE watchdog in order to
> properly kexec? The BOOKE watchdog cannot be disabled. With the current
> implementation, a watchdog daemon in userland is required to poke the
> /dev/watchdog continously in order to keep it from going off. In the kexec
> situation, the watchdog daemon in userland goes away when the new kernel is
> executed. It is very possible that the new kernel can potentially timeout on a
> certain hardware device initialization (i.e. SCSI discovery/timeout) and causes
> the watchdog to go off and reset the hardware. The reset is of course not
> wanted in this situation.

I would think the same situation exists when the bootloader loads the first
kernel.  If that works, then you should be able to use the same mechanism to
get the second kernel up.

-Geoff

^ permalink raw reply

* [RFC] BOOKE watchdog and kexec
From: Dave Jiang @ 2007-05-22 23:53 UTC (permalink / raw)
  To: linuxppc-dev

What would be the appropriate way to deal with the BOOKE watchdog in order to
properly kexec? The BOOKE watchdog cannot be disabled. With the current
implementation, a watchdog daemon in userland is required to poke the
/dev/watchdog continously in order to keep it from going off. In the kexec
situation, the watchdog daemon in userland goes away when the new kernel is
executed. It is very possible that the new kernel can potentially timeout on a
certain hardware device initialization (i.e. SCSI discovery/timeout) and causes
the watchdog to go off and reset the hardware. The reset is of course not
wanted in this situation.

Several solutions comes into mind:
1. Have the kernel timer poke the watchdog. This would ensure situation
described above would never happen. I think x86 does this with NMI watchdog.

2. Have the watchdog driver spawn a kernel thread to poke the watchdog at a
periodic time. Or perhaps use the delayed-work mechanism to do that.

3. Set the highest bit of the watchdog register so that it does not expire for
2^32 ticks.

IMHO, #2 seems to be a reasonable approach. Comments please?

-- 

------------------------------------------------------
Dave Jiang
Software Engineer
MontaVista Software, Inc.
http://www.mvista.com
------------------------------------------------------

^ permalink raw reply

* [PATCH 10/10] spidernet: increase the NAPI weight
From: Linas Vepstas @ 2007-05-22 23:52 UTC (permalink / raw)
  To: Jeff Garzik, Andrew Morton
  Cc: netdev, Florin Malita, cbe-oss-dev, linuxppc-dev
In-Reply-To: <20070522230942.GT5921@austin.ibm.com>


Another way of minimizing the likelyhood of RX ram from overflowing
is to empty out the entire rx ring every chance we get. Change
the crazy watchdog timeout from 50 seconds to 3 seconds, while
we're here.

Signed-off-by: Linas Vepstas <linas@austin.ibm.com>

----
 drivers/net/spider_net.h |    9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

Index: netdev-2.6/drivers/net/spider_net.h
===================================================================
--- netdev-2.6.orig/drivers/net/spider_net.h	2007-05-22 18:03:24.000000000 -0500
+++ netdev-2.6/drivers/net/spider_net.h	2007-05-22 18:03:43.000000000 -0500
@@ -56,8 +56,13 @@ extern char spider_net_driver_name[];
 
 #define SPIDER_NET_RX_CSUM_DEFAULT		1
 
-#define SPIDER_NET_WATCHDOG_TIMEOUT		50*HZ
-#define SPIDER_NET_NAPI_WEIGHT			64
+#define SPIDER_NET_WATCHDOG_TIMEOUT		3*HZ
+
+/* We really really want to empty the ring buffer every time,
+ * so as to avoid the RX ram full bug. So set the napi weight
+ * to the ring size.
+ */
+#define SPIDER_NET_NAPI_WEIGHT			SPIDER_NET_RX_DESCRIPTORS_DEFAULT
 
 #define SPIDER_NET_FIRMWARE_SEQS	6
 #define SPIDER_NET_FIRMWARE_SEQWORDS	1024

^ permalink raw reply

* [PATCH 9/10] spidernet: service TX later.
From: Linas Vepstas @ 2007-05-22 23:47 UTC (permalink / raw)
  To: Jeff Garzik, Andrew Morton
  Cc: netdev, Florin Malita, cbe-oss-dev, linuxppc-dev
In-Reply-To: <20070522230942.GT5921@austin.ibm.com>


When entering the netdev poll routine, empty out the RX
chain first, before cleaning up the TX chain. This should
help avoid RX buffer overflows.

Signed-off-by: Linas Vepstas <linas@austin.ibm.com>

----
 drivers/net/spider_net.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Index: netdev-2.6/drivers/net/spider_net.c
===================================================================
--- netdev-2.6.orig/drivers/net/spider_net.c	2007-05-22 18:03:39.000000000 -0500
+++ netdev-2.6/drivers/net/spider_net.c	2007-05-22 18:03:41.000000000 -0500
@@ -1212,7 +1212,6 @@ spider_net_poll(struct net_device *netde
 	int packets_to_do, packets_done = 0;
 	int no_more_packets = 0;
 
-	spider_net_cleanup_tx_ring(card);
 	packets_to_do = min(*budget, netdev->quota);
 
 	while (packets_to_do) {
@@ -1231,6 +1230,8 @@ spider_net_poll(struct net_device *netde
 	spider_net_refill_rx_chain(card);
 	spider_net_enable_rxdmac(card);
 
+	spider_net_cleanup_tx_ring(card);
+
 	/* if all packets are in the stack, enable interrupts and return 0 */
 	/* if not, return 1 */
 	if (no_more_packets) {

^ permalink raw reply

* [PATCH 8/10] spidernet: reset the card when an rxramfull is seen
From: Linas Vepstas @ 2007-05-22 23:45 UTC (permalink / raw)
  To: Jeff Garzik, Andrew Morton
  Cc: netdev, Florin Malita, cbe-oss-dev, linuxppc-dev
In-Reply-To: <20070522230942.GT5921@austin.ibm.com>


Some versions of the spider have a firmware bug, where the
RX ring sequencer goes crazy when the RX RAM on the device
fills up. Appearently the only viable wrkaround is a soft
reset of the card.

Signed-off-by: Linas Vepstas <linas@austin.ibm.com>

----
 drivers/net/spider_net.c |   14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

Index: netdev-2.6/drivers/net/spider_net.c
===================================================================
--- netdev-2.6.orig/drivers/net/spider_net.c	2007-05-22 18:03:37.000000000 -0500
+++ netdev-2.6/drivers/net/spider_net.c	2007-05-22 18:03:39.000000000 -0500
@@ -1506,11 +1506,17 @@ spider_net_handle_error_irq(struct spide
 	case SPIDER_NET_GRFBFLLINT: /* fallthrough */
 	case SPIDER_NET_GRFAFLLINT: /* fallthrough */
 	case SPIDER_NET_GRMFLLINT:
-		if (netif_msg_intr(card) && net_ratelimit())
-			pr_err("%s: Spider RX RAM full, incoming packets "
-			       "might be discarded!\n", card->netdev->name);
+		if (netif_msg_intr(card) && net_ratelimit()) {
+			pr_err("%s: Spider RX RAM full, reseting device.\n",
+			       card->netdev->name);
+			show_rx_chain(card);
+		}
 		spider_net_rx_irq_off(card);
 		netif_rx_schedule(card->netdev);
+
+		/* If the card is spewing rxramfulls, then reset */
+		atomic_inc(&card->tx_timeout_task_counter);
+		schedule_work(&card->tx_timeout_task);
 		show_error = 0;
 		break;
 
@@ -2087,6 +2093,8 @@ spider_net_workaround_rxramfull(struct s
 {
 	int i, sequencer = 0;
 
+	printk(KERN_INFO "%s: calling rxramfull workaround\n", card->netdev->name);
+
 	/* cancel reset */
 	spider_net_write_reg(card, SPIDER_NET_CKRCTRL,
 			     SPIDER_NET_CKRCTRL_RUN_VALUE);

^ permalink raw reply

* [PATCH 7/10] spidernet: enhance the dump routine
From: Linas Vepstas @ 2007-05-22 23:42 UTC (permalink / raw)
  To: Jeff Garzik, Andrew Morton
  Cc: netdev, Florin Malita, cbe-oss-dev, linuxppc-dev
In-Reply-To: <20070522230942.GT5921@austin.ibm.com>


Crazy device problems are hard to debug, when one does not have
good trace info. This patch makes a major enhancement to the
device dump routine.

Signed-off-by: Linas Vepstas <linas@austin.ibm.com>

----
 drivers/net/spider_net.c |   62 ++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 54 insertions(+), 8 deletions(-)

Index: netdev-2.6/drivers/net/spider_net.c
===================================================================
--- netdev-2.6.orig/drivers/net/spider_net.c	2007-05-22 18:03:35.000000000 -0500
+++ netdev-2.6/drivers/net/spider_net.c	2007-05-22 18:03:37.000000000 -0500
@@ -1024,34 +1024,78 @@ spider_net_pass_skb_up(struct spider_net
 	netif_receive_skb(skb);
 }
 
-#ifdef DEBUG
 static void show_rx_chain(struct spider_net_card *card)
 {
 	struct spider_net_descr_chain *chain = &card->rx_chain;
 	struct spider_net_descr *start= chain->tail;
 	struct spider_net_descr *descr= start;
+	struct spider_net_hw_descr *hwd = start->hwdescr;
+	char *iface = card->netdev->name;
+	u32 curr_desc, next_desc;
 	int status;
 
 	int cnt = 0;
-	int cstat = spider_net_get_descr_status(descr);
-	printk(KERN_INFO "RX chain tail at descr=%ld\n",
-	     (start - card->descr) - card->tx_chain.num_desc);
+	int off = 0;
+	int cstat = hwd->dmac_cmd_status;
+
+	printk(KERN_INFO "%s: Total number of descrs=%d\n",
+		iface, chain->num_desc);
+	printk(KERN_INFO "%s: Chain tail located at descr=%d\n",
+		iface, (int) (start - chain->ring));
+
+	curr_desc = spider_net_read_reg(card, SPIDER_NET_GDACTDPA);
+	next_desc = spider_net_read_reg(card, SPIDER_NET_GDACNEXTDA);
+
 	status = cstat;
 	do
 	{
-		status = spider_net_get_descr_status(descr);
+		hwd = descr->hwdescr;
+		off = descr - chain->ring;
+		if (descr==chain->head)
+			printk(KERN_INFO "%s: chain head is at %d\n", iface, off);
+		if (curr_desc == descr->bus_addr)
+			printk(KERN_INFO "%s: hw curr desc is at %d\n", iface, off);
+		if (next_desc == descr->bus_addr)
+			printk(KERN_INFO "%s: hw next desc is at %d\n", iface, off);
+		if (hwd->next_descr_addr == 0)
+			printk(KERN_INFO "%s: chain is cut at %d\n", iface, off);
+		status = hwd->dmac_cmd_status;
 		if (cstat != status) {
-			printk(KERN_INFO "Have %d descrs with stat=x%08x\n", cnt, cstat);
+			printk(KERN_INFO "%s: Have %d descrs with stat=x%08x\n",
+				iface, cnt, cstat);
 			cstat = status;
 			cnt = 0;
 		}
 		cnt ++;
 		descr = descr->next;
 	} while (descr != start);
-	printk(KERN_INFO "Last %d descrs with stat=x%08x\n", cnt, cstat);
-}
+	printk(KERN_INFO "%s: Last %d descrs with stat=x%08x\n",
+		iface, cnt, cstat);
+
+#ifdef DEBUG
+	/* Now dump the whole ring */
+	descr = start;
+	do
+	{
+		struct spider_net_hw_descr *hwd = descr->hwdescr;
+		status = spider_net_get_descr_status(hwd);
+		cnt = descr - chain->ring;
+		printk(KERN_INFO "Descr %d stat=0x%08x skb=%p\n",
+			cnt, status, descr->skb);
+		printk(KERN_INFO "bus addr=%08x buf addr=%08x sz=%d\n",
+			descr->bus_addr, hwd->buf_addr, hwd->buf_size);
+		printk(KERN_INFO "next=%08x result sz=%d valid sz=%d\n",
+			hwd->next_descr_addr, hwd->result_size, hwd->valid_size);
+		printk(KERN_INFO "dmac=%08x data stat=%08x data err=%08x\n",
+			hwd->dmac_cmd_status, hwd->data_status, hwd->data_error);
+		printk(KERN_INFO "\n");
+
+		descr = descr->next;
+	} while (descr != start);
 #endif
 
+}
+
 /**
  * spider_net_decode_one_descr - processes an RX descriptor
  * @card: card structure
@@ -1141,6 +1185,8 @@ spider_net_decode_one_descr(struct spide
 	return 1;
 
 bad_desc:
+	if (netif_msg_rx_err(card))
+		show_rx_chain(card);
 	dev_kfree_skb_irq(descr->skb);
 	descr->skb = NULL;
 	hwdescr->dmac_cmd_status = SPIDER_NET_DESCR_NOT_IN_USE;

^ permalink raw reply

* [PATCH 6/10] spidernet: Don't terminate the RX ring
From: Linas Vepstas @ 2007-05-22 23:40 UTC (permalink / raw)
  To: Jeff Garzik, Andrew Morton
  Cc: netdev, Florin Malita, cbe-oss-dev, linuxppc-dev
In-Reply-To: <20070522230942.GT5921@austin.ibm.com>


There is no real reason to terminate the RX ring; it
doesn't make the operation any smooother, and it does
require an extra sync. So don't do it.

Signed-off-by: Linas Vepstas <linas@austin.ibm.com>

----
 drivers/net/spider_net.c |   18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

Index: netdev-2.6/drivers/net/spider_net.c
===================================================================
--- netdev-2.6.orig/drivers/net/spider_net.c	2007-05-22 18:03:34.000000000 -0500
+++ netdev-2.6/drivers/net/spider_net.c	2007-05-22 18:03:35.000000000 -0500
@@ -462,13 +462,9 @@ spider_net_prepare_rx_descr(struct spide
 		hwdescr->dmac_cmd_status = SPIDER_NET_DESCR_NOT_IN_USE;
 	} else {
 		hwdescr->buf_addr = buf;
-		hwdescr->next_descr_addr = 0;
 		wmb();
 		hwdescr->dmac_cmd_status = SPIDER_NET_DESCR_CARDOWNED |
 					 SPIDER_NET_DMAC_NOINTR_COMPLETE;
-
-		wmb();
-		descr->prev->hwdescr->next_descr_addr = descr->bus_addr;
 	}
 
 	return 0;
@@ -557,12 +553,16 @@ spider_net_refill_rx_chain(struct spider
 static int
 spider_net_alloc_rx_skbs(struct spider_net_card *card)
 {
-	int result;
-	struct spider_net_descr_chain *chain;
+	struct spider_net_descr_chain *chain = &card->rx_chain;
+	struct spider_net_descr *start= chain->tail;
+	struct spider_net_descr *descr = start;
 
-	result = -ENOMEM;
+	/* Link up the hardware chain pointers */
+	do {
+		descr->prev->hwdescr->next_descr_addr = descr->bus_addr;
+		descr = descr->next;
+	} while (descr != start);
 
-	chain = &card->rx_chain;
 	/* Put at least one buffer into the chain. if this fails,
 	 * we've got a problem. If not, spider_net_refill_rx_chain
 	 * will do the rest at the end of this function. */
@@ -579,7 +579,7 @@ spider_net_alloc_rx_skbs(struct spider_n
 
 error:
 	spider_net_free_rx_chain_contents(card);
-	return result;
+	return -ENOMEM;
 }
 
 /**

^ permalink raw reply

* [PATCH 5/10] spidernet: null out skb pointer after its been used.
From: Linas Vepstas @ 2007-05-22 23:39 UTC (permalink / raw)
  To: Jeff Garzik, Andrew Morton
  Cc: netdev, Florin Malita, cbe-oss-dev, linuxppc-dev
In-Reply-To: <20070522230942.GT5921@austin.ibm.com>


If the ethernet interface is brought down while there is still
RX traffic in flight, the device shutdown routine can end up
trying to double-free an skb, leading to a crash in mm/slab.c
Avoid the double-free by nulling out the skb pointer.

Signed-off-by: Linas Vepstas <linas@austin.ibm.com>

----
 drivers/net/spider_net.c |    1 +
 1 file changed, 1 insertion(+)

Index: netdev-2.6/drivers/net/spider_net.c
===================================================================
--- netdev-2.6.orig/drivers/net/spider_net.c	2007-05-22 18:03:32.000000000 -0500
+++ netdev-2.6/drivers/net/spider_net.c	2007-05-22 18:03:34.000000000 -0500
@@ -1136,6 +1136,7 @@ spider_net_decode_one_descr(struct spide
 
 	/* Ok, we've got a packet in descr */
 	spider_net_pass_skb_up(descr, card);
+	descr->skb = NULL;
 	hwdescr->dmac_cmd_status = SPIDER_NET_DESCR_NOT_IN_USE;
 	return 1;
 

^ permalink raw reply

* [PATCH 4/10] spidernet: zero out a pointer.
From: Linas Vepstas @ 2007-05-22 23:36 UTC (permalink / raw)
  To: Jeff Garzik, Andrew Morton
  Cc: netdev, Florin Malita, cbe-oss-dev, linuxppc-dev
In-Reply-To: <20070522230942.GT5921@austin.ibm.com>


Invalidate a pointer as its pci_unmap'ed; this is a bit of 
paranoia to make sure hardware doesn't continue trying to 
DMA to it.

Signed-off-by: Linas Vepstas <linas@austin.ibm.com>

----
 drivers/net/spider_net.c |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Index: netdev-2.6/drivers/net/spider_net.c
===================================================================
--- netdev-2.6.orig/drivers/net/spider_net.c	2007-05-22 18:03:30.000000000 -0500
+++ netdev-2.6/drivers/net/spider_net.c	2007-05-22 18:03:32.000000000 -0500
@@ -1069,6 +1069,7 @@ spider_net_decode_one_descr(struct spide
 	struct spider_net_descr_chain *chain = &card->rx_chain;
 	struct spider_net_descr *descr = chain->tail;
 	struct spider_net_hw_descr *hwdescr = descr->hwdescr;
+	u32 hw_buf_addr;
 	int status;
 
 	status = spider_net_get_descr_status(hwdescr);
@@ -1082,7 +1083,9 @@ spider_net_decode_one_descr(struct spide
 	chain->tail = descr->next;
 
 	/* unmap descriptor */
-	pci_unmap_single(card->pdev, hwdescr->buf_addr,
+	hw_buf_addr = hwdescr->buf_addr;
+	hwdescr->buf_addr = 0x0;
+	pci_unmap_single(card->pdev, hw_buf_addr,
 			SPIDER_NET_MAX_FRAME, PCI_DMA_FROMDEVICE);
 
 	if ( (status == SPIDER_NET_DESCR_RESPONSE_ERROR) ||
@@ -1118,7 +1121,7 @@ spider_net_decode_one_descr(struct spide
 		pr_err("%s: bad status, cmd_status=x%08x\n",
 			       card->netdev->name,
 			       hwdescr->dmac_cmd_status);
-		pr_err("buf_addr=x%08x\n", hwdescr->buf_addr);
+		pr_err("buf_addr=x%08x\n", hw_buf_addr);
 		pr_err("buf_size=x%08x\n", hwdescr->buf_size);
 		pr_err("next_descr_addr=x%08x\n", hwdescr->next_descr_addr);
 		pr_err("result_size=x%08x\n", hwdescr->result_size);

^ permalink raw reply

* Re: [PATCH/RFC] Rework ptep_set_access_flags and fix sun4c
From: Benjamin Herrenschmidt @ 2007-05-22 23:29 UTC (permalink / raw)
  To: David Miller
  Cc: mark, linux-mm, wli, linuxppc-dev, andrea, tcallawa, sparclinux,
	hugh, akpm
In-Reply-To: <20070522.145340.59657045.davem@davemloft.net>

On Tue, 2007-05-22 at 14:53 -0700, David Miller wrote:
> From: Mark Fortescue <mark@mtfhpc.demon.co.uk>
> Date: Tue, 22 May 2007 22:52:13 +0100 (BST)
> 
> > Hi Benjamin,
> > 
> > I have just tested this patch on my Sun4c Sparcstation 1 using my 2.6.20.9 
> > test kernel without any problems.
> > 
> > Thank you for the work.
> 
> Thanks for your testing.
> 
> Someone please merge this in once any remaining issues have
> been resolved :-)

I'll send a patch fixing the couple of x86/ia64 nits & my bad english
spelling (no, I don't happen to raise castrated rams) as soon as I reach
the office later today.

Cheers,
Ben.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox