Re: [Xen-devel] Xen on ARM vITS Handling Draft B (Was Re: Xen/arm: Virtual ITS command queue handling)

[Julien Grall <julien.grall@xxxxxxxxxx>]

Hi Vijay,

On 22/05/15 13:16, Vijay Kilari wrote:
> On Tue, May 19, 2015 at 7:21 PM, Ian Campbell <ian.campbell@xxxxxxxxxx> wrote:
>> On Tue, 2015-05-19 at 14:37 +0100, Julien Grall wrote:
>>> Hi Ian,
>>>
>>> On 19/05/15 13:10, Ian Campbell wrote:
>>>> On Fri, 2015-05-15 at 15:55 +0100, Julien Grall wrote:
>>>> [...]
>>>>>> Translation of certain commands can be expensive (XXX citation
>>>>>> needed).
>>>>>
>>>>> The term "expensive" is subjective. I think we can end up to cheap
>>>>> translation if we properly pre-allocate information (such as device,
>>>>> LPIs...). We can have all the informations before the guest as boot or
>>>>> during hotplug part. It wouldn't take more memory than it should use.
>>>>>
>>>>> During command translation, we would just need to enable the device/LPIs.
>>>>>
>>>>> The remaining expensive part would be the validation. I think we can
>>>>> improve most of them of O(1) (such as collection checking) or O(log(n))
>>>>> (such as device checking).
>>>> [...]
>>>>>> XXX need a solution for this.
>>>>>
>>>>> Command translation can be improved. It may be good too add a section
>>>>> explaining how translation of command foo can be done.
>>>>
>>>> I think that is covered by the spec, however if there are operations
>>>> which form part of this which are potentially expensive we should
>>>> outline in our design how this will be dealt with.
>>>>
>>>> Perhaps you or Vijay could propose some additional text covering:
>>>>       * What the potentially expensive operations during a translation
>>>>         are.
>>>>       * How we are going to deal with those operations, including:
>>>>               * What data structure is used
>>>>               * What start of day setup is required to enable this
>>>>               * What operations are therefore required at translation
>>>>                 time
>>>
>>> I don't have much time to work on a proposal. I would be happy if Vijay
>>> do it.
>>
>> OK, Vijay could you make a proposal here please.
> 
> __text__
> 
> 1) Command translation:
> -----------------------------------
> 
>  - ITS commands contains device ID, Event ID (vID), Collection ID
> (vCID), Target Address (vTA)
>     parameters
>  - All these parameters should be validated
>  - These parameters should be translated from Virtual to Physical
> 
> Of the existing GICv3 ITS commands, MAPC, MAPD, MAPVI/MAPI are the time
> consuming commands as these commands creates entry in the Xen ITS structures,
> which are used to validate other ITS commands.
> 
> 1.1 MAPC command translation
> -----------------------------------------------
>    Format: MAPC vCID, vTA
> 
>    -  vTA is validated against Re-distributor address by searching
> Redistributor region /
>        CPU number based on GITS_TYPER.PAtype and Physical Collection
> ID & Physical
>        Target address are retrieved
>    -  Each vITS will have cid_map (struct cid_mapping) which holds mapping of
>       Virtual Collection ID, Virtual Targets address and Physical Collection 
> ID.

How the vCID is mapped to the pCID? How would that fit with interrupt
migration?

>    -  MAPC pCID, pTA physical ITS command is generated
> 
>    Here there is no overhead, the cid_map entries (approx 32 entries)
> are preallocated when
>    vITS is created.

Wrong, there is an overhead with your solution. If you have
GITS_TYPER.PTA == 1 (i.e using the physical address of re-distributors)
you have to loop through all the re-distributors which may be long.

As suggested on a previous mail, there is no reason to have
GITS_TYPER.PTA different per domain and we can choose the best value for us.

In our case, GITS_TYPER.PTA = 0 (i.e using linear processors numbers) is
the best one.

> 1.2 MAPD Command translation:
> -----------------------------------------------
>    Format: MAPD device, ITT IPA, ITT Size
> 
>    MAPD is sent with Validation bit set if device needs to be added
> and reset when device is removed
> 
> If Validation bit is set:

     - Check if the device is assigned to the domain

>    - Allocate memory for its_device struct

Allocation can't be done in interrupt context.

>    - Validate ITT IPA & ITT size and update its_device struct
>    - Find number of vectors(nrvecs) for this device by querying PCI
> helper function

This could be read only once when the device is added to Xen via the
hypercall PHYSDEV_*pci*.

>    - Allocate nrvecs number of LPI
>    - 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 for which this device is assigned

Not necessary in a 1 vITS = 1 pITS which seem to be the solution we will
choose.

>    - Call p2m_lookup on ITT IPA addr and get physical ITT address
>    - Validate ITT Size

You already do it in "validate ITT IPA & ITT size...". Although all the
checks should be done before any allocation.

>    - Generate/format physical ITS command: MAPD, ITT PA, ITT Size
> 
>    Here the overhead is with memory allocation for its_device and vlpi_map

As suggested earlier, the memory allocate of its_device and vlpi_map can
be done when the device is assigned to the domain or added to Xen

The only things you would have to do here is checking the ITT size and
mark the device enable.

> 
> If Validation bit is not set:
>     - Validate if the device exits by checking vITS device list

Using a list can be very expensive... I would use a radix tree.

>     - Clear all vlpis assigned for this device

What happens for interrupt assigned to this device? Are they disabled?
unroute?

>     - Remove this device from vITS list
>     - Free memory
>
> 1.3 MAPVI/MAPI Command translation:
> -----------------------------------------------
>    Format: MAPVI device, ID, vID, vCID
> 
> - Validate if the device exits by checking vITS device list

exists

> - Validate vCID and get pCID by searching cid_map
> - if vID does not have entry in vlpi_entries of this device
>   If not, Allot pID from vlpi_map of this device and update
> vlpi_entries with new pID
> - Allocate irq descriptor and add to RB tree
> - call route_irq_to_guest() for this pID
> - Generate/format physical ITS command: MAPVI device ID, pID, pCID
> 
> Here the overhead is allot physical ID, allocate memory for
> irq descriptor and  routing interrupt

An overhead which can be removed by routing the IRQ when the device is
assigned.

> All other ITS command like MOVI, DISCARD, INV, INVALL, INT, CLEAR,
> SYNC just validate and generate physical command

With the data structure you suggested it's not the case, the validation
can be very expensive.

> __text__
> 
> We can discuss and add how to reduce translation time.

I've suggested multiple way to reduce translation time over my previous
mail. It would have been nice to include them in your proposal...

Regards,

-- 
Julien Grall

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