There have been quite some changes https://github.com/riscv/riscv-fast-interrupt/blob/master/clic.adoc. It would be good to review and implement these.

According to the RISC-V CLIC spec, the memory layout of the memory mapped registers should be as follows:

  0x1000+4*i 1B/input R or RW clicintip[i]
  0x1001+4*i 1B/input RW clicintie[i]
  0x1002+4*i 1B/input RW clicintattr[i]
  0x1003+4*i 1B/input RW clicintctl[i]
  ...
  0x4FFC 1B/input R or RW clicintip[4095]
  0x4FFD 1B/input RW clicintie[4095]
  0x4FFE 1B/input RW clicintattr[4095]
  0x4FFF 1B/input RW clicintctl[4095]

The clicint[ip/ie/attr/clt] registers have a width of 1 byte. See https://github.com/riscv/riscv-fast-interrupt/blob/master/clic.adoc#clic-memory-map

However, in the current implementation they are 4 bytes wide.

Great to see this appearing and thanks for the contribution to open-source hardware, but the blurb is misleading.
The CLIC is still under development and is not standardized yet. Please be clear in the writeup that this is an implementation of an intermediate development version and spec will change in substantial software-visible ways before ratification. Thanks.

In CLINT mode, are reserved for special interrupt sources like software and timer interrupts. CLIC specification has this section:

https://github.com/riscv/riscv-fast-interrupt/blob/master/clic.adoc#clic-interrupt-id-ordering-recommendations

Does it make sense to prepare the IP for those configurations? Seems very realistic to me one would adopt those.

Most interrupt lists have gaps. Usually because a chip is available in many configuration with different peripherals. Sometimes sources are reserved to ensure you can grow in the future without reshuffling the interrupt map. Even the RISC-V definition of interrupts has this. Therefore, a top level parameter is needed to mask out certain sources.

parameter [N_SOURCE-1:0] MASK = {N_SOURCE{1'b1}}

I do not see a top level parameter to specify the number of interrupt levels. This would be important, as most designs will only need a few, not 256. I would expect between 3 and 5 level bits to be used, with the rest fixed to one.
The CLIC specification defines a standard parameter, but that definition is currently being reworked. Thus, I would suggest waiting on riscv/riscv-fast-interrupt#331 to complete

clicintattr.trig allows run-time configurability of edge vs level and active high vs low.

CLIC states:

Some implementations may want to save these bits so only certain trigger types are supported. In this case, these bits become hard-wired to fixed values (WARL).

This makes a lot of sense, as this is something that doesn't change at run-time. Will need a new eloboration time parameter.

Currently, we are forcing a 256 bytes alignment on xtvec.

Hi,
I am linting the clic_apb module with Verilator using verilator --lint-only --top-module clic_apb $(bender script verilator).
There are a few warnings, but no errors.

However, when I change the parameter INTCTLBITS of clic_apb to a non-default value (e.g. 4), I get the following error:

%Error: [...]/clic/src/clic.sv:114:19: SEL unexpected in assignment to unpacked array
  114 |     .prio_i      (intctl),
      |                   ^~~~~~

This happens because signal intctl has a hardcoded width of 8 and not INTCTLBITS bits (same for signal irq_max).
The error can likely be fixed by changing the width of these signals, but I am not sure if any other logic needs to be changed also.