On Thu, 30 Sep 2021 18:12:11 -0400 Alyssa Rosenzweig <alyssa@xxxxxxxxxxxxx> 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.
