Re: [PATCH v2 4/4] xen/cppc: introduce cppc data upload sub-hypercall

[Jason Andryuk <jason.andryuk@amd.com>]

On 2024-12-05 00:42, Penny Zheng wrote:
> As Xen is uncapable of parsing the ACPI dynamic table, this commit
> introduces a new sub-hypercall XEN_PM_CPPC to deliver CPPC perf
> caps data.
> 
> Signed-off-by: Penny Zheng <Penny.Zheng@amd.com>

> @@ -208,6 +213,44 @@ static int xen_copy_pct_data(struct acpi_pct_register *pct,
>   	dst_pct->address = pct->address;
>   	return 0;
>   }
> +static int push_cppc_to_hypervisor(struct acpi_processor *_pr)
> +{
> +	int ret = 0;
> +	struct xen_platform_op op = {
> +		.cmd            = XENPF_set_processor_pminfo,
> +		.interface_version  = XENPF_INTERFACE_VERSION,
> +		.u.set_pminfo.id    = _pr->acpi_id,
> +		.u.set_pminfo.type  = XEN_PM_CPPC,
> +	};
> +	struct cppc_perf_caps *cppc_perf = acpi_cppc_data + _pr->acpi_id;
> +
> +	op.u.set_pminfo.cppc_data.highest_perf = cppc_perf->highest_perf;
> +	op.u.set_pminfo.cppc_data.lowest_perf = cppc_perf->lowest_perf;
> +	op.u.set_pminfo.cppc_data.nominal_perf = cppc_perf->nominal_perf;
> +	op.u.set_pminfo.cppc_data.lowest_nonlinear_perf = cppc_perf->lowest_nonlinear_perf;
> +	op.u.set_pminfo.cppc_data.lowest_freq = cppc_perf->lowest_freq;
> +	op.u.set_pminfo.cppc_data.nominal_freq = cppc_perf->nominal_freq;
> +
> +	if (!no_hypercall)
> +		ret = HYPERVISOR_platform_op(&op);
> +
> +	if (!ret) {
> +		pr_debug("ACPI CPU%u - CPPC uploaded.\n", _pr->acpi_id);
> +		pr_debug("     highest_perf: %d\n", cppc_perf->highest_perf);
> +		pr_debug("     lowest_perf: %d\n", cppc_perf->lowest_perf);
> +		pr_debug("     lowest_nonlinear_perf: %d\n", cppc_perf->lowest_nonlinear_perf);
> +		pr_debug("     nominal_perf: %d\n", cppc_perf->nominal_perf);
> +		pr_debug("     lowest_freq: %d Mhz\n", cppc_perf->lowest_freq);
> +		pr_debug("     nominal_freq: %d Mhz\n", cppc_perf->nominal_freq);
> +	} else if ((ret != -EINVAL) && (ret != -ENOSYS))
> +		/* EINVAL means the ACPI ID is incorrect - meaning the ACPI
> +		 * table is referencing a non-existing CPU - which can happen
> +		 * with broken ACPI tables. */
> +		pr_warn("(_CPC): Hypervisor error (%d) for ACPI CPU%u\n",
> +			ret, _pr->acpi_id);
> +
> +		return ret;

I just saw gcc warn about misleading indent here given the lack of { } 
on else if above.  I think you just want a single tab indent for return.

Regards,
Jason

> +}
>   static int push_pxx_to_hypervisor(struct acpi_processor *_pr)
>   {
>   	int ret = 0;