On 6/11/2025 4:06 PM, Michael Kelley wrote:
> From: Nuno Das Neves <nunodasneves@xxxxxxxxxxxxxxxxxxx> 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/
>
Ack
>>
>> 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@xxxxxxxxxxxxxxxxxxx>
>> ---
>> 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.
>
Good point - letting the hypervisor return the error is fine.
>> +
>> /* 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.
>
I like the suggestion to open-code these cases.
There are several places we need to get/set nested MSRs, but as for hypercalls
it is just these 2, so far. When I consider it from that angle, it feels cleaner
to just open-code them, and remove the existing '_nested' versions of the
hypercall helpers for now.
Thanks
Nuno
> Michael
