Re: [PATCH] powernv/opal: Handle OPAL_WRONG_STATE error from OPAL fails

Vipin K Parashar [off-list ref] writes:

On Monday 13 February 2017 06:13 AM, Michael Ellerman wrote:

quoted

Vipin K Parashar [off-list ref] writes:

quoted

OPAL returns OPAL_WRONG_STATE for XSCOM operations

done to read any core FIR which is sleeping, offline.

OK.

Do we know why Linux is causing that to happen?

This issue is originally seen upon running STAF (Software Test
Automation Framework) stress tests and off-lining some cores
with stress tests running.

It can also be re-created after off-lining few cores and following
one of below methods.
1. Executing Linux "sensors" command
2. Reading contents of file /sys/class/hwmon/hwmon0/tempX_input,
    where X is offline CPU.

Its "opal_get_sensor_data" Linux API that that triggers
OPAL call "opal_sensor_read", performing XSCOM ops here.
If core is found sleeping/offline Linux throws up
"opal_error_code: Unexpected OPAL error" error onto console.

Currently Linux isn't aware about OPAL_WRONG_STATE return code
from OPAL. Thus it prints "Unexpected OPAL error" message, same
as it would log for any unknown OPAL return codes.

Seeing this error over console has been a concern for Test and
would puzzle real user as well. This patch makes Linux aware about
OPAL_WRONG_STATE return code from OPAL and stops printing
"Unexpected OPAL error" message onto console for OPAL fails
with OPAL_WRONG_STATE

Ahh... so this is a DTS sensor, which indeed is just XSCOMs and we
return the xscom_read return code in event of error.

I would argue that converting to EIO in that instance is probably
correct... or EAGAIN? EAGAIN may be more correct in the situation where
the core is just sleeping.

What kind of offlining are you doing?

Arguably, the correct behaviour would be to remove said sensors when the
core is offline.

quoted

It's also returned from many of the XIVE routines if we're in the wrong
xive mode, all of which would indicate a fairly bad Linux bug.

Also the skiboot patch which added WRONG_STATE for XSCOM ops did so
explicitly so we could differentiate from other errors:

     commit 9c2d82394fd2303847cac4a665dee62556ca528a
     Author:     Russell Currey [off-list ref]
     AuthorDate: Mon Mar 21 12:00:00 2016 +1100

     xscom: Return OPAL_WRONG_STATE on XSCOM ops if CPU is asleep
     
     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 [off-list ref]
     Reviewed-by: Alistair Popple [off-list ref]
     Signed-off-by: Stewart Smith [off-list ref]



So I'm still not convinced that quietly swallowing this error and
mapping it to -EIO along with several of the other error codes is the
right thing to do.

How about returning -ENXIO upon receiving OPAL_WRONG_STATE ?

I think ENXIO would be incorrect, as it is "No such device or address"
when that almost certainly isn't the case. It's a solvable error -
there's just some XSCOMS you can't do at certain times.


-- 
Stewart Smith
OPAL Architect, IBM.