Re: [Xen-devel] [PATCH v13 1/2] vmx: VT-d posted-interrupt core logic handling

["Wu, Feng" <feng.wu@xxxxxxxxx>]

> -----Original Message-----
> From: George Dunlap [mailto:george.dunlap@xxxxxxxxxx]
> Sent: Wednesday, February 24, 2016 8:09 PM
> To: Wu, Feng <feng.wu@xxxxxxxxx>; Doug Goldstein <cardoe@xxxxxxxxxx>;
> Jan Beulich <JBeulich@xxxxxxxx>
> Cc: Tian, Kevin <kevin.tian@xxxxxxxxx>; Keir Fraser <keir@xxxxxxx>; George
> Dunlap <george.dunlap@xxxxxxxxxxxxx>; Andrew Cooper
> <andrew.cooper3@xxxxxxxxxx>; Dario Faggioli <dario.faggioli@xxxxxxxxxx>; xen-
> devel@xxxxxxxxxxxxx
> Subject: Re: [Xen-devel] [PATCH v13 1/2] vmx: VT-d posted-interrupt core logic
> handling
> 
> On 24/02/16 03:09, Wu, Feng wrote:
> >
> >
> >> -----Original Message-----
> >> From: Doug Goldstein [mailto:cardoe@xxxxxxxxxx]
> >> Sent: Wednesday, February 24, 2016 11:02 AM
> >> To: Jan Beulich <JBeulich@xxxxxxxx>; Wu, Feng <feng.wu@xxxxxxxxx>
> >> Cc: Tian, Kevin <kevin.tian@xxxxxxxxx>; Keir Fraser <keir@xxxxxxx>; George
> >> Dunlap <george.dunlap@xxxxxxxxxxxxx>; Andrew Cooper
> >> <andrew.cooper3@xxxxxxxxxx>; Dario Faggioli <dario.faggioli@xxxxxxxxxx>;
> xen-
> >> devel@xxxxxxxxxxxxx
> >> Subject: Re: [Xen-devel] [PATCH v13 1/2] vmx: VT-d posted-interrupt core
> logic
> >> handling
> >>
> >> On 2/23/16 10:34 AM, Jan Beulich wrote:
> >>>>>> On 23.02.16 at 09:34, <feng.wu@xxxxxxxxx> wrote:
> >>>
> >>>> --- a/xen/include/asm-x86/hvm/hvm.h
> >>>> +++ b/xen/include/asm-x86/hvm/hvm.h
> >>>> @@ -565,6 +565,12 @@ const char *hvm_efer_valid(const struct vcpu *v,
> >>>> uint64_t value,
> >>>>                             signed int cr0_pg);
> >>>>  unsigned long hvm_cr4_guest_reserved_bits(const struct vcpu *v, bool_t
> >>>> restore);
> >>>>
> >>>> +#define arch_vcpu_block(v) ({                                           
> >>>>        \
> >>>> +    void (*func) (struct vcpu *) = (v)->domain-
> >>> arch.hvm_domain.vmx.vcpu_block;\
> >>>> +    if ( func )                                                         
> >>>>        \
> >>>> +        func(v);                                                        
> >>>>        \
> >>>> +})
> >>>
> >>> See my comment on v12. The code structure actually was better
> >>> there, and all you needed to do is introduce a local variable.
> >>
> >> Wouldn't this be a bit cleaner (and type-safier (inventing a word here))
> >> to do with a static inline function?
> >
> > As I mentioned in earlier version, after making it a inline function, I
> > encountered building failures, which is related to using
> > '(v)->domain->arch.hvm_domain.vmx.vcpu_block ' here since it refers to
> > some data structure, it is not so straightforward to address it, so I change
> > it to a macro, just like other micros in this file, which refers to
> > ' (v)->arch.hvm_vcpu.....'.
> 
> I did a brief search for this previous conversation, but I couldn't find
> where you said what the build failures were; and I couldn't off the top
> of my head imagine why dereferencing the pointer that way in a macro
> should be different than dereferencing it in an inline function (and
> "since it refers to some data structure" doesn't give me a clue).
> 
> Having just gone and made that change, it turns out that at the point of
> this definition, the vmx struct hasn't been defined yet, so the compiler
> can't verify that the struct elements actually exist.  (The actual error
> message is "dereferencing pointer to incomplete type".)
> 
> In general, if there is important information like that, you should add
> a comment, so that other people coming in and asking this sort of
> question can get the answer from the code, rather than having to ask
> and/or dig through mailing list archives; e.g.:
> 
> /*
>  * This must be defined as a macro instead of an inline, because the
>  * vmx struct has not yet been defined.
>  */

Thanks a lot for your analysis on this. However, the build error is not
just because of the vmx struct stuff, here is the build error message:

In file included from 
/mnt/home/feng/workspace/xen/xen/include/asm/hvm/irq.h:25:0,
                 from /mnt/home/feng/workspace/xen/xen/include/asm/hvm/vpt.h:31,
                 from 
/mnt/home/feng/workspace/xen/xen/include/asm/hvm/vlapic.h:26,
                 from 
/mnt/home/feng/workspace/xen/xen/include/asm/hvm/vcpu.h:24,
                 from /mnt/home/feng/workspace/xen/xen/include/asm/domain.h:7,
                 from /mnt/home/feng/workspace/xen/xen/include/xen/domain.h:6,
                 from /mnt/home/feng/workspace/xen/xen/include/xen/sched.h:10,
                 from x86_64/asm-offsets.c:10:
/mnt/home/feng/workspace/xen/xen/include/asm/hvm/hvm.h: In function 
âarch_vcpu_blockâ:
/mnt/home/feng/workspace/xen/xen/include/asm/hvm/hvm.h:585:6: error: 
dereferencing pointer to incomplete type
     v->domain->arch.hvm_domain.vmx.vcpu_block(v);
      ^
Makefile:156: recipe for target 'asm-offsets.s' failed
make[3]: *** [asm-offsets.s] Error 1
make[3]: Leaving directory '/mnt/home/feng/workspace/xen/xen/arch/x86'
Makefile:118: recipe for target '/mnt/home/feng/workspace/xen/xen/xen' failed
make[2]: *** [/mnt/home/feng/workspace/xen/xen/xen] Error 2
make[2]: Leaving directory '/mnt/home/feng/workspace/xen/xen'
Makefile:41: recipe for target 'install' failed
make[1]: *** [install] Error 2
make[1]: Leaving directory '/mnt/home/feng/workspace/xen/xen'
Makefile:97: recipe for target 'install-xen' failed
make: *** [install-xen] Error 2

From the above message, we can get the following header include chain:

asm-offsets.c -> <xen/sched.h> -> <xen/domain.h> -> <asm/domain.h> ->
<hvm/vcpu.h> -> <hvm/vlapic.h> -> <hvm/vpt.h> -> <hvm/irq.h> ->
<asm/hvm/hvm.h>

then in asm/hvm/hvm.h, we want to deference 'struct vcpu'  and 'struct domain',
which are define later in <xen/sched.h>. We get building failures for sure since
we got <asm/hvm/hvm.h> from the very beginning of <xen/sched.h>, where
the vcpu and domain structure have not been defined yet. And that is why
there are other similar macros in this header file. So I think using macro here
is the best method at current stage.

Thanks,
Feng


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel