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

[Vijay Kilari <vijay.kilari@xxxxxxxxx>]

On Mon, May 4, 2015 at 7:24 PM, Julien Grall <julien.grall@xxxxxxxxxx> wrote:
>
>
> On 04/05/2015 14:44, Julien Grall wrote:
>>
>> Hi Vijay,
>>
>> On 04/05/2015 14:27, Vijay Kilari wrote:
>>>
>>> On Mon, May 4, 2015 at 6:34 PM, Julien Grall <julien.grall@xxxxxxxxxx>
>>> wrote:
>>>>
>>>>
>>>>
>>>> On 04/05/2015 13:58, Vijay Kilari wrote:
>>>>>
>>>>>
>>>>> On Thu, Apr 30, 2015 at 7:59 PM, Julien Grall <julien.grall@xxxxxxxxxx>
>>>>> wrote:
>>>>>>
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> On 30/04/15 14:47, Stefano Stabellini wrote:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> If the devid is not within this range, the ITS won't recognize the
>>>>>>>>>> value and
>>>>>>>>>> won't be able to send the interrupt.
>>>>>>>>>>
>>>>>>>>>> So this is clearly not the right value.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Sure, in that case the maximum value allowed by GITS_TYPER.Devbits.
>>>>>>>>> Vijay, what is the value of GITS_TYPER.Devbits on your platform?
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> It is 21 bits
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> I would imagine that 21 bits would be plenty to find an unused devid.
>>>>>>>
>>>>>>> Alternatively we could use an inexistent function of a real
>>>>>>> device, such
>>>>>>> as 00:00.1 (function 1 of the host bridge).
>>>>>>
>>>>>>
>>>>>>
>>>>>> As discussed IRL, this idea sounds good to me.
>>>>>>
>>>>>> Although I would be happy with any other way which ensure the devid is
>>>>>> free.
>>>>>
>>>>>
>>>>>
>>>>> Has prototyped with 00.00.1 as device id. But I see that Dom0 boot is
>>>>> slow compared to polling mode. This could be because Dom0 has to keep
>>>>> trapping on creader to check if creader is updated or not.
>>>>
>>>>
>>>>
>>>> How did you implement the interrupt mode? Could it be improve?
>>>
>>>
>>>     1) In physical ITS driver its_device is created with devID 00:00.1
>>> with
>>> 256 MSI-x are reserved and is named as completion_dev, which is global.
>>
>>
>> That's a lot of MSI-x reserved... Can't you use only one per domain?
>
>
> Hmmm... I meant for all the domain, not "per domain".

   Complexity with one irq for all domains is that if completion interrupt
comes it is difficult to find out  for which vITS/Domain ITS command
it came for.

>>
>>>     2) In Domain init,
>>>           - one irq (called completion_irq) is allocated per domain for
>>> this device
>>
>>
>> So only 256 domain can run on your platform at the same time?
>>
>>>             and irq desc is allocated to this domain. This way we can
>>> get vITS
>>>             context when interrupt is received.
>>>           - An array of 32 requests(its_requests) is allocated which
>>> stores all
>>>             the information about the ITS commands that are converted
>>> and written
>>>             to physical ITS queue and each request info contains
>>>             [CReader vITS] [CWriter vITS] [Physical Q Index] [Number
>>> of commands]
>>>              [Completion irq] [ status ]
>>
>>
>> Why 32 requests?

   Thought that 32 requests would be sufficient. Can increase more.

>>
>> Also some of the fields don't make much sense to me such as "Number of
>> Commands" , "Completion IRQ"  and "status" I guess I will find out when
>> you will send the new series.
>>
>>>     3) When vITS received ITS command. This command is converted to
>>> physical
>>>        command  and written to physical queue, INT command is appended
>>> with
>>>        completion_irq and entry is made in its_requests.
>>>     4) On receiving completion_irq, vITS structure and its_requests
>>> info is parsed
>>>         and Creader of vITS is upstated with [ Cwriter vITS ] stored
>>> for this request
>>>         and this request is removed from its_requests.
>>
>>
>> You complicate the code by allowing 32 batch of command per domain
>> (looping is very slow). You should only allow one batch of command per
>> domain. When the batch is finished you can send another one.
>>
      There is an index to first request and last request. So don't loop for
all 32 requests. Since we have to maintain vITS and physical ITS commands
are always processed in the same order, always the completion interrupt
comes for the first pending request.

>>>   I am adding one INT per command. This can be improved to add one INT
>>> cmd for all
>>>   the pending commands. Existing Linux driver sends 2 commands at a time.
>>
>>
>> You should not assume that other OS will send 2 commands at the same
>> time... It could be more or less.
>>
>> Although, having a INT per command is rather slow. One INT command per
>> batch would improve the boot time.

   We cannot limit on number of commands sent at a time. we have to send all the
pending commands in vITS queue at a time when trapped on CWRITER, Otherwise
we have to check for pending interrupts on completion interrupt and translate
and send pending commands in interrupt context. Which complicates and adds more
delays.

Regards
Vijay

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