Thank you for your contribution! Sashiko AI review found 3 potential issue(s) 
to consider:
- [High] The globally allocated `hv_hostcall_array` is used for RSI host calls, 
but it is not guaranteed to be 256-byte aligned, which is strictly required by 
the RMM.
- [High] Using a single shared per-CPU buffer for hypercalls without masking 
NMIs/FIQs destroys re-entrancy, risking data corruption.
- [High] Hypercalls passing memory buffers will fail because the guest passes 
Protected IPAs to the host instead of Shared IPAs.
--

--- Patch [5]: [PATCH v3 5/6] arm64: hyperv: Route hypercalls through RSI host 
call in CCA Realms ---
commit a89ea311002c08edfbe0b6f9d1e3ca3c6d94b198
Author: Kameron Carr <[email protected]>

arm64: hyperv: Route hypercalls through RSI host call in CCA Realms

This patch modifies the hypercall wrapper functions to check for
is_realm_world(). When running inside a Realm, it routes the hypercalls
through the RSI interface using a per-CPU rsi_host_call structure.

> diff --git a/arch/arm64/hyperv/hv_core.c b/arch/arm64/hyperv/hv_core.c
> index e33a9e3c366a1..77cba08fca132 100644
> --- a/arch/arm64/hyperv/hv_core.c
> +++ b/arch/arm64/hyperv/hv_core.c

[ ... ]

> +static u64 hv_do_rsi_hypercall(u64 control, u64 input1, u64 input2)
> +{
> +     struct rsi_host_call *hostcall;
> +     unsigned long flags;
> +     u64 ret;
> +
> +     if (!hv_hostcall_array)
> +             return HV_STATUS_INVALID_HYPERCALL_INPUT;
> +
> +     local_irq_save(flags);
> +     hostcall = &hv_hostcall_array[smp_processor_id()];

[Severity: High]
Since local_irq_save() doesn't mask FIQs or pseudo-NMIs on ARM64, is this 
shared per-CPU buffer safe from re-entrancy?

If a pseudo-NMI like a perf interrupt or hardware watchdog interrupts an 
ongoing hypercall and issues a hypercall itself (e.g., hyperv_report_panic 
calling hv_set_vpreg), wouldn't it overwrite this same hostcall buffer and 
corrupt the state?

> +     memset(hostcall, 0, sizeof(*hostcall));
> +     hostcall->gprs[0] = HV_FUNC_ID;
> +     hostcall->gprs[1] = control;
> +     hostcall->gprs[2] = input1;
> +     hostcall->gprs[3] = input2;
> +
> +     if (rsi_host_call(virt_to_phys(hostcall)) == RSI_SUCCESS)

[Severity: High]
Does the allocation of hv_hostcall_array guarantee the 256-byte alignment 
required by the RMM ABI?

If it is allocated using kcalloc() during init, that only guarantees 
alignment up to ARCH_KMALLOC_MINALIGN. When slab debugging like SLUB_DEBUG 
redzoning is enabled, the objects can be shifted by redzone padding, 
completely breaking the 256-byte alignment and causing the RMM to reject the 
call.

> +             ret = hostcall->gprs[0];
> +     else
> +             ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
> +
> +     local_irq_restore(flags);
> +     return ret;
> +}

[ ... ]

> @@ -29,8 +61,11 @@ u64 hv_do_hypercall(u64 control, void *input, void *output)
>       input_address = input ? virt_to_phys(input) : 0;
>       output_address = output ? virt_to_phys(output) : 0;
>  
> -     arm_smccc_1_1_hvc(HV_FUNC_ID, control,
> -                       input_address, output_address, &res);
> +     if (is_realm_world())
> +             return hv_do_rsi_hypercall(control, input_address, 
> output_address);

[Severity: High]
Will the Hyper-V host be able to access these input and output buffers?

In a CCA Realm, the physical address space is divided by the prot_ns_shared 
bit, and virt_to_phys() returns a Protected IPA without this bit set.

If the host requires access to these buffers, does this need to pass a 
Shared IPA instead of the base Protected IPA?

> +
> +     arm_smccc_1_1_hvc(HV_FUNC_ID, control, input_address,
> +                       output_address, &res);
>       return res.a0;
>  }

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=5

Reply via email to