Re: [PATCH v1] drm/xe/ras: Fix invalid health error code

On Tue, Aug 04, 2026 at 02:01:16PM +0530, Tauro, Riana wrote:
> On 04-08-2026 13:37, Raag Jadav wrote:
> > On Tue, Aug 04, 2026 at 08:56:12AM +0200, Michal Wajdeczko wrote:
> > > On 8/4/2026 8:36 AM, Tauro, Riana wrote:
> > > > On 04-08-2026 10:42, Raag Jadav wrote:
> > > > > We use -EBADMSG for corrupted responses from sysctrl. Fix the error code.
> > > > Let's keep it -EIO as it's more appropriate for sysctrl errors and consistent than
> > > > 
> > > > #define    EBADMSG        74    /* Not a data message */
> > The response size is valid, it's just the contents are corrupted. Similar
> > to what we did with commit 948eee331bef ("drm/xe/ras: Validate sysctrl
> > response") and more consistent here.
> > 
> > > btw, inside GuC CTB code we use
> > > 
> > > 	-EPIPE		// for corrupted message/channel
> > > 	-EPROTO		// for unexpected content in the message
> > We have a bunch of these as a response status, check ras_status_to_errno().
> > The one proposed here is for the values deemed acceptable as per spec.
> > 
> > > as -EIO is too generic
> > Agree.
> 
> Using -EIO consistently for system controller errors allows easier debugging
> for
> sysctrl commands.  When any sysctrl ops fail we know it's not RAS layer.

We still log it as a sysctrl error, so nothing changed from debugging POV
here. Besides, error codes are what make the failure cases distinguishable
to the consumer, otherwise we wouldn't have so many of them :)

> The only other error code we see is directly from firmware status.

I'm all for trusting what the firmware tells me, but this has caused more
problems than solved historically :)

> This is my thought. I will leave it upto you

Sure.

Raag

> > > > > Fixes: 53a7115f9862 ("drm/xe/xe_ras: Add RAS GPU health indicator")
> > > > > Signed-off-by: Raag Jadav <[email protected]>
> > > > > ---
> > > > >    drivers/gpu/drm/xe/xe_ras.c | 4 ++--
> > > > >    1 file changed, 2 insertions(+), 2 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/xe/xe_ras.c b/drivers/gpu/drm/xe/xe_ras.c
> > > > > index f7af4da8172c..bdad0621ae12 100644
> > > > > --- a/drivers/gpu/drm/xe/xe_ras.c
> > > > > +++ b/drivers/gpu/drm/xe/xe_ras.c
> > > > > @@ -700,7 +700,7 @@ static ssize_t gpu_health_show(struct device *dev, struct device_attribute *attr
> > > > >        if (response.health >= XE_RAS_HEALTH_MAX) {
> > > > >            xe_err(xe, "sysctrl: invalid health state %u\n",
> > > > >                   response.health);
> > > > > -        return -EIO;
> > > > > +        return -EBADMSG;
> > > > >        }
> > > > >          health = gpu_health_states[response.health];
> > > > > @@ -753,7 +753,7 @@ static ssize_t gpu_health_store(struct device *dev, struct device_attribute *att
> > > > >        if (response.health >= XE_RAS_HEALTH_MAX) {
> > > > >            xe_err(xe, "sysctrl: invalid health state %u\n",
> > > > >                   response.health);
> > > > > -        return -EIO;
> > > > > +        return -EBADMSG;
> > > > >        }
> > > > >          health = gpu_health_states[response.health];