[ot] Define (unused) OTP scrambling properties for EG

[jwnrt]

Earl Grey does not support OTP scrambling and does not have these parameters declared. QEMU complains when they are provided by the INI file generated by `cfggen.py.

This PR declares the properties but does not use them to silence this warning. It also removes the default values set in ot_{darjeeling,earlgrey} to avoid silent misconfigurations if these change.

I did not notice that this was an issue before because I've been erroneously using -T earlgrey and not EarlGrey so the table was called ot-otp-e and wasn't being used.

[rivos-eblot]

I did not notice that this was an issue before because I've been erroneously using -T earlgrey and not EarlGrey so the table was called ot-otp-e and wasn't being used.

Have you tried with the latest cfggen.py version? There was a leftover opentitan[0] in the initial change for #117 but it has been addressed before the merge. If not, there is another issue :-(

[rivos-eblot]

Temporarily disable these constants for Earl Grey.

Would it be better to define and not use yet those properties in ot_otp_eg rather than adding a special case in the script (I may have miss something though)?

[jwnrt]

Have you tried with the latest cfggen.py version?

Yes, here's the generated file from the latest ot-earlgrey-9.1.0 and OpenTitan master:

$ scripts/opentitan/cfggen.py ~/projects/opentitan -T earlgrey -o cfg.ini

[ot_device "ot-otp-eg"]
  digest_const = "e95f517cb98955b4d5a89aa9109294a"
  digest_iv = "bead91d5fa4e0915"
  sram_const = "4a22d4b78fe0266fbee3958332f2939b"
  sram_iv = "f98c48b1f9377284"

[ot_device "ot-lc_ctrl"]
  lc_state_first = "ee75b407d2314d2ef84185ac8c990f536071632c086d4c924070be92d2948d6228b2711e9b2d8c4d"
  lc_state_last = "ee75fe0ffe7b6f3ffc5f9ffd9ff96fdb7f736f6c9e6fdcd35277fef2d3bdcd6ffbb2f59fdf3fbedd"
  lc_trscnt_first = "dfb6c45a241f85ce9f42229e8627462fdb02c6701242f14b41891180045c09c26c52744267c04aa055921b9461bb07da"
  lc_trscnt_last = "dfb6f4fabf1fefcebf5ba2ffc677c6afdbabcefeb672f36b4fbdb3988dfe1be67e7e77ca77c76af7ddde3b9e7fbfe7de"
  ownership_first = "3c61398d626885931da660ed2ab18a0f"
  ownership_last = "bc757dbdeaf9b7d35de7e9efabfbbecf"
  raw_unlock_token = "ea2b3f32cbe77554e43c8ea7ebf197c2"
  socdbg_first = "440af80d"
  socdbg_last = "6c9ef9cf"

I also noticed that:

• the ot-rom_ctrl table has been removed, is that intentional?
• the ot-otp-eg.scrmbl_key field has been removed. This might be due to upstream OpenTitan changes moving this to another file?

[jwnrt]

Temporarily disable these constants for Earl Grey.

Would it be better to define and not use yet those properties in ot_otp_eg rather than adding a special case in the script (I may have miss something though)?

Yes, this is probably a better solution, I just went with a quicker hack. I can do this properly if you'd prefer.

[rivos-eblot]

Yes

No sure to what it applies :) Is it ok now?

• the ot-rom_ctrl table has been removed, is that intentional?
• the ot-otp-eg.scrmbl_key field has been removed. This might be due to upstream OpenTitan changes moving this to another file?

No, not intentional. I'll have a look at both these issues.

[rivos-eblot]

Yes, this is probably a better solution, I just went with a quicker hack. I can do this properly if you'd prefer.

Since we want to deliver it to the master branch, I think it would be better yes, thanks.
I think it would be also safer to remove the default values in the ot_earlgrey.c as well, so that if no config file is provided, OTP fails rather than massaging data with the wrong values, which could cause some debug headaches :). I initially kept them so the command line was simpler, but there are now too many configuration parameters for OTP. This was a bad idea :)

[jwnrt]

No sure to what it applies :) Is it ok now?

I've fixed my original comment

[jwnrt]

I've updated the PR, the properties are now declared but not used in EG and I've removed the default values for DJ. I haven't tested DJ, but I can see warnings in otp_ctrl_dj.c already so I think this is handled.

[rivos-eblot]

LGTM

[AlexJones0]

LGTM, thanks @jwnrt.

[rivos-eblot]

nit pick: maybe update the PR title, as there is no more changes in cfggen.py

[rivos-eblot]

No, not intentional. I'll have a look at both these issues.

Both are due to a regression I've unfortunately added with #117. Working in on a patch

[rivos-eblot]

Both are due to a regression I've unfortunately added with #117. Working in on a patch

#124