Re: [PATCH] bpf: fix overflow of bpf_jit_limit when PAGE_SIZE >= 64K

[Michael Ellerman]

Michael Roth  writes:

> Commit ede95a63b5 introduced a bpf_jit_limit tuneable to limit BPF
> JIT allocations. At compile time it defaults to PAGE_SIZE * 4,
> and is adjusted again at init time if MODULES_VADDR is defined.
>
> For ppc64 kernels, MODULES_VADDR isn't defined, so we're stuck with

But maybe it should be, I don't know why we don't define it.

> the compile-time default at boot-time, which is 0x9c40 when
> using 64K page size. This overflows the signed 32-bit bpf_jit_limit
> value:
>
>   root@ubuntu:/tmp# cat /proc/sys/net/core/bpf_jit_limit
>   -1673527296
>
> and can cause various unexpected failures throughout the network
> stack. In one case `strace dhclient eth0` reported:
>
>   setsockopt(5, SOL_SOCKET, SO_ATTACH_FILTER, {len=11, filter=0x105dd27f8}, 
> 16) = -1 ENOTSUPP (Unknown error 524)
>
> and similar failures can be seen with tools like tcpdump. This doesn't
> always reproduce however, and I'm not sure why. The more consistent
> failure I've seen is an Ubuntu 18.04 KVM guest booted on a POWER9 host
> would time out on systemd/netplan configuring a virtio-net NIC with no
> noticeable errors in the logs.
>
> Fix this by limiting the compile-time default for bpf_jit_limit to
> INT_MAX.

INT_MAX is a lot more than (4k * 4), so I guess I'm not clear on
whether we should be using PAGE_SIZE here at all. I guess each BPF
program uses at least one page is the thinking?

Thanks for tracking this down. For some reason none of my ~10 test boxes
have hit this, perhaps I don't have new enough userspace?

You don't mention why you needed to add BPF_MIN(), I assume because the
kernel version of min() has gotten too complicated to work here?

Daniel I assume you'll merge this via your tree?

cheers

> Fixes: ede95a63b5e8 ("bpf: add bpf_jit_limit knob to restrict unpriv 
> allocations")
> Cc: linuxppc-...@ozlabs.org
> Cc: Daniel Borkmann 
> Cc: Sandipan Das 
> Cc: Alexei Starovoitov 
> Signed-off-by: Michael Roth 
> ---
>  kernel/bpf/core.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
> index b1a3545d0ec8..55de4746cdfd 100644
> --- a/kernel/bpf/core.c
> +++ b/kernel/bpf/core.c
> @@ -365,7 +365,8 @@ void bpf_prog_kallsyms_del_all(struct bpf_prog *fp)
>  }
>  
>  #ifdef CONFIG_BPF_JIT
> -# define BPF_JIT_LIMIT_DEFAULT   (PAGE_SIZE * 4)
> +# define BPF_MIN(x, y) ((x) < (y) ? (x) : (y))
> +# define BPF_JIT_LIMIT_DEFAULT   BPF_MIN((PAGE_SIZE * 4), INT_MAX)
>  
>  /* All BPF JIT sysctl knobs here. */
>  int bpf_jit_enable   __read_mostly = IS_BUILTIN(CONFIG_BPF_JIT_ALWAYS_ON);
> -- 
> 2.17.1

Re: [RFC][PATCH bpf v2 1/2] bpf: allow 64-bit offsets for bpf function calls

[Michael Ellerman]

"Naveen N. Rao"  writes:
> Daniel Borkmann wrote:
>> On 02/15/2018 05:25 PM, Daniel Borkmann wrote:
>>> On 02/13/2018 05:05 AM, Sandipan Das wrote:
 The imm field of a bpf_insn is a signed 32-bit integer. For
 JIT-ed bpf-to-bpf function calls, it stores the offset from
 __bpf_call_base to the start of the callee function.

 For some architectures, such as powerpc64, it was found that
 this offset may be as large as 64 bits because of which this
 cannot be accomodated in the imm field without truncation.

 To resolve this, we additionally make aux->func within each
 bpf_prog associated with the functions to point to the list
 of all function addresses determined by the verifier.

 We keep the value assigned to the off field of the bpf_insn
 as a way to index into aux->func and also set aux->func_cnt
 so that this can be used for performing basic upper bound
 checks for the off field.

 Signed-off-by: Sandipan Das 
 ---
 v2: Make aux->func point to the list of functions determined
 by the verifier rather than allocating a separate callee
 list for each function.
>>> 
>>> Approach looks good to me; do you know whether s390x JIT would
>>> have similar requirement? I think one limitation that would still
>>> need to be addressed later with such approach would be regarding the
>>> xlated prog dump in bpftool, see 'BPF calls via JIT' in 7105e828c087
>>> ("bpf: allow for correlation of maps and helpers in dump"). Any
>>> ideas for this (potentially if we could use off + imm for calls,
>>> we'd get to 48 bits, but that seems still not be enough as you say)?
>
> All good points. I'm not really sure how s390x works, so I can't comment 
> on that, but I'm copying Michael Holzheu for his consideration.
>
> With the existing scheme, 48 bits won't be enough, so we rejected that 
> approach. I can also see how this will be a problem with bpftool, but I 
> haven't looked into it in detail. I wonder if we can annotate the output 
> to indicate the function being referred to?
>
>> 
>> One other random thought, although I'm not sure how feasible this
>> is for ppc64 JIT to realize ... but idea would be to have something
>> like the below:
>> 
>> diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
>> index 29ca920..daa7258 100644
>> --- a/kernel/bpf/core.c
>> +++ b/kernel/bpf/core.c
>> @@ -512,6 +512,11 @@ int bpf_get_kallsym(unsigned int symnum, unsigned long 
>> *value, char *type,
>>  return ret;
>>  }
>> 
>> +void * __weak bpf_jit_image_alloc(unsigned long size)
>> +{
>> +return module_alloc(size);
>> +}
>> +
>>  struct bpf_binary_header *
>>  bpf_jit_binary_alloc(unsigned int proglen, u8 **image_ptr,
>>   unsigned int alignment,
>> @@ -525,7 +530,7 @@ bpf_jit_binary_alloc(unsigned int proglen, u8 
>> **image_ptr,
>>   * random section of illegal instructions.
>>   */
>>  size = round_up(proglen + sizeof(*hdr) + 128, PAGE_SIZE);
>> -hdr = module_alloc(size);
>> +hdr = bpf_jit_image_alloc(size);
>>  if (hdr == NULL)
>>  return NULL;
>> 
>> And ppc64 JIT could override bpf_jit_image_alloc() in a similar way
>> like some archs would override the module_alloc() helper through a
>> custom implementation, usually via __vmalloc_node_range(), so we
>> could perhaps fit the range for BPF JITed images in a way that they
>> could use the 32bit imm in the end? There are not that many progs
>> loaded typically, so the range could be a bit narrower in such case
>> anyway. (Not sure if this would work out though, but I thought to
>> bring it up.)
>
> That'd be a good option to consider. I don't think we want to allocate 
> anything from the linear memory range since users could load 
> unprivileged BPF programs and consume a lot of memory that way. I doubt 
> if we can map vmalloc'ed memory into the 0xc0 address range, but I'm not 
> entirely sure.
>
> Michael,
> Is the above possible? The question is if we can have BPF programs be 
> allocated within 4GB of __bpf_call_base (which is a kernel symbol), so 
> that calls to those programs can be encoded in a 32-bit immediate field 
> in a BPF instruction.

Hmmm.

It's not technically impossible, but I don't think it's really a good
option.

The 0xc range is a linear mapping of RAM, and the kernel tends to be
near the start of RAM for reasons. That means there generally isn't a
hole in the 0xc range within 4GB for you to map BPF programs.

You could create a hole by making the 0xc mapping non linear, ie.
mapping some RAM near the kernel elsewhere in the 0xc range, to make a
hole that you can then remap BPF programs into. But I think that would
cause a lot of bugs, it's a pretty fundamental assumption that the
linear mapping is 1:1.

> As an extension, we may be able to extend it to 
> 48-bits by combining with another BPF instruction field (offset). In 
> either case, the 

Re: [PATCH] headers: untangle kmemleak.h from mm.h

[Michael Ellerman]

Randy Dunlap <rdun...@infradead.org> writes:

> On 02/12/2018 04:28 AM, Michael Ellerman wrote:
>> Randy Dunlap <rdun...@infradead.org> writes:
>> 
>>> From: Randy Dunlap <rdun...@infradead.org>
>>>
>>> Currently  #includes  for no obvious
>>> reason. It looks like it's only a convenience, so remove kmemleak.h
>>> from slab.h and add  to any users of kmemleak_*
>>> that don't already #include it.
>>> Also remove  from source files that do not use it.
>>>
>>> This is tested on i386 allmodconfig and x86_64 allmodconfig. It
>>> would be good to run it through the 0day bot for other $ARCHes.
>>> I have neither the horsepower nor the storage space for the other
>>> $ARCHes.
>>>
>>> [slab.h is the second most used header file after module.h; kernel.h
>>> is right there with slab.h. There could be some minor error in the
>>> counting due to some #includes having comments after them and I
>>> didn't combine all of those.]
>>>
>>> This is Lingchi patch #1 (death by a thousand cuts, applied to kernel
>>> header files).
>>>
>>> Signed-off-by: Randy Dunlap <rdun...@infradead.org>
>> 
>> I threw it at a random selection of configs and so far the only failures
>> I'm seeing are:
>> 
>>   lib/test_firmware.c:134:2: error: implicit declaration of function 'vfree' 
>> [-Werror=implicit-function-declaration]  
>> 
>>   lib/test_firmware.c:620:25: error: implicit declaration of function 
>> 'vzalloc' [-Werror=implicit-function-declaration]
>>   lib/test_firmware.c:620:2: error: implicit declaration of function 
>> 'vzalloc' [-Werror=implicit-function-declaration]
>>   security/integrity/digsig.c:146:2: error: implicit declaration of function 
>> 'vfree' [-Werror=implicit-function-declaration]
>
> Both of those source files need to #include .

Yep, I added those and rebuilt. I don't see any more failures that look
related to your patch.

  http://kisskb.ellerman.id.au/kisskb/head/13399/


I haven't gone through the defconfigs I have enabled for a while, so
it's possible I have some missing but it's still a reasonable cross
section.

cheers

Re: [PATCH] headers: untangle kmemleak.h from mm.h

[Michael Ellerman]

Randy Dunlap  writes:

> From: Randy Dunlap 
>
> Currently  #includes  for no obvious
> reason. It looks like it's only a convenience, so remove kmemleak.h
> from slab.h and add  to any users of kmemleak_*
> that don't already #include it.
> Also remove  from source files that do not use it.
>
> This is tested on i386 allmodconfig and x86_64 allmodconfig. It
> would be good to run it through the 0day bot for other $ARCHes.
> I have neither the horsepower nor the storage space for the other
> $ARCHes.
>
> [slab.h is the second most used header file after module.h; kernel.h
> is right there with slab.h. There could be some minor error in the
> counting due to some #includes having comments after them and I
> didn't combine all of those.]
>
> This is Lingchi patch #1 (death by a thousand cuts, applied to kernel
> header files).
>
> Signed-off-by: Randy Dunlap 

I threw it at a random selection of configs and so far the only failures
I'm seeing are:

  lib/test_firmware.c:134:2: error: implicit declaration of function 'vfree' 
[-Werror=implicit-function-declaration] 
 
  lib/test_firmware.c:620:25: error: implicit declaration of function 'vzalloc' 
[-Werror=implicit-function-declaration]
  lib/test_firmware.c:620:2: error: implicit declaration of function 'vzalloc' 
[-Werror=implicit-function-declaration]
  security/integrity/digsig.c:146:2: error: implicit declaration of function 
'vfree' [-Werror=implicit-function-declaration]

Full results trickling in here, not all the failures there are caused by
this patch, ie. some configs are broken in mainline:

  http://kisskb.ellerman.id.au/kisskb/head/13396/

cheers

Re: [v3, 3/6] dt: booting-without-of: DT fix s/#interrupt-cell/#interrupt-cells/

[Michael Ellerman]

On Fri, 2017-06-02 at 12:38:46 UTC, Geert Uytterhoeven wrote:
> Signed-off-by: Geert Uytterhoeven 
> Acked-by: Rob Herring 

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/4be4119d1fbd93c44d5c639735c312

cheers

Re: [net] Revert "net: core: maybe return -EEXIST in __dev_alloc_name"

[Michael Ellerman]

David Miller <da...@davemloft.net> writes:

> From: Michael Ellerman <m...@ellerman.id.au>
> Date: Fri, 22 Dec 2017 15:22:22 +1100
>
>>> On Tue, Dec 19 2017, Michael Ellerman <mich...@concordia.ellerman.id.au> 
>>> wrote:
>>>> This revert seems to have broken networking on one of my powerpc
>>>> machines, according to git bisect.
>>>>
>>>> The symptom is DHCP fails and I don't get a link, I didn't dig any
>>>> further than that. I can if it's helpful.
>>>>
>>>> I think the problem is that 87c320e51519 ("net: core: dev_get_valid_name
>>>> is now the same as dev_alloc_name_ns") only makes sense while
>>>> d6f295e9def0 remains in the tree.
>>>
>>> I'm sorry about all of this, I really didn't think there would be such
>>> consequences of changing an errno return. Indeed, d6f29 was preparation
>>> for unifying the two functions that do the exact same thing (and how we
>>> ever got into that situation is somewhat unclear), except for
>>> their behaviour in the case the requested name already exists. So one of
>>> the two interfaces had to change its return value, and as I wrote, I
>>> thought EEXIST was the saner choice when an explicit name (no %d) had
>>> been requested.
>> 
>> No worries.
>> 
>>>> ie. before the entire series, dev_get_valid_name() would return EEXIST,
>>>> and that was retained when 87c320e51519 was merged, but now that
>>>> d6f295e9def0 has been reverted dev_get_valid_name() is returning ENFILE.
>>>>
>>>> I can get the network up again if I also revert 87c320e51519 ("net:
>>>> core: dev_get_valid_name is now the same as dev_alloc_name_ns"), or with
>>>> the gross patch below.
>>>
>>> I don't think changing -ENFILE to -EEXIST would be right either, since
>>> dev_get_valid_name() used to be able to return both (-EEXIST in the case
>>> where there's no %d, -ENFILE in the case where we end up calling
>>> dev_alloc_name_ns()). If anything, we could do the check for the old
>>> -EEXIST condition first, and then call dev_alloc_name_ns(). But I'm also
>>> fine with reverting.
>> 
>> Yeah I think a revert would be best, given it's nearly rc5.
>> 
>> My userspace is not exotic AFAIK, just debian something, so presumably
>> this will affect other people too.
>
> I've just queued up the following revert, thanks!

Thanks.

I don't see it in rc7, will it get to Linus sometime before the release?

cheers

Re: [net] Revert "net: core: maybe return -EEXIST in __dev_alloc_name"

[Michael Ellerman]

Rasmus Villemoes <li...@rasmusvillemoes.dk> writes:
> On Tue, Dec 19 2017, Michael Ellerman <mich...@concordia.ellerman.id.au> 
> wrote:
>>> From: Johannes Berg <johannes.b...@intel.com>
>>> 
>>> This reverts commit d6f295e9def0; some userspace (in the case
>>
>> This revert seems to have broken networking on one of my powerpc
>> machines, according to git bisect.
>>
>> The symptom is DHCP fails and I don't get a link, I didn't dig any
>> further than that. I can if it's helpful.
>>
>> I think the problem is that 87c320e51519 ("net: core: dev_get_valid_name
>> is now the same as dev_alloc_name_ns") only makes sense while
>> d6f295e9def0 remains in the tree.
>
> I'm sorry about all of this, I really didn't think there would be such
> consequences of changing an errno return. Indeed, d6f29 was preparation
> for unifying the two functions that do the exact same thing (and how we
> ever got into that situation is somewhat unclear), except for
> their behaviour in the case the requested name already exists. So one of
> the two interfaces had to change its return value, and as I wrote, I
> thought EEXIST was the saner choice when an explicit name (no %d) had
> been requested.

No worries.

>> ie. before the entire series, dev_get_valid_name() would return EEXIST,
>> and that was retained when 87c320e51519 was merged, but now that
>> d6f295e9def0 has been reverted dev_get_valid_name() is returning ENFILE.
>>
>> I can get the network up again if I also revert 87c320e51519 ("net:
>> core: dev_get_valid_name is now the same as dev_alloc_name_ns"), or with
>> the gross patch below.
>
> I don't think changing -ENFILE to -EEXIST would be right either, since
> dev_get_valid_name() used to be able to return both (-EEXIST in the case
> where there's no %d, -ENFILE in the case where we end up calling
> dev_alloc_name_ns()). If anything, we could do the check for the old
> -EEXIST condition first, and then call dev_alloc_name_ns(). But I'm also
> fine with reverting.

Yeah I think a revert would be best, given it's nearly rc5.

My userspace is not exotic AFAIK, just debian something, so presumably
this will affect other people too.

cheers

Re: [net] Revert "net: core: maybe return -EEXIST in __dev_alloc_name"

[Michael Ellerman]

Hi Johannes,

> From: Johannes Berg 
> 
> This reverts commit d6f295e9def0; some userspace (in the case
> we noticed it's wpa_supplicant), is relying on the current
> error code to determine that a fixed name interface already
> exists.
> 
> Reported-by: Jouni Malinen 
> Signed-off-by: Johannes Berg 
> ---
>  net/core/dev.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

This revert seems to have broken networking on one of my powerpc
machines, according to git bisect.

The symptom is DHCP fails and I don't get a link, I didn't dig any
further than that. I can if it's helpful.

I think the problem is that 87c320e51519 ("net: core: dev_get_valid_name
is now the same as dev_alloc_name_ns") only makes sense while
d6f295e9def0 remains in the tree.

ie. before the entire series, dev_get_valid_name() would return EEXIST,
and that was retained when 87c320e51519 was merged, but now that
d6f295e9def0 has been reverted dev_get_valid_name() is returning ENFILE.

I can get the network up again if I also revert 87c320e51519 ("net:
core: dev_get_valid_name is now the same as dev_alloc_name_ns"), or with
the gross patch below.

cheers

diff --git a/net/core/dev.c b/net/core/dev.c
index f47e96b62308..d0304461ad32 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1146,7 +1146,11 @@ EXPORT_SYMBOL(dev_alloc_name);
 int dev_get_valid_name(struct net *net, struct net_device *dev,
   const char *name)
 {
-   return dev_alloc_name_ns(net, dev, name);
+   int rc = dev_alloc_name_ns(net, dev, name);
+   if (rc == -ENFILE)
+   rc = -EEXIST;
+
+   return rc;
 }
 EXPORT_SYMBOL(dev_get_valid_name);
 

Re: [kernel-hardening] [PATCH 0/3] kallsyms: don't leak address

[Michael Ellerman]

"Tobin C. Harding"  writes:

> This set plugs a kernel address leak that occurs if kallsyms symbol
> look up fails. This set was prompted by a leaking address found using
> scripts/leaking_addresses.pl on a PowerPC machine in the wild.

Any details on that? I haven't heard about it.

cheers

Re: [kernel-hardening] [PATCH v4] scripts: add leaking_addresses.pl

[Michael Ellerman]

Frank Rowand <frowand.l...@gmail.com> writes:
> Hi Michael,
>
> On 11/12/17 03:49, Michael Ellerman wrote:
...
>> 
>> On our bare metal machines the device tree comes from skiboot
>> (firmware), with some of the content provided by hostboot (other
>> firmware), both of which are open source, so in theory most of the
>> information is available in *some* source tree. But there's still
>> information about runtime allocations etc. that is not available in the
>> source anywhere.
>
> Thanks for the additional information. 
>
> Can you explain a little bit what "runtime allocations" are?  Are you
> referring to the memory reservation block, the memory node(s) and the
> chosen node?  Or other information?

Yeah I was thinking of memory reservations. They're under the
reserved-memory node as well as the reservation block, eg:

$ ls -1 /proc/device-tree/reserved-memory/
ibm,firmware-allocs-memory@10
ibm,firmware-allocs-memory@18
ibm,firmware-allocs-memory@39c0
ibm,firmware-allocs-memory@8
ibm,firmware-code@3000
ibm,firmware-data@3100
ibm,firmware-heap@3030
ibm,firmware-stacks@31c0
ibm,hbrt-code-image@1ffd51
ibm,hbrt-target-image@1ffd6a
ibm,hbrt-vpd-image@1ffd70
ibm,slw-image@1ffda0
ibm,slw-image@1ffde0
ibm,slw-image@1ffe20
ibm,slw-image@1ffe60


There's also some new systems where a catalog of PMU events is stored in
flash as a DTB and then stitched into the device tree by skiboot before
booting Linux.

Anyway my point was mainly just that the device tree is not simply a
copy of something in the kernel source.

cheers

Re: [kernel-hardening] [PATCH v4] scripts: add leaking_addresses.pl

[Michael Ellerman]

Hi Frank,

Frank Rowand <frowand.l...@gmail.com> writes:
> Hi Michael, Tobin,
>
> On 11/08/17 04:10, Michael Ellerman wrote:
>> "Tobin C. Harding" <m...@tobin.cc> writes:
>>> Currently we are leaking addresses from the kernel to user space. This
>>> script is an attempt to find some of those leakages. Script parses
>>> `dmesg` output and /proc and /sys files for hex strings that look like
>>> kernel addresses.
>>>
>>> Only works for 64 bit kernels, the reason being that kernel addresses
>>> on 64 bit kernels have '' as the leading bit pattern making greping
>>> possible.
>> 
>> That doesn't work super well on other architectures :D
>> 
>> I don't speak perl but presumably you can check the arch somehow and
>> customise the regex?
>> 
>> ...
>>> +# Return _all_ non false positive addresses from $line.
>>> +sub extract_addresses
>>> +{
>>> +my ($line) = @_;
>>> +my $address = '\b(0x)?[[:xdigit:]]{12}\b';
>> 
>> On 64-bit powerpc (ppc64/ppc64le) we'd want:
>> 
>> +my $address = '\b(0x)?[89abcdef]00[[:xdigit:]]{13}\b';
>> 
>> 
>>> +# Do not parse these files (absolute path).
>>> +my @skip_parse_files_abs = ('/proc/kmsg',
>>> +   '/proc/kcore',
>>> +   '/proc/fs/ext4/sdb1/mb_groups',
>>> +   '/proc/1/fd/3',
>>> +   '/sys/kernel/debug/tracing/trace_pipe',
>>> +   '/sys/kernel/security/apparmor/revision');
>> 
>> Can you add:
>> 
>>   /sys/firmware/devicetree
>> 
>> and/or /proc/device-tree (which is a symlink to the above).
>
> /proc/device-tree is a symlink to /sys/firmware/devicetree/base

Oh yep, forgot about the base part.

> /sys/firmware contains
>fdt  -- the flattened device tree that was passed to the
>kernel on boot
>devicetree/base/ -- the data that is currently in the live device tree.
>This live device tree is represented as directories
>and files beneath base/
>
> The information in fdt is directly available in the kernel source tree

On ARM that might be true, but not on powerpc.

Remember FDT comes from DT which comes from OF - in which case the
information is definitely not in the kernel source! :)

On our bare metal machines the device tree comes from skiboot
(firmware), with some of the content provided by hostboot (other
firmware), both of which are open source, so in theory most of the
information is available in *some* source tree. But there's still
information about runtime allocations etc. that is not available in the
source anywhere.

cheers

Re: [kernel-hardening] [PATCH v4] scripts: add leaking_addresses.pl

[Michael Ellerman]

"Tobin C. Harding" <m...@tobin.cc> writes:

> On Wed, Nov 08, 2017 at 11:10:56PM +1100, Michael Ellerman wrote:
>> "Tobin C. Harding" <m...@tobin.cc> writes:
> [snip]
>
> Hi Michael,
>
> I'm working an adding support for ppc64 to leaking_addresses.pl, I've
> added the kernel address regular expression that you suggested.

Thanks!

> I'd like to add the false positive for vsyscall addresses. Excuse my
> ignorance but does PowerPC use a constant address range for vsyscall like 
> x86_64
> does? The ppc64 machine I have access to does not output anything for
>
>   $ cat /proc/PID/tasks/PID/smaps or
>   $ cat /proc/PID/tasks/PID/maps

No we only have the vdso style vsyscall, which is mapped at user
addresses and is subject to ASLR, so you shouldn't need to worry about
it.

cheers

Re: [kernel-hardening] [PATCH v4] scripts: add leaking_addresses.pl

[Michael Ellerman]

"Tobin C. Harding"  writes:
> Currently we are leaking addresses from the kernel to user space. This
> script is an attempt to find some of those leakages. Script parses
> `dmesg` output and /proc and /sys files for hex strings that look like
> kernel addresses.
>
> Only works for 64 bit kernels, the reason being that kernel addresses
> on 64 bit kernels have '' as the leading bit pattern making greping
> possible.

That doesn't work super well on other architectures :D

I don't speak perl but presumably you can check the arch somehow and
customise the regex?

...
> +# Return _all_ non false positive addresses from $line.
> +sub extract_addresses
> +{
> +my ($line) = @_;
> +my $address = '\b(0x)?[[:xdigit:]]{12}\b';

On 64-bit powerpc (ppc64/ppc64le) we'd want:

+my $address = '\b(0x)?[89abcdef]00[[:xdigit:]]{13}\b';


> +# Do not parse these files (absolute path).
> +my @skip_parse_files_abs = ('/proc/kmsg',
> + '/proc/kcore',
> + '/proc/fs/ext4/sdb1/mb_groups',
> + '/proc/1/fd/3',
> + '/sys/kernel/debug/tracing/trace_pipe',
> + '/sys/kernel/security/apparmor/revision');

Can you add:

  /sys/firmware/devicetree

and/or /proc/device-tree (which is a symlink to the above).

We should also start restricting access to that because it may have
potentially interesting physical addresses in it, but that will break
existing tools, so it will need to be opt-in and done over time.

cheers

Re: [1/1] bpf: take advantage of stack_depth tracking in powerpc JIT

[Michael Ellerman]

On Fri, 2017-09-01 at 18:53:01 UTC, Sandipan Das wrote:
> Take advantage of stack_depth tracking, originally introduced for
> x64, in powerpc JIT as well. Round up allocated stack by 16 bytes
> to make sure it stays aligned for functions called from JITed bpf
> program.
> 
> Signed-off-by: Sandipan Das 
> Reviewed-by: Naveen N. Rao 

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/ac0761ebcb08830d8f64b9181f6736

cheers

Re: [v3, 4/6] powerpc: dts: acadia: DT fix s/#interrupts-parent/#interrupt-parent/

[Michael Ellerman]

On Fri, 2017-06-02 at 12:38:47 UTC, Geert Uytterhoeven wrote:
> Signed-off-by: Geert Uytterhoeven 
> Acked-by: Rob Herring 

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/3d2d4339cc326c427638daa67e264d

cheers

Re: [PATCH 3/3][v2] selftests: silence test output by default

[Michael Ellerman]

Shuah Khan  writes:

> On 09/19/2017 07:51 AM, jo...@toxicpanda.com wrote:
>> From: Josef Bacik 
>> 
>> Some of the networking tests are very noisy and make it impossible to
>> see if we actually passed the tests as they run.  Default to suppressing
>> the output from any tests run in order to make it easier to track what
>> failed.
>> 
>> Signed-off-by: Josef Bacik 
>> ---
>> v1->v2:
>> - dump output into /tmp/testname instead of /dev/null
>> 
>
> Thanks for the fix. Applied to linux-kselftest for 4.14-rc2

Sorry this is not a fix.

This is a regression, it breaks all my test infrastructure, because we
use the output of the test case.

Can we please revert this and fix any tests that are overly verbose.

cheers

Re: [PATCH v3 3/6] dt: booting-without-of: DT fix s/#interrupt-cell/#interrupt-cells/

[Michael Ellerman]

Geert Uytterhoeven  writes:

> Signed-off-by: Geert Uytterhoeven 
> Acked-by: Rob Herring 
> ---

Rob this has your ack, but I'd expect it to go via your tree? Or should
I grab it?

cheers

> diff --git a/Documentation/devicetree/booting-without-of.txt 
> b/Documentation/devicetree/booting-without-of.txt
> index 280d283304bb82d8..f35d3adacb987f7d 100644
> --- a/Documentation/devicetree/booting-without-of.txt
> +++ b/Documentation/devicetree/booting-without-of.txt
> @@ -1309,7 +1309,7 @@ number and level/sense information. All interrupt 
> children in an
>  OpenPIC interrupt domain use 2 cells per interrupt in their interrupts
>  property.
>  
> -The PCI bus binding specifies a #interrupt-cell value of 1 to encode
> +The PCI bus binding specifies a #interrupt-cells value of 1 to encode
>  which interrupt pin (INTA,INTB,INTC,INTD) is used.
>  
>  2) interrupt-parent property
> -- 
> 2.7.4

Re: [PATCH] PCI: Allow PCI express root ports to find themselves

[Michael Ellerman]

Thierry Reding  writes:

> From: Thierry Reding 
>
> If the pci_find_pcie_root_port() function is called on a root port
> itself, return the root port rather than NULL.
>
> This effectively reverts commit 0e405232871d6 ("PCI: fix oops when
> try to find Root Port for a PCI device") which added an extra check
> that would now be redundant.
>
> Fixes: a99b646afa8a ("PCI: Disable PCIe Relaxed Ordering if unsupported")
> Fixes: c56d4450eb68 ("PCI: Turn off Request Attributes to avoid Chelsio T5 
> Completion erratum")
> Signed-off-by: Thierry Reding 
> ---
> This applies on top of and was tested on next-20170817.
>
> Michael, it'd be great if you could test this one again to clarify
> whether or not the fix that's already in Linus' tree is still needed, or
> whether it's indeed obsoleted by this patch.

This works fine for me, applied on top of Linus' tree (d33a2a914319).

cheers

Re: [PATCH v2] bpf: Update sysctl documentation to list all supported architectures

[Michael Ellerman]

Daniel Borkmann <dan...@iogearbox.net> writes:

> On 08/17/2017 12:30 PM, Michael Ellerman wrote:
>> The sysctl documentation states that the JIT is only available on
>> x86_64, which is no longer correct.
>>
>> Update the list, and break it out to indicate which architectures
>> support the cBPF JIT (via HAVE_CBPF_JIT) or the eBPF JIT
>> (HAVE_EBPF_JIT).
>>
>> Signed-off-by: Michael Ellerman <m...@ellerman.id.au>
>
> The last paragraph speaking about tcpdump, I can take care of
> later, also given the patch is about updating list of archs, so
> lgtm, thanks!

Thanks.

cheers

Re: [PATCH] bpf: Update sysctl documentation to list all supported architectures

[Michael Ellerman]

Daniel Borkmann <dan...@iogearbox.net> writes:
> On 08/16/2017 01:10 PM, Michael Ellerman wrote:
>> Daniel Borkmann <dan...@iogearbox.net> writes:
>>> On 08/16/2017 07:15 AM, Michael Ellerman wrote:
>>>> diff --git a/Documentation/sysctl/net.txt b/Documentation/sysctl/net.txt
>>>> index 14db18c970b1..f68356024d09 100644
>>>> --- a/Documentation/sysctl/net.txt
>>>> +++ b/Documentation/sysctl/net.txt
>>>> @@ -36,8 +36,9 @@ bpf_jit_enable
>>>>--
>>>>
>>>>This enables Berkeley Packet Filter Just in Time compiler.
>>>> -Currently supported on x86_64 architecture, bpf_jit provides a framework
>>>> -to speed packet filtering, the one used by tcpdump/libpcap for example.
>>>> +Currently supported on arm, arm64, mips, powerpc, s390, sparc and x86_64
>>>> +architectures, bpf_jit provides a framework to speed packet filtering, 
>>>> the one
>>>> +used by tcpdump/libpcap for example.
>>>
>>> Good point, could we actually make that as a bullet list and
>>> differentiate between cBPF and eBPF JITs, so that a user doesn't
>>> need to run git grep HAVE_{E,C}BPF_JIT to figure it out what the
>>> switch enables on the arch used? That would be great.
>>
>> We could.
>>
>> Does a user of the sysctl want/need to know the difference though? Or do
>> they just want to turn on "the JIT"?
>
> They would just turn it on, but I think it would be nice to inform
> them which archs support eBPF (which is a superset of cBPF in term
> of what can be jited), so in case they have some native eBPF programs
> they would see whether these can also be jited.

OK. v2 just sent.

The text could probably use some more tweaking to mention the other
things BPF is used for these days, but I didn't really feel qualified to
do that.

cheers

[PATCH v2] bpf: Update sysctl documentation to list all supported architectures

[Michael Ellerman]

The sysctl documentation states that the JIT is only available on
x86_64, which is no longer correct.

Update the list, and break it out to indicate which architectures
support the cBPF JIT (via HAVE_CBPF_JIT) or the eBPF JIT
(HAVE_EBPF_JIT).

Signed-off-by: Michael Ellerman <m...@ellerman.id.au>
---
 Documentation/sysctl/net.txt | 19 +--
 1 file changed, 17 insertions(+), 2 deletions(-)

v2: Split the list based on cBPF vs eBPF.

diff --git a/Documentation/sysctl/net.txt b/Documentation/sysctl/net.txt
index 14db18c970b1..b9c3c6078010 100644
--- a/Documentation/sysctl/net.txt
+++ b/Documentation/sysctl/net.txt
@@ -36,8 +36,23 @@ bpf_jit_enable
 --
 
 This enables Berkeley Packet Filter Just in Time compiler.
-Currently supported on x86_64 architecture, bpf_jit provides a framework
-to speed packet filtering, the one used by tcpdump/libpcap for example.
+
+There are two flavors of JIT, the new eBPF JIT supported on:
+  - x86_64
+  - arm64
+  - ppc64
+  - sparc64
+  - mips64
+
+And the older cBPF JIT supported on:
+  - arm
+  - mips
+  - ppc
+  - sparc
+
+The BPF JIT provides a framework to speed packet filtering, the one used by
+tcpdump/libpcap for example.
+
 Values :
0 - disable the JIT (default value)
1 - enable the JIT
-- 
2.7.4

Re: [PATCH net RESEND] PCI: fix oops when try to find Root Port for a PCI device

[Michael Ellerman]

Thierry Reding  writes:
...
>
> In case of Tegra, dev actually points to the root port. Now if I read
> the above code correctly, highest_pcie_bridge will still be NULL in that
> case, which in turn will return NULL from pci_find_pcie_root_port(). But
> shouldn't it really return dev?
>
> The patch that I used to fix the issue is this:
>
> --->8---
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 2c712dcfd37d..dd56c1c05614 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -514,7 +514,7 @@ EXPORT_SYMBOL(pci_find_resource);
>   */
>  struct pci_dev *pci_find_pcie_root_port(struct pci_dev *dev)
>  {
> -   struct pci_dev *bridge, *highest_pcie_bridge = NULL;
> +   struct pci_dev *bridge, *highest_pcie_bridge = dev;
>  
> bridge = pci_upstream_bridge(dev);
> while (bridge && pci_is_pcie(bridge)) {
> --->8---
>
> That works correctly if this function ends up being called on the PCIe
> root port, though perhaps that's not what this function is supposed to
> do. It's somewhat unclear from the kerneldoc what the function should
> be doing when called on a root port device itself.

That also works for me on powerpc (oops reported up thread).

cheers

Re: [PATCH] bpf: Update sysctl documentation to list all supported architectures

[Michael Ellerman]

Daniel Borkmann <dan...@iogearbox.net> writes:

> Hi Michael,
>
> On 08/16/2017 07:15 AM, Michael Ellerman wrote:
>> The sysctl documentation states that the JIT is only available on
>> x86_64, which is no longer correct.
>>
>> Update the list to include all architectures that enable HAVE_CBPF_JIT
>> or HAVE_EBPF_JIT under some configuration.
>>
>> Signed-off-by: Michael Ellerman <m...@ellerman.id.au>
>
> Thanks for the patch!
>
>>   Documentation/sysctl/net.txt | 5 +++--
>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/sysctl/net.txt b/Documentation/sysctl/net.txt
>> index 14db18c970b1..f68356024d09 100644
>> --- a/Documentation/sysctl/net.txt
>> +++ b/Documentation/sysctl/net.txt
>> @@ -36,8 +36,9 @@ bpf_jit_enable
>>   --
>>
>>   This enables Berkeley Packet Filter Just in Time compiler.
>> -Currently supported on x86_64 architecture, bpf_jit provides a framework
>> -to speed packet filtering, the one used by tcpdump/libpcap for example.
>> +Currently supported on arm, arm64, mips, powerpc, s390, sparc and x86_64
>> +architectures, bpf_jit provides a framework to speed packet filtering, the 
>> one
>> +used by tcpdump/libpcap for example.
>
> Good point, could we actually make that as a bullet list and
> differentiate between cBPF and eBPF JITs, so that a user doesn't
> need to run git grep HAVE_{E,C}BPF_JIT to figure it out what the
> switch enables on the arch used? That would be great.

We could.

Does a user of the sysctl want/need to know the difference though? Or do
they just want to turn on "the JIT"?

cheers

[PATCH] bpf: Update sysctl documentation to list all supported architectures

[Michael Ellerman]

The sysctl documentation states that the JIT is only available on
x86_64, which is no longer correct.

Update the list to include all architectures that enable HAVE_CBPF_JIT
or HAVE_EBPF_JIT under some configuration.

Signed-off-by: Michael Ellerman <m...@ellerman.id.au>
---
 Documentation/sysctl/net.txt | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/sysctl/net.txt b/Documentation/sysctl/net.txt
index 14db18c970b1..f68356024d09 100644
--- a/Documentation/sysctl/net.txt
+++ b/Documentation/sysctl/net.txt
@@ -36,8 +36,9 @@ bpf_jit_enable
 --
 
 This enables Berkeley Packet Filter Just in Time compiler.
-Currently supported on x86_64 architecture, bpf_jit provides a framework
-to speed packet filtering, the one used by tcpdump/libpcap for example.
+Currently supported on arm, arm64, mips, powerpc, s390, sparc and x86_64
+architectures, bpf_jit provides a framework to speed packet filtering, the one
+used by tcpdump/libpcap for example.
 Values :
0 - disable the JIT (default value)
1 - enable the JIT
-- 
2.7.4

Re: DNS (?) not working on G5 (64-bit powerpc) (was [net-next,v3,3/3] udp: try to avoid 2 cache miss on dequeue)

[Michael Ellerman]

Paolo Abeni  writes:
> Thank you!
>
> I'll submit formally the patch after some more testing.

Thanks.

> I noticed this version has entered the ppc patchwork, but I think that
> the formal submission should go towards the net-next tree.

Yeah it picks up all patches sent to the list. That's fine I'll just
mark it "Not applicable", and expect to see it arrive via net-next.

cheers

Re: DNS (?) not working on G5 (64-bit powerpc) (was [net-next,v3,3/3] udp: try to avoid 2 cache miss on dequeue)

[Michael Ellerman]

Hannes Frederic Sowa  writes:

> On Thu, Jun 22, 2017, at 22:57, Paolo Abeni wrote:
>> 
>> Can you please check if the following patch fixes the issue? Only
>> compiled tested here.
>> 
>> Thanks!!!
>> ---
>> diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
>> index 067a607..80d89fe 100644
>> --- a/net/ipv4/udp.c
>> +++ b/net/ipv4/udp.c
>> @@ -1446,16 +1446,19 @@ static struct sk_buff
>> *__first_packet_length(struct sock *sk,
>>  {
>>  struct sk_buff *skb;
>>  
>> -   while ((skb = skb_peek(rcvq)) != NULL &&
>> -      udp_lib_checksum_complete(skb)) {
>> -   __UDP_INC_STATS(sock_net(sk), UDP_MIB_CSUMERRORS,
>> -   IS_UDPLITE(sk));
>> -   __UDP_INC_STATS(sock_net(sk), UDP_MIB_INERRORS,
>> -   IS_UDPLITE(sk));
>> -   atomic_inc(>sk_drops);
>> -   __skb_unlink(skb, rcvq);
>> -   *total += skb->truesize;
>> -   kfree_skb(skb);
>> +   while ((skb = skb_peek(rcvq)) != NULL) {
>> +   if (udp_lib_checksum_complete(skb)) {
>> +   __UDP_INC_STATS(sock_net(sk), UDP_MIB_CSUMERRORS,
>> +   IS_UDPLITE(sk));
>> +   __UDP_INC_STATS(sock_net(sk), UDP_MIB_INERRORS,
>> +   IS_UDPLITE(sk));
>> +   atomic_inc(>sk_drops);
>> +   __skb_unlink(skb, rcvq);
>> +   *total += skb->truesize;
>> +   kfree_skb(skb);
>> +   } else {
>> +   udp_set_dev_scratch(skb);
>
> It needs a "break;" here.
>
>> +   }
>>  }
>>  return skb;
>>  }

That works!

$ wget google.com
--2017-06-23 16:56:31--  http://google.com/
Resolving proxy.pmdw.com (proxy.pmdw.com)... 10.1.2.3
Connecting to proxy.pmdw.com (proxy.pmdw.com)|10.1.2.3|:3128... connected.
Proxy request sent, awaiting response... 302 Found
Location: http://www.google.com.au/?gfe_rd=cr=n7tMWeb9JYPr8wfg4LXYAQ 
[following]
--2017-06-23 16:56:31--  
http://www.google.com.au/?gfe_rd=cr=n7tMWeb9JYPr8wfg4LXYAQ
Reusing existing connection to proxy.pmdw.com:3128.
Proxy request sent, awaiting response... 200 OK
Length: unspecified [text/html]
Saving to: ‘index.html’


The patch had whitespace issues or something and I had to apply it by
hand, here's what I actually tested.

cheers

diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 067a607917f9..d3227c1bbe8e 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1446,16 +1446,20 @@ static struct sk_buff *__first_packet_length(struct 
sock *sk,
 {
struct sk_buff *skb;
 
-   while ((skb = skb_peek(rcvq)) != NULL &&
-  udp_lib_checksum_complete(skb)) {
-   __UDP_INC_STATS(sock_net(sk), UDP_MIB_CSUMERRORS,
-   IS_UDPLITE(sk));
-   __UDP_INC_STATS(sock_net(sk), UDP_MIB_INERRORS,
-   IS_UDPLITE(sk));
-   atomic_inc(>sk_drops);
-   __skb_unlink(skb, rcvq);
-   *total += skb->truesize;
-   kfree_skb(skb);
+   while ((skb = skb_peek(rcvq)) != NULL) {
+   if (udp_lib_checksum_complete(skb)) {
+   __UDP_INC_STATS(sock_net(sk), UDP_MIB_CSUMERRORS,
+   IS_UDPLITE(sk));
+   __UDP_INC_STATS(sock_net(sk), UDP_MIB_INERRORS,
+   IS_UDPLITE(sk));
+   atomic_inc(>sk_drops);
+   __skb_unlink(skb, rcvq);
+   *total += skb->truesize;
+   kfree_skb(skb);
+   } else {
+   udp_set_dev_scratch(skb);
+   break;
+   }
}
return skb;
 }

Re: [PATCH 21/44] powerpc: implement ->mapping_error

[Michael Ellerman]

Christoph Hellwig <h...@lst.de> writes:

> DMA_ERROR_CODE is going to go away, so don't rely on it.  Instead
> define a ->mapping_error method for all IOMMU based dma operation
> instances.  The direct ops don't ever return an error and don't
> need a ->mapping_error method.
>
> Signed-off-by: Christoph Hellwig <h...@lst.de>
> ---
>  arch/powerpc/include/asm/dma-mapping.h |  4 
>  arch/powerpc/include/asm/iommu.h   |  4 
>  arch/powerpc/kernel/dma-iommu.c|  6 ++
>  arch/powerpc/kernel/iommu.c| 28 ++--
>  arch/powerpc/platforms/cell/iommu.c|  1 +
>  arch/powerpc/platforms/pseries/vio.c   |  3 ++-
>  6 files changed, 27 insertions(+), 19 deletions(-)

I also see:

  arch/powerpc/kernel/dma.c:const struct dma_map_ops dma_direct_ops = {

Which you mentioned can't fail.

  arch/powerpc/platforms/pseries/ibmebus.c:static const struct dma_map_ops 
ibmebus_dma_ops = {

Which can't fail.

And:

  arch/powerpc/platforms/powernv/npu-dma.c:static const struct dma_map_ops 
dma_npu_ops = {
  arch/powerpc/platforms/ps3/system-bus.c:static const struct dma_map_ops 
ps3_sb_dma_ops = {
  arch/powerpc/platforms/ps3/system-bus.c:static const struct dma_map_ops 
ps3_ioc0_dma_ops = {

All of which look like they definitely can fail, but return 0 on error
and don't implement ->mapping_error.

So I guess I'm acking this and adding a TODO to fix up the NPU code at
least, the ps3 code is probably better left alone these days.

Acked-by: Michael Ellerman <m...@ellerman.id.au>

cheers

Re: [PATCH net-next] net: phy: use of_mdio_parse_addr

[Michael Ellerman]

Liviu Dudau  writes:

> On Fri, Jun 02, 2017 at 02:22:51PM -0400, David Miller wrote:
>> From: Jon Mason 
>> Date: Wed, 31 May 2017 15:43:30 -0400
>> 
>> > use of_mdio_parse_addr() in place of an OF read of reg and a bounds
>> > check (which is litterally the exact same thing that
>> > of_mdio_parse_addr() does)
>> > 
>> > Signed-off-by: Jon Mason 
>> 
>> Applied, thanks Jon.
>
> This makes linux-next fail the modules_install target as depmod detects 2 
> circular
> dependencies. Reverting this patch fixes the issue.
>
> depmod: ERROR: Cycle detected: libphy -> of_mdio -> fixed_phy -> libphy
> depmod: ERROR: Cycle detected: libphy -> of_mdio -> libphy
> depmod: ERROR: Found 3 modules in dependency cycles!
> make[1]: *** [/home/dliviu/devel/kernel/Makefile:1245: _modinst_post] Error 1
>
> This is on an ARCH=arm build, build I doubt it makes a difference. Let me 
> know if
> you need some .config values in order to reproduce.

Same happens on 32-bit powerpc.

cheers

Re: [PATCH net-next] powerpc: use asm-generic/socket.h as much as possible

[Michael Ellerman]

Stephen Rothwell  writes:

> asm-generic/socket.h already has an exception for the differences that
> powerpc needs, so just include it after defining the differences.
>
> Signed-off-by: Stephen Rothwell 
> ---
>  arch/powerpc/include/uapi/asm/socket.h | 92 
> +-
>  1 file changed, 1 insertion(+), 91 deletions(-)
>
> Build tested using powerpc allyesconfig, pseries_le_defconfig, 32 bit
> and 64 bit allnoconfig and ppc44x_defconfig builds.

Did you boot it and test that userspace was happy doing sockety things?

cheers

Re: [PATCH 08/29] drivers, md: convert mddev.active from atomic_t to refcount_t

[Michael Ellerman]

Elena Reshetova  writes:

> refcount_t type and corresponding API should be
> used instead of atomic_t when the variable is used as
> a reference counter. This allows to avoid accidental
> refcounter overflows that might lead to use-after-free
> situations.
>
> Signed-off-by: Elena Reshetova 
> Signed-off-by: Hans Liljestrand 
> Signed-off-by: Kees Cook 
> Signed-off-by: David Windsor 
> ---
>  drivers/md/md.c | 6 +++---
>  drivers/md/md.h | 3 ++-
>  2 files changed, 5 insertions(+), 4 deletions(-)

When booting linux-next (specifically 5be4921c9958ec) I'm seeing the
backtrace below. I suspect this patch is just exposing an existing
issue?

cheers


[0.230738] md: Waiting for all devices to be available before autodetect
[0.230742] md: If you don't use raid, use raid=noautodetect
[0.230962] refcount_t: increment on 0; use-after-free.
[0.230988] [ cut here ]
[0.230996] WARNING: CPU: 0 PID: 1 at lib/refcount.c:114 
.refcount_inc+0x5c/0x70
[0.231001] Modules linked in:
[0.231006] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 
4.11.0-rc1-gccN-next-20170310-g5be4921 #1
[0.231012] task: c0004940 task.stack: c0004944
[0.231016] NIP: c05ac6bc LR: c05ac6b8 CTR: c0743390
[0.231021] REGS: c00049443160 TRAP: 0700   Not tainted  
(4.11.0-rc1-gccN-next-20170310-g5be4921)
[0.231026] MSR: 80029032 
[0.231033]   CR: 24024422  XER: 000c
[0.231038] CFAR: c0a5356c SOFTE: 1 
[0.231038] GPR00: c05ac6b8 c000494433e0 c1079d00 
002b 
[0.231038] GPR04:  00ef  
c10418a0 
[0.231038] GPR08: 4af8 c0ecc9a8 c0ecc9a8 
 
[0.231038] GPR12: 28024824 c6bb  
c00049443a00 
[0.231038] GPR16:  c00049443a10  
 
[0.231038] GPR20:   c0f7dd20 
 
[0.231038] GPR24: 014080c0 c12060b8 c1206080 
0009 
[0.231038] GPR28: c0f7dde0 0090  
c000461ae800 
[0.231100] NIP [c05ac6bc] .refcount_inc+0x5c/0x70
[0.231104] LR [c05ac6b8] .refcount_inc+0x58/0x70
[0.231108] Call Trace:
[0.231112] [c000494433e0] [c05ac6b8] .refcount_inc+0x58/0x70 
(unreliable)
[0.231120] [c00049443450] [c086c008] .mddev_find+0x1e8/0x430
[0.231125] [c00049443530] [c0872b6c] .md_open+0x2c/0x140
[0.231132] [c000494435c0] [c03962a4] .__blkdev_get+0xd4/0x520
[0.231138] [c00049443690] [c0396cc0] .blkdev_get+0x1c0/0x4f0
[0.231145] [c00049443790] [c0336d64] 
.do_dentry_open.isra.1+0x2a4/0x410
[0.231152] [c00049443830] [c03523f4] .path_openat+0x624/0x1580
[0.231157] [c00049443990] [c0354ce4] .do_filp_open+0x84/0x120
[0.231163] [c00049443b10] [c0338d74] .do_sys_open+0x214/0x300
[0.231170] [c00049443be0] [c0da69ac] .md_run_setup+0xa0/0xec
[0.231176] [c00049443c60] [c0da4fbc] 
.prepare_namespace+0x60/0x240
[0.231182] [c00049443ce0] [c0da47a8] 
.kernel_init_freeable+0x330/0x36c
[0.231190] [c00049443db0] [c000dc44] .kernel_init+0x24/0x160
[0.231197] [c00049443e30] [c000badc] 
.ret_from_kernel_thread+0x58/0x7c
[0.231202] Instruction dump:
[0.231206] 6000 3d22ffee 89296bfb 2f89 409effdc 3c62ffc6 3921 
3d42ffee 
[0.231216] 38630928 992a6bfb 484a6e79 6000 <0fe0> 4bb8 6000 
6000 
[0.231226] ---[ end trace 8c51f269ad91ffc2 ]---
[0.231233] md: Autodetecting RAID arrays.
[0.231236] md: autorun ...
[0.231239] md: ... autorun DONE.
[0.234188] EXT4-fs (sda4): mounting ext3 file system using the ext4 
subsystem
[0.250506] refcount_t: underflow; use-after-free.
[0.250531] [ cut here ]
[0.250537] WARNING: CPU: 0 PID: 3 at lib/refcount.c:207 
.refcount_dec_not_one+0x104/0x120
[0.250542] Modules linked in:
[0.250546] CPU: 0 PID: 3 Comm: kworker/0:0 Tainted: GW   
4.11.0-rc1-gccN-next-20170310-g5be4921 #1
[0.250553] Workqueue: events .delayed_fput
[0.250557] task: c00049404900 task.stack: c00049448000
[0.250562] NIP: c05ac964 LR: c05ac960 CTR: c0743390
[0.250567] REGS: c0004944b530 TRAP: 0700   Tainted: GW
(4.11.0-rc1-gccN-next-20170310-g5be4921)
[0.250572] MSR: 80029032 
[0.250578]   CR: 24002422  XER: 0007
[0.250584] CFAR: c0a5356c SOFTE: 1 
[0.250584] GPR00: c05ac960 

Re: [1/3] powerpc: bpf: remove redundant check for non-null image

[Michael Ellerman]

On Fri, 2017-01-13 at 17:10:00 UTC, "Naveen N. Rao" wrote:
> From: Daniel Borkmann 
> 
> We have a check earlier to ensure we don't proceed if image is NULL. As
> such, the redundant check can be removed.
> 
> Signed-off-by: Daniel Borkmann 
> [Added similar changes for classic BPF JIT]
> Signed-off-by: Naveen N. Rao 
> Acked-by: Alexei Starovoitov 

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/052de33ca4f840bf35587eacdf78b3

cheers

Re: [2/3] powerpc: bpf: flush the entire JIT buffer

[Michael Ellerman]

On Fri, 2017-01-13 at 17:10:01 UTC, "Naveen N. Rao" wrote:
> With bpf_jit_binary_alloc(), we allocate at a page granularity and fill
> the rest of the space with illegal instructions to mitigate BPF spraying
> attacks, while having the actual JIT'ed BPF program at a random location
> within the allocated space. Under this scenario, it would be better to
> flush the entire allocated buffer rather than just the part containing
> the actual program. We already flush the buffer from start to the end of
> the BPF program. Extend this to include the illegal instructions after
> the BPF program.
> 
> Signed-off-by: Naveen N. Rao 
> Acked-by: Alexei Starovoitov 
> Acked-by: Daniel Borkmann 

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/10528b9c45cfb9e8f45217ef2f5ef8

cheers

Re: Coding Style: Reverse XMAS tree declarations ? (was Re: [PATCH net-next v6 02/10] dpaa_eth: add support for DPAA Ethernet)

[Michael Ellerman]

Joe Perches  writes:
> On Thu, 2016-11-03 at 15:58 -0400, David Miller wrote:
>> From: Madalin Bucur 
>> Date: Wed, 2 Nov 2016 22:17:26 +0200
>> 
>> > This introduces the Freescale Data Path Acceleration Architecture
>> > +static inline size_t bpool_buffer_raw_size(u8 index, u8 cnt)
>> > +{
>> > + u8 i;
>> > + size_t res = DPAA_BP_RAW_SIZE / 2;
>> 
>> Always order local variable declarations from longest to shortest line,
>> also know as Reverse Christmas Tree Format.
>
> I think this declaration sorting order is misguided but
> here's a possible change to checkpatch adding a test for it
> that does this test just for net/ and drivers/net/

And arch/powerpc too please.

cheers

Re: [mm PATCH v2 18/26] arch/powerpc: Add option to skip DMA sync as a part of mapping

[Michael Ellerman]

Alexander Duyck <alexander.h.du...@intel.com> writes:

> This change allows us to pass DMA_ATTR_SKIP_CPU_SYNC which allows us to
> avoid invoking cache line invalidation if the driver will just handle it
> via a sync_for_cpu or sync_for_device call.
>
> Cc: Benjamin Herrenschmidt <b...@kernel.crashing.org>
> Cc: Paul Mackerras <pau...@samba.org>
> Cc: Michael Ellerman <m...@ellerman.id.au>
> Cc: linuxppc-...@lists.ozlabs.org
> Signed-off-by: Alexander Duyck <alexander.h.du...@intel.com>
> ---
>  arch/powerpc/kernel/dma.c |9 -
>  1 file changed, 8 insertions(+), 1 deletion(-)

LGTM.

Acked-by: Michael Ellerman <m...@ellerman.id.au> (powerpc)

cheers

Re: [1/3] bpf powerpc: introduce accessors for using the tmp local stack space

[Michael Ellerman]

On Fri, 2016-23-09 at 20:35:00 UTC, "Naveen N. Rao" wrote:
> While at it, ensure that the location of the local save area is
> consistent whether or not we setup our own stackframe. This property is
> utilised in the next patch that adds support for tail calls.
> 
> Signed-off-by: Naveen N. Rao 

Series applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/7b847f523fe07b4ad73a01cec49a4d

cheers

Re: [PATCH] MAINTAINERS: Remove myself from PA Semi entries

[Michael Ellerman]

Olof Johansson  writes:

> The platform is old, very few users and I lack bandwidth to keep after
> it these days.
>
> Mark the base platform as well as the drivers as orphans, patches have
> been flowing through the fallback maintainers for a while already.

Sorry to see you go, but thanks for keeping an eye on it as long as you
did!

> Jean, Dave,
>
> I was hoping to have Michael merge this since the bulk of the platform is 
> under him,
> cc:ing you mostly to be aware that I am orphaning a driver in your subsystems.

I'll merge it unless I hear otherwise from Dave.

Should we go the whole hog and just do as below? I think most folks use
get_maintainers.pl these days, so this should have basically the same
effect. Happy to go with your original version though if you prefer.

cheers


diff --git a/MAINTAINERS b/MAINTAINERS
index 0bbe4b105c34..8ca1c25d870d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7049,6 +7049,7 @@ N:powermac
 N: powernv
 N: [^a-z0-9]ps3
 N: pseries
+N: pasemi
 
 LINUX FOR POWER MACINTOSH
 M: Benjamin Herrenschmidt 
@@ -7098,14 +7099,6 @@ S:   Maintained
 F: arch/powerpc/platforms/83xx/
 F: arch/powerpc/platforms/85xx/
 
-LINUX FOR POWERPC PA SEMI PWRFICIENT
-M: Olof Johansson 
-L: linuxppc-...@lists.ozlabs.org
-S: Maintained
-F: arch/powerpc/platforms/pasemi/
-F: drivers/*/*pasemi*
-F: drivers/*/*/*pasemi*
-
 LINUX SECURITY MODULE (LSM) FRAMEWORK
 M: Chris Wright 
 L: linux-security-mod...@vger.kernel.org
@@ -8773,18 +8766,6 @@ W:   http://wireless.kernel.org/en/users/Drivers/p54
 S: Maintained
 F: drivers/net/wireless/intersil/p54/
 
-PA SEMI ETHERNET DRIVER
-M: Olof Johansson 
-L: netdev@vger.kernel.org
-S: Maintained
-F: drivers/net/ethernet/pasemi/*
-
-PA SEMI SMBUS DRIVER
-M: Olof Johansson 
-L: linux-...@vger.kernel.org
-S: Maintained
-F: drivers/i2c/busses/i2c-pasemi.c
-
 PADATA PARALLEL EXECUTION MECHANISM
 M: Steffen Klassert 
 L: linux-cry...@vger.kernel.org

Re: [PATCH 2/9] selftests: update filesystems Makefile to work under selftests

[Michael Ellerman]

Shuah Khan  writes:

> Update to work under selftests. dnotify_test will not be run as part of
> selftests suite and will not included in install targets. It can be built
> separately for now.
>
> Signed-off-by: Shuah Khan 
> ---
>  tools/testing/selftests/filesystems/Makefile | 10 ++
>  1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/tools/testing/selftests/filesystems/Makefile 
> b/tools/testing/selftests/filesystems/Makefile
> index 883010c..f1dce5c 100644
> --- a/tools/testing/selftests/filesystems/Makefile
> +++ b/tools/testing/selftests/filesystems/Makefile
> @@ -1,5 +1,7 @@
> -# List of programs to build
> -hostprogs-y := dnotify_test
> +TEST_PROGS := dnotify_test
> +all: $(TEST_PROGS)
>  
> -# Tell kbuild to always build the programs
> -always := $(hostprogs-y)
> +include ../lib.mk
> +
> +clean:
> + rm -fr dnotify_test

That's a complete rewrite of the Makefile, so I don't think there's any
value in bringing its content across from Documentation.

Better IMHO would be to squash this with the previous patch, so we get a
working test under selftests in a single commit.

cheers

[PATCH] net: Remove NO_IRQ from powerpc-only network drivers

[Michael Ellerman]

We'd like to eventually remove NO_IRQ on powerpc, so remove usages of it
from powerpc-only drivers.

Signed-off-by: Michael Ellerman <m...@ellerman.id.au>
---
 drivers/net/ethernet/freescale/fman/fman_mac.h   |  2 +-
 drivers/net/ethernet/freescale/fs_enet/mac-fcc.c |  2 +-
 drivers/net/ethernet/freescale/fs_enet/mac-fec.c |  2 +-
 drivers/net/ethernet/freescale/fs_enet/mac-scc.c |  2 +-
 drivers/net/ethernet/ibm/emac/core.c | 10 +-
 drivers/net/ethernet/ibm/emac/mal.c  |  5 ++---
 drivers/net/ethernet/ibm/ibmvnic.c   |  4 ++--
 drivers/net/ethernet/toshiba/ps3_gelic_net.c |  4 ++--
 8 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fman/fman_mac.h 
b/drivers/net/ethernet/freescale/fman/fman_mac.h
index 8ddeedbcef9c..ddf0260176c9 100644
--- a/drivers/net/ethernet/freescale/fman/fman_mac.h
+++ b/drivers/net/ethernet/freescale/fman/fman_mac.h
@@ -192,7 +192,7 @@ struct fman_mac_params {
/* A handle to the FM object this port related to */
void *fm;
/* MDIO exceptions interrupt source - not valid for all
-* MACs; MUST be set to 'NO_IRQ' for MACs that don't have
+* MACs; MUST be set to 0 for MACs that don't have
 * mdio-irq, or for polling
 */
void *dev_id; /* device cookie used by the exception cbs */
diff --git a/drivers/net/ethernet/freescale/fs_enet/mac-fcc.c 
b/drivers/net/ethernet/freescale/fs_enet/mac-fcc.c
index d71761a34022..7a3277419876 100644
--- a/drivers/net/ethernet/freescale/fs_enet/mac-fcc.c
+++ b/drivers/net/ethernet/freescale/fs_enet/mac-fcc.c
@@ -90,7 +90,7 @@ static int do_pd_setup(struct fs_enet_private *fep)
int ret = -EINVAL;
 
fep->interrupt = irq_of_parse_and_map(ofdev->dev.of_node, 0);
-   if (fep->interrupt == NO_IRQ)
+   if (!fep->interrupt)
goto out;
 
fep->fcc.fccp = of_iomap(ofdev->dev.of_node, 0);
diff --git a/drivers/net/ethernet/freescale/fs_enet/mac-fec.c 
b/drivers/net/ethernet/freescale/fs_enet/mac-fec.c
index 35a318ed3a62..a09ba70cf09f 100644
--- a/drivers/net/ethernet/freescale/fs_enet/mac-fec.c
+++ b/drivers/net/ethernet/freescale/fs_enet/mac-fec.c
@@ -99,7 +99,7 @@ static int do_pd_setup(struct fs_enet_private *fep)
struct platform_device *ofdev = to_platform_device(fep->dev);
 
fep->interrupt = irq_of_parse_and_map(ofdev->dev.of_node, 0);
-   if (fep->interrupt == NO_IRQ)
+   if (!fep->interrupt)
return -EINVAL;
 
fep->fec.fecp = of_iomap(ofdev->dev.of_node, 0);
diff --git a/drivers/net/ethernet/freescale/fs_enet/mac-scc.c 
b/drivers/net/ethernet/freescale/fs_enet/mac-scc.c
index e8b9c33d35b4..69654fa67ca0 100644
--- a/drivers/net/ethernet/freescale/fs_enet/mac-scc.c
+++ b/drivers/net/ethernet/freescale/fs_enet/mac-scc.c
@@ -99,7 +99,7 @@ static int do_pd_setup(struct fs_enet_private *fep)
struct platform_device *ofdev = to_platform_device(fep->dev);
 
fep->interrupt = irq_of_parse_and_map(ofdev->dev.of_node, 0);
-   if (fep->interrupt == NO_IRQ)
+   if (!fep->interrupt)
return -EINVAL;
 
fep->scc.sccp = of_iomap(ofdev->dev.of_node, 0);
diff --git a/drivers/net/ethernet/ibm/emac/core.c 
b/drivers/net/ethernet/ibm/emac/core.c
index 4c9771d57d6e..ec4d0f3b7850 100644
--- a/drivers/net/ethernet/ibm/emac/core.c
+++ b/drivers/net/ethernet/ibm/emac/core.c
@@ -2750,7 +2750,7 @@ static int emac_probe(struct platform_device *ofdev)
/* Get interrupts. EMAC irq is mandatory, WOL irq is optional */
dev->emac_irq = irq_of_parse_and_map(np, 0);
dev->wol_irq = irq_of_parse_and_map(np, 1);
-   if (dev->emac_irq == NO_IRQ) {
+   if (!dev->emac_irq) {
printk(KERN_ERR "%s: Can't map main interrupt\n", 
np->full_name);
goto err_free;
}
@@ -2913,9 +2913,9 @@ static int emac_probe(struct platform_device *ofdev)
  err_reg_unmap:
iounmap(dev->emacp);
  err_irq_unmap:
-   if (dev->wol_irq != NO_IRQ)
+   if (dev->wol_irq)
irq_dispose_mapping(dev->wol_irq);
-   if (dev->emac_irq != NO_IRQ)
+   if (dev->emac_irq)
irq_dispose_mapping(dev->emac_irq);
  err_free:
free_netdev(ndev);
@@ -2957,9 +2957,9 @@ static int emac_remove(struct platform_device *ofdev)
emac_dbg_unregister(dev);
iounmap(dev->emacp);
 
-   if (dev->wol_irq != NO_IRQ)
+   if (dev->wol_irq)
irq_dispose_mapping(dev->wol_irq);
-   if (dev->emac_irq != NO_IRQ)
+   if (dev->emac_irq)
irq_dispose_mapping(dev->emac_irq);
 
free_netdev(dev->ndev);
diff --git a/drivers/net/ethernet/ibm/emac/mal.c 
b/drivers/net/ethernet/ibm/emac/mal.c
index fdb5cdb3cd15..aaf6fec566b5 100644
--- a/drivers/net/

Re: [PATCHv2, 2/7] ppc: bpf/jit: Fix/enhance 32-bit Load Immediate implementation

[Michael Ellerman]

On Wed, 2016-22-06 at 16:25:02 UTC, "Naveen N. Rao" wrote:
> The existing LI32() macro can sometimes result in a sign-extended 32-bit
> load that does not clear the top 32-bits properly. As an example,
> loading 0x7fff results in the register containing
> 0x7fff. While this does not impact classic BPF JIT
> implementation (since that only uses the lower word for all operations),
> we would like to share this macro between classic BPF JIT and extended
> BPF JIT, wherein the entire 64-bit value in the register matters. Fix
> this by first doing a shifted LI followed by ORI.
> 
> An additional optimization is with loading values between -32768 to -1,
> where we now only need a single LI.
> 
> The new implementation now generates the same or less number of
> instructions.
> 
> Acked-by: Alexei Starovoitov 
> Signed-off-by: Naveen N. Rao 

Series applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/aaf2f7e09932a08c1287d8e4c6

cheers

Re: [PATCHv2,1/7] ppc bpf/jit: Disable classic BPF JIT on ppc64le

[Michael Ellerman]

On Wed, 2016-22-06 at 16:25:01 UTC, "Naveen N. Rao" wrote:
> Classic BPF JIT was never ported completely to work on little endian
> powerpc. However, it can be enabled and will crash the system when used.
> As such, disable use of BPF JIT on ppc64le.
> 
> Reported-by: Thadeu Lima de Souza Cascardo 
> Signed-off-by: Naveen N. Rao 
> Acked-by: Thadeu Lima de Souza Cascardo 

Applied to powerpc fixes, thanks.

https://git.kernel.org/powerpc/c/844e3be47693f92a108cb1fb3b

cheers

Re: [PATCH 6/6] ppc: ebpf/jit: Implement JIT compiler for extended BPF

[Michael Ellerman]

On Tue, 2016-06-07 at 19:02 +0530, Naveen N. Rao wrote:

> PPC64 eBPF JIT compiler.
> 
> Enable with:
> echo 1 > /proc/sys/net/core/bpf_jit_enable
> or
> echo 2 > /proc/sys/net/core/bpf_jit_enable
> 
> ... to see the generated JIT code. This can further be processed with
> tools/net/bpf_jit_disasm.
> 
> With CONFIG_TEST_BPF=m and 'modprobe test_bpf':
> test_bpf: Summary: 305 PASSED, 0 FAILED, [297/297 JIT'ed]
> 
> ... on both ppc64 BE and LE.
> 
> The details of the approach are documented through various comments in
> the code.

This is crashing for me on a Cell machine, not sure why at a glance:


test_bpf: #250 JMP_JSET_X: if (0x3 & 0x) return 1 jited:1 14 PASS
test_bpf: #251 JMP_JA: Jump, gap, jump, ... jited:1 15 PASS
test_bpf: #252 BPF_MAXINSNS: Maximum possible literals 
Unable to handle kernel paging request for data at address 0xd7b2
Faulting instruction address: 0xc0667b6c
cpu 0x0: Vector: 300 (Data Access) at [c007f83bf3a0]
pc: c0667b6c: .flush_icache_range+0x3c/0x84
lr: c0082354: .bpf_int_jit_compile+0x1fc/0x2c8
sp: c007f83bf620
   msr: 9200b032
   dar: d7b2
 dsisr: 4000
  current = 0xc007f8249580
  paca= 0xcfff   softe: 0irq_happened: 0x01
pid   = 1822, comm = insmod
Linux version 4.7.0-rc3-00061-g007c99b9d8c1 (mich...@ka3.ozlabs.ibm.com) (gcc 
version 6.1.0 (GCC) ) #3 SMP Wed Jun 22 19:22:23 AEST 2016
enter ? for help
[link register   ] c0082354 .bpf_int_jit_compile+0x1fc/0x2c8
[c007f83bf620] c00822fc .bpf_int_jit_compile+0x1a4/0x2c8 
(unreliable)
[c007f83bf700] c013cda4 .bpf_prog_select_runtime+0x24/0x108
[c007f83bf780] c0548918 .bpf_prepare_filter+0x9b0/0x9e8
[c007f83bf830] c05489d4 .bpf_prog_create+0x84/0xd0
[c007f83bf8c0] d3b21158 .test_bpf_init+0x28c/0x83c [test_bpf]
[c007f83bfa00] c000a7b4 .do_one_initcall+0x5c/0x1c0
[c007f83bfae0] c0669058 .do_init_module+0x80/0x21c
[c007f83bfb80] c011e3a0 .load_module+0x2028/0x23a8
[c007f83bfd20] c011e898 .SyS_init_module+0x178/0x1b0
[c007f83bfe30] c0009220 system_call+0x38/0x110
--- Exception: c01 (System Call) at 0ff5e0c4
SP (ffde0960) is in userspace
0:mon> r
R00 = c01c   R16 = 
R01 = c007f83bf620   R17 = 024000c0
R02 = c094ce00   R18 = 
R03 = d7b1   R19 = d3c32df0
R04 = d7b40338   R20 = c072b488
R05 = 007f   R21 = d7b1
R06 = d7b2   R22 = c098184c
R07 = 0080   R23 = 
R08 = 0607   R24 = 000300e0
R09 = 0007   R25 = c020
R10 = c0861ee0   R26 = d7b10270
R11 = c06755f8   R27 = c007fe0e
R12 = d7b10270   R28 = 2003
R13 = cfff   R29 = c007f83bf690
R14 = d3c32d61   R30 = 0003
R15 =    R31 = d7ae
pc  = c0667b6c .flush_icache_range+0x3c/0x84
lr  = c0082354 .bpf_int_jit_compile+0x1fc/0x2c8
msr = 9200b032   cr  = 44000248
ctr = 0407   xer = 2000   trap =  300
dar = d7b2   dsisr = 4000
0:mon> S
msr  = 90001032  sprg0= 8001
pvr  = 00703000  sprg1= cfff
dec  = 9f2d8ba4  sprg2= cfff
sp   = c007f83bed30  sprg3= 
toc  = c094ce00  dar  = d7b2
0:mon> u
SLB contents of cpu 0x0
00 c800 af32f5079500 256M ESID=c  VSID=af32f5079 
LLP:100 
01 d800 836935091510 256M ESID=d  VSID=836935091 
LLP:110 
02 c007f800 b52186c20500 256M ESID=c007f  VSID=b52186c20 
LLP:100 
03 c003f000 b224435e0500
04 c007f000 b52186c20500
05 c003f000 b224435e0500
06 c007f000 b52186c20500
07 c003f000 b224435e0500
08 c007f000 b52186c20500
09 c003f000 b224435e0500
10 c007f000 b52186c20500
11 c003f000 b224435e0500
12 c007f000 b52186c20500
13 c003f000 b224435e0500
14 c007f000 b52186c20500
15 c003f000 b224435e0500
16 c007f000 b52186c20500
17 c0007800 af86a8668500 256M ESID=c0007  VSID=af86a8668 
LLP:100 
18 c003f000 b224435e0500
19 c007f000 b52186c20500
20 c003f000 b224435e0500
21 c007f000 b52186c20500
22 c003f000 b224435e0500
23 c007f000 b52186c20500
24 c003f000 b224435e0500
25 c007f000 b52186c20500
26 c003f000 b224435e0500
27 c007f000 b52186c20500
28 c003f000 b224435e0500
29 c007f000 b52186c20500
30 c003f000 b224435e0500
31 c007f000 

Re: [PATCH] ppc: Fix BPF JIT for ABIv2

[Michael Ellerman]

On Tue, 2016-06-21 at 14:28 +0530, Naveen N. Rao wrote:
> On 2016/06/20 03:56PM, Thadeu Lima de Souza Cascardo wrote:
> > On Sun, Jun 19, 2016 at 11:19:14PM +0530, Naveen N. Rao wrote:
> > > On 2016/06/17 10:00AM, Thadeu Lima de Souza Cascardo wrote:
> > > > 
> > > > Hi, Michael and Naveen.
> > > > 
> > > > I noticed independently that there is a problem with BPF JIT and ABIv2, 
> > > > and
> > > > worked out the patch below before I noticed Naveen's patchset and the 
> > > > latest
> > > > changes in ppc tree for a better way to check for ABI versions.
> > > > 
> > > > However, since the issue described below affect mainline and stable 
> > > > kernels,
> > > > would you consider applying it before merging your two patchsets, so 
> > > > that we can
> > > > more easily backport the fix?
> > > 
> > > Hi Cascardo,
> > > Given that this has been broken on ABIv2 since forever, I didn't bother 
> > > fixing it. But, I can see why this would be a good thing to have for 
> > > -stable and existing distros. However, while your patch below may fix 
> > > the crash you're seeing on ppc64le, it is not sufficient -- you'll need 
> > > changes in bpf_jit_asm.S as well.
> > 
> > Hi, Naveen.
> > 
> > Any tips on how to exercise possible issues there? Or what changes you think
> > would be sufficient?
> 
> The calling convention is different with ABIv2 and so we'll need changes 
> in bpf_slow_path_common() and sk_negative_common().
> 
> However, rather than enabling classic JIT for ppc64le, are we better off 
> just disabling it?
> 
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -128,7 +128,7 @@ config PPC
> select IRQ_FORCED_THREADING
> select HAVE_RCU_TABLE_FREE if SMP
> select HAVE_SYSCALL_TRACEPOINTS
> -   select HAVE_CBPF_JIT
> +   select HAVE_CBPF_JIT if CPU_BIG_ENDIAN
> select HAVE_ARCH_JUMP_LABEL
> select ARCH_HAVE_NMI_SAFE_CMPXCHG
> select ARCH_HAS_GCOV_PROFILE_ALL
> 
> 
> Michael,
> Let me know your thoughts on whether you intend to take this patch or 
> Cascardo's patch for -stable before the eBPF patches. I can redo my 
> patches accordingly.

Can one of you send me a proper version of this patch, with change log and
sign-off etc.

cheers

Re: [PATCH] ppc: Fix BPF JIT for ABIv2

[Michael Ellerman]

On Fri, 2016-06-17 at 10:00 -0300, Thadeu Lima de Souza Cascardo wrote:
> From a984dc02b6317a1d3a3c2302385adba5227be5bd Mon Sep 17 00:00:00 2001
> From: Thadeu Lima de Souza Cascardo 
> Date: Wed, 15 Jun 2016 13:22:12 -0300
> Subject: [PATCH] ppc: Fix BPF JIT for ABIv2
> 
> ABIv2 used for ppc64le does not use function descriptors. Without this patch,
> whenever BPF JIT is enabled, we get a crash as below.
> 
...

> diff --git a/arch/powerpc/net/bpf_jit.h b/arch/powerpc/net/bpf_jit.h
> index 889fd19..28b89ed 100644
> --- a/arch/powerpc/net/bpf_jit.h
> +++ b/arch/powerpc/net/bpf_jit.h
> @@ -70,7 +70,7 @@ DECLARE_LOAD_FUNC(sk_load_half);
>  DECLARE_LOAD_FUNC(sk_load_byte);
>  DECLARE_LOAD_FUNC(sk_load_byte_msh);
>  
> -#ifdef CONFIG_PPC64
> +#if defined(CONFIG_PPC64) && (!defined(_CALL_ELF) || _CALL_ELF != 2)
>  #define FUNCTION_DESCR_SIZE  24
>  #else
>  #define FUNCTION_DESCR_SIZE  0
> diff --git a/arch/powerpc/net/bpf_jit_comp.c b/arch/powerpc/net/bpf_jit_comp.c
> index 2d66a84..035b887 100644
> --- a/arch/powerpc/net/bpf_jit_comp.c
> +++ b/arch/powerpc/net/bpf_jit_comp.c
> @@ -664,7 +664,7 @@ void bpf_jit_compile(struct bpf_prog *fp)
>  
>   if (image) {
>   bpf_flush_icache(code_base, code_base + (proglen/4));
> -#ifdef CONFIG_PPC64
> +#if defined(CONFIG_PPC64) && (!defined(_CALL_ELF) || _CALL_ELF != 2)
>   /* Function descriptor nastiness: Address + TOC */
>   ((u64 *)image)[0] = (u64)code_base;
>   ((u64 *)image)[1] = local_paca->kernel_toc;


Confirmed that even with this patch we still crash:

  # echo 1 > /proc/sys/net/core/bpf_jit_enable
  # modprobe test_bpf
  BPF filter opcode 0020 (@3) unsupported
  BPF filter opcode 0020 (@2) unsupported
  BPF filter opcode 0020 (@0) unsupported
  Unable to handle kernel paging request for data at address 0xd54f65e8
  Faulting instruction address: 0xc08765f8
  cpu 0x0: Vector: 300 (Data Access) at [c34f3480]
  pc: c08765f8: skb_copy_bits+0x158/0x330
  lr: c008fb7c: bpf_slow_path_byte+0x28/0x54
  sp: c34f3700
 msr: 80010280b033
 dar: d54f65e8
   dsisr: 4000
current = 0xc001f857d8d0
paca= 0xc7b8 softe: 0irq_happened: 0x01
  pid   = 2993, comm = modprobe
  Linux version 4.7.0-rc3-00055-g9497a1c1c5b4-dirty 
(mich...@ka3.ozlabs.ibm.com) () #30 SMP Wed Jun 22 15:06:58 AEST 2016
  enter ? for help
  [c34f3770] c008fb7c bpf_slow_path_byte+0x28/0x54
  [c34f37e0] d7bb004c
  [c34f3900] d5331668 test_bpf_init+0x5fc/0x7f8 [test_bpf]
  [c34f3a30] c000b628 do_one_initcall+0x68/0x1d0
  [c34f3af0] c09beb24 do_init_module+0x90/0x240
  [c34f3b80] c01642bc load_module+0x206c/0x22f0
  [c34f3d30] c01648b0 SyS_finit_module+0x120/0x180
  [c34f3e30] c0009260 system_call+0x38/0x108
  --- Exception: c01 (System Call) at 3fff7ffa2db4


cheers

Re: [PATCH] ppc: Fix BPF JIT for ABIv2

[Michael Ellerman]

On Tue, 2016-06-21 at 08:45 -0700, Alexei Starovoitov wrote:
> On 6/21/16 7:47 AM, Thadeu Lima de Souza Cascardo wrote:
> > > > 
> > > > The calling convention is different with ABIv2 and so we'll need changes
> > > > in bpf_slow_path_common() and sk_negative_common().
> > > 
> > > How big would those changes be? Do we know?
> > > 
> > > How come no one reported this was broken previously? This is the first 
> > > I've
> > > heard of it being broken.
> > > 
> > 
> > I just heard of it less than two weeks ago, and only could investigate it 
> > last
> > week, when I realized mainline was also affected.
> > 
> > It looks like the little-endian support for classic JIT were done before the
> > conversion to ABIv2. And as JIT is disabled by default, no one seems to have
> > exercised it.
> 
> it's not a surprise unfortunately. The JITs that were written before
> test_bpf.ko was developed were missing corner cases. Typical tcpdump
> would be fine, but fragmented packets, negative offsets and
> out-out-bounds wouldn't be handled correctly.
> I'd suggest to validate the stable backport with test_bpf as well.
 
OK thanks.

I have been running seltests/net/test_bpf, but I realise now it doesn't enable
the JIT.

cheers

Re: [PATCH] ppc: Fix BPF JIT for ABIv2

[Michael Ellerman]

On Tue, 2016-06-21 at 14:28 +0530, Naveen N. Rao wrote:
> On 2016/06/20 03:56PM, Thadeu Lima de Souza Cascardo wrote:
> > On Sun, Jun 19, 2016 at 11:19:14PM +0530, Naveen N. Rao wrote:
> > > On 2016/06/17 10:00AM, Thadeu Lima de Souza Cascardo wrote:
> > > > 
> > > > Hi, Michael and Naveen.
> > > > 
> > > > I noticed independently that there is a problem with BPF JIT and ABIv2, 
> > > > and
> > > > worked out the patch below before I noticed Naveen's patchset and the 
> > > > latest
> > > > changes in ppc tree for a better way to check for ABI versions.
> > > > 
> > > > However, since the issue described below affect mainline and stable 
> > > > kernels,
> > > > would you consider applying it before merging your two patchsets, so 
> > > > that we can
> > > > more easily backport the fix?
> > > 
> > > Hi Cascardo,
> > > Given that this has been broken on ABIv2 since forever, I didn't bother 
> > > fixing it. But, I can see why this would be a good thing to have for 
> > > -stable and existing distros. However, while your patch below may fix 
> > > the crash you're seeing on ppc64le, it is not sufficient -- you'll need 
> > > changes in bpf_jit_asm.S as well.
> > 
> > Hi, Naveen.
> > 
> > Any tips on how to exercise possible issues there? Or what changes you think
> > would be sufficient?
> 
> The calling convention is different with ABIv2 and so we'll need changes 
> in bpf_slow_path_common() and sk_negative_common().

How big would those changes be? Do we know?

How come no one reported this was broken previously? This is the first I've
heard of it being broken.

> However, rather than enabling classic JIT for ppc64le, are we better off 
> just disabling it?
> 
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -128,7 +128,7 @@ config PPC
> select IRQ_FORCED_THREADING
> select HAVE_RCU_TABLE_FREE if SMP
> select HAVE_SYSCALL_TRACEPOINTS
> -   select HAVE_CBPF_JIT
> +   select HAVE_CBPF_JIT if CPU_BIG_ENDIAN
> select HAVE_ARCH_JUMP_LABEL
> select ARCH_HAVE_NMI_SAFE_CMPXCHG
> select ARCH_HAS_GCOV_PROFILE_ALL
> 
> 
> Michael,
> Let me know your thoughts on whether you intend to take this patch or 
> Cascardo's patch for -stable before the eBPF patches. I can redo my 
> patches accordingly.

This patch sounds like the best option at the moment for something we can
backport. Unless the changes to fix it are minimal.

cheers

Re: [6/6] ppc: ebpf/jit: Implement JIT compiler for extended BPF

[Michael Ellerman]

On Tue, 2016-06-21 at 12:28 +0530, Naveen N. Rao wrote:
> On 2016/06/21 09:38AM, Michael Ellerman wrote:
> > On Sun, 2016-06-19 at 23:06 +0530, Naveen N. Rao wrote:
> > > 
> > > #include 
> > > 
> > > in bpf_jit_comp64.c
> > > 
> > > Can you please check if it resolves the build error?
> > 
> > Can you? :D
> 
> :)
> Sorry, I should have explained myself better. I did actually try your 
> config and I was able to reproduce the build error. After the above 
> #include, that error went away, but I saw some vdso related errors. I 
> thought I was doing something wrong and needed a different setup for 
> that particular kernel config, which is why I requested your help in the 
> matter. I just didn't do a good job of putting across that message...
 
Ah OK. Not sure why you're seeing VDSO errors?

> Note to self: randconfig builds *and* more time drafting emails :)

No stress. You don't need to do randconfig builds, or even build all the
arch/powerpc/ configs, just try to do a reasonable set, something like - ppc64,
powernv, pseries, pmac32, ppc64e.

I'm happy to catch the esoteric build failures.

> Do you want me to respin the patches?

No that's fine, I'll fix it up here.

cheers

Re: [6/6] ppc: ebpf/jit: Implement JIT compiler for extended BPF

[Michael Ellerman]

On Sun, 2016-06-19 at 23:06 +0530, Naveen N. Rao wrote:
> On 2016/06/17 10:53PM, Michael Ellerman wrote:
> > On Tue, 2016-07-06 at 13:32:23 UTC, "Naveen N. Rao" wrote:
> > > diff --git a/arch/powerpc/net/bpf_jit_comp64.c 
> > > b/arch/powerpc/net/bpf_jit_comp64.c
> > > new file mode 100644
> > > index 000..954ff53
> > > --- /dev/null
> > > +++ b/arch/powerpc/net/bpf_jit_comp64.c
> > > @@ -0,0 +1,956 @@
> > ...

> > > +
> > > +static void bpf_jit_fill_ill_insns(void *area, unsigned int size)
> > > +{
> > > + int *p = area;
> > > +
> > > + /* Fill whole space with trap instructions */
> > > + while (p < (int *)((char *)area + size))
> > > + *p++ = BREAKPOINT_INSTRUCTION;
> > > +}
> > 
> > This breaks the build for some configs, presumably you're missing a header:
> > 
> >   arch/powerpc/net/bpf_jit_comp64.c:30:10: error: 'BREAKPOINT_INSTRUCTION' 
> > undeclared (first use in this function)
> > 
> > http://kisskb.ellerman.id.au/kisskb/buildresult/12720611/
> 
> Oops. Yes, I should have caught that. I need to add:
> 
> #include 
> 
> in bpf_jit_comp64.c
> 
> Can you please check if it resolves the build error?

Can you? :D

cheers

Re: [6/6] ppc: ebpf/jit: Implement JIT compiler for extended BPF

[Michael Ellerman]

On Tue, 2016-07-06 at 13:32:23 UTC, "Naveen N. Rao" wrote:
> diff --git a/arch/powerpc/net/bpf_jit_comp64.c 
> b/arch/powerpc/net/bpf_jit_comp64.c
> new file mode 100644
> index 000..954ff53
> --- /dev/null
> +++ b/arch/powerpc/net/bpf_jit_comp64.c
> @@ -0,0 +1,956 @@
...
> +
> +static void bpf_jit_fill_ill_insns(void *area, unsigned int size)
> +{
> + int *p = area;
> +
> + /* Fill whole space with trap instructions */
> + while (p < (int *)((char *)area + size))
> + *p++ = BREAKPOINT_INSTRUCTION;
> +}

This breaks the build for some configs, presumably you're missing a header:

  arch/powerpc/net/bpf_jit_comp64.c:30:10: error: 'BREAKPOINT_INSTRUCTION' 
undeclared (first use in this function)

http://kisskb.ellerman.id.au/kisskb/buildresult/12720611/

cheers

Re: [PATCH 3/3] param: convert some "on"/"off" users to strtobool

[Michael Ellerman]

On Thu, 2016-01-28 at 06:17 -0800, Kees Cook wrote:

> This changes several users of manual "on"/"off" parsing to use strtobool.

You should probably point out that it's a slight behaviour change for some
users. ie. parameters that previously *only* worked with "on"/"off", can now
also take 0/1/y/n etc.

But I don't think that's a show stopper.


> Signed-off-by: Kees Cook <keesc...@chromium.org>
> Cc: x...@kernel.org
> Cc: linuxppc-...@lists.ozlabs.org
> Cc: linux-s...@vger.kernel.org
> ---
>  arch/powerpc/kernel/rtasd.c  | 10 +++---
>  arch/powerpc/platforms/pseries/hotplug-cpu.c | 11 +++

Acked-by: Michael Ellerman <m...@ellerman.id.au> (powerpc)


cheers

Re: sbc8641: drop bogus PHY IRQ entries from DTS file

[Michael Ellerman]

On Tue, 2015-08-12 at 22:44:02 UTC, Paul Gortmaker wrote:
> This file was originally cloned off of the MPC8641D-HPCN reference
> platform, which actually had a PHY IRQ line connected.  However
> this board does not.  The bogus entry was largely inert and went
> undetected until commit 321beec5047af83db90c88114b7e664b156f49fe
> ("net: phy: Use interrupts when available in NOLINK state") was
> added to the tree.
> 
> With the above commit, the board fails to NFS boot since it sits
> waiting for a PHY IRQ event that of course never arrives.  Removing
> the bogus entries from the DTS file fixes the issue.
> 
> Cc: Andrew Lunn 
> Signed-off-by: Paul Gortmaker 

Applied to powerpc fixes, thanks.

https://git.kernel.org/powerpc/c/5b01310cfc8d2302dcca1d8d

cheers
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] sbc8641: drop bogus PHY IRQ entries from DTS file

[Michael Ellerman]

On Tue, 2015-12-08 at 17:44 -0500, Paul Gortmaker wrote:

> This file was originally cloned off of the MPC8641D-HPCN reference
> platform, which actually had a PHY IRQ line connected.  However
> this board does not.  The bogus entry was largely inert and went
> undetected until commit 321beec5047af83db90c88114b7e664b156f49fe
> ("net: phy: Use interrupts when available in NOLINK state") was
> added to the tree.
> 
> With the above commit, the board fails to NFS boot since it sits
> waiting for a PHY IRQ event that of course never arrives.  Removing
> the bogus entries from the DTS file fixes the issue.

Commit 321beec5047a ("net: phy: Use interrupts when available in NOLINK state")
went into 4.4-rc2, so this is a fix for 4.4 right?

cheers

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] sbc8641: drop bogus PHY IRQ entries from DTS file

[Michael Ellerman]

On Tue, 2015-12-08 at 21:40 -0500, David Miller wrote:
>  From: Paul Gortmaker 
> Date: Tue, 8 Dec 2015 17:44:02 -0500
> > This file was originally cloned off of the MPC8641D-HPCN reference
> > platform, which actually had a PHY IRQ line connected.  However
> > this board does not.  The bogus entry was largely inert and went
> > undetected until commit 321beec5047af83db90c88114b7e664b156f49fe
> > ("net: phy: Use interrupts when available in NOLINK state") was
> > added to the tree.
> >
> > With the above commit, the board fails to NFS boot since it sits
> > waiting for a PHY IRQ event that of course never arrives.  Removing
> > the bogus entries from the DTS file fixes the issue.
> >
> > Cc: Andrew Lunn 
> > Signed-off-by: Paul Gortmaker 
>
> I'm assuming this will go via the powerpc tree, not mine.

Yep I'll take it.

cheers

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] sbc8641: drop bogus PHY IRQ entries from DTS file

[Michael Ellerman]

On Tue, 2015-12-08 at 21:04 -0500, Paul Gortmaker wrote:
> [Re: [PATCH] sbc8641: drop bogus PHY IRQ entries from DTS file] On 09/12/2015 
> (Wed 12:10) Michael Ellerman wrote:
> > On Tue, 2015-12-08 at 17:44 -0500, Paul Gortmaker wrote:
> > > This file was originally cloned off of the MPC8641D-HPCN reference
> > > platform, which actually had a PHY IRQ line connected.  However
> > > this board does not.  The bogus entry was largely inert and went
> > > undetected until commit 321beec5047af83db90c88114b7e664b156f49fe
> > > ("net: phy: Use interrupts when available in NOLINK state") was
> > > added to the tree.
> > >
> > > With the above commit, the board fails to NFS boot since it sits
> > > waiting for a PHY IRQ event that of course never arrives.  Removing
> > > the bogus entries from the DTS file fixes the issue.
> >
> > Commit 321beec5047a ("net: phy: Use interrupts when available in NOLINK 
> > state")
> > went into 4.4-rc2, so this is a fix for 4.4 right?
>
> Correct, but I'm guessing there are not thousands of users of this board
> out there, so it probably doesn't matter a whole lot if it goes in the
> same release where the regression happened or one release later...

OK sure. As it happens I'm just prepping a fixes branch, so I'll drop it in.

cheers

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [P.A. Semi] Does the ethernet interface work on your Electra, Chitra, Nemo, and Athena board?

[Michael Ellerman]

On Wed, 2015-12-02 at 05:59 +0100, Christian Zigotzky wrote:
> Hi all,
> 
> We tested some 4.3 kernels on a P.A. Semi reference board. Ultimately, 
> ethernet does not work, though on the reference board, the interface is 
> detected, gets link, but will not pass any packets/traffic.
> 
> Somewhere in the source code of the commit "'powerpc-4.3-1' of 
> git://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux (2015-09-03 
> 23:41:38 (GMT)" is the problem.
> 
> Could you please help us to solve the problem?

Hi Christian,

Unfortunately there is no source code in that commit, it's just a merge. And
your bisect run found that the very first commit in that merge was OK:

> > [390fd5929f52bdfb9dfcc03820041ba556780f4a] cxl: Set up and enable PSL 
> > git bisect good

So I'm not convinced your bisect run was correct, sorry.

My pasemi is booting OK, but I haven't had time to test the network. If you
remind me next week I will make sure network is working on my board.

Are you testing your board with your additional patch as well? Or just vanilla
mainline?

cheers

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] e1000e: Move e1000e_disable_aspm_locked() inside CONFIG_PM

[Michael Ellerman]

On Thu, 2015-07-23 at 07:07 -0700, Jeff Kirsher wrote:
 On Wed, 2015-07-22 at 11:41 +1000, Michael Ellerman wrote:
  On Wed, 2015-07-15 at 03:30 -0700, Jeff Kirsher wrote:
   On Tue, 2015-07-14 at 13:54 +1000, Michael Ellerman wrote:
e1000e_disable_aspm_locked() is only used in __e1000_resume()
  which is
inside CONFIG_PM. So when CONFIG_PM=n we get a defined but not
  used
warning for e1000e_disable_aspm_locked().

Move it inside the existing CONFIG_PM block to avoid the warning.

Signed-off-by: Michael Ellerman m...@ellerman.id.au
---
 drivers/net/ethernet/intel/e1000e/netdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
   
   NACK, this is already fixed in my next-queue tree.  Raanan submitted
  a
   patch back on July 6th to resolve this issue, see commit id
   a75787d2246a93d256061db602f252703559af65 in my dev-queue branch of
  my
   next-queue tree.
  
  OK. I take it your next-queue is destined for 4.3, so we'll just have
  to suck
  on the warning until then?
 
 Yes, but I can queue Raanan's patch up for 4.2 (and possibly stable) if
 necessary.  I have no issue with doing that.

For 4.2 would be nice, it would make my builds green again. But it's not the
end of the world if it has to wait until 4.3.

cheers


--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] e1000e: Move e1000e_disable_aspm_locked() inside CONFIG_PM

[Michael Ellerman]

On Wed, 2015-07-15 at 03:30 -0700, Jeff Kirsher wrote:
 On Tue, 2015-07-14 at 13:54 +1000, Michael Ellerman wrote:
  e1000e_disable_aspm_locked() is only used in __e1000_resume() which is
  inside CONFIG_PM. So when CONFIG_PM=n we get a defined but not used
  warning for e1000e_disable_aspm_locked().
  
  Move it inside the existing CONFIG_PM block to avoid the warning.
  
  Signed-off-by: Michael Ellerman m...@ellerman.id.au
  ---
   drivers/net/ethernet/intel/e1000e/netdev.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)
 
 NACK, this is already fixed in my next-queue tree.  Raanan submitted a
 patch back on July 6th to resolve this issue, see commit id
 a75787d2246a93d256061db602f252703559af65 in my dev-queue branch of my
 next-queue tree.

OK. I take it your next-queue is destined for 4.3, so we'll just have to suck
on the warning until then?

cheers


--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] e1000e: Move e1000e_disable_aspm_locked() inside CONFIG_PM

[Michael Ellerman]

e1000e_disable_aspm_locked() is only used in __e1000_resume() which is
inside CONFIG_PM. So when CONFIG_PM=n we get a defined but not used
warning for e1000e_disable_aspm_locked().

Move it inside the existing CONFIG_PM block to avoid the warning.

Signed-off-by: Michael Ellerman m...@ellerman.id.au
---
 drivers/net/ethernet/intel/e1000e/netdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c 
b/drivers/net/ethernet/intel/e1000e/netdev.c
index 89d788d8f263..f1d7fe2ea183 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -6439,6 +6439,7 @@ static void e1000e_disable_aspm(struct pci_dev *pdev, u16 
state)
__e1000e_disable_aspm(pdev, state, 0);
 }
 
+#ifdef CONFIG_PM
 /**
  * e1000e_disable_aspm_locked   Disable ASPM states.
  * @pdev: pointer to PCI device struct
@@ -6452,7 +6453,6 @@ static void e1000e_disable_aspm_locked(struct pci_dev 
*pdev, u16 state)
__e1000e_disable_aspm(pdev, state, 1);
 }
 
-#ifdef CONFIG_PM
 static int __e1000_resume(struct pci_dev *pdev)
 {
struct net_device *netdev = pci_get_drvdata(pdev);
-- 
2.1.0

--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [v2,9/9] fsl/fman: Add FMan MAC driver

[Michael Ellerman]

On Thu, 2015-06-25 at 21:25 -0500, Scott Wood wrote:
 On Fri, 2015-06-26 at 12:21 +1000, Michael Ellerman wrote:
  On Thu, 2015-06-25 at 19:59 -0500, Scott Wood wrote:
   On Fri, 2015-06-26 at 01:06 +0200, Paul Bolle wrote:
(Evolution 3.16 is basically unbearable for replying to patches. 
Anyone
else running into this?) 
   
   If you mean the crazy lag when selecting moderate-to-large amounts of 
   text (for snipping), yes.
  
  I recommend the external editor plugin with vim.
 
 I tried the external editor plugin (not with vim) and it failed to bring the 
 externally made edits back into the evolution compose window.  It then 
 started spastically respawning the external editor without my doing anything.

I guess it only works with vim ;)

cheers


--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [v2,9/9] fsl/fman: Add FMan MAC driver

[Michael Ellerman]

On Thu, 2015-06-25 at 19:59 -0500, Scott Wood wrote:
 On Fri, 2015-06-26 at 01:06 +0200, Paul Bolle wrote:
  (Evolution 3.16 is basically unbearable for replying to patches. 
  Anyone
  else running into this?) 
 
 If you mean the crazy lag when selecting moderate-to-large amounts of 
 text (for snipping), yes.

I recommend the external editor plugin with vim.

cheers


--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ehea: Fix memory hook reference counting crashes

[Michael Ellerman]

On Sat, 2015-04-25 at 14:43 -0400, David Miller wrote:
 From: Michael Ellerman m...@ellerman.id.au
 Date: Fri, 24 Apr 2015 15:52:32 +1000
 
  The recent commit to only register the EHEA memory hotplug hooks on
  adapter probe has a few problems.
  
  Firstly the reference counting is wrong for multiple adapters, in that
  the hooks are registered multiple times. Secondly the check in the tear
  down path is backward. Finally the error path doesn't decrement the
  count.
  
  The multiple registration of the hooks is the biggest problem, as it
  leads to oopses when the system is rebooted, and/or errors during memory
  hotplug, eg:
  ...
  Fixes: aa183323312d (ehea: Register memory hotplug, reboot and crash hooks 
  on adapter probe)
  Signed-off-by: Michael Ellerman m...@ellerman.id.au
 
 Applied, but using an atomic counter for this is really inappropriate
 and is what lead to this bug in the first place.
 
 You're not counting anything, because if you were, then you would be
 decrementing this thing somewhere.
 
 Rather, it's purely a boolean state saying I did X.  So it should be
 a boolean, and no atomicity nor other special considerations are
 needed for setting it to true.

Yeah I agree, it's a mess.

We should be unregistering the hooks when the last adapter is removed, which is
where we'd do the decrement. As it's written the hooks stay registered until
the driver is removed.

I'll try and find time, or someone else with time, to fix it up properly for 
4.2.

cheers


--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] ehea: Fix memory hook reference counting crashes

[Michael Ellerman]

The recent commit to only register the EHEA memory hotplug hooks on
adapter probe has a few problems.

Firstly the reference counting is wrong for multiple adapters, in that
the hooks are registered multiple times. Secondly the check in the tear
down path is backward. Finally the error path doesn't decrement the
count.

The multiple registration of the hooks is the biggest problem, as it
leads to oopses when the system is rebooted, and/or errors during memory
hotplug, eg:

  $ ./mem-on-off-test.sh -r 2
  ...
  ehea: memory is going offline
  ehea: LPAR memory changed - re-initializing driver
  ehea: re-initializing driver complete
  ehea: memory is going offline
  ehea: LPAR memory changed - re-initializing driver
  ehea: opcode=26c ret=fffc arg1=8303 arg2=0 
arg3=7006d600 arg4=3fded arg5=200 arg6=0 arg7=0
  ehea: register_rpage_mr failed
  ehea: registering mr failed
  ehea: register MR failed - driver inoperable!
  ehea: memory is going offline

Fixes: aa183323312d (ehea: Register memory hotplug, reboot and crash hooks on 
adapter probe)
Signed-off-by: Michael Ellerman m...@ellerman.id.au
---
 drivers/net/ethernet/ibm/ehea/ehea_main.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ehea/ehea_main.c 
b/drivers/net/ethernet/ibm/ehea/ehea_main.c
index c05e50759621..00d86be0c831 100644
--- a/drivers/net/ethernet/ibm/ehea/ehea_main.c
+++ b/drivers/net/ethernet/ibm/ehea/ehea_main.c
@@ -3347,7 +3347,7 @@ static int ehea_register_memory_hooks(void)
 {
int ret = 0;
 
-   if (atomic_inc_and_test(ehea_memory_hooks_registered))
+   if (atomic_inc_return(ehea_memory_hooks_registered)  1)
return 0;
 
ret = ehea_create_busmap();
@@ -3381,12 +3381,14 @@ out3:
 out2:
unregister_reboot_notifier(ehea_reboot_nb);
 out:
+   atomic_dec(ehea_memory_hooks_registered);
return ret;
 }
 
 static void ehea_unregister_memory_hooks(void)
 {
-   if (atomic_read(ehea_memory_hooks_registered))
+   /* Only remove the hooks if we've registered them */
+   if (atomic_read(ehea_memory_hooks_registered) == 0)
return;
 
unregister_reboot_notifier(ehea_reboot_nb);
-- 
2.1.0

--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch 1/6] pasemi_mac: Move RX/TX section enablement to dma_lib

[Michael Ellerman]

On Wed, 2008-02-20 at 20:57 -0600, Olof Johansson wrote:
 plain text document attachment (in-progress)
 Also stop both rx and tx sections before changing the configuration of
 the dma device during init.
 
 Signed-off-by: Olof Johansson [EMAIL PROTECTED]
 
 Index: k.org/arch/powerpc/platforms/pasemi/dma_lib.c
 ===
 --- k.org.orig/arch/powerpc/platforms/pasemi/dma_lib.c
 +++ k.org/arch/powerpc/platforms/pasemi/dma_lib.c
 @@ -478,6 +478,30 @@ int pasemi_dma_init(void)
   for (i = 0; i  MAX_RXCH; i++)
   __set_bit(i, rxch_free);
  
 + i = 1000;
 + pasemi_write_dma_reg(PAS_DMA_COM_RXCMD, 0);
 + while ((i  0)  (pasemi_read_dma_reg(PAS_DMA_COM_RXSTA)  1))
 + i--;
 + if (i  0)
 + printk(KERN_INFO Warning: Could not disable RX section\n);
 +
 + i = 1000;
 + pasemi_write_dma_reg(PAS_DMA_COM_TXCMD, 0);
 + while ((i  0)  (pasemi_read_dma_reg(PAS_DMA_COM_TXSTA)  1))
 + i--;

This kind of caught my eye, is it still going to work when the next core
is twice as fast?

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


signature.asc
Description: This is a digitally signed message part

Re: [PATCH 2/2] ehea: add memory remove hotplug support

[Michael Ellerman]

On Mon, 2008-02-04 at 14:04 +0100, Jan-Bernd Themann wrote:
 Add memory remove hotplug support
 
 Signed-off-by: Jan-Bernd Themann [EMAIL PROTECTED]
 
 ---
  Comment: This patch depends on the following patch that
  exports the symbols
   register_memory_notifier()
   unregister_memory_notifier()
 
  http://lkml.org/lkml/2008/2/1/293
 
 
  drivers/net/ehea/ehea_main.c |   25 +
  1 files changed, 25 insertions(+), 0 deletions(-)
 
 diff --git a/drivers/net/ehea/ehea_main.c b/drivers/net/ehea/ehea_main.c
 index 21af674..b75afcc 100644
 --- a/drivers/net/ehea/ehea_main.c
 +++ b/drivers/net/ehea/ehea_main.c

 @@ -3559,6 +3578,10 @@ int __init ehea_module_init(void)
   if (ret)
   ehea_info(failed registering reboot notifier);
  
 + ret = register_memory_notifier(ehea_mem_nb);
 + if (ret)
 + ehea_info(failed registering memory remove notifier);
 
   ret = crash_shutdown_register(ehea_crash_handler);
   if (ret)
   ehea_info(failed registering crash handler);

You don't do anything except print a message if the registration fails.
What happens when someone tries to remove memory but the memory notifier
wasn't registered properly? Bang?

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


signature.asc
Description: This is a digitally signed message part

Re: [PATCH 2/2] ehea: add memory remove hotplug support

[Michael Ellerman]

On Mon, 2008-02-04 at 16:24 +0100, Jan-Bernd Themann wrote:
 On Monday 04 February 2008 15:46, Michael Ellerman wrote:
  On Mon, 2008-02-04 at 14:04 +0100, Jan-Bernd Themann wrote:
   Add memory remove hotplug support
 
   @@ -3559,6 +3578,10 @@ int __init ehea_module_init(void)
 if (ret)
 ehea_info(failed registering reboot notifier);

   + ret = register_memory_notifier(ehea_mem_nb);
   + if (ret)
   + ehea_info(failed registering memory remove notifier);
   
 ret = crash_shutdown_register(ehea_crash_handler);
 if (ret)
 ehea_info(failed registering crash handler);
  
  You don't do anything except print a message if the registration fails.
  What happens when someone tries to remove memory but the memory notifier
  wasn't registered properly? Bang?
 
 In case the registration fails and somebody tries to free memory:
 - Driver will not remove the affected memory from the eHEA memory region
   -- Firmware (phyp) can not free that memory (as marked as used)
   -- Therefore the removed memory could not be used in an other partition
 
 It makes sense to allow the driver to work anyway. Having no ethernet
 would not really be a good alternative.

Yeah I agree. I wasn't clear from the patch that no notifier - no
memory removed. Rather than, no notifier - memory removed and ehea
isn't told about it.

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


signature.asc
Description: This is a digitally signed message part

Re: [RFC] ehea: kdump support using new shutdown hook

[Michael Ellerman]

On Wed, 2007-12-12 at 12:04 -0500, Dave Jones wrote:
 On Wed, Dec 12, 2007 at 05:53:43PM +0100, Thomas Klein wrote:
 
   +static void ehea_update_adapter_handles(struct ehea_adapter *adapter)
   +{
   +  int i, k;
   +  int j = 0;
   +
   +  memset(adapter-res_handles, sizeof(adapter-res_handles), 0);
 
 arguments wrong way around.

Remind me why bzero is deprecated again? :)

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


signature.asc
Description: This is a digitally signed message part

Re: [RFC] ehea: kdump support using new shutdown hook

[Michael Ellerman]

 is asking for trouble, if a single pointer
is corrupted you'll walk off into lala land. Then you're pulling fields
out of structs via pointers, again hoping that they're all valid. Then
you walk another linked list ..  And then you're pulling bits out of the
adapter again.

Ideally it would look like this:

static u64 things_to_free[];

void ehea_crash_deregister(void)
{
for (i = 0; i  num_things_to_free; i++)
h_call_free_a_thing(things_to_free[i]);
}

  static int ehea_reboot_notifier(struct notifier_block *nb,
   unsigned long action, void *unused)
  {
 @@ -3373,6 +3469,9 @@ int __init ehea_module_init(void)
   goto out;
  
   register_reboot_notifier(ehea_reboot_nb);
 + ret = crash_shutdown_register(ehea_crash_deregister);
 + if (ret)
 + ehea_info(failed registering crash handler);

Your naming is a little confusing here, you're registering the
deregister function. I think I get it, you're unregistering the fw
handles, but perhaps ehea_crash_shutdown() would be clearer.

 @@ -3396,10 +3496,15 @@ out:
  
  static void __exit ehea_module_exit(void)
  {
 + int ret;
 +
   flush_scheduled_work();
   driver_remove_file(ehea_driver.driver, driver_attr_capabilities);
   ibmebus_unregister_driver(ehea_driver);
   unregister_reboot_notifier(ehea_reboot_nb);
 + ret = crash_shutdown_unregister(ehea_crash_deregister);
 + if (ret)
 + ehea_info(failed unregistering crash handler);

You don't need ret if that's all you're going to do with it.

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


signature.asc
Description: This is a digitally signed message part

Re: [PATCH] ehea: Add kdump support

[Michael Ellerman]

On Fri, 2007-11-09 at 14:33 +0100, Thomas Klein wrote:
 To support ehea driver reloading in a kdump kernel the driver has to perform
 firmware handle deregistrations when the original kernel crashes. As there's
 currently no notifier chain for machine crashes this patch enables kdump 
 support
 in the ehea driver by bending the ppc_md.machine_crash_shutdown hook to its 
 own
 machine crash handler. The original machine_crash_shutdown() fn is called
 afterwards. This works fine as long as the ehea driver is the only one which
 does so. Problems may occur if other drivers do the same and unload regularly.
 This patch enables 2.6.24-rc2 to use kdump with ehea and only puts a very
 low risk on base kernel. In 2.6.24 we know ehea is the only user of this
 mechanism. The next step for 2.6.25 would be to add a proper notifier chain.
 The full solution might be that register_reboot_notifier() provides sth
 like a SYS_CRASH action. Please apply.
 
 Signed-off-by: Thomas Klein [EMAIL PROTECTED]
 
 ---
  drivers/net/ehea/ehea.h  |2 +-
  drivers/net/ehea/ehea_main.c |   28 
  2 files changed, 29 insertions(+), 1 deletions(-)
 

Hi Thomas,

I'm sorry, but this patch is all wrong IMHO.

For kdump we have to assume that the kernel is fundamentally broken,
we've panicked, so something bad has happened - every line of kernel
code that is run decreases the chance that we'll successfully make it
into the kdump kernel.

So just calling unregister_driver() is no good, that's going to call
lots of code, try to take lots of locks, rely on lots of data structures
being uncorrupted etc. etc.

And the hijacking of machine_crash_shutdown() is IMO not an acceptable
solution, as you say it only works if EHEA is the only driver to do it.
And as soon as EHEA does it other drivers will want to do it.

What we need is the _minimal_ set of actions that can happen to make
EHEA work in the kdump kernel.

Solutions that might be better:

 a) if there are a finite number of handles and we can predict their 
values, just delete them all in the kdump kernel before the driver
loads.
 b) if there are a small  finite number of handles, save their values 
in a device tree property and have the kdump kernel read them and 
delete them before the driver loads.
 c) if neither of those work, provide a minimal routine that _only_
deletes the handles in the crashed kernel.
 d) something else

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


signature.asc
Description: This is a digitally signed message part

Re: [PATCH] ehea: add kexec support

[Michael Ellerman]

On Wed, 2007-10-31 at 20:48 +0100, Christoph Raisch wrote:
 Michael Ellerman [EMAIL PROTECTED] wrote on 30.10.2007 23:50:36:
 
  On Tue, 2007-10-30 at 09:39 +0100, Christoph Raisch wrote:
  
   Michael Ellerman [EMAIL PROTECTED] wrote on 28.10.2007 23:32:17:
   Hope I didn't miss anything here...
 
  Perhaps. When we kdump the kernel does not call the reboot notifiers, so
  the code Jan-Bernd just added won't get called. So the eHEA resources
  won't be freed. When the kdump kernel tries to load the eHEA driver what
  will happen?
 
 Good point.
 
 If the device driver tries to allocate resources again (in the kdump
 kernel),
 which have been allocated before (in the crashed kernel) the hcalls will
 fail because from the hypervisor view the resources are still in use.
 Currently there's no method to find out the resource handles for these
 HEA resources allocated by the crashed kernel within the hypervisor...

So the hypervisor can't allocate more resources, because they're already
allocated, but it can't free the ones that are allocated because it
doesn't know what they are? I don't think I understand.

If that's really the way it works then eHEA is more or less broken for
kdump I'm afraid.

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


signature.asc
Description: This is a digitally signed message part

Re: [PATCH] ehea: add kexec support

[Michael Ellerman]

On Tue, 2007-10-30 at 09:39 +0100, Christoph Raisch wrote:
 
 Michael Ellerman [EMAIL PROTECTED] wrote on 28.10.2007 23:32:17:
 
 
  How do you plan to support kdump?
 
 
 When kexec is fully supported kdump should work out of the box
 as for any other ethernet card (if you load the right eth driver).
 There's nothing specific to kdump you have to handle in
 ethernet device drivers.
 Hope I didn't miss anything here...

Perhaps. When we kdump the kernel does not call the reboot notifiers, so
the code Jan-Bernd just added won't get called. So the eHEA resources
won't be freed. When the kdump kernel tries to load the eHEA driver what
will happen?

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


signature.asc
Description: This is a digitally signed message part

Re: [PATCH] ehea: add kexec support

[Michael Ellerman]

On Fri, 2007-10-26 at 14:37 +0200, Jan-Bernd Themann wrote:
 eHEA resources that are allocated via H_CALLs have a unique identifier each.
 These identifiers are necessary to free the resources. A reboot notifier
 is used to free all eHEA resources before the indentifiers get lost, i.e
 before kexec starts a new kernel.
 
 Signed-off-by: Jan-Bernd Themann [EMAIL PROTECTED]
 

How do you plan to support kdump?

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


signature.asc
Description: This is a digitally signed message part

Re: [PATCH 5/7] pci: Export the pci_restore_msi_state() function

[Michael Ellerman]

On Mon, 2007-10-22 at 13:13 -0500, Linas Vepstas wrote:
 On Mon, Oct 22, 2007 at 11:49:24AM +1000, Michael Ellerman wrote:
  
  On pseries there's a chance it will work for PCI error recovery, but if
  so it's just lucky that firmware has left everything configured the same
  way. 
 
 ? The papr is quite clear that i is up to the OS to restore the msi
 state after an eeh error.

Perhaps. I see R1-7.3.10.5.1-10, which says we should restore the config
space after a reset operation, but after an isolate/unisolate we must
recall RTAS. I thought EEH was doing the isolate/unisolate, but if it's
just a reset then just blatting the config space back should work.

  Yes I think so. That way we can properly reconfigure via the firmware
  interface. The other option would be to design some new arch hook to do
  resume, but just doing a disable/enable seems simpler to me.
 
 Err, If you read the code for suspend/resume, it never actually calls
 disable/enable (and thus doesn't go to the firmware); it calls 
 restore_msi_state() function!

I know. That was a proposed solution, to explicitly call disable/enable
instead of restore_msi_state().

 If suspend/resume needs to call firmware to restore the state, then,
 at the moment, suspend/resume is broken.  As I mentioned earlier,
 I presumed that no powerpc laptops currently use msi-enabled devices,
 as otherwise, this would have been flushed out.

On _pseries_ suspend/resume with MSI is broken. The other powerpc
platforms that implement MSI should be fine, although I don't think
anyone's tested them, because they're not laptops and so suspend/resume
is not that interesting.

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


signature.asc
Description: This is a digitally signed message part

Re: [PATCH 5/7] pci: Export the pci_restore_msi_state() function

[Michael Ellerman]

On Sun, 2007-10-21 at 16:21 -0700, David Miller wrote:
 From: Matt Carlson [EMAIL PROTECTED]
 Date: Fri, 19 Oct 2007 14:36:56 -0700
 
  This patch exports the pci_restore_msi_state() function.  This function
  is needed to restore the MSI state during PCI error recovery.
  
  Signed-off-by: Linas Vepstas [EMAIL PROTECTED]
  Signed-off-by: Matt Carlson [EMAIL PROTECTED]
  Signed-off-by: Michael Chan [EMAIL PROTECTED]
 
 I'm not so sure about this.

On pseries there's a chance it will work for PCI error recovery, but if
so it's just lucky that firmware has left everything configured the same
way. For actual suspend/resume it will never work, we need to ask
firmware to configure things.

 Perhaps, instead, you should do a pci_msi_disable() and
 pci_msi_enable() in the error detection and recovery sequence.

Yes I think so. That way we can properly reconfigure via the firmware
interface. The other option would be to design some new arch hook to do
resume, but just doing a disable/enable seems simpler to me.

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


signature.asc
Description: This is a digitally signed message part

Re: [BUG] powerpc does not save msi state [was Re: [PATCH 5/7] pci: Export the pci_restore_msi_state() function

[Michael Ellerman]

On Fri, 2007-10-19 at 17:53 -0700, David Miller wrote:
 From: [EMAIL PROTECTED] (Linas Vepstas)
 Date: Fri, 19 Oct 2007 19:46:10 -0500
 
  FWIW, it looks like not all that many arches do this; the output
  for grep -r address_hi * is pretty thin. Then, looking at
  i386/kernel/io_apic.c as an example, one can see that the 
  msi state save happens by accident if CONFIG_SMP is enabled;
  and so its surely broekn on uniprocesor machines.
 
 I don't see this, in all cases write_msi_msg() will transfer
 the given *msg to entry-msg by this assignment in
 drivers/pci/msi.c:
 
 void write_msi_msg(unsigned int irq, struct msi_msg *msg)
 {
  ...
   entry-msg = *msg;
 }
 
 So as long as write_msi_msg() is invoked, it will be saved
 properly.
 
 Platforms need not do this explicitly.

I'm short on context here, and it's Saturday, so excuse me if I'm
missing the point somewhere.

On pseries machines we don't call write_msi_msg(), because we don't
control the contents of the message, firmware does. So entry-msg will
be bogus.

That's a pity, but AFAIK it shouldn't be a problem because we don't
enable CONFIG_PM on those machines anyway. If we ever want to we'll need
to sort out with firmware how that will work WRT restoring MSI state.

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


signature.asc
Description: This is a digitally signed message part

Re: [Cbe-oss-dev] [PATCH 0/15] spidernet driver bug fixes

[Michael Ellerman]

On Fri, 2007-06-15 at 00:07 +0100, David Woodhouse wrote:
 On Thu, 2007-06-14 at 19:04 -0400, Jeff Garzik wrote:
  Think about the actual kernel tree source code, not just the
  metadata...
 
 Disk is cheap. Waiting for the whole damn thing to rebuild after
 switching branches and back again is less so.

ccache!

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


signature.asc
Description: This is a digitally signed message part

Re: [PATCH 0/15] spidernet driver bug fixes

[Michael Ellerman]

On Tue, 2007-06-12 at 21:54 -0400, Jeff Garzik wrote:
 Michael Ellerman wrote:
  Linas posted the patches, I responded querying whether the bug fixes
  should go into 2.6.22, and then you told him you need to order your bug
  fixes first in the queue. Which seemed pretty clear to me that you'd
  wait for the reordered series.
 
 This was presuming Linas actually knew what he himself had submitted 
 previously, and had been accepted...
 
 I explicitly emailed Linas on May 24, 2007 detailing each patch that had 
 been applied, and to which netdev-2.6.git branch it had been applied 
 (and thus whether it was queued for 2.6.22 or 2.6.23).  Relevant 
 Message-id is [EMAIL PROTECTED], and was sent not only to 
 Linas but also to netdev@vger.kernel.org, [EMAIL PROTECTED], and 
 [EMAIL PROTECTED]
 
 These changes were subsequently made public immediately via 
 git://git.kernel.org/.../jgarzik/netdev-2.6.git branches 
 'upstream-fixes' and 'upstream', and were followed a few days later by 
 akpm's public tree, starting with 2.6.22-rc3-mm1 (and all subsequent 
 releases).
 
 All of the above seemed pretty clear, too.

Yeah fair enuf, that's fairly clear.

 To move forward, it sounds like the best thing to do is drop all 
 spidernet patches and start over, yes?

Well see what Linas thinks, but that is probably easiest. I was just
keen to see the major bugfixes get into 22, rather than waiting
another few months for 23.

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


signature.asc
Description: This is a digitally signed message part

Re: [Cbe-oss-dev] [PATCH 0/15] spidernet driver bug fixes

[Michael Ellerman]

On Wed, 2007-06-13 at 21:01 +0200, Segher Boessenkool wrote:
  I wish there was a git option to just make my shit look like the
  remote, dammit!  The above is the easiest way I know how to do that.
 
 git-fetch -f remote:local ?

There's always git reset --hard sha1

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


signature.asc
Description: This is a digitally signed message part

Re: [PATCH 0/15] spidernet driver bug fixes

[Michael Ellerman]

On Tue, 2007-06-12 at 19:00 -0400, Jeff Garzik wrote:
 Linas Vepstas wrote:
  On Fri, Jun 08, 2007 at 01:20:20PM -0400, Jeff Garzik wrote:
  On Fri, Jun 08, 2007 at 12:06:08PM -0500, Linas Vepstas wrote:
  On Fri, Jun 08, 2007 at 11:12:31AM +1000, Michael Ellerman wrote:
  On Thu, 2007-06-07 at 14:17 -0500, Linas Vepstas wrote:
  The major bug fixes are: 
  I realise it's late, but shouldn't major bugfixes be going into 22 ?
  Yeah, I suppose, I admit I've lost track of the process. 
  You need to order your bug fixes first in the queue. 
  
  OK, here are the patches, re-ordered. There is a different number
  than last time, as I threw out one, merged one, and got cold feet
  on a third one.  They still pass the tests.
  
  The first five patches focus on three serious bugs, fixing crashes or
  hangs.
  
  -- patch 1 -- kernel crash when ifdown while receiving packets.
  -- patch 2,3,4 -- device driver deadlocks on RX ram full mesgs.
(kernel stays up, ifdown/up clear the problem).
  -- patch 5 -- misconfigured TX interrupts results in 3x-4x per
degradation for small packets.
  
  -- patch 6 -- rx stats may be mangled
  -- patch 7 -- hw checksum sometimes breaks ipv6 operation
  
  -- patches 8-15 -- misc tweaks, and documentation.
  
  
  I re-ran my stress tests with patches 1-7 applied; they pass.
 
 This is a bit frustrating, because this includes many patches that you 
 ALREADY told me to queue for 2.6.23, which I did, in 
 netdev-2.6.git#upstream.

Linas posted the patches, I responded querying whether the bug fixes
should go into 2.6.22, and then you told him you need to order your bug
fixes first in the queue. Which seemed pretty clear to me that you'd
wait for the reordered series.

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


signature.asc
Description: This is a digitally signed message part

Re: [PATCH 0/15] spidernet driver bug fixes

[Michael Ellerman]

On Mon, 2007-06-11 at 13:14 -0500, Linas Vepstas wrote:
 On Fri, Jun 08, 2007 at 01:20:20PM -0400, Jeff Garzik wrote:
  On Fri, Jun 08, 2007 at 12:06:08PM -0500, Linas Vepstas wrote:
   On Fri, Jun 08, 2007 at 11:12:31AM +1000, Michael Ellerman wrote:
On Thu, 2007-06-07 at 14:17 -0500, Linas Vepstas wrote:
 
 The major bug fixes are: 
I realise it's late, but shouldn't major bugfixes be going into 22 ?
   Yeah, I suppose, I admit I've lost track of the process. 
 
  You need to order your bug fixes first in the queue. 
 
 OK, here are the patches, re-ordered. There is a different number
 than last time, as I threw out one, merged one, and got cold feet
 on a third one.  They still pass the tests.
 
 The first five patches focus on three serious bugs, fixing crashes or
 hangs.

Nice one.

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


signature.asc
Description: This is a digitally signed message part

Re: [Cbe-oss-dev] [PATCH 0/18] spidernet driver bug fixes

[Michael Ellerman]

On Thu, 2007-06-07 at 14:17 -0500, Linas Vepstas wrote:
 Jeff, please apply for the 2.6.23 kernel tree.  The pach series
 consists of two major bugfixes, and several bits of cleanup.
 
 The major bug fixes are: 
 
 1) a rare but fatal bug involving RX ram full messages, 
which results in a driver deadlock.
 
 2) misconfigured TX interrupts, causing a sever performance
degardation for small packets.

I realise it's late, but shouldn't major bugfixes be going into 22 ?

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


signature.asc
Description: This is a digitally signed message part

Re: [PATCH 2.6.22-rc4] ehea: Fixed possible kernel panic on VLAN packet recv

[Michael Ellerman]

On Wed, 2007-06-06 at 20:53 +0200, Thomas Klein wrote:
 This patch fixes a possible kernel panic due to not checking the vlan group
 when processing received VLAN packets and a malfunction in VLAN/hypervisor
 registration.
 
 
 Signed-off-by: Thomas Klein [EMAIL PROTECTED]
 ---
 
 
 diff -Nurp -X dontdiff linux-2.6.22-rc4/drivers/net/ehea/ehea.h 
 patched_kernel/drivers/net/ehea/ehea.h
 --- linux-2.6.22-rc4/drivers/net/ehea/ehea.h  2007-06-05 02:57:25.0 
 +0200
 +++ patched_kernel/drivers/net/ehea/ehea.h2007-06-06 12:53:58.0 
 +0200
 @@ -39,7 +39,7 @@
  #include asm/io.h
  
  #define DRV_NAME ehea
 -#define DRV_VERSION  EHEA_0061
 +#define DRV_VERSION  EHEA_0064
  
  #define EHEA_MSG_DEFAULT (NETIF_MSG_LINK | NETIF_MSG_TIMER \
   | NETIF_MSG_RX_ERR | NETIF_MSG_TX_ERR)
 diff -Nurp -X dontdiff linux-2.6.22-rc4/drivers/net/ehea/ehea_main.c 
 patched_kernel/drivers/net/ehea/ehea_main.c
 --- linux-2.6.22-rc4/drivers/net/ehea/ehea_main.c 2007-06-05 
 02:57:25.0 +0200
 +++ patched_kernel/drivers/net/ehea/ehea_main.c   2007-06-06 
 12:53:58.0 +0200
 @@ -1947,7 +1945,7 @@ static void ehea_vlan_rx_add_vid(struct 
   }
  
   index = (vid / 64);
 - cb1-vlan_filter[index] |= ((u64)(1  (vid  0x3F)));
 + cb1-vlan_filter[index] |= ((u64)(0x8000  (vid  0x3F)));
  
   hret = ehea_h_modify_ehea_port(adapter-handle, port-logical_port_id,
  H_PORT_CB1, H_PORT_CB1_ALL, cb1);
 @@ -1982,7 +1980,7 @@ static void ehea_vlan_rx_kill_vid(struct
   }
  
   index = (vid / 64);
 - cb1-vlan_filter[index] = ~((u64)(1  (vid  0x3F)));
 + cb1-vlan_filter[index] = ~((u64)(0x8000  (vid  0x3F)));

These two seem ripe for splitting into some sort of helper routine.
Which would leave only one place to get it right.

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


signature.asc
Description: This is a digitally signed message part

Re: [Cbe-oss-dev] [PATCH 4/10] spidernet: zero out a pointer.

[Michael Ellerman]

On Wed, 2007-05-16 at 17:09 -0500, Linas Vepstas wrote:
 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 [EMAIL PROTECTED]
 
 
  drivers/net/spider_net.c |7 +--
  1 file changed, 5 insertions(+), 2 deletions(-)
 
 Index: linux-2.6.22-rc1/drivers/net/spider_net.c
 ===
 --- linux-2.6.22-rc1.orig/drivers/net/spider_net.c2007-05-15 
 13:31:11.0 -0500
 +++ linux-2.6.22-rc1/drivers/net/spider_net.c 2007-05-15 13:31:16.0 
 -0500
 @@ -1081,7 +1082,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;

If you're going to be paranoid, shouldn't you do something here to make
sure the value's hit the device?

 + pci_unmap_single(card-pdev, hw_buf_addr,
   SPIDER_NET_MAX_FRAME, PCI_DMA_FROMDEVICE);
  

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


signature.asc
Description: This is a digitally signed message part

Re: [Cbe-oss-dev] [PATCH 10/10] spidernet: increase the NAPI weight

[Michael Ellerman]

On Wed, 2007-05-16 at 17:23 -0500, Linas Vepstas wrote:
 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 [EMAIL PROTECTED]
 
 
  drivers/net/spider_net.h |9 +++--
  1 file changed, 7 insertions(+), 2 deletions(-)
 
 Index: linux-2.6.22-rc1/drivers/net/spider_net.h
 ===
 --- linux-2.6.22-rc1.orig/drivers/net/spider_net.h2007-05-16 
 12:00:35.0 -0500
 +++ linux-2.6.22-rc1/drivers/net/spider_net.h 2007-05-16 12:00:35.0 
 -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 te napi wieght

Speeeling .. ^^  ^^

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


signature.asc
Description: This is a digitally signed message part

Re: [PATCH 2/2] ehea: NAPI multi queue TX/RX path for SMP

[Michael Ellerman]

On Wed, 2007-02-28 at 18:34 +0100, Jan-Bernd Themann wrote:
 This patch provides a functionality that allows parallel 
 RX processing on multiple RX queues by using dummy netdevices.
 
 
 Signed-off-by: Jan-Bernd Themann [EMAIL PROTECTED]
 ---

 @@ -1789,6 +1798,22 @@ static void ehea_xmit3(struct sk_buff *s
   dev_kfree_skb(skb);
  }
  
 +static inline int ehea_hash_skb(struct sk_buff *skb, int num_qps)
 +{
 + struct tcphdr *tcp;
 + u32 tmp;
 +
 + if ((skb-protocol == htons(ETH_P_IP)) 
 + (skb-nh.iph-protocol == IPPROTO_TCP)) {

This breaks the build, looks like skb-nh went away:
b0e380b1d8a8e0aca215df97702f99815f05c094

/scratch/michael/kisskb-build/src/drivers/net/ehea/ehea_main.c:1806: error: 
'struct sk_buff' has no member named 'nh'
/scratch/michael/kisskb-build/src/drivers/net/ehea/ehea_main.c:1807: error: 
'struct sk_buff' has no member named 'nh'
/scratch/michael/kisskb-build/src/drivers/net/ehea/ehea_main.c:1807: error: 
'struct sk_buff' has no member named 'nh'
/scratch/michael/kisskb-build/src/drivers/net/ehea/ehea_main.c:1809: error: 
'struct sk_buff' has no member named 'nh'

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


signature.asc
Description: This is a digitally signed message part

Re: [PATCH] [2/4] pasemi_mac: irq mapping changes

[Michael Ellerman]

On Mon, 2007-04-16 at 01:18 -0500, Olof Johansson wrote:
 Fixes for ethernet IRQ mapping, to move it to the driver instead of in
 the platform setup code.
 
 
 Signed-off-by: Olof Johansson [EMAIL PROTECTED]
 
 Index: powerpc/arch/powerpc/platforms/pasemi/pci.c
 ===
 --- powerpc.orig/arch/powerpc/platforms/pasemi/pci.c
 +++ powerpc/arch/powerpc/platforms/pasemi/pci.c
 @@ -163,19 +163,6 @@ static void __init pas_fixup_phb_resourc
  }
  
 
 -void __devinit pas_pci_irq_fixup(struct pci_dev *dev)
 -{
 - /* DMA is special, 84 interrupts (128 - 211), all but 128
 -  * need to be mapped by hand here.
 -  */
 - if (dev-vendor == 0x1959  dev-device == 0xa007) {
 - int i;
 - for (i = 129; i  212; i++)
 - irq_create_mapping(NULL, i);
 - }
 -}
 -
 -
  void __init pas_pci_init(void)
  {
   struct device_node *np, *root;
 Index: powerpc/arch/powerpc/platforms/pasemi/setup.c
 ===
 --- powerpc.orig/arch/powerpc/platforms/pasemi/setup.c
 +++ powerpc/arch/powerpc/platforms/pasemi/setup.c
 @@ -240,5 +240,4 @@ define_machine(pas) {
   .check_legacy_ioport= pas_check_legacy_ioport,
   .progress   = pas_progress,
   .machine_check_exception = pas_machine_check_handler,
 - .pci_irq_fixup  = pas_pci_irq_fixup,
  };
 Index: powerpc/drivers/net/pasemi_mac.c
 ===
 --- powerpc.orig/drivers/net/pasemi_mac.c
 +++ powerpc/drivers/net/pasemi_mac.c
 @@ -537,6 +537,9 @@ static irqreturn_t pasemi_mac_tx_intr(in
  static int pasemi_mac_open(struct net_device *dev)
  {
   struct pasemi_mac *mac = netdev_priv(dev);
 + struct device_node *dma_dn = pci_device_to_OF_node(mac-dma_pdev);
 + const unsigned int *prop;
 + int base_irq;
   unsigned int flags;
   int ret;
  
 @@ -600,8 +603,24 @@ static int pasemi_mac_open(struct net_de
   netif_start_queue(dev);
   netif_poll_enable(dev);
  
 - ret = request_irq(mac-dma_pdev-irq + mac-dma_txch,
 -   pasemi_mac_tx_intr, IRQF_DISABLED,
 + /* Interrupts are a bit different for our DMA controller: While
 +  * it's got one a regular PCI device header, the interrupt there
 +  * is really the base of the range it's using. Each tx and rx
 +  * channel has it's own interrupt source.
 +  *
 +  * The only way to get to the actual hardware interrupt is by
 +  * getting it from the device tree, since the kernel has done
 +  * virtual remapping of the sources by the time we can get them
 +  * from the PCI device.
 +  */
 +
 + prop = of_get_property(dma_dn, interrupts, NULL);
 + base_irq = *prop;

The recently added virq_to_hw() makes it nice 'n easy to reverse map the
hw irq from the pci_dev:

  base_irq = virq_to_hw(pdev-irq);


You should also probably check that irq_create_mapping() succeeds, just
to be pedantic.

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


signature.asc
Description: This is a digitally signed message part

[PATCH] Fix sparse errors in drivers/net/ibmveth.c

[Michael Ellerman]

drivers/net/ibmveth.c:96:46: error: marked inline, but without a definition
drivers/net/ibmveth.c:96: warning: 'ibmveth_rxq_harvest_buffer' declared inline 
after being called
drivers/net/ibmveth.c:96: warning: previous declaration of 
'ibmveth_rxq_harvest_buffer' was here

Just let the compiler decide, as it happens gcc 4.~ inlines it anyway.

drivers/net/ibmveth.c:957:71: warning: Using plain integer as NULL pointer
drivers/net/ibmveth.c:964:85: warning: Using plain integer as NULL pointer

Split the long lines as well, ugly, but  80 columns.

Signed-off-by: Michael Ellerman [EMAIL PROTECTED]
---

 drivers/net/ibmveth.c |   10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

Index: powerpc/drivers/net/ibmveth.c
===
--- powerpc.orig/drivers/net/ibmveth.c
+++ powerpc/drivers/net/ibmveth.c
@@ -93,7 +93,7 @@ static void ibmveth_proc_unregister_driv
 static void ibmveth_proc_register_adapter(struct ibmveth_adapter *adapter);
 static void ibmveth_proc_unregister_adapter(struct ibmveth_adapter *adapter);
 static irqreturn_t ibmveth_interrupt(int irq, void *dev_instance);
-static inline void ibmveth_rxq_harvest_buffer(struct ibmveth_adapter *adapter);
+static void ibmveth_rxq_harvest_buffer(struct ibmveth_adapter *adapter);
 static struct kobj_type ktype_veth_pool;
 
 #ifdef CONFIG_PROC_FS
@@ -389,7 +389,7 @@ static void ibmveth_rxq_recycle_buffer(s
}
 }
 
-static inline void ibmveth_rxq_harvest_buffer(struct ibmveth_adapter *adapter)
+static void ibmveth_rxq_harvest_buffer(struct ibmveth_adapter *adapter)
 {
ibmveth_remove_buffer_from_pool(adapter, 
adapter-rx_queue.queue_addr[adapter-rx_queue.index].correlator);
 
@@ -954,14 +954,16 @@ static int __devinit ibmveth_probe(struc
ibmveth_debug_printk_no_adapter(entering ibmveth_probe for UA 0x%x\n,
dev-unit_address);
 
-   mac_addr_p = (unsigned char *) vio_get_attribute(dev, VETH_MAC_ADDR, 0);
+   mac_addr_p = (unsigned char *) vio_get_attribute(dev,
+   VETH_MAC_ADDR, NULL);
if(!mac_addr_p) {
printk(KERN_ERR (%s:%3.3d) ERROR: Can't find VETH_MAC_ADDR 
attribute\n, __FILE__, __LINE__);
return 0;
}
 
-   mcastFilterSize_p= (unsigned int *) vio_get_attribute(dev, 
VETH_MCAST_FILTER_SIZE, 0);
+   mcastFilterSize_p = (unsigned int *) vio_get_attribute(dev,
+   VETH_MCAST_FILTER_SIZE, NULL);
if(!mcastFilterSize_p) {
printk(KERN_ERR (%s:%3.3d) ERROR: Can't find 
VETH_MCAST_FILTER_SIZE attribute\n,
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 12/14] Spidernet Avoid possible RX chain corruption

[Michael Ellerman]

On Thu, 2006-12-14 at 11:15 -0600, Linas Vepstas wrote:
 On Thu, Dec 14, 2006 at 11:22:43AM +1100, Michael Ellerman wrote:
 spider_net_refill_rx_chain(card);
   - spider_net_enable_rxchtails(card);
 spider_net_enable_rxdmac(card);
 return 0;
  
  Didn't you just add that line?
 
 Dagnabbit. The earlier pach was moving around existing code.
 Or, more precisely, trying to maintain the general function
 of the old code even while moving things around.
 
 Later on, when I started looking at what the danged function 
 actually did, and the context it was in, I realized that it 
 was a bad idea to call the thing.  So then I removed it. :-/
 
 How should I handle this proceedurally? Resend the patch sequence? 
 Let it slide?

If it was my code I'd redo the series, it's confusing and it's going to
look confused in the git history IMHO.

Currently the driver calls spider_net_enable_rxchtails() from
spider_net_enable_card() and spider_net_handle_rxram_full().

Your patch 3/14 removes spider_net_handle_rxram_full() entirely, leaving
spider_net_enable_card() as the only caller of
spider_net_enable_rxchtails().

Patch 10/14 adds a call to spider_net_enable_rxchtails() in
spider_net_alloc_rx_skbs(), and nothing else (except comment changes).

Patch 12/14 removes the call to spider_net_enable_rxchtails() in
spider_net_alloc_rx_skbs(), and nothing else.

So as far as I can tell you should just drop 10/14 and 12/14. 

My worry is that amongst all that rearranging of code, it's not clear
what the semantic change is. Admittedly I don't know the driver that
well, but that's kind of the point - if you and Jim get moved onto a new
project, someone needs to be able to pick up the driver and maintain it.

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


signature.asc
Description: This is a digitally signed message part

Re: [PATCH 2/14] Spidernet add net_ratelimit to suppress long output

[Michael Ellerman]

On Wed, 2006-12-13 at 15:08 -0600, Linas Vepstas wrote:
 This patch adds net_ratelimit to many of the printks in order to
 limit extraneous warning messages (created in response to Bug 28554).

Bug 28554 in which bug tracking system? I can't find it in LTC or
kernel.org bugzilla.

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


signature.asc
Description: This is a digitally signed message part

Re: [PATCH 12/14] Spidernet Avoid possible RX chain corruption

[Michael Ellerman]

On Wed, 2006-12-13 at 15:23 -0600, Linas Vepstas wrote:
 Delete possible source of chain corruption; the hardware
 already knows the location of the tail, and writing it
 again is likely to mess it up.
 
 Signed-off-by: Linas Vepstas [EMAIL PROTECTED]
 Cc: James K Lewis [EMAIL PROTECTED]
 Cc: Arnd Bergmann [EMAIL PROTECTED]
 
 
 
  drivers/net/spider_net.c |1 -
  1 file changed, 1 deletion(-)
 
 Index: linux-2.6.19-git7/drivers/net/spider_net.c
 ===
 --- linux-2.6.19-git7.orig/drivers/net/spider_net.c   2006-12-13 
 14:28:23.0 -0600
 +++ linux-2.6.19-git7/drivers/net/spider_net.c2006-12-13 
 14:28:25.0 -0600
 @@ -513,7 +513,6 @@ spider_net_alloc_rx_skbs(struct spider_n
   /* This will allocate the rest of the rx buffers;
* if not, it's business as usual later on. */
   spider_net_refill_rx_chain(card);
 - spider_net_enable_rxchtails(card);
   spider_net_enable_rxdmac(card);
   return 0;

Didn't you just add that line?

From [PATCH 10/14] Spidernet RX Chain tail:

 @@ -501,17 +501,18 @@ spider_net_alloc_rx_skbs(struct spider_n
 
 snip comment change
  
 -   /* this will allocate the rest of the rx buffers; if not, it's
 -* business as usual later on */
 +   /* This will allocate the rest of the rx buffers;
 +* if not, it's business as usual later on. */
 spider_net_refill_rx_chain(card);
 +   spider_net_enable_rxchtails(card);
 spider_net_enable_rxdmac(card);
 return 0;

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


signature.asc
Description: This is a digitally signed message part

[PATCH] powerpc: ibmveth: Harden driver initilisation for kexec

[Michael Ellerman]

Hi Jeff,

This patch has been floating around for a while now, Santi originally
sent it in March: http://www.spinics.net/lists/netdev/msg00471.html

You replied saying you thought it was bonkers, I think I explained why
it wasn't, perhaps you disagree.

I'm resending it now in the hope you can either give us more info on
your objections, or merge it.


After a kexec the ibmveth driver will fail when trying to register with
the Hypervisor because the previous kernel has not unregistered.

So if the registration fails, we unregister and then try again.

We don't unconditionally unregister, because we don't want to disturb the
regular code path for 99% of users.

Signed-off-by: Michael Ellerman [EMAIL PROTECTED]
Acked-by: Anton Blanchard [EMAIL PROTECTED]
Signed-off-by: Santiago Leon [EMAIL PROTECTED]

---
 ibmveth.c |   31 ++-
 1 file changed, 26 insertions(+), 5 deletions(-)

Index: to-merge/drivers/net/ibmveth.c
===
--- to-merge.orig/drivers/net/ibmveth.c
+++ to-merge/drivers/net/ibmveth.c
@@ -437,6 +437,31 @@ static void ibmveth_cleanup(struct ibmve
 adapter-rx_buff_pool[i]);
 }
 
+static int ibmveth_register_logical_lan(struct ibmveth_adapter *adapter,
+union ibmveth_buf_desc rxq_desc, u64 mac_address)
+{
+   int rc, try_again = 1;
+
+   /* After a kexec the adapter will still be open, so our attempt to
+   * open it will fail. So if we get a failure we free the adapter and
+   * try again, but only once. */
+retry:
+   rc = h_register_logical_lan(adapter-vdev-unit_address,
+   adapter-buffer_list_dma, rxq_desc.desc,
+   adapter-filter_list_dma, mac_address);
+
+   if (rc != H_SUCCESS  try_again) {
+   do {
+   rc = h_free_logical_lan(adapter-vdev-unit_address);
+   } while (H_IS_LONG_BUSY(rc) || (rc == H_BUSY));
+
+   try_again = 0;
+   goto retry;
+   }
+
+   return rc;
+}
+
 static int ibmveth_open(struct net_device *netdev)
 {
struct ibmveth_adapter *adapter = netdev-priv;
@@ -503,11 +528,7 @@ static int ibmveth_open(struct net_devic
ibmveth_debug_printk(receive q   @ 0x%p\n, 
adapter-rx_queue.queue_addr);
 

-   lpar_rc = h_register_logical_lan(adapter-vdev-unit_address,
-adapter-buffer_list_dma,
-rxq_desc.desc,
-adapter-filter_list_dma,
-mac_address);
+   lpar_rc = ibmveth_register_logical_lan(adapter, rxq_desc, mac_address);
 
if(lpar_rc != H_SUCCESS) {
ibmveth_error_printk(h_register_logical_lan failed with 
%ld\n, lpar_rc);


-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] powerpc: ibmveth: Harden driver initilisation for kexec

[Michael Ellerman]

On 4/29/06, Santiago Leon [EMAIL PROTECTED] wrote:

Michael Ellerman wrote:
 Any chance of getting it into to 2.6.17 ...


2.6.19 perhaps?

cheers
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [2.6.19 PATCH 4/7] ehea: ethtool interface

[Michael Ellerman]

On Fri, 2006-08-18 at 17:41 +0200, Thomas Klein wrote:
 Hi Alexey,
 
 first of all thanks a lot for the extensive review.
 
 
 Alexey Dobriyan wrote:
  +  u64 hret = H_HARDWARE;
  
  Useless assignment here and everywhere.
  
 
 Initializing returncodes to errorstate is a cheap way to prevent
 accidentally returning (uninitalized) success returncodes which
 can lead to catastrophic misbehaviour.

If you try to return an uninitialized value the compiler will warn you,
you'll then look at the code and realise you missed a case, you might
save yourself a bug. By unconditionally initialising you are lying to
the compiler, and it can no longer help you.

cheers

-- 
Michael Ellerman
IBM OzLabs

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


signature.asc
Description: This is a digitally signed message part

Re: [2.6.19 PATCH 4/7] ehea: ethtool interface

[Michael Ellerman]

On Sat, 2006-08-19 at 02:48 -0400, Andy Gay wrote:
 On Sat, 2006-08-19 at 16:18 +1000, Michael Ellerman wrote:
 
  
  If you try to return an uninitialized value the compiler will warn you,
  you'll then look at the code and realise you missed a case, you might
  save yourself a bug. 
 
 You *should* look at the code :)
 
 So should we be reporting these as bugs?

No you're better off sending patches ;)

A lot of these have started appearing recently, which I think is due to
GCC becoming more vocal. Unfortunately many of them are false positives
caused by GCC not seeming to grok that this is ok:

void foo(int *x) { *x = 1; }
...
int x;
foo(x);
return x;

It's a pity because it creates noise, but still it's beside the point.

New code going into the kernel should be 100% warning free, and so if
the eHEA guys had missed an error case they'd spot the warning before
they submitted it.

Doing the initialise-to-some-value trick means you only spot the bug
via testing.

cheers

-- 
Michael Ellerman
IBM OzLabs

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


signature.asc
Description: This is a digitally signed message part

Re: [2.6.19 PATCH 7/7] ehea: Makefile Kconfig

[Michael Ellerman]

On Fri, 2006-08-18 at 13:37 +0200, Jan-Bernd Themann wrote:
 Signed-off-by: Jan-Bernd Themann [EMAIL PROTECTED] 
 
 
  drivers/net/Kconfig  |6 ++
  drivers/net/Makefile |1 +
  2 files changed, 7 insertions(+)
 
 
 
 diff -Nurp -X dontdiff linux-2.6.18-rc4/drivers/net/Kconfig 
 patched_kernel/drivers/net/Kconfig
 --- linux-2.6.18-rc4/drivers/net/Kconfig  2006-08-06 11:20:11.0 
 -0700
 +++ patched_kernel/drivers/net/Kconfig2006-08-08 03:00:49.526421944 
 -0700
 @@ -2277,6 +2277,12 @@ config CHELSIO_T1
To compile this driver as a module, choose M here: the module
will be called cxgb.
  
 +config EHEA
 +tristate eHEA Ethernet support
 +depends on IBMEBUS
 +---help---
 +  This driver supports the IBM pSeries ethernet adapter
 +

Please give it a more detailed description. I have a pSeries machine
here with three NICs and none of them are eHEA.

cheers

-- 
Michael Ellerman
IBM OzLabs

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


signature.asc
Description: This is a digitally signed message part

Re: [PATCH 4/4]: powerpc/cell spidernet ethtool -i version number info.

[Michael Ellerman]

On Tue, 2006-08-15 at 14:05 -0500, Olof Johansson wrote:
 On Fri, Aug 11, 2006 at 01:50:19PM -0500, James K Lewis wrote:
   Hi Olof,
  
There are several reasons why an Ethernet driver should have an up to 
  date version number:
  
  1. Customers like to see they are really getting a new version.
  
  2. It makes it easier for support personnel (me in this case) to see which 
  driver they have. Sure, sometimes I can talk them thru doing a sum on 
  the .ko and all that, but why not just use the version number? That's what 
  it is for. And no, you can't just assume they have the version that came 
  with the kernel they are running. It doesn't work that way.
  
  3. It makes bug reporting easier. 
  
  4. I have already run into too many problems and wasted too much time 
  working with drivers when the number was NOT getting updated. 
 
 Thanks for the info, Jim.
 
 Sounds like it's most useful if a customer (or distro) takes the driver
 out of the tree and run it with a different kernel, i.e. when kernel
 and driver versions no longer go together. Makes sense.

It only makes sense in addition to the kernel version number (which in
some cases can be meaningless), plus any distro and/or local patches,
plus the kernel config.

Without all that information you don't really know what you're talking
about, because any one of the many interfaces between the driver and the
core kernel may have changed.

So in practice I find it's much simpler to just get the exact source
that they're running, rather than trying to guess based on version
numbers. But that's just me :)

cheers

-- 
Michael Ellerman
IBM OzLabs

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


signature.asc
Description: This is a digitally signed message part

RE: [PATCH 4/6] ehea: header files

[Michael Ellerman]

On Tue, 2006-08-15 at 13:07 +0200, Christoph Raisch wrote:
 
 Jenkins, Clive wrote on 15.08.2006 12:53:05:
 
You mean the eHEA has its own concept of page size? Separate from
  the
page size used by the MMU?
   
  
   yes, the eHEA currently supports only 4K pages for queues
 
  In that case, I suggest use the kernel's page size, but add a
  compile-time
  check, and quit with an error message if driver does not support it.
 
 eHEA does support other page sizes than 4k, but the HW interface expects to
 see 4k pages
 The adaption is done in the device driver, therefore we have a seperate 4k
 define.

Fair enough. You seem to only use it in drivers/net/ehea/ehea_qmr.c, if
so put the definition in there, that way someone is less likely to use
the EHEA_PAGESIZE definition where they really need PAGE_SIZE.

cheers

-- 
Michael Ellerman
IBM OzLabs

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


signature.asc
Description: This is a digitally signed message part

Re: ehea debug output discussion

[Michael Ellerman]

On Mon, 2006-08-14 at 14:38 +0200, Jan-Bernd Themann wrote:
 Hi
 
 Anton Blanchard wrote:
  What is going to be done about the debug infrastructure in the ehea
  driver? The entry and exit traces really need to go, and any other debug
  you think is important to users needs to go into debugfs or something
  similar.
  
  I see a similar issue in the ehca driver that I am in the middle of
  reviewing.
  
  Anton
 
 This is a statement for the eHEA driver:
 
 Most of the debug outputs are redundant and we'll remove them
 (EDEB_EN / EDEB_EX). We can use the standard mechanism for ethernet devices
 (netif_msg_x) in most functions of ehea_main.c as we have the device struct
 as a parameter available. However, some debug output mechanism is needed
 where the standard mechanism does not work (functions that have no relation
 to the dev struct do not have a dev parameter, for example
 ehea_hcall_9arg_9ret in ehea_phyp.h)
 
 The outcome of some internal discussions was that it is not acceptable for
 our enterprise users of this type of driver on this target system to need a
 recompile / reload of the driver for error analysis, so we need a mechanism
 that allows us to switch on / off debug output at runtime. Therefore, we'd
 introduce a stripped down version of EDEB.

Well, it might be better if the driver just worked, then your enterprise
users wouldn't need debugging output ;) ...

But if you think you need that facility then you should look at debugfs,
or relayfs, or just plain old printk, rather than implementing something
new.

cheers

-- 
Michael Ellerman
IBM OzLabs

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


signature.asc
Description: This is a digitally signed message part

Re: [PATCH 4/6] ehea: header files

[Michael Ellerman]

On Sat, 2006-08-12 at 07:40 +1000, Anton Blanchard wrote:
 Hi,
 
   drivers/net/ehea/ehea.h|  452 
 
  +#define EHEA_DRIVER_NAME   IBM eHEA
 
 You are using this for ethtool get_drvinfo. Im not sure if it should
 match the module name, and I worry about having a space in the name. Any
 ideas on what we should be doing here?

I believe it must match the module name. It also might be nice to call
it DRV_NAME like most other network drivers do.

cheers

-- 
Michael Ellerman
IBM OzLabs

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


signature.asc
Description: This is a digitally signed message part

Re: [PATCH 4/6] ehea: header files

[Michael Ellerman]

 ipz_qp_handle;  /* QP handle for h-calls */
 + struct ipz_queue ipz_squeue;
 + struct ipz_queue ipz_rqueue1;
 + struct ipz_queue ipz_rqueue2;
 + struct ipz_queue ipz_rqueue3;
 + struct h_galpas galpas;
 + struct ehea_qp_init_attr init_attr;
 +};
 +
 +struct ehea_cq_attr {
 +/* input parameter */
 + u32 max_nr_of_cqes;
 + u32 cq_token;
 + u64 eq_handle;
 +
 +/* output parameter */
 + u32 act_nr_of_cqes;
 + u32 nr_pages;
 +};
 +
 +struct ehea_cq {
 + struct ehea_adapter *adapter;
 + u64 ipz_cq_handle;
 + struct ipz_queue ipz_queue;
 + struct h_galpas galpas;
 + struct ehea_cq_attr attr;
 +};
 +
 +struct ehea_mr {
 + u64 handle;
 + u64 vaddr;
 + u32 lkey;
 +};
 +
 +struct port_state {
 + int poll_max_processed;
 + int poll_receive_errors;
 + int ehea_poll;
 + int queue_stopped;
 + int min_swqe_avail;
 + u64 sqc_stop_sum;
 + int pkt_send;
 + int pkt_xmit;
 + int send_tasklet;
 + int nwqe;
 +};
 +
 +#define EHEA_IRQ_NAME_SIZE 20
 +struct ehea_port_res {
 + struct ehea_mr send_mr;
 + struct ehea_mr recv_mr;
 + spinlock_t xmit_lock;
 + struct ehea_port *port;
 + char int_recv_name[EHEA_IRQ_NAME_SIZE];
 + char int_send_name[EHEA_IRQ_NAME_SIZE];
 + struct ehea_qp *qp;
 + struct ehea_cq *send_cq;
 + struct ehea_cq *recv_cq;
 + struct ehea_eq *send_eq;
 + struct ehea_eq *recv_eq;
 + spinlock_t send_lock;
 + struct sk_buff **skb_arr_rq1;
 + struct sk_buff **skb_arr_rq2;
 + struct sk_buff **skb_arr_rq3;
 + struct sk_buff **skb_arr_sq;
 + int skb_arr_rq1_len;
 + int skb_arr_rq2_len;
 + int skb_arr_rq3_len;
 + int skb_arr_sq_len;
 + int skb_rq2_index;
 + int skb_rq3_index;
 + int skb_sq_index;
 + spinlock_t netif_queue;
 + atomic_t swqe_avail;
 + int swqe_ll_count;
 + int swqe_count;
 + u32 swqe_id_counter;
 + u64 tx_packets;
 + struct tasklet_struct send_comp_task;
 + spinlock_t recv_lock;
 + struct timer_list timer;/* polling mode, no interrupts */
 + struct timer_list skb_timer;/* skb cleanup timer */
 + struct port_state p_state;
 + u64 rx_packets;
 + u32 poll_counter;
 +};
 +
 +
 +struct ehea_adapter {
 + u64 handle;
 + u8 num_ports;
 + struct ehea_port *port[16];
 + struct ehea_eq *neq;
 + struct tasklet_struct neq_tasklet;
 + struct ehea_mr mr;
 + u32 pd;
 + u64 max_mc_mac;
 +};
 +
 +
 +struct ehea_mc_list {
 + struct list_head list;
 + u64 macaddr;
 +};
 +
 +#define EHEA_MAX_PORT_RES 16
 +struct ehea_port {
 + struct ehea_adapter *adapter;/* adapter that owns this port */
 + struct net_device *netdev;
 + struct net_device_stats stats;
 + struct ehea_port_res port_res[EHEA_MAX_PORT_RES];
 + struct device_node *of_dev_node; /* Open Firmware Device Node */
 + struct ehea_mc_list *mc_list;/* Multicast MAC addresses */
 + struct vlan_group *vgrp;
 + struct ehea_eq *qp_eq;
 + char int_aff_name[EHEA_IRQ_NAME_SIZE];
 + int allmulti;/* Indicates IFF_ALLMULTI state */
 + int promisc; /* Indicates IFF_PROMISC state */
 + int kernel_l_key;
 + int num_tx_qps;
 + u64 mac_addr;
 + u32 logical_port_id;
 + u32 port_speed;
 + u8 full_duplex;
 + u8 num_def_qps;
 +};
 +
 +struct port_res_cfg {
 + int max_entries_rcq;
 + int max_entries_scq;
 + int max_entries_sq;
 + int max_entries_rq1;
 + int max_entries_rq2;
 + int max_entries_rq3;
 +};

Enormous structs with no comments.

-- 
Michael Ellerman
IBM OzLabs

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


signature.asc
Description: This is a digitally signed message part

Re: [PATCH 1/6] ehea: interface to network stack

[Michael Ellerman]

On Thu, 2006-08-10 at 16:15 +1000, Michael Ellerman wrote:
  +   struct hcp_query_ehea_port_cb_2 *cb2 = NULL;
  +   struct net_device_stats *stats = port-stats;
  +
  +   EDEB_EN(7, net_device=%p, dev);
  +
  +   cb2 = kzalloc(H_CB_ALIGNMENT, GFP_KERNEL);
  +   if (!cb2) {
  +   EDEB_ERR(4, No memory for cb2);
  +   goto get_stat_exit;
 
 You leak cb2 here.
 
  +   }
  +
  +   hret = ehea_h_query_ehea_port(adapter-handle,
  + port-logical_port_id,
  + H_PORT_CB2,
  + H_PORT_CB2_ALL,
  + cb2);
  +
  +   if (hret != H_SUCCESS) {
  +   EDEB_ERR(4, query_ehea_port failed for cb2);
  +   goto get_stat_exit;

Sorry, here.

cheers

-- 
Michael Ellerman
IBM OzLabs

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


signature.asc
Description: This is a digitally signed message part

Re: [PATCH] powerpc: ibmveth: Harden driver initilisation for kexec

[Michael Ellerman]

On Mon, 2006-03-27 at 12:11 +1100, Michael Ellerman wrote:
 On Thu, 2006-03-02 at 13:40 -0600, Santiago Leon wrote:
  From: Michael Ellerman [EMAIL PROTECTED]
  
  After a kexec the veth driver will fail when trying to register with the
  Hypervisor because the previous kernel has not unregistered.
  
  So if the registration fails, we unregister and then try again.
  
  Signed-off-by: Michael Ellerman [EMAIL PROTECTED]
  Acked-by: Anton Blanchard [EMAIL PROTECTED]
  Signed-off-by: Santiago Leon [EMAIL PROTECTED]
  ---
  
drivers/net/ibmveth.c |   32 ++--
1 files changed, 26 insertions(+), 6 deletions(-)
 
 Looks like this hit the floor. Any chance of getting it into to 2.6.17
 Jeff? AFAICT it should still apply cleanly.

/me knocks politely

-- 
Michael Ellerman
IBM OzLabs

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


signature.asc
Description: This is a digitally signed message part