Re: [Xen-devel] [RFC PATCH 00/19] xen/arm: Add ITS support

[Vijay Kilari <vijay.kilari@xxxxxxxxx>]

On Tue, Mar 3, 2015 at 5:13 PM, Julien Grall <julien.grall@xxxxxxxxxx> wrote:
> Hello Vijay,
>
> On 03/03/2015 03:55, Vijay Kilari wrote:
>>
>> On Mon, Mar 2, 2015 at 6:49 PM, Julien Grall <julien.grall@xxxxxxxxxx>
>> wrote:
>>>
>>> On 02/03/15 12:30, vijay.kilari@xxxxxxxxx wrote:
>>>>
>>>> From: Vijaya Kumar K <Vijaya.Kumar@xxxxxxxxxxxxxxxxxx>
>>>>
>>>> Add ITS support for arm. Following major features
>>>> are supported
>>>>   - GICv3 ITS support for arm64 platform
>>>>   - Supports only single ITS node
>>>
>>>
>>> Why only one ITS node supported? I though Cavium was using 2 ITS...
>>
>>
>> I will update for 2 ITS nodes later when NUMA is supported
>
>
> Why do you speak about NUMA? AFAICT, there is no requirement to support NUMA
> for having multiple ITS...
>
> With multiple ITS support, your vITS emulation will likely heavily change.
> So it would be nice to have this support as soon as possible.

Incremental changes (as separate patch series)
would be more meaningful. I will have a look at it
and if possible I will incorporate in next series

>
>>>
>>>>   - LPI descriptors are allocated on-demand
>>>>   - Only ITS Dom0 is supported
>>>
>>>
>>> Any plan to support guest?
>>
>>
>> Yes, I will do it in next version
>
>
> What is missing to support guest? Only the toolstack part?

Yes, it is only toolstack part similar to GICv3

>
>>>
>>>> Vijaya Kumar K (19):
>>>>    xen/arm: add linked list apis
>>>>    xen/arm: its: Import GICv3 ITS driver from linux
>>>>    xen/arm: its: Port ITS driver to xen
>>>
>>>
>>> A general comment (I haven't read closely the patches). The GICv3 ITS
>>> taken from Linux is modified so heavily (rename function, move out code,
>>> dropping unused code...) that your assumption in patch #1 [1] is wrong.
>>>
>>> At the end of this series it would make impossible to backport patch
>>> from Linux.
>>
>>
>>    Most of the code is reused or moved to different file based on Xen
>> requirement.
>>      - code like msi registered callback (setup_irq & teardown_irq) is of
>> no use in Xen. So removed
>>      - irq_chip is different in linux
>>      - some of the functions like encode of ITS commands can be used in
>> virtual ITS
>>        driver as well. So have to be moved out to header file
>>      - the LPI allocation is moved to virtual ITS driver. We can
>> consider it keeping in physical ITS
>>        driver but it fits well in virtual ITS driver rather than
>> physical ITS driver.
>
>
> IHMO, moving the code around two files make the code more difficult to
> review because the patch is bigger.
>
> That also means we can't really review the first couple of patches because
> the code will change a lot after.
>
> But the maintainers may be disagree with me...
>
>>>
>>> Regards,
>>>
>>> [1] "This is actual GICv3 ITS driver from Linux. [..] This helps to
>>> import any issues found in Linux"
>>
>>
>> I have kept the linux GICv3 ITS driver in first patch and made incremental
>> changes just to have better understanding and incremental approach
>
>
> But this is doesn't help to backport issue from Linux...
>
> BTW, do you have a tree with all the code?

Yes, I have a tree, But I cannot share it. Github is ok?

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