Re: [PATCH v7 04/12] platform/x86: hfi: Introduce AMD Hardware Feedback Interface Driver

[Mario Limonciello <mario.limonciello@amd.com>]

On 12/2/2024 05:35, Peter Zijlstra wrote:
> On Sat, Nov 30, 2024 at 08:06:55AM -0600, Mario Limonciello wrote:
> 
>> +/**
>> + * struct amd_hfi_cpuinfo - HFI workload class info per CPU
>> + * @cpu:		cpu index
>> + * @cpus:		mask of cpus associated with amd_hfi_cpuinfo
>> + * @class_index:	workload class ID index
>> + * @nr_class:		max number of workload class supported
>> + * @amd_hfi_classes:	current cpu workload class ranking data
>> + *
>> + * Parameters of a logical processor linked with hardware feedback class
>> + */
>> +struct amd_hfi_cpuinfo {
>> +	int		cpu;
>> +	cpumask_var_t	cpus;
> 
> This appears unused.
> 
>> +	s16		class_index;
>> +	u8		nr_class;
>> +	struct amd_hfi_classes	*amd_hfi_classes;
>> +};
>> +
>> +static DEFINE_PER_CPU(struct amd_hfi_cpuinfo, amd_hfi_cpuinfo) = {.class_index = -1};
>> +
>> +static int amd_hfi_alloc_class_data(struct platform_device *pdev)
>> +{
>> +	struct amd_hfi_cpuinfo *hfi_cpuinfo;
>> +	struct device *dev = &pdev->dev;
>> +	int idx;
>> +	int nr_class_id;
>> +
>> +	nr_class_id = cpuid_eax(AMD_HETERO_CPUID_27);
>> +	if (nr_class_id < 0 || nr_class_id > 255) {
>> +		dev_err(dev, "failed to get number of supported classes: %d\n",
>> +			nr_class_id);
>> +		return -EINVAL;
>> +	}
>> +
>> +	for_each_present_cpu(idx) {
> 
> This uses present, but does not have means of handling changes to
> present. Does this want to be possible?
> 
Thanks, I'll adjust this to *_possible().

I think the *_present() calls make sense for the other consumers in 
suspend/resume though. Agree?