linux-kernel - Re: [patches] [PATCH v7 05/15] irqchip: New RISC-V PLIC Driver

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]

Date:   Fri, 4 Aug 2017 00:22:50 +0200
From:   Jonathan Neuschäfer <j.neuschaefer@....net>
To:     patches@...ups.riscv.org
Cc:     peterz@...radead.org, tglx@...utronix.de, jason@...edaemon.net,
        marc.zyngier@....com, Arnd Bergmann <arnd@...db.de>,
        yamada.masahiro@...ionext.com, mmarek@...e.com, albert@...ive.com,
        will.deacon@....com, boqun.feng@...il.com, oleg@...hat.com,
        mingo@...hat.com, daniel.lezcano@...aro.org,
        gregkh@...uxfoundation.org, jslaby@...e.com, davem@...emloft.net,
        mchehab@...nel.org, hverkuil@...all.nl, rdunlap@...radead.org,
        viro@...iv.linux.org.uk, mhiramat@...nel.org, fweisbec@...il.com,
        mcgrof@...nel.org, dledford@...hat.com, bart.vanassche@...disk.com,
        sstabellini@...nel.org, mpe@...erman.id.au,
        rmk+kernel@...linux.org.uk, paul.gortmaker@...driver.com,
        nicolas.dichtel@...nd.com, linux@...ck-us.net,
        heiko.carstens@...ibm.com, schwidefsky@...ibm.com,
        geert@...ux-m68k.org, akpm@...ux-foundation.org, jiri@...lanox.com,
        vgupta@...opsys.com, airlied@...hat.com, jk@...abs.org,
        chris@...is-wilson.co.uk, Jason@...c4.com,
        paulmck@...ux.vnet.ibm.com, ncardwell@...gle.com,
        linux-kernel@...r.kernel.org, linux-kbuild@...r.kernel.org,
        Palmer Dabbelt <palmer@...belt.com>
Subject: Re: [patches] [PATCH v7 05/15] irqchip: New RISC-V PLIC Driver

Hi,

On Mon, Jul 31, 2017 at 05:59:59PM -0700, Palmer Dabbelt wrote:
> This patch adds a driver for the Platform Level Interrupt Controller
> (PLIC) specified as part of the RISC-V supervisor level ISA manual.
> The PLIC connocts global interrupt sources to the local interrupt

s/connocts/connects/

> controller on each hart.  A PLIC is present on all RISC-V systems.
> 
> Signed-off-by: Palmer Dabbelt <palmer@...belt.com>
> ---
[...]
> +/*
> + * From the RISC-V Privlidged Spec v1.10:
> + *
> + * Global interrupt sources are assigned small unsigned integer identifiers,
> + * beginning at the value 1.  An interrupt ID of 0 is reserved to mean “no
> + * interrupt”.  Interrupt identifiers are also used to break ties when two or
> + * more interrupt sources have the same assigned priority. Smaller values of
> + * interrupt ID take precedence over larger values of interrupt ID.
> + *
> + * While the RISC-V supervisor spec doesn't define the maximum number of
> + * devices supported by the PLIC, the largest number supported by devices
> + * marked as 'riscv,plic0' (which is the only device type this driver supports,
> + * and is the only extant PLIC as of now) is 1024.  As mentioned above, device
> + * 0 is defined to be non-existant so this device really only supports 1023
> + * devices.
> + */
> +#define MAX_DEVICES	1024
> +#define MAX_CONTEXTS	15872

How do you derive 15872 as the value of MAX_CONTEXTS?

> +
> +/*
> + * The PLIC consists of memory-mapped control registers, with a memory map as
> + * follows:
> + *
> + * base + 0x000000: Reserved (interrupt source 0 does not exist)
> + * base + 0x000004: Interrupt source 1 priority
> + * base + 0x000008: Interrupt source 2 priority
> + * ...
> + * base + 0x000FFC: Interrupt source 1023 priority
> + * base + 0x001000: Pending 0
> + * base + 0x001FFF: Pending

"Pending"?

> + * base + 0x002000: Enable bits for sources 0-31 on context 0
> + * base + 0x002004: Enable bits for sources 32-63 on context 0
> + * ...

> + * base + 0x0020FC: Enable bits for sources 992-1023 on context 0
> + * base + 0x002080: Enable bits for sources 0-31 on context 1

This seems to overlap. Are more than 512 sources per context actually
possible?

> + * ...
> + * base + 0x002100: Enable bits for sources 0-31 on context 2
> + * ...
> + * base + 0x1F1F80: Enable bits for sources 992-1023 on context 15871
> + * base + 0x1F1F84: Reserved
> + * ...              (higher context IDs would fit here, but wouldn't fit
> + *                   inside the per-context priority vector)
> + * base + 0x1FFFFC: Reserved
> + * base + 0x200000: Priority threshold for context 0
> + * base + 0x200004: Claim/complete for context 0
> + * base + 0x200008: Reserved
> + * ...
> + * base + 0x200FFC: Reserved
> + * base + 0x201000: Priority threshold for context 1
> + * base + 0x201004: Claim/complete for context 1
> + * ...
> + * base + 0xFFE000: Priority threshold for context 15871
> + * base + 0xFFE004: Claim/complete for context 15871
> + * base + 0xFFF008: Reserved

0xFFE004 and 0xFFF008 are 0x1004 bytes apart. Is 0xFFF008 a typo?

> + * ...
> + * base + 0xFFFFFC: Reserved

As far as I can see, given that the Priority threshold/Claim/complete
area begins at base+0x200000 and ends at base+0x1000000 (exclusive), and
the space occupied by one context is 0x1000 bytes, there should be space
for (0x1000000-0x200000)/0x1000 = 0xe00 = 3584, not 15872 contexts.
Am I missing something?


Jonathan Neuschäfer

Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)