Re: [PATCH v2 07/15] arm64: hyperv: Add support for mshv_vtl_return_call

[Naman Jain <namjain@linux.microsoft.com>]

On 5/4/2026 9:36 PM, Michael Kelley wrote:
> From: Naman Jain <namjain@linux.microsoft.com> Sent: Wednesday, April 29, 2026 2:57 AM
>>
>> On 4/27/2026 11:08 AM, Michael Kelley wrote:
>>> From: Naman Jain <namjain@linux.microsoft.com> Sent: Thursday, April 23, 2026 5:42 AM
>>>>
> 
> [snip]
> 
>>>> diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
>>>> index 08278547b84c..b4d80c9a673a 100644
>>>> --- a/arch/x86/include/asm/mshyperv.h
>>>> +++ b/arch/x86/include/asm/mshyperv.h
>>>> @@ -286,7 +286,6 @@ struct mshv_vtl_cpu_context {
>>>>    #ifdef CONFIG_HYPERV_VTL_MODE
>>>>    void __init hv_vtl_init_platform(void);
>>>>    int __init hv_vtl_early_init(void);
>>>> -void mshv_vtl_return_call(struct mshv_vtl_cpu_context *vtl0);
>>>>    void mshv_vtl_return_call_init(u64 vtl_return_offset);
>>>>    void mshv_vtl_return_hypercall(void);
>>>>    void __mshv_vtl_return_call(struct mshv_vtl_cpu_context *vtl0);
>>>> @@ -294,7 +293,6 @@ int hv_vtl_get_set_reg(struct hv_register_assoc *regs, bool set, bool shared);
>>>>    #else
>>>>    static inline void __init hv_vtl_init_platform(void) {}
>>>>    static inline int __init hv_vtl_early_init(void) { return 0; }
>>>> -static inline void mshv_vtl_return_call(struct mshv_vtl_cpu_context *vtl0) {}
>>>>    static inline void mshv_vtl_return_call_init(u64 vtl_return_offset) {}
>>>>    static inline void mshv_vtl_return_hypercall(void) {}
>>>>    static inline void __mshv_vtl_return_call(struct mshv_vtl_cpu_context *vtl0) {}
>>>> diff --git a/drivers/hv/mshv_vtl.h b/drivers/hv/mshv_vtl.h
>>>> index a6eea52f7aa2..103f07371f3f 100644
>>>> --- a/drivers/hv/mshv_vtl.h
>>>> +++ b/drivers/hv/mshv_vtl.h
>>>> @@ -22,4 +22,7 @@ struct mshv_vtl_run {
>>>>    	char vtl_ret_actions[MSHV_MAX_RUN_MSG_SIZE];
>>>>    };
>>>>
>>>> +static_assert(sizeof(struct mshv_vtl_cpu_context) <= 1024,
>>>> +	      "struct mshv_vtl_cpu_context exceeds reserved space in struct mshv_vtl_run");
>>>> +
>>>>    #endif /* _MSHV_VTL_H */
>>>> diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
>>>> index db183c8cfb95..8cdf2a9fbdfb 100644
>>>> --- a/include/asm-generic/mshyperv.h
>>>> +++ b/include/asm-generic/mshyperv.h
>>>> @@ -396,8 +396,10 @@ static inline int hv_deposit_memory(u64 partition_id, u64 status)
>>>>
>>>>    #if IS_ENABLED(CONFIG_HYPERV_VTL_MODE)
>>>>    u8 __init get_vtl(void);
>>>> +void mshv_vtl_return_call(struct mshv_vtl_cpu_context *vtl0);
>>>>    #else
>>>>    static inline u8 get_vtl(void) { return 0; }
>>>> +static inline void mshv_vtl_return_call(struct mshv_vtl_cpu_context *vtl0) {}
>>>
>>> Is this stub needed? Maybe I missed something, but it looks to me like none
>>> of the code that calls this gets built unless CONFIG_HYPERV_VTL_MODE is set.
>>> See further comments about stubs in Patch 8 of this series.
>>>
>>
>> Config dependencies would handle such cases, and this is not required. I
>> saw similar stubs added in the code, so I thought this is a norm that
>> should be followed, and not rely on config dependencies.
>> I can remove it.
>>
> 
> Others might disagree with me, but I don't think it's the norm to add
> stubs when they aren't truly needed. As you can see from some of my
> other comments, I look for ways to eliminate stubs. Stubs are indicative
> of a boundary between separately built components, and I generally
> try to minimize the surface area of such boundaries. A large surface area
> often means that the overall design could be improved by re-thinking
> which code goes with which component.
> 
> Michael

I agree. I'll remove these in next version.

Thanks,
Naman