[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH v5 11/30] ARM: GICv3 ITS: introduce device mapping
Hi Andre,
On 06/04/17 00:19, Andre Przywara wrote:
The ITS uses device IDs to map LPIs to a device. Dom0 will later use
those IDs, which we directly pass on to the host.
For this we have to map each device that Dom0 may request to a host
ITS device with the same identifier.
Allocate the respective memory and enter each device into an rbtree to
later be able to iterate over it or to easily teardown guests.
Because device IDs are per ITS, we need to identify a virtual ITS. We
use the doorbell address for that purpose, as it is a nice architectural
MSI property and spares us handling with opaque pointer or break
the VGIC abstraction.
Signed-off-by: Andre Przywara <andre.przywara@xxxxxxx>
---
xen/arch/arm/gic-v3-its.c | 263 +++++++++++++++++++++++++++++++++++++++
xen/arch/arm/vgic-v3-its.c | 3 +
xen/include/asm-arm/domain.h | 3 +
xen/include/asm-arm/gic_v3_its.h | 13 ++
4 files changed, 282 insertions(+)
diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
index eb47c9d..45bbfa7 100644
--- a/xen/arch/arm/gic-v3-its.c
+++ b/xen/arch/arm/gic-v3-its.c
@@ -21,6 +21,8 @@
#include <xen/lib.h>
#include <xen/delay.h>
#include <xen/mm.h>
+#include <xen/rbtree.h>
+#include <xen/sched.h>
#include <xen/sizes.h>
#include <asm/gic.h>
#include <asm/gic_v3_defs.h>
@@ -36,6 +38,26 @@
*/
LIST_HEAD(host_its_list);
+/*
+ * Describes a device which is using the ITS and is used by a guest.
+ * Since device IDs are per ITS (in contrast to vLPIs, which are per
+ * guest), we have to differentiate between different virtual ITSes.
+ * We use the doorbell address here, since this is a nice architectural
+ * property of MSIs in general and we can easily get to the base address
+ * of the ITS and look that up.
+ */
+struct its_devices {
Again, why its_devices? You only store the information for one device.
+ struct rb_node rbnode;
+ struct host_its *hw_its;
+ void *itt_addr;
+ paddr_t guest_doorbell; /* Identifies the virtual ITS */
+ uint32_t host_devid;
+ uint32_t guest_devid;
+ uint32_t eventids; /* Number of event IDs (MSIs) */
+ uint32_t *host_lpi_blocks; /* Which LPIs are used on the host */
+ struct pending_irq *pend_irqs; /* One struct per event */
+};
[...]
+static int remove_mapped_guest_device(struct its_devices *dev)
+{
+ int ret = 0;
+ unsigned int i;
+
+ if ( dev->hw_its )
+ /* MAPD also discards all events with this device ID. */
+ ret = its_send_cmd_mapd(dev->hw_its, dev->host_devid, 0, 0, false);
+
+ for ( i = 0; i < dev->eventids / LPI_BLOCK; i++ )
+ gicv3_free_host_lpi_block(dev->host_lpi_blocks[i]);
+
+ /* Make sure the MAPD command above is really executed. */
+ if ( !ret )
+ ret = gicv3_its_wait_commands(dev->hw_its);
+
+ /* This should never happen, but just in case ... */
+ if ( ret )
+ printk(XENLOG_WARNING "Can't unmap host ITS device 0x%x\n",
+ dev->host_devid);
I think we want to ratelimit this.
+
+ xfree(dev->itt_addr);
+ xfree(dev->pend_irqs);
+ xfree(dev->host_lpi_blocks);
+ xfree(dev);
+
+ return 0;
+}
[...]
+/*
+ * Map a hardware device, identified by a certain host ITS and its device ID
+ * to domain d, a guest ITS (identified by its doorbell address) and device ID.
+ * Also provide the number of events (MSIs) needed for that device.
+ * This does not check if this particular hardware device is already mapped
+ * at another domain, it is expected that this would be done by the caller.
+ */
+int gicv3_its_map_guest_device(struct domain *d,
+ paddr_t host_doorbell, uint32_t host_devid,
+ paddr_t guest_doorbell, uint32_t guest_devid,
+ uint32_t nr_events, bool valid)
+{
+ void *itt_addr = NULL;
+ struct host_its *hw_its;
+ struct its_devices *dev = NULL;
+ struct rb_node **new = &d->arch.vgic.its_devices.rb_node, *parent = NULL;
+ unsigned int i;
+ int ret = -ENOENT;
+
+ hw_its = gicv3_its_find_by_doorbell(host_doorbell);
+ if ( !hw_its )
+ return ret;
+
+ /* Sanitise the provided hardware values against the host ITS. */
+ if ( host_devid >= BIT(hw_its->devid_bits) )
+ return -EINVAL;
+
+ /* We allocate events and LPIs in chunks of LPI_BLOCK (=32). */
+ nr_events = ROUNDUP(nr_events, LPI_BLOCK);
I think it is still not enough. The MAPD command will use the number of
bits, so you want to make sure you cover all the events in the table.
For instance, if nr_events = 66, you will round up to 96. My
understanding, is you will call mapd with 7 bits rather 8 bits.
So you have to round up to the next power of two. Whether you want to
allocate a power of two of LPIs is another question. But the ITT should
be configured correctly.
[...]
+ if ( ret )
+ {
+ do {
+ i--;
+ gicv3_free_host_lpi_block(dev->host_lpi_blocks[i]);
+ if ( i == 0 )
+ break;
+ } while (1);
This could be } while ( i > 0 ) saving 3 lines.
+
+ /* Unmapping the device will discard all LPIs mapped so far. */
+ its_send_cmd_mapd(hw_its, host_devid, 1, 0, false);
You likely need to check the return of its_send_mapd...
Also why 1 for the 3rd argument?
+
+ goto out;
+ }
+
+ return 0;
+
+out_unlock:
+ spin_unlock(&d->arch.vgic.its_devices_lock);
+
+out:
+ if ( dev )
+ {
+ xfree(dev->pend_irqs);
+ xfree(dev->host_lpi_blocks);
+ }
+ xfree(itt_addr);
+ xfree(dev);
+
+ return ret;
+}
+
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
