Re: [PATCH] drm/panfrost: Add PANFROST_BO_NO{READ,WRITE} flags

[Boris Brezillon <boris.brezillon@collabora.com>]

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.