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

[Julien Grall <julien.grall@xxxxxxx>]

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