-
Notifications
You must be signed in to change notification settings - Fork 165
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
Add ELF flags for some profile-0.8 extensions #351
base: master
Are you sure you want to change the base?
Conversation
v0.8 of the profile specification defines Za64rs, which is defined as "Reservation sets are contiguous, naturally aligned, and a maximum of 64 bytes." Signed-off-by: Palmer Dabbelt <[email protected]>
v0.8 of the profile specification defines Ziccif, which is defined as "Main memory regions with both the cacheability and coherence PMAs must support instruction fetch, and any instruction fetches of naturally aligned power-of-2 sizes up to min(ILEN,XLEN) (i.e., 32 bits for RVA20) are atomic." Signed-off-by: Palmer Dabbelt <[email protected]>
If they have ISA names then they should be in the ISA string encoded in the .riscv.attributes? EF_RISCV_TSO only exists because its definition in the spec predates encoding the ISA string in object files (ditto RVC which is both redundant and not expressive enough by itself for all the Zc* that now exist). Given the limited number of flag bits they should really only be used for fundamental things that have significant effects on the ABI, like the floating-point ABI, otherwise we'll quickly run out of bits. |
Za64rs is just an additional restriction for Zicbo*. Any Zicbo* compatible code should also work with and without Za64rs. Why is a dedicated flag needed? Similar situation for Ziccif, which also only reduces the possible implementation variants. |
That's true for Ztso as well. This issue is that software that depends on these additional restrictions won't run correctly on implementations that don't follow them. |
Yes, SW built for za64rs won't run (correctly) on a system that has a cache block size different than 64 bytes. |
At least for the standard extensions aimed at portable systems we have non-conflicting instruction encodings and can thus detect these via traps (and even emulate them if necessary). We punted on that for Zfinx and friends under the rationale they're aimed at non-binary-portable systems, but the profile stuff seems pretty squarely aimed at binary-portable systems. These don't trap, though, they just silently produce different results. There's no good way to detect that during execution, so our only option is to just prevent them from executing in the first place. |
That seems reasonable. I'm happy to stash these somewhere other than the ELF header bits, Mark was just pretty adamant that the profile stuff should go there at the Cauldron (though it wasn't clear that makes any sense, there's not even enough bits to do this right now). Given how rapidly these show up we're going to run out of bits pretty fast. IMO it's best to just have a specific attribute defining these? That way it's clear and not mixed in to the ISA string parsing which gets kind of hairy. I'd want to go hook attributes into the Linux and glibc ELF loaders first, though -- it seems manageable, but the early ELF loading stuff tends to be wacky and I might be missing something. |
I came here to say basically this... these extensions are a little different from others, because omitting most extensions will cause noisy failure (SIGILL) vs. silently doing the wrong thing. But I agree using the ISA string is fine, as long as we're willing to validate these extensions in the ISA string anywhere we would've been willing to validate them using ELF flags. |
Of course, I agree on the silent different result aspect. Still, I don't think that illegal instructions should be gracefully handled without a purpose or that we should execute binaries that are built for extensions that are not explicitly listed as implemented by the execution environment. E.g.: A system that decides to implement zba via trap-and-emulate can do so. But then it would list zba as an implemented extension on that system. So under the assumption that we have a compatibility check that prevents binaries from running in incompatible environments, dedicated flags for za64rs and ziccif should not be needed. |
You mean the ISA string in the attributes? That one's not really usable to base any behavior on, there's been so many different flavors that ended up in binaries it's kind of just meaningless. I think there was some plan for adding another one, but we end up with the same pile of complexity that ended up driving the Linux hwprobe stuff towards the bitmap-based interface. That's not really meant to be statically encoded in binaries, but maybe the right answer there is to deal with whatever fallout comes from making it suitable for that? I haven't really thought about it, so not sure what (if any) problems we'd have. |
I'm agnostic as to the mechanism. I mostly meant to say that, if the reason we were going to add ELF flags was to validate that it's safe to load the ELFs, then whatever alternative to ELF flags we use should also be validated. |
Ya, I don't think there's much of an argument there, just whether we have a viable mechanism to do that already. |
I am a bit worried that we'll keep adding new fields instead of unifying our compatibility checks. We currently already define one tag that pretty much means "this binary requires all of the following features present" and (consequently) we can use a subset relationship to test for compatibility. However, if we want to have a binary that works exactly for zic64b and zic256b (let's say exactly two ifuncs provided), but not for zic128b, we'll be in trouble. Admittedly, processing these will require more cycles than dedicated flag fields, but it will reduce the number of mechanisms and provide a forward-compatible way to support the next parameterizable feature. |
@ptomsich 's approach sound like more scalable. |
These are essentially the same as Ztso: they're new requirements on existing instructions, with no way to probe them in the ISA. This treats them essentially the same way.