diff mbox

[1/1] ARM: thumb: Have the machine name indicate operation in thumb mode.

Message ID 20110518223319.GB21384@n2100.arm.linux.org.uk
State New
Headers show

Commit Message

Russell King - ARM Linux May 18, 2011, 10:33 p.m. UTC
On Wed, May 18, 2011 at 06:27:56PM -0400, Nicolas Pitre wrote:
> On Wed, 18 May 2011, Vadim Bendebury (вб) wrote:
> 
> > On Mon, May 16, 2011 at 7:43 AM, Dave Martin <dave.martin@linaro.org> wrote:
> > >On Mon, May 16, 2011 at 10:32:16AM -0400, Nicolas Pitre wrote:
> > >> I think this can be useful to know how the kernel was compiled for bug
> > >> tracking purposes, just like we record the Git commit ID of the checked
> > >> out source tree, the gcc version, the build date, etc.  Which pretty
> > >> much indicates that the thumbness of the kernel should probably be
> > >> stored in the kernel banner not the platform name.
> > >
> > > That seems a reasonable approach to me.
> > >
> > 
> > guys, thank you for your comments. Indeed, this is meant to be just a
> > convenience to allow to quickly tell what kind of kernel a system is
> > running, comes handy when there are many systems around with
> > differently compiled kernels.
> > 
> > So, would it be acceptable if I move this information into the kernel
> > banner (namely, add it to the machine hardware name part of the output
> > included in 'uname -m' and 'uname -a')?
> 
> Yes, I think that would be the most sensible place.  Looking at 
> scripts/mkcompile_h which is used to create include/generated/compile.h, 
> it appears that $CONFIG_FLAGS would be the most appropriate place for 
> this, however it is not currently set up to allow architecture specific 
> flags to be passed in.

We already have a way of adding stuff for bug tracking purposes.  The
"Internal error" line is perpended with 'PREEMPT' or 'SMP' depending
on what build options were enabled.  We just need to do the same with
T2.  IOW, something like this:

Comments

Nicolas Pitre May 18, 2011, 10:49 p.m. UTC | #1
On Wed, 18 May 2011, Russell King - ARM Linux wrote:

> On Wed, May 18, 2011 at 06:27:56PM -0400, Nicolas Pitre wrote:
> > On Wed, 18 May 2011, Vadim Bendebury (вб) wrote:
> > 
> > > On Mon, May 16, 2011 at 7:43 AM, Dave Martin <dave.martin@linaro.org> wrote:
> > > >On Mon, May 16, 2011 at 10:32:16AM -0400, Nicolas Pitre wrote:
> > > >> I think this can be useful to know how the kernel was compiled for bug
> > > >> tracking purposes, just like we record the Git commit ID of the checked
> > > >> out source tree, the gcc version, the build date, etc.  Which pretty
> > > >> much indicates that the thumbness of the kernel should probably be
> > > >> stored in the kernel banner not the platform name.
> > > >
> > > > That seems a reasonable approach to me.
> > > >
> > > 
> > > guys, thank you for your comments. Indeed, this is meant to be just a
> > > convenience to allow to quickly tell what kind of kernel a system is
> > > running, comes handy when there are many systems around with
> > > differently compiled kernels.
> > > 
> > > So, would it be acceptable if I move this information into the kernel
> > > banner (namely, add it to the machine hardware name part of the output
> > > included in 'uname -m' and 'uname -a')?
> > 
> > Yes, I think that would be the most sensible place.  Looking at 
> > scripts/mkcompile_h which is used to create include/generated/compile.h, 
> > it appears that $CONFIG_FLAGS would be the most appropriate place for 
> > this, however it is not currently set up to allow architecture specific 
> > flags to be passed in.
> 
> We already have a way of adding stuff for bug tracking purposes.  The
> "Internal error" line is perpended with 'PREEMPT' or 'SMP' depending
> on what build options were enabled.  We just need to do the same with
> T2.  IOW, something like this:
[...]

This is indeed useful if the kernel oopses.  However this doesn't help 
for those cases where the kernel doesn't crash but still exhibits 
unexpected behaviors.  Having the thumbness of the kernel also displayed 
in dmesg and uname -a would also be good to have.


Nicolas
Russell King - ARM Linux May 18, 2011, 10:57 p.m. UTC | #2
On Wed, May 18, 2011 at 06:49:16PM -0400, Nicolas Pitre wrote:
> On Wed, 18 May 2011, Russell King - ARM Linux wrote:
> > We already have a way of adding stuff for bug tracking purposes.  The
> > "Internal error" line is perpended with 'PREEMPT' or 'SMP' depending
> > on what build options were enabled.  We just need to do the same with
> > T2.  IOW, something like this:
> [...]
> 
> This is indeed useful if the kernel oopses.  However this doesn't help 
> for those cases where the kernel doesn't crash but still exhibits 
> unexpected behaviors.  Having the thumbness of the kernel also displayed 
> in dmesg and uname -a would also be good to have.

In which case I think we'll have to talk to kbuild people to get
mkcompile_h modified.  Also maybe asking what other architectures
do for this kind of problem may be a good idea - especially before
we start abusing and hacking stuff like the hardware name.
Dave Martin May 19, 2011, 8:36 a.m. UTC | #3
On Wed, May 18, 2011 at 11:33:19PM +0100, Russell King - ARM Linux wrote:

[...]

> We already have a way of adding stuff for bug tracking purposes.  The
> "Internal error" line is perpended with 'PREEMPT' or 'SMP' depending
> on what build options were enabled.  We just need to do the same with
> T2.  IOW, something like this:
> 
> diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c
> index 3b54ad1..78d7714 100644
> --- a/arch/arm/kernel/traps.c
> +++ b/arch/arm/kernel/traps.c
> @@ -225,6 +225,11 @@ void show_stack(struct task_struct *tsk, unsigned long *sp)
>  #else
>  #define S_SMP ""
>  #endif
> +#ifdef CONFIG_THUMB2_KERNEL
> +#define S_ISA " T2"

Just for consistency, can I suggest that if this is implemented, we
make the string " thumb2"? (upper case optional)  This matches what
we now put in MODULE_ARCH_VERMAGIC, as well as being closer to the
CONFIG_ name.

Cheers
---Dave
Russell King - ARM Linux May 26, 2011, 1:01 p.m. UTC | #4
On Wed, May 25, 2011 at 03:08:05PM -0700, Vadim Bendebury (вб) wrote:
> On Wed, May 18, 2011 at 3:57 PM, Russell King - ARM Linux <
> linux@arm.linux.org.uk> wrote:
> > In which case I think we'll have to talk to kbuild people to get
> > mkcompile_h modified.  Also maybe asking what other architectures
> > do for this kind of problem may be a good idea - especially before
> > we start abusing and hacking stuff like the hardware name.
> 
> So, what would be an equivalent in other architectures to ask about - it
> seems that the ability to run in Thumb/ARM mode is fairly unique for ARM.

What about 32-bit vs 64-bit x86 ?

> Consulting /boot/config file is not reliable: in my own setup I can netboot
> either thumb or ARM kernel without changing the root file system contents. I
> agree though that changing the hardware name is not the best way to do this
> either.
> 
> I could look into adding 'thumb' to any element of the uname output, or as a
> watered down version - just add a line to dmesg output?

The thing that I worry about is that userspace will start using this
information inappropriately.  Whether the kernel is built as T2 or ARM
is really only a matter for the kernel itself (and people debugging the
kernel), rather than random userspace programs.

Maybe a way forward would be to produce a patch for mkcompile_h, post it
to here, kbuild people and LKML and see what response is forthcoming.
Dave Martin May 26, 2011, 1:56 p.m. UTC | #5
On Thu, May 26, 2011 at 02:01:04PM +0100, Russell King - ARM Linux wrote:
> On Wed, May 25, 2011 at 03:08:05PM -0700, Vadim Bendebury (вб) wrote:
> > On Wed, May 18, 2011 at 3:57 PM, Russell King - ARM Linux <
> > linux@arm.linux.org.uk> wrote:
> > > In which case I think we'll have to talk to kbuild people to get
> > > mkcompile_h modified.  Also maybe asking what other architectures
> > > do for this kind of problem may be a good idea - especially before
> > > we start abusing and hacking stuff like the hardware name.
> > 
> > So, what would be an equivalent in other architectures to ask about - it
> > seems that the ability to run in Thumb/ARM mode is fairly unique for ARM.
> 
> What about 32-bit vs 64-bit x86 ?

Thumb seems different because is is irrelevant to the kernel/user ABI
(or, if it's not irrelevant, we have a bug somewhere...)

Documenting what ISA the kernel was built with is therefore similar to
documenting what optimisation options were used -- i.e., it's interesting
and occasionally useful for humans, but not something software should
normally be querying or care about in any way.

> > Consulting /boot/config file is not reliable: in my own setup I can netboot
> > either thumb or ARM kernel without changing the root file system contents. I
> > agree though that changing the hardware name is not the best way to do this
> > either.
> > 
> > I could look into adding 'thumb' to any element of the uname output, or as a
> > watered down version - just add a line to dmesg output?
> 
> The thing that I worry about is that userspace will start using this
> information inappropriately.  Whether the kernel is built as T2 or ARM
> is really only a matter for the kernel itself (and people debugging the
> kernel), rather than random userspace programs.

Agreed

> 
> Maybe a way forward would be to produce a patch for mkcompile_h, post it
> to here, kbuild people and LKML and see what response is forthcoming.
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Russell King - ARM Linux May 26, 2011, 1:59 p.m. UTC | #6
On Thu, May 26, 2011 at 02:56:04PM +0100, Dave Martin wrote:
> On Thu, May 26, 2011 at 02:01:04PM +0100, Russell King - ARM Linux wrote:
> > On Wed, May 25, 2011 at 03:08:05PM -0700, Vadim Bendebury (вб) wrote:
> > > On Wed, May 18, 2011 at 3:57 PM, Russell King - ARM Linux <
> > > linux@arm.linux.org.uk> wrote:
> > > > In which case I think we'll have to talk to kbuild people to get
> > > > mkcompile_h modified.  Also maybe asking what other architectures
> > > > do for this kind of problem may be a good idea - especially before
> > > > we start abusing and hacking stuff like the hardware name.
> > > 
> > > So, what would be an equivalent in other architectures to ask about - it
> > > seems that the ability to run in Thumb/ARM mode is fairly unique for ARM.
> > 
> > What about 32-bit vs 64-bit x86 ?
> 
> Thumb seems different because is is irrelevant to the kernel/user ABI
> (or, if it's not irrelevant, we have a bug somewhere...)
> 
> Documenting what ISA the kernel was built with is therefore similar to
> documenting what optimisation options were used -- i.e., it's interesting
> and occasionally useful for humans, but not something software should
> normally be querying or care about in any way.

In which case its a bit like exposing whether the kernel was built with
or without frame pointers, or which -O level, which compiler flags, etc.

I think its best therefore that it remains in the kernel config along
with all the other 'optimisation' settings.

We don't even need to print it in the oops report because that already
tells us what ISA we were executing as - and from that and the Code: line
we can imply the kernel build ISA.
diff mbox

Patch

diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c
index 3b54ad1..78d7714 100644
--- a/arch/arm/kernel/traps.c
+++ b/arch/arm/kernel/traps.c
@@ -225,6 +225,11 @@  void show_stack(struct task_struct *tsk, unsigned long *sp)
 #else
 #define S_SMP ""
 #endif
+#ifdef CONFIG_THUMB2_KERNEL
+#define S_ISA " T2"
+#else
+#define S_ISA " ARM"
+#endif
 
 static int __die(const char *str, int err, struct thread_info *thread, struct pt_regs *regs)
 {
@@ -232,8 +237,8 @@  static int __die(const char *str, int err, struct thread_info *thread, struct pt
 	static int die_counter;
 	int ret;
 
-	printk(KERN_EMERG "Internal error: %s: %x [#%d]" S_PREEMPT S_SMP "\n",
-	       str, err, ++die_counter);
+	printk(KERN_EMERG "Internal error: %s: %x [#%d]" S_PREEMPT S_SMP
+	       S_ISA "\n", str, err, ++die_counter);
 	sysfs_printk_last_file();
 
 	/* trap and error numbers are mostly meaningless on ARM */