Re: [PATCH v3] x86: wrap kexec feature with CONFIG_KEXEC

[Andrew Cooper <andrew.cooper3@citrix.com>]

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