Re: [RFC PATCH 3/6] arm64: hyperv: Add per-CPU RSI host call infrastructure for CCA Realms

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The generic hypercall wrappers (`hv_do_hypercall`, `hv_do_fast_hypercall8`, `hv_do_fast_hypercall16`) were not updated to use the RSI_HOST_CALL interface, breaking hypercalls for CCA Realms.
- [High] Missing NULL pointer check for the dynamically allocated per-CPU `hostcall` buffer in `hv_set_vpreg()` and `hv_get_vpreg_128()` can cause a recursive crash during a panic.
- [Low] Complex error handling and cleanup logic for early boot memory allocation failures violates subsystem guidelines.
--

commit 286a7465cb0cf6433c1d0e69d74a2bf4a0c1a935
Author: Kameron Carr <kameroncarr@linux.microsoft.com>

arm64: hyperv: Add per-CPU RSI host call infrastructure for CCA Realms

This patch introduces the hyperv_pcpu_hostcall_struct to support Arm CCA
Realms, which must route hypercalls through the RSI_HOST_CALL interface.
It allocates this per-CPU structure for the boot CPU and secondary CPUs.

> Arm CCA Realms cannot issue Hyper-V hypercalls via HVC; the guest must
> route them through the RSI_HOST_CALL interface, which takes the IPA of a
> per-CPU rsi_host_call structure as its argument.

[Severity: High]
Does this patch miss updating the generic hypercall wrappers?

Looking at hv_do_hypercall() in arch/arm64/hyperv/hv_core.c, it still issues
an HVC instruction unconditionally via arm_smccc_1_1_hvc().

Since these generic functions are heavily used throughout the Hyper-V stack
(including early initialization like hv_get_partition_id()), could a CCA
Realm guest crash when it inadvertently executes the trapped HVC instruction?

> diff --git a/arch/arm64/hyperv/mshyperv.c b/arch/arm64/hyperv/mshyperv.c
> index 4fdc26ade1d74..08fec82691683 100644
> --- a/arch/arm64/hyperv/mshyperv.c
> +++ b/arch/arm64/hyperv/mshyperv.c

[ ... ]

> @@ -60,6 +66,46 @@ static bool __init hyperv_detect_via_acpi(void)
>  
>  #endif
>  
> +static void hv_hostcall_free(void)
> +{
> +	int cpu;
> +
> +	if (!hyperv_pcpu_hostcall_struct)
> +		return;
> +
> +	for_each_possible_cpu(cpu)
> +		kfree(*per_cpu_ptr(hyperv_pcpu_hostcall_struct, cpu));
> +	free_percpu(hyperv_pcpu_hostcall_struct);
> +	hyperv_pcpu_hostcall_struct = NULL;
> +}
> +
> +static int hv_cpu_init(unsigned int cpu)
> +{
> +	void **hostcall_struct;
> +	gfp_t flags;
> +	void *mem;
> +
> +	if (hyperv_pcpu_hostcall_struct) {
> +		/* hv_cpu_init() can be called with IRQs disabled from hv_resume() */
> +		flags = irqs_disabled() ? GFP_ATOMIC : GFP_KERNEL;
> +
> +		hostcall_struct = (void **)this_cpu_ptr(hyperv_pcpu_hostcall_struct);
> +		/*
> +		 * The hostcall_struct memory is not freed when the CPU
> +		 * goes offline. If a previously offlined CPU is brought
> +		 * back online, the memory is reused here.
> +		 */
> +		if (!*hostcall_struct) {
> +			mem = kzalloc_obj(struct rsi_host_call, flags);
> +			if (!mem)
> +				return -ENOMEM;
> +			*hostcall_struct = mem;
> +		}

[Severity: High]
Is it safe to assume this pointer is always initialized during a panic?

If a secondary CPU panics before or during its bringup phase, its per-CPU
hostcall_struct pointer remains NULL.

The globally registered Hyper-V panic notifier runs on the panicking CPU and
calls hv_set_vpreg() and hv_get_vpreg_128(). These functions unconditionally
fetch the local pointer and perform an unprotected memset(hostcall, 0,
sizeof(*hostcall)).

Additionally, if cpuhp_setup_state() fails later during initialization,
hv_hostcall_free() clears hyperv_pcpu_hostcall_struct to NULL. Since
hv_common_free() does not unregister the panic notifiers, a subsequent
panic would cause this_cpu_ptr(NULL) to evaluate to __my_cpu_offset.

Could doing a memset() on these uninitialized pointers trigger a double
fault in the panic handler or memory corruption, losing the crash report
entirely?

> +	}
> +
> +	return hv_common_cpu_init(cpu);
> +}

[ ... ]

> @@ -85,6 +133,27 @@ static int __init hyperv_init(void)
>  	if (!hyperv_detect_via_acpi() && !hyperv_detect_via_smccc())
>  		return 0;
>  
> +	/*
> +	 * The RSI host-call buffer is only ever used when
> +	 * is_realm_world() is true. Skip the per-CPU allocation on
> +	 * non-Realm guests.
> +	 */
> +	if (is_realm_world()) {
> +		hyperv_pcpu_hostcall_struct = alloc_percpu(void *);
> +		if (!hyperv_pcpu_hostcall_struct)
> +			return -ENOMEM;
> +
> +		hostcall_struct = (void **)this_cpu_ptr(hyperv_pcpu_hostcall_struct);
> +		if (!*hostcall_struct) {
> +			mem = kzalloc_obj(struct rsi_host_call);
> +			if (!mem) {
> +				ret = -ENOMEM;
> +				goto free_hostcall_mem;

[Severity: Low]
Is this complex error handling and cleanup logic necessary here?

Since hyperv_init() is an early_initcall, physical memory should be
available. A failure at this stage typically implies the system cannot
boot anyway, making this teardown logic unnecessary dead code.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260609181030.2378391-1-kameroncarr@linux.microsoft.com?part=3