From: marc.zyngier@arm•com (Marc Zyngier)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH v3 2/2] drivers/irqchip: gicv3: add workaround for Synquacer pre-ITS
Date: Sat, 14 Oct 2017 10:13:13 +0100 [thread overview]
Message-ID: <86po9qks7q.fsf@arm.com> (raw)
In-Reply-To: <CAKv+Gu_3zWqiUOhr+GOFZY5SWfJpo+y0DVsA8tYpVLjsN6hnrw@mail.gmail.com> (Ard Biesheuvel's message of "Fri, 13 Oct 2017 21:38:02 +0100")
On Fri, Oct 13 2017 at 9:38:02 pm BST, Ard Biesheuvel <ard.biesheuvel@linaro•org> wrote:
> On 13 October 2017 at 19:50, Rob Herring <robh+dt@kernel•org> wrote:
>> On Fri, Oct 13, 2017 at 4:47 AM, Marc Zyngier <marc.zyngier@arm•com> wrote:
>>> [+Mark]
>>>
>>> On 12/10/17 23:24, Ard Biesheuvel wrote:
>>>> On 12 October 2017 at 22:34, Rob Herring <robh+dt@kernel•org> wrote:
>>>>> On Thu, Oct 12, 2017 at 1:32 PM, Ard Biesheuvel
>>>>> <ard.biesheuvel@linaro•org> wrote:
>>>>>> The Socionext Synquacer SoC's implementation of GICv3 has a so-called
>>>>>> 'pre-ITS', which maps 32-bit writes targeted at a separate window of
>>>>>> size '4 << device_id_bits' onto writes to GITS_TRANSLATER with device
>>>>>> ID taken from bits [device_id_bits + 1:2] of the window offset.
>>>>>> Writes that target GITS_TRANSLATER directly are reported as originating
>>>>>> from device ID #0.
>>>>>>
>>>>>> So add a workaround for this. Given that this breaks isolation, clear
>>>>>> the IRQ_DOMAIN_FLAG_MSI_REMAP flag as well.
>>>>>>
>>>>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro•org>
>>>>>> ---
>>>>>> Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.txt | 4 ++
>>>>>> arch/arm64/Kconfig | 8 +++
>>>>>> drivers/irqchip/irq-gic-v3-its.c | 63 +++++++++++++++++++-
>>>>>> 3 files changed, 72 insertions(+), 3 deletions(-)
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.txt b/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.txt
>>>>>> index 4c29cdab0ea5..0798a61bbf99 100644
>>>>>> --- a/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.txt
>>>>>> +++ b/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.txt
>>>>>> @@ -75,6 +75,10 @@ These nodes must have the following properties:
>>>>>> - reg: Specifies the base physical address and size of the ITS
>>>>>> registers.
>>>>>>
>>>>>> +Optional:
>>>>>> +- socionext,synquacer-pre-its: (u32, u32) tuple describing the host address
>>>>>> + and size of the pre-ITS window, as implemented on the Socionext Synquacer SoC
>>>>>
>>>>> Sounds like a separate h/w block to me and addresses should be in
>>>>> "reg". I would suggest you define a separate node for the pre-its
>>>>> block and then use of_find_compatible_node() from the GIC driver to
>>>>> retrieve the node and whatever you need from it. Or do an SoC specific
>>>>> compatible string for the GIC and let that imply whatever information
>>>>> you need. Then the next quirk doesn't need a DT update.
>>>>>
>>>>
>>>> For my understanding, you mean either
>>>>
>>>> gic: interrupt-controller at 30000000 {
>>>> compatible = "arm,gic-v3";
>>>> ...
>>>>
>>>> its: gic-its at 30020000 {
>>>> compatible = "arm,gic-v3-its";
>>>> reg = <0x0 0x30020000 0x0 0x20000>;
>>>> #msi-cells = <1>;
>>>> msi-controller;
>>>> };
>>>> };
>>>>
>>>> preits at 58000000 {
>>>> compatible = "socionext,synquacer-pre-its";
>>>> reg = <0x0 0x58000000 0x0 0x200000>;
>>>> msi-slave = <&its>;
>>>> };
>>>>
>>>> or
>>>>
>>>> gic: interrupt-controller at 30000000 {
>>>> compatible = "arm,gic-v3";
>>>> ...
>>>>
>>>> its: gic-its at 30020000 {
>>>> compatible = "socionext,synquacer-pre-its", "arm,gic-v3-its";
>>>> reg = <0x0 0x30020000 0x0 0x20000>,
>>>> <0x0 0x58000000 0x0 0x200000>;
>>>> #msi-cells = <1>;
>>>> msi-controller;
>>>> };
>>>> };
>>>>
>>>> right?
>>>>
>>>> Marc, what do you think? IIRC we did discuss option #2 at some point,
>>>> but I don't remember if/why we rejected it.
>>>
>>> I dislike #2 because these registers are not part of the regular ITS,
>>> and would get in the way of potential extensions of the ITS (I don't
>>> know of any, but just in case...).
>>>
>>> I also dislike #1 as the "msi-slave" part is both ugly and confusing
>>> (are we writing to the ITS? to the pre-ITS?).
>>
>> Why do you need this? Are you ever going to have multiple pre-its's?
>
> The question is really whether we are ever going to have multiple
> ITSes, in which case we will need to identify which ITS this single
> pre-ITS is associated with, given that it can only target one. And
> while I would prefer zero pre-ITSes in all cases [as would Marc, I'm
> sure], having a pre-ITS for each ITS frame is not unimaginable either.
> So yes, we need to record this information in one way or the other.
Multi-ITS systems are out there. The box I'm using for a large part of
my GICv4 testing has 8 of them. It is quite common to place ITSs close
to the PCIe RC, and multi-socket systems usually have at least one per
socket.
Hopefully the pre-ITS concept is a one-off nightmare, something we'll
laugh about in two years from now. Maybe.
Thanks,
M.
--
Jazz is not dead. It just smells funny.
next prev parent reply other threads:[~2017-10-14 9:13 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-10-12 18:32 [PATCH v3 0/2] implement workaround for Socionext Synquacer pre-ITS Ard Biesheuvel
2017-10-12 18:32 ` [PATCH v3 1/2] drivers/irqchip: gicv3: probe device ID space before quirks handling Ard Biesheuvel
2017-10-12 18:32 ` [PATCH v3 2/2] drivers/irqchip: gicv3: add workaround for Synquacer pre-ITS Ard Biesheuvel
2017-10-12 19:11 ` Ard Biesheuvel
2017-10-13 10:00 ` Marc Zyngier
2017-10-12 21:34 ` Rob Herring
2017-10-12 22:24 ` Ard Biesheuvel
2017-10-13 9:47 ` Marc Zyngier
2017-10-13 9:55 ` Ard Biesheuvel
2017-10-13 10:02 ` Marc Zyngier
2017-10-13 18:50 ` Rob Herring
2017-10-13 20:38 ` Ard Biesheuvel
2017-10-14 9:13 ` Marc Zyngier [this message]
2017-10-14 9:05 ` Marc Zyngier
2017-10-17 17:31 ` Rob Herring
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=86po9qks7q.fsf@arm.com \
--to=marc.zyngier@arm$(echo .)com \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox