[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v7a 13/14] xen/arm: split vgic driver into generic and vgic-v2 driver
Hi Vijay, You continue to ignore my comments on this patch... Please explain why you don't want to make those changes. Sending 3 times the same patch without any change is a waste of time for both of us. On 07/01/2014 07:41 AM, vijay.kilari@xxxxxxxxx wrote: > From: Vijaya Kumar K <Vijaya.Kumar@xxxxxxxxxxxxxxxxxx> > > Existing vGIC driver has both generic code and hw specific > code. Segregate vGIC low level driver into vgic-v2.c and > keep generic code in existing vgic.c file Missing "." at the end? > some static generic functions in vgic.c is made as non-static Uppercase for the first letter of the first word. s/is/are/ [..] > +const struct mmio_handler_ops vgic_v2_distr_mmio_handler = { There was a static on the previous location. You should keep it. [..] > +const static struct vgic_ops vgic_v2_ops = { > + .vcpu_init = vgic_v2_vcpu_init, > + .domain_init = vgic_v2_domain_init, > + .send_sgi = vgic_v2_to_sgi, > +}; static const. [..] > diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c > index 78004b0..9f18347 100644 > --- a/xen/arch/arm/vgic.c > +++ b/xen/arch/arm/vgic.c [..] > int domain_vgic_init(struct domain *d) > { > int i; > @@ -75,6 +67,15 @@ int domain_vgic_init(struct domain *d) > else > d->arch.vgic.nr_lines = 0; /* We don't need SPIs for the guest */ > > + switch ( gic_hw_version() ) > + { > + case GIC_V2: > + vgic_v2_init(d); vgic_v2_init is returning an int to indicate if the initialization has succeeded or not. Please check the return to see if an error occurred. > -static int vgic_to_sgi(struct vcpu *v, register_t sgir) > +/* TODO: unsigned long is used to fit vcpu_mask. Change to cpu_mask */ > +int vgic_to_sgi(struct vcpu *v, register_t sgir, enum gic_sgi_mode irqmode, > int virq, > + unsigned long vcpu_mask) [..] > + case SGI_TARGET_OTHERS: [..] > + break; > + case SGI_TARGET_SELF: > + set_bit(current->vcpu_id, &vcpu_mask); I already said it on V4, V5 and now V6a and don't see any change or comment from you side. For SGI_TARGET_{OTHERS,SELF}, you can't assume that vcpu_mask will be equal to 0... It comes directly guest write to GICD_SIGR. Please make sure to handle it correctly or the guest could send SGI to the wrong VCPUs. [..] > +struct vgic_ops { > + /* Initialize vGIC */ > + int (*vcpu_init)(struct vcpu *v); > + /* Domain specific initialization of vGIC */ > + int (*domain_init)(struct domain *d); > + /* SGI handler of vGIC */ > + int (*send_sgi)(struct vcpu *v, register_t sgir); You've introduced vgic_send_sgi here, I'm not sure to understand why... Can you give more input? > +}; > + > /* Number of ranks of interrupt registers for a domain */ > #define DOMAIN_NR_RANKS(d) (((d)->arch.vgic.nr_lines+31)/32) > > @@ -133,6 +142,8 @@ static inline void vgic_byte_write(uint32_t *reg, > uint32_t var, int offset) > *reg |= var; > } > > +extern enum gic_sgi_mode irqmode; > + Hu? What is used for? Regards, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |