Hi Jani, let me know if you need more info or more changes are needed. Thanks! On Wed, May 25, 2022 at 3:31 PM Mark Yacoub <markyacoub@xxxxxxxxxxxx> wrote: > > Hi Jani, thanks for your review. I got all the user space > implementation ready to see it in context. > > libdrm patch to wrap this functionality: > https://www.spinics.net/lists/dri-devel/msg347318.html > > Chromium user space implementation making direct use of the new prop flag: > crrev.com/c/3655850 > The first call made to such functionality is in > https://chromium-review.googlesource.com/c/chromium/src/+/3655850/2/ui/display/manager/content_protection_key_manager.cc#137 > where the call stack flows to the libdrm wrapper call at > https://chromium-review.googlesource.com/c/chromium/src/+/3655850/2/ui/ozone/platform/drm/gpu/drm_display.cc#203 > > I also wrote an IGT test to verify the intended behavior: > https://patchwork.freedesktop.org/patch/487331/?series=104373&rev=1 > > Let me know if I would need to update the commit message with any of > the aforementioned context. > > Thanks! > -Mark Yacoub > > On Tue, May 17, 2022 at 3:53 AM Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx> wrote: > > > > On Tue, 10 May 2022, Mark Yacoub <markyacoub@xxxxxxxxxxxx> wrote: > > > [Why] > > > User space might need to inject data into the kernel without allowing it > > > to be read again by any user space. > > > An example of where this is particularly useful is secret keys fetched > > > by user space and injected into the kernel to enable content protection. > > > > I think we're going to need more than an example in the commit > > message. See Documentation/gpu/drm-uapi.rst. > > > > BR, > > Jani. > > > > > > > > > > [How] > > > Create a DRM_MODE_CREATE_BLOB_WRITE_ONLY flag used by user space to > > > create a blob and mark the blob as write only. > > > On reading back the blob, data will be not be copied if it's a write > > > only blob > > > > > > Signed-off-by: Mark Yacoub <markyacoub@xxxxxxxxxxxx> > > > > > > --- > > > drivers/gpu/drm/drm_property.c | 3 ++- > > > include/drm/drm_property.h | 2 ++ > > > include/uapi/drm/drm_mode.h | 6 ++++++ > > > 3 files changed, 10 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/drm_property.c b/drivers/gpu/drm/drm_property.c > > > index dfec479830e4..afedf7109d00 100644 > > > --- a/drivers/gpu/drm/drm_property.c > > > +++ b/drivers/gpu/drm/drm_property.c > > > @@ -765,7 +765,7 @@ int drm_mode_getblob_ioctl(struct drm_device *dev, > > > if (!blob) > > > return -ENOENT; > > > > > > - if (out_resp->length == blob->length) { > > > + if (out_resp->length == blob->length && !blob->is_write_only) { > > > if (copy_to_user(u64_to_user_ptr(out_resp->data), > > > blob->data, > > > blob->length)) { > > > @@ -800,6 +800,7 @@ int drm_mode_createblob_ioctl(struct drm_device *dev, > > > ret = -EFAULT; > > > goto out_blob; > > > } > > > + blob->is_write_only = out_resp->flags & DRM_MODE_CREATE_BLOB_WRITE_ONLY; > > > > > > /* Dropping the lock between create_blob and our access here is safe > > > * as only the same file_priv can remove the blob; at this point, it is > > > diff --git a/include/drm/drm_property.h b/include/drm/drm_property.h > > > index 65bc9710a470..700782f021b9 100644 > > > --- a/include/drm/drm_property.h > > > +++ b/include/drm/drm_property.h > > > @@ -205,6 +205,7 @@ struct drm_property { > > > * &drm_mode_config.property_blob_list. > > > * @head_file: entry on the per-file blob list in &drm_file.blobs list. > > > * @length: size of the blob in bytes, invariant over the lifetime of the object > > > + * @is_write_only: user space can't read the blob data. > > > * @data: actual data, embedded at the end of this structure > > > * > > > * Blobs are used to store bigger values than what fits directly into the 64 > > > @@ -219,6 +220,7 @@ struct drm_property_blob { > > > struct list_head head_global; > > > struct list_head head_file; > > > size_t length; > > > + bool is_write_only; > > > void *data; > > > }; > > > > > > diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h > > > index 0a0d56a6158e..de192d3813e9 100644 > > > --- a/include/uapi/drm/drm_mode.h > > > +++ b/include/uapi/drm/drm_mode.h > > > @@ -1107,6 +1107,9 @@ struct drm_format_modifier { > > > __u64 modifier; > > > }; > > > > > > +#define DRM_MODE_CREATE_BLOB_WRITE_ONLY \ > > > + (1 << 0) /* data of the blob can't be read by user space */ > > > + > > > /** > > > * struct drm_mode_create_blob - Create New blob property > > > * > > > @@ -1120,6 +1123,9 @@ struct drm_mode_create_blob { > > > __u32 length; > > > /** @blob_id: Return: new property ID. */ > > > __u32 blob_id; > > > + /** Flags for special handling. */ > > > + __u32 flags; > > > + __u32 pad; > > > }; > > > > > > /** > > > > -- > > Jani Nikula, Intel Open Source Graphics Center
