Re: [Xen-devel] [PATCH v3 08/16] arm/xen: move GIC context data structure to gic driver

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Tue, 2014-04-15 at 19:41 +0100, Julien Grall wrote:
> Hello Vijaya,
> 
> Thank you for your patch.
> 
> On 04/15/2014 12:17 PM, vijay.kilari@xxxxxxxxx wrote:
> > diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
> > index 999bbdd..be1b084 100644
> > --- a/xen/include/asm-arm/domain.h
> > +++ b/xen/include/asm-arm/domain.h
> > @@ -8,6 +8,7 @@
> >  #include <asm/p2m.h>
> >  #include <asm/vfp.h>
> >  #include <asm/mmio.h>
> > +#include <asm/gic.h>
> >  #include <public/hvm/params.h>
> >  #include <xen/serial.h>
> >  
> > @@ -260,8 +261,7 @@ struct arch_vcpu
> >      uint32_t csselr;
> >      register_t vmpidr;
> >  
> > -    uint32_t gic_hcr, gic_vmcr, gic_apr;
> > -    uint32_t gic_lr[64];
> 
> I would add a comment such as "GIC state" here.

If that isn't obvious from the name of the union then a comment isn't
going to help.

> > +    union gic_state_data gic;
> >      uint64_t event_mask;
> >      uint64_t lr_mask;
> 
> [..]
> 
> > +struct gic_v2 {
> > +    uint32_t hcr, vmcr;
> > +    uint32_t apr;
> > +    uint32_t lr[64];
> > +};
> > +
> 
> Even if with the name you know it's GICv2. Can you document quickly the
> structure?

Lets not ask him to continually shave yaks as part of this. These were
clear enough before and they are clear enough now.

Ian.


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