Re: [PATCH v4 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 2 potential issue(s) to consider:
- [High] arm64: hyperv: kcalloc does not guarantee 256-byte alignment for rsi_host_call
- [Low] arm64: hyperv: unnecessary early boot memory allocation error handling
--

commit ef58d4d121fbefca0e2e24ac57c717abd6fc93ba
Author: Kameron Carr <kameroncarr@linux.microsoft.com>
arm64: hyperv: Add per-CPU RSI host call infrastructure for CCA Realms

This commit allocates a per-CPU array of rsi_host_call structures in
hyperv_init() for Realm guests. The array is used to route hypercalls
through the RSI_HOST_CALL interface since Arm CCA Realms cannot issue
Hyper-V hypercalls via HVC.

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

[ ... ]

> @@ -85,6 +90,20 @@ static int __init hyperv_init(void)
>  	if (!hyperv_detect_via_acpi() && !hyperv_detect_via_smccc())
>  		return 0;
>  
> +	/*
> +	 * The RSI host-call buffers are only ever used when
> +	 * is_realm_world() is true. Skip the allocation on non-Realm
> +	 * guests. A single contiguous array of nr_cpu_ids entries is
> +	 * allocated; each CPU indexes into it by its processor ID.
> +	 */
> +	if (is_realm_world()) {
> +		hv_hostcall_array = kcalloc(nr_cpu_ids,
> +					    sizeof(struct rsi_host_call),
> +					    GFP_KERNEL);

[Severity: High]
Does kcalloc guarantee the 256-byte alignment required by the RMM ABI for
the RSI_HOST_CALL structure?

When hv_do_rsi_hypercall() passes
virt_to_phys(&hv_hostcall_array[smp_processor_id()]) to rsi_host_call(),
the RMM might reject the hypercall if the physical address is unaligned,
which would break Realm guest booting.

The kcalloc API only guarantees ARCH_KMALLOC_MINALIGN (e.g., 8 or 16 bytes)
and does not respect the __aligned(256) attribute of the type. Furthermore,
when CONFIG_SLUB_DEBUG is enabled, redzones will shift the object pointer
away from natural alignment, ensuring that the 256-byte alignment
requirement is violated.

> +		if (!hv_hostcall_array)
> +			return -ENOMEM;

[Severity: Low]
Is returning -ENOMEM here necessary?

Because hyperv_init() is executed via early_initcall, physical memory
should be available. Complex error handling, cleanup logic, or returning
-ENOMEM in these early boot functions is often unnecessary dead code,
as an allocation failure typically means the system cannot boot anyway.

> +	}
>  
>  	/* Setup the guest ID */
>  	guest_id = hv_generate_guest_id(LINUX_VERSION_CODE);

[ ... ]

> @@ -125,6 +144,11 @@ static int __init hyperv_init(void)
>  
>  	hyperv_initialized = true;
>  	return 0;
> +
> +free_hostcall_mem:
> +	kfree(hv_hostcall_array);
> +	hv_hostcall_array = NULL;
> +	return ret;

[Severity: Low]
Can we omit this explicit cleanup path?

As noted above, executing cleanup logic for failures that are fatal
during early boot adds unnecessary code complexity.

>  }
>  
>  early_initcall(hyperv_init);

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