Re: [Xen-devel] [PATCH v2 0/1] xen/device-tree: Do not remap IRQs for secondary IRQ controllers

[Wei Chen <wei.chen@xxxxxxxxxx>]

Hi Julien,

On 17 May 2016 at 00:30, Julien Grall <julien.grall@xxxxxxx> wrote:
> Hi Wei,
>
>
> On 16/05/16 16:47, Wei Liu wrote:
>>
>> On Mon, May 16, 2016 at 05:03:54PM +0200, Edgar E. Iglesias wrote:
>>>
>>> From: "Edgar E. Iglesias" <edgar.iglesias@xxxxxxxxxx>
>>>
>>> I'm sending this as a v2 considering that I previously posted a diff
>>> in email discussions.
>>>
>>> This patch fixes an DT problem with the ZynqMP PCIe node.
>>> Today, we try to remap IRQs regardless of if they are directly
>>> connected to the primary controller or not. If they are indirectly
>>> connected via secondary IRQ controllers, we abort the boot with
>>> an error.
>>>
>>> The ZynqMP PCIe DTS bindings were upstreamed to Linux after Xen 4.6, so
>>> this issue is not a regression.
>>> PCIe is also not generally a critical feature of current ZynqMP boards,
>>> i.e the boards are functional without PCIe although for some users PCIe
>>> may be more or less critical.
>>>
>>> As I see it we have two options:
>>>
>>> 1. A safe option is to disable the PCIe node in the ZynqMP platform.
>>>     We can then fix this with calm for 4.8.
>>>     + It will fix the dom0 boot.
>>>     + Very safe and unintrusive.
>>>     - PCIe will not be functional.
>>>
>>> 2. Apply this patch to 4.7
>>>     + It will fix the dom0 boot.
>>>     + PCIe will be functional.
>>>     - Intrusive fix. Although the fix really only affects a case that
>>>       otherwise would have resulted in an aborted boot of dom0.
>>>
>>> I'm happy to submit a patch for option nr 1 if you guys feel that's
>>> safer/better at this point.
>>>
>>
>> I'm a bit wary that this patch touches common code because the issue you
>> describe seems to be board specific.
>
>
> Quite a few platforms (e.g Exynos, Tegra, ZynqMP) provide multiple interrupt
> controllers in cascade, with a GIC has the main controller.
> We solved the issue for device in handle_device, but not when mapping
> interrupts associated to a bus.
>
> Even if this code touches common code, there is only one caller
> (map_device_children) and the call will only be done for PCI bus. So the
> patch is not as scary as it looks like :).
>
> It's also unclear why this would
>>
>> make PCIe function on ZynqMP from the commit message.
>>
>> I admit I don't know much about ARM so I will leave the further review
>> and judgement to ARM maintainers. I don't think OSSTest will pick up bug
>> in this patch FWIW.
>
>
> Correct, there are no ARM platforms with PCI in OSSTest.
> Nonetheless, this patch is not too risky as the usage of the function is
> very limited.
>
> IIRC, we currently support 4 platforms with PCI: X-Gene, Seattle/Overdrive,
> Juno, and ZynqMP.
>

I have tested on Seattle/Overdrive and I have not noticed any regression too.
I also created a dummy secondary interrupt controller in DTS to test this patch.
It works, Xen is doing the right things in both way.

> I gave a try on Juno r2 and I have not noticed any regression. I can give a
> try on X-Gene and Seattle if necessary.
>
> For me the change is good to go in Xen 4.7. Stefano do you have any opinion?
>
> Regards,
>
> --
> Julien Grall
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel

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