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.przyw...@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,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
