regtool generates a reg_steer signal that is one bit too short

[cousteaulecommandant]

x-heep/hw/vendor/pulp_platform_register_interface/vendor/lowrisc_opentitan/util/reggen/reg_top.sv.tpl

Line 192 in a639054

logic [${num_wins_width-1}:0] reg_steer;

The above line should say ${num_wins_width}, not ${num_wins_width-1}, since the definition of num_wins_width already includes the -1 (and in newer OpenTitan versions has been renamed to steer_msb which is a less misleading name):

x-heep/hw/vendor/pulp_platform_register_interface/vendor/lowrisc_opentitan/util/reggen/reg_top.sv.tpl

Line 16 in a639054

num_wins_width = ((num_wins+1).bit_length()) - 1

The following files are affected:

• hw/vendor/pulp_platform_register_interface/vendor/lowrisc_opentitan/util/reggen/reg_top.sv.tpl
• hw/vendor/pulp_platform_gpio/util/reggen/reggen/reg_top.sv.tpl
• hw/vendor/lowrisc_opentitan/util/reggen/reg_top.sv.tpl (not affected by -1 issue, but see comment below about superfluous +1)

[cousteaulecommandant]

https://github.com/lowRISC/opentitan/blob/bfed7763827d338e014aba80884273c23355009d/util/reggen/reg_top.sv.tpl#L26

In fact, in OpenTitan they have ditched the +1 in num_wins+1 since reg_steer only ever needs to take a value between 0 and num_wins (0 to num_wins-1 for a window, or num_wins for the plain register interface, i.e., num_wins+1 possible values).
This difference made the code work for num_wins=1 (as well as 3, 7, 15, etc) since it was erroneously allocating a larger than needed range, and then cutting down that range by 1 bit, and the two errors cancelled out each other in the case of powers of 2 minus 1 (1, 3, 7...) but not for any other number of windows.

In short, this error does not surface if you only have 1 window, which I guess was the most widely tested use case, but it does surface e.g. for 2 or 4 windows.

[davideschiavone]

@cousteaulecommandant sorry for the late reply - if you have a way to fix it, feel free as usual to submit a PR - as this is touching a vendorized repository, you have two choices:

1. PR to the pulp_platform register interface repository - then revendorize it here if it gets accepted
2. PR to this repository by patching the script above
3. PR to this repository by removing the pulp script and using a new one that will be maintained in X-HEEP

My favorites are in order 1, 2, and 3.

[cousteaulecommandant]

Good news. It looks like pulp-platform/register_interface already uses the logic [${steer_msb}:0] reg_steer; definition from OpenTitan, so it's just a matter of revendorizing it here.
I don't have much experience with revendorizing though, so for now I'll leave it to the experts if possible :) (if not, I might give it a try in the future)

(The change was introduced in pulp-platform/register_interface@7cf6ae7 from February 2023, in case you don't want to upgrade all the way to the newest version.)