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

Re: [Xen-devel] [PATCH v2 19/27] ARM: vITS: handle MAPD command



Hi Andre,

On 03/16/2017 11:20 AM, Andre Przywara wrote:
The MAPD command maps a device by associating a memory region for
storing ITEs with a certain device ID.
We store the given guest physical address in the device table, and, if
this command comes from Dom0, tell the host ITS driver about this new
mapping, so it can issue the corresponing host MAPD command and create
the required tables.
We don't map the device tables permanently, as their alignment
requirement is only 256 Bytes, thus making mapping of several tables
complicated. Instead we map the device tables on demand when we need
them later.

Signed-off-by: Andre Przywara <andre.przywara@xxxxxxx>
---
 xen/arch/arm/vgic-v3-its.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/xen/arch/arm/vgic-v3-its.c b/xen/arch/arm/vgic-v3-its.c
index 175a213..c26d5d4 100644
--- a/xen/arch/arm/vgic-v3-its.c
+++ b/xen/arch/arm/vgic-v3-its.c
@@ -38,6 +38,7 @@
 /* Data structure to describe a virtual ITS */
 struct virt_its {
     struct domain *d;
+    paddr_t doorbell_address;
     spinlock_t vcmd_lock;       /* protects the virtual command buffer */
     uint64_t cbaser;
     uint64_t *cmdbuf;
@@ -291,6 +292,46 @@ static int its_handle_mapc(struct virt_its *its, uint64_t 
*cmdptr)
     return ret;
 }

+static int its_handle_mapd(struct virt_its *its, uint64_t *cmdptr)
+{
+    uint32_t devid = its_cmd_get_deviceid(cmdptr);
+    int size = its_cmd_get_size(cmdptr);
+    bool valid = its_cmd_get_validbit(cmdptr);
+    paddr_t itt_addr = its_cmd_mask_field(cmdptr, 2, 0, 52) & GENMASK(51, 8);
+    int ret;
+
+    if ( !its->dev_table )
+        return -1;
+
+    /*
+     * There is no easy and clean way for Xen to know the ITS device ID of a
+     * particular (PCI) device, so we have to rely on the guest telling
+     * us about it. For *now* we are just using the device ID *Dom0* uses,
+     * because the driver there has the actual knowledge.
+     * Eventually this will be replaced with a dedicated hypercall to
+     * announce pass-through of devices.
+     */
+    if ( is_hardware_domain(its->d) )
+    {
+        /* Dom0's ITSes are mapped 1:1, so both address are the same. */
+        ret = gicv3_its_map_guest_device(its->d, its->doorbell_address, devid,
+                                         its->doorbell_address, devid,
+                                         BIT(size + 1), valid);

The size should be satinized against the number of event support by the vITS.

And it should also be sanitzed in gicv3_its_map_* against the number of event supported by the pITS.

+        if ( ret )
+            return ret;
+    }
+
+    spin_lock(&its->its_lock);
+    if ( valid )
+        its->dev_table[devid] = DEV_TABLE_ENTRY(itt_addr, size + 1);
+    else
+        its->dev_table[devid] = 0;
+
+    spin_unlock(&its->its_lock);
+
+    return 0;
+}
+
 #define ITS_CMD_BUFFER_SIZE(baser)      ((((baser) & 0xff) + 1) << 12)

 static int vgic_its_handle_cmds(struct domain *d, struct virt_its *its,
@@ -320,6 +361,9 @@ static int vgic_its_handle_cmds(struct domain *d, struct 
virt_its *its,
         case GITS_CMD_MAPC:
             its_handle_mapc(its, cmdptr);
             break;
+        case GITS_CMD_MAPD:
+            its_handle_mapd(its, cmdptr);

Should not you check the return value?

+           break;
         case GITS_CMD_SYNC:
             /* We handle ITS commands synchronously, so we ignore SYNC. */
            break;


Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel

 


Rackspace

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