[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v10 11/32] ARM: GICv3: forward pending LPIs to guests

Hi Stefano,

On 30/05/17 23:07, Stefano Stabellini wrote:
On Tue, 30 May 2017, Julien Grall wrote:

@@ -136,6 +135,80 @@ uint64_t gicv3_get_redist_address(unsigned int cpu,
bool use_pta)
         return per_cpu(lpi_redist, cpu).redist_id << 16;

+static void vgic_vcpu_inject_lpi(struct domain *d, unsigned int virq)
+    struct pending_irq *p = irq_to_pending(d->vcpu[0], virq);
+    struct vcpu *v = NULL;
+    if ( !p )
+        return;
+    if ( p->lpi_vcpu_id < d->max_vcpus )
+        v = d->vcpu[read_atomic(&p->lpi_vcpu_id)];

Hmmm, what does prevent lpi_vcpu_id to change between the check and the read?

Supposedly we are going to set lpi_vcpu_id only to good values? Meaning
that we are going to do the lpi_vcpu_id checks at the time of setting
lpi_vcpu_id. Thus, even if lpi_vcpu_id changes, it is not a problem. In
fact, if that is true, can we even drop the if ( p->lpi_vcpu_id <
d->max_vcpus ) test here?

I am not too confident to say lpi_vcpu_id will always be valid with the current locking in the vGIC.

There is a potential race between its_discard_event and this function. The former may reset pending_irq whilst reading lpi_vcpu_id as we cannot take the vCPU lock yet.

But all of this is racy anyway because of the locking. This will get solved by the vGIC rework after the merge.

So For the time being I would keep the check. We can revisit it later if necessary.


Julien Grall

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.