Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

armstub7: Also reset CNTVOFF #113

Closed
wants to merge 3 commits into from

Conversation

popcornmix
Copy link
Contributor

See: raspberrypi/linux#3579 (comment)
Ping @lategoodbye - is this what was wanted?

Currently only one free instruction for GIC code below 0x100
so move it above 0x100 live armstub8
@lategoodbye
Copy link

Unfortunately i'm not that assembler / ARM expert, but from my limited PoV this looks like the requested ones. The timer frequency is already been set.

Maybe @vianpl wants to take a look.

@vianpl
Copy link

vianpl commented Jun 16, 2020

Like @lategoodbye I'm not an expert in such low level matters. That said, it looks correct to me. We could go ahead and drop "arm,cpu-registers-not-fw-configured" altogether, but I guess we'd have to time it with a firmware release. Let me do some extra research/testing to really validate this.

As for raspberrypi/linux#3579, FYI, it's still being looked at and I doubt this will fix the underlying issue (albeit probably mask it):
https://lkml.kernel.org/lkml/CAK8P3a2+_a+Qj8uvSToLKwf_pj14EouAFzEO6Un5uc8fT9Vr-w@mail.gmail.com/

@lategoodbye
Copy link

The DT property "arm,cpu-registers-not-fw-configured" is really ugly and now hard to remove. Maybe in mainline we should do this (first) only for RPi 4 because of the few users.

I agree regarding raspberrypi/linux#3579 because this is an orthogonal issue.

@pelwell
Copy link
Contributor

pelwell commented Jun 16, 2020

Am I right in thinking that "armstub7: Increase GIC version to 0x200" is required in order for "armstub7: Also reset CNTVOFF" to fit? We might get some complaints from users of other OSs expecting to be able to put their DTB at 0x100, but some headroom would be nice.

@popcornmix
Copy link
Contributor Author

Yes, currently one free word is available for armstub8-32-gic.bin.
I spent a little while looking for something to squeeze but that's been done a few times and there wasn't anything too obvious (and we may want to tweak the code in the future).

@pelwell
Copy link
Contributor

pelwell commented Jun 16, 2020

I suppose the GIC / noddy-DTB-handling intersection is probably vanishingly small.

@popcornmix
Copy link
Contributor Author

Yes - I made sure non-GIC code wasn't increased.

pelwell pushed a commit that referenced this pull request Oct 8, 2020
- The bootstub has been completely rewritten taking advantage of the
  Thumb-2 instruction set, which results in major space gains
- Res0 bits of NSACR are no longer set (supersedes #85)
- CNTVOFF is set to zero, now consistent with armstub8 (supersedes #113)
- SMC instructions are now disabled, now consistent with armstub8
- ACTLR is now configured to allow Non-secure access to several CPU
  configuration registers (CPUACTLR/CPUECTLR/L2CTLR/L2ECTLR/L2ACTLR),
  which makes it possible to e.g. enable Spectre v4 mitigations directly
  in the kernel without needing a separate bootstub variant
  (potentially supersedes #115)

Free space in each affected bootstub after this commit:

  armstub7.bin:        108 bytes
  armstub8-32.bin:     104 bytes
  armstub8-32-gic.bin:  44 bytes (!)
@popcornmix popcornmix closed this Oct 8, 2020
@popcornmix
Copy link
Contributor Author

This fix was included in #117

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants