Re: [PATCH i-g-t 2/5] lib/xe/xe_sriov_provisioning: Define resource types and provisioned range structure

["Bernatowicz, Marcin" <marcin.bernatowicz@linux.intel.com>]

On 11/5/2024 12:21 PM, Adam Miszczak wrote:
> 
> On 30.10.2024 20:36, Marcin Bernatowicz wrote:
>> Add shared resource enumeration, provisioned range structure definition,
>> and provide a function to convert shared resource enums to their string
>> representation. These types will be used in a subsequent patch to read
>> VF provisioned resources from the debug filesystem.
>>
>> Signed-off-by: Marcin Bernatowicz<marcin.bernatowicz@linux.intel.com>
>> Cc: Adam Miszczak<adam.miszczak@linux.intel.com>
>> Cc: Jakub Kolakowski<jakub1.kolakowski@intel.com>
>> Cc: Marcin Bernatowicz<marcin.bernatowicz@linux.intel.com>
>> Cc: Michał Wajdeczko<michal.wajdeczko@intel.com>
>> Cc: Michał Winiarski<michal.winiarski@intel.com>
>> Cc: Narasimha C V<narasimha.c.v@intel.com>
>> Cc: Piotr Piórkowski<piotr.piorkowski@intel.com>
>> Cc: Satyanarayana K V P<satyanarayana.k.v.p@intel.com>
>> Cc: Tomasz Lis<tomasz.lis@intel.com>
>> ---
>>   lib/meson.build                |  1 +
>>   lib/xe/xe_sriov_provisioning.c | 33 +++++++++++++++++++++++++
>>   lib/xe/xe_sriov_provisioning.h | 45 ++++++++++++++++++++++++++++++++++
>>   3 files changed, 79 insertions(+)
>>   create mode 100644 lib/xe/xe_sriov_provisioning.c
>>   create mode 100644 lib/xe/xe_sriov_provisioning.h
>>
>> diff --git a/lib/meson.build b/lib/meson.build
>> index 3d5d68b75..3d459c7a3 100644
>> --- a/lib/meson.build
>> +++ b/lib/meson.build
>> @@ -117,6 +117,7 @@ lib_sources = [
>>   	'xe/xe_query.c',
>>   	'xe/xe_spin.c',
>>   	'xe/xe_sriov_debugfs.c',
>> +	'xe/xe_sriov_provisioning.c',
>>   	'xe/xe_util.c',
>>   ]
>>   
>> diff --git a/lib/xe/xe_sriov_provisioning.c b/lib/xe/xe_sriov_provisioning.c
>> new file mode 100644
>> index 000000000..6a9ad411a
>> --- /dev/null
>> +++ b/lib/xe/xe_sriov_provisioning.c
>> @@ -0,0 +1,33 @@
>> +// SPDX-License-Identifier: MIT
>> +/*
>> + * Copyright(c) 2024 Intel Corporation. All rights reserved.
>> + */
>> +
>> +#include <stdlib.h>
>> +
>> +#include "xe/xe_sriov_provisioning.h"
>> +
>> +/**
>> + * xe_sriov_shared_res_to_string:
>> + * @key: The shared resource of type enum xe_sriov_shared_res
>> + *
>> + * Converts a shared resource enum to its corresponding string
>> + * representation. It is useful for logging and debugging purposes.
>> + *
>> + * Return: A string representing the shared resource key.
>> + */
>> +const char *xe_sriov_shared_res_to_string(enum xe_sriov_shared_res res)
>> +{
>> +	switch (res) {
>> +	case XE_SRIOV_SHARED_RES_CONTEXTS:
>> +		return "contexts";
>> +	case XE_SRIOV_SHARED_RES_DOORBELLS:
>> +		return "doorbells";
>> +	case XE_SRIOV_SHARED_RES_GGTT:
>> +		return "ggtt";
>> +	case XE_SRIOV_SHARED_RES_LMEM:
>> +		return "lmem";
>> +	}
>> +
>> +	return NULL;
>> +}
>> diff --git a/lib/xe/xe_sriov_provisioning.h b/lib/xe/xe_sriov_provisioning.h
>> new file mode 100644
>> index 000000000..7b7b3db90
>> --- /dev/null
>> +++ b/lib/xe/xe_sriov_provisioning.h
>> @@ -0,0 +1,45 @@
>> +/* SPDX-License-Identifier: MIT */
>> +/*
>> + * Copyright(c) 2024 Intel Corporation. All rights reserved.
>> + */
>> +
>> +#ifndef __XE_SRIOV_PROVISIONING_H__
>> +#define __XE_SRIOV_PROVISIONING_H__
>> +
>> +#include <stdint.h>
>> +
>> +/**
>> + * enum xe_sriov_shared_res - Shared resource types
>> + * @XE_SRIOV_SHARED_RES_CONTEXTS: Contexts
>> + * @XE_SRIOV_SHARED_RES_DOORBELLS: Doorbells
>> + * @XE_SRIOV_SHARED_RES_GGTT: GGTT (Global Graphics Translation Table)
>> + * @XE_SRIOV_SHARED_RES_LMEM: Local memory
>> + *
>> + * This enumeration defines the types of shared resources
>> + * that can be provisioned to Virtual Functions (VFs).
>> + */
>> +enum xe_sriov_shared_res {
>> +	XE_SRIOV_SHARED_RES_CONTEXTS,
>> +	XE_SRIOV_SHARED_RES_DOORBELLS,
>> +	XE_SRIOV_SHARED_RES_GGTT,
>> +	XE_SRIOV_SHARED_RES_LMEM,
>> +};
>> +
>> +/**
>> + * struct xe_sriov_provisioned_range - Provisioned range for a Virtual Function (VF)
>> + * @vf_id: The ID of the VF
>> + * @start: The inclusive start of the provisioned range
>> + * @end: The inclusive end of the provisioned range
>> + *
>> + * This structure represents a range of resources that have been provisioned
>> + * for a specific VF, with both start and end values included in the range.
>> + */
>> +struct xe_sriov_provisioned_range {
>> +	unsigned int vf_id;
>> +	uint64_t start;
>> +	uint64_t end;
>> +};
>> +
> 
> Debugfs *_provisioned attributes carry also some additional data (e.g. 
> number of ctxs or ggtt size) which is not covered here.
> I understand it's not used by tests now, but assume this could be 
> extended if there's such demand (e.g. some test needs it), right?

But number of ctxs or ggtt size can be computed from range, or I miss 
something ?

number of ctxs = end - start + 1
ggtt size = end - start + 1

> 
>> +const char *xe_sriov_shared_res_to_string(enum xe_sriov_shared_res res);
> Is this really required as an interface? Is this used anywhere?

To have human readable shared resource name for debug purposes, as part 
of test name etc.

>> +
>> +#endif /* __XE_SRIOV_PROVISIONING_H__ */
> 
> Overall, LGTM:
> Reviewed-by: Adam Miszczak <adam.miszczak@linux.intel.com>
> 
>