Re: [PATCH v3 0/6] Hyper-V: Implement hypervisor core collection

[Mukesh R <mrathor@linux.microsoft.com>]

On 10/17/25 19:54, Michael Kelley wrote:
> From: Mukesh R <mrathor@linux.microsoft.com> Sent: Friday, October 17, 2025 4:58 PM
>>
>> On 10/17/25 15:57, Wei Liu wrote:
>>> On Fri, Oct 17, 2025 at 10:33:00PM +0000, Wei Liu wrote:
>>>> On Mon, Oct 06, 2025 at 03:42:02PM -0700, Mukesh Rathor wrote:
>>>> [...]
>>>>> Mukesh Rathor (6):
>>>>>   x86/hyperv: Rename guest crash shutdown function
>>>>>   hyperv: Add two new hypercall numbers to guest ABI public header
>>>>>   hyperv: Add definitions for hypervisor crash dump support
>>>>>   x86/hyperv: Add trampoline asm code to transition from hypervisor
>>>>>   x86/hyperv: Implement hypervisor RAM collection into vmcore
>>>>>   x86/hyperv: Enable build of hypervisor crashdump collection files
>>>>>
>>>>
>>>> Applied to hyperv-next. Thanks.
>>>
>>> This breaks i386 build.
>>>
>>> /work/linux-on-hyperv/linux.git/arch/x86/hyperv/hv_init.c: In function ?hyperv_init?:
>>> /work/linux-on-hyperv/linux.git/arch/x86/hyperv/hv_init.c:557:17: error: implicit declaration of function ?hv_root_crash_init? [-Werror=implicit-function-declaration]
>>>   557 |                 hv_root_crash_init();
>>>       |                 ^~~~~~~~~~~~~~~~~~
>>>
>>> That's because CONFIG_MSHV_ROOT is only available on x86_64. And the
>>> crash feature is guarded by CONFIG_MSHV_ROOT.
>>>
>>> Applying the following diff fixes the build.
>>
>>
>> Thanks. A bit surprising tho that CONFIG_MSHV_ROOT doesn't have
>> hard dependency on x86_64. It should, no?
> 
> CONFIG_MSHV_ROOT *does* have a hard dependency on X86_64.
> 
> But the problem is actually more pervasive than just 32-bit builds. Because
> of the hard dependency, 32-bit builds imply CONFIG_MSHV_ROOT=n, which is
> the real problem. In arch/x86/include/asm/mshyperv.h the declaration for
> hv_root_crash_init() is available only when CONFIG_MSHV_ROOT is defined
> (m or y). There's a stub hv_root_crash_init() if CONFIG_MSHV_ROOT is defined
> and CONFIG_CRASH_DUMP=n, but not if CONFIG_MSHV_ROOT=n. The solution
> is to add a stub when CONFIG_MSHV_ROOT=n, as below:
> 
> diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
> index 76582affefa8..a5b258d268ed 100644
> --- a/arch/x86/include/asm/mshyperv.h
> +++ b/arch/x86/include/asm/mshyperv.h
> @@ -248,6 +248,8 @@ void hv_crash_asm_end(void);
>  static inline void hv_root_crash_init(void) {}
>  #endif  /* CONFIG_CRASH_DUMP */
> 
> +#else   /* CONFIG_MSHV_ROOT */
> +static inline void hv_root_crash_init(void) {}
>  #endif  /* CONFIG_MSHV_ROOT */
> 
>  #else /* CONFIG_HYPERV */
> 
> Annoyingly, this solution duplicates the hv_root_crash_init() stub.  So
> an alternate approach that changes a few more lines of code is this:
> 
> diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
> index 76582affefa8..1342d55c2545 100644
> --- a/arch/x86/include/asm/mshyperv.h
> +++ b/arch/x86/include/asm/mshyperv.h
> @@ -237,18 +237,14 @@ static __always_inline u64 hv_raw_get_msr(unsigned int reg)
>  }
>  int hv_apicid_to_vp_index(u32 apic_id);
> 
> -#if IS_ENABLED(CONFIG_MSHV_ROOT)
> -
> -#ifdef CONFIG_CRASH_DUMP
> +#if IS_ENABLED(CONFIG_MSHV_ROOT) && IS_ENABLED(CONFIG_CRASH_DUMP)
>  void hv_root_crash_init(void);
>  void hv_crash_asm32(void);
>  void hv_crash_asm64(void);
>  void hv_crash_asm_end(void);
> -#else   /* CONFIG_CRASH_DUMP */
> +#else   /* CONFIG_MSHV_ROOT && CONFIG_CRASH_DUMP */
>  static inline void hv_root_crash_init(void) {}
> -#endif  /* CONFIG_CRASH_DUMP */
> -
> -#endif  /* CONFIG_MSHV_ROOT */
> +#endif  /* CONFIG_MSHV_ROOT && CONFIG_CRASH_DUMP */
> 
>  #else /* CONFIG_HYPERV */
>  static inline void hyperv_init(void) {}
> 
> Michael

Thanks. Yeah, either of the above two is ok. The latter does not
duplicate, so may be tiny bit better. Wei will pick one and apply
directly.

Thanks,
-Mukesh