Re: [PATCH v3 02/12] drm/xe/cper: Retrieve the error counter record for CPER reporting

[Raag Jadav <raag.jadav@intel.com>]

On Tue, Sep 08, 2026 at 12:16:27PM +0200, Raag Jadav wrote:
> On Sun, Sep 06, 2026 at 10:56:07PM +0530, Badal Nilawar wrote:
> > Retrieve error counter record required to build a CPER record when
> > one not provided by the caller.
> > 
> > Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> > ---
> >  drivers/gpu/drm/xe/xe_cper.c |  15 ++++++
> >  drivers/gpu/drm/xe/xe_ras.c  | 100 ++++++++++++++++++++++-------------
> >  drivers/gpu/drm/xe/xe_ras.h  |   3 ++
> >  3 files changed, 80 insertions(+), 38 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_cper.c b/drivers/gpu/drm/xe/xe_cper.c
> > index e8017e3ee3a0..f04a91223a43 100644
> > --- a/drivers/gpu/drm/xe/xe_cper.c
> > +++ b/drivers/gpu/drm/xe/xe_cper.c
> > @@ -9,6 +9,8 @@
> >  
> >  #include "xe_cper.h"
> >  #include "xe_device.h"
> > +#include "xe_printk.h"
> > +#include "xe_ras.h"
> >  #include "xe_ras_types.h"
> >  
> >  /**
> > @@ -26,6 +28,8 @@ void xe_emit_hardware_error_cper(struct pci_dev *pdev, int cper_sev, enum xe_sig
> >  				 struct xe_ras_get_counter_response *response)
> >  {
> >  	struct xe_device *xe = pdev_to_xe_device(pdev);
> > +	struct xe_ras_get_counter_response local_resp = {};
> > +	struct xe_ras_get_counter_response *counter_response = response;
> >  
> >  	if (!xe)
> >  		return;
> > @@ -33,5 +37,16 @@ void xe_emit_hardware_error_cper(struct pci_dev *pdev, int cper_sev, enum xe_sig
> >  	if ((int)sigid >= INTEL_SIGID_GPU_XE_HARDWARE_START)
> >  		return;
> >  
> > +	if (!counter || !xe_ras_counter_is_valid(xe, counter))
> 
> Why? Should we even be at this point without a valid counter?

ras_counter_is_valid() is already part of get_counter() and !counter
should be covered by IS_ERR_OR_NULL() in patch 1, which makes the entire
condition redundant.

> > +		return;
> > +
> > +	if (!counter_response) {
> > +		counter_response = &local_resp;
> > +		if (xe_ras_get_counter_response(xe, counter, counter_response)) {
> 
> Dumb question, shouldn't the whole xe_emit_hardware_error_cper() be the
> consequence of get_counter() and not the other way around?

Thanks for the offline pointers. From what I understand this is diverging
from the original hw cper design and correct for whatever we have in place.

Raag

> > +			xe_err(xe, "[RAS]: CPER: failed to get counter, skipping record\n");
> > +			return;
> > +		}
> > +	}
> > +
> >  	/* TODO */
> >  }