On Thu, 30 Sep 2021 18:12:11 -0400 Alyssa Rosenzweig alyssa@collabora.com wrote:
- /* Executable implies readable */
- if ((args->flags & PANFROST_BO_NOREAD) &&
!(args->flags & PANFROST_BO_NOEXEC))return -EINVAL;Generally, executable also implies not-writeable. Should we check that?
We were allowing it until now, so doing that would break the backward compat, unfortunately.
Not a problem if you only enforce this starting with the appropriate UABI version, but...
I still don't see how that solves the <old-userspace,new-kernel> situation, since old-userspace doesn't know about the new UABI, and there's no version field on the CREATE_BO ioctl() to let the kernel know about the UABI used by this userspace program. I mean, we could add one, or add a new PANFROST_BO_EXTENDED_FLAGS flag to enforce this 'noexec implies nowrite' behavior, but is it really simpler than explicitly passing the NOWRITE flag when NOEXEC is passed?
Steve also mentioned that the DDK might use shaders modifying other shaders here [1]
What? I believe it, but what?
For the case of pilot shaders, that shouldn't require self-modifying code. As I understand, the DDK binds the push uniform (FAU / RMU) buffer as global shader memory (SSBO) and uses regular STORE instructions on it. That requires writability on that BO but that should be fine.
Okay.