Hi Andre,
On 11/05/17 18:53, Andre Przywara wrote:
For each device we allocate one struct pending_irq for each virtual
event (MSI).
Provide a helper function which returns the pointer to the appropriate
struct, to be able to find the right struct when given a virtual
deviceID/eventID pair.
Signed-off-by: Andre Przywara <andre.przyw...@arm.com>
---
xen/arch/arm/gic-v3-its.c | 69 ++++++++++++++++++++++++++++++++++++++++
xen/include/asm-arm/gic_v3_its.h | 4 +++
2 files changed, 73 insertions(+)
diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
index aebc257..fd6a394 100644
--- a/xen/arch/arm/gic-v3-its.c
+++ b/xen/arch/arm/gic-v3-its.c
@@ -800,6 +800,75 @@ out:
return ret;
}
+/* Must be called with the its_device_lock held. */
+static struct its_device *get_its_device(struct domain *d, paddr_t vdoorbell,
+ uint32_t vdevid)
+{
+ struct rb_node *node = d->arch.vgic.its_devices.rb_node;
+ struct its_device *dev;
+
+ ASSERT(spin_is_locked(&d->arch.vgic.its_devices_lock));
+
+ while (node)
+ {
+ int cmp;
+
+ dev = rb_entry(node, struct its_device, rbnode);
+ cmp = compare_its_guest_devices(dev, vdoorbell, vdevid);
+
+ if ( !cmp )
+ return dev;
+
+ if ( cmp > 0 )
+ node = node->rb_left;
+ else
+ node = node->rb_right;
+ }
+
+ return NULL;
+}
+
+static uint32_t get_host_lpi(struct its_device *dev, uint32_t eventid)
+{
+ uint32_t host_lpi = INVALID_LPI;
+
+ if ( dev && (eventid < dev->eventids) )
+ host_lpi = dev->host_lpi_blocks[eventid / LPI_BLOCK] +
+ (eventid % LPI_BLOCK);
+
+ return host_lpi;
IHMO, it would be easier to read if you invert the condition:
if ( !dev || (eventid >= dev->eventids) )
return INVALID_LPI;
return dev->host_lpi_blocks[eventid / LPI_BLOCK] + (eventid % LPI_BLOCK);
Also, whilst I agree about sanitizing eventid, someone calling this
function with dev = NULL is already wrong. Defensive programming is
good, but there are some place where I don't think it is necessary. You
have to trust a bit the caller, otherwise you will end up making the
check 10 times before accessing it.
+}
+
+static struct pending_irq *get_event_pending_irq(struct domain *d,
+ paddr_t vdoorbell_address,
+ uint32_t vdevid,
+ uint32_t veventid,
s/veventid/ as it is not virtual a virtual one and make the call to
get_host_lpi fairly confusing.
+ uint32_t *host_lpi)
+{
+ struct its_device *dev;
+ struct pending_irq *pirq = NULL;
+
+ spin_lock(&d->arch.vgic.its_devices_lock);
+ dev = get_its_device(d, vdoorbell_address, vdevid);
+ if ( dev && veventid <= dev->eventids )
Why are you using "<=" here and not "<" like in get_host_lpi? Surely one
of them is wrong.
+ {
+ pirq = &dev->pend_irqs[veventid];
+ if ( host_lpi )
+ *host_lpi = get_host_lpi(dev, veventid);
Getting the host_lpi is fairly cheap. I would impose to pass host_lpi.
This would also avoid multiple check on the eventid as you currently do. I.e
dev = ...
if ( !dev )
goto out;
*host_lpi = get_host_lpi(dev, ...);
if ( *host_lpi == INVALID_LPI )
goto out;
pirq = &dev->pend_irqs[veventid];
out:
spin_unlock(...)
return pirq;
+ }
+ spin_unlock(&d->arch.vgic.its_devices_lock);
+
+ return pirq;
+}
+
+struct pending_irq *gicv3_its_get_event_pending_irq(struct domain *d,
+ paddr_t vdoorbell_address,
+ uint32_t vdevid,
+ uint32_t veventid)
s/veventid/eventid/
+{
+ return get_event_pending_irq(d, vdoorbell_address, vdevid, veventid, NULL);
+}
This wrapper looks a bit pointless to me. Why don't you directly expose
get_event_pending_irq(...)?
+
/* Scan the DT for any ITS nodes and create a list of host ITSes out of it. */
void gicv3_its_dt_init(const struct dt_device_node *node)
{
diff --git a/xen/include/asm-arm/gic_v3_its.h b/xen/include/asm-arm/gic_v3_its.h
index 40f4ef5..d162e89 100644
--- a/xen/include/asm-arm/gic_v3_its.h
+++ b/xen/include/asm-arm/gic_v3_its.h
@@ -169,6 +169,10 @@ int gicv3_its_map_guest_device(struct domain *d,
int gicv3_allocate_host_lpi_block(struct domain *d, uint32_t *first_lpi);
void gicv3_free_host_lpi_block(uint32_t first_lpi);
+struct pending_irq *gicv3_its_get_event_pending_irq(struct domain *d,
+ paddr_t vdoorbell_address,
+ uint32_t vdevid,
+ uint32_t veventid);
#else
static inline void gicv3_its_dt_init(const struct dt_device_node *node)
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
