| Message ID | 20230307-rust-drm-v1-13-917ff5bc80a8@asahilina.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Rust DRM subsystem abstractions (& preview AGX driver) | expand |
On Tue, Mar 07, 2023 at 11:25:38PM +0900, Asahi Lina wrote: > Drivers may want to support driver-private objects, which cannot be > shared. This allows them to share a single lock and enables other > optimizations. > > Add an `exportable` field to drm_gem_object, which blocks PRIME export > if set to false. It is initialized to true in > drm_gem_private_object_init. > > Signed-off-by: Asahi Lina <lina@asahilina.net> Two comments on this: - for kernel objects which userspace never access itself the usual approach is to simply not install a gem handle on that drm_file. If userspace doesn't even have a handle they also can't export it. I think that should take care of the kernel object case you have in the asahi driver. - for the vm-private object case you need some more checks anyway, since you can't even use such objects on a different vm within the same drm_file. Maybe the gpuva helpers can eventually cover this, but in general these driver cases are handled by simply overwriting the ->export case, you can check there for vm_id.is_none() and if that's not the case, hand the actual exporting to the helper function. Whether this is done in the rust wrappers and you keep the set_exportable or just in asahi code is kinda meh, but personally for consistency I'd put that into asahi code. Imo it's much clearer when you explicitly list (by coding them into your export impl) the reasons why a buffer isn't exportable, instead of forcing people to chase set_exportable calls throughout the codebase. But also a bit matters of taste :-) Either way (unless a missed a case) this should imo be handled in asahi code and not in C or the rust glue. -Daniel > --- > drivers/gpu/drm/drm_gem.c | 1 + > drivers/gpu/drm/drm_prime.c | 5 +++++ > include/drm/drm_gem.h | 8 ++++++++ > 3 files changed, 14 insertions(+) > > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c > index 7a3cb08dc942..152ad9295a8d 100644 > --- a/drivers/gpu/drm/drm_gem.c > +++ b/drivers/gpu/drm/drm_gem.c > @@ -166,6 +166,7 @@ void drm_gem_private_object_init(struct drm_device *dev, > > drm_vma_node_reset(&obj->vma_node); > INIT_LIST_HEAD(&obj->lru_node); > + obj->exportable = true; > } > EXPORT_SYMBOL(drm_gem_private_object_init); > > diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c > index f924b8b4ab6b..9d2dd982580e 100644 > --- a/drivers/gpu/drm/drm_prime.c > +++ b/drivers/gpu/drm/drm_prime.c > @@ -391,6 +391,11 @@ static struct dma_buf *export_and_register_object(struct drm_device *dev, > return dmabuf; > } > > + if (!obj->exportable) { > + dmabuf = ERR_PTR(-EINVAL); > + return dmabuf; > + } > + > if (obj->funcs && obj->funcs->export) > dmabuf = obj->funcs->export(obj, flags); > else > diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h > index 772a4adf5287..852dec3cf763 100644 > --- a/include/drm/drm_gem.h > +++ b/include/drm/drm_gem.h > @@ -361,6 +361,14 @@ struct drm_gem_object { > * The current LRU list that the GEM object is on. > */ > struct drm_gem_lru *lru; > + > + /** > + * @exportable: > + * > + * Whether this GEM object can be exported via the drm_gem_object_funcs->export > + * callback. Defaults to true. > + */ > + bool exportable; > }; > > /** > > -- > 2.35.1 >
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index 7a3cb08dc942..152ad9295a8d 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -166,6 +166,7 @@ void drm_gem_private_object_init(struct drm_device *dev, drm_vma_node_reset(&obj->vma_node); INIT_LIST_HEAD(&obj->lru_node); + obj->exportable = true; } EXPORT_SYMBOL(drm_gem_private_object_init); diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index f924b8b4ab6b..9d2dd982580e 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -391,6 +391,11 @@ static struct dma_buf *export_and_register_object(struct drm_device *dev, return dmabuf; } + if (!obj->exportable) { + dmabuf = ERR_PTR(-EINVAL); + return dmabuf; + } + if (obj->funcs && obj->funcs->export) dmabuf = obj->funcs->export(obj, flags); else diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h index 772a4adf5287..852dec3cf763 100644 --- a/include/drm/drm_gem.h +++ b/include/drm/drm_gem.h @@ -361,6 +361,14 @@ struct drm_gem_object { * The current LRU list that the GEM object is on. */ struct drm_gem_lru *lru; + + /** + * @exportable: + * + * Whether this GEM object can be exported via the drm_gem_object_funcs->export + * callback. Defaults to true. + */ + bool exportable; }; /**
Drivers may want to support driver-private objects, which cannot be shared. This allows them to share a single lock and enables other optimizations. Add an `exportable` field to drm_gem_object, which blocks PRIME export if set to false. It is initialized to true in drm_gem_private_object_init. Signed-off-by: Asahi Lina <lina@asahilina.net> --- drivers/gpu/drm/drm_gem.c | 1 + drivers/gpu/drm/drm_prime.c | 5 +++++ include/drm/drm_gem.h | 8 ++++++++ 3 files changed, 14 insertions(+)