Re: [Xen-devel] [PATCH v6 01/12] xen: allow only sizeof(bool) variables for boolean_param()

[Julien Grall <julien@xen.org>]

Hi Juergen,

On 26/02/2020 12:46, Juergen Gross wrote:
> Support of other variable sizes than that of normal bool ones for
> boolean_parameter() don't make sense, so catch any other sized
> variables at build time.
> 
> Fix the one parameter using a plain int instead of bool.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
> V6:
> - new patch
> ---
>   xen/arch/x86/hvm/asid.c | 2 +-
>   xen/include/xen/param.h | 8 ++++++--
>   2 files changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/asid.c b/xen/arch/x86/hvm/asid.c
> index 8e00a28443..8b5bb86dfd 100644
> --- a/xen/arch/x86/hvm/asid.c
> +++ b/xen/arch/x86/hvm/asid.c
> @@ -25,7 +25,7 @@
>   #include <asm/hvm/asid.h>
>   
>   /* Xen command-line option to enable ASIDs */
> -static int opt_asid_enabled = 1;
> +static bool opt_asid_enabled = true;
>   boolean_param("asid", opt_asid_enabled);
>   
>   /*
> diff --git a/xen/include/xen/param.h b/xen/include/xen/param.h
> index 75471eb4ad..d4578cd27f 100644
> --- a/xen/include/xen/param.h
> +++ b/xen/include/xen/param.h
> @@ -2,6 +2,8 @@
>   #define _XEN_PARAM_H
>   
>   #include <xen/init.h>
> +#include <xen/lib.h>
> +#include <xen/stdbool.h>
>   
>   /*
>    * Used for kernel command line parameter setup
> @@ -46,7 +48,8 @@ extern const struct kernel_param __param_start[], __param_end[];
>       __kparam __setup_##_var = \
>           { .name = __setup_str_##_var, \
>             .type = OPT_BOOL, \
> -          .len = sizeof(_var), \
> +          .len = sizeof(_var) + \
> +                 BUILD_BUG_ON_ZERO(sizeof(_var) != sizeof(bool)), \

 From my understanding, sizeof(bool) is not necessarily 1 (it can be 
greater). While this is fine to use it in Xen, I think we want it to 
always be one when exposed in the hypfs.

>             .par.var = &_var }
>   #define integer_param(_name, _var) \
>       __setup_str __setup_str_##_var[] = _name; \
> @@ -86,7 +89,8 @@ extern const struct kernel_param __param_start[], __param_end[];
>       __rtparam __rtpar_##_var = \
>           { .name = _name, \
>             .type = OPT_BOOL, \
> -          .len = sizeof(_var), \
> +          .len = sizeof(_var) + \
> +                 BUILD_BUG_ON_ZERO(sizeof(_var) != sizeof(bool)), \
>             .par.var = &_var }
>   #define integer_runtime_only_param(_name, _var) \
>       __rtparam __rtpar_##_var = \
> 

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel