All of lore.kernel.org
 help / color / mirror / Atom feed
From: Chris Packham <judge.packham@gmail.com>
To: linux-mips@linux-mips.org
Cc: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>,
	Chris Packham <judge.packham@gmail.com>,
	"Steven J. Hill" <Steven.Hill@imgtec.com>,
	"Maciej W. Rozycki" <macro@linux-mips.org>,
	linux-kernel@vger.kernel.org,
	James Hogan <james.hogan@imgtec.com>,
	Markos Chandras <markos.chandras@imgtec.com>,
	Ralf Baechle <ralf@linux-mips.org>,
	Paul Burton <paul.burton@imgtec.com>
Subject: [RFC PATCH v1] mips: Use unsigned int when reading CP0 registers
Date: Wed, 15 Jul 2015 10:44:30 +1200	[thread overview]
Message-ID: <1436913870-17738-1-git-send-email-judge.packham@gmail.com> (raw)

Update __read_32bit_c0_register() and __read_32bit_c0_ctrl_register() to
use "unsigned int res;" instead of "int res;". There is little reason to
treat these register values as signed. They are either counters (which
by definition are unsigned) or are made up of various bit fields to be
interpreted as per the CPU datasheet.

Signed-off-by: Chris Packham <judge.packham@gmail.com>

---
This has come up via u-boot[1] which sync's asm/mipsregs.h with the
kernel. In u-boots case the value read from read_c0_count() is assigned
to an unsigned long [2] which triggers a sign extension and causes a
bug.

U-boot should probably be more explicit about the types used for the
timer_read_counter() API but that aside is there any reason to treat
these values as signed integers? A quick grep around the arch/mips makes
me thing that there may be some bugs lurking when read_c0_count() starts
to yield a negative value but I haven't really explored any of them.

I also notice that read_32bit_cp1_register has a similar issue. I
haven't touched it at this stage but it probably makes sense to do so
for consistency if the CP0 macros are changed. Looking at the users of
read_32bit_cp1_register() it's probably less of an issue.

Thanks,
Chris
--
[1] - http://lists.denx.de/pipermail/u-boot/2015-July/219086.html
[2] - http://git.denx.de/?p=u-boot.git;a=blob;f=arch/mips/cpu/time.c#l11

 arch/mips/include/asm/mipsregs.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/mips/include/asm/mipsregs.h b/arch/mips/include/asm/mipsregs.h
index c5b0956..54a942f 100644
--- a/arch/mips/include/asm/mipsregs.h
+++ b/arch/mips/include/asm/mipsregs.h
@@ -932,7 +932,7 @@ do {								\
  */
 
 #define __read_32bit_c0_register(source, sel)				\
-({ int __res;								\
+({ unsigned int __res;							\
 	if (sel == 0)							\
 		__asm__ __volatile__(					\
 			"mfc0\t%0, " #source "\n\t"			\
@@ -1014,7 +1014,7 @@ do {									\
  * On RM7000/RM9000 these are uses to access cop0 set 1 registers
  */
 #define __read_32bit_c0_ctrl_register(source)				\
-({ int __res;								\
+({ unsigned int __res;							\
 	__asm__ __volatile__(						\
 		"cfc0\t%0, " #source "\n\t"				\
 		: "=r" (__res));					\
-- 
2.5.0.rc0

             reply	other threads:[~2015-07-14 22:45 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-14 22:44 Chris Packham [this message]
2015-07-15 10:05 ` [RFC PATCH v1] mips: Use unsigned int when reading CP0 registers Ralf Baechle

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1436913870-17738-1-git-send-email-judge.packham@gmail.com \
    --to=judge.packham@gmail.com \
    --cc=Steven.Hill@imgtec.com \
    --cc=daniel.schwierzeck@gmail.com \
    --cc=james.hogan@imgtec.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mips@linux-mips.org \
    --cc=macro@linux-mips.org \
    --cc=markos.chandras@imgtec.com \
    --cc=paul.burton@imgtec.com \
    --cc=ralf@linux-mips.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.