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 <pal...@dabbelt.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

Attachment: signature.asc
Description: PGP signature

Reply via email to