Re: [PATCH 5/6] drm/xe/pf: Add SR-IOV PF specific early GT initialization

["Ghimiray, Himal Prasad" <himal.prasad.ghimiray@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 5637 bytes --]


On 15-04-2024 00:31, Michal Wajdeczko wrote:
> The PF driver must maintain additional GT level data per each VF.
> This additional per-VF data will be added in upcoming patches and
> will include: provisioning configuration (like GGTT space or LMEM
> allocation sizes or scheduling parameters), monitoring thresholds
> and counters, and more.
>
> As number of supported VFs varies across platforms use flexible
> array where first entry will contain metadata for the PF itself
> (if such configuration parameter is applicable for the PF) and
> all remaining entries will contain data for potential VFs.
>
> Signed-off-by: Michal Wajdeczko<michal.wajdeczko@intel.com>
> ---
>   drivers/gpu/drm/xe/Makefile               |  1 +
>   drivers/gpu/drm/xe/xe_gt.c                |  7 +++
>   drivers/gpu/drm/xe/xe_gt_sriov_pf.c       | 52 +++++++++++++++++++++++
>   drivers/gpu/drm/xe/xe_gt_sriov_pf.h       | 20 +++++++++
>   drivers/gpu/drm/xe/xe_gt_sriov_pf_types.h |  9 ++++
>   5 files changed, 89 insertions(+)
>   create mode 100644 drivers/gpu/drm/xe/xe_gt_sriov_pf.c
>   create mode 100644 drivers/gpu/drm/xe/xe_gt_sriov_pf.h
>
> diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
> index c46d145606f6..daa5865a2773 100644
> --- a/drivers/gpu/drm/xe/Makefile
> +++ b/drivers/gpu/drm/xe/Makefile
> @@ -160,6 +160,7 @@ xe-y += \
>   	xe_sriov.o
>   
>   xe-$(CONFIG_PCI_IOV) += \
> +	xe_gt_sriov_pf.o \
>   	xe_gt_sriov_pf_control.o \
>   	xe_gt_sriov_pf_policy.o \
>   	xe_lmtt.o \
> diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c
> index cfa5da900461..38956b60e084 100644
> --- a/drivers/gpu/drm/xe/xe_gt.c
> +++ b/drivers/gpu/drm/xe/xe_gt.c
> @@ -29,6 +29,7 @@
>   #include "xe_gt_mcr.h"
>   #include "xe_gt_pagefault.h"
>   #include "xe_gt_printk.h"
> +#include "xe_gt_sriov_pf.h"
>   #include "xe_gt_sysfs.h"
>   #include "xe_gt_tlb_invalidation.h"
>   #include "xe_gt_topology.h"
> @@ -311,6 +312,12 @@ int xe_gt_init_early(struct xe_gt *gt)
>   {
>   	int err;
>   
> +	if (IS_SRIOV_PF(gt_to_xe(gt))) {
> +		err = xe_gt_sriov_pf_init_early(gt);
> +		if (err)
> +			return err;
> +	}
> +
>   	err = xe_force_wake_get(gt_to_fw(gt), XE_FW_GT);
>   	if (err)
>   		return err;
> diff --git a/drivers/gpu/drm/xe/xe_gt_sriov_pf.c b/drivers/gpu/drm/xe/xe_gt_sriov_pf.c
> new file mode 100644
> index 000000000000..791dcdd767e2
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/xe_gt_sriov_pf.c
> @@ -0,0 +1,52 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2023-2024 Intel Corporation
> + */
> +
> +#include <drm/drm_managed.h>
> +
> +#include "xe_gt_sriov_pf.h"
> +#include "xe_gt_sriov_pf_helpers.h"
> +
> +/*
> + * VF's metadata is maintained in the flexible array where:
> + *   - entry [0] contains metadata for the PF (only if applicable),
> + *   - entries [1..n] contain metadata for VF1..VFn::
> + *
> + *       <--------------------------- 1 + total_vfs ----------->
> + *      +-------+-------+-------+-----------------------+-------+
> + *      |   0   |   1   |   2   |                       |   n   |
> + *      +-------+-------+-------+-----------------------+-------+
> + *      |  PF   |  VF1  |  VF2  |      ...     ...      |  VFn  |
> + *      +-------+-------+-------+-----------------------+-------+
> + */
> +static int pf_alloc_metadata(struct xe_gt *gt)
> +{
> +	unsigned int num_vfs = xe_gt_sriov_pf_get_totalvfs(gt);
> +
> +	gt->sriov.pf.vfs = drmm_kcalloc(&gt_to_xe(gt)->drm, 1 + num_vfs,
> +					sizeof(*gt->sriov.pf.vfs), GFP_KERNEL);
> +	if (!gt->sriov.pf.vfs)
> +		return -ENOMEM;
> +
> +	return 0;
> +}
> +
> +/**
> + * xe_gt_sriov_pf_init_early - Prepare SR-IOV PF data structures on PF.
> + * @gt: the &xe_gt to initialize
> + *
> + * Early initialization of the PF data.
> + *
> + * Return: 0 on success or a negative error code on failure.
> + */
> +int xe_gt_sriov_pf_init_early(struct xe_gt *gt)
> +{
> +	int err;
> +
> +	err = pf_alloc_metadata(gt);
> +	if (err)
> +		return err;
> +
> +	return 0;

nit: If there's a possibility of extending this function in the future 
to accommodate other initializations, it's best to leave it as it is. 
Otherwise, return pf_alloc_metadata(gt) looks cleaner.

LGTM

Reviewed-by: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>


> +}
> diff --git a/drivers/gpu/drm/xe/xe_gt_sriov_pf.h b/drivers/gpu/drm/xe/xe_gt_sriov_pf.h
> new file mode 100644
> index 000000000000..05142ffc4319
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/xe_gt_sriov_pf.h
> @@ -0,0 +1,20 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2023-2024 Intel Corporation
> + */
> +
> +#ifndef _XE_GT_SRIOV_PF_H_
> +#define _XE_GT_SRIOV_PF_H_
> +
> +struct xe_gt;
> +
> +#ifdef CONFIG_PCI_IOV
> +int xe_gt_sriov_pf_init_early(struct xe_gt *gt);
> +#else
> +static inline int xe_gt_sriov_pf_init_early(struct xe_gt *gt)
> +{
> +	return 0;
> +}
> +#endif
> +
> +#endif
> diff --git a/drivers/gpu/drm/xe/xe_gt_sriov_pf_types.h b/drivers/gpu/drm/xe/xe_gt_sriov_pf_types.h
> index 768277b8bc95..223f280ef748 100644
> --- a/drivers/gpu/drm/xe/xe_gt_sriov_pf_types.h
> +++ b/drivers/gpu/drm/xe/xe_gt_sriov_pf_types.h
> @@ -10,12 +10,21 @@
>   
>   #include "xe_gt_sriov_pf_policy_types.h"
>   
> +/**
> + * struct xe_gt_sriov_metadata - GT level per-VF metadata.
> + */
> +struct xe_gt_sriov_metadata {
> +	/* XXX: VF metadata will go here */
> +};
> +
>   /**
>    * struct xe_gt_sriov_pf - GT level PF virtualization data.
>    * @policy: policy data.
> + * @vfs: metadata for all VFs.
>    */
>   struct xe_gt_sriov_pf {
>   	struct xe_gt_sriov_pf_policy policy;
> +	struct xe_gt_sriov_metadata *vfs;
>   };
>   
>   #endif

[-- Attachment #2: Type: text/html, Size: 7193 bytes --]