| Message ID | 20170316112030.20419-28-andre.przywara@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Andre, On 03/16/2017 11:20 AM, Andre Przywara wrote: > To let a guest know about the availability of virtual LPIs, set the > respective bits in the virtual GIC registers and let a guest control > the LPI enable bit. > Only report the LPI capability if the host has initialized at least > one ITS. > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > --- > xen/arch/arm/vgic-v3.c | 88 +++++++++++++++++++++++++++++++++++++++++++++++--- > 1 file changed, 83 insertions(+), 5 deletions(-) > > diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c > index 8faec95..6d5b7f4 100644 > --- a/xen/arch/arm/vgic-v3.c > +++ b/xen/arch/arm/vgic-v3.c > @@ -169,8 +169,10 @@ static int __vgic_v3_rdistr_rd_mmio_read(struct vcpu *v, mmio_info_t *info, > switch ( gicr_reg ) > { > case VREG32(GICR_CTLR): > - /* We have not implemented LPI's, read zero */ > - goto read_as_zero_32; > + if ( dabt.size != DABT_WORD ) goto bad_width; > + *r = vgic_reg32_extract(!!(v->arch.vgic.flags & VGIC_V3_LPIS_ENABLED), > + info); I would be more readable to have: uint32_t ctlr; ctlr = !!(v->arch.vgic.flags & VGIC_V3_LPIS_ENABLED); *r = vgic_reg32_extract(ctlr, info); > + return 1; > > case VREG32(GICR_IIDR): > if ( dabt.size != DABT_WORD ) goto bad_width; > @@ -182,16 +184,19 @@ static int __vgic_v3_rdistr_rd_mmio_read(struct vcpu *v, mmio_info_t *info, > uint64_t typer, aff; > > if ( !vgic_reg64_check_access(dabt) ) goto bad_width; > - /* TBD: Update processor id in [23:8] when ITS support is added */ > aff = (MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 3) << 56 | > MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 2) << 48 | > MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 1) << 40 | > MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 0) << 32); > typer = aff; > + typer |= (v->vcpu_id & 0xffff) << 8; Please explain in the commit message why this change. At first glance, this should be a separate patch. > > if ( v->arch.vgic.flags & VGIC_V3_RDIST_LAST ) > typer |= GICR_TYPER_LAST; > > + if ( v->domain->arch.vgic.has_its ) > + typer |= GICR_TYPER_PLPIS; > + > *r = vgic_reg64_extract(typer, info); > > return 1; > @@ -502,6 +507,40 @@ bool vgic_can_inject_lpi(struct vcpu *vcpu, uint32_t vlpi) > return false; > } > > +static void vgic_vcpu_enable_lpis(struct vcpu *v) > +{ > + uint64_t reg = v->domain->arch.vgic.rdist_propbase; > + unsigned int nr_lpis = BIT((reg & 0x1f) + 1) - LPI_OFFSET; > + int nr_pages; > + > + /* The first VCPU to enable LPIs maps the property table. */ > + if ( !v->domain->arch.vgic.proptable ) > + { > + v->domain->arch.vgic.nr_lpis = nr_lpis; > + nr_pages = DIV_ROUND_UP(nr_lpis, PAGE_SIZE); > + > + get_guest_pages(v->domain, reg & GENMASK(51, 12), nr_pages); > + v->domain->arch.vgic.proptable = map_guest_pages(v->domain, > + reg & GENMASK(51, 12), > + nr_pages); What is map_guest_pages fail? > + printk("VGIC-v3: VCPU%d mapped %d pages for property table\n", > + v->vcpu_id, nr_pages); Please don't use printk in function called by a guest, as a guest could potentially DOS xen. I think this should be gdprintk(XENLOG_DEBUG,...) > + } > + nr_pages = DIV_ROUND_UP(((nr_lpis + LPI_OFFSET) / 8), PAGE_SIZE); > + reg = v->arch.vgic.rdist_pendbase; > + > + get_guest_pages(v->domain, reg & GENMASK(51, 12), nr_pages); > + v->arch.vgic.pendtable = map_guest_pages(v->domain, > + reg & GENMASK(51, 12), nr_pages); > + > + printk("VGIC-v3: VCPU%d mapped %d pages for pending table\n", > + v->vcpu_id, nr_pages); Ditto. > + > + v->arch.vgic.flags |= VGIC_V3_LPIS_ENABLED; > + > + printk("VGICv3: enabled %d LPIs for VCPU%d\n", nr_lpis, v->vcpu_id); Ditto. > +} > + > static int __vgic_v3_rdistr_rd_mmio_write(struct vcpu *v, mmio_info_t *info, > uint32_t gicr_reg, > register_t r) > @@ -512,8 +551,18 @@ static int __vgic_v3_rdistr_rd_mmio_write(struct vcpu *v, mmio_info_t *info, > switch ( gicr_reg ) > { > case VREG32(GICR_CTLR): > - /* LPI's not implemented */ > - goto write_ignore_32; > + if ( dabt.size != DABT_WORD ) goto bad_width; > + if ( !v->domain->arch.vgic.has_its ) > + return 1; > + > + /* LPIs can only be enabled once, but never disabled again. */ > + if ( !(r & GICR_CTLR_ENABLE_LPIS) || > + (v->arch.vgic.flags & VGIC_V3_LPIS_ENABLED) ) This is fragile. A re-distributor can be updated from any vCPU. So you would end up to call vgic_vcpu_enable_lpis twice. You like want to use a lock protecting the GICR_CTLR emulation. > + return 1; > + > + vgic_vcpu_enable_lpis(v); > + > + return 1; > > case VREG32(GICR_IIDR): > /* RO */ > @@ -1113,6 +1162,11 @@ static int vgic_v3_distr_mmio_read(struct vcpu *v, mmio_info_t *info, > typer = ((ncpus - 1) << GICD_TYPE_CPUS_SHIFT | > DIV_ROUND_UP(v->domain->arch.vgic.nr_spis, 32)); > > + if ( v->domain->arch.vgic.has_its ) > + { > + typer |= GICD_TYPE_LPIS; > + irq_bits = 16; Why 16? Cheers,
diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c index 8faec95..6d5b7f4 100644 --- a/xen/arch/arm/vgic-v3.c +++ b/xen/arch/arm/vgic-v3.c @@ -169,8 +169,10 @@ static int __vgic_v3_rdistr_rd_mmio_read(struct vcpu *v, mmio_info_t *info, switch ( gicr_reg ) { case VREG32(GICR_CTLR): - /* We have not implemented LPI's, read zero */ - goto read_as_zero_32; + if ( dabt.size != DABT_WORD ) goto bad_width; + *r = vgic_reg32_extract(!!(v->arch.vgic.flags & VGIC_V3_LPIS_ENABLED), + info); + return 1; case VREG32(GICR_IIDR): if ( dabt.size != DABT_WORD ) goto bad_width; @@ -182,16 +184,19 @@ static int __vgic_v3_rdistr_rd_mmio_read(struct vcpu *v, mmio_info_t *info, uint64_t typer, aff; if ( !vgic_reg64_check_access(dabt) ) goto bad_width; - /* TBD: Update processor id in [23:8] when ITS support is added */ aff = (MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 3) << 56 | MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 2) << 48 | MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 1) << 40 | MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 0) << 32); typer = aff; + typer |= (v->vcpu_id & 0xffff) << 8; if ( v->arch.vgic.flags & VGIC_V3_RDIST_LAST ) typer |= GICR_TYPER_LAST; + if ( v->domain->arch.vgic.has_its ) + typer |= GICR_TYPER_PLPIS; + *r = vgic_reg64_extract(typer, info); return 1; @@ -502,6 +507,40 @@ bool vgic_can_inject_lpi(struct vcpu *vcpu, uint32_t vlpi) return false; } +static void vgic_vcpu_enable_lpis(struct vcpu *v) +{ + uint64_t reg = v->domain->arch.vgic.rdist_propbase; + unsigned int nr_lpis = BIT((reg & 0x1f) + 1) - LPI_OFFSET; + int nr_pages; + + /* The first VCPU to enable LPIs maps the property table. */ + if ( !v->domain->arch.vgic.proptable ) + { + v->domain->arch.vgic.nr_lpis = nr_lpis; + nr_pages = DIV_ROUND_UP(nr_lpis, PAGE_SIZE); + + get_guest_pages(v->domain, reg & GENMASK(51, 12), nr_pages); + v->domain->arch.vgic.proptable = map_guest_pages(v->domain, + reg & GENMASK(51, 12), + nr_pages); + printk("VGIC-v3: VCPU%d mapped %d pages for property table\n", + v->vcpu_id, nr_pages); + } + nr_pages = DIV_ROUND_UP(((nr_lpis + LPI_OFFSET) / 8), PAGE_SIZE); + reg = v->arch.vgic.rdist_pendbase; + + get_guest_pages(v->domain, reg & GENMASK(51, 12), nr_pages); + v->arch.vgic.pendtable = map_guest_pages(v->domain, + reg & GENMASK(51, 12), nr_pages); + + printk("VGIC-v3: VCPU%d mapped %d pages for pending table\n", + v->vcpu_id, nr_pages); + + v->arch.vgic.flags |= VGIC_V3_LPIS_ENABLED; + + printk("VGICv3: enabled %d LPIs for VCPU%d\n", nr_lpis, v->vcpu_id); +} + static int __vgic_v3_rdistr_rd_mmio_write(struct vcpu *v, mmio_info_t *info, uint32_t gicr_reg, register_t r) @@ -512,8 +551,18 @@ static int __vgic_v3_rdistr_rd_mmio_write(struct vcpu *v, mmio_info_t *info, switch ( gicr_reg ) { case VREG32(GICR_CTLR): - /* LPI's not implemented */ - goto write_ignore_32; + if ( dabt.size != DABT_WORD ) goto bad_width; + if ( !v->domain->arch.vgic.has_its ) + return 1; + + /* LPIs can only be enabled once, but never disabled again. */ + if ( !(r & GICR_CTLR_ENABLE_LPIS) || + (v->arch.vgic.flags & VGIC_V3_LPIS_ENABLED) ) + return 1; + + vgic_vcpu_enable_lpis(v); + + return 1; case VREG32(GICR_IIDR): /* RO */ @@ -1113,6 +1162,11 @@ static int vgic_v3_distr_mmio_read(struct vcpu *v, mmio_info_t *info, typer = ((ncpus - 1) << GICD_TYPE_CPUS_SHIFT | DIV_ROUND_UP(v->domain->arch.vgic.nr_spis, 32)); + if ( v->domain->arch.vgic.has_its ) + { + typer |= GICD_TYPE_LPIS; + irq_bits = 16; + } typer |= (irq_bits - 1) << GICD_TYPE_ID_BITS_SHIFT; *r = vgic_reg32_extract(typer, info); @@ -1729,6 +1783,30 @@ static int vgic_v3_domain_init(struct domain *d) static void vgic_v3_domain_free(struct domain *d) { + int nr_pages; + struct vcpu *v; + + if ( d->arch.vgic.proptable ) + { + nr_pages = DIV_ROUND_UP(d->arch.vgic.nr_lpis, PAGE_SIZE); + + unmap_guest_pages(d->arch.vgic.proptable, nr_pages); + put_guest_pages(d, d->arch.vgic.rdist_propbase & GENMASK(51, 12), + nr_pages); + printk("VGICv3: freeing PROPBASE for Domain%d\n", d->domain_id); + } + + nr_pages = DIV_ROUND_UP((d->arch.vgic.nr_lpis + LPI_OFFSET) / 8, PAGE_SIZE); + for_each_vcpu(d, v) + { + if ( !v->arch.vgic.pendtable ) + continue; + + unmap_guest_pages(v->arch.vgic.pendtable, nr_pages); + put_guest_pages(d, v->arch.vgic.rdist_pendbase & GENMASK(51, 12), + nr_pages); + printk("VGICv3: freeing PENDBASE for VCPU%d\n", v->vcpu_id); + } xfree(d->arch.vgic.rdist_regions); }
To let a guest know about the availability of virtual LPIs, set the respective bits in the virtual GIC registers and let a guest control the LPI enable bit. Only report the LPI capability if the host has initialized at least one ITS. Signed-off-by: Andre Przywara <andre.przywara@arm.com> --- xen/arch/arm/vgic-v3.c | 88 +++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 83 insertions(+), 5 deletions(-)