linux-kernel - Re: [PATCH] dt-bindings: interrupt-controller: sifive, plic: Fix number of interrupts

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAMuHMdXpmnmAy7cvA4b9LZe1skzjyOmYgUMgY7KiLRPznSj+2Q@mail.gmail.com>
Date:   Fri, 26 Nov 2021 09:58:25 +0100
From:   Geert Uytterhoeven <geert@...ux-m68k.org>
To:     Jessica Clarke <jrtc27@...c27.com>
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        Marc Zyngier <maz@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Palmer Dabbelt <palmer@...belt.com>,
        Paul Walmsley <paul.walmsley@...ive.com>,
        Sagar Kadam <sagar.kadam@...ive.com>,
        "linux-kernel@...r.kernel.org List" <linux-kernel@...r.kernel.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@...r.kernel.org>,
        linux-riscv <linux-riscv@...ts.infradead.org>,
        Michael Zhu <michael.zhu@...rfivetech.com>
Subject: Re: [PATCH] dt-bindings: interrupt-controller: sifive, plic: Fix
 number of interrupts

Hi Jessica,

On Thu, Nov 25, 2021 at 5:08 PM Jessica Clarke <jrtc27@...c27.com> wrote:
> On 25 Nov 2021, at 15:22, Geert Uytterhoeven <geert@...ux-m68k.org> wrote:
> > To improve human readability and enable automatic validation, the tuples
> > in "interrupts-extended" properties should be grouped using angle
> > brackets.  As the DT bindings lack an upper bound on the number of
> > interrupts, thus assuming one, proper grouping is currently flagged as
> > an error.

Rob: Is this a bug in the tooling that should be fixed?
Regardless, specifying a real upper limit is always a good idea.

> > Fix this by adding the missing "maxItems", limiting it to 9 interrupts
> > (one interrupt for a system management core, and two interrupts per core
> > for other cores), which should be sufficient for now.
>
> This is SiFive’s IP, so is this actually true? I would imagine it’s
> just parameterised and could be generated with as many targets as fit
> in the MMIO space, and that this is thus inaccurate. Besides, such a

Yes, this is implementation-defined.  I just used the maximum value
currently in use.

drivers/irqchip/irq-sifive-plic.c has #define MAX_CONTEXTS 15872,
which matches the value of CONTEXT_PER_HART and the available address
space in the driver and in [1].
Would you be more comfortable with "maxItems: 15872"?
Or do you prefer setting the maximum based on the compatible value
(2 for k210, 9 for fu540, 4 for jh7100[2])?

> function change should be made separately from the grouping change.

OK, will split in v2.

> The same goes for your equivalent sifive,clint0 patch.

OK.

> > Group the tuples in the example.
> >
> > Signed-off-by: Geert Uytterhoeven <geert@...ux-m68k.org>
> > ---
> > .../interrupt-controller/sifive,plic-1.0.0.yaml      | 12 ++++++------
> > 1 file changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml
> > index 08d5a57ce00ff446..198b373f984f3438 100644
> > --- a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml
> > +++ b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml
> > @@ -61,6 +61,7 @@ properties:
> >
> >   interrupts-extended:
> >     minItems: 1
> > +    maxItems: 9
> >     description:
> >       Specifies which contexts are connected to the PLIC, with "-1" specifying
> >       that a context is not present. Each node pointed to should be a

[1] https://static.dev.sifive.com/U54-MC-RVCoreIP.pdf
[2] The PLIC memory map in
    https://github.com/starfive-tech/JH7100_Docs/blob/main/vic_u7_manual_with_creativecommons.pdf
    indicate other values of ENABLE_PER_HART and CONTEXT_PER_HART?
    Is memory the map correct?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds