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

[Vijay Kilari <vijay.kilari@xxxxxxxxx>]

On Fri, May 22, 2015 at 6:19 PM, Julien Grall <julien.grall@xxxxxxxxxx> wrote:
> 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?

Physical ITS driver create one collection ID (pCID) per CPU.
DomU's vCID should always 0 to MAXVCPUS as GITS.TYPER.PTAtype is set to 0.
(as suggested by you below)

So Migration should be within 0 - 8. Here there is scope for improvement
to migration to pCPU on which vCPU is running.

>
>>    -  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.
>
agreed

>> 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.

Can't we allocate in softirq 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*

If so, this value should be in pci_dev struct
.
>
>>    - 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?

    Should be disable with LPI configuration table update. I think
release_irq is called
>
>>     - 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.

   But, routing requires pID which is not known when device is assigned.
nrvecs could be as high as 256/2K so cannot route all the pID when 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.

which data structure?

>
>> __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