* [PATCH] drm: xe: select configfs @ 2026-10-26 10:25 Arnd Bergmann 2026-11-11 19:01 ` Rodrigo Vivi 0 siblings, 1 reply; 6+ messages in thread From: Arnd Bergmann @ 2026-10-26 10:25 UTC (permalink / raw) To: Matthew Brost, Thomas Hellström, Rodrigo Vivi, David Airlie, Simona Vetter, Matt Roper, Lucas De Marchi, Riana Tauro Cc: Arnd Bergmann, Matthew Auld, Shuicheng Lin, Armin Wolf, Zhanjun Dong, Michal Wajdeczko, Piotr Piórkowski, Tejas Upadhyay, intel-xe, dri-devel, linux-kernel From: Arnd Bergmann <arnd@arndb.de> The original addition of configfs to xe missed a compile-time dependency, which leads to a link failure when configfs is enabled as a loadable module: ld.lld-21: error: undefined symbol: xe_configfs_get_survivability_mode >>> referenced by xe_survivability_mode.c:224 >>> drivers/gpu/drm/xe/xe_survivability_mode.o:(xe_survivability_mode_is_requested) in archive vmlinux.a A recent workaround added a 'IS_REACHABLE()' check around the calls into configfs, which makes it link in this configuration, but this is generally a bad idea because it makes the actual behavior inconsistent and confusing to users that expect configfs to work if both xe and configfs are enabled. Use 'select CONFIGFS_FS' instead, like every other configfs user does, and remove the unnecessary stub functions. There is little practical difference here as anyone using XE graphics is likely to have a complex configuration that already selects configfs anyway, and the link error or the IS_REACHABLE() would only manifest during randconfig testing. Fixes: 30499cbd7961 ("drm/xe: Fix build with DRM_XE=y and CONFIGFS_FS=m") Fixes: 16280ded45fb ("drm/xe: Add configfs to enable survivability mode") Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- drivers/gpu/drm/xe/Kconfig | 1 + drivers/gpu/drm/xe/xe_configfs.h | 30 ------------------------------ 2 files changed, 1 insertion(+), 30 deletions(-) diff --git a/drivers/gpu/drm/xe/Kconfig b/drivers/gpu/drm/xe/Kconfig index 4fc8abdc5cb6..b52a8dc28f97 100644 --- a/drivers/gpu/drm/xe/Kconfig +++ b/drivers/gpu/drm/xe/Kconfig @@ -11,6 +11,7 @@ config DRM_XE # the shmem_readpage() which depends upon tmpfs select SHMEM select TMPFS + select CONFIGFS_FS select DRM_BUDDY select DRM_CLIENT_SELECTION select DRM_KMS_HELPER diff --git a/drivers/gpu/drm/xe/xe_configfs.h b/drivers/gpu/drm/xe/xe_configfs.h index 327b648944ff..d726800de26a 100644 --- a/drivers/gpu/drm/xe/xe_configfs.h +++ b/drivers/gpu/drm/xe/xe_configfs.h @@ -14,7 +14,6 @@ struct pci_dev; -#if IS_REACHABLE(CONFIG_CONFIGFS_FS) int xe_configfs_init(void); void xe_configfs_exit(void); void xe_configfs_check_device(struct pci_dev *pdev); @@ -36,34 +35,5 @@ u32 xe_configfs_get_ctx_restore_post_bb(struct pci_dev *pdev, unsigned int xe_configfs_get_max_vfs(struct pci_dev *pdev); bool xe_configfs_admin_only_pf(struct pci_dev *pdev); #endif -#else -static inline int xe_configfs_init(void) { return 0; } -static inline void xe_configfs_exit(void) { } -static inline void xe_configfs_check_device(struct pci_dev *pdev) { } -static inline bool xe_configfs_get_survivability_mode(struct pci_dev *pdev) { return false; } -static inline bool xe_configfs_primary_gt_allowed(struct pci_dev *pdev) { return true; } -static inline bool xe_configfs_media_gt_allowed(struct pci_dev *pdev) { return true; } -static inline u64 xe_configfs_get_engines_allowed(struct pci_dev *pdev) { return U64_MAX; } -static inline bool xe_configfs_get_psmi_enabled(struct pci_dev *pdev) { return false; } -static inline bool xe_configfs_get_enable_multi_queue(struct pci_dev *pdev) { return true; } -static inline u32 xe_configfs_get_migrate_ulls_period_ms(struct pci_dev *pdev) { return 5; } -static inline bool xe_configfs_get_disable_vram_page_offline(struct pci_dev *pdev) { return false; } -static inline u32 xe_configfs_get_ctx_restore_mid_bb(struct pci_dev *pdev, - enum xe_engine_class class, - const u32 **cs) { return 0; } -static inline u32 xe_configfs_get_ctx_restore_post_bb(struct pci_dev *pdev, - enum xe_engine_class class, - const u32 **cs) { return 0; } -#ifdef CONFIG_PCI_IOV -static inline unsigned int xe_configfs_get_max_vfs(struct pci_dev *pdev) -{ - return xe_modparam.max_vfs; -} -static inline bool xe_configfs_admin_only_pf(struct pci_dev *pdev) -{ - return XE_DEFAULT_ADMIN_ONLY_PF; -} -#endif -#endif #endif -- 2.53.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm: xe: select configfs 2026-10-26 10:25 [PATCH] drm: xe: select configfs Arnd Bergmann @ 2026-11-11 19:01 ` Rodrigo Vivi 2026-11-13 16:22 ` Matt Roper 0 siblings, 1 reply; 6+ messages in thread From: Rodrigo Vivi @ 2026-11-11 19:01 UTC (permalink / raw) To: Arnd Bergmann Cc: Matthew Brost, Thomas Hellström, David Airlie, Simona Vetter, Matt Roper, Lucas De Marchi, Riana Tauro, Arnd Bergmann, Matthew Auld, Shuicheng Lin, Armin Wolf, Zhanjun Dong, Michal Wajdeczko, Piotr Piórkowski, Tejas Upadhyay, intel-xe, dri-devel, linux-kernel On Mon, Oct 26, 2026 at 11:25:50AM +0100, Arnd Bergmann wrote: > From: Arnd Bergmann <arnd@arndb.de> > > The original addition of configfs to xe missed a compile-time dependency, > which leads to a link failure when configfs is enabled as a loadable module: > > ld.lld-21: error: undefined symbol: xe_configfs_get_survivability_mode > >>> referenced by xe_survivability_mode.c:224 > >>> drivers/gpu/drm/xe/xe_survivability_mode.o:(xe_survivability_mode_is_requested) in archive vmlinux.a > > A recent workaround added a 'IS_REACHABLE()' check around the calls into > configfs, which makes it link in this configuration, but this is generally > a bad idea because it makes the actual behavior inconsistent and confusing > to users that expect configfs to work if both xe and configfs are enabled. > > Use 'select CONFIGFS_FS' instead, like every other configfs user does, > and remove the unnecessary stub functions. There is little practical > difference here as anyone using XE graphics is likely to have a complex > configuration that already selects configfs anyway, and the link error > or the IS_REACHABLE() would only manifest during randconfig testing. > > Fixes: 30499cbd7961 ("drm/xe: Fix build with DRM_XE=y and CONFIGFS_FS=m") > Fixes: 16280ded45fb ("drm/xe: Add configfs to enable survivability mode") > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > drivers/gpu/drm/xe/Kconfig | 1 + > drivers/gpu/drm/xe/xe_configfs.h | 30 ------------------------------ > 2 files changed, 1 insertion(+), 30 deletions(-) > > diff --git a/drivers/gpu/drm/xe/Kconfig b/drivers/gpu/drm/xe/Kconfig > index 4fc8abdc5cb6..b52a8dc28f97 100644 > --- a/drivers/gpu/drm/xe/Kconfig > +++ b/drivers/gpu/drm/xe/Kconfig > @@ -11,6 +11,7 @@ config DRM_XE > # the shmem_readpage() which depends upon tmpfs > select SHMEM > select TMPFS > + select CONFIGFS_FS Perhaps another option would be what Jani had suggested: config DRM_XE_CONFIGFS def_bool y depends on DRM_XE && CONFIGFS_FS depends on CONFIGFS_FS=y || DRM_XE=m But anyway, we are depending more and more on the configfs for production choices. So, let's go with you select and close this for good. I'm pushing this to drm-xe-next. Thanks for the patch. > select DRM_BUDDY > select DRM_CLIENT_SELECTION > select DRM_KMS_HELPER > diff --git a/drivers/gpu/drm/xe/xe_configfs.h b/drivers/gpu/drm/xe/xe_configfs.h > index 327b648944ff..d726800de26a 100644 > --- a/drivers/gpu/drm/xe/xe_configfs.h > +++ b/drivers/gpu/drm/xe/xe_configfs.h > @@ -14,7 +14,6 @@ > > struct pci_dev; > > -#if IS_REACHABLE(CONFIG_CONFIGFS_FS) > int xe_configfs_init(void); > void xe_configfs_exit(void); > void xe_configfs_check_device(struct pci_dev *pdev); > @@ -36,34 +35,5 @@ u32 xe_configfs_get_ctx_restore_post_bb(struct pci_dev *pdev, > unsigned int xe_configfs_get_max_vfs(struct pci_dev *pdev); > bool xe_configfs_admin_only_pf(struct pci_dev *pdev); > #endif > -#else > -static inline int xe_configfs_init(void) { return 0; } > -static inline void xe_configfs_exit(void) { } > -static inline void xe_configfs_check_device(struct pci_dev *pdev) { } > -static inline bool xe_configfs_get_survivability_mode(struct pci_dev *pdev) { return false; } > -static inline bool xe_configfs_primary_gt_allowed(struct pci_dev *pdev) { return true; } > -static inline bool xe_configfs_media_gt_allowed(struct pci_dev *pdev) { return true; } > -static inline u64 xe_configfs_get_engines_allowed(struct pci_dev *pdev) { return U64_MAX; } > -static inline bool xe_configfs_get_psmi_enabled(struct pci_dev *pdev) { return false; } > -static inline bool xe_configfs_get_enable_multi_queue(struct pci_dev *pdev) { return true; } > -static inline u32 xe_configfs_get_migrate_ulls_period_ms(struct pci_dev *pdev) { return 5; } > -static inline bool xe_configfs_get_disable_vram_page_offline(struct pci_dev *pdev) { return false; } > -static inline u32 xe_configfs_get_ctx_restore_mid_bb(struct pci_dev *pdev, > - enum xe_engine_class class, > - const u32 **cs) { return 0; } > -static inline u32 xe_configfs_get_ctx_restore_post_bb(struct pci_dev *pdev, > - enum xe_engine_class class, > - const u32 **cs) { return 0; } > -#ifdef CONFIG_PCI_IOV > -static inline unsigned int xe_configfs_get_max_vfs(struct pci_dev *pdev) > -{ > - return xe_modparam.max_vfs; > -} > -static inline bool xe_configfs_admin_only_pf(struct pci_dev *pdev) > -{ > - return XE_DEFAULT_ADMIN_ONLY_PF; > -} > -#endif > -#endif > > #endif > -- > 2.53.0 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm: xe: select configfs 2026-11-11 19:01 ` Rodrigo Vivi @ 2026-11-13 16:22 ` Matt Roper 2026-11-13 17:51 ` Raag Jadav 2026-11-13 17:58 ` Rodrigo Vivi 0 siblings, 2 replies; 6+ messages in thread From: Matt Roper @ 2026-11-13 16:22 UTC (permalink / raw) To: Rodrigo Vivi Cc: Arnd Bergmann, Matthew Brost, Thomas Hellström, David Airlie, Simona Vetter, Lucas De Marchi, Riana Tauro, Arnd Bergmann, Matthew Auld, Shuicheng Lin, Armin Wolf, Zhanjun Dong, Michal Wajdeczko, Piotr Piórkowski, Tejas Upadhyay, intel-xe, dri-devel, linux-kernel On Wed, Nov 11, 2026 at 02:01:45PM -0500, Rodrigo Vivi wrote: > On Mon, Oct 26, 2026 at 11:25:50AM +0100, Arnd Bergmann wrote: > > From: Arnd Bergmann <arnd@arndb.de> > > > > The original addition of configfs to xe missed a compile-time dependency, > > which leads to a link failure when configfs is enabled as a loadable module: > > > > ld.lld-21: error: undefined symbol: xe_configfs_get_survivability_mode > > >>> referenced by xe_survivability_mode.c:224 > > >>> drivers/gpu/drm/xe/xe_survivability_mode.o:(xe_survivability_mode_is_requested) in archive vmlinux.a > > > > A recent workaround added a 'IS_REACHABLE()' check around the calls into > > configfs, which makes it link in this configuration, but this is generally > > a bad idea because it makes the actual behavior inconsistent and confusing > > to users that expect configfs to work if both xe and configfs are enabled. > > > > Use 'select CONFIGFS_FS' instead, like every other configfs user does, > > and remove the unnecessary stub functions. There is little practical > > difference here as anyone using XE graphics is likely to have a complex > > configuration that already selects configfs anyway, and the link error > > or the IS_REACHABLE() would only manifest during randconfig testing. > > > > Fixes: 30499cbd7961 ("drm/xe: Fix build with DRM_XE=y and CONFIGFS_FS=m") > > Fixes: 16280ded45fb ("drm/xe: Add configfs to enable survivability mode") > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > > --- > > drivers/gpu/drm/xe/Kconfig | 1 + > > drivers/gpu/drm/xe/xe_configfs.h | 30 ------------------------------ > > 2 files changed, 1 insertion(+), 30 deletions(-) > > > > diff --git a/drivers/gpu/drm/xe/Kconfig b/drivers/gpu/drm/xe/Kconfig > > index 4fc8abdc5cb6..b52a8dc28f97 100644 > > --- a/drivers/gpu/drm/xe/Kconfig > > +++ b/drivers/gpu/drm/xe/Kconfig > > @@ -11,6 +11,7 @@ config DRM_XE > > # the shmem_readpage() which depends upon tmpfs > > select SHMEM > > select TMPFS > > + select CONFIGFS_FS > > Perhaps another option would be what Jani had suggested: > > config DRM_XE_CONFIGFS > def_bool y > depends on DRM_XE && CONFIGFS_FS > depends on CONFIGFS_FS=y || DRM_XE=m > > But anyway, we are depending more and more on the configfs for > production choices. So, let's go with you select and close this > for good. I really don't think this patch was the way we wanted to go. IS_REACHABLE was giving us exactly the behavior that we wanted, but now we're getting behavior that we did not want. Xe absolutely does not depend on configfs in any way. _If_ configfs is there we'll enable some config options in it, but none of those are mandatory for our driver as a whole. We're perfectly happy to build and run without configfs if it's not selected in the config. We're also happy to build and run without configfs functionality if the user wanted Xe built directly into the kernel, but configfs was built as an unreachable module solely for other non-Xe purposes. While this change won't matter to 99% of the cases where someone is running with a typical distro kernel, this is the kind of thing that makes embedded users unhappy since they're very carefully managing their configurations and we've taken away their ability to avoid configfs now. Also, as mentioned earlier in the threads, using 'select' on a user-visible config option rather than a hidden option often causes cascading problems down the road because it ignores any dependencies of the option being selected (configfs in this case). At the moment I don't think configfs has any dependencies of its own, but if it gets some down the road, this can cause problems. Matt > > I'm pushing this to drm-xe-next. > > Thanks for the patch. > > > > select DRM_BUDDY > > select DRM_CLIENT_SELECTION > > select DRM_KMS_HELPER > > diff --git a/drivers/gpu/drm/xe/xe_configfs.h b/drivers/gpu/drm/xe/xe_configfs.h > > index 327b648944ff..d726800de26a 100644 > > --- a/drivers/gpu/drm/xe/xe_configfs.h > > +++ b/drivers/gpu/drm/xe/xe_configfs.h > > @@ -14,7 +14,6 @@ > > > > struct pci_dev; > > > > -#if IS_REACHABLE(CONFIG_CONFIGFS_FS) > > int xe_configfs_init(void); > > void xe_configfs_exit(void); > > void xe_configfs_check_device(struct pci_dev *pdev); > > @@ -36,34 +35,5 @@ u32 xe_configfs_get_ctx_restore_post_bb(struct pci_dev *pdev, > > unsigned int xe_configfs_get_max_vfs(struct pci_dev *pdev); > > bool xe_configfs_admin_only_pf(struct pci_dev *pdev); > > #endif > > -#else > > -static inline int xe_configfs_init(void) { return 0; } > > -static inline void xe_configfs_exit(void) { } > > -static inline void xe_configfs_check_device(struct pci_dev *pdev) { } > > -static inline bool xe_configfs_get_survivability_mode(struct pci_dev *pdev) { return false; } > > -static inline bool xe_configfs_primary_gt_allowed(struct pci_dev *pdev) { return true; } > > -static inline bool xe_configfs_media_gt_allowed(struct pci_dev *pdev) { return true; } > > -static inline u64 xe_configfs_get_engines_allowed(struct pci_dev *pdev) { return U64_MAX; } > > -static inline bool xe_configfs_get_psmi_enabled(struct pci_dev *pdev) { return false; } > > -static inline bool xe_configfs_get_enable_multi_queue(struct pci_dev *pdev) { return true; } > > -static inline u32 xe_configfs_get_migrate_ulls_period_ms(struct pci_dev *pdev) { return 5; } > > -static inline bool xe_configfs_get_disable_vram_page_offline(struct pci_dev *pdev) { return false; } > > -static inline u32 xe_configfs_get_ctx_restore_mid_bb(struct pci_dev *pdev, > > - enum xe_engine_class class, > > - const u32 **cs) { return 0; } > > -static inline u32 xe_configfs_get_ctx_restore_post_bb(struct pci_dev *pdev, > > - enum xe_engine_class class, > > - const u32 **cs) { return 0; } > > -#ifdef CONFIG_PCI_IOV > > -static inline unsigned int xe_configfs_get_max_vfs(struct pci_dev *pdev) > > -{ > > - return xe_modparam.max_vfs; > > -} > > -static inline bool xe_configfs_admin_only_pf(struct pci_dev *pdev) > > -{ > > - return XE_DEFAULT_ADMIN_ONLY_PF; > > -} > > -#endif > > -#endif > > > > #endif > > -- > > 2.53.0 > > -- Matt Roper Graphics Software Engineer Linux GPU Platform Enablement Intel Corporation ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm: xe: select configfs 2026-11-13 16:22 ` Matt Roper @ 2026-11-13 17:51 ` Raag Jadav 2026-11-13 17:58 ` Rodrigo Vivi 1 sibling, 0 replies; 6+ messages in thread From: Raag Jadav @ 2026-11-13 17:51 UTC (permalink / raw) To: Matt Roper Cc: Rodrigo Vivi, Arnd Bergmann, Matthew Brost, Thomas Hellström, David Airlie, Simona Vetter, Lucas De Marchi, Riana Tauro, Arnd Bergmann, Matthew Auld, Shuicheng Lin, Armin Wolf, Zhanjun Dong, Michal Wajdeczko, Piotr Piórkowski, Tejas Upadhyay, intel-xe, dri-devel, linux-kernel On Fri, Nov 13, 2026 at 08:22:12AM -0800, Matt Roper wrote: > On Wed, Nov 11, 2026 at 02:01:45PM -0500, Rodrigo Vivi wrote: > > On Mon, Oct 26, 2026 at 11:25:50AM +0100, Arnd Bergmann wrote: > > > From: Arnd Bergmann <arnd@arndb.de> > > > > > > The original addition of configfs to xe missed a compile-time dependency, > > > which leads to a link failure when configfs is enabled as a loadable module: > > > > > > ld.lld-21: error: undefined symbol: xe_configfs_get_survivability_mode > > > >>> referenced by xe_survivability_mode.c:224 > > > >>> drivers/gpu/drm/xe/xe_survivability_mode.o:(xe_survivability_mode_is_requested) in archive vmlinux.a > > > > > > A recent workaround added a 'IS_REACHABLE()' check around the calls into > > > configfs, which makes it link in this configuration, but this is generally > > > a bad idea because it makes the actual behavior inconsistent and confusing > > > to users that expect configfs to work if both xe and configfs are enabled. > > > > > > Use 'select CONFIGFS_FS' instead, like every other configfs user does, > > > and remove the unnecessary stub functions. There is little practical > > > difference here as anyone using XE graphics is likely to have a complex > > > configuration that already selects configfs anyway, and the link error > > > or the IS_REACHABLE() would only manifest during randconfig testing. > > > > > > Fixes: 30499cbd7961 ("drm/xe: Fix build with DRM_XE=y and CONFIGFS_FS=m") > > > Fixes: 16280ded45fb ("drm/xe: Add configfs to enable survivability mode") > > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > > > --- > > > drivers/gpu/drm/xe/Kconfig | 1 + > > > drivers/gpu/drm/xe/xe_configfs.h | 30 ------------------------------ > > > 2 files changed, 1 insertion(+), 30 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/xe/Kconfig b/drivers/gpu/drm/xe/Kconfig > > > index 4fc8abdc5cb6..b52a8dc28f97 100644 > > > --- a/drivers/gpu/drm/xe/Kconfig > > > +++ b/drivers/gpu/drm/xe/Kconfig > > > @@ -11,6 +11,7 @@ config DRM_XE > > > # the shmem_readpage() which depends upon tmpfs > > > select SHMEM > > > select TMPFS > > > + select CONFIGFS_FS > > > > Perhaps another option would be what Jani had suggested: > > > > config DRM_XE_CONFIGFS > > def_bool y > > depends on DRM_XE && CONFIGFS_FS > > depends on CONFIGFS_FS=y || DRM_XE=m > > > > But anyway, we are depending more and more on the configfs for > > production choices. So, let's go with you select and close this > > for good. > > I really don't think this patch was the way we wanted to go. > IS_REACHABLE was giving us exactly the behavior that we wanted, but now > we're getting behavior that we did not want. Xe absolutely does not > depend on configfs in any way. _If_ configfs is there we'll enable some > config options in it, but none of those are mandatory for our driver as > a whole. We're perfectly happy to build and run without configfs if > it's not selected in the config. We're also happy to build and run > without configfs functionality if the user wanted Xe built directly into > the kernel, but configfs was built as an unreachable module solely for > other non-Xe purposes. > > While this change won't matter to 99% of the cases where someone is > running with a typical distro kernel, this is the kind of thing that > makes embedded users unhappy since they're very carefully managing their > configurations and we've taken away their ability to avoid configfs now. +1'ing on this. People go to great lengths to save a few bytes, only to be undone by a non-essential config. Raag ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm: xe: select configfs 2026-11-13 16:22 ` Matt Roper 2026-11-13 17:51 ` Raag Jadav @ 2026-11-13 17:58 ` Rodrigo Vivi 2026-11-13 18:37 ` Matt Roper 1 sibling, 1 reply; 6+ messages in thread From: Rodrigo Vivi @ 2026-11-13 17:58 UTC (permalink / raw) To: Matt Roper Cc: Arnd Bergmann, Matthew Brost, Thomas Hellström, David Airlie, Simona Vetter, Lucas De Marchi, Riana Tauro, Arnd Bergmann, Matthew Auld, Shuicheng Lin, Armin Wolf, Zhanjun Dong, Michal Wajdeczko, Piotr Piórkowski, Tejas Upadhyay, intel-xe, dri-devel, linux-kernel On Fri, Nov 13, 2026 at 08:22:12AM -0800, Matt Roper wrote: > On Wed, Nov 11, 2026 at 02:01:45PM -0500, Rodrigo Vivi wrote: > > On Mon, Oct 26, 2026 at 11:25:50AM +0100, Arnd Bergmann wrote: > > > From: Arnd Bergmann <arnd@arndb.de> > > > > > > The original addition of configfs to xe missed a compile-time dependency, > > > which leads to a link failure when configfs is enabled as a loadable module: > > > > > > ld.lld-21: error: undefined symbol: xe_configfs_get_survivability_mode > > > >>> referenced by xe_survivability_mode.c:224 > > > >>> drivers/gpu/drm/xe/xe_survivability_mode.o:(xe_survivability_mode_is_requested) in archive vmlinux.a > > > > > > A recent workaround added a 'IS_REACHABLE()' check around the calls into > > > configfs, which makes it link in this configuration, but this is generally > > > a bad idea because it makes the actual behavior inconsistent and confusing > > > to users that expect configfs to work if both xe and configfs are enabled. > > > > > > Use 'select CONFIGFS_FS' instead, like every other configfs user does, > > > and remove the unnecessary stub functions. There is little practical > > > difference here as anyone using XE graphics is likely to have a complex > > > configuration that already selects configfs anyway, and the link error > > > or the IS_REACHABLE() would only manifest during randconfig testing. > > > > > > Fixes: 30499cbd7961 ("drm/xe: Fix build with DRM_XE=y and CONFIGFS_FS=m") > > > Fixes: 16280ded45fb ("drm/xe: Add configfs to enable survivability mode") > > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > > > --- > > > drivers/gpu/drm/xe/Kconfig | 1 + > > > drivers/gpu/drm/xe/xe_configfs.h | 30 ------------------------------ > > > 2 files changed, 1 insertion(+), 30 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/xe/Kconfig b/drivers/gpu/drm/xe/Kconfig > > > index 4fc8abdc5cb6..b52a8dc28f97 100644 > > > --- a/drivers/gpu/drm/xe/Kconfig > > > +++ b/drivers/gpu/drm/xe/Kconfig > > > @@ -11,6 +11,7 @@ config DRM_XE > > > # the shmem_readpage() which depends upon tmpfs > > > select SHMEM > > > select TMPFS > > > + select CONFIGFS_FS > > > > Perhaps another option would be what Jani had suggested: > > > > config DRM_XE_CONFIGFS > > def_bool y > > depends on DRM_XE && CONFIGFS_FS > > depends on CONFIGFS_FS=y || DRM_XE=m > > > > But anyway, we are depending more and more on the configfs for > > production choices. So, let's go with you select and close this > > for good. > > I really don't think this patch was the way we wanted to go. > IS_REACHABLE was giving us exactly the behavior that we wanted, But it caused other issues like reported in the commit msg of this patch. With the risk of other complications like Jani had pointed out earlier. > but now > we're getting behavior that we did not want. Xe absolutely does not > depend on configfs in any way. _If_ configfs is there we'll enable some > config options in it, but none of those are mandatory for our driver as > a whole. We're perfectly happy to build and run without configfs if > it's not selected in the config. We're also happy to build and run > without configfs functionality if the user wanted Xe built directly into > the kernel, but configfs was built as an unreachable module solely for > other non-Xe purposes. > > While this change won't matter to 99% of the cases where someone is > running with a typical distro kernel, this is the kind of thing that > makes embedded users unhappy since they're very carefully managing their > configurations and we've taken away their ability to avoid configfs now. I believe some other user could be angry as well for ending in a case where they need manual survivability mode to flash the firmware, but they don't have this option available because in his config this option was not reachable. Soon SRIOV will be another production case that will depend on the configfs. I know, one might argue that we should then select only if SRIOV is enabled. But I don't like the complication this is taking on a very simple thing. > > Also, as mentioned earlier in the threads, using 'select' on a > user-visible config option rather than a hidden option often causes > cascading problems down the road because it ignores any dependencies of > the option being selected (configfs in this case). At the moment I > don't think configfs has any dependencies of its own, but if it gets > some down the road, this can cause problems. I'm not strongly in favor of any approach, as long that it doesn't cause other issues and regressions. So, if someone want to try to bring this again, proving it is a good config combination solution that won't cause further issue I will be glad to accept. Thanks, Rodrigo. > > > Matt > > > > > I'm pushing this to drm-xe-next. > > > > Thanks for the patch. > > > > > > > select DRM_BUDDY > > > select DRM_CLIENT_SELECTION > > > select DRM_KMS_HELPER > > > diff --git a/drivers/gpu/drm/xe/xe_configfs.h b/drivers/gpu/drm/xe/xe_configfs.h > > > index 327b648944ff..d726800de26a 100644 > > > --- a/drivers/gpu/drm/xe/xe_configfs.h > > > +++ b/drivers/gpu/drm/xe/xe_configfs.h > > > @@ -14,7 +14,6 @@ > > > > > > struct pci_dev; > > > > > > -#if IS_REACHABLE(CONFIG_CONFIGFS_FS) > > > int xe_configfs_init(void); > > > void xe_configfs_exit(void); > > > void xe_configfs_check_device(struct pci_dev *pdev); > > > @@ -36,34 +35,5 @@ u32 xe_configfs_get_ctx_restore_post_bb(struct pci_dev *pdev, > > > unsigned int xe_configfs_get_max_vfs(struct pci_dev *pdev); > > > bool xe_configfs_admin_only_pf(struct pci_dev *pdev); > > > #endif > > > -#else > > > -static inline int xe_configfs_init(void) { return 0; } > > > -static inline void xe_configfs_exit(void) { } > > > -static inline void xe_configfs_check_device(struct pci_dev *pdev) { } > > > -static inline bool xe_configfs_get_survivability_mode(struct pci_dev *pdev) { return false; } > > > -static inline bool xe_configfs_primary_gt_allowed(struct pci_dev *pdev) { return true; } > > > -static inline bool xe_configfs_media_gt_allowed(struct pci_dev *pdev) { return true; } > > > -static inline u64 xe_configfs_get_engines_allowed(struct pci_dev *pdev) { return U64_MAX; } > > > -static inline bool xe_configfs_get_psmi_enabled(struct pci_dev *pdev) { return false; } > > > -static inline bool xe_configfs_get_enable_multi_queue(struct pci_dev *pdev) { return true; } > > > -static inline u32 xe_configfs_get_migrate_ulls_period_ms(struct pci_dev *pdev) { return 5; } > > > -static inline bool xe_configfs_get_disable_vram_page_offline(struct pci_dev *pdev) { return false; } > > > -static inline u32 xe_configfs_get_ctx_restore_mid_bb(struct pci_dev *pdev, > > > - enum xe_engine_class class, > > > - const u32 **cs) { return 0; } > > > -static inline u32 xe_configfs_get_ctx_restore_post_bb(struct pci_dev *pdev, > > > - enum xe_engine_class class, > > > - const u32 **cs) { return 0; } > > > -#ifdef CONFIG_PCI_IOV > > > -static inline unsigned int xe_configfs_get_max_vfs(struct pci_dev *pdev) > > > -{ > > > - return xe_modparam.max_vfs; > > > -} > > > -static inline bool xe_configfs_admin_only_pf(struct pci_dev *pdev) > > > -{ > > > - return XE_DEFAULT_ADMIN_ONLY_PF; > > > -} > > > -#endif > > > -#endif > > > > > > #endif > > > -- > > > 2.53.0 > > > > > -- > Matt Roper > Graphics Software Engineer > Linux GPU Platform Enablement > Intel Corporation ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm: xe: select configfs 2026-11-13 17:58 ` Rodrigo Vivi @ 2026-11-13 18:37 ` Matt Roper 0 siblings, 0 replies; 6+ messages in thread From: Matt Roper @ 2026-11-13 18:37 UTC (permalink / raw) To: Rodrigo Vivi Cc: Arnd Bergmann, Matthew Brost, Thomas Hellström, David Airlie, Simona Vetter, Lucas De Marchi, Riana Tauro, Arnd Bergmann, Matthew Auld, Shuicheng Lin, Armin Wolf, Zhanjun Dong, Michal Wajdeczko, Piotr Piórkowski, Tejas Upadhyay, intel-xe, dri-devel, linux-kernel On Fri, Nov 13, 2026 at 12:58:21PM -0500, Rodrigo Vivi wrote: > On Fri, Nov 13, 2026 at 08:22:12AM -0800, Matt Roper wrote: > > On Wed, Nov 11, 2026 at 02:01:45PM -0500, Rodrigo Vivi wrote: > > > On Mon, Oct 26, 2026 at 11:25:50AM +0100, Arnd Bergmann wrote: > > > > From: Arnd Bergmann <arnd@arndb.de> > > > > > > > > The original addition of configfs to xe missed a compile-time dependency, > > > > which leads to a link failure when configfs is enabled as a loadable module: > > > > > > > > ld.lld-21: error: undefined symbol: xe_configfs_get_survivability_mode > > > > >>> referenced by xe_survivability_mode.c:224 > > > > >>> drivers/gpu/drm/xe/xe_survivability_mode.o:(xe_survivability_mode_is_requested) in archive vmlinux.a > > > > > > > > A recent workaround added a 'IS_REACHABLE()' check around the calls into > > > > configfs, which makes it link in this configuration, but this is generally > > > > a bad idea because it makes the actual behavior inconsistent and confusing > > > > to users that expect configfs to work if both xe and configfs are enabled. > > > > > > > > Use 'select CONFIGFS_FS' instead, like every other configfs user does, > > > > and remove the unnecessary stub functions. There is little practical > > > > difference here as anyone using XE graphics is likely to have a complex > > > > configuration that already selects configfs anyway, and the link error > > > > or the IS_REACHABLE() would only manifest during randconfig testing. > > > > > > > > Fixes: 30499cbd7961 ("drm/xe: Fix build with DRM_XE=y and CONFIGFS_FS=m") > > > > Fixes: 16280ded45fb ("drm/xe: Add configfs to enable survivability mode") > > > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > > > > --- > > > > drivers/gpu/drm/xe/Kconfig | 1 + > > > > drivers/gpu/drm/xe/xe_configfs.h | 30 ------------------------------ > > > > 2 files changed, 1 insertion(+), 30 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/xe/Kconfig b/drivers/gpu/drm/xe/Kconfig > > > > index 4fc8abdc5cb6..b52a8dc28f97 100644 > > > > --- a/drivers/gpu/drm/xe/Kconfig > > > > +++ b/drivers/gpu/drm/xe/Kconfig > > > > @@ -11,6 +11,7 @@ config DRM_XE > > > > # the shmem_readpage() which depends upon tmpfs > > > > select SHMEM > > > > select TMPFS > > > > + select CONFIGFS_FS > > > > > > Perhaps another option would be what Jani had suggested: > > > > > > config DRM_XE_CONFIGFS > > > def_bool y > > > depends on DRM_XE && CONFIGFS_FS > > > depends on CONFIGFS_FS=y || DRM_XE=m > > > > > > But anyway, we are depending more and more on the configfs for > > > production choices. So, let's go with you select and close this > > > for good. > > > > I really don't think this patch was the way we wanted to go. > > IS_REACHABLE was giving us exactly the behavior that we wanted, > > But it caused other issues like reported in the commit msg of this patch. > With the risk of other complications like Jani had pointed out earlier. No, the existing code had already fixed the issue reported in the commit message; this commit isn't fixing any further issues. The justification for this patch was "users who don't understand the reachability implications of 'M' vs 'Y' might not get the behavior they expect if they make mistakes while hand-modifying their config." But that's just user error; those exact configuration settings (Xe="Y" and configfs="M") might have been precisely what some other user wanted. > > > but now > > we're getting behavior that we did not want. Xe absolutely does not > > depend on configfs in any way. _If_ configfs is there we'll enable some > > config options in it, but none of those are mandatory for our driver as > > a whole. We're perfectly happy to build and run without configfs if > > it's not selected in the config. We're also happy to build and run > > without configfs functionality if the user wanted Xe built directly into > > the kernel, but configfs was built as an unreachable module solely for > > other non-Xe purposes. > > > > While this change won't matter to 99% of the cases where someone is > > running with a typical distro kernel, this is the kind of thing that > > makes embedded users unhappy since they're very carefully managing their > > configurations and we've taken away their ability to avoid configfs now. > > I believe some other user could be angry as well for ending in a case > where they need manual survivability mode to flash the firmware, but > they don't have this option available because in his config this > option was not reachable. Kconfig lets users select the options they want. If they accidentally create a config that doesn't include the options they need (either because they left them at 'N' or because they set them at 'M' to prevent them from being accessible as part of the built-in drivers), then that's their own mistake and they can correct it themselves. In some embedded environments, there are very strict certification steps that have to be applied to code built directly into the kernel, and sometimes a bit more lee-way for the .ko's that may or may not get loaded later. If an embedded device knows they have no use for Xe's configfs settings, they may not want configfs built into the kernel due to the extra burdens that places on them. There may be other drivers or facilities that are not part of their critical path that are built as modules, and having configfs also built as a .ko for those other areas may be useful. So Xe='Y' + configs='M' is a deliberate choice to match a specialized environment. But we've just broken that flexibility with this change. > > Soon SRIOV will be another production case that will depend on the configfs. > I know, one might argue that we should then select only if SRIOV is enabled. > > But I don't like the complication this is taking on a very simple thing. I'd argue that this *isn't* a simple thing, even if throwing a 'select' in the code and removing the stubs is easy for us as developers. The reality is that crafting a fine-tuned kernel config for some specific purpose is always going to be a complex task. Generic configs like those provided by distros sidestep the complexity by including anything that someone potentially might want, but if you're fine-tuning a config then you really have to pay attention to all the dependencies and consider exactly what you do or don't want included as builtin or module. It's easy to make mistakes, but that's always been the case if you're hand-crafting a minimal kernel configuration. > > > > > Also, as mentioned earlier in the threads, using 'select' on a > > user-visible config option rather than a hidden option often causes > > cascading problems down the road because it ignores any dependencies of > > the option being selected (configfs in this case). At the moment I > > don't think configfs has any dependencies of its own, but if it gets > > some down the road, this can cause problems. > > I'm not strongly in favor of any approach, as long that it doesn't cause > other issues and regressions. So, if someone want to try to bring this > again, proving it is a good config combination solution that won't > cause further issue I will be glad to accept. There weren't any issues before this patch (except for hypothetical user error), but now we've removed flexibility that was important to a [small] subset of users. Matt > > Thanks, > Rodrigo. > > > > > > > Matt > > > > > > > > I'm pushing this to drm-xe-next. > > > > > > Thanks for the patch. > > > > > > > > > > select DRM_BUDDY > > > > select DRM_CLIENT_SELECTION > > > > select DRM_KMS_HELPER > > > > diff --git a/drivers/gpu/drm/xe/xe_configfs.h b/drivers/gpu/drm/xe/xe_configfs.h > > > > index 327b648944ff..d726800de26a 100644 > > > > --- a/drivers/gpu/drm/xe/xe_configfs.h > > > > +++ b/drivers/gpu/drm/xe/xe_configfs.h > > > > @@ -14,7 +14,6 @@ > > > > > > > > struct pci_dev; > > > > > > > > -#if IS_REACHABLE(CONFIG_CONFIGFS_FS) > > > > int xe_configfs_init(void); > > > > void xe_configfs_exit(void); > > > > void xe_configfs_check_device(struct pci_dev *pdev); > > > > @@ -36,34 +35,5 @@ u32 xe_configfs_get_ctx_restore_post_bb(struct pci_dev *pdev, > > > > unsigned int xe_configfs_get_max_vfs(struct pci_dev *pdev); > > > > bool xe_configfs_admin_only_pf(struct pci_dev *pdev); > > > > #endif > > > > -#else > > > > -static inline int xe_configfs_init(void) { return 0; } > > > > -static inline void xe_configfs_exit(void) { } > > > > -static inline void xe_configfs_check_device(struct pci_dev *pdev) { } > > > > -static inline bool xe_configfs_get_survivability_mode(struct pci_dev *pdev) { return false; } > > > > -static inline bool xe_configfs_primary_gt_allowed(struct pci_dev *pdev) { return true; } > > > > -static inline bool xe_configfs_media_gt_allowed(struct pci_dev *pdev) { return true; } > > > > -static inline u64 xe_configfs_get_engines_allowed(struct pci_dev *pdev) { return U64_MAX; } > > > > -static inline bool xe_configfs_get_psmi_enabled(struct pci_dev *pdev) { return false; } > > > > -static inline bool xe_configfs_get_enable_multi_queue(struct pci_dev *pdev) { return true; } > > > > -static inline u32 xe_configfs_get_migrate_ulls_period_ms(struct pci_dev *pdev) { return 5; } > > > > -static inline bool xe_configfs_get_disable_vram_page_offline(struct pci_dev *pdev) { return false; } > > > > -static inline u32 xe_configfs_get_ctx_restore_mid_bb(struct pci_dev *pdev, > > > > - enum xe_engine_class class, > > > > - const u32 **cs) { return 0; } > > > > -static inline u32 xe_configfs_get_ctx_restore_post_bb(struct pci_dev *pdev, > > > > - enum xe_engine_class class, > > > > - const u32 **cs) { return 0; } > > > > -#ifdef CONFIG_PCI_IOV > > > > -static inline unsigned int xe_configfs_get_max_vfs(struct pci_dev *pdev) > > > > -{ > > > > - return xe_modparam.max_vfs; > > > > -} > > > > -static inline bool xe_configfs_admin_only_pf(struct pci_dev *pdev) > > > > -{ > > > > - return XE_DEFAULT_ADMIN_ONLY_PF; > > > > -} > > > > -#endif > > > > -#endif > > > > > > > > #endif > > > > -- > > > > 2.53.0 > > > > > > > > -- > > Matt Roper > > Graphics Software Engineer > > Linux GPU Platform Enablement > > Intel Corporation -- Matt Roper Graphics Software Engineer Linux GPU Platform Enablement Intel Corporation ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-11-13 18:38 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-10-26 10:25 [PATCH] drm: xe: select configfs Arnd Bergmann 2026-11-11 19:01 ` Rodrigo Vivi 2026-11-13 16:22 ` Matt Roper 2026-11-13 17:51 ` Raag Jadav 2026-11-13 17:58 ` Rodrigo Vivi 2026-11-13 18:37 ` Matt Roper
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®