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

Re: [Xen-devel] [PATCH v5 17/30] ARM: vITS: add command handling stub and MMIO emulation



Hi Andre,

Another thing I missed.

On 04/06/2017 12:19 AM, Andre Przywara wrote:
+    case VREG64(GITS_BASER0):           /* device table */
+        if ( !vgic_reg64_check_access(info->dabt) ) goto bad_width;
+
+        spin_lock(&its->its_lock);
+
+        /*
+         * Changing base registers with the ITS enabled is UNPREDICTABLE,
+         * we choose to ignore it, but warn.
+         */
+        if ( its->enabled )
+        {
+            spin_unlock(&its->its_lock);
+            gdprintk(XENLOG_WARNING, "ITS: tried to change BASER with the ITS 
enabled.\n");
+
+            return 1;
+        }
+
+        reg = its->baser_dev;
+        vgic_reg64_update(&reg, r, info);
+
+        reg &= ~GITS_BASER_RO_MASK;
+        reg |= (sizeof(uint64_t) - 1) << GITS_BASER_ENTRY_SIZE_SHIFT;
+        reg |= GITS_BASER_TYPE_DEVICE << GITS_BASER_TYPE_SHIFT;
+        sanitize_its_base_reg(&reg);
+
+        if ( reg & GITS_VALID_BIT )
+            its->max_devices = its_baser_nr_entries(reg);

Should not you validate this against the maximum number of device supported by the ITS (i.e devid)?

+        else
+            its->max_devices = 0;
+
+        its->baser_dev = reg;
+        spin_unlock(&its->its_lock);
+        return 1;
+    case VREG64(GITS_BASER1):           /* collection table */
+        if ( !vgic_reg64_check_access(info->dabt) ) goto bad_width;
+
+        spin_lock(&its->its_lock);
+        /*
+         * Changing base registers with the ITS enabled is UNPREDICTABLE,
+         * we choose to ignore it, but warn.
+         */
+        if ( its->enabled )
+        {
+            spin_unlock(&its->its_lock);
+            gdprintk(XENLOG_INFO, "ITS: tried to change BASER with the ITS 
enabled.\n");
+            return 1;
+        }
+
+        reg = its->baser_coll;
+        vgic_reg64_update(&reg, r, info);
+        reg &= ~GITS_BASER_RO_MASK;
+        reg |= (sizeof(uint16_t) - 1) << GITS_BASER_ENTRY_SIZE_SHIFT;
+        reg |= GITS_BASER_TYPE_COLLECTION << GITS_BASER_TYPE_SHIFT;
+        sanitize_its_base_reg(&reg);
+
+        if ( reg & GITS_VALID_BIT )
+            its->max_collections = its_baser_nr_entries(reg);

Similar question for the collection. Although, I am not sure against what.

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®.