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 <[email protected]> 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/[email protected]?part=3
