[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH v10 16/32] ARM: vGIC: advertise LPI support
Hi Andre,
On 26/05/17 18:35, 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.
I am not sure to understand this sentence. You report LPI capability if
the domain has an ITS exposed. It does not matter whether the host has one.
For Dom0 we report the same number of interrupts identifiers as the
host, whereas DomUs get a number fixed at 10 bits for the moments, which
covers all SPIs. Also we fix a slight inaccuracy here, since the
number of interrupt identifier specified in GICD_TYPER depends on the
stream interface and is independent from the number of actually wired
SPIs.
This justification would have been useful in patch #2.
This also removes a "TBD" comment, as we now populate the processor
number in the GICR_TYPER register.
Again, I think it would be worth explaining that you populate
GICR_TYPER.Process_Number because the ITS will use it later on.
Signed-off-by: Andre Przywara <andre.przywara@xxxxxxx>
---
xen/arch/arm/vgic-v3.c | 76 +++++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 69 insertions(+), 7 deletions(-)
diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
index e2e5bc1..134e1b0 100644
--- a/xen/arch/arm/vgic-v3.c
+++ b/xen/arch/arm/vgic-v3.c
@@ -170,8 +170,19 @@ 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;
+ {
+ unsigned long flags;
+
+ if ( !v->domain->arch.vgic.has_its )
+ goto read_as_zero_32;
+ if ( dabt.size != DABT_WORD ) goto bad_width;
+
+ spin_lock_irqsave(&v->arch.vgic.lock, flags);
+ *r = vgic_reg32_extract(!!(v->arch.vgic.flags & VGIC_V3_LPIS_ENABLED),
+ info);
+ spin_unlock_irqrestore(&v->arch.vgic.lock, flags);
+ return 1;
+ }
case VREG32(GICR_IIDR):
if ( dabt.size != DABT_WORD ) goto bad_width;
@@ -183,16 +194,20 @@ 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;
+ /* We use the VCPU ID as the redistributor ID in bits[23:8] */
+ typer |= v->vcpu_id << GICR_TYPER_PROC_NUM_SHIFT;
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;
@@ -426,6 +441,33 @@ static uint64_t sanitize_pendbaser(uint64_t reg)
return reg;
}
+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);
+
+ /* rdists_enabled is protected by the domain lock. */
+ ASSERT(spin_is_locked(&v->domain->arch.vgic.lock));
+
+ if ( nr_lpis < LPI_OFFSET )
+ nr_lpis = 0;
+ else
+ nr_lpis -= LPI_OFFSET;
+
+ if ( !v->domain->arch.vgic.rdists_enabled )
+ {
+ v->domain->arch.vgic.nr_lpis = nr_lpis;
+ v->domain->arch.vgic.rdists_enabled = true;
+ /*
+ * Make sure the per-domain rdists_enabled flag has been set before
+ * enabling this particular redistributor.
Hmmm, you also used nr_lpis in the vITS code without any locked. So this
barrier is not only there for rdists_enabled.
+ */
+ smp_mb();
I would have expected a bit more documentation based on the discussion
we had on the previous version. For instance, why do you need this
memory barrier?...
+ }
+
+ v->arch.vgic.flags |= VGIC_V3_LPIS_ENABLED;
+}
+
static int __vgic_v3_rdistr_rd_mmio_write(struct vcpu *v, mmio_info_t *info,
uint32_t gicr_reg,
register_t r)
@@ -436,8 +478,26 @@ 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;
+ {
+ unsigned long flags;
+
+ if ( !v->domain->arch.vgic.has_its )
+ goto write_ignore_32;
+ if ( dabt.size != DABT_WORD ) goto bad_width;
+
+ vgic_lock(v); /* protects rdists_enabled */
+ spin_lock_irqsave(&v->arch.vgic.lock, flags);
+
+ /* LPIs can only be enabled once, but never disabled again. */
+ if ( (r & GICR_CTLR_ENABLE_LPIS) &&
+ !(v->arch.vgic.flags & VGIC_V3_LPIS_ENABLED) )
+ vgic_vcpu_enable_lpis(v);
+
+ spin_unlock_irqrestore(&v->arch.vgic.lock, flags);
+ vgic_unlock(v);
+
+ return 1;
+ }
case VREG32(GICR_IIDR):
/* RO */
@@ -1045,7 +1105,6 @@ static int vgic_v3_distr_mmio_read(struct vcpu *v,
mmio_info_t *info,
* Number of interrupt identifier bits supported by the GIC
* Stream Protocol Interface
*/
- unsigned int irq_bits = get_count_order(vgic_num_irqs(v->domain));
/*
* Number of processors that may be used as interrupt targets when ARE
* bit is zero. The maximum is 8.
@@ -1058,7 +1117,10 @@ 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));
- typer |= (irq_bits - 1) << GICD_TYPE_ID_BITS_SHIFT;
+ if ( v->domain->arch.vgic.has_its )
+ typer |= GICD_TYPE_LPIS;
+
+ typer |= (v->domain->arch.vgic.intid_bits - 1) <<
GICD_TYPE_ID_BITS_SHIFT;
*r = vgic_reg32_extract(typer, info);
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
