Re: [PATCH v4 11/17] xen/arm: ITS: Add GICR register emulation

[Ian Campbell <ian.campbell@citrix.com>]

On Mon, 2015-07-13 at 10:28 +0100, Ian Campbell wrote:
> On Sat, 2015-07-11 at 20:25 +0200, Julien Grall wrote:
> > Hi,
> > 
> > On 10/07/2015 17:10, Ian Campbell wrote:
> > > Extra space after the &.
> > >> @@ -694,6 +755,14 @@ static int vgic_v3_distr_mmio_read(struct vcpu *v, mmio_info_t *info)
> > >>           *r = ((ncpus - 1) << GICD_TYPE_CPUS_SHIFT |
> > >>                 DIV_ROUND_UP(v->domain->arch.vgic.nr_spis, 32));
> > >>
> > >> +        if ( gic_lpi_supported() )
> > >> +        {
> > >> +            irq_bits = gic_nr_id_bits();
> > >> +            *r |= GICD_TYPE_LPIS;
> > >> +        }
> > >> +        else
> > >> +            irq_bits = get_count_order(vgic_num_irqs(v->domain));
> > >
> > > I think gic_nr_id_bits should return the correct thing whether or not
> > > LPIs are supported, i.e.
> > >
> > >          if ( gic_lpi_supported() )
> > >              *r |= GICD_TYPE_LPIS;
> > >          irq_bits = gic_nr_id_bits();
> > >
> > > should be sufficient.
> > 
> > Well no. The field GICD_TYPER.IDbits represents the number of bits 
> > supported for the interrupt identifier.
> > 
> > The guest may have a different number of IDbits than the hardware which 
> > could be higher (for instance a guest where emulated SPI is supported).
> 
> Yes, I really meant vgic_nr_id_bits(), which might for the dom0 case end
> up returning something related to the h/w value from the appropriate
> vgic hw cfg struct.

Vijay, to be more specific, the number of idbits should be added to
xen/arch/arm/vgic-v3.c:vgic_v3_hw and as a new argument to
vgic_v3_setup_hw to initialise it.

Then vgic_v3_domain_init() should consult vgic_v3_hw in the
is_hardware_domain case to initialise a new field
d->arch.vgic.nr_id_bits.

For the !is_hardware_domain case I suppose it ought to be some hardcoded
value corresponding to whatever the right value is when LPIs are not
supported.

Ian.