Re: [PATCH] drm/xe/: Mutual Exclusivity b/w Multi CCS Mode & SRIOV VF Provisioning

[Michal Wajdeczko <michal.wajdeczko@intel.com>]

On 9/15/2025 4:33 PM, Nareshkumar Gollakoti wrote:
> Multi CCS mode can only be enabled when SRIOV is in PF mode with
> no VFs provisioned.

maybe: "Due to SLA agreement between PF and VFs, multi CCS mode can't
be enabled when VFs are already enabled.

> Similarly, provisioning VFs is not allowed when Multi CCS mode is active.
> 
> Signed-off-by: Nareshkumar Gollakoti <naresh.kumar.g@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_gt_ccs_mode.c |  7 ++++---
>  drivers/gpu/drm/xe/xe_gt_ccs_mode.h | 10 ++++++++++
>  drivers/gpu/drm/xe/xe_pci_sriov.c   |  7 +++++++
>  drivers/gpu/drm/xe/xe_sriov.h       | 12 ++++++++++++
>  4 files changed, 33 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_gt_ccs_mode.c b/drivers/gpu/drm/xe/xe_gt_ccs_mode.c
> index 50fffc9ebf62..39df801e9cd5 100644
> --- a/drivers/gpu/drm/xe/xe_gt_ccs_mode.c
> +++ b/drivers/gpu/drm/xe/xe_gt_ccs_mode.c
> @@ -117,9 +117,10 @@ ccs_mode_store(struct device *kdev, struct device_attribute *attr,
>  	u32 num_engines, num_slices;
>  	int ret;
>  
> -	if (IS_SRIOV(xe)) {
> -		xe_gt_dbg(gt, "Can't change compute mode when running as %s\n",
> -			  xe_sriov_mode_to_string(xe_device_sriov_mode(xe)));
> +	if (IS_SRIOV_VF_ENABLED(xe)) {
> +		xe_gt_dbg(gt, "Can't change compute mode when running as %s / %s\n",
> +			  xe_sriov_mode_to_string(xe_device_sriov_mode(xe)),
> +			  "One or more VFs enabled");

this should be treated separately, as there is different rationale:

- it can't be changed on PF with enabled VFs, as this would impact existing VFs
- it can't be changed by VFs, as they simply can't do that at all

>  		return -EOPNOTSUPP;
>  	}

what if right after this check here someone else enables VFs?

>  
> diff --git a/drivers/gpu/drm/xe/xe_gt_ccs_mode.h b/drivers/gpu/drm/xe/xe_gt_ccs_mode.h
> index f8779852cf0d..1a80a293daa2 100644
> --- a/drivers/gpu/drm/xe/xe_gt_ccs_mode.h
> +++ b/drivers/gpu/drm/xe/xe_gt_ccs_mode.h
> @@ -20,5 +20,15 @@ static inline bool xe_gt_ccs_mode_enabled(const struct xe_gt *gt)
>  	return hweight32(CCS_MASK(gt)) > 1;
>  }
>  

it's always good to add kerne-doc for public functions

> +static inline bool xe_is_primary_gt_multi_ccs_enabled(struct xe_device *xe)

it's quite bad to have "xe" level function in "gt" level file
maybe move to xe_device.h where other top level helpers are defined?

> +{
> +	/* multi CCS mode supported exclusively on GT0 */
> +	struct xe_gt *gt = xe_device_get_gt(xe, 0);
> +
> +	return (gt->ccs_mode > 1);

no need to use ( )

> +}
> +
> +#define IS_PRIMARY_GT_MULTI_CCS_ENABLED(xe) xe_is_primary_gt_multi_ccs_enabled(xe)

do you really need macro ?

> +
>  #endif
>  
> diff --git a/drivers/gpu/drm/xe/xe_pci_sriov.c b/drivers/gpu/drm/xe/xe_pci_sriov.c
> index af05db07162e..96e3fd51b8f5 100644
> --- a/drivers/gpu/drm/xe/xe_pci_sriov.c
> +++ b/drivers/gpu/drm/xe/xe_pci_sriov.c
> @@ -19,6 +19,7 @@
>  #include "xe_sriov_pf.h"
>  #include "xe_sriov_pf_helpers.h"
>  #include "xe_sriov_printk.h"
> +#include "xe_gt_ccs_mode.h"
>  
>  static int pf_needs_provisioning(struct xe_gt *gt, unsigned int num_vfs)
>  {
> @@ -153,6 +154,12 @@ static int pf_enable_vfs(struct xe_device *xe, int num_vfs)
>  	xe_assert(xe, IS_SRIOV_PF(xe));
>  	xe_assert(xe, num_vfs > 0);
>  	xe_assert(xe, num_vfs <= total_vfs);
> +

move this extra condition below dbg message below [1]
> +	if (IS_PRIMARY_GT_MULTI_CCS_ENABLED(xe)) {

	if (xe_multi_ccs_mode_enabled(xe))

> +		xe_sriov_info(xe, "Can't change the number of VFs while multi-CCS mode is enabled.");

"Disable multi-CCS mode before enabling VF%s.\n"  
> +
> +		return -EOPNOTSUPP;

this error suggest some permanent condition, which is not true here
maybe -EBUSY or -EUSERS or -ECANCELED ?

> +	}
>  	xe_sriov_dbg(xe, "enabling %u VF%s\n", num_vfs, str_plural(num_vfs));

[1] this supposed to be first dbg message

>  
>  	err = xe_sriov_pf_wait_ready(xe);
> diff --git a/drivers/gpu/drm/xe/xe_sriov.h b/drivers/gpu/drm/xe/xe_sriov.h
> index 6db45df55615..78019cee61fe 100644
> --- a/drivers/gpu/drm/xe/xe_sriov.h
> +++ b/drivers/gpu/drm/xe/xe_sriov.h
> @@ -36,6 +36,16 @@ static inline bool xe_device_is_sriov_vf(const struct xe_device *xe)
>  	return xe_device_sriov_mode(xe) == XE_SRIOV_MODE_VF;
>  }
>  
> +static inline bool xe_device_is_vf_enabled(const struct xe_device *xe)
> +{
> +	struct pci_dev *pdev = to_pci_dev(xe->drm.dev);
> +
> +	if (xe_device_is_sriov_vf(xe))
> +		return true;
> +
> +	return (pci_num_vf(pdev) > 0);
> +}

please don't mix PF and VF flows

use IS_SRIOV_VF(xe) separately

and if needed add (with kernel-doc)

static inline bool xe_sriov_pf_has_vfs_enabled(const struct xe_device *xe)

to xe_sriov_pf_helpers.h

> +
>  #ifdef CONFIG_PCI_IOV
>  #define IS_SRIOV_PF(xe) xe_device_is_sriov_pf(xe)
>  #else
> @@ -45,4 +55,6 @@ static inline bool xe_device_is_sriov_vf(const struct xe_device *xe)
>  
>  #define IS_SRIOV(xe) (IS_SRIOV_PF(xe) || IS_SRIOV_VF(xe))
>  
> +#define IS_SRIOV_VF_ENABLED(xe) xe_device_is_vf_enabled(xe)

please no

> +
>  #endif