subject:"\[PATCH\] powerpc\: Fix bogus usage of MSR

Re: [PATCH] powerpc: Fix bogus usage of MSR_RI on BookE and 40x

2018-12-11 Thread Christophe Leroy





Le 11/12/2018 à 02:57, Benjamin Herrenschmidt a écrit :

BookE and 40x processors lack the MSR:RI bit. However, we have a
few common code places that rely on it.

This fixes it by not defining MSR_RI on those processor types and
using the appropriate ifdef's in those locations.

Signed-off-by: Benjamin Herrenschmidt 
---
  arch/powerpc/include/asm/reg.h   | 2 ++
  arch/powerpc/include/asm/reg_booke.h | 4 ++--
  arch/powerpc/kernel/process.c| 2 +-
  arch/powerpc/kernel/traps.c  | 8 ++--
  arch/powerpc/lib/sstep.c | 2 ++
  5 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
index de52c31..41d0b2e 100644
--- a/arch/powerpc/include/asm/reg.h
+++ b/arch/powerpc/include/asm/reg.h
@@ -110,7 +110,9 @@
  #ifndef MSR_PMM
  #define MSR_PMM   __MASK(MSR_PMM_LG)  /* Performance monitor 
*/
  #endif
+#if !defined(CONFIG_BOOKE) && !defined(CONFIG_4xx)
  #define MSR_RI__MASK(MSR_RI_LG)   /* Recoverable 
Exception */
+#endif
  #define MSR_LE__MASK(MSR_LE_LG)   /* Little Endian */
  
  #define MSR_TM		__MASK(MSR_TM_LG)	/* Transactional Mem Available */

diff --git a/arch/powerpc/include/asm/reg_booke.h 
b/arch/powerpc/include/asm/reg_booke.h
index eb2a33d..06967f1 100644
--- a/arch/powerpc/include/asm/reg_booke.h
+++ b/arch/powerpc/include/asm/reg_booke.h
@@ -46,10 +46,10 @@
  #define MSR_USER32(MSR_ | MSR_PR | MSR_EE)
  #define MSR_USER64(MSR_USER32 | MSR_64BIT)
  #elif defined (CONFIG_40x)
-#define MSR_KERNEL (MSR_ME|MSR_RI|MSR_IR|MSR_DR|MSR_CE)
+#define MSR_KERNEL (MSR_ME|MSR_IR|MSR_DR|MSR_CE)
  #define MSR_USER  (MSR_KERNEL|MSR_PR|MSR_EE)
  #else
-#define MSR_KERNEL (MSR_ME|MSR_RI|MSR_CE)
+#define MSR_KERNEL (MSR_ME|MSR_CE)
  #define MSR_USER  (MSR_KERNEL|MSR_PR|MSR_EE)
  #endif
  
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c

index 96f3473..77679a7 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -1359,7 +1359,7 @@ static struct regbit msr_bits[] = {
{MSR_IR,"IR"},
{MSR_DR,"DR"},
{MSR_PMM,   "PMM"},
-#ifndef CONFIG_BOOKE
+#if !defined(CONFIG_BOOKE) && !defined(CONFIG_40x)
{MSR_RI,"RI"},
{MSR_LE,"LE"},
  #endif
diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index 9a86572..2d00696 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -429,10 +429,11 @@ void system_reset_exception(struct pt_regs *regs)
if (get_paca()->in_nmi > 1)
nmi_panic(regs, "Unrecoverable nested System Reset");
  #endif
+#ifdef MSR_RI
/* Must die if the interrupt is not recoverable */
if (!(regs->msr & MSR_RI))
nmi_panic(regs, "Unrecoverable System Reset");
-
+#endif


Could we have a helper in .h file instead of #ifdefs ?

For instance something like

#if defined(CONFIG_4xx) || defined(CONFIG_BOOKE)
static inline bool exception_is_recoverable(u32 msr)
{
return msr & MSR_RI;
}
#else
static inline bool exception_is_recoverable(u32 msr)
{
return true;
}
#endif

Or reuse the function unrecoverable_excp() defined in xmon.c ?


if (!nested)
nmi_exit();
  
@@ -478,7 +479,9 @@ static inline int check_io_access(struct pt_regs *regs)

printk(KERN_DEBUG "%s bad port %lx at %p\n",
   (*nip & 0x100)? "OUT to": "IN from",
   regs->gpr[rb] - _IO_BASE, nip);
+#ifdef MSR_RI
regs->msr |= MSR_RI;
+#endif


Same here, a helper, something like that, to also be used in 
arch/powerpc/sysdev/fsl_rio.c ?


#if defined(CONFIG_4xx) || defined(CONFIG_BOOKE)
static inline void exception_set_recoverable(struct pt_regs *regs)
{
regs->msr |= MSR_RI;
}
#else
static inline void exception_set_recoverable(struct pt_regs *regs)
{
}
#endif


regs->nip = extable_fixup(entry);
return 1;
}
@@ -763,10 +766,11 @@ void machine_check_exception(struct pt_regs *regs)
if (check_io_access(regs))
goto bail;
  
+#ifdef MSR_RI

/* Must die if the interrupt is not recoverable */
if (!(regs->msr & MSR_RI))
nmi_panic(regs, "Unrecoverable Machine check");
-
+#endif


Same


if (!nested)
nmi_exit();
  
diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c

index d81568f..c03c453 100644
--- a/arch/powerpc/lib/sstep.c
+++ b/arch/powerpc/lib/sstep.c
@@ -3059,9 +3059,11 @@ int emulate_step(struct pt_regs *regs, unsigned int 
instr)
  
  	case MTMSR:

val = regs->gpr[op.reg];
+#ifdef MSR_RI
if ((val & MSR_RI) == 0)


Could be something like the following instead of this #ifdef stuff ?

		if (!IS_ENABLED(CONFIG_4xx) &&

Re: [PATCH] powerpc: Fix bogus usage of MSR_RI on BookE and 40x

2018-12-10 Thread kbuild test robot

Hi Benjamin,

I love your patch! Yet something to improve:

[auto build test ERROR on powerpc/next]
[also build test ERROR on v4.20-rc6 next-20181210]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Benjamin-Herrenschmidt/powerpc-Fix-bogus-usage-of-MSR_RI-on-BookE-and-40x/20181211-110017
base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-ppa8548_defconfig (attached as .config)
compiler: powerpc-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.2.0 make.cross ARCH=powerpc 

All errors (new ones prefixed by >>):

   arch/powerpc/sysdev/fsl_rio.c: In function 'fsl_rio_mcheck_exception':
>> arch/powerpc/sysdev/fsl_rio.c:115:17: error: 'MSR_RI' undeclared (first use 
>> in this function); did you mean 'MSR_BE'?
   regs->msr |= MSR_RI;
^~
MSR_BE
   arch/powerpc/sysdev/fsl_rio.c:115:17: note: each undeclared identifier is 
reported only once for each function it appears in

vim +115 arch/powerpc/sysdev/fsl_rio.c

a52c8f52 Alexandre Bounine 2010-05-26   96  
ff33f182 Li Yang   2010-06-18   97  #ifdef CONFIG_E500
cce1f106 Shaohui Xie   2010-11-18   98  int fsl_rio_mcheck_exception(struct 
pt_regs *regs)
a52c8f52 Alexandre Bounine 2010-05-26   99  {
82a9a480 Scott Wood2011-06-16  100  const struct 
exception_table_entry *entry;
82a9a480 Scott Wood2011-06-16  101  unsigned long reason;
82a9a480 Scott Wood2011-06-16  102  
82a9a480 Scott Wood2011-06-16  103  if (!rio_regs_win)
82a9a480 Scott Wood2011-06-16  104  return 0;
a52c8f52 Alexandre Bounine 2010-05-26  105  
a52c8f52 Alexandre Bounine 2010-05-26  106  reason = in_be32((u32 
*)(rio_regs_win + RIO_LTLEDCSR));
a52c8f52 Alexandre Bounine 2010-05-26  107  if (reason & (RIO_LTLEDCSR_IER 
| RIO_LTLEDCSR_PRT)) {
a52c8f52 Alexandre Bounine 2010-05-26  108  /* Check if we are 
prepared to handle this fault */
a52c8f52 Alexandre Bounine 2010-05-26  109  entry = 
search_exception_tables(regs->nip);
a52c8f52 Alexandre Bounine 2010-05-26  110  if (entry) {
a52c8f52 Alexandre Bounine 2010-05-26  111  pr_debug("RIO: 
%s - MC Exception handled\n",
a52c8f52 Alexandre Bounine 2010-05-26  112   
__func__);
a52c8f52 Alexandre Bounine 2010-05-26  113  out_be32((u32 
*)(rio_regs_win + RIO_LTLEDCSR),
a52c8f52 Alexandre Bounine 2010-05-26  114   0);
a52c8f52 Alexandre Bounine 2010-05-26 @115  regs->msr |= 
MSR_RI;
61a92f70 Nicholas Piggin   2016-10-14  116  regs->nip = 
extable_fixup(entry);
a52c8f52 Alexandre Bounine 2010-05-26  117  return 1;
a52c8f52 Alexandre Bounine 2010-05-26  118  }
a52c8f52 Alexandre Bounine 2010-05-26  119  }
a52c8f52 Alexandre Bounine 2010-05-26  120  
cce1f106 Shaohui Xie   2010-11-18  121  return 0;
a52c8f52 Alexandre Bounine 2010-05-26  122  }
cce1f106 Shaohui Xie   2010-11-18  123  
EXPORT_SYMBOL_GPL(fsl_rio_mcheck_exception);
ff33f182 Li Yang   2010-06-18  124  #endif
a52c8f52 Alexandre Bounine 2010-05-26  125  

:: The code at line 115 was first introduced by commit
:: a52c8f521fed43bce53451d7dfddf2b42a2af689 rapidio, powerpc/85xx: Add MChk 
handler for SRIO port

:: TO: Alexandre Bounine 
:: CC: Linus Torvalds 

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

[PATCH] powerpc: Fix bogus usage of MSR_RI on BookE and 40x

2018-12-10 Thread Benjamin Herrenschmidt

BookE and 40x processors lack the MSR:RI bit. However, we have a
few common code places that rely on it.

This fixes it by not defining MSR_RI on those processor types and
using the appropriate ifdef's in those locations.

Signed-off-by: Benjamin Herrenschmidt 
---
 arch/powerpc/include/asm/reg.h   | 2 ++
 arch/powerpc/include/asm/reg_booke.h | 4 ++--
 arch/powerpc/kernel/process.c| 2 +-
 arch/powerpc/kernel/traps.c  | 8 ++--
 arch/powerpc/lib/sstep.c | 2 ++
 5 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
index de52c31..41d0b2e 100644
--- a/arch/powerpc/include/asm/reg.h
+++ b/arch/powerpc/include/asm/reg.h
@@ -110,7 +110,9 @@
 #ifndef MSR_PMM
 #define MSR_PMM__MASK(MSR_PMM_LG)  /* Performance monitor 
*/
 #endif
+#if !defined(CONFIG_BOOKE) && !defined(CONFIG_4xx)
 #define MSR_RI __MASK(MSR_RI_LG)   /* Recoverable Exception */
+#endif
 #define MSR_LE __MASK(MSR_LE_LG)   /* Little Endian */
 
 #define MSR_TM __MASK(MSR_TM_LG)   /* Transactional Mem Available 
*/
diff --git a/arch/powerpc/include/asm/reg_booke.h 
b/arch/powerpc/include/asm/reg_booke.h
index eb2a33d..06967f1 100644
--- a/arch/powerpc/include/asm/reg_booke.h
+++ b/arch/powerpc/include/asm/reg_booke.h
@@ -46,10 +46,10 @@
 #define MSR_USER32 (MSR_ | MSR_PR | MSR_EE)
 #define MSR_USER64 (MSR_USER32 | MSR_64BIT)
 #elif defined (CONFIG_40x)
-#define MSR_KERNEL (MSR_ME|MSR_RI|MSR_IR|MSR_DR|MSR_CE)
+#define MSR_KERNEL (MSR_ME|MSR_IR|MSR_DR|MSR_CE)
 #define MSR_USER   (MSR_KERNEL|MSR_PR|MSR_EE)
 #else
-#define MSR_KERNEL (MSR_ME|MSR_RI|MSR_CE)
+#define MSR_KERNEL (MSR_ME|MSR_CE)
 #define MSR_USER   (MSR_KERNEL|MSR_PR|MSR_EE)
 #endif
 
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 96f3473..77679a7 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -1359,7 +1359,7 @@ static struct regbit msr_bits[] = {
{MSR_IR,"IR"},
{MSR_DR,"DR"},
{MSR_PMM,   "PMM"},
-#ifndef CONFIG_BOOKE
+#if !defined(CONFIG_BOOKE) && !defined(CONFIG_40x)
{MSR_RI,"RI"},
{MSR_LE,"LE"},
 #endif
diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index 9a86572..2d00696 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -429,10 +429,11 @@ void system_reset_exception(struct pt_regs *regs)
if (get_paca()->in_nmi > 1)
nmi_panic(regs, "Unrecoverable nested System Reset");
 #endif
+#ifdef MSR_RI
/* Must die if the interrupt is not recoverable */
if (!(regs->msr & MSR_RI))
nmi_panic(regs, "Unrecoverable System Reset");
-
+#endif
if (!nested)
nmi_exit();
 
@@ -478,7 +479,9 @@ static inline int check_io_access(struct pt_regs *regs)
printk(KERN_DEBUG "%s bad port %lx at %p\n",
   (*nip & 0x100)? "OUT to": "IN from",
   regs->gpr[rb] - _IO_BASE, nip);
+#ifdef MSR_RI
regs->msr |= MSR_RI;
+#endif
regs->nip = extable_fixup(entry);
return 1;
}
@@ -763,10 +766,11 @@ void machine_check_exception(struct pt_regs *regs)
if (check_io_access(regs))
goto bail;
 
+#ifdef MSR_RI
/* Must die if the interrupt is not recoverable */
if (!(regs->msr & MSR_RI))
nmi_panic(regs, "Unrecoverable Machine check");
-
+#endif
if (!nested)
nmi_exit();
 
diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
index d81568f..c03c453 100644
--- a/arch/powerpc/lib/sstep.c
+++ b/arch/powerpc/lib/sstep.c
@@ -3059,9 +3059,11 @@ int emulate_step(struct pt_regs *regs, unsigned int 
instr)
 
case MTMSR:
val = regs->gpr[op.reg];
+#ifdef MSR_RI
if ((val & MSR_RI) == 0)
/* can't step mtmsr[d] that would clear MSR_RI */
return -1;
+#endif
/* here op.val is the mask of bits to change */
regs->msr = (regs->msr & ~op.val) | (val & op.val);
goto instr_done;

Re: [PATCH] powerpc: Fix bogus usage of MSR_RI on BookE and 40x

Re: [PATCH] powerpc: Fix bogus usage of MSR_RI on BookE and 40x

[PATCH] powerpc: Fix bogus usage of MSR_RI on BookE and 40x

3 matches

Site Navigation

Mail list logo

Footer information