From: Nicolas DET <hidden> Date: 2006-10-17 12:00:15
This patch enables RTAS /proc support for PowerPC 32bits / CHRP platform.
A new entry is created (/proc/ppc/rtas/) which contains the RTAS nodes.
Theses nodes and the code are the exact same than with the ppc64
architecture.
This patch has been succefully applied on the kernel 2.6.18.1 and tested
on bPlan's OpenFirmware.
Signed-off-by: Nicolas DET <redacted>
Please don't do any pathname changes. Even if ppc64 isn't correct it's
what applications expect and what we should provide for a coherent user
interface.
- if (!machine_is(pseries))
+ if ( ! ( machine_is(pseries) || machine_is(chrp) ) )
return -ENODEV;
This should be the only change you need, and it should follow kernel
coding style, aka:
if (!machine_is(pseries) && !machine_is(chrp))
return -ENODEV;
rtas_node = of_find_node_by_name(NULL, "rtas");
if (rtas_node == NULL)
return -ENODEV;
And given this check I wonder why we need the platform check at all. It
should be safe to just remove it.
From: Nicolas DET <hidden> Date: 2006-10-18 05:54:10
Christoph Hellwig wrote:
>> --- a/arch/powerpc/kernel/rtas-proc.c 2006-10-14
05:34:03.000000000 +0200
>> +++ b/arch/powerpc/kernel/rtas-proc.c 2006-10-16
10:46:16.000000000 +0200
>> @@ -253,43 +253,70 @@ static void get_location_code(struct seq
>> static void check_location_string(struct seq_file *m, char *c);
>> static void check_location(struct seq_file *m, char *c);
>>
>> +#ifdef CONFIG_PPC64
>> +#define PROCRTAS_ROOT "ppc64"
>> +#else
>> +#define PROCRTAS_ROOT "ppc"
>
> Please don't do any pathname changes. Even if ppc64 isn't correct it's
> what applications expect and what we should provide for a coherent user
> interface.
Humm, ok.
However, in this case 'ppc' (could be 32 or 64 as it is not specified)
is more generic than 'ppc64'.
> This should be the only change you need, and it should follow kernel
> coding style, aka:
>
> if (!machine_is(pseries) && !machine_is(chrp))
> return -ENODEV;
>
>> rtas_node = of_find_node_by_name(NULL, "rtas");
>> if (rtas_node == NULL)
>> return -ENODEV;
>
> And given this check I wonder why we need the platform check at all. It
> should be safe to just remove it.
>
>
Indeed, however I can only test on CHRP. I'll remove the check in the
upcomming patch.
The patch also include a small code to create the /proc/ppc/rtas entry.
Should this be done here, or somewhere in arch/powerpc/chrp/setup.c ?
I will re submit a patch as soon as I can.
Please don't do any pathname changes. Even if ppc64 isn't correct it's
what applications expect and what we should provide for a coherent user
interface.
Humm, ok.
However, in this case 'ppc' (could be 32 or 64 as it is not specified)
is more generic than 'ppc64'.
Christoph, Nicolas, ...
What about doing the logical thing, and switching it to "powerpc" like the
recent arch change, or more logically, to "power" like the current rebranding
of the whole thing ?
I suppose we can also provide some symlinking or similar for compatibility ?
Or move the whole stuff under /sys/.../power/rtas ?
Friendly,
Sven Luther
From: Michael Ellerman <hidden> Date: 2006-10-18 06:15:09
On Wed, 2006-10-18 at 07:51 +0200, Nicolas DET wrote:
Christoph Hellwig wrote:
>> --- a/arch/powerpc/kernel/rtas-proc.c 2006-10-14
05:34:03.000000000 +0200
>> +++ b/arch/powerpc/kernel/rtas-proc.c 2006-10-16
10:46:16.000000000 +0200
>> @@ -253,43 +253,70 @@ static void get_location_code(struct seq
>> static void check_location_string(struct seq_file *m, char *c);
>> static void check_location(struct seq_file *m, char *c);
>>
>> +#ifdef CONFIG_PPC64
>> +#define PROCRTAS_ROOT "ppc64"
>> +#else
>> +#define PROCRTAS_ROOT "ppc"
>
> Please don't do any pathname changes. Even if ppc64 isn't correct it's
> what applications expect and what we should provide for a coherent user
> interface.
Humm, ok.
However, in this case 'ppc' (could be 32 or 64 as it is not specified)
is more generic than 'ppc64'.
But it's called '/proc/ppc64' right now on lots of machines, so you
can't go changing it.
> This should be the only change you need, and it should follow kernel
> coding style, aka:
>
> if (!machine_is(pseries) && !machine_is(chrp))
> return -ENODEV;
>
>> rtas_node = of_find_node_by_name(NULL, "rtas");
>> if (rtas_node == NULL)
>> return -ENODEV;
>
> And given this check I wonder why we need the platform check at all. It
> should be safe to just remove it.
>
>
Indeed, however I can only test on CHRP. I'll remove the check in the
upcomming patch.
That should be fine AFAICT, you should probably just check that each of
the proc routines checks for errors - ie. just because you have an
"/rtas" node doesn't mean you necessarily have "/rtas/set-indicator" or
whatever.
The patch also include a small code to create the /proc/ppc/rtas entry.
Should this be done here, or somewhere in arch/powerpc/chrp/setup.c ?
That code is almost entirely the same as proc_ppc64_create(), so I think
you should try and merge them - we want to minimise the number of
foo_ppc64() and foo_ppc32() routines we have.
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
From: Nicolas DET <hidden> Date: 2006-10-18 06:36:21
Michael Ellerman wrote:
>> >> +#ifdef CONFIG_PPC64
>> >> +#define PROCRTAS_ROOT "ppc64"
>> >> +#else
>> >> +#define PROCRTAS_ROOT "ppc"
>> >
>> > Please don't do any pathname changes. Even if ppc64 isn't
correct it's
>> > what applications expect and what we should provide for a
coherent user
>> > interface.
>>
>> Humm, ok.
>> However, in this case 'ppc' (could be 32 or 64 as it is not specified)
>> is more generic than 'ppc64'.
>
> But it's called '/proc/ppc64' right now on lots of machines, so you
> can't go changing it.
Ok. No problem
>>
>> Indeed, however I can only test on CHRP. I'll remove the check in the
>> upcomming patch.
>
> That should be fine AFAICT, you should probably just check that each of
> the proc routines checks for errors - ie. just because you have an
> "/rtas" node doesn't mean you necessarily have "/rtas/set-indicator" or
> whatever.
ok
>
>> The patch also include a small code to create the /proc/ppc/rtas entry.
>> Should this be done here, or somewhere in arch/powerpc/chrp/setup.c ?
>
> That code is almost entirely the same as proc_ppc64_create(), so I think
> you should try and merge them - we want to minimise the number of
> foo_ppc64() and foo_ppc32() routines we have.
>
Ok.
proc_ppc64_create() is now locate in arch/powerpc/kernel/proc_ppc64.c.
Maybe a new file could be created (arch/powerpc/kernel/proc_ppc.c) ?
Any other suggestions?
Just indicate me the direction, and I would provide others patches.
Regards
From: Olaf Hering <hidden> Date: 2006-10-18 07:39:08
On Wed, Oct 18, Michael Ellerman wrote:
But it's called '/proc/ppc64' right now on lots of machines, so you
can't go changing it.
if test -d /proc/ppc64 .. is a quick way to check wether the system is
64bit or not. Similar to if test -d /proc/iSeries ..
You better leave that simple interface.
=20
And given this check I wonder why we need the platform check at all. =A0It
should be safe to just remove it.
=20
One difference would be that it triggers on machines running SLOF (QS20,=20
some JS20/JS21) and maybe some older Macs, which is probably a good
thing.
I wonder if it should be a little stricter though:
rtas_node =3D of_find_node_by_path("/rtas");
if (!rtas_node)
return -ENODEV;
In case there is a node called "rtas" somewhere else.
Arnd <><
From: Olaf Hering <hidden> Date: 2006-10-20 05:44:50
On Fri, Oct 20, Benjamin Herrenschmidt wrote:
On Thu, 2006-10-19 at 09:03 +0200, Olaf Hering wrote:
quoted
On Thu, Oct 19, Benjamin Herrenschmidt wrote:
quoted
On Wed, 2006-10-18 at 09:38 +0200, Olaf Hering wrote:
quoted
On Wed, Oct 18, Michael Ellerman wrote:
quoted
But it's called '/proc/ppc64' right now on lots of machines, so you
can't go changing it.
if test -d /proc/ppc64 .. is a quick way to check wether the system is
64bit or not. Similar to if test -d /proc/iSeries ..
That's really ugly ! You really do that ? Gack...
What way should I use?
uname ? /proc/cpuinfo ? Whatever but not that.
uname can be faked with powerpc32 or setarch. grep POWER /proc/cpuinfo
might be an option. I think its only used in rpm preinstall scripts to
reject 64bit kernel on 32bit systems or the other way around.
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2006-10-20 05:57:00
On Fri, 2006-10-20 at 07:44 +0200, Olaf Hering wrote:
On Fri, Oct 20, Benjamin Herrenschmidt wrote:
quoted
On Thu, 2006-10-19 at 09:03 +0200, Olaf Hering wrote:
quoted
On Thu, Oct 19, Benjamin Herrenschmidt wrote:
quoted
On Wed, 2006-10-18 at 09:38 +0200, Olaf Hering wrote:
quoted
On Wed, Oct 18, Michael Ellerman wrote:
quoted
But it's called '/proc/ppc64' right now on lots of machines, so you
can't go changing it.
if test -d /proc/ppc64 .. is a quick way to check wether the system is
64bit or not. Similar to if test -d /proc/iSeries ..
That's really ugly ! You really do that ? Gack...
What way should I use?
uname ? /proc/cpuinfo ? Whatever but not that.
uname can be faked with powerpc32 or setarch. grep POWER /proc/cpuinfo
might be an option. I think its only used in rpm preinstall scripts to
reject 64bit kernel on 32bit systems or the other way around.
grep POWER won't do good on 970 among others.
uname can be faked but why do you care ?
In fact, is somebody is faking it, maybe it's because they -want- your
scripts to think they are running on a 32 bits box....
I still think uname is the way to go.
Ben.
From: Sven Luther <hidden> Date: 2006-10-20 06:28:21
On Fri, Oct 20, 2006 at 03:56:37PM +1000, Benjamin Herrenschmidt wrote:
On Fri, 2006-10-20 at 07:44 +0200, Olaf Hering wrote:
quoted
On Fri, Oct 20, Benjamin Herrenschmidt wrote:
quoted
On Thu, 2006-10-19 at 09:03 +0200, Olaf Hering wrote:
quoted
On Thu, Oct 19, Benjamin Herrenschmidt wrote:
quoted
On Wed, 2006-10-18 at 09:38 +0200, Olaf Hering wrote:
quoted
On Wed, Oct 18, Michael Ellerman wrote:
quoted
But it's called '/proc/ppc64' right now on lots of machines, so you
can't go changing it.
if test -d /proc/ppc64 .. is a quick way to check wether the system is
64bit or not. Similar to if test -d /proc/iSeries ..
That's really ugly ! You really do that ? Gack...
What way should I use?
uname ? /proc/cpuinfo ? Whatever but not that.
uname can be faked with powerpc32 or setarch. grep POWER /proc/cpuinfo
might be an option. I think its only used in rpm preinstall scripts to
reject 64bit kernel on 32bit systems or the other way around.
grep POWER won't do good on 970 among others.
uname can be faked but why do you care ?
In fact, is somebody is faking it, maybe it's because they -want- your
scripts to think they are running on a 32 bits box....
I still think uname is the way to go.
Ben, would it make sense to add two informations to /proc/cpuinfo ?
It would be great to have a field there which will allow to check for the
kernel flavour to run, namely if the kernel/processor/whatever is running in
32bit or 64bit mode. The second field would be a generalized machine field,
or plateform or whatever, which would easily allow to differentiate between an
apple machine, a ibm chrp, a pegasos or other genesi product, etc. Without
necessarily having to maintain a huge amount of userlanf mappings from all
those different machine: fields, which all have CHRP, true, but no finer
grained informations, while we do have it in the kernel.
Friendly,
Sven Luther
From: Olaf Hering <hidden> Date: 2006-10-20 06:45:15
On Fri, Oct 20, Sven Luther wrote:
Ben, would it make sense to add two informations to /proc/cpuinfo ?
It would be great to have a field there which will allow to check for the
kernel flavour to run, namely if the kernel/processor/whatever is running in
32bit or 64bit mode. The second field would be a generalized machine field,
or plateform or whatever, which would easily allow to differentiate between an
apple machine, a ibm chrp, a pegasos or other genesi product, etc. Without
necessarily having to maintain a huge amount of userlanf mappings from all
those different machine: fields, which all have CHRP, true, but no finer
grained informations, while we do have it in the kernel.
/proc/device-tree/{model,device_type,compatible} is enough to map
everything reliable in the installer. Dont pollute /proc/cpuinfo
further. There has already enough stuff that doesnt belong there, like
pmac-generation, 'pmac flags', 'detected as', motherboard and machine.
Oh, and bogomips...
About 32bit/64bit, maybe VmallocTotal from /proc/meminfo can be used.
incredible large numer == must be a 64bit kernel
No idea how reliable it is. There are those 36bit systems, but I bet
they dont run a distro.
From: Sven Luther <hidden> Date: 2006-10-20 07:02:55
On Fri, Oct 20, 2006 at 08:44:56AM +0200, Olaf Hering wrote:
On Fri, Oct 20, Sven Luther wrote:
quoted
Ben, would it make sense to add two informations to /proc/cpuinfo ?
It would be great to have a field there which will allow to check for the
kernel flavour to run, namely if the kernel/processor/whatever is running in
32bit or 64bit mode. The second field would be a generalized machine field,
or plateform or whatever, which would easily allow to differentiate between an
apple machine, a ibm chrp, a pegasos or other genesi product, etc. Without
necessarily having to maintain a huge amount of userlanf mappings from all
those different machine: fields, which all have CHRP, true, but no finer
grained informations, while we do have it in the kernel.
/proc/device-tree/{model,device_type,compatible} is enough to map
Yeah, well.
everything reliable in the installer. Dont pollute /proc/cpuinfo
further. There has already enough stuff that doesnt belong there, like
pmac-generation, 'pmac flags', 'detected as', motherboard and machine.
Oh, and bogomips...
Exact, it is not about poluting, but about rationalizing. The fact is that
these are the two infos that are the most necessary to userland, then why
disseminate the info in thousand different places, instead of making it easily
available in a canonical place in a standard format for everyone ?
About 32bit/64bit, maybe VmallocTotal from /proc/meminfo can be used.
incredible large numer == must be a 64bit kernel
No idea how reliable it is. There are those 36bit systems, but I bet
they dont run a distro.
This sounds like the ugliest hack i have seen around, and is prone to break. A
proper /proc/cpuinfo flag would be most welcome to solve this cleanly.
Friendly,
Sven Luther
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2006-10-20 07:12:20
On Fri, 2006-10-20 at 08:44 +0200, Olaf Hering wrote:
On Fri, Oct 20, Sven Luther wrote:
quoted
Ben, would it make sense to add two informations to /proc/cpuinfo ?
It would be great to have a field there which will allow to check for the
kernel flavour to run, namely if the kernel/processor/whatever is running in
32bit or 64bit mode. The second field would be a generalized machine field,
or plateform or whatever, which would easily allow to differentiate between an
apple machine, a ibm chrp, a pegasos or other genesi product, etc. Without
necessarily having to maintain a huge amount of userlanf mappings from all
those different machine: fields, which all have CHRP, true, but no finer
grained informations, while we do have it in the kernel.
/proc/device-tree/{model,device_type,compatible} is enough to map
everything reliable in the installer. Dont pollute /proc/cpuinfo
further. There has already enough stuff that doesnt belong there, like
pmac-generation, 'pmac flags', 'detected as', motherboard and machine.
Oh, and bogomips...
Some powermac detection bits for older models are not actually available
via the device-tree :)
About 32bit/64bit, maybe VmallocTotal from /proc/meminfo can be used.
incredible large numer == must be a 64bit kernel
No idea how reliable it is. There are those 36bit systems, but I bet
they dont run a distro.
But why are you absolutely trying to defeat uname ?
If somebody is trying to fake it, then use the faked version !
Ben.
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2006-10-20 07:13:15
Exact, it is not about poluting, but about rationalizing. The fact is that
these are the two infos that are the most necessary to userland, then why
disseminate the info in thousand different places, instead of making it easily
available in a canonical place in a standard format for everyone ?
Why would that be absolutely necessary to userland ? We've lived very
well without that so far.
quoted
About 32bit/64bit, maybe VmallocTotal from /proc/meminfo can be used.
incredible large numer == must be a 64bit kernel
No idea how reliable it is. There are those 36bit systems, but I bet
they dont run a distro.
This sounds like the ugliest hack i have seen around, and is prone to break. A
proper /proc/cpuinfo flag would be most welcome to solve this cleanly.
uname is your friend... damn, and if you want to be real sure, then just
try to run a 64 bits binary and see what happens :)
Ben.
From: Olaf Hering <hidden> Date: 2006-10-20 07:14:20
On Fri, Oct 20, Benjamin Herrenschmidt wrote:
quoted
/proc/device-tree/{model,device_type,compatible} is enough to map
everything reliable in the installer. Dont pollute /proc/cpuinfo
further. There has already enough stuff that doesnt belong there, like
pmac-generation, 'pmac flags', 'detected as', motherboard and machine.
Oh, and bogomips...
Some powermac detection bits for older models are not actually available
via the device-tree :)
From: Olaf Hering <hidden> Date: 2006-10-20 07:20:59
On Fri, Oct 20, Sven Luther wrote:
Exact, it is not about poluting, but about rationalizing. The fact is that
these are the two infos that are the most necessary to userland, then why
disseminate the info in thousand different places, instead of making it easily
available in a canonical place in a standard format for everyone ?
cpuinfo is about the cpu, not the board layout nor the firmware
expectations.
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2006-10-20 07:37:17
On Fri, 2006-10-20 at 09:14 +0200, Olaf Hering wrote:
On Fri, Oct 20, Benjamin Herrenschmidt wrote:
quoted
quoted
/proc/device-tree/{model,device_type,compatible} is enough to map
everything reliable in the installer. Dont pollute /proc/cpuinfo
further. There has already enough stuff that doesnt belong there, like
pmac-generation, 'pmac flags', 'detected as', motherboard and machine.
Oh, and bogomips...
Some powermac detection bits for older models are not actually available
via the device-tree :)
What is missing?
Well, PowerBook 2400 vs. 3400 typically. There is more that we don't
even implement in the kernel yet but maybe one day ... :)
Ben.
From: Sven Luther <hidden> Date: 2006-10-20 07:40:52
On Fri, Oct 20, 2006 at 05:12:52PM +1000, Benjamin Herrenschmidt wrote:
quoted
Exact, it is not about poluting, but about rationalizing. The fact is that
these are the two infos that are the most necessary to userland, then why
disseminate the info in thousand different places, instead of making it easily
available in a canonical place in a standard format for everyone ?
Why would that be absolutely necessary to userland ? We've lived very
well without that so far.
Because i already coded at least 4 times in 4 different places, code which
does the differentiation for userland, and it is only in debian. All other
distributions have this kind of code in their own individual way.
And if there is a new hardware coming out you didn't think about, it is not
enough to add support in the kernel, but you have to change all those userland
tools all over the place, with load of chances to forget something and break
them.
The kernel knows about this, and it would be easy enough to pass that
information clearly to userland. /proc/device-tree is indeed a solution for
this part.
quoted
quoted
About 32bit/64bit, maybe VmallocTotal from /proc/meminfo can be used.
incredible large numer == must be a 64bit kernel
No idea how reliable it is. There are those 36bit systems, but I bet
they dont run a distro.
This sounds like the ugliest hack i have seen around, and is prone to break. A
proper /proc/cpuinfo flag would be most welcome to solve this cleanly.
uname is your friend... damn, and if you want to be real sure, then just
try to run a 64 bits binary and see what happens :)
Why not say it directly in a standard way ? So we avoid all distros and/or
different tools who need to know, to implement their own slightly different
way ?
Friendly,
Sven Luther
From: Sven Luther <hidden> Date: 2006-10-20 07:41:44
On Fri, Oct 20, 2006 at 09:20:37AM +0200, Olaf Hering wrote:
On Fri, Oct 20, Sven Luther wrote:
quoted
Exact, it is not about poluting, but about rationalizing. The fact is that
these are the two infos that are the most necessary to userland, then why
disseminate the info in thousand different places, instead of making it easily
available in a canonical place in a standard format for everyone ?
cpuinfo is about the cpu, not the board layout nor the firmware
expectations.
What is the machine: field doing there then ? Would it make sense to provide a
machineinfo then ?
Friendly,
Sven Luther
From: Olaf Hering <hidden> Date: 2006-10-20 07:50:10
On Fri, Oct 20, Sven Luther wrote:
On Fri, Oct 20, 2006 at 09:20:37AM +0200, Olaf Hering wrote:
quoted
On Fri, Oct 20, Sven Luther wrote:
quoted
Exact, it is not about poluting, but about rationalizing. The fact is that
these are the two infos that are the most necessary to userland, then why
disseminate the info in thousand different places, instead of making it easily
available in a canonical place in a standard format for everyone ?
cpuinfo is about the cpu, not the board layout nor the firmware
expectations.
What is the machine: field doing there then ? Would it make sense to provide a
machineinfo then ?
From: Nicolas DET <hidden> Date: 2006-10-20 08:16:13
Arnd Bergmann wrote:
> On Tuesday 17 October 2006 15:22, Christoph Hellwig wrote:
>>> rtas_node = of_find_node_by_name(NULL, "rtas");
>>> if (rtas_node == NULL)
>>> return -ENODEV;
>> And given this check I wonder why we need the platform check at all. It
>> should be safe to just remove it.
>>
> One difference would be that it triggers on machines running SLOF
(QS20, some JS20/JS21) and maybe some older Macs, which is probably a good
> thing.
>
> I wonder if it should be a little stricter though:
>
>> rtas_node = of_find_node_by_path("/rtas");
>> if (!rtas_node)
>> return -ENODEV;
>
> In case there is a node called "rtas" somewhere else.
>
> Arnd <><
>
Maybe we could check for the RTAS revision. The CHRP manual define this
properties 'rtas-version' in /rtas/. This should be 1 for current
implementation.
You can find here a new patch. It does not create the /proc/rtas and the
symlink /proc/ppc64/rtas in arch/powerpc/kernel/proc_ppc64.c anymore
but rather create the entry (/proc/rtas) in
arch/powerpc/kernel/rtas-proc.c and only the ppc64/ratas link for 64bit
machine.
This has been tested on our PowerPC 32bit machine. It would be nice if
PowerPC64 developer could reveiw and test it.
I stay ready to test upcomming patch if required.
Regards
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2006-10-20 08:17:26
Why not say it directly in a standard way ? So we avoid all distros and/or
different tools who need to know, to implement their own slightly different
way ?
From: Sven Luther <hidden> Date: 2006-10-20 08:55:56
On Fri, Oct 20, 2006 at 10:12:40AM +0200, Segher Boessenkool wrote:
quoted
quoted
cpuinfo is about the cpu, not the board layout nor the firmware
expectations.
What is the machine: field doing there then ? Would it make sense
to provide a
machineinfo then ?
Yes. Or better, don't use /proc, get all info from sysfs
(with a nice userland tool to summarize the info, perhaps).
Yeah, that makes sense. Will the device-tree move to /sys some day ? What
about things like the /rtas node ?
But it will be years before all the distros move to it probably, see how most
of them still use parsing of the parted command line output for handling
partition table detection, and thus fail on anything not MBR.
Friendly,
Sven Luther
From: Paul Mackerras <hidden> Date: 2006-10-20 10:01:46
Olaf Hering writes:
About 32bit/64bit, maybe VmallocTotal from /proc/meminfo can be used.
incredible large numer == must be a 64bit kernel
No idea how reliable it is. There are those 36bit systems, but I bet
they dont run a distro.
They only have 32-bit effective addresses and therefore a similar
VmallocTotal to any other 32-bit powerpc machine.
Paul.