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

[David Vrabel <david.vrabel@citrix.com>]

On 27/08/15 15:47, 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.

What's your use case for this?

I think you should consider providing empty stub functions for !KEXEC
instead of all the #ifdefs.

> --- a/xen/drivers/passthrough/vtd/dmar.h
> +++ b/xen/drivers/passthrough/vtd/dmar.h
> @@ -108,6 +108,7 @@ struct acpi_atsr_unit *acpi_find_matched_atsr_unit(const struct pci_dev *);
>  
>  #define DMAR_OPERATION_TIMEOUT MILLISECS(1000)
>  
> +#ifdef CONFIG_KEXEC
>  #define IOMMU_WAIT_OP(iommu, offset, op, cond, sts) \
>  do {                                                \
>      s_time_t start_time = NOW();                    \
> @@ -125,6 +126,22 @@ do {                                                \
>          cpu_relax();                                            \
>      }                                                           \
>  } while (0)
> +#else
> +#define IOMMU_WAIT_OP(iommu, offset, op, cond, sts) \
> +do {                                                \
> +    s_time_t start_time = NOW();                    \
> +    while (1) {                                     \
> +        sts = op(iommu->reg, offset);               \
> +        if ( cond )                                 \
> +            break;                                  \
> +        if ( NOW() > start_time + DMAR_OPERATION_TIMEOUT ) {    \
> +	  panic("%s:%d:%s: DMAR hardware is malfunctional",	\
> +		__FILE__, __LINE__, __func__);			\
> +        }                                                       \
> +        cpu_relax();                                            \
> +    }                                                           \
> +} while (0)
> +#endif

This is particular might be best done by making kexecing a static const
variable equal to 0 in the !KEXEC case.

> --- a/xen/include/asm-x86/config.h
> +++ b/xen/include/asm-x86/config.h
> @@ -1,6 +1,6 @@
>  /******************************************************************************
>   * config.h
> - * 
> + *
>   * A Linux-style configuration list.
>   */

Stray whitespace change.

David