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

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

On 15.04.2024 07:19, Ghimiray, Himal Prasad wrote:
> 
> 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.

that's correct, at least one more sub-component (PF services) will be
initialized here in the very near future

> Otherwise, return pf_alloc_metadata(gt) looks cleaner.
> 
> LGTM
> 
> Reviewed-by: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>

thanks!

> 
> 
>> +}
>> 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