Re: [PATCH v5 4/4] arm64: paravirt: Enable errata based on implementation CPUs

[Oliver Upton <oliver.upton@linux.dev>]

On Fri, Jan 24, 2025 at 03:17:32PM +0000, Shameer Kolothum wrote:
> +void  __init pv_target_impl_cpu_init(void)

I get the sense that this implementation should actually be part of the
KVM guest services driver in drivers/firmware/smccc/kvm_guest.c

> +{
> +	int i;
> +	unsigned long max_cpus;
> +	struct arm_smccc_res res;
> +	const u32 funcs[] = {
> +		ARM_SMCCC_KVM_FUNC_DISCOVER_IMPL_VER,
> +		ARM_SMCCC_KVM_FUNC_DISCOVER_IMPL_CPUS,
> +	};
> +
> +	/* Check we have already set targets */
> +	if (target_impl_cpu_num)
> +		return;
> +
> +	for (i = 0; i < ARRAY_SIZE(funcs); ++i) {
> +		if (!kvm_arm_hyp_service_available(funcs[i]))
> +			return;
> +	}

nit: drop the for loop and just use the literal values.

> +	arm_smccc_1_1_invoke(ARM_SMCCC_VENDOR_HYP_KVM_DISCOVER_IMPL_VER_FUNC_ID,
> +			     0, &res);
> +	if (res.a0 != SMCCC_RET_SUCCESS)
> +		return;
> +
> +	if (res.a1 != ARM_SMCCC_KVM_DISCOVER_IMPL_VER_1_0 || !res.a2) {

This is brittle. The versioning scheme really should match that of PSCI
/ SMCCC, where minor revisions within the same major are backwards
compatible with older clients.

So consider just checking that the major version is 1.

> +		pr_warn("Unsupported target impl version or CPU implementations\n");

This would be more useful if you provide context (i.e. the detected
version).

-- 
Thanks,
Oliver