Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Critical] kcalloc does not guarantee the 256-byte alignment required by the RSI host call structure, leading to RSI hypercall failures and boot crashes. - [Low] Unnecessary complex error handling and cleanup logic for an early boot memory allocation failure. --
--- Patch [3]: [PATCH v3 3/6] arm64: hyperv: Add per-CPU RSI host call infrastructure for CCA Realms --- commit d6de874ec14c2f54fe70b4639d2b20bb51fd11c9 Author: Kameron Carr <[email protected]> arm64: hyperv: Add per-CPU RSI host call infrastructure for CCA Realms This patch introduces a per-CPU array of rsi_host_call structures to support Hyper-V hypercalls in Arm CCA Realm guests. The array is allocated dynamically during hyperv_init() so that non-Realm guests pay no memory overhead. > diff --git a/arch/arm64/hyperv/mshyperv.c b/arch/arm64/hyperv/mshyperv.c > index 4fdc26ade1d74..d9010e64ddd7c 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: Critical] Does using kcalloc() violate the __aligned(256) constraint on struct rsi_host_call? If slab debugging (e.g., redzoning) or KASAN is enabled, the allocator offsets the returned pointer, breaking natural power-of-two alignment. When the unaligned IPA is passed to the RMM via SMC, it gets rejected, causing the guest to trigger a BUG_ON() inside hv_set_vpreg() during early boot. Would it be safer to use alloc_percpu() here to guarantee the required alignment natively? > + if (!hv_hostcall_array) > + return -ENOMEM; [Severity: Low] This isn't a bug, but since hyperv_init() runs early in the boot process, is it necessary to handle this memory allocation failure gracefully? At this stage, physical memory should be available, and an allocation failure typically means the system cannot boot anyway. It seems the explicit error handling and cleanup logic add unnecessary complexity. > + } > + > /* Setup the guest ID */ > guest_id = hv_generate_guest_id(LINUX_VERSION_CODE); > hv_set_vpreg(HV_REGISTER_GUEST_OS_ID, guest_id); [ ... ] > @@ -106,13 +125,13 @@ static int __init hyperv_init(void) > > ret = hv_common_init(); > if (ret) > - return ret; > + goto free_hostcall_mem; > > ret = cpuhp_setup_state(CPUHP_AP_HYPERV_ONLINE, > "arm64/hyperv_init:online", > hv_common_cpu_init, hv_common_cpu_die); > if (ret < 0) { > hv_common_free(); > - return ret; > + goto free_hostcall_mem; > } > > if (ms_hyperv.priv_high & HV_ACCESS_PARTITION_ID) > @@ -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] This isn't a bug, but following up on the previous question, could we avoid adding this entire cleanup block since this is early boot code? > } > > early_initcall(hyperv_init); -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=3
