lparcfg code

[Nathan Lynch]

Hi-

I noticed this just got committed.

ppc64 at source.scl.ameslab.gov wrote:
> full patch URL:
> http://source.scl.ameslab.gov:14690//linux-2.5/patch@1.1343
>
> ChangeSet
>   1.1343 03/12/10 15:46:38 will_schmidt at vnet.ibm.com +5 -0
>   add/forward port of lparcfg
>
>   arch/ppc64/kernel/lparcfg.c
>     1.1 03/12/10 15:43:51 will_schmidt at vnet.ibm.com +474 -0
>
>   include/asm-ppc64/hvcall.h
>     1.10 03/12/10 15:43:51 will_schmidt at vnet.ibm.com +17 -0
>     add hcall for 4 output parms
>
>   arch/ppc64/kernel/pSeries_hvCall.S
>     1.6 03/12/10 15:43:51 will_schmidt at vnet.ibm.com +38 -0
>     add hcall for 4 output parms
>
>   arch/ppc64/kernel/lparcfg.c
>     1.0 03/12/10 15:43:51 will_schmidt at vnet.ibm.com +0 -0
>     BitKeeper file /development/willschm/kernels/bk25.dec10/linux-2.5/arch/ppc64/kernel/lparcfg.c
>
>   arch/ppc64/kernel/Makefile
>     1.30 03/12/10 15:43:51 will_schmidt at vnet.ibm.com +2 -0
>     add/forward port of lparcfg
>
>   arch/ppc64/Kconfig
>     1.31 03/12/10 15:43:51 will_schmidt at vnet.ibm.com +4 -0
>     add/forward port of lparcfg
>
> ======== ChangeSet 1.1343 ========
> will_schmidt at vnet.ibm.com|ChangeSet|20031210214638|52756


The lparcfg thing doesn't want to build as a module:

*** Warning: "cur_cpu_spec" [arch/ppc64/kernel/lparcfg.ko] undefined!
*** Warning: "systemcfg" [arch/ppc64/kernel/lparcfg.ko] undefined!
*** Warning: ".plpar_hcall_4out" [arch/ppc64/kernel/lparcfg.ko] undefined!
   CC      arch/ppc64/kernel/lparcfg.mod.o
   LD [M]  arch/ppc64/kernel/lparcfg.ko

Do we really need it to be a module?  To me, it seems like something
that could just be a boolean config option instead of tristate.

Some other points and questions:

- the init function in arch/ppc64/kernel/lparcfg.c should check whether
this is an lpar system; if not, it should return without creating
/proc/ppc64/lparcfg.

- the way the lparcfg code uses the proc_dir_entry->data pointer as a
"scratch" buffer before copying to user space is pretty weird.  The read
function should allocate a separate buffer for each invocation, or use
the stack.  The data member of proc_dir_entry is usually used for
symlinks or static data, such as device tree properties.

- the lparcfg_open function seems unnecessary.

- the h_get_ppp function should be declared static, I suspect.  It
should also check the return value of plpar_hcall_4out.

- functions' opening braces should be on their own lines.

- why is the pSeries version of lparcfg_data gathering information (e.g.
serial number, system type) that is already available to users in
/proc/device-tree?

- in 2.5, of_find_node_by_path should be used instead of find_path_device.

- I noticed a few lines like this:

   if (cur_cpu_spec->firmware_features && FW_FEATURE_SPLPAR)

   I think these should be bitwise-and'ing instead of logical.

- Is /proc/ppc64/lparcfg going to be writable in order to change certain
parameters?  I am doing some work along these lines and I don't want to
duplicate effort.

Thanks,
Nathan


** Sent via the linuxppc64-dev mail list. See http://lists.linuxppc.org/