[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v2 13/15] xen/arm: Add support for GIC v3
On Thu, Apr 10, 2014 at 3:30 PM, Ian Campbell <Ian.Campbell@xxxxxxxxxx> wrote: > On Fri, 2014-04-04 at 17:26 +0530, vijay.kilari@xxxxxxxxx wrote: > >> +static void write_aprn_regs(struct gic_state_data *d) >> +{ >> + switch ( nr_priorities ) >> + { >> + case 7: >> + WRITE_SYSREG32(d->v3.gic_apr0[2], ICH_AP0R2_EL2); >> + WRITE_SYSREG32(d->v3.gic_apr1[2], ICH_AP1R2_EL2); > + /* Fall-thru */ > > when doing so deliberately please. > > Is it harmful/illegal to write to AP0R2 etc if priorities < 7? The APR register is implementation defined. It is upto the platform to support number of APR registers. So based on nr_priorities value we read save and restore only platform supported registers >> + >> +static void gic_clear_lr(int lr) >> +{ >> + gich_write_lr(lr, 0); >> +} >> + >> +static void gic_read_lr(int lr, struct gic_lr *lr_reg) > > I can't find struct gic_lr anywhere. Already defined in previous patch > >> +{ >> + u64 lrv; >> + lrv = gich_read_lr(lr); >> + lr_reg->pirq = (lrv >> GICH_LR_PHYSICAL_SHIFT) & GICH_LR_PHYSICAL_MASK; >> + lr_reg->virq = (lrv >> GICH_LR_VIRTUAL_SHIFT) & GICH_LR_VIRTUAL_MASK; >> + lr_reg->priority = (lrv >> GICH_LR_PRIORITY_SHIFT) & >> GICH_LR_PRIORITY_MASK; >> + lr_reg->state = (lrv >> GICH_LR_STATE_SHIFT) & GICH_LR_STATE_MASK; >> + lr_reg->hw_status = (lrv >> GICH_LR_HW_SHIFT) & GICH_LR_HW_MASK; >> + lr_reg->grp = (lrv >> GICH_LR_GRP_SHIFT) & GICH_LR_GRP_MASK; > > If you want to be able to do field-by-field accesses then please do what > the page table code does and use a union and bit fields. See lpae_pt_t. > > typedef union __packed { > uint64_t bits; > struct { > unsigned long pirq:4; > unsugned long virq:4; > etc, including explicit padding > }; > } gicv3_lr; > > Then: > gicv3 lrv = {.bits = gich_read_lr(lr)}; > The complexity is LR register is 64 bit in v3 and 32 bit v2. Though I define this like above for v2 & v3, generic code still need to access based on hw version. So I have make it without bit-fields > >> +static struct dt_irq * gic_maintenance_irq(void) >> +{ >> + return &gic.maintenance; >> +} >> + >> +static void gic_hcr_status(uint32_t flag, uint8_t status) >> +{ >> + if ( status ) > > status is actually "bool_t set" ? > >> + WRITE_SYSREG32((READ_SYSREG32(ICH_HCR_EL2) | flag), ICH_HCR_EL2); >> + else >> + WRITE_SYSREG32((READ_SYSREG32(ICH_HCR_EL2) & (~flag)), ICH_HCR_EL2); >> +} >> + >> + rdist_regs = xzalloc_array(struct rdist_region, gic.rdist_count); > > I thought I saw a comment at the top saying that only a single region > was supported? Shouldn't this be checked somewhere, or even better > fixed. Yes, only rdist_region[0] is read from dt, which supports upto 32 cores. So one can add if more than 32 cores is required. > > Is there a limit on gic.rdist_count? How large is it? Can rdist_regs be > a static array? The rdist count is read from dt. so it is implementation defined > > Ian. > > _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |