RE: [PATCH v8 4/6] arm64: hyperv: Initialize hypervisor on boot

From: Sunil Muthuswamy
Date: Wed Mar 03 2021 - 19:03:41 EST


> +static u64 hypercall_output __initdata;
> +
> +static int __init hyperv_init(void)
> +{
> + struct hv_get_vpindex_from_apicid_input *input;
> + u64 status;
> + int i;
nit: both, tabs & spaces are being used to indent variable names. Can we stick
to one?

> +
> + /*
> + * Hypercall inputs must not cross a page boundary, so allocate
> + * power of 2 size, which will be aligned to that size.
> + */
> + input = kzalloc(roundup_pow_of_two(sizeof(input->header) +
> + sizeof(input->element[0])), GFP_KERNEL);
> + if (!input)
> + return -ENOMEM;
Similar to the comment on the other patch, can this be done through a
per-cpu hypercall input page?

> + if ((status & HV_HYPERCALL_RESULT_MASK) == HV_STATUS_SUCCESS)
> + hv_vp_index[i] = hypercall_output;
> + else {
CNF suggests using braces even for single line 'if' statements if the 'else' is a multi-line
statement

> + pr_warn("Hyper-V: No VP index for CPU %d MPIDR %llx status %llx\n",
> + i, cpu_logical_map(i), status);
> + hv_vp_index[i] = VP_INVAL;
> + }

> +bool hv_is_hyperv_initialized(void)
> +{
> + return hyperv_initialized;
> +}
> +EXPORT_SYMBOL_GPL(hv_is_hyperv_initialized);

This to me seems like more of an indication that whether the hypervisor is Hyper-V or
something else, rather than if necessary Hyper-V pieces have been initialized. If that is
the case, suggest renaming. Maybe just call it 'hv_is_hyperv'?