[Skiboot] [PATCH 1/7] xscom: Return OPAL_WRONG_STATE on XSCOM ops if CPU is asleep

Russell Currey ruscur at russell.cc
Thu Mar 17 16:02:53 AEDT 2016


On Thu, 2016-03-17 at 15:58 +1100, Alistair Popple wrote:
> On Tue, 15 Mar 2016 18:33:51 Russell Currey wrote:
> > 
> > xscom_read and xscom_write return OPAL_SUCCESS if they worked, and
> > OPAL_HARDWARE if they didn't.  This doesn't provide information about
> > why
> > the operation failed, such as if the CPU happens to be asleep.
> > 
> > This is specifically useful in error scanning, so if every CPU is being
> > scanned for errors, sleeping CPUs likely aren't the cause of failures.
> > 
> > So, return OPAL_WRONG_STATE in xscom_read and xscom_write if the CPU is
> > sleeping.
> > 
> > Signed-off-by: Russell Currey <ruscur at russell.cc>
> > ---
> > This will potentially add some new functionality to the kernel.  I'm
> > not
> > sure how that might be useful, but I don't think it can be harmful.
> I assume the kernel only checks for rc != 0 such that this minor OPAL
> API 
> change doesn't actually result in any behaviour change for current and 
> previous kernels?

Yeah, it ends up in opal_xscom_err_xlate, which treats non-zero as -EIO.
If something ever needed to know the CPU was asleep that could be changed
in future to return something different.

> 
> Reviewed-by: Alistair Popple <alistair at popple.id.au>
> 
> > 
> > ---
> >  doc/opal-api/opal-xscom-read-write-65-66.txt |  5 +++++
> >  hw/xscom.c                                   | 29 
> +++++++++++++++++-----------
> > 
> >  2 files changed, 23 insertions(+), 11 deletions(-)
> > 
> > diff --git a/doc/opal-api/opal-xscom-read-write-65-66.txt b/doc/opal-
> api/opal-xscom-read-write-65-66.txt
> > 
> > index 92916df..4ed0134 100644
> > --- a/doc/opal-api/opal-xscom-read-write-65-66.txt
> > +++ b/doc/opal-api/opal-xscom-read-write-65-66.txt
> > @@ -10,3 +10,8 @@ each takes three parameters:
> >  
> >  int xscom_read(uint32_t partid, uint64_t pcb_addr, uint64_t *val)
> >  int xscom_write(uint32_t partid, uint64_t pcb_addr, uint64_t val)
> > +
> > +Returns:
> > +	OPAL_SUCCESS
> > +	OPAL_HARDWARE if operation failed
> > +	OPAL_WRONG_STATE if CPU is asleep
> > diff --git a/hw/xscom.c b/hw/xscom.c
> > index a7a1705..1c45cbc 100644
> > --- a/hw/xscom.c
> > +++ b/hw/xscom.c
> > @@ -118,7 +118,7 @@ static void xscom_reset(uint32_t gcid)
> >  	 */
> >  }
> >  
> > -static bool xscom_handle_error(uint64_t hmer, uint32_t gcid, uint32_t 
> pcb_addr,
> > 
> > +static int xscom_handle_error(uint64_t hmer, uint32_t gcid, uint32_t 
> pcb_addr,
> > 
> >  			       bool is_write)
> >  {
> >  	unsigned int stat = GETFIELD(SPR_HMER_XSCOM_STATUS, hmer);
> > @@ -129,7 +129,10 @@ static bool xscom_handle_error(uint64_t hmer,
> > uint32_t 
> gcid, uint32_t pcb_addr,
> > 
> >  	switch(stat) {
> >  	/* XSCOM blocked, just retry */
> >  	case 1:
> > -		return true;
> > +		return OPAL_BUSY;
> > +	/* CPU is asleep, don't retry */
> > +	case 2:
> > +		return OPAL_WRONG_STATE;
> >  	}
> >  
> >  	/* XXX: Create error log entry ? */
> > @@ -141,7 +144,7 @@ static bool xscom_handle_error(uint64_t hmer,
> > uint32_t 
> gcid, uint32_t pcb_addr,
> > 
> >  	xscom_reset(gcid);
> >  
> >  	/* Non recovered ... just fail */
> > -	return false;
> > +	return OPAL_HARDWARE;
> >  }
> >  
> >  static void xscom_handle_ind_error(uint64_t data, uint32_t gcid,
> > @@ -175,6 +178,7 @@ static bool xscom_gcid_ok(uint32_t gcid)
> >  static int __xscom_read(uint32_t gcid, uint32_t pcb_addr, uint64_t
> > *val)
> >  {
> >  	uint64_t hmer;
> > +	int64_t ret;
> >  
> >  	if (!xscom_gcid_ok(gcid)) {
> >  		prerror("%s: invalid XSCOM gcid 0x%x\n", __func__,
> > gcid);
> > @@ -197,16 +201,18 @@ static int __xscom_read(uint32_t gcid, uint32_t 
> pcb_addr, uint64_t *val)
> > 
> >  		if (!(hmer & SPR_HMER_XSCOM_FAIL))
> >  			break;
> >  
> > -		/* Handle error and eventually retry */
> > -		if (!xscom_handle_error(hmer, gcid, pcb_addr, false))
> > -			return OPAL_HARDWARE;
> > +		/* Handle error and possibly eventually retry */
> > +		ret = xscom_handle_error(hmer, gcid, pcb_addr, false);
> > +		if (ret == OPAL_HARDWARE || ret == OPAL_WRONG_STATE)
> > +			return ret;
> >  	}
> > -	return 0;
> > +	return OPAL_SUCCESS;
> >  }
> >  
> >  static int __xscom_write(uint32_t gcid, uint32_t pcb_addr, uint64_t
> > val)
> >  {
> >  	uint64_t hmer;
> > +	int64_t ret;
> >  
> >  	if (!xscom_gcid_ok(gcid)) {
> >  		prerror("%s: invalid XSCOM gcid 0x%x\n", __func__,
> > gcid);
> > @@ -229,11 +235,12 @@ static int __xscom_write(uint32_t gcid, uint32_t 
> pcb_addr, uint64_t val)
> > 
> >  		if (!(hmer & SPR_HMER_XSCOM_FAIL))
> >  			break;
> >  
> > -		/* Handle error and eventually retry */
> > -		if (!xscom_handle_error(hmer, gcid, pcb_addr, true))
> > -			return OPAL_HARDWARE;
> > +		/* Handle error and possibly eventually retry */
> > +		ret = xscom_handle_error(hmer, gcid, pcb_addr, true);
> > +		if (ret == OPAL_HARDWARE || ret == OPAL_WRONG_STATE)
> > +			return ret;
> >  	}
> > -	return 0;
> > +	return OPAL_SUCCESS;
> >  }
> >  
> >  /*
> > 
> 



More information about the Skiboot mailing list