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

[Julien Grall <julien.grall@xxxxxxx>]

Hi Andre,

On 26/05/17 18:35, Andre Przywara wrote:
> +/**************************************
> + * Functions that handle ITS commands *
> + **************************************/
> +
> +static uint64_t its_cmd_mask_field(uint64_t *its_cmd, unsigned int word,
> +                                   unsigned int shift, unsigned int size)
> +{
> +    return ((its_cmd[word])) >> (shift & (BIT(size) - 1));
This code is wrong. The mask applies to shift and not the shifted 
result. I fixed with:
return (its_cmd[word] >> shift) * (BIT(size) - 1).

Though,  BIT(size) - 1 would not work if size == 64. I would replace by 
GENMASK(0, size - 1) to be safe in all the case.
[...]

> +static int vgic_its_handle_cmds(struct domain *d, struct virt_its *its)
> +{
> +    paddr_t addr = its->cbaser & GENMASK(51, 12);
> +    uint64_t command[4];
> +
> +    ASSERT(spin_is_locked(&its->vcmd_lock));
> +
> +    if ( its->cwriter >= ITS_CMD_BUFFER_SIZE(its->cbaser) )
> +        return -1;
> +
> +    while ( its->creadr != its->cwriter )
> +    {
> +        int ret;
> +
> +        ret = vgic_access_guest_memory(d, addr + its->creadr,
> +                                       command, sizeof(command), false);
> +        if ( ret )
> +            return ret;
> +
> +        switch ( its_cmd_get_command(command) )
> +        {
> +        case GITS_CMD_SYNC:
> +            /* We handle ITS commands synchronously, so we ignore SYNC. */
> +            break;
> +        default:
> +            gdprintk(XENLOG_WARNING, "vGITS: unhandled ITS command %lu\n",
> +                     its_cmd_get_command(command));
Would it be possible to dump the full command (i.e command[0...3]) here 
and ...
> +            break;
> +        }
> +
> +        write_u64_atomic(&its->creadr, (its->creadr + ITS_CMD_SIZE) %
> +                         ITS_CMD_BUFFER_SIZE(its->cbaser));
> +
> +        if ( ret )
> +            gdprintk(XENLOG_WARNING,
> +                     "vGITS: ITS command error %d while handling command 
> %lu\n",
> +                     ret, its_cmd_get_command(command));
... here?

This could be helpful whilst debugging vITS command emulation.

Cheers,

--
Julien Grall

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