Hi Stefano,
On 22/05/17 23:19, Stefano Stabellini wrote:
On Tue, 16 May 2017, Julien Grall wrote:
@@ -436,8 +473,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 */
Getting back to the locking. I don't see any place where we get the domain
vgic lock before vCPU vgic lock. So this raises the question why this ordering
and not moving this lock into vgic_vcpu_enable_lpis.
At least this require documentation in the code and explanation in the commit
message.
It doesn't look like we need to take the v->arch.vgic.lock here. What is
it protecting?
The name of the function is a bit confusion. It does not take the vCPU
vgic lock but the domain vgic lock.
I believe the vcpu is passed to avoid have v->domain in most of the
callers. But we should probably rename the function.
In this case it protects vgic_vcpu_enable_lpis because you can configure
the number of LPIs per re-distributor but this is a domain wide value. I
know the spec is confusing on this.
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel