[PATCH 2/8] ppc/hugetlbfs: Replace ACCESS_ONCE with READ_ONCE

[Christian Borntraeger]

ACCESS_ONCE does not work reliably on non-scalar types. For
example gcc 4.6 and 4.7 might remove the volatile tag for such
accesses during the SRA (scalar replacement of aggregates) step
(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145)

Change the ppc/hugetlbfs code to replace ACCESS_ONCE with READ_ONCE.

Signed-off-by: Christian Borntraeger 
---
 arch/powerpc/mm/hugetlbpage.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/mm/hugetlbpage.c b/arch/powerpc/mm/hugetlbpage.c
index 5ff4e07..620d0ec 100644
--- a/arch/powerpc/mm/hugetlbpage.c
+++ b/arch/powerpc/mm/hugetlbpage.c
@@ -978,7 +978,7 @@ pte_t *find_linux_pte_or_hugepte(pgd_t *pgdir, unsigned 
long ea, unsigned *shift
 */
pdshift = PUD_SHIFT;
pudp = pud_offset(&pgd, ea);
-   pud  = ACCESS_ONCE(*pudp);
+   pud  = READ_ONCE(*pudp);
 
if (pud_none(pud))
return NULL;
@@ -990,7 +990,7 @@ pte_t *find_linux_pte_or_hugepte(pgd_t *pgdir, unsigned 
long ea, unsigned *shift
else {
pdshift = PMD_SHIFT;
pmdp = pmd_offset(&pud, ea);
-   pmd  = ACCESS_ONCE(*pmdp);
+   pmd  = READ_ONCE(*pmdp);
/*
 * A hugepage collapse is captured by pmd_none, because
 * it mark the pmd none and do a hpte invalidate.
-- 
1.9.3

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 1/8] ppc/kvm: Replace ACCESS_ONCE with READ_ONCE

[Christian Borntraeger]

ACCESS_ONCE does not work reliably on non-scalar types. For
example gcc 4.6 and 4.7 might remove the volatile tag for such
accesses during the SRA (scalar replacement of aggregates) step
(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145)

Change the ppc/kvm code to replace ACCESS_ONCE with READ_ONCE.

Signed-off-by: Christian Borntraeger 
---
 arch/powerpc/kvm/book3s_hv_rm_xics.c |  8 
 arch/powerpc/kvm/book3s_xics.c   | 16 
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/arch/powerpc/kvm/book3s_hv_rm_xics.c 
b/arch/powerpc/kvm/book3s_hv_rm_xics.c
index 7b066f6..7c22997 100644
--- a/arch/powerpc/kvm/book3s_hv_rm_xics.c
+++ b/arch/powerpc/kvm/book3s_hv_rm_xics.c
@@ -152,7 +152,7 @@ static void icp_rm_down_cppr(struct kvmppc_xics *xics, 
struct kvmppc_icp *icp,
 * in virtual mode.
 */
do {
-   old_state = new_state = ACCESS_ONCE(icp->state);
+   old_state = new_state = READ_ONCE(icp->state);
 
/* Down_CPPR */
new_state.cppr = new_cppr;
@@ -211,7 +211,7 @@ unsigned long kvmppc_rm_h_xirr(struct kvm_vcpu *vcpu)
 * pending priority
 */
do {
-   old_state = new_state = ACCESS_ONCE(icp->state);
+   old_state = new_state = READ_ONCE(icp->state);
 
xirr = old_state.xisr | (((u32)old_state.cppr) << 24);
if (!old_state.xisr)
@@ -277,7 +277,7 @@ int kvmppc_rm_h_ipi(struct kvm_vcpu *vcpu, unsigned long 
server,
 * whenever the MFRR is made less favored.
 */
do {
-   old_state = new_state = ACCESS_ONCE(icp->state);
+   old_state = new_state = READ_ONCE(icp->state);
 
/* Set_MFRR */
new_state.mfrr = mfrr;
@@ -352,7 +352,7 @@ int kvmppc_rm_h_cppr(struct kvm_vcpu *vcpu, unsigned long 
cppr)
icp_rm_clr_vcpu_irq(icp->vcpu);
 
do {
-   old_state = new_state = ACCESS_ONCE(icp->state);
+   old_state = new_state = READ_ONCE(icp->state);
 
reject = 0;
new_state.cppr = cppr;
diff --git a/arch/powerpc/kvm/book3s_xics.c b/arch/powerpc/kvm/book3s_xics.c
index 807351f..a4a8d9f 100644
--- a/arch/powerpc/kvm/book3s_xics.c
+++ b/arch/powerpc/kvm/book3s_xics.c
@@ -327,7 +327,7 @@ static bool icp_try_to_deliver(struct kvmppc_icp *icp, u32 
irq, u8 priority,
 icp->server_num);
 
do {
-   old_state = new_state = ACCESS_ONCE(icp->state);
+   old_state = new_state = READ_ONCE(icp->state);
 
*reject = 0;
 
@@ -512,7 +512,7 @@ static void icp_down_cppr(struct kvmppc_xics *xics, struct 
kvmppc_icp *icp,
 * in virtual mode.
 */
do {
-   old_state = new_state = ACCESS_ONCE(icp->state);
+   old_state = new_state = READ_ONCE(icp->state);
 
/* Down_CPPR */
new_state.cppr = new_cppr;
@@ -567,7 +567,7 @@ static noinline unsigned long kvmppc_h_xirr(struct kvm_vcpu 
*vcpu)
 * pending priority
 */
do {
-   old_state = new_state = ACCESS_ONCE(icp->state);
+   old_state = new_state = READ_ONCE(icp->state);
 
xirr = old_state.xisr | (((u32)old_state.cppr) << 24);
if (!old_state.xisr)
@@ -634,7 +634,7 @@ static noinline int kvmppc_h_ipi(struct kvm_vcpu *vcpu, 
unsigned long server,
 * whenever the MFRR is made less favored.
 */
do {
-   old_state = new_state = ACCESS_ONCE(icp->state);
+   old_state = new_state = READ_ONCE(icp->state);
 
/* Set_MFRR */
new_state.mfrr = mfrr;
@@ -679,7 +679,7 @@ static int kvmppc_h_ipoll(struct kvm_vcpu *vcpu, unsigned 
long server)
if (!icp)
return H_PARAMETER;
}
-   state = ACCESS_ONCE(icp->state);
+   state = READ_ONCE(icp->state);
kvmppc_set_gpr(vcpu, 4, ((u32)state.cppr << 24) | state.xisr);
kvmppc_set_gpr(vcpu, 5, state.mfrr);
return H_SUCCESS;
@@ -721,7 +721,7 @@ static noinline void kvmppc_h_cppr(struct kvm_vcpu *vcpu, 
unsigned long cppr)
  BOOK3S_INTERRUPT_EXTERNAL_LEVEL);
 
do {
-   old_state = new_state = ACCESS_ONCE(icp->state);
+   old_state = new_state = READ_ONCE(icp->state);
 
reject = 0;
new_state.cppr = cppr;
@@ -885,7 +885,7 @@ static int xics_debug_show(struct seq_file *m, void 
*private)
if (!icp)
continue;
 
-   state.raw = ACCESS_ONCE(icp->state.raw);
+   state.raw = READ_ONCE(icp->state.raw);
seq_printf(m, "cpu server %#lx XIRR:%#x PPRI:%#x CPPR:%#x 
MFRR:%#x OUT:%d NR:%d\n",
   icp->server_num, state.xisr,
   state.pending_pri, state.cppr, state.mfrr,

[PATCH 0/8] current ACCESS_ONCE patch queue

[Christian Borntraeger]

Folks,

fyi, this is my current patch queue for the next merge window. It
does contain a patch that will disallow ACCESS_ONCE on non-scalar
types.

The tree is part of linux-next and can be found at
git://git.kernel.org/pub/scm/linux/kernel/git/borntraeger/linux.git linux-next


Christian Borntraeger (7):
  ppc/kvm: Replace ACCESS_ONCE with READ_ONCE
  ppc/hugetlbfs: Replace ACCESS_ONCE with READ_ONCE
  x86/xen/p2m: Replace ACCESS_ONCE with READ_ONCE
  x86/spinlock: Leftover conversion ACCESS_ONCE->READ_ONCE
  mm/gup: Replace ACCESS_ONCE with READ_ONCE
  kernel: tighten rules for ACCESS ONCE
  kernel: Fix sparse warning for ACCESS_ONCE

Guenter Roeck (1):
  next: sh: Fix compile error

 arch/powerpc/kvm/book3s_hv_rm_xics.c |  8 
 arch/powerpc/kvm/book3s_xics.c   | 16 
 arch/powerpc/mm/hugetlbpage.c|  4 ++--
 arch/sh/mm/gup.c |  2 +-
 arch/x86/include/asm/spinlock.h  |  2 +-
 arch/x86/xen/p2m.c   |  2 +-
 include/linux/compiler.h | 21 -
 mm/gup.c |  2 +-
 8 files changed, 34 insertions(+), 23 deletions(-)

-- 
1.9.3

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 5/8] mm/gup: Replace ACCESS_ONCE with READ_ONCE

[Christian Borntraeger]

ACCESS_ONCE does not work reliably on non-scalar types. For
example gcc 4.6 and 4.7 might remove the volatile tag for such
accesses during the SRA (scalar replacement of aggregates) step
(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145)

Fixup gup_pmd_range.

Signed-off-by: Christian Borntraeger 
---
 mm/gup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/gup.c b/mm/gup.c
index a900759..bed30efa 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -926,7 +926,7 @@ static int gup_pmd_range(pud_t pud, unsigned long addr, 
unsigned long end,
 
pmdp = pmd_offset(&pud, addr);
do {
-   pmd_t pmd = ACCESS_ONCE(*pmdp);
+   pmd_t pmd = READ_ONCE(*pmdp);
 
next = pmd_addr_end(addr, end);
if (pmd_none(pmd) || pmd_trans_splitting(pmd))
-- 
1.9.3

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 4/8] x86/spinlock: Leftover conversion ACCESS_ONCE->READ_ONCE

[Christian Borntraeger]

commit 78bff1c8684f ("x86/ticketlock: Fix spin_unlock_wait() livelock")
introduced another ACCESS_ONCE case in x86 spinlock.h.

Change that as well.

Signed-off-by: Christian Borntraeger 
Cc: Oleg Nesterov 
---
 arch/x86/include/asm/spinlock.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/spinlock.h b/arch/x86/include/asm/spinlock.h
index 625660f..9264f0f 100644
--- a/arch/x86/include/asm/spinlock.h
+++ b/arch/x86/include/asm/spinlock.h
@@ -186,7 +186,7 @@ static inline void arch_spin_unlock_wait(arch_spinlock_t 
*lock)
__ticket_t head = ACCESS_ONCE(lock->tickets.head);
 
for (;;) {
-   struct __raw_tickets tmp = ACCESS_ONCE(lock->tickets);
+   struct __raw_tickets tmp = READ_ONCE(lock->tickets);
/*
 * We need to check "unlocked" in a loop, tmp.head == head
 * can be false positive because of overflow.
-- 
1.9.3

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 3/8] x86/xen/p2m: Replace ACCESS_ONCE with READ_ONCE

[Christian Borntraeger]

ACCESS_ONCE does not work reliably on non-scalar types. For
example gcc 4.6 and 4.7 might remove the volatile tag for such
accesses during the SRA (scalar replacement of aggregates) step
(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145)

Change the p2m code to replace ACCESS_ONCE with READ_ONCE.

Signed-off-by: Christian Borntraeger 
---
 arch/x86/xen/p2m.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/xen/p2m.c b/arch/x86/xen/p2m.c
index edbc7a6..cb71016 100644
--- a/arch/x86/xen/p2m.c
+++ b/arch/x86/xen/p2m.c
@@ -554,7 +554,7 @@ static bool alloc_p2m(unsigned long pfn)
mid_mfn = NULL;
}
 
-   p2m_pfn = pte_pfn(ACCESS_ONCE(*ptep));
+   p2m_pfn = pte_pfn(READ_ONCE(*ptep));
if (p2m_pfn == PFN_DOWN(__pa(p2m_identity)) ||
p2m_pfn == PFN_DOWN(__pa(p2m_missing))) {
/* p2m leaf page is missing */
-- 
1.9.3

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 6/8] kernel: tighten rules for ACCESS ONCE

[Christian Borntraeger]

Now that all non-scalar users of ACCESS_ONCE have been converted
to READ_ONCE or ASSIGN once, lets tighten ACCESS_ONCE to only
work on scalar types.
This variant was proposed by Alexei Starovoitov.

Signed-off-by: Christian Borntraeger 
Reviewed-by: Paul E. McKenney 
---
 include/linux/compiler.h | 21 -
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index a1c81f8..5e186bf 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -447,12 +447,23 @@ static __always_inline void __assign_once_size(volatile 
void *p, void *res, int
  * to make the compiler aware of ordering is to put the two invocations of
  * ACCESS_ONCE() in different C statements.
  *
- * This macro does absolutely -nothing- to prevent the CPU from reordering,
- * merging, or refetching absolutely anything at any time.  Its main intended
- * use is to mediate communication between process-level code and irq/NMI
- * handlers, all running on the same CPU.
+ * ACCESS_ONCE will only work on scalar types. For union types, ACCESS_ONCE
+ * on a union member will work as long as the size of the member matches the
+ * size of the union and the size is smaller than word size.
+ *
+ * The major use cases of ACCESS_ONCE used to be (1) Mediating communication
+ * between process-level code and irq/NMI handlers, all running on the same 
CPU,
+ * and (2) Ensuring that the compiler does not  fold, spindle, or otherwise
+ * mutilate accesses that either do not require ordering or that interact
+ * with an explicit memory barrier or atomic instruction that provides the
+ * required ordering.
+ *
+ * If possible use READ_ONCE/ASSIGN_ONCE instead.
  */
-#define ACCESS_ONCE(x) (*(volatile typeof(x) *)&(x))
+#define __ACCESS_ONCE(x) ({ \
+__maybe_unused typeof(x) __var = 0; \
+   (volatile typeof(x) *)&(x); })
+#define ACCESS_ONCE(x) (*__ACCESS_ONCE(x))
 
 /* Ignore/forbid kprobes attach on very low level functions marked by this 
attribute: */
 #ifdef CONFIG_KPROBES
-- 
1.9.3

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 8/8] kernel: Fix sparse warning for ACCESS_ONCE

[Christian Borntraeger]

Commit a91ed664749c ("kernel: tighten rules for ACCESS ONCE") results in
sparse warnings like "Using plain integer as NULL pointer" - Let's add a
type cast to the dummy assignment.
To avoid warnings lik "sparse: warning: cast to restricted __hc32" we also
use __force on that cast.

Fixes: a91ed664749c ("kernel: tighten rules for ACCESS ONCE")
Signed-off-by: Christian Borntraeger 
---
 include/linux/compiler.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 5e186bf..7bebf05 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -461,7 +461,7 @@ static __always_inline void __assign_once_size(volatile 
void *p, void *res, int
  * If possible use READ_ONCE/ASSIGN_ONCE instead.
  */
 #define __ACCESS_ONCE(x) ({ \
-__maybe_unused typeof(x) __var = 0; \
+__maybe_unused typeof(x) __var = (__force typeof(x)) 0; \
(volatile typeof(x) *)&(x); })
 #define ACCESS_ONCE(x) (*__ACCESS_ONCE(x))
 
-- 
1.9.3

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 7/8] next: sh: Fix compile error

[Christian Borntraeger]

From: Guenter Roeck 

Commit a91ed664749c ("kernel: tighten rules for ACCESS ONCE") results in a
compile failure for sh builds with CONFIG_X2TLB enabled.

arch/sh/mm/gup.c: In function 'gup_get_pte':
arch/sh/mm/gup.c:20:2: error: invalid initializer
make[1]: *** [arch/sh/mm/gup.o] Error 1

Replace ACCESS_ONCE with READ_ONCE to fix the problem.

Fixes: a91ed664749c ("kernel: tighten rules for ACCESS ONCE")
Cc: Paul E. McKenney 
Cc: Christian Borntraeger 
Signed-off-by: Guenter Roeck 
Reviewed-by: Paul E. McKenney 
Signed-off-by: Christian Borntraeger 
---
 arch/sh/mm/gup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/sh/mm/gup.c b/arch/sh/mm/gup.c
index 37458f3..e113bb4 100644
--- a/arch/sh/mm/gup.c
+++ b/arch/sh/mm/gup.c
@@ -17,7 +17,7 @@
 static inline pte_t gup_get_pte(pte_t *ptep)
 {
 #ifndef CONFIG_X2TLB
-   return ACCESS_ONCE(*ptep);
+   return READ_ONCE(*ptep);
 #else
/*
 * With get_user_pages_fast, we walk down the pagetables without
-- 
1.9.3

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 3/8] x86/xen/p2m: Replace ACCESS_ONCE with READ_ONCE

[Jürgen Groß]

On 01/15/2015 09:58 AM, Christian Borntraeger wrote:

ACCESS_ONCE does not work reliably on non-scalar types. For
example gcc 4.6 and 4.7 might remove the volatile tag for such
accesses during the SRA (scalar replacement of aggregates) step
(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145)

Change the p2m code to replace ACCESS_ONCE with READ_ONCE.

Signed-off-by: Christian Borntraeger 


Reviewed-by: Juergen Gross 


---
  arch/x86/xen/p2m.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/xen/p2m.c b/arch/x86/xen/p2m.c
index edbc7a6..cb71016 100644
--- a/arch/x86/xen/p2m.c
+++ b/arch/x86/xen/p2m.c
@@ -554,7 +554,7 @@ static bool alloc_p2m(unsigned long pfn)
mid_mfn = NULL;
}

-   p2m_pfn = pte_pfn(ACCESS_ONCE(*ptep));
+   p2m_pfn = pte_pfn(READ_ONCE(*ptep));
if (p2m_pfn == PFN_DOWN(__pa(p2m_identity)) ||
p2m_pfn == PFN_DOWN(__pa(p2m_missing))) {
/* p2m leaf page is missing */



___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: linux-next: Tree for Jan 12 (build failures: m68k, ppc)

[Geert Uytterhoeven]

On Mon, Jan 12, 2015 at 6:59 PM, Guenter Roeck  wrote:
>> > Build failures, seen since next-20150109:
>> > m68k:allmodconfig
>> > powerpc:ppc6xx_defconfig

It looks like parisc is also suffering:
http://kisskb.ellerman.id.au/kisskb/buildresult/12343847/

>> > Due to:
>> > ERROR: "__get_user_bad" [drivers/gpu/drm/drm.ko] undefined!
>> > make[1]: *** [__modpost] Error 1
>> >
>> > Caused by commit d34f20d6e2f (drm: Atomic modeset ioctl).
>>
>> Yeah, it needs a get_user() that supports 64-bit data.
>>
> Hi Geert,
>
> I assume you mean m68k, where 64 bit support for get_user has been disabled.
>
> The problem on powerpc is different though: __get_user_nocheck()
> and __get_user_check() use
> unsigned long __gu_val;
> followed by
> __get_user_size(__gu_val, __gu_addr, (size), __gu_err);
>
> __get_user_size() fails in
> if (size > sizeof(x))
>  (x) = __get_user_bad();
>
> Presumably "unsigned long" is 32 bit on 32 bit powerpc, not 64 bit.
>
> Overall, the explicit 64-bit use of get_user() seems to be quite unusual.

I noticed you've sent a fix for DRM.

Doh, and I was just fixing m68k...

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [Xen-devel] [PATCH 3/8] x86/xen/p2m: Replace ACCESS_ONCE with READ_ONCE

[David Vrabel]

On 15/01/15 08:58, Christian Borntraeger wrote:
> ACCESS_ONCE does not work reliably on non-scalar types. For
> example gcc 4.6 and 4.7 might remove the volatile tag for such
> accesses during the SRA (scalar replacement of aggregates) step
> (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145)
> 
> Change the p2m code to replace ACCESS_ONCE with READ_ONCE.

Acked-by: David Vrabel 

Let me know if you want me to merge this via the Xen tree.

David
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [Xen-devel] [PATCH 3/8] x86/xen/p2m: Replace ACCESS_ONCE with READ_ONCE

[Christian Borntraeger]

Am 15.01.2015 um 11:43 schrieb David Vrabel:
> On 15/01/15 08:58, Christian Borntraeger wrote:
>> ACCESS_ONCE does not work reliably on non-scalar types. For
>> example gcc 4.6 and 4.7 might remove the volatile tag for such
>> accesses during the SRA (scalar replacement of aggregates) step
>> (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145)
>>
>> Change the p2m code to replace ACCESS_ONCE with READ_ONCE.
> 
> Acked-by: David Vrabel 

Thanks

> Let me know if you want me to merge this via the Xen tree.


With your Acked-by, I can easily carry this in my tree. We can 
then ensure that this patch is merged before the ACCESS_ONCE is
made stricter - making bisecting easier.

Christian

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: offlining cpus breakage

[Preeti U Murthy]

Hi Alexey,

Can you let me know if the following patch fixes the issue for you ?
It did for us on one of our machines that we were investigating on.

Anton,
Can you let me know if the patch fixes the odd perf report that you observed?
This is the latest fix that there is to it.

---START PATCH--

tick/broadcast-hrtimer : Make movement of broadcast hrtimer robust against cpu 
offline

From: Preeti U Murthy 

When a cpu on which the broadcast hrtimer is queued goes offline, the hrtimer
needs to be moved to another cpu. There maybe potential race conditions here,
which can lead to the hrtimer not being queued on any cpu.

There was a report on softlockups, with cpus being stuck in deep idle states,
when smt mode switching was being done, especially on systems with smaller 
number of
cpus [1]. This is hard to reproduce on a large machine because the chances that 
an
offline cpu was the stand by cpu handling broadcast become lesser. Given the
current code, the only situation that looks capable of triggering scenarios 
where
broadcast IPIs are not delivered, is in the cpu offline path. I am
at a loss to pin point the precise scenario.

Therefore to avoid any possible race condition between cpu offline and
movement of the broadcast hrtimer, this patch ensures that the act of keeping
track of broadcasting wakeup IPIs is started afresh after a cpu offline 
operation.
This is done by reseting the expiry time of the hrtimer to a max value. This
has to be done in the CPU_DYING phase so that it is visible to all cpus right
after exiting stop_machine.

The rationale is that during cpu offline, since all cpus are woken up anyway
to run stop_machine, we are only required to keep track of broadcasting to cpus
that henceforth enter deep idle states. This ensures that the broadcast hrtimer
gets positively queued on a cpu as long as there are cpus in deep idle states.

It has to be noted that this code is in addition to retaining the logic that we
have today in the broadcast code on an offline operation. If this logic fails to
move the broadcast hrtimer due to a race condition we have the following patch 
to
handle it right.

[1]http://linuxppc.10917.n7.nabble.com/offlining-cpus-breakage-td88619.html
There is no issue in programming the decrementer as was presumed and stated in
this link.

Signed-off-by: Preeti U Murthy 
---
 kernel/time/clockevents.c|2 +-
 kernel/time/tick-broadcast.c |1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/kernel/time/clockevents.c b/kernel/time/clockevents.c
index 5544990..f3907c9 100644
--- a/kernel/time/clockevents.c
+++ b/kernel/time/clockevents.c
@@ -568,6 +568,7 @@ int clockevents_notify(unsigned long reason, void *arg)
 
case CLOCK_EVT_NOTIFY_CPU_DYING:
tick_handover_do_timer(arg);
+   tick_shutdown_broadcast_oneshot(arg);
break;
 
case CLOCK_EVT_NOTIFY_SUSPEND:
@@ -580,7 +581,6 @@ int clockevents_notify(unsigned long reason, void *arg)
break;
 
case CLOCK_EVT_NOTIFY_CPU_DEAD:
-   tick_shutdown_broadcast_oneshot(arg);
tick_shutdown_broadcast(arg);
tick_shutdown(arg);
/*
diff --git a/kernel/time/tick-broadcast.c b/kernel/time/tick-broadcast.c
index 066f0ec..1f5eda6 100644
--- a/kernel/time/tick-broadcast.c
+++ b/kernel/time/tick-broadcast.c
@@ -677,6 +677,7 @@ static void broadcast_move_bc(int deadcpu)
return;
/* This moves the broadcast assignment to this cpu */
clockevents_program_event(bc, bc->next_event, 1);
+   bc->next_event.tv64 = KTIME_MAX;
 }
 
 /*

---END PATCH-
Thanks

Regards
Preeti U Murthy

On 01/07/2015 03:07 PM, Alexey Kardashevskiy wrote:
> Hi!
> 
> "ppc64_cpu --smt=off" produces multiple error on the latest upstream kernel
> (sha1 bdec419):
> 
> NMI watchdog: BUG: soft lockup - CPU#20 stuck for 23s! [swapper/20:0]
> 
> or
> 
> INFO: rcu_sched detected stalls on CPUs/tasks: { 2 7 8 9 10 11 12 13 14 15
> 16 17 18 19 20 21 22 23 2
> 4 25 26 27 28 29 30 31} (detected by 6, t=2102 jiffies, g=1617, c=1616,
> q=1441)
> 
> and many others, all about lockups
> 
> I did bisecting and found out that reverting these helps:
> 
> 77b54e9f213f76a23736940cf94bcd765fc00f40 powernv/powerpc: Add winkle
> support for offline cpus
> 7cba160ad789a3ad7e68b92bf20eaad6ed171f80 powernv/cpuidle: Redesign idle
> states management
> 8eb8ac89a364305d05ad16be983b7890eb462cc3 powerpc/powernv: Enable Offline
> CPUs to enter deep idle states
> 
> btw reverting just two of them produces a compile error.
> 
> It is pseries_le_defconfig, POWER8 machine:
> timebase: 51200
> platform: PowerNV
> model   : palmetto
> machine : PowerNV palmetto
> firmware: OPAL v3
> 
> 
> Please help to fix it. Thanks.
> 
> 

___

Re: linux-next: Tree for Jan 12 (build failures: m68k, ppc)

[Guenter Roeck]

On 01/15/2015 02:12 AM, Geert Uytterhoeven wrote:

On Mon, Jan 12, 2015 at 6:59 PM, Guenter Roeck  wrote:

Build failures, seen since next-20150109:
 m68k:allmodconfig
 powerpc:ppc6xx_defconfig


It looks like parisc is also suffering:
http://kisskb.ellerman.id.au/kisskb/buildresult/12343847/


Due to:
 ERROR: "__get_user_bad" [drivers/gpu/drm/drm.ko] undefined!
 make[1]: *** [__modpost] Error 1

Caused by commit d34f20d6e2f (drm: Atomic modeset ioctl).


Yeah, it needs a get_user() that supports 64-bit data.


Hi Geert,

I assume you mean m68k, where 64 bit support for get_user has been disabled.

The problem on powerpc is different though: __get_user_nocheck()
and __get_user_check() use
 unsigned long __gu_val;
followed by
 __get_user_size(__gu_val, __gu_addr, (size), __gu_err);

__get_user_size() fails in
 if (size > sizeof(x))
  (x) = __get_user_bad();

Presumably "unsigned long" is 32 bit on 32 bit powerpc, not 64 bit.

Overall, the explicit 64-bit use of get_user() seems to be quite unusual.


I noticed you've sent a fix for DRM.

Doh, and I was just fixing m68k...



Sorry, should have Cc:'d you.

Guenter

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] pseries/le: Fix endiannes issue in RTAS call from xmon

[Laurent Dufour]

On 15/01/2015 05:25, Michael Ellerman wrote:
> On Mon, 2014-11-24 at 15:07 +0100, Laurent Dufour wrote:
>> On pseries system (LPAR) xmon failed to enter when running in LE mode, system
>> is hunging. Inititating xmon will lead to such an output on the console:
>>
>> SysRq : Entering xmon
>> cpu 0x15: Vector: 0  at [c003f39ffb10]
>> pc: c007ed7c: sysrq_handle_xmon+0x5c/0x70
>> lr: c007ed7c: sysrq_handle_xmon+0x5c/0x70
>> sp: c003f39ffc70
>>msr: 80009033
>>   current = 0xc003fafa7180
>>   paca= 0xc7d75e80softe: 0irq_happened: 0x01
>> pid   = 14617, comm = bash
>> Bad kernel stack pointer fafb4b0 at eca7cc4
>> cpu 0x15: Vector: 300 (Data Access) at [c7f07d40]
>> pc: 0eca7cc4
>> lr: 0eca7c44
>> sp: fafb4b0
>>msr: 80001000
>>dar: 1000
>>  dsisr: 4200
>>   current = 0xc003fafa7180
>>   paca= 0xc7d75e80softe: 0irq_happened: 0x01
>> pid   = 14617, comm = bash
>> cpu 0x15: Exception 300 (Data Access) in xmon, returning to main loop
>> xmon: WARNING: bad recursive fault on cpu 0x15
>>
>> The root cause is that xmon is calling RTAS to turn off the surveillance
>> when entering xmon, and RTAS is requiring big endian parameters.
>>
>> This patch is byte swapping the RTAS arguments when running in LE mode.
>>
>> Signed-off-by: Laurent Dufour 
>> ---
>>  arch/powerpc/xmon/xmon.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
>> index b988b5addf86..c8efbb37d6e0 100644
>> --- a/arch/powerpc/xmon/xmon.c
>> +++ b/arch/powerpc/xmon/xmon.c
>> @@ -293,10 +293,10 @@ static inline void disable_surveillance(void)
>>  args.token = rtas_token("set-indicator");
>>  if (args.token == RTAS_UNKNOWN_SERVICE)
>>  return;
> 
> I just noticed we're not handling the token correctly here. It is be32 also.

Ouch, my mistake :(

I will drop a new patch to complete this one.

Cheers.

> cheers
> 
> 

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] fsl/smp: add low power boot support to replace spin boot

[York Sun]

On 01/14/2015 10:05 PM, Dongsheng Wang wrote:
> From: Wang Dongsheng 
> 
> U-boot put non-boot cpus into an low power state(PW10/PW20 or DOZE) when cpu
> powered up. To exit low power state kernel will send DOORBELL or MPIC-IPI
> signal to all those CPUs.
> 
> e500/e500v2 use mpic to send IPI signal.
> e500mc and later use doorbell to send IPI signal.
> 
> This feature tested on:
> POWER UP TEST:
> P1022DS(e500v2),96k times.
> P4080(e500mc),  110k times.
> T1024(e5500),   83k times.
> T4240(e6500),   150k times.
> 
> CPU HOTPLUG TEST:
> P1022DS(e500v2),1.4 million times.
> P4080(e500mc),  1.8 million times.
> T1024(e5500),   1.3 million times.
> T4240(e6500),   1.1 million times.
> 
> Signed-off-by: Wang Dongsheng 
> 

This is a very good move. Can you explain what has been tested for 100K times,
or over a million times?

Have you verified on older e500v2 platforms, such as MPC8572?

York
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH] pseries/le: Fix another endiannes issue in RTAS call from xmon

[Laurent Dufour]

The commit 3b8a3c010969 ("powerpc/pseries: Fix endiannes issue in RTAS
call from xmon") was fixing an endianness issue in the call made from
xmon to RTAS.

However, as Michael Ellerman noticed, this fix was not complete, the
token value was not byte swapped. This lead to call an unexpected and
most of the time unexisting RTAS function, which is silently ignored
by RTAS.

This fix addresses this hole.

Reported-by: Michael Ellerman 
Cc: sta...@vger.kernel.org
Signed-off-by: Laurent Dufour 
---
 arch/powerpc/xmon/xmon.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index 5b150f0c5df9..13c6e200b24e 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -337,6 +337,7 @@ static inline void disable_surveillance(void)
args.token = rtas_token("set-indicator");
if (args.token == RTAS_UNKNOWN_SERVICE)
return;
+   args.token = cpu_to_be32(args.token);
args.nargs = cpu_to_be32(3);
args.nret = cpu_to_be32(1);
args.rets = &args.args[3];
-- 
1.9.1

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 4/8] x86/spinlock: Leftover conversion ACCESS_ONCE->READ_ONCE

[Oleg Nesterov]

On 01/15, Christian Borntraeger wrote:
>
> --- a/arch/x86/include/asm/spinlock.h
> +++ b/arch/x86/include/asm/spinlock.h
> @@ -186,7 +186,7 @@ static inline void arch_spin_unlock_wait(arch_spinlock_t 
> *lock)
>   __ticket_t head = ACCESS_ONCE(lock->tickets.head);
>  
>   for (;;) {
> - struct __raw_tickets tmp = ACCESS_ONCE(lock->tickets);
> + struct __raw_tickets tmp = READ_ONCE(lock->tickets);

Agreed, but what about another ACCESS_ONCE() above?

Oleg.

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] pseries/le: Fix another endiannes issue in RTAS call from xmon

[Tyrel Datwyler]

On 01/15/2015 09:23 AM, Laurent Dufour wrote:
> The commit 3b8a3c010969 ("powerpc/pseries: Fix endiannes issue in RTAS
> call from xmon") was fixing an endianness issue in the call made from
> xmon to RTAS.
> 
> However, as Michael Ellerman noticed, this fix was not complete, the
> token value was not byte swapped. This lead to call an unexpected and
> most of the time unexisting RTAS function, which is silently ignored
> by RTAS.

Nit. Not so much that is silently ignored by RTAS as much as
disable_surveillance silently doesn't check the return status of the
RTAS call. Maybe a check is warranted and reporting of non-success.

-Tyrel

> 
> This fix addresses this hole.
> 
> Reported-by: Michael Ellerman 
> Cc: sta...@vger.kernel.org
> Signed-off-by: Laurent Dufour 
> ---
>  arch/powerpc/xmon/xmon.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> index 5b150f0c5df9..13c6e200b24e 100644
> --- a/arch/powerpc/xmon/xmon.c
> +++ b/arch/powerpc/xmon/xmon.c
> @@ -337,6 +337,7 @@ static inline void disable_surveillance(void)
>   args.token = rtas_token("set-indicator");
>   if (args.token == RTAS_UNKNOWN_SERVICE)
>   return;
> + args.token = cpu_to_be32(args.token);
>   args.nargs = cpu_to_be32(3);
>   args.nret = cpu_to_be32(1);
>   args.rets = &args.args[3];
> 

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 4/8] x86/spinlock: Leftover conversion ACCESS_ONCE->READ_ONCE

[Christian Borntraeger]

Am 15.01.2015 um 20:38 schrieb Oleg Nesterov:
> On 01/15, Christian Borntraeger wrote:
>>
>> --- a/arch/x86/include/asm/spinlock.h
>> +++ b/arch/x86/include/asm/spinlock.h
>> @@ -186,7 +186,7 @@ static inline void arch_spin_unlock_wait(arch_spinlock_t 
>> *lock)
>>  __ticket_t head = ACCESS_ONCE(lock->tickets.head);
>>  
>>  for (;;) {
>> -struct __raw_tickets tmp = ACCESS_ONCE(lock->tickets);
>> +struct __raw_tickets tmp = READ_ONCE(lock->tickets);
> 
> Agreed, but what about another ACCESS_ONCE() above?
> 
> Oleg.
> 

tickets.head is a scalar type, so ACCESS_ONCE does work fine with gcc 4.6/4.7.
My goal was to convert all accesses on non-scalar types as until 
"kernel: tighten rules for ACCESS ONCE" is merged because anything else would be
a Whac-a-mole like adventure (I learned that during the last round in next: all
conversions in this series fix up changes made during this merge window)

We probably going to do a bigger bunch of bulk conversion later on when 
"kernel: tighten rules for ACCESS ONCE" prevents new problems.

Makes sense?

Christian

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 4/8] x86/spinlock: Leftover conversion ACCESS_ONCE->READ_ONCE

[Oleg Nesterov]

On 01/15, Christian Borntraeger wrote:
>
> Am 15.01.2015 um 20:38 schrieb Oleg Nesterov:
> > On 01/15, Christian Borntraeger wrote:
> >>
> >> --- a/arch/x86/include/asm/spinlock.h
> >> +++ b/arch/x86/include/asm/spinlock.h
> >> @@ -186,7 +186,7 @@ static inline void 
> >> arch_spin_unlock_wait(arch_spinlock_t *lock)
> >>__ticket_t head = ACCESS_ONCE(lock->tickets.head);
> >>
> >>for (;;) {
> >> -  struct __raw_tickets tmp = ACCESS_ONCE(lock->tickets);
> >> +  struct __raw_tickets tmp = READ_ONCE(lock->tickets);
> >
> > Agreed, but what about another ACCESS_ONCE() above?
> >
> > Oleg.
>
> tickets.head is a scalar type, so ACCESS_ONCE does work fine with gcc 4.6/4.7.
> My goal was to convert all accesses on non-scalar types

I understand, but READ_ONCE(lock->tickets.head) looks better anyway and
arch_spin_lock() already use READ_ONCE() for this.

So why we should keep the last ACCESS_ONCE() in spinlock.h ? Just to make
another cosmetic cleanup which touches the same function later?

Oleg.

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 4/8] x86/spinlock: Leftover conversion ACCESS_ONCE->READ_ONCE

[Christian Borntraeger]

Am 15.01.2015 um 21:01 schrieb Oleg Nesterov:
> On 01/15, Christian Borntraeger wrote:
>>
>> Am 15.01.2015 um 20:38 schrieb Oleg Nesterov:
>>> On 01/15, Christian Borntraeger wrote:

 --- a/arch/x86/include/asm/spinlock.h
 +++ b/arch/x86/include/asm/spinlock.h
 @@ -186,7 +186,7 @@ static inline void 
 arch_spin_unlock_wait(arch_spinlock_t *lock)
__ticket_t head = ACCESS_ONCE(lock->tickets.head);

for (;;) {
 -  struct __raw_tickets tmp = ACCESS_ONCE(lock->tickets);
 +  struct __raw_tickets tmp = READ_ONCE(lock->tickets);
>>>
>>> Agreed, but what about another ACCESS_ONCE() above?
>>>
>>> Oleg.
>>
>> tickets.head is a scalar type, so ACCESS_ONCE does work fine with gcc 
>> 4.6/4.7.
>> My goal was to convert all accesses on non-scalar types
> 
> I understand, but READ_ONCE(lock->tickets.head) looks better anyway and
> arch_spin_lock() already use READ_ONCE() for this.
> 
> So why we should keep the last ACCESS_ONCE() in spinlock.h ? Just to make
> another cosmetic cleanup which touches the same function later?

OK, I will change that one as well.


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 1/3] CXL: Add image control to sysfs

[Ryan Grimm]

Ian,

Thanks for reviewing!

On 01/14/2015 11:41 PM, Ian Munsie wrote:

Excerpts from Ryan Grimm's message of 2015-01-15 13:56:39 +1100:

Add reset_loads_image and reset_image_select to sysfs.

reset_image_select identifies which image will be loaded to the card on the
next PERST.  Valid entries are: "user" and "factory".

reset_loads_image defines functionality on a PERST.  Value of 0 means PERST
will not cause image load.  A power cycle is required to load the image.  Value
of 1 means PERST will cause image load.

sysfs updates the cxl struct in the driver then calls cxl_update_image_control
to write the vals in the VSEC.


Let's combine both of these into a single sysfs file, with "none",
"user" and "factory" options and have the show & read functions handle
mapping those three options to the two bits in the register.



I like that idea!


Of the two names I'd probably go with reset_image_select.


+What:   /sys/class/cxl//reset_loads_image
+Date:   December 2014
+Contact:linuxppc-dev@lists.ozlabs.org
+Description:read/write
+Value of 0 means PERST will not cause image load.  A power
+cycle is required to load the image.  Value of 1 means PERST
+will cause image load.


It also seems to be that having this disabled also means that PERST
doesn't fully reset the card. Might want to clarify that somewhat and
recommend it only be disabled for debugging purposes (e.g. to retain
the contents of the PSL trace arrays across a reset), and to always
enable it for production.



Yeah, that is the main reason you'd disable it.  Will add that info to 
the doc.



At the moment we don't set it at boot - we just go with whatever the
card is already set to do. I'm thinking it might be a good idea to
always set this bit on boot so the only time it's disabled is if a user
has explicitly gone and disabled it.


+static ssize_t reset_loads_image_show(struct device *device,
+ struct device_attribute *attr,
+ char *buf)
+{
+struct cxl *adapter = to_cxl_adapter(device);
+return sprintf(buf, "%d\n", adapter->perst_loads_image);


We've used scnprintf for the other sysfs reads in this file, why sprintf
here?



No reason, scnprintf is safer so fixed.


+static ssize_t reset_loads_image_store(struct device *device,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+struct cxl *adapter = to_cxl_adapter(device);
+unsigned long val;
+int rc;
+
+if (kstrtoul(buf, 0, &val) < 0)
+return -EINVAL;
+
+adapter->perst_loads_image = !!val;
+if ((rc = cxl_update_image_control(adapter)))
+return rc;


Seems to be some indentation mismatches here - some lines are using
spaces other are using tabs. Please use tabs for everything.



Fixed.


+static ssize_t reset_image_select_store(struct device *device,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+struct cxl *adapter = to_cxl_adapter(device);
+int rc;
+
+if (!strncmp(buf, "user", 4))
+adapter->perst_select_user = true;
+else if (!strncmp(buf, "factory", 7))
+adapter->perst_select_user = false;
+else
+return -EINVAL;


More indentation mismatches here.




Fixed.

-Ryan


Cheers,
-Ian



___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 2/3] CXL: Snoop control

[Ryan Grimm]

On 01/15/2015 12:16 AM, Ian Munsie wrote:

Excerpts from Ryan Grimm's message of 2015-01-15 13:56:40 +1100:

Add mode to opal call.  SNOOP control turns CAPP unit snooping on/off.  This is
needed for the following reset patch, which turns snoops off in the CAPP
recovery path.


Looking at patch 3 in this series I think this description needs to be
updated, as it doesn't seem to turn off snoops?



OK, will make commit message more clear.

Maybe I'll rename the first line to CXL: Enable CAPP recovery, since 
that's what it does.





+/* CAPI modes for PHB */
+enum {
+OPAL_PHB_CAPI_MODE_PCIE = 0,
+OPAL_PHB_CAPI_MODE_CAPI = 1,
+OPAL_PHB_CAPI_MODE_SNOOP_OFF= 2,
+OPAL_PHB_CAPI_MODE_SNOOP_ON = 3,
+};


Spaces have been used for indention here



Fixed.




+/* CAPI feature flags (in device-tree) */
+#define OPAL_PHB_CAPI_FLAG_SNOOP_CONTROL0x0001
+#define OPAL_PHB_CAPI_FLAG_REVERT_TO_PCIE   0x0002


It doesn't look like these are used?


Yeah.





-int pnv_phb_to_cxl(struct pci_dev *dev)
+int pnv_phb_to_cxl(struct pci_dev *dev, uint64_t mode)


Should we rename this function since it no longer just sets the PHB to
CXL mode? Maybe something like pnv_phb_set_cxl_mode?



Good call.  The names are updated in the skiboot code base...missed this 
one.





+if ((rc = pnv_phb_to_cxl(dev, OPAL_PHB_CAPI_MODE_SNOOP_ON))) {
+dev_err(&dev->dev, "enable capp snoops: %i\n", rc);
+}


Ok, we turn on snooping here, but I don't see where we turned it off -
has patch 3 changed so that never happens?



snoops are disabled by Sapphire in the CAPP recovery path.


Also - why this late in in the init sequence? Not saying it's wrong, just
wondering if this has to happen after all the AFUs have been initialised, or if
it can happen earlier in the adapter initialisation, like when we set
the PHB to capi mode?



It has no effect when the driver is bound initially since Sapphire 
already turns on snoops as part of the phb to cxl procedure.


I did it this way to handle CAPP recovery.  The driver doesn't know that 
capp recovery happened...EEH unbinds the driver, Sapphire does the CAPP 
recovery procedure, then EEH rebinds the driver.


The last step in capp recovery is to turn on snoops.  I'll put a comment 
to explain that better...but, sure it could be earlier in the sequence.


-Ryan



Cheers,
-Ian



___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 3/3] CXL: Add reset to sysfs

[Ryan Grimm]

On 01/15/2015 12:42 AM, Ian Munsie wrote:

Excerpts from Ryan Grimm's message of 2015-01-15 13:56:41 +1100:

This allows an image to be downloaded to the flash without rebooting the
machine.  The driver perform a PERST, which results in FPGA image downloaded to
flash and the CAPP unit enters recovery.  CAPP recovery triggers an HMI, which
is handled by EEH in Linux.  EEH removes the driver, calls into Sapphire to
reinitialize the PHB, and then loads the driver.

reset_image_select must be set to "user" and reset_load_image set to 1.  The
driver writes "user" to the vsec if a user image was loaded.  It writes 1 to
reset_load_image on initialization by default.  Other values could be used by
hand for debugging purposes.


That last paragraph will need to be updated if we merge those two sysfs
files into one. Might as well mention an example of why someone might do
a reset with no image selected for reload, e.g. the PSL trace arrays are
preserved, which can be read out through debugfs after the card comes
back up.



OK, fixed that up a bit.  Let me know if the commit logs and 
documentations make sense.  There's a bit of overlap and hopefully it's 
clear now.



+What:   /sys/class/cxl//reset
+Date:   October 2014
+Contact:linuxppc-dev@lists.ozlabs.org
+Description:write only
+Writing 1 here will issue a PERST to card.


"..., which may cause the card to reload the FPGA image depending on the
settings of reset_image_select."




Sure, can be explicit about that.




+if ((rc = pci_set_pcie_reset_state(dev, pcie_warm_reset))) {


Can you add a comment here to explain why we first do a warm reset?



+dev_err(&dev->dev, "cxl: pcie_warm_reset failed\n");
+return rc;
+}
+
+/* Do mmio read to trigger EEH.  Retry for a few seconds. */


This seems a little unusual - can you expand this comment a little to
explain *why* we are using this method to trigger an EEH and reset the
card?



Added better commenting to both above.


+i = 0;
+while ((val = mmio_read32be(adapter->p1_mmio) != 0x) &&
+(i < 5)) {
+msleep(500);
+i++;
+}
+
+if (val != 0x)
+dev_err(&dev->dev, "cxl: PERST failed to trigger EEH\n");
+
+return rc;


Some of the indentation here is a bit funky - some lines are using tabs,
others are using spaces.



Ouch, yep, fixed.




@@ -806,8 +837,8 @@ static int cxl_read_vsec(struct cxl *adapter, struct 
pci_dev *dev)
  CXL_READ_VSEC_BASE_IMAGE(dev, vsec, &adapter->base_image);
  CXL_READ_VSEC_IMAGE_STATE(dev, vsec, &image_state);
  adapter->user_image_loaded = !!(image_state & CXL_VSEC_USER_IMAGE_LOADED);
-adapter->perst_loads_image = !!(image_state & CXL_VSEC_PERST_LOADS_IMAGE);
-adapter->perst_select_user = !!(image_state & CXL_VSEC_PERST_SELECT_USER);
+adapter->perst_loads_image = true;
+adapter->perst_select_user = !!(image_state & CXL_VSEC_USER_IMAGE_LOADED);

...

+if ((rc = cxl_update_image_control(adapter)))
+goto err2;


Thanks - that seems like a better default than what we had before,
should make things more stable :)



Yeah for sure.

-Ryan




Cheers,
-Ian



___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] pseries/le: Fix another endiannes issue in RTAS call from xmon

[Michael Ellerman]

On Thu, 2015-01-15 at 11:44 -0800, Tyrel Datwyler wrote:
> On 01/15/2015 09:23 AM, Laurent Dufour wrote:
> > The commit 3b8a3c010969 ("powerpc/pseries: Fix endiannes issue in RTAS
> > call from xmon") was fixing an endianness issue in the call made from
> > xmon to RTAS.
> > 
> > However, as Michael Ellerman noticed, this fix was not complete, the
> > token value was not byte swapped. This lead to call an unexpected and
> > most of the time unexisting RTAS function, which is silently ignored
> > by RTAS.
> 
> Nit. Not so much that is silently ignored by RTAS as much as
> disable_surveillance silently doesn't check the return status of the
> RTAS call. Maybe a check is warranted and reporting of non-success.

Yeah you're right, I added a printf of the result and got -3, which is also
wrong as far as I can tell, but I didn't have the energy to chase it any
further.

Because this is in xmon we want to be extra careful about what we do, but an
xmon_printf() should be safe. I'll do that as a cleanup after this.

cheers



___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 1/3] CXL: Add image control to sysfs

[Ryan Grimm]

reset_image_select identifies whether a PERST will cause the image to be
flashed to the card.  And if so, which image.

Valid entries are: "none", "user" and "factory".

A value of "none" means PERST will not cause the image to be flashed.  A power
cycle to the pcie slot is required to load the image.

"user" loads the user provided image and "factory" loads the factory image upon
PERST.

sysfs updates the cxl struct in the driver then calls cxl_update_image_control
to write the vals in the VSEC.

Signed-off-by: Ryan Grimm 
---
 Documentation/ABI/testing/sysfs-class-cxl |  9 +++
 drivers/misc/cxl/cxl.h|  1 +
 drivers/misc/cxl/pci.c| 35 +++
 drivers/misc/cxl/sysfs.c  | 39 +++
 4 files changed, 84 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-class-cxl 
b/Documentation/ABI/testing/sysfs-class-cxl
index 554405e..d23d1c7 100644
--- a/Documentation/ABI/testing/sysfs-class-cxl
+++ b/Documentation/ABI/testing/sysfs-class-cxl
@@ -127,3 +127,12 @@ Contact:linuxppc-dev@lists.ozlabs.org
 Description:read only
 Will return "user" or "factory" depending on the image loaded
 onto the card.
+
+What:   /sys/class/cxl//load_image_on_perst
+Date:   December 2014
+Contact:linuxppc-dev@lists.ozlabs.org
+Description:read/write
+Value of "none" means PERST will not cause image to be loaded
+to the card.  A power cycle is required to load the image.  
+Value of "user" and "factory" means PERST will cause either the
+user or factory image to be loaded.
diff --git a/drivers/misc/cxl/cxl.h b/drivers/misc/cxl/cxl.h
index 0df0438..518c4c6 100644
--- a/drivers/misc/cxl/cxl.h
+++ b/drivers/misc/cxl/cxl.h
@@ -488,6 +488,7 @@ void cxl_release_one_irq(struct cxl *adapter, int hwirq);
 int cxl_alloc_irq_ranges(struct cxl_irq_ranges *irqs, struct cxl *adapter, 
unsigned int num);
 void cxl_release_irq_ranges(struct cxl_irq_ranges *irqs, struct cxl *adapter);
 int cxl_setup_irq(struct cxl *adapter, unsigned int hwirq, unsigned int virq);
+int cxl_update_image_control(struct cxl *adapter);
 
 /* common == phyp + powernv */
 struct cxl_process_element_common {
diff --git a/drivers/misc/cxl/pci.c b/drivers/misc/cxl/pci.c
index f801c28..26cacc1 100644
--- a/drivers/misc/cxl/pci.c
+++ b/drivers/misc/cxl/pci.c
@@ -362,6 +362,41 @@ int cxl_setup_irq(struct cxl *adapter, unsigned int hwirq,
return pnv_cxl_ioda_msi_setup(dev, hwirq, virq);
 }
 
+int cxl_update_image_control(struct cxl *adapter)
+{
+   struct pci_dev *dev = to_pci_dev(adapter->dev.parent);
+   int rc;
+   int vsec;
+   u8 image_state;
+
+   if (!(vsec = find_cxl_vsec(dev))) {
+   dev_err(&dev->dev, "ABORTING: CXL VSEC not found!\n");
+   return -ENODEV;
+   }
+
+   if ((rc = CXL_READ_VSEC_IMAGE_STATE(dev, vsec, &image_state))) {
+   dev_err(&dev->dev, "failed to read image state: %i\n", rc);
+   return rc;
+   }
+
+   if (adapter->perst_loads_image)
+   image_state |= CXL_VSEC_PERST_LOADS_IMAGE;
+   else
+   image_state &= ~CXL_VSEC_PERST_LOADS_IMAGE;
+
+   if (adapter->perst_select_user)
+   image_state |= CXL_VSEC_PERST_SELECT_USER;
+   else
+   image_state &= ~CXL_VSEC_PERST_SELECT_USER;
+
+   if ((rc = CXL_WRITE_VSEC_IMAGE_STATE(dev, vsec, image_state))) {
+   dev_err(&dev->dev, "failed to update image control: %i\n", rc);
+   return rc;
+   }
+
+   return 0;
+}
+
 int cxl_alloc_one_irq(struct cxl *adapter)
 {
struct pci_dev *dev = to_pci_dev(adapter->dev.parent);
diff --git a/drivers/misc/cxl/sysfs.c b/drivers/misc/cxl/sysfs.c
index 461bdbd..ed4ad46 100644
--- a/drivers/misc/cxl/sysfs.c
+++ b/drivers/misc/cxl/sysfs.c
@@ -56,11 +56,50 @@ static ssize_t image_loaded_show(struct device *device,
return scnprintf(buf, PAGE_SIZE, "factory\n");
 }
 
+static ssize_t load_image_on_perst_show(struct device *device,
+struct device_attribute *attr,
+char *buf)
+{
+   struct cxl *adapter = to_cxl_adapter(device);
+
+   if (!adapter->perst_loads_image)
+   return scnprintf(buf, PAGE_SIZE, "none\n");
+
+   if (adapter->perst_select_user)
+   return scnprintf(buf, PAGE_SIZE, "user\n");
+   return scnprintf(buf, PAGE_SIZE, "factory\n");
+}
+
+static ssize_t load_image_on_perst_store(struct device *device,
+struct device_attribute *attr,
+const char *buf, size_t count)
+{
+   struct cxl *adapter = to_cxl_adapter(device);
+   int rc;
+
+   if (!strncmp(buf, "none", 4))
+   adapter->perst_loads_image = false;
+   else if (!strncmp(buf, "user", 4)) {

[PATCH 2/3] CXL: Enable CAPP recovery

[Ryan Grimm]

Turning snoops on is the last step in CAPP recovery.  Sapphire is expected to
have reinitialized the PHB and done the previous recovery steps.

Add mode argument to opal call to do this.  Driver can turn snoops off although
it does not currently.

Signed-off-by: Ryan Grimm 
---
 arch/powerpc/include/asm/opal.h   | 8 
 arch/powerpc/include/asm/pnv-pci.h| 2 +-
 arch/powerpc/platforms/powernv/pci-ioda.c | 6 +++---
 drivers/misc/cxl/pci.c| 8 +++-
 4 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
index eb95b67..2baf8a5 100644
--- a/arch/powerpc/include/asm/opal.h
+++ b/arch/powerpc/include/asm/opal.h
@@ -595,6 +595,14 @@ enum {
OPAL_PHB3_NUM_PEST_REGS = 256
 };
 
+/* CAPI modes for PHB */
+enum {
+   OPAL_PHB_CAPI_MODE_PCIE = 0,
+   OPAL_PHB_CAPI_MODE_CAPI = 1,
+   OPAL_PHB_CAPI_MODE_SNOOP_OFF= 2,
+   OPAL_PHB_CAPI_MODE_SNOOP_ON = 3,
+};
+
 struct OpalIoPhbErrorCommon {
__be32 version;
__be32 ioType;
diff --git a/arch/powerpc/include/asm/pnv-pci.h 
b/arch/powerpc/include/asm/pnv-pci.h
index f09a22f..3c00d64 100644
--- a/arch/powerpc/include/asm/pnv-pci.h
+++ b/arch/powerpc/include/asm/pnv-pci.h
@@ -13,7 +13,7 @@
 #include 
 #include 
 
-int pnv_phb_to_cxl(struct pci_dev *dev);
+int pnv_phb_to_cxl_mode(struct pci_dev *dev, uint64_t mode);
 int pnv_cxl_ioda_msi_setup(struct pci_dev *dev, unsigned int hwirq,
   unsigned int virq);
 int pnv_cxl_alloc_hwirqs(struct pci_dev *dev, int num);
diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c 
b/arch/powerpc/platforms/powernv/pci-ioda.c
index fac88ed..5d52d6f 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -1468,7 +1468,7 @@ struct device_node *pnv_pci_to_phb_node(struct pci_dev 
*dev)
 }
 EXPORT_SYMBOL(pnv_pci_to_phb_node);
 
-int pnv_phb_to_cxl(struct pci_dev *dev)
+int pnv_phb_to_cxl_mode(struct pci_dev *dev, uint64_t mode)
 {
struct pci_controller *hose = pci_bus_to_host(dev->bus);
struct pnv_phb *phb = hose->private_data;
@@ -1481,13 +1481,13 @@ int pnv_phb_to_cxl(struct pci_dev *dev)
 
pe_info(pe, "Switching PHB to CXL\n");
 
-   rc = opal_pci_set_phb_cxl_mode(phb->opal_id, 1, pe->pe_number);
+   rc = opal_pci_set_phb_cxl_mode(phb->opal_id, mode, pe->pe_number);
if (rc)
dev_err(&dev->dev, "opal_pci_set_phb_cxl_mode failed: %i\n", 
rc);
 
return rc;
 }
-EXPORT_SYMBOL(pnv_phb_to_cxl);
+EXPORT_SYMBOL(pnv_phb_to_cxl_mode);
 
 /* Find PHB for cxl dev and allocate MSI hwirqs?
  * Returns the absolute hardware IRQ number
diff --git a/drivers/misc/cxl/pci.c b/drivers/misc/cxl/pci.c
index 26cacc1..0f546f6 100644
--- a/drivers/misc/cxl/pci.c
+++ b/drivers/misc/cxl/pci.c
@@ -924,9 +924,15 @@ static struct cxl *cxl_init_adapter(struct pci_dev *dev)
if ((rc = init_implementation_adapter_regs(adapter, dev)))
goto err3;
 
-   if ((rc = pnv_phb_to_cxl(dev)))
+   if ((rc = pnv_phb_to_cxl_mode(dev, OPAL_PHB_CAPI_MODE_CAPI)))
goto err3;
 
+   /* If recovery happened, the last step is to turn on snooping.
+* In the non-recovery case this has no effect */
+   if ((rc = pnv_phb_to_cxl_mode(dev, OPAL_PHB_CAPI_MODE_SNOOP_ON))) {
+   goto err3;
+   }
+
if ((rc = cxl_register_psl_err_irq(adapter)))
goto err3;
 
-- 
1.9.1

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 3/3] CXL: Add ability to reset the card

[Ryan Grimm]

Adds reset to sysfs which will PERST the card.  If load_image_on_perst is set
to "user" or "factory", the PERST will cause that image to be loaded.

load_image_on_perst is set to "user" for production.

"none" could be used for debugging.  The PSL trace arrays are preserved which
then can be read through debugfs.

PERST also triggers CAPP recovery.  An HMI comes in, which is handled by EEH.
EEH unbinds the driver, calls into Sapphire to reinitialize the PHB, then
rebinds the driver.

Signed-off-by: Ryan Grimm 
---
 Documentation/ABI/testing/sysfs-class-cxl | 14 --
 drivers/misc/cxl/cxl.h|  1 +
 drivers/misc/cxl/pci.c| 44 +--
 drivers/misc/cxl/sysfs.c  | 13 +
 4 files changed, 68 insertions(+), 4 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-class-cxl 
b/Documentation/ABI/testing/sysfs-class-cxl
index d23d1c7..043bfbf 100644
--- a/Documentation/ABI/testing/sysfs-class-cxl
+++ b/Documentation/ABI/testing/sysfs-class-cxl
@@ -133,6 +133,16 @@ Date:   December 2014
 Contact:linuxppc-dev@lists.ozlabs.org
 Description:read/write
 Value of "none" means PERST will not cause image to be loaded
-to the card.  A power cycle is required to load the image.  
+to the card.  A power cycle is required to load the image.
+"none" is useful for debugging so the contents of the trace 
+arrays are preserved.
 Value of "user" and "factory" means PERST will cause either the
-user or factory image to be loaded.
+user or factory image to be loaded.  "user" is default and 
+should be used in production.  
+
+What:   /sys/class/cxl//reset
+Date:   October 2014
+Contact:linuxppc-dev@lists.ozlabs.org
+Description:write only
+Writing 1 will issue a PERST to card which may cause the card
+to reload the FPGA depending on load_image_on_perst.
diff --git a/drivers/misc/cxl/cxl.h b/drivers/misc/cxl/cxl.h
index 518c4c6..6a6a487 100644
--- a/drivers/misc/cxl/cxl.h
+++ b/drivers/misc/cxl/cxl.h
@@ -489,6 +489,7 @@ int cxl_alloc_irq_ranges(struct cxl_irq_ranges *irqs, 
struct cxl *adapter, unsig
 void cxl_release_irq_ranges(struct cxl_irq_ranges *irqs, struct cxl *adapter);
 int cxl_setup_irq(struct cxl *adapter, unsigned int hwirq, unsigned int virq);
 int cxl_update_image_control(struct cxl *adapter);
+int cxl_reset(struct cxl *adapter);
 
 /* common == phyp + powernv */
 struct cxl_process_element_common {
diff --git a/drivers/misc/cxl/pci.c b/drivers/misc/cxl/pci.c
index 0f546f6..5137ee5 100644
--- a/drivers/misc/cxl/pci.c
+++ b/drivers/misc/cxl/pci.c
@@ -21,6 +21,7 @@
 #include 
 #include  /* for struct pci_controller */
 #include 
+#include 
 
 #include "cxl.h"
 
@@ -742,6 +743,42 @@ static void cxl_remove_afu(struct cxl_afu *afu)
device_unregister(&afu->dev);
 }
 
+int cxl_reset(struct cxl *adapter)
+{
+   struct pci_dev *dev = to_pci_dev(adapter->dev.parent);
+   int rc;
+   int i;
+   u32 val;
+
+   dev_info(&dev->dev, "CXL reset\n");
+
+   for (i = 0; i < adapter->slices; i++)
+   cxl_remove_afu(adapter->afu[i]);
+
+   /* pcie_warm_reset requests a fundamental pci reset which includes a
+* PERST assert/deassert.  PERST triggers a loading of the image
+* if "user" or "factory" is selected in sysfs */
+   if ((rc = pci_set_pcie_reset_state(dev, pcie_warm_reset))) {
+   dev_err(&dev->dev, "cxl: pcie_warm_reset failed\n");
+   return rc;
+   }
+
+   /* the PERST done above fences the PHB.  So, reset depends on EEH
+* to unbind the driver, tell Sapphire to reinit the PHB, and rebind
+* the driver.  Do an mmio read explictly to ensure EEH notices the
+* fenced PHB.  Retry for a few seconds before giving up. */
+   i = 0;
+   while (((val = mmio_read32be(adapter->p1_mmio)) != 0x) &&
+   (i < 5)) {
+   msleep(500);
+   i++;
+   }
+
+   if (val != 0x)
+   dev_err(&dev->dev, "cxl: PERST failed to trigger EEH\n");
+
+   return rc;
+}
 
 static int cxl_map_adapter_regs(struct cxl *adapter, struct pci_dev *dev)
 {
@@ -806,8 +843,8 @@ static int cxl_read_vsec(struct cxl *adapter, struct 
pci_dev *dev)
CXL_READ_VSEC_BASE_IMAGE(dev, vsec, &adapter->base_image);
CXL_READ_VSEC_IMAGE_STATE(dev, vsec, &image_state);
adapter->user_image_loaded = !!(image_state & 
CXL_VSEC_USER_IMAGE_LOADED);
-   adapter->perst_loads_image = !!(image_state & 
CXL_VSEC_PERST_LOADS_IMAGE);
-   adapter->perst_select_user = !!(image_state & 
CXL_VSEC_PERST_SELECT_USER);
+   adapter->perst_loads_image = true;
+   adapter->perst_select_user = !!(image_state & 
CXL_VSEC_USER_IMAGE_LOADED);
 
CXL_READ

Re: How to make use of SPE instructions?

[Michael Ellerman]

On Thu, 2015-01-08 at 09:58 +, Markus Stockhausen wrote:
> Hello,
> 
> I developed a SHA224/256 kernel crypto module with SPE instructions.
> The result looks quite promising (~ +50% speedup). Nevertheless the
> flooding of kernel messages "SPE used in kernel" makes me feel 
> uncomfortable.
> 
> My findings so far:
> 
> - I can configure the kernel with "SPE support". 
> - arch/powerpc/kernel/head_fsl_booke.S suggests that the message is
>   triggerd unconditionally whenwever we make use of SPE in kernel.
> - There exists a function enable_kernel_spe() but I don't know how
>   this could help me in my work.
> 
> I guess I need some kind of "brackets" around my coding to make sure 
> the upper 32 bit of the registers are stored correctly during task switch. 
> Or is the use of SPE instructions inside the kernel totally forbidden? Any 
> expert with some helpful advise?

I don't know about SPE specifically, hopefully someone from FSL can chime in.

But, IIUIC you should just be able to call enable_kernel_spe() prior to your
code, ie. before you execute any SPE instructions. That will save any values
userspace has left in the SPE regs, and then enable SPE for you in the kernel.
That should get rid of the "SPE used in kernel" messages.  

cheers


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: How to make use of SPE instructions?

[Scott Wood]

On Thu, 2015-01-08 at 09:58 +, Markus Stockhausen wrote:
> Hello,
> 
> I developed a SHA224/256 kernel crypto module with SPE instructions.
> The result looks quite promising (~ +50% speedup). Nevertheless the
> flooding of kernel messages "SPE used in kernel" makes me feel 
> uncomfortable.
> 
> My findings so far:
> 
> - I can configure the kernel with "SPE support". 
> - arch/powerpc/kernel/head_fsl_booke.S suggests that the message is
>   triggerd unconditionally whenwever we make use of SPE in kernel.
> - There exists a function enable_kernel_spe() but I don't know how
>   this could help me in my work.
> 
> I guess I need some kind of "brackets" around my coding to make sure 
> the upper 32 bit of the registers are stored correctly during task switch. 
> Or is the use of SPE instructions inside the kernel totally forbidden? Any 
> expert with some helpful advise?

You need to disable preemption, call enable_kernel_spe(), and finish
using SPE before you enable preemption.  This assumes that SPE is never
used from interrupt context.  Be careful to not disable preemption for
too long.

-Scott


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 1/8] ppc/kvm: Replace ACCESS_ONCE with READ_ONCE

[Michael Ellerman]

On Thu, 2015-01-15 at 09:58 +0100, Christian Borntraeger wrote:
> ACCESS_ONCE does not work reliably on non-scalar types. For
> example gcc 4.6 and 4.7 might remove the volatile tag for such
> accesses during the SRA (scalar replacement of aggregates) step
> (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145)
> 
> Change the ppc/kvm code to replace ACCESS_ONCE with READ_ONCE.
> 
> Signed-off-by: Christian Borntraeger 
> ---
>  arch/powerpc/kvm/book3s_hv_rm_xics.c |  8 
>  arch/powerpc/kvm/book3s_xics.c   | 16 
>  2 files changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/powerpc/kvm/book3s_hv_rm_xics.c 
> b/arch/powerpc/kvm/book3s_hv_rm_xics.c
> index 7b066f6..7c22997 100644
> --- a/arch/powerpc/kvm/book3s_hv_rm_xics.c
> +++ b/arch/powerpc/kvm/book3s_hv_rm_xics.c
> @@ -152,7 +152,7 @@ static void icp_rm_down_cppr(struct kvmppc_xics *xics, 
> struct kvmppc_icp *icp,
>* in virtual mode.
>*/
>   do {
> - old_state = new_state = ACCESS_ONCE(icp->state);
> + old_state = new_state = READ_ONCE(icp->state);

These are all icp->state.

Which is a union, but it's only the size of unsigned long. So in practice there
shouldn't be a bug here right?

cheers


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] pseries/le: Fix another endiannes issue in RTAS call from xmon

[Tyrel Datwyler]

On 01/15/2015 02:19 PM, Michael Ellerman wrote:
> On Thu, 2015-01-15 at 11:44 -0800, Tyrel Datwyler wrote:
>> On 01/15/2015 09:23 AM, Laurent Dufour wrote:
>>> The commit 3b8a3c010969 ("powerpc/pseries: Fix endiannes issue in RTAS
>>> call from xmon") was fixing an endianness issue in the call made from
>>> xmon to RTAS.
>>>
>>> However, as Michael Ellerman noticed, this fix was not complete, the
>>> token value was not byte swapped. This lead to call an unexpected and
>>> most of the time unexisting RTAS function, which is silently ignored
>>> by RTAS.
>>
>> Nit. Not so much that is silently ignored by RTAS as much as
>> disable_surveillance silently doesn't check the return status of the
>> RTAS call. Maybe a check is warranted and reporting of non-success.
> 
> Yeah you're right, I added a printf of the result and got -3, which is also
> wrong as far as I can tell, but I didn't have the energy to chase it any
> further.

If this was on a powerkvm guest set-indicator should be present for
hotplug (DLPAR) support. However, the surveillance indicator would not
be implemented. I know sometimes I forget if I'm on a powervm or
powerkvm guest. Just a thought.

-Tyrel

> 
> Because this is in xmon we want to be extra careful about what we do, but an
> xmon_printf() should be safe. I'll do that as a cleanup after this.
> 
> cheers
> 
> 

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: offlining cpus breakage

[Alexey Kardashevskiy]

On 01/16/2015 02:22 AM, Preeti U Murthy wrote:
> Hi Alexey,
> 
> Can you let me know if the following patch fixes the issue for you ?
> It did for us on one of our machines that we were investigating on.


This fixes the issue for me as well, thanks!

Tested-by: Alexey Kardashevskiy 


> Anton,
> Can you let me know if the patch fixes the odd perf report that you observed?
> This is the latest fix that there is to it.
> 
> ---START 
> PATCH--
> 
> tick/broadcast-hrtimer : Make movement of broadcast hrtimer robust against 
> cpu offline
> 
> From: Preeti U Murthy 
> 
> When a cpu on which the broadcast hrtimer is queued goes offline, the hrtimer
> needs to be moved to another cpu. There maybe potential race conditions here,
> which can lead to the hrtimer not being queued on any cpu.
> 
> There was a report on softlockups, with cpus being stuck in deep idle states,
> when smt mode switching was being done, especially on systems with smaller 
> number of
> cpus [1]. This is hard to reproduce on a large machine because the chances 
> that an
> offline cpu was the stand by cpu handling broadcast become lesser. Given the
> current code, the only situation that looks capable of triggering scenarios 
> where
> broadcast IPIs are not delivered, is in the cpu offline path. I am
> at a loss to pin point the precise scenario.
> 
> Therefore to avoid any possible race condition between cpu offline and
> movement of the broadcast hrtimer, this patch ensures that the act of keeping
> track of broadcasting wakeup IPIs is started afresh after a cpu offline 
> operation.
> This is done by reseting the expiry time of the hrtimer to a max value. This
> has to be done in the CPU_DYING phase so that it is visible to all cpus right
> after exiting stop_machine.
> 
> The rationale is that during cpu offline, since all cpus are woken up anyway
> to run stop_machine, we are only required to keep track of broadcasting to 
> cpus
> that henceforth enter deep idle states. This ensures that the broadcast 
> hrtimer
> gets positively queued on a cpu as long as there are cpus in deep idle states.
> 
> It has to be noted that this code is in addition to retaining the logic that 
> we
> have today in the broadcast code on an offline operation. If this logic fails 
> to
> move the broadcast hrtimer due to a race condition we have the following 
> patch to
> handle it right.
> 
> [1]http://linuxppc.10917.n7.nabble.com/offlining-cpus-breakage-td88619.html
> There is no issue in programming the decrementer as was presumed and stated in
> this link.
> 
> Signed-off-by: Preeti U Murthy 
> ---
>  kernel/time/clockevents.c|2 +-
>  kernel/time/tick-broadcast.c |1 +
>  2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/time/clockevents.c b/kernel/time/clockevents.c
> index 5544990..f3907c9 100644
> --- a/kernel/time/clockevents.c
> +++ b/kernel/time/clockevents.c
> @@ -568,6 +568,7 @@ int clockevents_notify(unsigned long reason, void *arg)
>  
>   case CLOCK_EVT_NOTIFY_CPU_DYING:
>   tick_handover_do_timer(arg);
> + tick_shutdown_broadcast_oneshot(arg);
>   break;
>  
>   case CLOCK_EVT_NOTIFY_SUSPEND:
> @@ -580,7 +581,6 @@ int clockevents_notify(unsigned long reason, void *arg)
>   break;
>  
>   case CLOCK_EVT_NOTIFY_CPU_DEAD:
> - tick_shutdown_broadcast_oneshot(arg);
>   tick_shutdown_broadcast(arg);
>   tick_shutdown(arg);
>   /*
> diff --git a/kernel/time/tick-broadcast.c b/kernel/time/tick-broadcast.c
> index 066f0ec..1f5eda6 100644
> --- a/kernel/time/tick-broadcast.c
> +++ b/kernel/time/tick-broadcast.c
> @@ -677,6 +677,7 @@ static void broadcast_move_bc(int deadcpu)
>   return;
>   /* This moves the broadcast assignment to this cpu */
>   clockevents_program_event(bc, bc->next_event, 1);
> + bc->next_event.tv64 = KTIME_MAX;
>  }
>  
>  /*
> 
> ---END PATCH-
> Thanks
> 
> Regards
> Preeti U Murthy
> 
> On 01/07/2015 03:07 PM, Alexey Kardashevskiy wrote:
>> Hi!
>>
>> "ppc64_cpu --smt=off" produces multiple error on the latest upstream kernel
>> (sha1 bdec419):
>>
>> NMI watchdog: BUG: soft lockup - CPU#20 stuck for 23s! [swapper/20:0]
>>
>> or
>>
>> INFO: rcu_sched detected stalls on CPUs/tasks: { 2 7 8 9 10 11 12 13 14 15
>> 16 17 18 19 20 21 22 23 2
>> 4 25 26 27 28 29 30 31} (detected by 6, t=2102 jiffies, g=1617, c=1616,
>> q=1441)
>>
>> and many others, all about lockups
>>
>> I did bisecting and found out that reverting these helps:
>>
>> 77b54e9f213f76a23736940cf94bcd765fc00f40 powernv/powerpc: Add winkle
>> support for offline cpus
>> 7cba160ad789a3ad7e68b92bf20eaad6ed171f80 powernv/cpuidle: Redesign idle
>> states management
>> 8eb8ac89a364305d05ad16be983b7890eb462cc3 powerpc/powernv: Enable Offline
>> CPUs to enter deep idle states
>>
>> b

Re: [PATCH 13/28] pci: host: drop owner assignment from platform_drivers

[Bjorn Helgaas]

[+cc Julia]

On Sun, Dec 21, 2014 at 10:14:34PM +0100, Wolfram Sang wrote:
> This platform_driver does not need to set an owner, it will be populated by 
> the
> driver core.
> 
> Signed-off-by: Wolfram Sang 

I already applied the equivalent patch from Julia for v3.20, thanks!
(I know this is my fault because it took me so long to apply Julia's
patch.)

> ---
> Generated with coccinelle. SmPL file is in the introductory msg. The big
> cleanup was pulled in this merge window. This series catches the bits fallen
> through. The patches shall go in via the subsystem trees.
> 
>  drivers/pci/host/pci-layerscape.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/pci/host/pci-layerscape.c 
> b/drivers/pci/host/pci-layerscape.c
> index 6697b1a4d4fa..68c9e5e9b0a8 100644
> --- a/drivers/pci/host/pci-layerscape.c
> +++ b/drivers/pci/host/pci-layerscape.c
> @@ -167,7 +167,6 @@ MODULE_DEVICE_TABLE(of, ls_pcie_of_match);
>  static struct platform_driver ls_pcie_driver = {
>   .driver = {
>   .name = "layerscape-pcie",
> - .owner = THIS_MODULE,
>   .of_match_table = ls_pcie_of_match,
>   },
>  };
> -- 
> 2.1.3
> 
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

RE: [PATCH] fsl/smp: add low power boot support to replace spin boot

[dongsheng.w...@freescale.com]

Thanks for your review.

> -Original Message-
> From: Sun York-R58495
> Sent: Friday, January 16, 2015 1:09 AM
> To: Wang Dongsheng-B40534; Wood Scott-B07421; Li Yang-Leo-R58472
> Cc: linuxppc-dev@lists.ozlabs.org
> Subject: Re: [PATCH] fsl/smp: add low power boot support to replace spin boot
> 
> 
> 
> On 01/14/2015 10:05 PM, Dongsheng Wang wrote:
> > From: Wang Dongsheng 
> >
> > U-boot put non-boot cpus into an low power state(PW10/PW20 or DOZE)
> > when cpu powered up. To exit low power state kernel will send DOORBELL
> > or MPIC-IPI signal to all those CPUs.
> >
> > e500/e500v2 use mpic to send IPI signal.
> > e500mc and later use doorbell to send IPI signal.
> >
> > This feature tested on:
> > POWER UP TEST:
> > P1022DS(e500v2),96k times.
> > P4080(e500mc),  110k times.
> > T1024(e5500),   83k times.
> > T4240(e6500),   150k times.
> >
> > CPU HOTPLUG TEST:
> > P1022DS(e500v2),1.4 million times.
> > P4080(e500mc),  1.8 million times.
> > T1024(e5500),   1.3 million times.
> > T4240(e6500),   1.1 million times.
> >
> > Signed-off-by: Wang Dongsheng 
> >
> 
> This is a very good move. Can you explain what has been tested for 100K times,
> or over a million times?
> 
Thanks, I tested linux boot process and cpu hotplug.

For Linux boot process test:
Each CPU can normally be started and random run an application, the application
works well.

For cpu hotplug test:
Just plug out and plug in for each cpu. Cpu hotplug test works well.

If I missed some case please let me know.

> Have you verified on older e500v2 platforms, such as MPC8572?
> 
I haven't tested MPC8572 and Just test P1022DS. Because is e500 and e500v2 use 
the same
Low power boot codes.

I will test this feature on MPC8572 platform. 

Regards,
-Dongsheng
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] pseries/le: Fix another endiannes issue in RTAS call from xmon

[Michael Ellerman]

On Thu, 2015-01-15 at 15:41 -0800, Tyrel Datwyler wrote:
> On 01/15/2015 02:19 PM, Michael Ellerman wrote:
> > On Thu, 2015-01-15 at 11:44 -0800, Tyrel Datwyler wrote:
> >> On 01/15/2015 09:23 AM, Laurent Dufour wrote:
> >>> The commit 3b8a3c010969 ("powerpc/pseries: Fix endiannes issue in RTAS
> >>> call from xmon") was fixing an endianness issue in the call made from
> >>> xmon to RTAS.
> >>>
> >>> However, as Michael Ellerman noticed, this fix was not complete, the
> >>> token value was not byte swapped. This lead to call an unexpected and
> >>> most of the time unexisting RTAS function, which is silently ignored
> >>> by RTAS.
> >>
> >> Nit. Not so much that is silently ignored by RTAS as much as
> >> disable_surveillance silently doesn't check the return status of the
> >> RTAS call. Maybe a check is warranted and reporting of non-success.
> > 
> > Yeah you're right, I added a printf of the result and got -3, which is also
> > wrong as far as I can tell, but I didn't have the energy to chase it any
> > further.
> 
> If this was on a powerkvm guest set-indicator should be present for
> hotplug (DLPAR) support. However, the surveillance indicator would not
> be implemented. I know sometimes I forget if I'm on a powervm or
> powerkvm guest. Just a thought.

Right that does explain it. I went looking for the KVM kernel/qemu code that
implements set-indicator but couldn't find it. Presumably it's in some branch
other than the one I was looking at, or I was grepping for the wrong thing.

So I guess a printf there is probably not helpful, because it will fire always
on PowerKVM (at least at the moment).

cheers


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: offlining cpus breakage

[Michael Ellerman]

On Fri, 2015-01-16 at 13:28 +1300, Alexey Kardashevskiy wrote:
> On 01/16/2015 02:22 AM, Preeti U Murthy wrote:
> > Hi Alexey,
> > 
> > Can you let me know if the following patch fixes the issue for you ?
> > It did for us on one of our machines that we were investigating on.
> 
> This fixes the issue for me as well, thanks!
> 
> Tested-by: Alexey Kardashevskiy   

OK, that's great.

But, I really don't think we can ask upstream to merge this patch to generic
code when we don't have a good explanation for why it's necessary. At least I'm
not going to ask anyone to do that :)

So Pretti can you either write a 100% convincing explanation of why this patch
is correct in the general case, or (preferably) do some more investigating to
work out what Alexey's bug actually is.

cheers


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 1/3] CXL: Add image control to sysfs

[Ian Munsie]

Hi Ryan,

Excerpts from Ryan Grimm's message of 2015-01-16 09:27:15 +1100:
> reset_image_select identifies whether a PERST will cause the image to be

s/reset_image_select/load_image_on_perst/

> +What:   /sys/class/cxl//load_image_on_perst
> +Date:   December 2014
> +Contact:linuxppc-dev@lists.ozlabs.org
> +Description:read/write
> +Value of "none" means PERST will not cause image to be loaded
> +to the card.  A power cycle is required to load the image.  

git am spotted some whitespace at the end of this line.


> +int cxl_update_image_control(struct cxl *adapter)

Would you be able to pull this function out into a separate patch (can
be first in the series so everything still compiles after each patch)
along with the change to the image_state defaults from patch 3? It's a
fix that I think would be good to be backported to stable, which will be
simpler if it's in a separate patch.



Otherwise, looks good :)

Acked-by: Ian Munsie 

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 2/3] CXL: Enable CAPP recovery

[Ian Munsie]

Acked-by: Ian Munsie 

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 3/3] CXL: Add ability to reset the card

[Ian Munsie]

Hi Ryan,

Excerpts from Ryan Grimm's message of 2015-01-16 09:27:17 +1100:
> Adds reset to sysfs which will PERST the card.  If load_image_on_perst is set
> to "user" or "factory", the PERST will cause that image to be loaded.
> 
> load_image_on_perst is set to "user" for production.

While it generally will be "user" for production, some cards may only
ship with only a factory image, which is then going to be the image used
for production.  It might be better to say that it will default to
whichever image the card has already loaded.


>  Value of "none" means PERST will not cause image to be loaded
> -to the card.  A power cycle is required to load the image.  
> +to the card.  A power cycle is required to load the image.
> +"none" is useful for debugging so the contents of the trace 
> +arrays are preserved.
>  Value of "user" and "factory" means PERST will cause either 
> the
> -user or factory image to be loaded.
> +user or factory image to be loaded.  "user" is default and 
> +should be used in production.  

git am spotted some whitespace at the end of a couple of lines here



> +/* pcie_warm_reset requests a fundamental pci reset which includes a
> + * PERST assert/deassert.  PERST triggers a loading of the image
> + * if "user" or "factory" is selected in sysfs */
> +if ((rc = pci_set_pcie_reset_state(dev, pcie_warm_reset))) {
> +dev_err(&dev->dev, "cxl: pcie_warm_reset failed\n");
> +return rc;
> +}
> +
> +/* the PERST done above fences the PHB.  So, reset depends on EEH
> + * to unbind the driver, tell Sapphire to reinit the PHB, and rebind
> + * the driver.  Do an mmio read explictly to ensure EEH notices the
> + * fenced PHB.  Retry for a few seconds before giving up. */

Great, thanks for adding the explanations here :)


> @@ -806,8 +843,8 @@ static int cxl_read_vsec(struct cxl *adapter, struct 
> pci_dev *dev)
>  CXL_READ_VSEC_BASE_IMAGE(dev, vsec, &adapter->base_image);
>  CXL_READ_VSEC_IMAGE_STATE(dev, vsec, &image_state);
>  adapter->user_image_loaded = !!(image_state & 
> CXL_VSEC_USER_IMAGE_LOADED);
> -adapter->perst_loads_image = !!(image_state & 
> CXL_VSEC_PERST_LOADS_IMAGE);
> -adapter->perst_select_user = !!(image_state & 
> CXL_VSEC_PERST_SELECT_USER);
> +adapter->perst_loads_image = true;
> +adapter->perst_select_user = !!(image_state & 
> CXL_VSEC_USER_IMAGE_LOADED);

> +if ((rc = cxl_update_image_control(adapter)))
> +goto err2;

Please move these two hunks into a separate patch (can be first in the
series) along with the cxl_update_image_control() function from patch 1
- I'd like to get this backported to stable, which will be simpler if it
is in it's own patch.


> +static ssize_t reset_adapter_store(struct device *device,
> +   struct device_attribute *attr,
> +   const char *buf, size_t count)
> +{
> +struct cxl *adapter = to_cxl_adapter(device);
> +int rc;
> +
> +if ((rc = cxl_reset(adapter)))
> +return rc;
> +return count;
> +}

Please add a check here so the reset only occurs when a "1" is written
to this file to match the documentation.


Cheers,
-Ian

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

AW: How to make use of SPE instructions?

[Markus Stockhausen]

> Von: Scott Wood [scottw...@freescale.com]
> Gesendet: Donnerstag, 15. Januar 2015 23:56
> An: Markus Stockhausen
> Cc: linuxppc-dev@lists.ozlabs.org
> Betreff: Re: How to make use of SPE instructions?
> 
> On Thu, 2015-01-08 at 09:58 +, Markus Stockhausen wrote:
> > Hello,
> >
> > I developed a SHA224/256 kernel crypto module with SPE instructions.
> > The result looks quite promising (~ +50% speedup). Nevertheless the
> > flooding of kernel messages "SPE used in kernel" makes me feel
> > uncomfortable.
> >
> > My findings so far:
> >
> > - I can configure the kernel with "SPE support".
> > - arch/powerpc/kernel/head_fsl_booke.S suggests that the message is
> >   triggerd unconditionally whenwever we make use of SPE in kernel.
> - There exists a function enable_kernel_spe() but I don't know how
>   this could help me in my work.
> >
> > I guess I need some kind of "brackets" around my coding to make sure
> > the upper 32 bit of the registers are stored correctly during task switch.
> > Or is the use of SPE instructions inside the kernel totally forbidden? Any
> > expert with some helpful advise?
> 
> You need to disable preemption, call enable_kernel_spe(), and finish
> using SPE before you enable preemption.  This assumes that SPE is never
> used from interrupt context.  Be careful to not disable preemption for
> too long.

Thanks for your feedback. That did the trick. I'm currently working on
a (low power) 800 MHz single core P1014 CPU. That should be the
cheapest and slowest hardware that is available with SPE. My target 
is to use the module for calculating hash values of IPsec packets. So 
we are talking about input data of up to ~1500 bytes. 

I did some tests with the tcrypt module and I get a hashing speed of
~ 46MByte/s for 2K data chunks. Stock module gives 29MByte/s. In 
other words ~22,000 hashes per second. Overhead of the tcrypt data 
feeder of around 10% included. That are worst case 46us per hash and 
therefore 46us inside a non preemptive task.

In beetween I spent some time to do the same for SHA-1. There
we have ~46,000 hashes per second or 21us per 2K data. That are
+13% compared to the already available PPC assembler module.

Three questions are left:

- Does the setup conflict with the mentioned interrupt context?
- Is that a reasonable time interval for disabling preemption?
- Should I send the patches to this or to the crypto list?

Markus

Diese E-Mail enthält vertrauliche und/oder rechtlich geschützte
Informationen. Wenn Sie nicht der richtige Adressat sind oder diese E-Mail
irrtümlich erhalten haben, informieren Sie bitte sofort den Absender und
vernichten Sie diese Mail. Das unerlaubte Kopieren sowie die unbefugte
Weitergabe dieser Mail ist nicht gestattet.

Über das Internet versandte E-Mails können unter fremden Namen erstellt oder
manipuliert werden. Deshalb ist diese als E-Mail verschickte Nachricht keine
rechtsverbindliche Willenserklärung.

Collogia
Unternehmensberatung AG
Ubierring 11
D-50678 Köln

Vorstand:
Kadir Akin
Dr. Michael Höhnerbach

Vorsitzender des Aufsichtsrates:
Hans Kristian Langva

Registergericht: Amtsgericht Köln
Registernummer: HRB 52 497

This e-mail may contain confidential and/or privileged information. If you
are not the intended recipient (or have received this e-mail in error)
please notify the sender immediately and destroy this e-mail. Any
unauthorized copying, disclosure or distribution of the material in this
e-mail is strictly forbidden.

e-mails sent over the internet may have been written under a wrong name or
been manipulated. That is why this message sent as an e-mail is not a
legally binding declaration of intention.

Collogia
Unternehmensberatung AG
Ubierring 11
D-50678 Köln

executive board:
Kadir Akin
Dr. Michael Höhnerbach

President of the supervisory board:
Hans Kristian Langva

Registry office: district court Cologne
Register number: HRB 52 497


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [RFC 01/11] i2c: add quirk structure to describe adapter flaws

[Eddie Huang]

Hi Wolfram,

On Fri, 2015-01-09 at 18:21 +0100, Wolfram Sang wrote:
>  
> + */
> +struct i2c_adapter_quirks {
> + u64 flags;
> + int max_num_msgs;
> + u16 max_write_len;
> + u16 max_read_len;
> + u16 max_comb_write_len;
> + u16 max_comb_read_len;
> +};
> +
> +#define I2C_ADAPTER_QUIRK_COMB_WRITE_FIRST   BIT(0)
> +#define I2C_ADAPTER_QUIRK_COMB_READ_SECOND   BIT(1)
> +#define I2C_ADAPTER_QUIRK_COMB_WRITE_THEN_READ   
> (I2C_ADAPTER_QUIRK_COMB_WRITE_FIRST | \
> + 
> I2C_ADAPTER_QUIRK_COMB_READ_SECOND)
> +
>  /*
>   * i2c_adapter is the structure used to identify a physical i2c bus along
>   * with the access algorithms necessary to access it.
> @@ -472,6 +506,7 @@ struct i2c_adapter {
>   struct list_head userspace_clients;
>  
>   struct i2c_bus_recovery_info *bus_recovery_info;
> + struct i2c_adapter_quirks *quirks;
>  };
>  #define to_i2c_adapter(d) container_of(d, struct i2c_adapter, dev)
>  

I suggest to add const.
const struct i2c_adapter_quirks *quirks;

also, in i2c-core.c, should modify:
const struct i2c_adapter_quirks *q = adap->quirks;



___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev