On 10/17/25 19:54, Michael Kelley wrote: > From: Mukesh R <[email protected]> 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
