diff mbox series

[3/3] xscom: Don't create PELs for non-severe read/write failures

Message ID 1575465990-25353-4-git-send-email-ego@linux.vnet.ibm.com
State Superseded
Headers show
Series xscom: Don't create PELs for non-severe read/write failures | expand

Checks

Context Check Description
snowpatch_ozlabs/apply_patch success Successfully applied on branch master (d75e82dbfbb9443efeb3f9a5921ac23605aab469)
snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot success Test snowpatch/job/snowpatch-skiboot on branch master
snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot-dco success Signed-off-by present

Commit Message

Gautham R Shenoy Dec. 4, 2019, 1:26 p.m. UTC 
From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>

Certain xscom read/write calls initiated by the userspace (either via
HBRT or xscom-utils) can fail with an incorrect-address error code due
to known hardware errors. These failures are not severe, in that they
do not cause the machine to checkstop and hence, elogs need not be
created for them.

This patch defines new function calls named opal_xscom_read() and
opal_xscom_write() to map to OPAL_XSCOM_READ and OPAL_XSCOM_WRITE
calls respectively, so that we can distinguish these calls from the
calls to perform xscom-read/writes initiated within OPAL.

Further, the patch ensures that we do not create PELs for
opal_xscom_read()/write() failures due to incorrect address.

Signed-off-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
---
 hw/xscom.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

Comments

Klaus Heinrich Kiwi Dec. 4, 2019, 11 p.m. UTC | #1 
On 12/4/2019 10:26 AM, Gautham R. Shenoy wrote
> -opal_call(OPAL_XSCOM_READ, xscom_read, 3);
> +static int opal_xscom_read(uint32_t partid, uint64_t pcb_addr, uint64_t *val)
> +{
> +	return _xscom_read(partid, pcb_addr, val, true, false);
> +}
> +opal_call(OPAL_XSCOM_READ, opal_xscom_read, 3);
>
>   int _xscom_write(uint32_t partid, uint64_t pcb_addr, uint64_t val, bool take_lock,
>   		 bool create_addr_elog)
> @@ -708,7 +712,12 @@ int _xscom_write(uint32_t partid, uint64_t pcb_addr, uint64_t val, bool take_loc
>   	}
>   	return rc;
>   }
> -opal_call(OPAL_XSCOM_WRITE, xscom_write, 3);
> +
> +static int opal_xscom_write(uint32_t partid, uint64_t pcb_addr, uint64_t val)
> +{
> +	return _xscom_write(partid, pcb_addr, val, true, false);
> +}
> +opal_call(OPAL_XSCOM_WRITE, opal_xscom_write, 3);
>
>   /*
>    * Perform a xscom read-modify-write.

if external calls will always suppress the error logs 
(create_addr_elog=false), and only internal calls will have the option 
to do the logging or not, wouldn't it be better to just create 
_fail/nofail versions of _xscom_read/write()?

you can use the _fail() versions when you want the failure to be logged, 
and the _nofail versions for when you don't want it to be logged, and 
for the default case..


  -Klaus
Gautham R Shenoy Dec. 5, 2019, 5:10 a.m. UTC | #2 
Hello Klaus,

On Wed, Dec 04, 2019 at 08:00:07PM -0300, Klaus Heinrich Kiwi wrote:
> On 12/4/2019 10:26 AM, Gautham R. Shenoy wrote
> >-opal_call(OPAL_XSCOM_READ, xscom_read, 3);
> >+static int opal_xscom_read(uint32_t partid, uint64_t pcb_addr, uint64_t *val)
> >+{
> >+	return _xscom_read(partid, pcb_addr, val, true, false);
> >+}
> >+opal_call(OPAL_XSCOM_READ, opal_xscom_read, 3);
> >
> >  int _xscom_write(uint32_t partid, uint64_t pcb_addr, uint64_t val, bool take_lock,
> >  		 bool create_addr_elog)
> >@@ -708,7 +712,12 @@ int _xscom_write(uint32_t partid, uint64_t pcb_addr, uint64_t val, bool take_loc
> >  	}
> >  	return rc;
> >  }
> >-opal_call(OPAL_XSCOM_WRITE, xscom_write, 3);
> >+
> >+static int opal_xscom_write(uint32_t partid, uint64_t pcb_addr, uint64_t val)
> >+{
> >+	return _xscom_write(partid, pcb_addr, val, true, false);
> >+}
> >+opal_call(OPAL_XSCOM_WRITE, opal_xscom_write, 3);
> >
> >  /*
> >   * Perform a xscom read-modify-write.
> 
> if external calls will always suppress the error logs
> (create_addr_elog=false), and only internal calls will have the option to do
> the logging or not, wouldn't it be better to just create _fail/nofail
> versions of _xscom_read/write()?

Yes, that seems simpler. I too thought of it, only after sending the
patch.

Will resend a version today with the _xscom_read_nofail() version.

> 
> you can use the _fail() versions when you want the failure to be logged, and
> the _nofail versions for when you don't want it to be logged, and for the
> default case..
> 
> 
>  -Klaus
> 
> -- 
> Klaus Heinrich Kiwi <klaus@linux.vnet.ibm.com>
>
diff mbox series

Patch

diff --git a/hw/xscom.c b/hw/xscom.c
index e8cdabc..6da0d8c 100644
--- a/hw/xscom.c
+++ b/hw/xscom.c
@@ -659,7 +659,11 @@  int _xscom_read(uint32_t partid, uint64_t pcb_addr, uint64_t *val, bool take_loc
 	return rc;
 }
 
-opal_call(OPAL_XSCOM_READ, xscom_read, 3);
+static int opal_xscom_read(uint32_t partid, uint64_t pcb_addr, uint64_t *val)
+{
+	return _xscom_read(partid, pcb_addr, val, true, false);
+}
+opal_call(OPAL_XSCOM_READ, opal_xscom_read, 3);
 
 int _xscom_write(uint32_t partid, uint64_t pcb_addr, uint64_t val, bool take_lock,
 		 bool create_addr_elog)
@@ -708,7 +712,12 @@  int _xscom_write(uint32_t partid, uint64_t pcb_addr, uint64_t val, bool take_loc
 	}
 	return rc;
 }
-opal_call(OPAL_XSCOM_WRITE, xscom_write, 3);
+
+static int opal_xscom_write(uint32_t partid, uint64_t pcb_addr, uint64_t val)
+{
+	return _xscom_write(partid, pcb_addr, val, true, false);
+}
+opal_call(OPAL_XSCOM_WRITE, opal_xscom_write, 3);
 
 /*
  * Perform a xscom read-modify-write.