From: "Ghimiray, Himal Prasad" <himal.prasad.ghimiray@intel.com>
To: Riana Tauro <riana.tauro@intel.com>, <intel-xe@lists.freedesktop.org>
Cc: <anshuman.gupta@intel.com>, <rodrigo.vivi@intel.com>,
<aravind.iddamsetty@linux.intel.com>, <badal.nilawar@intel.com>,
<raag.jadav@intel.com>, <ravi.kishore.koppuravuri@intel.com>,
<mallesh.koujalagi@intel.com>, <soham.purkait@intel.com>,
<tejas.upadhyay@intel.com>
Subject: Re: [PATCH v2 3/6] drm/xe: Separate drm-ras netlink data from device and firmware RAS state
Date: Tue, 8 Sep 2026 00:34:40 +0530 [thread overview]
Message-ID: <c6b2e25b-ced3-41a7-b2a0-2029877bf351@intel.com> (raw)
In-Reply-To: <20260907094706.1407436-11-riana.tauro@intel.com>
On 07-09-2026 15:17, Riana Tauro wrote:
> Keep the DRM RAS data used for netlink reporting separate from the
> device and firmware RAS state in xe_device.
>
> Signed-off-by: Riana Tauro <riana.tauro@intel.com>
> ---
> drivers/gpu/drm/xe/xe_device_types.h | 11 +++++++++--
> drivers/gpu/drm/xe/xe_drm_ras.c | 16 ++++++++--------
> drivers/gpu/drm/xe/xe_drm_ras_types.h | 3 ---
> drivers/gpu/drm/xe/xe_hw_error.c | 6 +++---
> drivers/gpu/drm/xe/xe_ras.c | 15 +++++++--------
> drivers/gpu/drm/xe/xe_ras.h | 6 ++++++
> drivers/gpu/drm/xe/xe_ras_types.h | 10 ++++++++++
> drivers/gpu/drm/xe/xe_ttm_vram_mgr.c | 3 ++-
> 8 files changed, 45 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
> index 180d450a6deb..f9dd65d94417 100644
> --- a/drivers/gpu/drm/xe/xe_device_types.h
> +++ b/drivers/gpu/drm/xe/xe_device_types.h
> @@ -21,6 +21,7 @@
> #include "xe_platform_types.h"
> #include "xe_pmu_types.h"
> #include "xe_pt_types.h"
> +#include "xe_ras_types.h"
> #include "xe_sriov_pf_types.h"
> #include "xe_sriov_types.h"
> #include "xe_sriov_vf_types.h"
> @@ -550,8 +551,14 @@ struct xe_device {
> /** @pmu: performance monitoring unit */
> struct xe_pmu pmu;
>
> - /** @ras: RAS structure for device */
> - struct xe_drm_ras ras;
> + /** @ras: RAS (Reliability, Availability, Serviceability) structures */
> + struct {
> + /** @ras.nl_data: drm-ras netlink data */
> + struct xe_drm_ras nl_data;
> +
> + /** @ras.state: RAS device and firmware state */
> + struct xe_ras_state state;
> + } ras;
+1 for this separation. Looks much cleaner
>
> /** @i2c: I2C host controller */
> struct xe_i2c *i2c;
> diff --git a/drivers/gpu/drm/xe/xe_drm_ras.c b/drivers/gpu/drm/xe/xe_drm_ras.c
> index 7f3695707611..38d77561facf 100644
> --- a/drivers/gpu/drm/xe/xe_drm_ras.c
> +++ b/drivers/gpu/drm/xe/xe_drm_ras.c
> @@ -20,7 +20,7 @@ static int query_error_counter(struct xe_device *xe,
> enum drm_xe_ras_error_severity severity,
> u32 error_id, const char **name, u32 *val)
> {
> - struct xe_drm_ras *ras = &xe->ras;
> + struct xe_drm_ras *ras = &xe->ras.nl_data;
> struct xe_drm_ras_counter *info = ras->info[severity];
>
> if (!info || !info[error_id].name)
> @@ -41,7 +41,7 @@ static int clear_error_counter(struct xe_device *xe,
> enum drm_xe_ras_error_severity severity,
> u32 error_id)
> {
> - struct xe_drm_ras *ras = &xe->ras;
> + struct xe_drm_ras *ras = &xe->ras.nl_data;
> struct xe_drm_ras_counter *info = ras->info[severity];
>
> if (!info || !info[error_id].name)
> @@ -90,7 +90,7 @@ static int query_correctable_error_threshold(struct drm_ras_node *ep, u32 error_
> const char **name, u32 *threshold)
> {
> struct xe_device *xe = ep->priv;
> - struct xe_drm_ras *ras = &xe->ras;
> + struct xe_drm_ras *ras = &xe->ras.nl_data;
> struct xe_drm_ras_counter *info = ras->info[DRM_XE_RAS_ERR_SEV_CORRECTABLE];
>
> if (!info || !info[error_id].name)
> @@ -106,7 +106,7 @@ static int query_correctable_error_threshold(struct drm_ras_node *ep, u32 error_
> static int set_correctable_error_threshold(struct drm_ras_node *ep, u32 error_id, u32 threshold)
> {
> struct xe_device *xe = ep->priv;
> - struct xe_drm_ras *ras = &xe->ras;
> + struct xe_drm_ras *ras = &xe->ras.nl_data;
> struct xe_drm_ras_counter *info = ras->info[DRM_XE_RAS_ERR_SEV_CORRECTABLE];
>
> if (!info || !info[error_id].name)
> @@ -142,7 +142,7 @@ static int assign_node_params(struct xe_device *xe, struct drm_ras_node *node,
> const enum drm_xe_ras_error_severity severity)
> {
> struct pci_dev *pdev = to_pci_dev(xe->drm.dev);
> - struct xe_drm_ras *ras = &xe->ras;
> + struct xe_drm_ras *ras = &xe->ras.nl_data;
> const char *device_name;
>
> device_name = kasprintf(GFP_KERNEL, "%04x:%02x:%02x.%d",
> @@ -190,7 +190,7 @@ static void cleanup_node(struct drm_device *drm, void *node)
>
> static int register_nodes(struct xe_device *xe)
> {
> - struct xe_drm_ras *ras = &xe->ras;
> + struct xe_drm_ras *ras = &xe->ras.nl_data;
> struct drm_ras_node *node;
> int i, ret;
>
> @@ -230,7 +230,7 @@ static int register_nodes(struct xe_device *xe)
> */
> void xe_drm_ras_event(struct xe_device *xe, u32 component, u32 severity, u32 value)
> {
> - struct xe_drm_ras *ras = &xe->ras;
> + struct xe_drm_ras *ras = &xe->ras.nl_data;
> struct xe_drm_ras_counter *info = ras->info[severity];
> struct drm_ras_node *node;
> int ret;
> @@ -260,7 +260,7 @@ void xe_drm_ras_event(struct xe_device *xe, u32 component, u32 severity, u32 val
> */
> int xe_drm_ras_init(struct xe_device *xe)
> {
> - struct xe_drm_ras *ras = &xe->ras;
> + struct xe_drm_ras *ras = &xe->ras.nl_data;
> struct drm_ras_node *node;
> int err;
>
> diff --git a/drivers/gpu/drm/xe/xe_drm_ras_types.h b/drivers/gpu/drm/xe/xe_drm_ras_types.h
> index 0be218ba2db7..8d729ad6a264 100644
> --- a/drivers/gpu/drm/xe/xe_drm_ras_types.h
> +++ b/drivers/gpu/drm/xe/xe_drm_ras_types.h
> @@ -43,9 +43,6 @@ struct xe_drm_ras {
>
> /** @info: info array for all types of errors */
> struct xe_drm_ras_counter *info[DRM_XE_RAS_ERR_SEV_MAX];
> -
> - /** @disable_vram_page_offline: cached configfs policy, immutable after init */
> - bool disable_vram_page_offline;
> };
>
> #endif
> diff --git a/drivers/gpu/drm/xe/xe_hw_error.c b/drivers/gpu/drm/xe/xe_hw_error.c
> index 5f2abc9485ff..f53a6b3055de 100644
> --- a/drivers/gpu/drm/xe/xe_hw_error.c
> +++ b/drivers/gpu/drm/xe/xe_hw_error.c
> @@ -240,7 +240,7 @@ static void log_soc_error(struct xe_tile *tile, const char * const *reg_info,
> {
> const char *severity_str = error_severity[severity];
> struct xe_device *xe = tile_to_xe(tile);
> - struct xe_drm_ras *ras = &xe->ras;
> + struct xe_drm_ras *ras = &xe->ras.nl_data;
> struct xe_drm_ras_counter *info = ras->info[severity];
> const char *name;
>
> @@ -260,7 +260,7 @@ static void gt_hw_error_handler(struct xe_tile *tile, const enum hardware_error
> {
> const enum drm_xe_ras_error_severity severity = hw_err_to_severity(hw_err);
> struct xe_device *xe = tile_to_xe(tile);
> - struct xe_drm_ras *ras = &xe->ras;
> + struct xe_drm_ras *ras = &xe->ras.nl_data;
> struct xe_drm_ras_counter *info = ras->info[severity];
> struct xe_mmio *mmio = &tile->mmio;
> unsigned long err_stat = 0;
> @@ -422,7 +422,7 @@ static void hw_error_source_handler(struct xe_tile *tile, const enum hardware_er
> const enum drm_xe_ras_error_severity severity = hw_err_to_severity(hw_err);
> const char *severity_str = error_severity[severity];
> struct xe_device *xe = tile_to_xe(tile);
> - struct xe_drm_ras *ras = &xe->ras;
> + struct xe_drm_ras *ras = &xe->ras.nl_data;
> struct xe_drm_ras_counter *info = ras->info[severity];
> unsigned long flags, err_src;
> u32 err_bit;
> diff --git a/drivers/gpu/drm/xe/xe_ras.c b/drivers/gpu/drm/xe/xe_ras.c
> index 648e46df831d..b5b0c1a3a663 100644
> --- a/drivers/gpu/drm/xe/xe_ras.c
> +++ b/drivers/gpu/drm/xe/xe_ras.c
> @@ -1126,19 +1126,18 @@ void xe_ras_init(struct xe_device *xe)
> {
> int ret;
>
> - /*
> - * TODO: Replace platform check with xe->info.has_disable_vram_page_offline
> - * once the feature flag is plumbed through device info.
> - */
> - if (xe->info.platform == XE_CRESCENTISLAND)
> - xe->ras.disable_vram_page_offline =
> - xe_configfs_get_disable_vram_page_offline(to_pci_dev(xe->drm.dev));
> -
> xe_drm_ras_init(xe);
>
> if (!xe->info.has_sysctrl)
> return;
>
I believe has_sysctrl is currently enabled only for CRI. Is it possible
that future platforms will support sysctrl but not VRAM page offlining?
If so, it would be better to have a separate device-info flag for that,
or at least keep a TODO here.
> + /*
> + * TODO: Replace platform check with xe->info.has_disable_vram_page_offline
> + * once the feature flag is plumbed through device info.
> + */
> + xe->ras.state.disable_page_offline =
> + xe_configfs_get_disable_vram_page_offline(to_pci_dev(xe->drm.dev));
> +
> if (IS_ENABLED(CONFIG_PCIEAER))
> ras_usp_aer_init(xe);
>
> diff --git a/drivers/gpu/drm/xe/xe_ras.h b/drivers/gpu/drm/xe/xe_ras.h
> index 0b8669f28d56..6e65443cd262 100644
> --- a/drivers/gpu/drm/xe/xe_ras.h
> +++ b/drivers/gpu/drm/xe/xe_ras.h
> @@ -7,6 +7,7 @@
> #define _XE_RAS_H_
>
> #include <linux/types.h>
> +#include "xe_device_types.h"
No, please. Let's not bring this dependency here. Move the function
definition to xe_ras.c.
> #include "xe_ras_types.h"
>
> struct xe_device;
> @@ -21,4 +22,9 @@ int xe_ras_set_threshold(struct xe_device *xe, u8 severity, u8 component, u32 th
> void xe_ras_init(struct xe_device *xe);
> enum xe_ras_recovery_action xe_ras_process_errors(struct xe_device *xe);
>
> +static inline bool xe_ras_get_disable_page_offline(struct xe_device *xe)
> +{
> + return xe->ras.state.disable_page_offline;
> +}
> +
> #endif
> diff --git a/drivers/gpu/drm/xe/xe_ras_types.h b/drivers/gpu/drm/xe/xe_ras_types.h
> index 7bfd86c9d135..ca459e785add 100644
> --- a/drivers/gpu/drm/xe/xe_ras_types.h
> +++ b/drivers/gpu/drm/xe/xe_ras_types.h
> @@ -414,4 +414,14 @@ struct xe_ras_set_health_response {
> /** @reserved1: Reserved for future use */
> u32 reserved1[2];
> } __packed;
> +
> +/* Device structures */
> +
> +/**
> + * struct xe_ras_state - RAS device and firmware state
> + */
> +struct xe_ras_state {
> + /** @disable_page_offline: cached configfs policy, immutable after init */
> + bool disable_page_offline;
> +};
> #endif
> diff --git a/drivers/gpu/drm/xe/xe_ttm_vram_mgr.c b/drivers/gpu/drm/xe/xe_ttm_vram_mgr.c
> index 9a514d983e90..2c4722a956a0 100644
> --- a/drivers/gpu/drm/xe/xe_ttm_vram_mgr.c
> +++ b/drivers/gpu/drm/xe/xe_ttm_vram_mgr.c
> @@ -24,6 +24,7 @@
> #include "xe_mmio.h"
> #include "xe_pm.h"
> #include "xe_printk.h"
> +#include "xe_ras.h"
> #include "xe_res_cursor.h"
> #include "xe_ttm_stolen_mgr.h"
> #include "xe_ttm_vram_mgr.h"
> @@ -933,7 +934,7 @@ int xe_ttm_vram_handle_addr_fault(struct xe_device *xe, u64 addr)
> vram_mgr = &vr->ttm;
> mm = &vram_mgr->mm;
>
> - if (xe->ras.disable_vram_page_offline) {
> + if (xe_ras_get_disable_page_offline(xe)) {
> xe_err(xe, "0x%llx is reported as corrupted address by HW\n",
> addr);
> return -EOPNOTSUPP;
next prev parent reply other threads:[~2026-09-07 19:04 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-07 9:47 [PATCH v2 0/6] Add support to handle memory double-bit ecc errors Riana Tauro
2026-09-07 9:47 ` [PATCH v2 1/6] drm/xe/xe_ras: Handle page offline requests for device memory " Riana Tauro
2026-09-07 10:14 ` sashiko-bot
2026-09-07 17:35 ` Ghimiray, Himal Prasad
2026-09-11 6:45 ` Tauro, Riana
2026-09-07 9:47 ` [PATCH v2 2/6] drm/xe/xe_ras: Add support to query page offline queue and list Riana Tauro
2026-09-07 10:00 ` sashiko-bot
2026-09-07 18:25 ` Ghimiray, Himal Prasad
2026-09-11 6:11 ` Tauro, Riana
2026-09-11 6:39 ` Ghimiray, Himal Prasad
2026-09-07 9:47 ` [PATCH v2 3/6] drm/xe: Separate drm-ras netlink data from device and firmware RAS state Riana Tauro
2026-09-07 19:04 ` Ghimiray, Himal Prasad [this message]
2026-09-07 19:21 ` Ghimiray, Himal Prasad
2026-09-09 5:43 ` Tauro, Riana
2026-09-07 9:47 ` [PATCH v2 4/6] drm/xe/xe_ras: Add function to get maximum pages firmware can store Riana Tauro
2026-09-07 19:21 ` Ghimiray, Himal Prasad
2026-09-07 19:24 ` Ghimiray, Himal Prasad
2026-09-09 5:23 ` Tauro, Riana
2026-09-07 9:47 ` [PATCH v2 5/6] drm/xe/xe_ttm_vram: Report max_pages reported by firmware in debugfs Riana Tauro
2026-09-07 19:26 ` Ghimiray, Himal Prasad
2026-09-07 9:47 ` [PATCH v2 6/6] drm/xe/xe_ras: Track offlined pages by firmware to avoid duplicates Riana Tauro
2026-09-07 10:02 ` sashiko-bot
2026-09-07 11:01 ` ✓ CI.KUnit: success for Add support to handle memory double-bit ecc errors (rev2) Patchwork
2026-09-07 11:45 ` ✓ Xe.CI.BAT: " Patchwork
2026-09-07 13:41 ` ✓ Xe.CI.FULL: " Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=c6b2e25b-ced3-41a7-b2a0-2029877bf351@intel.com \
--to=himal.prasad.ghimiray@intel.com \
--cc=anshuman.gupta@intel.com \
--cc=aravind.iddamsetty@linux.intel.com \
--cc=badal.nilawar@intel.com \
--cc=intel-xe@lists.freedesktop.org \
--cc=mallesh.koujalagi@intel.com \
--cc=raag.jadav@intel.com \
--cc=ravi.kishore.koppuravuri@intel.com \
--cc=riana.tauro@intel.com \
--cc=rodrigo.vivi@intel.com \
--cc=soham.purkait@intel.com \
--cc=tejas.upadhyay@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.