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

Morello 'rrlen' instruction is implemented incorrectly. #197

Open
jacobbramley opened this issue May 30, 2022 · 2 comments
Open

Morello 'rrlen' instruction is implemented incorrectly. #197

jacobbramley opened this issue May 30, 2022 · 2 comments

Comments

@jacobbramley
Copy link

Morello specifies a conservative behaviour for rrlen; some representable lengths are rounded up even if they are already representable. However, QEMU appears to use a slightly different implementation that doesn't do this. Specifically, QEMU's rrlen appears empirically consistent with setting the bounds (scbnds) on a null capability, then taking its actual length (gclen).

For example:

  • rrlen of 0x7ff8 should be 0x8000, but QEMU reports 0x7ff8.
  • rrlen of 0xfff0 should be 0x10000, but QEMU reports 0xfff0.
  • rrlen of 0x1ffe0 should be 0x20000, but QEMU reports 0x1ffe0.
  • ... // Patterns like 0xfff << n, for n >= 3.

I'm taking FVP and Morello hardware as "correct" because they both seem to match the architectural pseudocode.

This is unlikely to break software using rrlen as intended, but may lead to false assumptions.

@arichardson
Copy link
Member

QEMU currently sets bounds on a NULL capabability:

static target_ulong crap_impl(target_ulong len) {
, but it should really call the helper in CTSRD-CHERI/cheri-compressed-cap: https://github.com/CTSRD-CHERI/cheri-compressed-cap/blob/6762a19ba9fb6a4291011fa690ce9a2647af10b2/cheri_compressed_cap_common.h#L949

I think that might also be incorrect for Morello, so we need to make two changes: 1) QEMU should use the cheri-compressed-cap helper and 2) cheri-compressed-cap needs to handle the special cases and we should add unit tests.

@jrtc27
Copy link
Member

jrtc27 commented May 30, 2022

Note that what Morello does is actually wrong and results in RRLEN not being idempotent for the largest representable length for a given exponent (i.e. the all ones case that you point out), which doesn't matter if you're just using it to pad but does matter if you want to do things like assert things are representable. I have seen such an assertion panic the kernel, as ELF loader code rounded up the length to be representable and then the higher-level code asserted the segment was representable by RRLEN(len) == len, which does not hold despite len being representable. So yes, QEMU should faithfully implement the architecture, but the architecture is flawed and what QEMU does it what Morello should have done.

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

No branches or pull requests

3 participants