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

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Mon, Aug 20, 2018 at 07:04:09AM -0600, Jan Beulich wrote:
> >>> 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").

This patch has been rewritten to provide more stubs etc.

Wei.

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