Re: [Xen-devel] [RFC PATCH v2 13/22] xen/arm: its: Add virtual ITS command support

[Vijay Kilari <vijay.kilari@xxxxxxxxx>]

Hi Julien,

On Tue, Mar 24, 2015 at 5:18 PM, Julien Grall <julien.grall@xxxxxxxxxx> wrote:
> Hello Vijay,
>
> More questions/remarks about command processing.
>
> On 19/03/2015 14:38, vijay.kilari@xxxxxxxxx wrote:
>>
>> +int vgic_its_process_cmd(struct vcpu *v, struct vgic_its *vits)
>> +{
>> +    struct its_cmd_block virt_cmd;
>> +
>> +    /* XXX: Currently we are processing one cmd at a time */
>> +    ASSERT(spin_is_locked(&vits->lock));
>> +
>> +    do {
>> +        if ( vgic_its_read_virt_cmd(v, vits, &virt_cmd) )
>> +            goto err;
>> +        if ( vgic_its_parse_its_command(v, vits, &virt_cmd) )
>> +            goto err;
>> +    } while ( vits->cmd_write != vits->cmd_write_save );
>> +
>> +    vits->cmd_write_save = vits->cmd_write;
>> +    DPRINTK("vITS: write_save 0x%lx write 0x%lx \n",
>> +            vits->cmd_write_save,
>> +            vits->cmd_write);
>> +    /* XXX: Currently we are processing one cmd at a time */
>> +    vgic_its_update_read_ptr(v, vits);
>
>
> From the spec the GITS_CREADR should be updated at every command processing.
> That would make cmd_write_save pointless.

See notes under section 4.9.9 Adding New Commands to the Queue
Multiple commands can be written to a queue at once.

>
> Also, you are taking the VITS lock for the whole process. This process can
> be very long. How will it affect the other vCPUs of the domain?
>

Yes, lock is taken on first command trap and holds until all commands
are processed.
In any case ITS commands are processed in synchronously. So any VCPU that
send ITS commands is blocked.

Also ITS commands are sent while setting up device/irq and while
releasing device/irq.
So there should not be any overhead when device is under use.

> Finally, in environment with multiple guests using ITS, the ITS command send
> to the physical ITS may be interleaved (i.e DOM1 cmd, DOM2 cmd, DOM1 cmd
> ...). Is there any possible side-effect?

Each command is independent.  Generally SYNC/INV is followed after some
commands. But it should not be a problem if they are interleaved.

Regards,
Vijay

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