Re: [RESEND FOR CI][PATCH v3] drm/xe/configfs: Add enable_multi_queue attribute

["Summers, Stuart" <stuart.summers@intel.com>]

On Thu, 2026-07-09 at 15:41 +0000, Lin, Shuicheng wrote:
> On Wed, Jul 8, 2026 3:57 PM Stuart Summers wrote:
> > On Wed, 2026-07-08 at 22:22 +0000, Shuicheng Lin wrote:
> > > Add a new configfs boolean attribute 'enable_multi_queue' that
> > > lets an
> > > administrator force-disable multi-queue support on a device
> > > before it
> > > binds to the driver. The attribute defaults to true (use the
> > > platform
> > > hardware capability as-is); writing 0 force-disables multi-queue.
> > > This
> > > is intended for debugging and for validating non-multi-queue code
> > > paths on hardware that would otherwise expose multi-queue.
> > > 
> > > The override disables multi-queue at two levels:
> > > 
> > > - UAPI: In alloc_primary_gt(), clear
> > >   gt->info.multi_queue_engine_class_mask on the primary GT so
> > > that
> > >   xe_gt_supports_multi_queue() returns false and attempts to
> > > create
> > >   a multi-queue group via
> > DRM_XE_EXEC_QUEUE_SET_PROPERTY_MULTI_GROUP
> > >   are rejected.
> > > 
> > > - GuC: In guc_ctl_feature_flags(), set the
> > > GUC_CTL_DISABLE_MULTI_QUEUE
> > >   (BIT(24)) init-params bit on GuC firmware older than 70.66. On
> > > GuC
> > >   firmware 70.66 and above, guc_waklv_init() emits the new
> > >   GUC_FEATURE_KLV_DISABLE_MULTI_QUEUE Feature KLV (0x5001) via
> > > the
> > ADS
> > >   WA/Feature KLV buffer instead. Feature KLVs share the WA KLV
> > > buffer.
> > > 
> > > The attribute is rejected after the device has been bound, so it
> > > only
> > > takes effect during probe:
> > > 
> > >         # echo 0 >
> > > /sys/kernel/config/xe/0000:03:00.0/enable_multi_queue
> > >         # echo 0000:03:00.0 > /sys/bus/pci/drivers/xe/bind
> > > 
> > > v2: Add log for multi-queue disabled. (Niranjana)
> > > v3: Rename attribute to enable_multi_queue with default true.
> > > (Stuart
> > > &&
> > >     Niranjana)
> > > 
> > > Assisted-by: Claude:claude-opus-4.7
> > > Cc: Niranjana Vishwanathapura
> > > <niranjana.vishwanathapura@intel.com>
> > > Cc: Stuart Summers <stuart.summers@intel.com>
> > > Signed-off-by: Shuicheng Lin <shuicheng.lin@intel.com>
> > > ---
> > >  drivers/gpu/drm/xe/abi/guc_klvs_abi.h |  3 +-
> > >  drivers/gpu/drm/xe/xe_configfs.c      | 65
> > > +++++++++++++++++++++++++++
> > >  drivers/gpu/drm/xe/xe_configfs.h      |  2 +
> > >  drivers/gpu/drm/xe/xe_guc.c           |  8 ++++
> > >  drivers/gpu/drm/xe/xe_guc_ads.c       | 14 ++++++
> > >  drivers/gpu/drm/xe/xe_guc_fwif.h      |  1 +
> > >  drivers/gpu/drm/xe/xe_pci.c           |  5 +++
> > >  7 files changed, 97 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/xe/abi/guc_klvs_abi.h
> > > b/drivers/gpu/drm/xe/abi/guc_klvs_abi.h
> > > index 644f5a4226d7..998272a007e5 100644
> > > --- a/drivers/gpu/drm/xe/abi/guc_klvs_abi.h
> > > +++ b/drivers/gpu/drm/xe/abi/guc_klvs_abi.h
> > > @@ -491,9 +491,10 @@ enum  {
> > >  #define
> > GUC_KLV_VF_CFG_ENGINE_GROUP_PREEMPT_TIMEOUT_MIN_LEN    1u
> > >  #define
> > > 
> > GUC_KLV_VF_CFG_ENGINE_GROUP_PREEMPT_TIMEOUT_MAX_LEN    GUC_
> > MAX_SCHED_
> > > GROUPS
> > >  /*
> > > - * Workaround keys:
> > > + * Feature and Workaround keys:
> > >   */
> > >  enum xe_guc_klv_ids {
> > > +       GUC_FEATURE_KLV_DISABLE_MULTI_QUEUE
> > >                    = 0x5001,
> > >         GUC_WORKAROUND_KLV_BLOCK_INTERRUPTS_WHEN_MGSR_BLOCKE
> > D
> > >                    = 0x9002,
> > >         GUC_WORKAROUND_KLV_DISABLE_PSMI_INTERRUPTS_AT_C6_ENTRY_
> > RESTOR
> > > E_AT_EXIT          = 0x9004,
> > >         GUC_WORKAROUND_KLV_ID_GAM_PFQ_SHADOW_TAIL_POLLING
> > >                    = 0x9005,
> > > diff --git a/drivers/gpu/drm/xe/xe_configfs.c
> > > b/drivers/gpu/drm/xe/xe_configfs.c
> > > index 32102600a148..0055487294fb 100644
> > > --- a/drivers/gpu/drm/xe/xe_configfs.c
> > > +++ b/drivers/gpu/drm/xe/xe_configfs.c
> > > @@ -237,6 +237,18 @@
> > >   *
> > >   * This setting only takes effect when probing the device.
> > >   *
> > > + * Enable multi-queue
> > > + * ------------------
> > > + *
> > > + * Multi-queue support on the device is enabled by default where
> > > the
> > > + * hardware supports it. Writing 0 force-disables multi-queue
> > 
> > @Niranjana I had thought we were trying to stick to a "multi queue"
> > naming instead of "multi-queue", but looking in xe_guc_submit.c and
> > others, I
> > see both forms being used. So no issue here, just wanted to point
> > out...
> > 
> > > support:
> > > + * multi-queue exec-queue group creation via ioctl is refused,
> > > and
> > > the
> > > + * GuC feature is disabled::
> > > + *
> > > + *     # echo 0 >
> > > /sys/kernel/config/xe/0000:03:00.0/enable_multi_queue
> > > + *
> > > + * This attribute can only be set before binding to the device.
> > > + *
> > >   * Remove devices
> > >   * ==============
> > >   *
> > > @@ -262,6 +274,7 @@ struct xe_config_group_device {
> > >                 struct wa_bb
> > > ctx_restore_mid_bb[XE_ENGINE_CLASS_MAX];
> > >                 bool survivability_mode;
> > >                 bool enable_psmi;
> > > +               bool enable_multi_queue;
> > >                 struct {
> > >                         unsigned int max_vfs;
> > >                         bool admin_only_pf; @@ -281,6 +294,7 @@
> > > static
> > > const struct xe_config_device device_defaults = {
> > >         .engines_allowed = U64_MAX,
> > >         .survivability_mode = false,
> > >         .enable_psmi = false,
> > > +       .enable_multi_queue = true,
> > >         .sriov = {
> > >                 .max_vfs = XE_DEFAULT_MAX_VFS,
> > >                 .admin_only_pf = XE_DEFAULT_ADMIN_ONLY_PF, @@ -
> > > 575,6
> > > +589,33 @@ static ssize_t enable_psmi_store(struct config_item
> > > *item,
> > > const char *page, siz
> > >         return len;
> > >  }
> > > 
> > > +static ssize_t enable_multi_queue_show(struct config_item *item,
> > > char *page)
> > > +{
> > > +       struct xe_config_device *dev = to_xe_config_device(item);
> > > +
> > > +       return sprintf(page, "%d\n", dev->enable_multi_queue); }
> > > +
> > > +static ssize_t enable_multi_queue_store(struct config_item
> > > *item,
> > > const char *page,
> > > +                                       size_t len) {
> > > +       struct xe_config_group_device *dev =
> > > to_xe_config_group_device(item);
> > > +       bool val;
> > > +       int ret;
> > > +
> > > +       ret = kstrtobool(page, &val);
> > > +       if (ret)
> > > +               return ret;
> > > +
> > > +       guard(mutex)(&dev->lock);
> > > +       if (is_bound(dev))
> > > +               return -EBUSY;
> > > +
> > > +       dev->config.enable_multi_queue = val;
> > > +
> > > +       return len;
> > > +}
> > > +
> > >  static bool wa_bb_read_advance(bool dereference, char **p,
> > >                                const char *append, size_t len,
> > >                                size_t *max_size) @@ -812,6 +853,7
> > > @@
> > > static ssize_t ctx_restore_post_bb_store(struct config_item
> > > *item,
> > > 
> > >  CONFIGFS_ATTR(, ctx_restore_mid_bb);
> > >  CONFIGFS_ATTR(, ctx_restore_post_bb);
> > > +CONFIGFS_ATTR(, enable_multi_queue);
> > >  CONFIGFS_ATTR(, enable_psmi);
> > >  CONFIGFS_ATTR(, engines_allowed);
> > >  CONFIGFS_ATTR(, gt_types_allowed);
> > > @@ -820,6 +862,7 @@ CONFIGFS_ATTR(, survivability_mode);
> > >  static struct configfs_attribute *xe_config_device_attrs[] = {
> > >         &attr_ctx_restore_mid_bb,
> > >         &attr_ctx_restore_post_bb,
> > > +       &attr_enable_multi_queue,
> > >         &attr_enable_psmi,
> > >         &attr_engines_allowed,
> > >         &attr_gt_types_allowed,
> > > @@ -1098,6 +1141,7 @@ static void dump_custom_dev_config(struct
> > > pci_dev *pdev,
> > >         PRI_CUSTOM_ATTR("%llx", gt_types_allowed);
> > >         PRI_CUSTOM_ATTR("%llx", engines_allowed);
> > >         PRI_CUSTOM_ATTR("%d", enable_psmi);
> > > +       PRI_CUSTOM_ATTR("%d", enable_multi_queue);
> > 
> > nit: should put this above enable_psmi to keep the alphabetical
> > order we're
> > also using in xe_config_device_attrs
> 
> Let me adjust it.
> 
> > 
> > >         PRI_CUSTOM_ATTR("%d", survivability_mode);
> > >         PRI_CUSTOM_ATTR("%u", sriov.admin_only_pf);
> > > 
> > > @@ -1225,6 +1269,27 @@ bool xe_configfs_get_psmi_enabled(struct
> > > pci_dev *pdev)
> > >         return ret;
> > >  }
> > > 
> > > +/**
> > > + * xe_configfs_get_enable_multi_queue - get configfs
> > > enable_multi_queue setting
> > > + * @pdev: pci device
> > > + *
> > > + * Return: true if multi-queue is enabled for this device (the
> > > default),
> > > + * false if it has been force-disabled via configfs.
> > > + */
> > > +bool xe_configfs_get_enable_multi_queue(struct pci_dev *pdev) {
> > > +       struct xe_config_group_device *dev =
> > > find_xe_config_group_device(pdev);
> > > +       bool ret;
> > > +
> > > +       if (!dev)
> > > +               return true;
> > > +
> > > +       ret = dev->config.enable_multi_queue;
> > > +       config_group_put(&dev->group);
> > > +
> > > +       return ret;
> > > +}
> > > +
> > >  /**
> > >   * xe_configfs_get_ctx_restore_mid_bb - get configfs
> > > ctx_restore_mid_bb setting
> > >   * @pdev: pci device
> > > diff --git a/drivers/gpu/drm/xe/xe_configfs.h
> > > b/drivers/gpu/drm/xe/xe_configfs.h
> > > index 07d62bf0c152..4fbbeafba473 100644
> > > --- a/drivers/gpu/drm/xe/xe_configfs.h
> > > +++ b/drivers/gpu/drm/xe/xe_configfs.h
> > > @@ -23,6 +23,7 @@ bool xe_configfs_primary_gt_allowed(struct
> > > pci_dev
> > > *pdev);
> > >  bool xe_configfs_media_gt_allowed(struct pci_dev *pdev);
> > >  u64 xe_configfs_get_engines_allowed(struct pci_dev *pdev);
> > >  bool xe_configfs_get_psmi_enabled(struct pci_dev *pdev);
> > > +bool xe_configfs_get_enable_multi_queue(struct pci_dev *pdev);
> > >  u32 xe_configfs_get_ctx_restore_mid_bb(struct pci_dev *pdev,
> > >                                        enum xe_engine_class
> > > class,
> > >                                        const u32 **cs); @@ -42,6
> > > +43,7
> > > @@ static inline bool xe_configfs_primary_gt_allowed(struct
> > > pci_dev
> > > *pdev) { return
> > >  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_ctx_restore_mid_bb(struct
> > > pci_dev
> > > *pdev,
> > >                                                      enum
> > > xe_engine_class class,
> > >                                                      const u32
> > > **cs) {
> > > return 0; } diff --git a/drivers/gpu/drm/xe/xe_guc.c
> > > b/drivers/gpu/drm/xe/xe_guc.c index 4023700ff2a9..c2f930afb298
> > 100644
> > > --- a/drivers/gpu/drm/xe/xe_guc.c
> > > +++ b/drivers/gpu/drm/xe/xe_guc.c
> > > @@ -101,6 +101,14 @@ static u32 guc_ctl_feature_flags(struct
> > > xe_guc
> > > *guc)
> > >         if (xe_device_is_l2_flush_optimized(xe) &&
> > > xe_gt_is_media_type(guc_to_gt(guc)))
> > >                 flags |= GUC_CTL_ENABLE_L2FLUSH_OPT;
> > > 
> > > +       /*
> > > +        * On GuC firmware 70.66 and above, the
> > > GUC_FEATURE_KLV_DISABLE_MULTI_QUEUE
> > > +        * Feature KLV is used instead.
> > 
> > Like Niranjana mentioned, it would be nice to make sure the new GuC
> > version
> > is merged first.
> 
> It should be OK to merge this patch since the code supports both old
> and new GuC versions, there is no dependency on it.
> 
> > 
> > > +        */
> > > +       if (!xe_configfs_get_enable_multi_queue(to_pci_dev(xe-
> > > > drm.dev)) &&
> > > +           !GUC_FIRMWARE_VER_AT_LEAST(guc, 70, 66))
> > > +               flags |= GUC_CTL_DISABLE_MULTI_QUEUE;
> > > +
> > >         return flags;
> > >  }
> > > 
> > > diff --git a/drivers/gpu/drm/xe/xe_guc_ads.c
> > > b/drivers/gpu/drm/xe/xe_guc_ads.c index
> > > c98454545a85..0b1872b6eb3e
> > > 100644
> > > --- a/drivers/gpu/drm/xe/xe_guc_ads.c
> > > +++ b/drivers/gpu/drm/xe/xe_guc_ads.c
> > > @@ -16,6 +16,7 @@
> > >  #include "regs/xe_gt_regs.h"
> > >  #include "regs/xe_guc_regs.h"
> > >  #include "xe_bo.h"
> > > +#include "xe_configfs.h"
> > >  #include "xe_gt.h"
> > >  #include "xe_gt_ccs_mode.h"
> > >  #include "xe_gt_mcr.h"
> > > @@ -364,6 +365,19 @@ static void guc_waklv_init(struct xe_guc_ads
> > > *ads)
> > >                 guc_waklv_enable(ads, NULL, 0, &offset, &remain,
> > > 
> > > GUC_WA_KLV_CLR_CS_INDIRECT_RING_STATE_IF_IDLE_AT_CTX_REG);
> > > 
> > > +       /*
> > > +        * On GuC firmware 70.66 and above, use the Feature KLV
> > > (shared with the
> > > +        * WA KLV buffer); older firmware uses
> > > GUC_CTL_DISABLE_MULTI_QUEUE in
> > > +        * the init params instead.
> > > +        */
> > > +       if
> > > (!xe_configfs_get_enable_multi_queue(to_pci_dev(gt_to_xe(gt)-
> > > > drm.dev)) &&
> > > +           GUC_FIRMWARE_VER_AT_LEAST(&gt->uc.guc, 70, 66)) {
> > > +               u32 data = 1;
> > 
> > nit: this feels a little weird to me, should we set data to
> > !enable_multi_queue
> > instead? But also we're only doing this if the GuC version matches
> > so the bool
> > is already there... not a blocker
> 
> This GuC KLV is only for disable multi-queue. It does not support
> enable multi-queue by this KLV. So, the only acceptable input is 1.

Ok makes sense and it doesn't hurt to be explicit here anyway...

> 
> > 
> > > +
> > > +               guc_waklv_enable(ads, &data, 1, &offset, &remain,
> > > +
> > > GUC_FEATURE_KLV_DISABLE_MULTI_QUEUE);
> > > +       }
> > > +
> > >         size = guc_ads_waklv_size(ads) - remain;
> > >         if (!size)
> > >                 return;
> > > diff --git a/drivers/gpu/drm/xe/xe_guc_fwif.h
> > > b/drivers/gpu/drm/xe/xe_guc_fwif.h
> > > index 3fbda4798cff..971b850f2136 100644
> > > --- a/drivers/gpu/drm/xe/xe_guc_fwif.h
> > > +++ b/drivers/gpu/drm/xe/xe_guc_fwif.h
> > > @@ -68,6 +68,7 @@ struct guc_update_exec_queue_policy {
> > >  #define   GUC_CTL_MAIN_GAMCTRL_QUEUES  BIT(9)
> > >  #define   GUC_CTL_DISABLE_SCHEDULER    BIT(14)
> > >  #define   GUC_CTL_ENABLE_L2FLUSH_OPT   BIT(15)
> > > +#define   GUC_CTL_DISABLE_MULTI_QUEUE  BIT(24)
> > > 
> > >  #define GUC_CTL_DEBUG                  3
> > >  #define   GUC_LOG_VERBOSITY            REG_GENMASK(1, 0) diff --
> > > git
> > > a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c index
> > > 91af603e9431..d6ec3b242c02 100644
> > > --- a/drivers/gpu/drm/xe/xe_pci.c
> > > +++ b/drivers/gpu/drm/xe/xe_pci.c
> > > @@ -22,6 +22,7 @@
> > >  #include "xe_device.h"
> > >  #include "xe_drv.h"
> > >  #include "xe_gt.h"
> > > +#include "xe_gt_printk.h"
> > >  #include "xe_gt_sriov_vf.h"
> > >  #include "xe_guc.h"
> > >  #include "xe_mmio.h"
> > > @@ -876,6 +877,10 @@ static struct xe_gt *alloc_primary_gt(struct
> > > xe_tile *tile,
> > >         gt->info.id = tile->id * xe->info.max_gt_per_tile;
> > >         gt->info.has_indirect_ring_state = graphics_desc-
> > > > has_indirect_ring_state;
> > >         gt->info.multi_queue_engine_class_mask = graphics_desc-
> > > > multi_queue_engine_class_mask;
> > > +       if (!xe_configfs_get_enable_multi_queue(to_pci_dev(xe-
> > > > drm.dev))) {
> > > +               xe_gt_info(gt, "Multi-queue disabled via
> > > configfs\n");
> > > +               gt->info.multi_queue_engine_class_mask = 0;
> > 
> > And since we're using this mask, should we use that directly in the
> > KLV feature
> > too instead of calling xe_configfs_get_enable_multi_queue
> > twice?
> 
> For platform doesn't support multi-queue, the configfs will return
> true in default, while the engine_class_mask is 0.
> So, it is better to use the configfs instead of the mask.

Hm.. but in that case it still would show a mask of 0 regardless of
what the configfs is set to. It honestly might not be a bad idea to
reject any configfs values set for platforms that aren't supported and
just use -1 as a default or something so it isn't confusing if a user
reads out configfs as enabled when the platform doesn't support it.

I think the hw_engines debugfs does indicate if the engine is enabled
for multi queue already, so the user does have a way to determine
whether at runtime an engine actually supports this or not.

Given this is all about root level configurability, I'm not too
concerned here. Maybe something we can do as a follow-up.

Thanks,
Stuart

> 
> > 
> > Everything else looks good to me and none of my comments are
> > blockers:
> > Reviewed-by: Stuart Summers <stuart.summers@intel.com>
> 
> Thanks for the review. I really appreciate it.
> 
> Shuicheng
> 
> > 
> > Thanks,
> > Stuart
> > 
> > > +       }
> > >         gt->info.engine_mask = graphics_desc->hw_engine_mask;
> > >         gt->info.num_geometry_xecore_fuse_regs = graphics_desc-
> > > > num_geometry_xecore_fuse_regs;
> > >         gt->info.num_compute_xecore_fuse_regs = graphics_desc-
> > > > num_compute_xecore_fuse_regs;
>