Re: [PATCH 1/1] X86: mshyperv.c: Fix a compilation issue.

Re: [PATCH 1/1] X86: mshyperv.c: Fix a compilation issue.

[Greg KH]

On Thu, Aug 06, 2015 at 02:41:24PM -0700, K. Y. Srinivasan wrote:
> Building with a random configuration file, this build failure
> was reported:
> 
> arch/x86/built-in.o: In function `hv_machine_crash_shutdown':
> arch/x86/kernel/cpu/mshyperv.c:112: undefined
> reference to `native_machine_crash_shutdown'
> 
> This patch fixes the problem
> 
> Reported-by: Jim Davis <jim.epost@gmail.com>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> ---
>  arch/x86/kernel/cpu/mshyperv.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
> index f794bfa..0faed5e0 100644
> --- a/arch/x86/kernel/cpu/mshyperv.c
> +++ b/arch/x86/kernel/cpu/mshyperv.c
> @@ -109,7 +109,9 @@ static void hv_machine_crash_shutdown(struct pt_regs *regs)
>  {
>  	if (hv_crash_handler)
>  		hv_crash_handler(regs);
> +#ifdef CONFIG_KEXEC_CORE
>  	native_machine_crash_shutdown(regs);
> +#endif

Why is kexec the factor here?  And if it really does, can't it just be
CONFIG_KEXEC, or, can kexec provide a "dummy" inline function so that
you don't have to have a #ifdef in a .c file?

thanks,

greg k-h

[PATCH 1/1] X86: mshyperv.c: Fix a compilation issue.

[K. Y. Srinivasan]

Building with a random configuration file, this build failure
was reported:

arch/x86/built-in.o: In function `hv_machine_crash_shutdown':
arch/x86/kernel/cpu/mshyperv.c:112: undefined
reference to `native_machine_crash_shutdown'

This patch fixes the problem

Reported-by: Jim Davis <jim.epost@gmail.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 arch/x86/kernel/cpu/mshyperv.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
index f794bfa..0faed5e0 100644
--- a/arch/x86/kernel/cpu/mshyperv.c
+++ b/arch/x86/kernel/cpu/mshyperv.c
@@ -109,7 +109,9 @@ static void hv_machine_crash_shutdown(struct pt_regs *regs)
 {
 	if (hv_crash_handler)
 		hv_crash_handler(regs);
+#ifdef CONFIG_KEXEC_CORE
 	native_machine_crash_shutdown(regs);
+#endif
 }
 
 
-- 
1.7.4.1

RE: [PATCH 1/1] X86: mshyperv.c: Fix a compilation issue.

[KY Srinivasan]

> -----Original Message-----
> From: Greg KH [mailto:gregkh@linuxfoundation.org]
> Sent: Thursday, August 6, 2015 2:11 PM
> To: KY Srinivasan <kys@microsoft.com>
> Cc: linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> vkuznets@redhat.com; sfr@canb.auug.org.au; tglx@linutronix.de;
> mingo@redhat.com; hpa@zytor.com; x86@kernel.org; Haiyang Zhang
> <haiyangz@microsoft.com>
> Subject: Re: [PATCH 1/1] X86: mshyperv.c: Fix a compilation issue.
> 
> On Thu, Aug 06, 2015 at 02:41:24PM -0700, K. Y. Srinivasan wrote:
> > Building with a random configuration file, this build failure
> > was reported:
> >
> > arch/x86/built-in.o: In function `hv_machine_crash_shutdown':
> > arch/x86/kernel/cpu/mshyperv.c:112: undefined
> > reference to `native_machine_crash_shutdown'
> >
> > This patch fixes the problem
> >
> > Reported-by: Jim Davis <jim.epost@gmail.com>
> > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > ---
> >  arch/x86/kernel/cpu/mshyperv.c |    2 ++
> >  1 files changed, 2 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/x86/kernel/cpu/mshyperv.c
> b/arch/x86/kernel/cpu/mshyperv.c
> > index f794bfa..0faed5e0 100644
> > --- a/arch/x86/kernel/cpu/mshyperv.c
> > +++ b/arch/x86/kernel/cpu/mshyperv.c
> > @@ -109,7 +109,9 @@ static void hv_machine_crash_shutdown(struct
> pt_regs *regs)
> >  {
> >  	if (hv_crash_handler)
> >  		hv_crash_handler(regs);
> > +#ifdef CONFIG_KEXEC_CORE
> >  	native_machine_crash_shutdown(regs);
> > +#endif
> 
> Why is kexec the factor here?  And if it really does, can't it just be
> CONFIG_KEXEC, or, can kexec provide a "dummy" inline function so that
> you don't have to have a #ifdef in a .c file?

Greg,

I am on vacation till next Tuesday. I will address your concern after I get back if Vitaly
has not addressed it by then.

Regards,

K. Y

Re: [PATCH 1/1] X86: mshyperv.c: Fix a compilation issue.

[Vitaly Kuznetsov]

KY Srinivasan <kys@microsoft.com> writes:

>> -----Original Message-----
>> From: Greg KH [mailto:gregkh@linuxfoundation.org]
>> Sent: Thursday, August 6, 2015 2:11 PM
>> To: KY Srinivasan <kys@microsoft.com>
>> Cc: linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
>> vkuznets@redhat.com; sfr@canb.auug.org.au; tglx@linutronix.de;
>> mingo@redhat.com; hpa@zytor.com; x86@kernel.org; Haiyang Zhang
>> <haiyangz@microsoft.com>
>> Subject: Re: [PATCH 1/1] X86: mshyperv.c: Fix a compilation issue.
>> 
>> On Thu, Aug 06, 2015 at 02:41:24PM -0700, K. Y. Srinivasan wrote:
>> > Building with a random configuration file, this build failure
>> > was reported:
>> >
>> > arch/x86/built-in.o: In function `hv_machine_crash_shutdown':
>> > arch/x86/kernel/cpu/mshyperv.c:112: undefined
>> > reference to `native_machine_crash_shutdown'
>> >
>> > This patch fixes the problem
>> >
>> > Reported-by: Jim Davis <jim.epost@gmail.com>
>> > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
>> > ---
>> >  arch/x86/kernel/cpu/mshyperv.c |    2 ++
>> >  1 files changed, 2 insertions(+), 0 deletions(-)
>> >
>> > diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
>> > index f794bfa..0faed5e0 100644
>> > --- a/arch/x86/kernel/cpu/mshyperv.c
>> > +++ b/arch/x86/kernel/cpu/mshyperv.c
>> > @@ -109,7 +109,9 @@ static void hv_machine_crash_shutdown(struct pt_regs *regs)
>> >  {
>> >  	if (hv_crash_handler)
>> >  		hv_crash_handler(regs);
>> > +#ifdef CONFIG_KEXEC_CORE
>> >  	native_machine_crash_shutdown(regs);
>> > +#endif
>> 
>> Why is kexec the factor here?  And if it really does, can't it just be
>> CONFIG_KEXEC, or, can kexec provide a "dummy" inline function so that
>> you don't have to have a #ifdef in a .c file?
>
> Greg,
>
> I am on vacation till next Tuesday. I will address your concern after I get back if Vitaly
> has not addressed it by then.

I think the issue is not Hyper-V-related: we have
native_machine_crash_shutdown() defined in asm/reboot.h but its
implementation in kernel/crash.c is only being compiled with
CONFIG_KEXEC. I suggest we fix it with something like the following:

diff --git a/arch/x86/include/asm/reboot.h b/arch/x86/include/asm/reboot.h
index a82c4f1..6763e3e 100644
--- a/arch/x86/include/asm/reboot.h
+++ b/arch/x86/include/asm/reboot.h
@@ -16,8 +16,15 @@ struct machine_ops {

extern struct machine_ops machine_ops;

-void native_machine_crash_shutdown(struct pt_regs *regs);
void native_machine_shutdown(void);
+#ifdef CONFIG_KEXEC
+void native_machine_crash_shutdown(struct pt_regs *regs);
+#else
+static inline void native_machine_crash_shutdown(struct pt_regs *regs)
+{
+       native_machine_shutdown();
+}
+#endif
void __noreturn machine_real_restart(unsigned int type);
/* These must match dispatch in arch/x86/realmore/rm/reboot.S */
#define MRR_BIOS       0

I'll post a patch if there are no objections.

-- 
  Vitaly