Re: [PATCH v2 13/23] x86: provide stubs, declarations and macros in hvm.h

[Wei Liu <wei.liu2@citrix.com>]

On Mon, Sep 03, 2018 at 10:45:01AM +0100, Paul Durrant wrote:
> > -----Original Message-----
> > From: Xen-devel [mailto:xen-devel-bounces@lists.xenproject.org] On Behalf
> > Of Wei Liu
> > Sent: 26 August 2018 13:20
> > To: xen-devel@lists.xenproject.org
> > Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; Wei Liu
> > <wei.liu2@citrix.com>; Jan Beulich <jbeulich@suse.com>
> > Subject: [Xen-devel] [PATCH v2 13/23] x86: provide stubs, declarations and
> > macros in hvm.h
> > 
> > Make sure hvm_enabled evaluate to false then provide necessary stubs,
> > declarations and macros to make Xen build.
> > 
> > The is_viridian_domain macro can't be turned into an inline function
> > easily,
> 
> Why? It's just an is_hvm_domain() and a feature check.

It is domain.h which includes hvm/domain.h, not the other way around.
When is_viridian_domain is defined, struct domain is not available. If
is_viridian_domain is a function, it will need to know the definition of
struct domain.

Another way to fix this is to move is_viridian_domain to another header
file. I don't know which one would be preferred.

> 
> > so instead its caller is modified to avoid unused variable
> > warning.
> > 
> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > ---
> >  xen/arch/x86/traps.c          |   4 +-
> >  xen/include/asm-x86/hvm/hvm.h | 149
> > ++++++++++++++++++++++++++++++++++-
> >  2 files changed, 148 insertions(+), 5 deletions(-)
> > 
> > diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
> > index ddff346..27b9651 100644
> > --- a/xen/arch/x86/traps.c
> > +++ b/xen/arch/x86/traps.c
> > @@ -771,9 +771,9 @@ static void do_trap(struct cpu_user_regs *regs)
> >  /* Returns 0 if not handled, and non-0 for success. */
> >  int rdmsr_hypervisor_regs(uint32_t idx, uint64_t *val)
> >  {
> > -    struct domain *d = current->domain;
> >      /* Optionally shift out of the way of Viridian architectural MSRs. */
> > -    uint32_t base = is_viridian_domain(d) ? 0x40000200 : 0x40000000;
> > +    uint32_t base = is_viridian_domain(current->domain)
> > +        ? 0x40000200 : 0x40000000;
> > 
> 
> This is a bit ugly so it would be nice to keep it on a single line.

OK.

Wei.

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