From mboxrd@z Thu Jan  1 00:00:00 1970
From: Andrew Cooper <andrew.cooper3@citrix.com>
Subject: Re: [PATCH v3] x86: wrap kexec feature with CONFIG_KEXEC
Date: Mon, 31 Aug 2015 19:47:42 +0100
Message-ID: <55E4A14E.2000105@citrix.com>
References: <1441045917-2167-1-git-send-email-jonathan.creekmore@gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
Received: from mail6.bemta14.messagelabs.com ([193.109.254.103])
	by lists.xen.org with esmtp (Exim 4.72)
	(envelope-from <prvs=678dd82be=Andrew.Cooper3@citrix.com>)
	id 1ZWUtq-0001yD-9Z
	for xen-devel@lists.xenproject.org; Mon, 31 Aug 2015 19:37:50 +0000
In-Reply-To: <1441045917-2167-1-git-send-email-jonathan.creekmore@gmail.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Jonathan Creekmore <jonathan.creekmore@gmail.com>, xen-devel@lists.xenproject.org
Cc: keir@xen.org, david.vrabel@citrix.com, jbeulich@suse.com
List-Id: xen-devel@lists.xenproject.org

On 31/08/15 19:31, Jonathan Creekmore wrote:
> Add the appropriate #if checks around the kexec code in the x86 codebase
> so that the feature can actually be turned off by the flag instead of
> always required to be enabled on x86.
>
> Signed-off-by: Jonathan Creekmore <jonathan.creekmore@gmail.com>

This is still not quite correct with makefile level constructs.  (Also
you have broken the ARM build).  Allow me to explain.

HAS_$foo is used to indicate whether there is support for $foo in a
particular arch.  This is statement of support, not an option. 
Therefore, xen/arch/x86/Rules.mk should remain unchanged.

The top xen/rules.mk 'kexec' option is ok, but you need a further clause
after including the per-arch rules.  Something like

ifeq($(HAS_KEXEC)$(kexec),yy)
CONFIG_KEXEC := y
endif

Which will set a Makefile level CONFIG_KEXEC which indicates that the
arch has support, and the developer wishes to enable it.

For the rest of the makefile changes, you should use things like
obj-$(CONFIG_KEXEC) and CLFAGS-$(CONFIG_KEXEC) as opposed to
$(HAS_KEXEC) and $(kexec)

With that fixed, I think this patch is ready, with one further tweak...

> @@ -79,6 +79,27 @@ void vmcoreinfo_append_str(const char *fmt, ...)
>  #else /* !CONFIG_KEXEC */
>  
>  #define crashinfo_maxaddr_bits 0
> +#define kexecing 0
> +
> +static inline void kexec_early_calculations(void)
> +{
> +
> +}
> +
> +static inline void kexec_crash(void)
> +{
> +
> +}
> +
> +static inline void kexec_crash_save_cpu(void)
> +{
> +
> +}
> +
> +static inline void set_kexec_crash_area_size(u64 system_ram)
> +{
> +    (void)system_ram;

You shouldn't need this statement, as the callee's parameter should be
evaluated by virtue of this being a static inline.

If it is to stop the compiler complaining about an unused parameter,
please use the __unused attribute instead.

~Andrew