Re: [Xen-devel] [Draft C] Xen on ARM vITS Handling

[Ian Campbell <ian.campbell@xxxxxxxxxx>]

On Wed, 2015-05-27 at 22:14 +0530, Vijay Kilari wrote:
> > ## pITS Scheduling
> >
> > A pITS scheduling pass is attempted:
> >
> > * On write to any virtual `CWRITER` iff that write results in there
> >   being new outstanding requests for that vits;
> 
>    You mean, scheduling pass (softirq trigger)  is triggered iff there is no
> ongoing requests from that vits?

Yes, this has changed with the switch to only a single outstanding
batch. I went with:
        
        * On write to any virtual `CWRITER` iff that write results in there
          being new outstanding requests for that vits which could be consumed
          by the pits (i.e. subject to only a single batch only being
          permitted by the scheduler);
        

Although implementationwise it may be OK to defer that decision to the
scheduler, rather than try to figure it out in the mmio trap.

> 
> > * On read from a virtual `CREADR` iff there are commands outstanding
> >   on that vits;
> > * On receipt of an interrupt notification arising from Xen's own use
> >   of `INT`; (see discussion under Completion)
> > * On any interrupt injection arising from a guests use of the `INT`
> >   command; (XXX perhaps, see discussion under Completion)
> >
> > This may result in lots of contention on the scheduler
> > locking. Therefore we consider that in each case all which happens is
> > triggering of a softirq which will be processed on return to guest,
> > and just once even for multiple events.
> 
> Is it required to have all the cases to trigger scheduling pass?
> Just on CWRITER if no ongoing request and on Xen's own completion INT
> is not sufficient?

I think CREADR is needed too, so the guest sees up to date info.

And on injection arising from the guest use of INT is marked as optional
here and considered later on. Whether it is needed depends on the
decision there.

> [...]
> > The second option is likely to be preferable if the issue of selecting
> > a device ID can be addressed.
> >
> > A secondary question is when these `INT` commands should be inserted
> > into the command stream:

(Nb, this is a list of options, not a list of places where it must be
done)

> >
> > * After each batch taken from a single `vits_cq`;
> 
>    Is this not enough? because Scheduling pass just sends a one batch of
> command with Xen's INT command

It is almost certainly _sufficient_, the question is more whether it is
_necessary_ or whether we can reduce the number of interrupts which are
required for correct emulation of a vits, iow can we get away with one
of the other two options.

The following text argues that only one Xen INT is needed in the stream
at any given moment.

> > ### Device ID (`ID`)
> >
> > This parameter is used by commands which manage a specific device and
> > the interrupts associated with that device. Checking if a device is
> > present and retrieving the data structure must be fast.
> >
> > The device identifiers may not be assigned contiguously and the maximum
> > number is very high (2^32).
> >
> > XXX In the context of virtualised device ids this may not be the case,
> > e.g. we can arrange for (mostly) contiguous device ids and we know the
> > bound is significantly lower than 2^32
> >
> > Possible efficient data structures would be:
> >
> > 1. List: The lookup/deletion is in O(n) and the insertion will depend
> >    if the device should be sorted following their identifier. The
> >    memory overhead is 18 bytes per element.
> > 2. Red-black tree: All the operations are O(log(n)). The memory
> >    overhead is 24 bytes per element.
> >
> > A Red-black tree seems the more suitable for having fast deviceID
> > validation even though the memory overhead is a bit higher compare to
> > the list.
> 
> When PHYSDEVOP_pci_device_add is called, memory for its_device structure
> and other needed structure for this device is allocated added to RB-tree
> with all necessary information

Sounds like a reasonable time to do it. I added something based on your
words.

[...]
> > Format: `MAPC vCID, vTA`
> >
>    -  The GITS_TYPER.PAtype is emulated as 0.

ITYM `GITS_TYPER.PTA`?

I've updated various introductory section to reflect the decision to
emulate as 0.

> 
> > - `MAPC pCID, pTA` physical ITS command is generated
> >
> > ### `MAPD` Command translation
> >
> > Format: `MAPD device, Valid, ITT IPA, ITT Size`
> >
> > `MAPD` is sent with `Valid` bit set if device needs to be added and reset
> > when device is removed.
> >
> > If `Valid` bit is set:
> >
> > - Allocate memory for `its_device` struct
> > - Validate ITT IPA & ITT size and update its_device struct
> > - Find number of vectors(nrvecs) for this device by querying PCI
> >   helper function
> > - Allocate nrvecs number of LPI XXX nrvecs is a function of `ITT Size`?
> > - Allocate memory for `struct vlpi_map` for this device. This
> >   `vlpi_map` holds mapping of Virtual LPI to Physical LPI and ID.
> > - Find physical ITS node with which this device is associated
> > - Call `p2m_lookup` on ITT IPA addr and get physical ITT address
> > - Validate ITT Size
> > - Generate/format physical ITS command: `MAPD, ITT PA, ITT Size`
> >
> > Here the overhead is with memory allocation for `its_device` and `vlpi_map`
> >
> > XXX Suggestion was to preallocate some of those at device passthrough
> > setup time?
> 
> If Validation bit is set:

I think the proper name for this is just "Valid bit", it just indicates
that the entry should be made valid or invalid (i.e. added or removed)
rather than requesting any kind of validation.

>    - Query its_device tree and get its_device structure for this device.
>    - (XXX: If pci device is hidden from dom0, does this device is added
>        with PHYSDEVOP_pci_device_add hypercall?)

We do/will not hide PCI devices from dom0 in the same way we do for
platform devices, we should do as for x86 and rely on xen-pciback to
bind to the PCI device and keep real drivers away rather than on actual
hiding (which will be complex).

So there should always be a PHYSDEVOP_pci_device_add.

If a PCI host-bridge/controller itself is hidden from dom0 then devices
behind it will not be available for passthrough.

>    - If device does not exists return
>    - If device exists in RB-tree then
>           - Validate ITT IPA & ITT size and update its_device struct
>           - Check if device is already assigned to the domain,

If a device isn't assigned to the domain then we shouldn't be assigning
it.

I suspect you actually meant something like if the device is already
mapped in the (v)ITS table, but then the following text seems to
discount updating the table with a second MAPD after, since it only
considers the case that the device is not already mapped.

I didn't update the remainder of this entry since I'm not sure what to
change to account for the above.

Ian.


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