RE: [PATCH 2/4] Drivers: hv: Use nested hypercall for post message and signal event

Michael Kelley
2025-06-11T23:06:54Z

From: Nuno Das Neves <nunodasneves@linux.microsoft.com> Sent: Tuesday, June 10, 2025 4:52 PM
>
> When running nested, these hypercalls must be sent to the L0 hypervisor
> or vmbus will fail.

s/vmbus/VMBus/

>
> Add ARM64 stubs for the nested hypercall helpers to not break
> compilation (nested is still only supported in x86).
>
> Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
> ---
> arch/arm64/include/asm/mshyperv.h | 10 ++++++++++
> drivers/hv/connection.c | 3 +++
> drivers/hv/hv.c | 3 +++
> 3 files changed, 16 insertions(+)
>
> diff --git a/arch/arm64/include/asm/mshyperv.h
> b/arch/arm64/include/asm/mshyperv.h
> index b721d3134ab6..893d6a2e8dab 100644
> --- a/arch/arm64/include/asm/mshyperv.h
> +++ b/arch/arm64/include/asm/mshyperv.h
> @@ -53,6 +53,16 @@ static inline u64 hv_get_non_nested_msr(unsigned int reg)
> return hv_get_msr(reg);
> }
>
> +static inline u64 hv_do_nested_hypercall(u64 control, void *input, void *output)
> +{
> + return U64_MAX;
> +}
> +
> +static inline u64 hv_do_fast_nested_hypercall8(u64 control, u64 input1)
> +{
> + return U64_MAX;
> +}

I think the definitions of hv_do_nested_hypercall() and
hv_do_fast_nested_hypercall8() are architecture independent. All
they do is add the HV_HYPERCALL_NESTED flag, which when
implemented for ARM64, will presumably be the same flag as
currently defined for x86. As such, couldn't the definitions of
hv_do_nested_hypercall() and hv_do_fast_nested_hypercall8()
be moved to asm-generic/mshyperv.h? Then stubs would not
be needed for ARM64. These two functions would never be
called on ARM64 because hv_nested is never true on ARM64
(at least for now), but the code would compile. And if either
function was erroneously called on ARM64, presumably
Hyper-V would return an error because HV_HYPERCALL_NESTED
is set.

> +
> /* SMCCC hypercall parameters */
> #define HV_SMCCC_FUNC_NUMBER 1
> #define HV_FUNC_ID ARM_SMCCC_CALL_VAL( \
> diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
> index be490c598785..992022bc770c 100644
> --- a/drivers/hv/connection.c
> +++ b/drivers/hv/connection.c
> @@ -518,6 +518,9 @@ void vmbus_set_event(struct vmbus_channel *channel)
> channel->sig_event, 0);
> else
> WARN_ON_ONCE(1);
> + } else if (hv_nested) {
> + hv_do_fast_nested_hypercall8(HVCALL_SIGNAL_EVENT,
> + channel->sig_event);
> } else {
> hv_do_fast_hypercall8(HVCALL_SIGNAL_EVENT, channel->sig_event);
> }
> diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
> index 308c8f279df8..99b73e779bf0 100644
> --- a/drivers/hv/hv.c
> +++ b/drivers/hv/hv.c
> @@ -84,6 +84,9 @@ int hv_post_message(union hv_connection_id connection_id,
> sizeof(*aligned_msg));
> else
> status = HV_STATUS_INVALID_PARAMETER;
> + } else if (hv_nested) {
> + status = hv_do_nested_hypercall(HVCALL_POST_MESSAGE,
> + aligned_msg, NULL);
> } else {
> status = hv_do_hypercall(HVCALL_POST_MESSAGE,
> aligned_msg, NULL);

Are HVCALL_SIGNAL_EVENT and HVCALL_POST_MESSAGE the only two
hypercalls that are ever expected to need a "nested" version? I'm
wondering if the function hv_do_nested_hypercall() and
hv_do_fast_nested_hypercall8() could be dropped entirely, and just
pass the first argument to hv_do_hypercall() or hv_do_fast_hypercall8()
as <hypercall_name> | HV_HYPERCALL_NESTED. For only two cases, a
little bit of open coding might be preferable to the overhead of defining
functions just to wrap the or'ing of HV_HYPERCALL_NESTED.

The code above could then look like:

} else {
u64 control = HVCALL_POST_MESSAGE;

control |= hv_nested ? HV_HYPERCALL_NESTED : 0;
status = hv_do_hypercall(control, aligned_msg, NULL);
}

Again, ARM64 is implicitly handled because hv_nested is never set.

This is just a suggestion. It's motivated by the fact that we already have
three flavors of hypercall for HVCALL_SIGNAL_EVENT and
HVCALL_POST_MESSAGE, and I was looking for a way to avoid adding
a fourth flavor. But it's a marginal win, and if you prefer to keep the
inline functions, I'm OK with that.

Michael