[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 16/34] x86/hvm: enclose hvm_enabled and hvm_funcs in CONFIG_HVM



>>> On 17.08.18 at 17:12, <wei.liu2@xxxxxxxxxx> wrote:
> This helps to take advantage of dead code elimination. Turn
> hvm_enabled into proper bool while at it.
> 
> Providing an empty hvm_funcs resolves a lot of undefined references to
> it in the header. It is safe to do so because those functions / macros
> are not expected to be used.

"not expected to be used" != "not used". This ...

> --- a/xen/include/asm-x86/hvm/hvm.h
> +++ b/xen/include/asm-x86/hvm/hvm.h
> @@ -234,8 +234,14 @@ struct hvm_function_table {
>      } tsc_scaling;
>  };
>  
> +#if CONFIG_HVM
> +extern bool hvm_enabled;
>  extern struct hvm_function_table hvm_funcs;
> -extern bool_t hvm_enabled;
> +#else
> +#define hvm_enabled false
> +static struct hvm_function_table hvm_funcs = {};

... is a no-go imo. Either keep the extern visible (but don't have a
definition anywhere, relying on DCE once again), or make sure the
structure has no members at all in the !HVM case (but that would
assume the references you talk about aren't field accesses, but
only accesses to the entire structure). Otherwise we'd end up
with a significant amount of NULL pointers.

But in the end it's not really clear to me what problem you're trying
to solve: The header here should not be included at all when HVM
is not enabled (or most of it be inside "#ifdef CONFIG_HVM").

Jan



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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.