linux-kernel - Re: [PATCH 08/17] dts: include documentation for the RISC-V interrupt controllers

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20170619143027.GA4433@leverpostej>
Date:   Mon, 19 Jun 2017 15:30:51 +0100
From:   Mark Rutland <mark.rutland@....com>
To:     Wesley Terpstra <wesley@...ive.com>
Cc:     Geert Uytterhoeven <geert@...ux-m68k.org>,
        Palmer Dabbelt <palmer@...belt.com>,
        Linux-Arch <linux-arch@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Arnd Bergmann <arnd@...db.de>, Olof Johansson <olof@...om.net>,
        Albert Ou <albert@...ive.com>, patches@...ups.riscv.org,
        Thomas Gleixner <tglx@...utronix.de>,
        Jason Cooper <jason@...edaemon.net>,
        Marc Zyngier <Marc.Zyngier@....com>,
        Rob Herring <robh+dt@...nel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>
Subject: Re: [PATCH 08/17] dts: include documentation for the RISC-V
 interrupt controllers

On Fri, Jun 09, 2017 at 02:58:14PM -0700, Wesley Terpstra wrote:
> Ugh. Clicked reply without being done writing the reply!
> 
> On Thu, Jun 8, 2017 at 3:52 AM, Mark Rutland <mark.rutland@....com> wrote:
> > Edge vs level, active high vs active low. Typically some of these are
> > programmable, and are described as flags in the interrupt-specifier.
> >
> > See the examples in:
> >
> > Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
> 
> Ok. Those are not really relevant to the PLIC. It has a flat interrupt
> namespace and the way you handle all four kinds of interrupts in the
> driver is uniform. I don't think we want to architecturally expose
> this to the operating system.

Sure thing. I was just making sure I clarified my initial statement.

> > So do any CSRs affect the state of the PLIC? If it's just MMIO, the
> > mention of CSRs above is just a little confusing.
> 
> HLIC = CSR only. PLIC = MMIO only.
> 
> > It might be best to just say "The PLIC is connect to the HLIC embedded
> > in each RISC-V core".
> 
> Fair enough.
> 
> > It sounds like we'd need these to distinguish edge/level interrupts,
> > unless that's fixed at integration time and you can determine it from
> > the PLIC itself?
> 
> They are fixed at integration time and the PLIC driver does not need to care.

Ok.

> > Please update the binding to explicitly define the ordering requirement.
> 
> The ordering requirement is that the first interrupts-extended entry
> corresponds to the first context, second to second, etc. If a context
> is unused for some reason, that's when you need a -1. The contexts are
> linear and contiguous in the MMIO address map.

Ah. It wasn't at all clear to me that the order was in relation to the
MMIO space.

How is it possible to tell if an context is unused?

Are those mandated to be linear and contiguous in the address space?

How is the relationship between an MMIO context and hart determined?

I'm not sure that it makes sense to use -1 in this manner, since the
first cell of each interrupts-extended tuple entry is a phandle. IIRC
zero is currently de-facto invalid, and if we really need a NULL
phandle, we should get that clarified in the DT spec.

It might be better to describe each MMIO context separately, and then
associate IRQs with those in-order.

> 
> > Does this mean that you expect Linux logical CPU IDs to be equal to
> > physical CPU IDs in all cases?
> 
> No. There is no 'physical CPU ID' anyway, except the mhartid CSR which
> is unavailable to linux. The SBI conveys your CPU ID by passing it as
> the first argument to the linux kernel.
> 
> The interrupts-extended in the PLIC uses a phandle to reference the
> matching CPU in DTS. The num in cpu@<num> only need correspond to the
> first register argument to the kernel.

So you use the reference to the HLIC to implictly provide the CPU
affinity? Or do you assume that these align with the SBI-provided CPU
IDs?

Thanks,
Mark.