On 10/05/17 16:11, Andre Przywara wrote:
Hi,
Hi Andre,
On 12/04/17 18:20, Julien Grall wrote:
On 12/04/17 01:44, Andre Przywara wrote:
+{
+ ASSERT(spin_is_locked(&v->arch.vgic.lock));
The locking is likely to wrong here too (see patch #2). For instance
with a MOVI then INV on interrupt enabled.
+
+ if ( test_bit(GIC_IRQ_GUEST_ENABLED, &p->status) )
+ {
+ if ( !list_empty(&p->inflight) &&
+ !test_bit(GIC_IRQ_GUEST_VISIBLE, &p->status) )
+ gic_raise_guest_irq(v, vlpi, p->lpi_priority);
+ }
+ else
+ {
+ clear_bit(GIC_IRQ_GUEST_ENABLED, &p->status);
+ list_del_init(&p->lr_queue);
+ }
+}
+
+static int its_handle_inv(struct virt_its *its, uint64_t *cmdptr)
+{
+ struct domain *d = its->d;
+ uint32_t devid = its_cmd_get_deviceid(cmdptr);
+ uint32_t eventid = its_cmd_get_id(cmdptr);
+ struct pending_irq *p;
+ unsigned long flags;
+ struct vcpu *vcpu;
+ uint32_t vlpi;
+ int ret = -1;
+
+ /* Translate the event into a vCPU/vLPI pair. */
+ if ( !read_itte(its, devid, eventid, &vcpu, &vlpi) )
+ return -1;
+
+ if ( vlpi == INVALID_LPI )
+ return -1;
+
+ spin_lock_irqsave(&vcpu->arch.vgic.lock, flags);
+
+ p = d->arch.vgic.handler->lpi_to_pending(d, vlpi);
+ if ( !p )
+ goto out_unlock;
As said on v5, this could be simpler and use the pending_irqs in the
device. That would be an improvement though. So a would be good.
Originally I found it more straight-forward to use the one existing
interface (the rbtree) we also use in the VGIC part, which would allow
us to handle locking or ref-counting in one central place.
But indeed the ITS command handling has all the data we need to find the
pending_irq directly from the virtual device.
So I replaced all lpi_to_pending() calls in those handlers with a new
function gicv3_its_get_event_pending_irq(), which looks up the struct
from an ITS/device/event triple.
I take and keep the its->lock for the runtime of these functions, so
those events and their memory will not vanish meanwhile.
Does that make sense?
It makes sense to keep the ref-counting in one central place. But it is
better to avoid reading guest memory and therefore avoid most of
checking and overhead to translate the IPA to PA.
That's why I suggested to use pending_irqs :).
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
