Re: [RFC PATCH v3 09/18] xen/arm: ITS: Add virtual ITS commands support

[Ian Campbell <ian.campbell@citrix.com>]

On Fri, 2015-07-03 at 12:20 +0530, Vijay Kilari wrote:
> On Mon, Jun 29, 2015 at 5:53 PM, Ian Campbell <ian.campbell@citrix.com> wrote:
> > On Mon, 2015-06-22 at 17:31 +0530, vijay.kilari@gmail.com wrote:
> >> +static int vgic_its_process_mapvi(struct vcpu *v, struct vgic_its *vits,
> >> +                                  its_cmd_block *virt_cmd)
> >> +{
> >> +    struct vitt entry;
> >> +    struct vits_device *vdev;
> >> +    uint8_t vcol_id, cmd;
> >> +    uint32_t vid, dev_id, event;
> >> +
> >> +    vcol_id = virt_cmd->mapvi.col;
> >> +    vid = virt_cmd->mapvi.phy_id;
> >> +    dev_id = its_decode_devid(v->domain, virt_cmd);
> >
> > If you used the "union its_cmd" I proposed earlier for the virt_cmd
> > argument then this would just be "virt_command->mapvi.devid".
> 
> its_decode_devid() is used to get physical devid for a given
> virtual device id. I have to use virt_command->mapvi.devid.
> 
> If vitt entries are made base on physical dev id then
> 
>   - This helps where in if interrupt is received we can extract
> physical dev id from
> the its_device structure held by irq_desc. With this physical device id we can
> search vitt entry to get collection/vcpu and vlpi information.

http://xenbits.xen.org/people/ianc/vits/draftG.html#virtual-lpi-injection shows that there is no need for the physical device id when handling an pLPI and turning it into a vPLI injection.

In the pLPI case the vlpi comes out of the irq_desc and
vgic_vcpu_inject_lpi only requires a vpli, not physical, which it then
uses as indexes into the per-domain irq_pending
(http://xenbits.xen.org/people/ianc/vits/draftG.html#per-domain-struct-pending_irq-for-vlpis)

See also
http://xenbits.xen.org/people/ianc/vits/draftG.html#mapvi-map-an-input-identifier-to-a-physical-interrupt-and-an-interrupt-collection.
which describes how a MAPVI from the guest needs to be handled.

>   However fake devices cannot have physical device id. We cannot search vitt
> entries. Esp INT command has only virtual dev id. So I think this
> approach is not
> viable.

As shown above vgic_vcpu_inject_lpi only takes a vpli param, so the fact
there that there is no physical device id for an INT command on a fake
command doesn't matter.

> I propose:
>  Manage vitt entries based on virtual dev id and manage domain specific
> RB-tree (which holds list of devices assigned to domain) with physical devid.
> 
> Now on receiving interrupt, will search for RB-tree based on physical dev id and
> get virtual dev id. With virtual dev id search vitt entries to get
> vcpu/collection & vlpi
> info.

The design document should shows there is no need for this.

You are very welcome to propose an alternative design, but I'm afraid
that in that case I'm going to have to ask you to produce a
comprehensive document covering all aspects of your proposed design,
either from scratch or by modifying the existing draft to account for
the consequences of your proposed changes across the board, i.e. it
needs remain a coherent design across the board after your changes.

The fact that you are proposing an RB-tree looked during interrupt
however is a problematic aspect of your proposal, in the existing design
has been arranged such that this is not necessary.

> Also for fake devices, we search vitt entries with vdevice id for INT command.
> 
> Any suggestions?
> 
> Regards
> Vijay