Re: [Intel-gfx] [PATCH v6 10/15] drm/i915/pxp: interfaces for using protected objects
On Mon, Aug 16, 2021 at 08:58:49AM -0700, Daniele Ceraolo Spurio wrote:
>
>
> On 8/16/2021 8:15 AM, Daniel Vetter wrote:
> > On Fri, Aug 13, 2021 at 08:18:02AM -0700, Daniele Ceraolo Spurio wrote:
> > >
> > > On 8/13/2021 7:37 AM, Daniel Vetter wrote:
> > > > On Wed, Jul 28, 2021 at 07:01:01PM -0700, Daniele Ceraolo Spurio wrote:
> > > > > This api allow user mode to create protected buffers and to mark
> > > > > contexts as making use of such objects. Only when using contexts
> > > > > marked in such a way is the execution guaranteed to work as expected.
> > > > >
> > > > > Contexts can only be marked as using protected content at creation
> > > > > time
> > > > > (i.e. the parameter is immutable) and they must be both bannable and
> > > > > not
> > > > > recoverable.
> > > > >
> > > > > All protected objects and contexts that have backing storage will be
> > > > > considered invalid when the PXP session is destroyed and all new
> > > > > submissions using them will be rejected. All intel contexts within the
> > > > > invalidated gem contexts will be marked banned. A new flag has been
> > > > > added to the RESET_STATS ioctl to report the context invalidation to
> > > > > userspace.
> > > > >
> > > > > This patch was previously sent as 2 separate patches, which have been
> > > > > squashed following a request to have all the uapi in a single patch.
> > > > > I've retained the s-o-b from both.
> > > > >
> > > > > v5: squash patches, rebase on proto_ctx, update kerneldoc
> > > > >
> > > > > v6: rebase on obj create_ext changes
> > > > >
> > > > > Signed-off-by: Daniele Ceraolo Spurio
> > > > >
> > > > > Signed-off-by: Bommu Krishnaiah
> > > > > Cc: Rodrigo Vivi
> > > > > Cc: Chris Wilson
> > > > > Cc: Lionel Landwerlin
> > > > > Cc: Jason Ekstrand
> > > > > Cc: Daniel Vetter
> > > > > Reviewed-by: Rodrigo Vivi #v5
> > > > > ---
> > > > >drivers/gpu/drm/i915/gem/i915_gem_context.c | 68 --
> > > > >drivers/gpu/drm/i915/gem/i915_gem_context.h | 18
> > > > >.../gpu/drm/i915/gem/i915_gem_context_types.h | 2 +
> > > > >drivers/gpu/drm/i915/gem/i915_gem_create.c| 75
> > > > >.../gpu/drm/i915/gem/i915_gem_execbuffer.c| 40 -
> > > > >drivers/gpu/drm/i915/gem/i915_gem_object.c| 6 ++
> > > > >drivers/gpu/drm/i915/gem/i915_gem_object.h| 12 +++
> > > > >.../gpu/drm/i915/gem/i915_gem_object_types.h | 9 ++
> > > > >drivers/gpu/drm/i915/pxp/intel_pxp.c | 89
> > > > > +++
> > > > >drivers/gpu/drm/i915/pxp/intel_pxp.h | 15
> > > > >drivers/gpu/drm/i915/pxp/intel_pxp_session.c | 3 +
> > > > >drivers/gpu/drm/i915/pxp/intel_pxp_types.h| 5 ++
> > > > >include/uapi/drm/i915_drm.h | 55 +++-
> > > > >13 files changed, 371 insertions(+), 26 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > > > > b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > > > > index cff72679ad7c..0cd3e2d06188 100644
> > > > > --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > > > > +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > > > > @@ -77,6 +77,8 @@
> > > > >#include "gt/intel_gpu_commands.h"
> > > > >#include "gt/intel_ring.h"
> > > > > +#include "pxp/intel_pxp.h"
> > > > > +
> > > > >#include "i915_gem_context.h"
> > > > >#include "i915_trace.h"
> > > > >#include "i915_user_extensions.h"
> > > > > @@ -241,6 +243,25 @@ static int proto_context_set_persistence(struct
> > > > > drm_i915_private *i915,
> > > > > return 0;
> > > > >}
> > > > > +static int proto_context_set_protected(struct drm_i915_private *i915,
> > > > > +struct i915_gem_proto_context
> > > > > *pc,
> > > > > +bool protected)
> > > > > +{
> > > > > + int ret = 0;
> > > > > +
> > > > > + if (!intel_pxp_is_enabled(>gt.pxp))
> > > > > + ret = -ENODEV;
> > > > > + else if (!protected)
> > > > > + pc->user_flags &= ~BIT(UCONTEXT_PROTECTED);
> > > > > + else if ((pc->user_flags & BIT(UCONTEXT_RECOVERABLE)) ||
> > > > > + !(pc->user_flags & BIT(UCONTEXT_BANNABLE)))
> > > > > + ret = -EPERM;
> > > > > + else
> > > > > + pc->user_flags |= BIT(UCONTEXT_PROTECTED);
> > > > > +
> > > > > + return ret;
> > > > > +}
> > > > > +
> > > > >static struct i915_gem_proto_context *
> > > > >proto_context_create(struct drm_i915_private *i915, unsigned int
> > > > > flags)
> > > > >{
> > > > > @@ -686,6 +707,8 @@ static int set_proto_ctx_param(struct
> > > > > drm_i915_file_private *fpriv,
> > > > > ret = -EPERM;
> > > > > else if (args->value)
> > > > > pc->user_flags |= BIT(UCONTEXT_BANNABLE);
> > > > > + else if (pc->user_flags & BIT(UCONTEXT_PROTECTED))
> > > > > +
Re: [Intel-gfx] [PATCH v6 10/15] drm/i915/pxp: interfaces for using protected objects
On 8/16/2021 8:15 AM, Daniel Vetter wrote:
On Fri, Aug 13, 2021 at 08:18:02AM -0700, Daniele Ceraolo Spurio wrote:
On 8/13/2021 7:37 AM, Daniel Vetter wrote:
On Wed, Jul 28, 2021 at 07:01:01PM -0700, Daniele Ceraolo Spurio wrote:
This api allow user mode to create protected buffers and to mark
contexts as making use of such objects. Only when using contexts
marked in such a way is the execution guaranteed to work as expected.
Contexts can only be marked as using protected content at creation time
(i.e. the parameter is immutable) and they must be both bannable and not
recoverable.
All protected objects and contexts that have backing storage will be
considered invalid when the PXP session is destroyed and all new
submissions using them will be rejected. All intel contexts within the
invalidated gem contexts will be marked banned. A new flag has been
added to the RESET_STATS ioctl to report the context invalidation to
userspace.
This patch was previously sent as 2 separate patches, which have been
squashed following a request to have all the uapi in a single patch.
I've retained the s-o-b from both.
v5: squash patches, rebase on proto_ctx, update kerneldoc
v6: rebase on obj create_ext changes
Signed-off-by: Daniele Ceraolo Spurio
Signed-off-by: Bommu Krishnaiah
Cc: Rodrigo Vivi
Cc: Chris Wilson
Cc: Lionel Landwerlin
Cc: Jason Ekstrand
Cc: Daniel Vetter
Reviewed-by: Rodrigo Vivi #v5
---
drivers/gpu/drm/i915/gem/i915_gem_context.c | 68 --
drivers/gpu/drm/i915/gem/i915_gem_context.h | 18
.../gpu/drm/i915/gem/i915_gem_context_types.h | 2 +
drivers/gpu/drm/i915/gem/i915_gem_create.c| 75
.../gpu/drm/i915/gem/i915_gem_execbuffer.c| 40 -
drivers/gpu/drm/i915/gem/i915_gem_object.c| 6 ++
drivers/gpu/drm/i915/gem/i915_gem_object.h| 12 +++
.../gpu/drm/i915/gem/i915_gem_object_types.h | 9 ++
drivers/gpu/drm/i915/pxp/intel_pxp.c | 89 +++
drivers/gpu/drm/i915/pxp/intel_pxp.h | 15
drivers/gpu/drm/i915/pxp/intel_pxp_session.c | 3 +
drivers/gpu/drm/i915/pxp/intel_pxp_types.h| 5 ++
include/uapi/drm/i915_drm.h | 55 +++-
13 files changed, 371 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c
b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index cff72679ad7c..0cd3e2d06188 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -77,6 +77,8 @@
#include "gt/intel_gpu_commands.h"
#include "gt/intel_ring.h"
+#include "pxp/intel_pxp.h"
+
#include "i915_gem_context.h"
#include "i915_trace.h"
#include "i915_user_extensions.h"
@@ -241,6 +243,25 @@ static int proto_context_set_persistence(struct
drm_i915_private *i915,
return 0;
}
+static int proto_context_set_protected(struct drm_i915_private *i915,
+ struct i915_gem_proto_context *pc,
+ bool protected)
+{
+ int ret = 0;
+
+ if (!intel_pxp_is_enabled(>gt.pxp))
+ ret = -ENODEV;
+ else if (!protected)
+ pc->user_flags &= ~BIT(UCONTEXT_PROTECTED);
+ else if ((pc->user_flags & BIT(UCONTEXT_RECOVERABLE)) ||
+!(pc->user_flags & BIT(UCONTEXT_BANNABLE)))
+ ret = -EPERM;
+ else
+ pc->user_flags |= BIT(UCONTEXT_PROTECTED);
+
+ return ret;
+}
+
static struct i915_gem_proto_context *
proto_context_create(struct drm_i915_private *i915, unsigned int flags)
{
@@ -686,6 +707,8 @@ static int set_proto_ctx_param(struct drm_i915_file_private
*fpriv,
ret = -EPERM;
else if (args->value)
pc->user_flags |= BIT(UCONTEXT_BANNABLE);
+ else if (pc->user_flags & BIT(UCONTEXT_PROTECTED))
+ ret = -EPERM;
else
pc->user_flags &= ~BIT(UCONTEXT_BANNABLE);
break;
@@ -693,10 +716,12 @@ static int set_proto_ctx_param(struct
drm_i915_file_private *fpriv,
case I915_CONTEXT_PARAM_RECOVERABLE:
if (args->size)
ret = -EINVAL;
- else if (args->value)
- pc->user_flags |= BIT(UCONTEXT_RECOVERABLE);
- else
+ else if (!args->value)
pc->user_flags &= ~BIT(UCONTEXT_RECOVERABLE);
+ else if (pc->user_flags & BIT(UCONTEXT_PROTECTED))
+ ret = -EPERM;
+ else
+ pc->user_flags |= BIT(UCONTEXT_RECOVERABLE);
break;
case I915_CONTEXT_PARAM_PRIORITY:
@@ -724,6 +749,11 @@ static int set_proto_ctx_param(struct
drm_i915_file_private *fpriv,
args->value);
break;
+ case
Re: [Intel-gfx] [PATCH v6 10/15] drm/i915/pxp: interfaces for using protected objects
On Fri, Aug 13, 2021 at 08:24:44AM -0700, Daniele Ceraolo Spurio wrote:
>
>
> On 8/13/2021 7:42 AM, Daniel Vetter wrote:
> > On Fri, Aug 13, 2021 at 04:37:53PM +0200, Daniel Vetter wrote:
> > > On Wed, Jul 28, 2021 at 07:01:01PM -0700, Daniele Ceraolo Spurio wrote:
> > > > This api allow user mode to create protected buffers and to mark
> > > > contexts as making use of such objects. Only when using contexts
> > > > marked in such a way is the execution guaranteed to work as expected.
> > > >
> > > > Contexts can only be marked as using protected content at creation time
> > > > (i.e. the parameter is immutable) and they must be both bannable and not
> > > > recoverable.
> > > >
> > > > All protected objects and contexts that have backing storage will be
> > > > considered invalid when the PXP session is destroyed and all new
> > > > submissions using them will be rejected. All intel contexts within the
> > > > invalidated gem contexts will be marked banned. A new flag has been
> > > > added to the RESET_STATS ioctl to report the context invalidation to
> > > > userspace.
> > > >
> > > > This patch was previously sent as 2 separate patches, which have been
> > > > squashed following a request to have all the uapi in a single patch.
> > > > I've retained the s-o-b from both.
> > > >
> > > > v5: squash patches, rebase on proto_ctx, update kerneldoc
> > > >
> > > > v6: rebase on obj create_ext changes
> > > >
> > > > Signed-off-by: Daniele Ceraolo Spurio
> > > > Signed-off-by: Bommu Krishnaiah
> > > > Cc: Rodrigo Vivi
> > > > Cc: Chris Wilson
> > > > Cc: Lionel Landwerlin
> > > > Cc: Jason Ekstrand
> > > > Cc: Daniel Vetter
> > > > Reviewed-by: Rodrigo Vivi #v5
> > > > ---
> > > > drivers/gpu/drm/i915/gem/i915_gem_context.c | 68 --
> > > > drivers/gpu/drm/i915/gem/i915_gem_context.h | 18
> > > > .../gpu/drm/i915/gem/i915_gem_context_types.h | 2 +
> > > > drivers/gpu/drm/i915/gem/i915_gem_create.c| 75
> > > > .../gpu/drm/i915/gem/i915_gem_execbuffer.c| 40 -
> > > > drivers/gpu/drm/i915/gem/i915_gem_object.c| 6 ++
> > > > drivers/gpu/drm/i915/gem/i915_gem_object.h| 12 +++
> > > > .../gpu/drm/i915/gem/i915_gem_object_types.h | 9 ++
> > > > drivers/gpu/drm/i915/pxp/intel_pxp.c | 89 +++
> > > > drivers/gpu/drm/i915/pxp/intel_pxp.h | 15
> > > > drivers/gpu/drm/i915/pxp/intel_pxp_session.c | 3 +
> > > > drivers/gpu/drm/i915/pxp/intel_pxp_types.h| 5 ++
> > > > include/uapi/drm/i915_drm.h | 55 +++-
> > > > 13 files changed, 371 insertions(+), 26 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > > > b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > > > index cff72679ad7c..0cd3e2d06188 100644
> > > > --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > > > +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > > > @@ -77,6 +77,8 @@
> > > > #include "gt/intel_gpu_commands.h"
> > > > #include "gt/intel_ring.h"
> > > > +#include "pxp/intel_pxp.h"
> > > > +
> > > > #include "i915_gem_context.h"
> > > > #include "i915_trace.h"
> > > > #include "i915_user_extensions.h"
> > > > @@ -241,6 +243,25 @@ static int proto_context_set_persistence(struct
> > > > drm_i915_private *i915,
> > > > return 0;
> > > > }
> > > > +static int proto_context_set_protected(struct drm_i915_private *i915,
> > > > + struct i915_gem_proto_context
> > > > *pc,
> > > > + bool protected)
> > > > +{
> > > > + int ret = 0;
> > > > +
> > > > + if (!intel_pxp_is_enabled(>gt.pxp))
> > > > + ret = -ENODEV;
> > > > + else if (!protected)
> > > > + pc->user_flags &= ~BIT(UCONTEXT_PROTECTED);
> > > > + else if ((pc->user_flags & BIT(UCONTEXT_RECOVERABLE)) ||
> > > > +!(pc->user_flags & BIT(UCONTEXT_BANNABLE)))
> > > > + ret = -EPERM;
> > > > + else
> > > > + pc->user_flags |= BIT(UCONTEXT_PROTECTED);
> > > > +
> > > > + return ret;
> > > > +}
> > > > +
> > > > static struct i915_gem_proto_context *
> > > > proto_context_create(struct drm_i915_private *i915, unsigned int
> > > > flags)
> > > > {
> > > > @@ -686,6 +707,8 @@ static int set_proto_ctx_param(struct
> > > > drm_i915_file_private *fpriv,
> > > > ret = -EPERM;
> > > > else if (args->value)
> > > > pc->user_flags |= BIT(UCONTEXT_BANNABLE);
> > > > + else if (pc->user_flags & BIT(UCONTEXT_PROTECTED))
> > > > + ret = -EPERM;
> > > > else
> > > > pc->user_flags &= ~BIT(UCONTEXT_BANNABLE);
> > > > break;
> > > > @@ -693,10 +716,12 @@ static int set_proto_ctx_param(struct
> > > > drm_i915_file_private *fpriv,
> > > > case
Re: [Intel-gfx] [PATCH v6 10/15] drm/i915/pxp: interfaces for using protected objects
On Fri, Aug 13, 2021 at 08:18:02AM -0700, Daniele Ceraolo Spurio wrote:
>
>
> On 8/13/2021 7:37 AM, Daniel Vetter wrote:
> > On Wed, Jul 28, 2021 at 07:01:01PM -0700, Daniele Ceraolo Spurio wrote:
> > > This api allow user mode to create protected buffers and to mark
> > > contexts as making use of such objects. Only when using contexts
> > > marked in such a way is the execution guaranteed to work as expected.
> > >
> > > Contexts can only be marked as using protected content at creation time
> > > (i.e. the parameter is immutable) and they must be both bannable and not
> > > recoverable.
> > >
> > > All protected objects and contexts that have backing storage will be
> > > considered invalid when the PXP session is destroyed and all new
> > > submissions using them will be rejected. All intel contexts within the
> > > invalidated gem contexts will be marked banned. A new flag has been
> > > added to the RESET_STATS ioctl to report the context invalidation to
> > > userspace.
> > >
> > > This patch was previously sent as 2 separate patches, which have been
> > > squashed following a request to have all the uapi in a single patch.
> > > I've retained the s-o-b from both.
> > >
> > > v5: squash patches, rebase on proto_ctx, update kerneldoc
> > >
> > > v6: rebase on obj create_ext changes
> > >
> > > Signed-off-by: Daniele Ceraolo Spurio
> > > Signed-off-by: Bommu Krishnaiah
> > > Cc: Rodrigo Vivi
> > > Cc: Chris Wilson
> > > Cc: Lionel Landwerlin
> > > Cc: Jason Ekstrand
> > > Cc: Daniel Vetter
> > > Reviewed-by: Rodrigo Vivi #v5
> > > ---
> > > drivers/gpu/drm/i915/gem/i915_gem_context.c | 68 --
> > > drivers/gpu/drm/i915/gem/i915_gem_context.h | 18
> > > .../gpu/drm/i915/gem/i915_gem_context_types.h | 2 +
> > > drivers/gpu/drm/i915/gem/i915_gem_create.c| 75
> > > .../gpu/drm/i915/gem/i915_gem_execbuffer.c| 40 -
> > > drivers/gpu/drm/i915/gem/i915_gem_object.c| 6 ++
> > > drivers/gpu/drm/i915/gem/i915_gem_object.h| 12 +++
> > > .../gpu/drm/i915/gem/i915_gem_object_types.h | 9 ++
> > > drivers/gpu/drm/i915/pxp/intel_pxp.c | 89 +++
> > > drivers/gpu/drm/i915/pxp/intel_pxp.h | 15
> > > drivers/gpu/drm/i915/pxp/intel_pxp_session.c | 3 +
> > > drivers/gpu/drm/i915/pxp/intel_pxp_types.h| 5 ++
> > > include/uapi/drm/i915_drm.h | 55 +++-
> > > 13 files changed, 371 insertions(+), 26 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > > b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > > index cff72679ad7c..0cd3e2d06188 100644
> > > --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > > +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > > @@ -77,6 +77,8 @@
> > > #include "gt/intel_gpu_commands.h"
> > > #include "gt/intel_ring.h"
> > > +#include "pxp/intel_pxp.h"
> > > +
> > > #include "i915_gem_context.h"
> > > #include "i915_trace.h"
> > > #include "i915_user_extensions.h"
> > > @@ -241,6 +243,25 @@ static int proto_context_set_persistence(struct
> > > drm_i915_private *i915,
> > > return 0;
> > > }
> > > +static int proto_context_set_protected(struct drm_i915_private *i915,
> > > +struct i915_gem_proto_context *pc,
> > > +bool protected)
> > > +{
> > > + int ret = 0;
> > > +
> > > + if (!intel_pxp_is_enabled(>gt.pxp))
> > > + ret = -ENODEV;
> > > + else if (!protected)
> > > + pc->user_flags &= ~BIT(UCONTEXT_PROTECTED);
> > > + else if ((pc->user_flags & BIT(UCONTEXT_RECOVERABLE)) ||
> > > + !(pc->user_flags & BIT(UCONTEXT_BANNABLE)))
> > > + ret = -EPERM;
> > > + else
> > > + pc->user_flags |= BIT(UCONTEXT_PROTECTED);
> > > +
> > > + return ret;
> > > +}
> > > +
> > > static struct i915_gem_proto_context *
> > > proto_context_create(struct drm_i915_private *i915, unsigned int flags)
> > > {
> > > @@ -686,6 +707,8 @@ static int set_proto_ctx_param(struct
> > > drm_i915_file_private *fpriv,
> > > ret = -EPERM;
> > > else if (args->value)
> > > pc->user_flags |= BIT(UCONTEXT_BANNABLE);
> > > + else if (pc->user_flags & BIT(UCONTEXT_PROTECTED))
> > > + ret = -EPERM;
> > > else
> > > pc->user_flags &= ~BIT(UCONTEXT_BANNABLE);
> > > break;
> > > @@ -693,10 +716,12 @@ static int set_proto_ctx_param(struct
> > > drm_i915_file_private *fpriv,
> > > case I915_CONTEXT_PARAM_RECOVERABLE:
> > > if (args->size)
> > > ret = -EINVAL;
> > > - else if (args->value)
> > > - pc->user_flags |= BIT(UCONTEXT_RECOVERABLE);
> > > - else
> > > + else if (!args->value)
> > > pc->user_flags &=
Re: [Intel-gfx] [PATCH v6 10/15] drm/i915/pxp: interfaces for using protected objects
On 8/13/2021 7:42 AM, Daniel Vetter wrote:
On Fri, Aug 13, 2021 at 04:37:53PM +0200, Daniel Vetter wrote:
On Wed, Jul 28, 2021 at 07:01:01PM -0700, Daniele Ceraolo Spurio wrote:
This api allow user mode to create protected buffers and to mark
contexts as making use of such objects. Only when using contexts
marked in such a way is the execution guaranteed to work as expected.
Contexts can only be marked as using protected content at creation time
(i.e. the parameter is immutable) and they must be both bannable and not
recoverable.
All protected objects and contexts that have backing storage will be
considered invalid when the PXP session is destroyed and all new
submissions using them will be rejected. All intel contexts within the
invalidated gem contexts will be marked banned. A new flag has been
added to the RESET_STATS ioctl to report the context invalidation to
userspace.
This patch was previously sent as 2 separate patches, which have been
squashed following a request to have all the uapi in a single patch.
I've retained the s-o-b from both.
v5: squash patches, rebase on proto_ctx, update kerneldoc
v6: rebase on obj create_ext changes
Signed-off-by: Daniele Ceraolo Spurio
Signed-off-by: Bommu Krishnaiah
Cc: Rodrigo Vivi
Cc: Chris Wilson
Cc: Lionel Landwerlin
Cc: Jason Ekstrand
Cc: Daniel Vetter
Reviewed-by: Rodrigo Vivi #v5
---
drivers/gpu/drm/i915/gem/i915_gem_context.c | 68 --
drivers/gpu/drm/i915/gem/i915_gem_context.h | 18
.../gpu/drm/i915/gem/i915_gem_context_types.h | 2 +
drivers/gpu/drm/i915/gem/i915_gem_create.c| 75
.../gpu/drm/i915/gem/i915_gem_execbuffer.c| 40 -
drivers/gpu/drm/i915/gem/i915_gem_object.c| 6 ++
drivers/gpu/drm/i915/gem/i915_gem_object.h| 12 +++
.../gpu/drm/i915/gem/i915_gem_object_types.h | 9 ++
drivers/gpu/drm/i915/pxp/intel_pxp.c | 89 +++
drivers/gpu/drm/i915/pxp/intel_pxp.h | 15
drivers/gpu/drm/i915/pxp/intel_pxp_session.c | 3 +
drivers/gpu/drm/i915/pxp/intel_pxp_types.h| 5 ++
include/uapi/drm/i915_drm.h | 55 +++-
13 files changed, 371 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c
b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index cff72679ad7c..0cd3e2d06188 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -77,6 +77,8 @@
#include "gt/intel_gpu_commands.h"
#include "gt/intel_ring.h"
+#include "pxp/intel_pxp.h"
+
#include "i915_gem_context.h"
#include "i915_trace.h"
#include "i915_user_extensions.h"
@@ -241,6 +243,25 @@ static int proto_context_set_persistence(struct
drm_i915_private *i915,
return 0;
}
+static int proto_context_set_protected(struct drm_i915_private *i915,
+ struct i915_gem_proto_context *pc,
+ bool protected)
+{
+ int ret = 0;
+
+ if (!intel_pxp_is_enabled(>gt.pxp))
+ ret = -ENODEV;
+ else if (!protected)
+ pc->user_flags &= ~BIT(UCONTEXT_PROTECTED);
+ else if ((pc->user_flags & BIT(UCONTEXT_RECOVERABLE)) ||
+!(pc->user_flags & BIT(UCONTEXT_BANNABLE)))
+ ret = -EPERM;
+ else
+ pc->user_flags |= BIT(UCONTEXT_PROTECTED);
+
+ return ret;
+}
+
static struct i915_gem_proto_context *
proto_context_create(struct drm_i915_private *i915, unsigned int flags)
{
@@ -686,6 +707,8 @@ static int set_proto_ctx_param(struct drm_i915_file_private
*fpriv,
ret = -EPERM;
else if (args->value)
pc->user_flags |= BIT(UCONTEXT_BANNABLE);
+ else if (pc->user_flags & BIT(UCONTEXT_PROTECTED))
+ ret = -EPERM;
else
pc->user_flags &= ~BIT(UCONTEXT_BANNABLE);
break;
@@ -693,10 +716,12 @@ static int set_proto_ctx_param(struct
drm_i915_file_private *fpriv,
case I915_CONTEXT_PARAM_RECOVERABLE:
if (args->size)
ret = -EINVAL;
- else if (args->value)
- pc->user_flags |= BIT(UCONTEXT_RECOVERABLE);
- else
+ else if (!args->value)
pc->user_flags &= ~BIT(UCONTEXT_RECOVERABLE);
+ else if (pc->user_flags & BIT(UCONTEXT_PROTECTED))
+ ret = -EPERM;
+ else
+ pc->user_flags |= BIT(UCONTEXT_RECOVERABLE);
break;
case I915_CONTEXT_PARAM_PRIORITY:
@@ -724,6 +749,11 @@ static int set_proto_ctx_param(struct
drm_i915_file_private *fpriv,
args->value);
break;
+ case I915_CONTEXT_PARAM_PROTECTED_CONTENT:
+ ret =
Re: [Intel-gfx] [PATCH v6 10/15] drm/i915/pxp: interfaces for using protected objects
On 8/13/2021 7:37 AM, Daniel Vetter wrote:
On Wed, Jul 28, 2021 at 07:01:01PM -0700, Daniele Ceraolo Spurio wrote:
This api allow user mode to create protected buffers and to mark
contexts as making use of such objects. Only when using contexts
marked in such a way is the execution guaranteed to work as expected.
Contexts can only be marked as using protected content at creation time
(i.e. the parameter is immutable) and they must be both bannable and not
recoverable.
All protected objects and contexts that have backing storage will be
considered invalid when the PXP session is destroyed and all new
submissions using them will be rejected. All intel contexts within the
invalidated gem contexts will be marked banned. A new flag has been
added to the RESET_STATS ioctl to report the context invalidation to
userspace.
This patch was previously sent as 2 separate patches, which have been
squashed following a request to have all the uapi in a single patch.
I've retained the s-o-b from both.
v5: squash patches, rebase on proto_ctx, update kerneldoc
v6: rebase on obj create_ext changes
Signed-off-by: Daniele Ceraolo Spurio
Signed-off-by: Bommu Krishnaiah
Cc: Rodrigo Vivi
Cc: Chris Wilson
Cc: Lionel Landwerlin
Cc: Jason Ekstrand
Cc: Daniel Vetter
Reviewed-by: Rodrigo Vivi #v5
---
drivers/gpu/drm/i915/gem/i915_gem_context.c | 68 --
drivers/gpu/drm/i915/gem/i915_gem_context.h | 18
.../gpu/drm/i915/gem/i915_gem_context_types.h | 2 +
drivers/gpu/drm/i915/gem/i915_gem_create.c| 75
.../gpu/drm/i915/gem/i915_gem_execbuffer.c| 40 -
drivers/gpu/drm/i915/gem/i915_gem_object.c| 6 ++
drivers/gpu/drm/i915/gem/i915_gem_object.h| 12 +++
.../gpu/drm/i915/gem/i915_gem_object_types.h | 9 ++
drivers/gpu/drm/i915/pxp/intel_pxp.c | 89 +++
drivers/gpu/drm/i915/pxp/intel_pxp.h | 15
drivers/gpu/drm/i915/pxp/intel_pxp_session.c | 3 +
drivers/gpu/drm/i915/pxp/intel_pxp_types.h| 5 ++
include/uapi/drm/i915_drm.h | 55 +++-
13 files changed, 371 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c
b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index cff72679ad7c..0cd3e2d06188 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -77,6 +77,8 @@
#include "gt/intel_gpu_commands.h"
#include "gt/intel_ring.h"
+#include "pxp/intel_pxp.h"
+
#include "i915_gem_context.h"
#include "i915_trace.h"
#include "i915_user_extensions.h"
@@ -241,6 +243,25 @@ static int proto_context_set_persistence(struct
drm_i915_private *i915,
return 0;
}
+static int proto_context_set_protected(struct drm_i915_private *i915,
+ struct i915_gem_proto_context *pc,
+ bool protected)
+{
+ int ret = 0;
+
+ if (!intel_pxp_is_enabled(>gt.pxp))
+ ret = -ENODEV;
+ else if (!protected)
+ pc->user_flags &= ~BIT(UCONTEXT_PROTECTED);
+ else if ((pc->user_flags & BIT(UCONTEXT_RECOVERABLE)) ||
+!(pc->user_flags & BIT(UCONTEXT_BANNABLE)))
+ ret = -EPERM;
+ else
+ pc->user_flags |= BIT(UCONTEXT_PROTECTED);
+
+ return ret;
+}
+
static struct i915_gem_proto_context *
proto_context_create(struct drm_i915_private *i915, unsigned int flags)
{
@@ -686,6 +707,8 @@ static int set_proto_ctx_param(struct drm_i915_file_private
*fpriv,
ret = -EPERM;
else if (args->value)
pc->user_flags |= BIT(UCONTEXT_BANNABLE);
+ else if (pc->user_flags & BIT(UCONTEXT_PROTECTED))
+ ret = -EPERM;
else
pc->user_flags &= ~BIT(UCONTEXT_BANNABLE);
break;
@@ -693,10 +716,12 @@ static int set_proto_ctx_param(struct
drm_i915_file_private *fpriv,
case I915_CONTEXT_PARAM_RECOVERABLE:
if (args->size)
ret = -EINVAL;
- else if (args->value)
- pc->user_flags |= BIT(UCONTEXT_RECOVERABLE);
- else
+ else if (!args->value)
pc->user_flags &= ~BIT(UCONTEXT_RECOVERABLE);
+ else if (pc->user_flags & BIT(UCONTEXT_PROTECTED))
+ ret = -EPERM;
+ else
+ pc->user_flags |= BIT(UCONTEXT_RECOVERABLE);
break;
case I915_CONTEXT_PARAM_PRIORITY:
@@ -724,6 +749,11 @@ static int set_proto_ctx_param(struct
drm_i915_file_private *fpriv,
args->value);
break;
+ case I915_CONTEXT_PARAM_PROTECTED_CONTENT:
+ ret = proto_context_set_protected(fpriv->dev_priv, pc,
+
Re: [Intel-gfx] [PATCH v6 10/15] drm/i915/pxp: interfaces for using protected objects
On Fri, Aug 13, 2021 at 04:37:53PM +0200, Daniel Vetter wrote:
> On Wed, Jul 28, 2021 at 07:01:01PM -0700, Daniele Ceraolo Spurio wrote:
> > This api allow user mode to create protected buffers and to mark
> > contexts as making use of such objects. Only when using contexts
> > marked in such a way is the execution guaranteed to work as expected.
> >
> > Contexts can only be marked as using protected content at creation time
> > (i.e. the parameter is immutable) and they must be both bannable and not
> > recoverable.
> >
> > All protected objects and contexts that have backing storage will be
> > considered invalid when the PXP session is destroyed and all new
> > submissions using them will be rejected. All intel contexts within the
> > invalidated gem contexts will be marked banned. A new flag has been
> > added to the RESET_STATS ioctl to report the context invalidation to
> > userspace.
> >
> > This patch was previously sent as 2 separate patches, which have been
> > squashed following a request to have all the uapi in a single patch.
> > I've retained the s-o-b from both.
> >
> > v5: squash patches, rebase on proto_ctx, update kerneldoc
> >
> > v6: rebase on obj create_ext changes
> >
> > Signed-off-by: Daniele Ceraolo Spurio
> > Signed-off-by: Bommu Krishnaiah
> > Cc: Rodrigo Vivi
> > Cc: Chris Wilson
> > Cc: Lionel Landwerlin
> > Cc: Jason Ekstrand
> > Cc: Daniel Vetter
> > Reviewed-by: Rodrigo Vivi #v5
> > ---
> > drivers/gpu/drm/i915/gem/i915_gem_context.c | 68 --
> > drivers/gpu/drm/i915/gem/i915_gem_context.h | 18
> > .../gpu/drm/i915/gem/i915_gem_context_types.h | 2 +
> > drivers/gpu/drm/i915/gem/i915_gem_create.c| 75
> > .../gpu/drm/i915/gem/i915_gem_execbuffer.c| 40 -
> > drivers/gpu/drm/i915/gem/i915_gem_object.c| 6 ++
> > drivers/gpu/drm/i915/gem/i915_gem_object.h| 12 +++
> > .../gpu/drm/i915/gem/i915_gem_object_types.h | 9 ++
> > drivers/gpu/drm/i915/pxp/intel_pxp.c | 89 +++
> > drivers/gpu/drm/i915/pxp/intel_pxp.h | 15
> > drivers/gpu/drm/i915/pxp/intel_pxp_session.c | 3 +
> > drivers/gpu/drm/i915/pxp/intel_pxp_types.h| 5 ++
> > include/uapi/drm/i915_drm.h | 55 +++-
> > 13 files changed, 371 insertions(+), 26 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > index cff72679ad7c..0cd3e2d06188 100644
> > --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > @@ -77,6 +77,8 @@
> > #include "gt/intel_gpu_commands.h"
> > #include "gt/intel_ring.h"
> >
> > +#include "pxp/intel_pxp.h"
> > +
> > #include "i915_gem_context.h"
> > #include "i915_trace.h"
> > #include "i915_user_extensions.h"
> > @@ -241,6 +243,25 @@ static int proto_context_set_persistence(struct
> > drm_i915_private *i915,
> > return 0;
> > }
> >
> > +static int proto_context_set_protected(struct drm_i915_private *i915,
> > + struct i915_gem_proto_context *pc,
> > + bool protected)
> > +{
> > + int ret = 0;
> > +
> > + if (!intel_pxp_is_enabled(>gt.pxp))
> > + ret = -ENODEV;
> > + else if (!protected)
> > + pc->user_flags &= ~BIT(UCONTEXT_PROTECTED);
> > + else if ((pc->user_flags & BIT(UCONTEXT_RECOVERABLE)) ||
> > +!(pc->user_flags & BIT(UCONTEXT_BANNABLE)))
> > + ret = -EPERM;
> > + else
> > + pc->user_flags |= BIT(UCONTEXT_PROTECTED);
> > +
> > + return ret;
> > +}
> > +
> > static struct i915_gem_proto_context *
> > proto_context_create(struct drm_i915_private *i915, unsigned int flags)
> > {
> > @@ -686,6 +707,8 @@ static int set_proto_ctx_param(struct
> > drm_i915_file_private *fpriv,
> > ret = -EPERM;
> > else if (args->value)
> > pc->user_flags |= BIT(UCONTEXT_BANNABLE);
> > + else if (pc->user_flags & BIT(UCONTEXT_PROTECTED))
> > + ret = -EPERM;
> > else
> > pc->user_flags &= ~BIT(UCONTEXT_BANNABLE);
> > break;
> > @@ -693,10 +716,12 @@ static int set_proto_ctx_param(struct
> > drm_i915_file_private *fpriv,
> > case I915_CONTEXT_PARAM_RECOVERABLE:
> > if (args->size)
> > ret = -EINVAL;
> > - else if (args->value)
> > - pc->user_flags |= BIT(UCONTEXT_RECOVERABLE);
> > - else
> > + else if (!args->value)
> > pc->user_flags &= ~BIT(UCONTEXT_RECOVERABLE);
> > + else if (pc->user_flags & BIT(UCONTEXT_PROTECTED))
> > + ret = -EPERM;
> > + else
> > + pc->user_flags |= BIT(UCONTEXT_RECOVERABLE);
> > break;
> >
> > case I915_CONTEXT_PARAM_PRIORITY:
> > @@ -724,6 +749,11 @@ static int
Re: [Intel-gfx] [PATCH v6 10/15] drm/i915/pxp: interfaces for using protected objects
On Wed, Jul 28, 2021 at 07:01:01PM -0700, Daniele Ceraolo Spurio wrote:
> This api allow user mode to create protected buffers and to mark
> contexts as making use of such objects. Only when using contexts
> marked in such a way is the execution guaranteed to work as expected.
>
> Contexts can only be marked as using protected content at creation time
> (i.e. the parameter is immutable) and they must be both bannable and not
> recoverable.
>
> All protected objects and contexts that have backing storage will be
> considered invalid when the PXP session is destroyed and all new
> submissions using them will be rejected. All intel contexts within the
> invalidated gem contexts will be marked banned. A new flag has been
> added to the RESET_STATS ioctl to report the context invalidation to
> userspace.
>
> This patch was previously sent as 2 separate patches, which have been
> squashed following a request to have all the uapi in a single patch.
> I've retained the s-o-b from both.
>
> v5: squash patches, rebase on proto_ctx, update kerneldoc
>
> v6: rebase on obj create_ext changes
>
> Signed-off-by: Daniele Ceraolo Spurio
> Signed-off-by: Bommu Krishnaiah
> Cc: Rodrigo Vivi
> Cc: Chris Wilson
> Cc: Lionel Landwerlin
> Cc: Jason Ekstrand
> Cc: Daniel Vetter
> Reviewed-by: Rodrigo Vivi #v5
> ---
> drivers/gpu/drm/i915/gem/i915_gem_context.c | 68 --
> drivers/gpu/drm/i915/gem/i915_gem_context.h | 18
> .../gpu/drm/i915/gem/i915_gem_context_types.h | 2 +
> drivers/gpu/drm/i915/gem/i915_gem_create.c| 75
> .../gpu/drm/i915/gem/i915_gem_execbuffer.c| 40 -
> drivers/gpu/drm/i915/gem/i915_gem_object.c| 6 ++
> drivers/gpu/drm/i915/gem/i915_gem_object.h| 12 +++
> .../gpu/drm/i915/gem/i915_gem_object_types.h | 9 ++
> drivers/gpu/drm/i915/pxp/intel_pxp.c | 89 +++
> drivers/gpu/drm/i915/pxp/intel_pxp.h | 15
> drivers/gpu/drm/i915/pxp/intel_pxp_session.c | 3 +
> drivers/gpu/drm/i915/pxp/intel_pxp_types.h| 5 ++
> include/uapi/drm/i915_drm.h | 55 +++-
> 13 files changed, 371 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> index cff72679ad7c..0cd3e2d06188 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> @@ -77,6 +77,8 @@
> #include "gt/intel_gpu_commands.h"
> #include "gt/intel_ring.h"
>
> +#include "pxp/intel_pxp.h"
> +
> #include "i915_gem_context.h"
> #include "i915_trace.h"
> #include "i915_user_extensions.h"
> @@ -241,6 +243,25 @@ static int proto_context_set_persistence(struct
> drm_i915_private *i915,
> return 0;
> }
>
> +static int proto_context_set_protected(struct drm_i915_private *i915,
> +struct i915_gem_proto_context *pc,
> +bool protected)
> +{
> + int ret = 0;
> +
> + if (!intel_pxp_is_enabled(>gt.pxp))
> + ret = -ENODEV;
> + else if (!protected)
> + pc->user_flags &= ~BIT(UCONTEXT_PROTECTED);
> + else if ((pc->user_flags & BIT(UCONTEXT_RECOVERABLE)) ||
> + !(pc->user_flags & BIT(UCONTEXT_BANNABLE)))
> + ret = -EPERM;
> + else
> + pc->user_flags |= BIT(UCONTEXT_PROTECTED);
> +
> + return ret;
> +}
> +
> static struct i915_gem_proto_context *
> proto_context_create(struct drm_i915_private *i915, unsigned int flags)
> {
> @@ -686,6 +707,8 @@ static int set_proto_ctx_param(struct
> drm_i915_file_private *fpriv,
> ret = -EPERM;
> else if (args->value)
> pc->user_flags |= BIT(UCONTEXT_BANNABLE);
> + else if (pc->user_flags & BIT(UCONTEXT_PROTECTED))
> + ret = -EPERM;
> else
> pc->user_flags &= ~BIT(UCONTEXT_BANNABLE);
> break;
> @@ -693,10 +716,12 @@ static int set_proto_ctx_param(struct
> drm_i915_file_private *fpriv,
> case I915_CONTEXT_PARAM_RECOVERABLE:
> if (args->size)
> ret = -EINVAL;
> - else if (args->value)
> - pc->user_flags |= BIT(UCONTEXT_RECOVERABLE);
> - else
> + else if (!args->value)
> pc->user_flags &= ~BIT(UCONTEXT_RECOVERABLE);
> + else if (pc->user_flags & BIT(UCONTEXT_PROTECTED))
> + ret = -EPERM;
> + else
> + pc->user_flags |= BIT(UCONTEXT_RECOVERABLE);
> break;
>
> case I915_CONTEXT_PARAM_PRIORITY:
> @@ -724,6 +749,11 @@ static int set_proto_ctx_param(struct
> drm_i915_file_private *fpriv,
> args->value);
> break;
>
> + case I915_CONTEXT_PARAM_PROTECTED_CONTENT:
> + ret =
Re: [Intel-gfx] [PATCH v6 10/15] drm/i915/pxp: interfaces for using protected objects
On Thu, Jul 29, 2021 at 08:17:44AM -0700, Daniele Ceraolo Spurio wrote: > > > On 7/29/2021 4:10 AM, Rodrigo Vivi wrote: > > On Wed, Jul 28, 2021 at 07:01:01PM -0700, Daniele Ceraolo Spurio wrote: > > > This api allow user mode to create protected buffers and to mark > > > contexts as making use of such objects. Only when using contexts > > > marked in such a way is the execution guaranteed to work as expected. > > > > > > Contexts can only be marked as using protected content at creation time > > > (i.e. the parameter is immutable) and they must be both bannable and not > > > recoverable. > > > > > > All protected objects and contexts that have backing storage will be > > > considered invalid when the PXP session is destroyed and all new > > > submissions using them will be rejected. All intel contexts within the > > > invalidated gem contexts will be marked banned. A new flag has been > > > added to the RESET_STATS ioctl to report the context invalidation to > > > userspace. > > > > > > This patch was previously sent as 2 separate patches, which have been > > > squashed following a request to have all the uapi in a single patch. > > > I've retained the s-o-b from both. > > > > > > v5: squash patches, rebase on proto_ctx, update kerneldoc > > > > > > v6: rebase on obj create_ext changes > > The "rebase" word led me to think it was a small change caused > > only by rebasing conflicts, but then I spotted something on > > i915_gem_create.c that I didn't remember seeing before. > > Apologies for not being clear. > > > > > Since it took me a while to understand what was going on, let me > > try to summarize to see if I got it right and to check > > with others (Jason in special) > > if we are aligned with the recent directions: > > > > With the removal of the vanilla_object from the create_ext, > > the addition of the flags was needed. > > yes. > > > > > Then, instead of adding a new user_flags it adds the I915_BO_PROTECTED > > to the existent flags (what is cleaner) but then it does > > > > /* Add any flag set by create_ext options */ > > flags |= ext_flags; > > > > on the new user_create_ext. > > > > What shouldn't be a problem because the ext_flags is really only > > the I915_BO_PROTECTED set by our new extension and immutable. > > > > But I'm just trying to see if we are not opening any holes to accept > > some undesired flags. > > > > Apparently not. > > ext_flags are not set directly by the user, but by the create_ext extension > functions, based on the extensions the user has provided. This should allow > the kernel to perform any required checks in the extension functions before > setting the flag (like it happens in this case for the PXP option). > > > > > Am I getting things right? Anything else we should check in here? > > yup, got it right. Great, thanks for confirming so I'm okay with: Reviewed-by: Rodrigo Vivi > > Daniele > > > > > Thanks, > > Rodrigo. > > > > > Signed-off-by: Daniele Ceraolo Spurio > > > Signed-off-by: Bommu Krishnaiah > > > Cc: Rodrigo Vivi > > > Cc: Chris Wilson > > > Cc: Lionel Landwerlin > > > Cc: Jason Ekstrand > > > Cc: Daniel Vetter > > > Reviewed-by: Rodrigo Vivi #v5 > > > --- > > > drivers/gpu/drm/i915/gem/i915_gem_context.c | 68 -- > > > drivers/gpu/drm/i915/gem/i915_gem_context.h | 18 > > > .../gpu/drm/i915/gem/i915_gem_context_types.h | 2 + > > > drivers/gpu/drm/i915/gem/i915_gem_create.c| 75 > > > .../gpu/drm/i915/gem/i915_gem_execbuffer.c| 40 - > > > drivers/gpu/drm/i915/gem/i915_gem_object.c| 6 ++ > > > drivers/gpu/drm/i915/gem/i915_gem_object.h| 12 +++ > > > .../gpu/drm/i915/gem/i915_gem_object_types.h | 9 ++ > > > drivers/gpu/drm/i915/pxp/intel_pxp.c | 89 +++ > > > drivers/gpu/drm/i915/pxp/intel_pxp.h | 15 > > > drivers/gpu/drm/i915/pxp/intel_pxp_session.c | 3 + > > > drivers/gpu/drm/i915/pxp/intel_pxp_types.h| 5 ++ > > > include/uapi/drm/i915_drm.h | 55 +++- > > > 13 files changed, 371 insertions(+), 26 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c > > > b/drivers/gpu/drm/i915/gem/i915_gem_context.c > > > index cff72679ad7c..0cd3e2d06188 100644 > > > --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c > > > +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c > > > @@ -77,6 +77,8 @@ > > > #include "gt/intel_gpu_commands.h" > > > #include "gt/intel_ring.h" > > > +#include "pxp/intel_pxp.h" > > > + > > > #include "i915_gem_context.h" > > > #include "i915_trace.h" > > > #include "i915_user_extensions.h" > > > @@ -241,6 +243,25 @@ static int proto_context_set_persistence(struct > > > drm_i915_private *i915, > > > return 0; > > > } > > > +static int proto_context_set_protected(struct drm_i915_private *i915, > > > +struct i915_gem_proto_context *pc, > > > +
Re: [Intel-gfx] [PATCH v6 10/15] drm/i915/pxp: interfaces for using protected objects
On 7/29/2021 4:10 AM, Rodrigo Vivi wrote:
On Wed, Jul 28, 2021 at 07:01:01PM -0700, Daniele Ceraolo Spurio wrote:
This api allow user mode to create protected buffers and to mark
contexts as making use of such objects. Only when using contexts
marked in such a way is the execution guaranteed to work as expected.
Contexts can only be marked as using protected content at creation time
(i.e. the parameter is immutable) and they must be both bannable and not
recoverable.
All protected objects and contexts that have backing storage will be
considered invalid when the PXP session is destroyed and all new
submissions using them will be rejected. All intel contexts within the
invalidated gem contexts will be marked banned. A new flag has been
added to the RESET_STATS ioctl to report the context invalidation to
userspace.
This patch was previously sent as 2 separate patches, which have been
squashed following a request to have all the uapi in a single patch.
I've retained the s-o-b from both.
v5: squash patches, rebase on proto_ctx, update kerneldoc
v6: rebase on obj create_ext changes
The "rebase" word led me to think it was a small change caused
only by rebasing conflicts, but then I spotted something on
i915_gem_create.c that I didn't remember seeing before.
Apologies for not being clear.
Since it took me a while to understand what was going on, let me
try to summarize to see if I got it right and to check
with others (Jason in special)
if we are aligned with the recent directions:
With the removal of the vanilla_object from the create_ext,
the addition of the flags was needed.
yes.
Then, instead of adding a new user_flags it adds the I915_BO_PROTECTED
to the existent flags (what is cleaner) but then it does
/* Add any flag set by create_ext options */
flags |= ext_flags;
on the new user_create_ext.
What shouldn't be a problem because the ext_flags is really only
the I915_BO_PROTECTED set by our new extension and immutable.
But I'm just trying to see if we are not opening any holes to accept
some undesired flags.
Apparently not.
ext_flags are not set directly by the user, but by the create_ext
extension functions, based on the extensions the user has provided. This
should allow the kernel to perform any required checks in the extension
functions before setting the flag (like it happens in this case for the
PXP option).
Am I getting things right? Anything else we should check in here?
yup, got it right.
Daniele
Thanks,
Rodrigo.
Signed-off-by: Daniele Ceraolo Spurio
Signed-off-by: Bommu Krishnaiah
Cc: Rodrigo Vivi
Cc: Chris Wilson
Cc: Lionel Landwerlin
Cc: Jason Ekstrand
Cc: Daniel Vetter
Reviewed-by: Rodrigo Vivi #v5
---
drivers/gpu/drm/i915/gem/i915_gem_context.c | 68 --
drivers/gpu/drm/i915/gem/i915_gem_context.h | 18
.../gpu/drm/i915/gem/i915_gem_context_types.h | 2 +
drivers/gpu/drm/i915/gem/i915_gem_create.c| 75
.../gpu/drm/i915/gem/i915_gem_execbuffer.c| 40 -
drivers/gpu/drm/i915/gem/i915_gem_object.c| 6 ++
drivers/gpu/drm/i915/gem/i915_gem_object.h| 12 +++
.../gpu/drm/i915/gem/i915_gem_object_types.h | 9 ++
drivers/gpu/drm/i915/pxp/intel_pxp.c | 89 +++
drivers/gpu/drm/i915/pxp/intel_pxp.h | 15
drivers/gpu/drm/i915/pxp/intel_pxp_session.c | 3 +
drivers/gpu/drm/i915/pxp/intel_pxp_types.h| 5 ++
include/uapi/drm/i915_drm.h | 55 +++-
13 files changed, 371 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c
b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index cff72679ad7c..0cd3e2d06188 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -77,6 +77,8 @@
#include "gt/intel_gpu_commands.h"
#include "gt/intel_ring.h"
+#include "pxp/intel_pxp.h"
+
#include "i915_gem_context.h"
#include "i915_trace.h"
#include "i915_user_extensions.h"
@@ -241,6 +243,25 @@ static int proto_context_set_persistence(struct
drm_i915_private *i915,
return 0;
}
+static int proto_context_set_protected(struct drm_i915_private *i915,
+ struct i915_gem_proto_context *pc,
+ bool protected)
+{
+ int ret = 0;
+
+ if (!intel_pxp_is_enabled(>gt.pxp))
+ ret = -ENODEV;
+ else if (!protected)
+ pc->user_flags &= ~BIT(UCONTEXT_PROTECTED);
+ else if ((pc->user_flags & BIT(UCONTEXT_RECOVERABLE)) ||
+!(pc->user_flags & BIT(UCONTEXT_BANNABLE)))
+ ret = -EPERM;
+ else
+ pc->user_flags |= BIT(UCONTEXT_PROTECTED);
+
+ return ret;
+}
+
static struct i915_gem_proto_context *
proto_context_create(struct drm_i915_private *i915, unsigned int flags)
{
@@ -686,6 +707,8 @@ static int set_proto_ctx_param(struct drm_i915_file_private
*fpriv,
Re: [Intel-gfx] [PATCH v6 10/15] drm/i915/pxp: interfaces for using protected objects
On Wed, Jul 28, 2021 at 07:01:01PM -0700, Daniele Ceraolo Spurio wrote:
> This api allow user mode to create protected buffers and to mark
> contexts as making use of such objects. Only when using contexts
> marked in such a way is the execution guaranteed to work as expected.
>
> Contexts can only be marked as using protected content at creation time
> (i.e. the parameter is immutable) and they must be both bannable and not
> recoverable.
>
> All protected objects and contexts that have backing storage will be
> considered invalid when the PXP session is destroyed and all new
> submissions using them will be rejected. All intel contexts within the
> invalidated gem contexts will be marked banned. A new flag has been
> added to the RESET_STATS ioctl to report the context invalidation to
> userspace.
>
> This patch was previously sent as 2 separate patches, which have been
> squashed following a request to have all the uapi in a single patch.
> I've retained the s-o-b from both.
>
> v5: squash patches, rebase on proto_ctx, update kerneldoc
>
> v6: rebase on obj create_ext changes
The "rebase" word led me to think it was a small change caused
only by rebasing conflicts, but then I spotted something on
i915_gem_create.c that I didn't remember seeing before.
Since it took me a while to understand what was going on, let me
try to summarize to see if I got it right and to check
with others (Jason in special)
if we are aligned with the recent directions:
With the removal of the vanilla_object from the create_ext,
the addition of the flags was needed.
Then, instead of adding a new user_flags it adds the I915_BO_PROTECTED
to the existent flags (what is cleaner) but then it does
/* Add any flag set by create_ext options */
flags |= ext_flags;
on the new user_create_ext.
What shouldn't be a problem because the ext_flags is really only
the I915_BO_PROTECTED set by our new extension and immutable.
But I'm just trying to see if we are not opening any holes to accept
some undesired flags.
Apparently not.
Am I getting things right? Anything else we should check in here?
Thanks,
Rodrigo.
>
> Signed-off-by: Daniele Ceraolo Spurio
> Signed-off-by: Bommu Krishnaiah
> Cc: Rodrigo Vivi
> Cc: Chris Wilson
> Cc: Lionel Landwerlin
> Cc: Jason Ekstrand
> Cc: Daniel Vetter
> Reviewed-by: Rodrigo Vivi #v5
> ---
> drivers/gpu/drm/i915/gem/i915_gem_context.c | 68 --
> drivers/gpu/drm/i915/gem/i915_gem_context.h | 18
> .../gpu/drm/i915/gem/i915_gem_context_types.h | 2 +
> drivers/gpu/drm/i915/gem/i915_gem_create.c| 75
> .../gpu/drm/i915/gem/i915_gem_execbuffer.c| 40 -
> drivers/gpu/drm/i915/gem/i915_gem_object.c| 6 ++
> drivers/gpu/drm/i915/gem/i915_gem_object.h| 12 +++
> .../gpu/drm/i915/gem/i915_gem_object_types.h | 9 ++
> drivers/gpu/drm/i915/pxp/intel_pxp.c | 89 +++
> drivers/gpu/drm/i915/pxp/intel_pxp.h | 15
> drivers/gpu/drm/i915/pxp/intel_pxp_session.c | 3 +
> drivers/gpu/drm/i915/pxp/intel_pxp_types.h| 5 ++
> include/uapi/drm/i915_drm.h | 55 +++-
> 13 files changed, 371 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> index cff72679ad7c..0cd3e2d06188 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> @@ -77,6 +77,8 @@
> #include "gt/intel_gpu_commands.h"
> #include "gt/intel_ring.h"
>
> +#include "pxp/intel_pxp.h"
> +
> #include "i915_gem_context.h"
> #include "i915_trace.h"
> #include "i915_user_extensions.h"
> @@ -241,6 +243,25 @@ static int proto_context_set_persistence(struct
> drm_i915_private *i915,
> return 0;
> }
>
> +static int proto_context_set_protected(struct drm_i915_private *i915,
> +struct i915_gem_proto_context *pc,
> +bool protected)
> +{
> + int ret = 0;
> +
> + if (!intel_pxp_is_enabled(>gt.pxp))
> + ret = -ENODEV;
> + else if (!protected)
> + pc->user_flags &= ~BIT(UCONTEXT_PROTECTED);
> + else if ((pc->user_flags & BIT(UCONTEXT_RECOVERABLE)) ||
> + !(pc->user_flags & BIT(UCONTEXT_BANNABLE)))
> + ret = -EPERM;
> + else
> + pc->user_flags |= BIT(UCONTEXT_PROTECTED);
> +
> + return ret;
> +}
> +
> static struct i915_gem_proto_context *
> proto_context_create(struct drm_i915_private *i915, unsigned int flags)
> {
> @@ -686,6 +707,8 @@ static int set_proto_ctx_param(struct
> drm_i915_file_private *fpriv,
> ret = -EPERM;
> else if (args->value)
> pc->user_flags |= BIT(UCONTEXT_BANNABLE);
> + else if (pc->user_flags & BIT(UCONTEXT_PROTECTED))
> + ret = -EPERM;
> else
> pc->user_flags &=
[Intel-gfx] [PATCH v6 10/15] drm/i915/pxp: interfaces for using protected objects
This api allow user mode to create protected buffers and to mark
contexts as making use of such objects. Only when using contexts
marked in such a way is the execution guaranteed to work as expected.
Contexts can only be marked as using protected content at creation time
(i.e. the parameter is immutable) and they must be both bannable and not
recoverable.
All protected objects and contexts that have backing storage will be
considered invalid when the PXP session is destroyed and all new
submissions using them will be rejected. All intel contexts within the
invalidated gem contexts will be marked banned. A new flag has been
added to the RESET_STATS ioctl to report the context invalidation to
userspace.
This patch was previously sent as 2 separate patches, which have been
squashed following a request to have all the uapi in a single patch.
I've retained the s-o-b from both.
v5: squash patches, rebase on proto_ctx, update kerneldoc
v6: rebase on obj create_ext changes
Signed-off-by: Daniele Ceraolo Spurio
Signed-off-by: Bommu Krishnaiah
Cc: Rodrigo Vivi
Cc: Chris Wilson
Cc: Lionel Landwerlin
Cc: Jason Ekstrand
Cc: Daniel Vetter
Reviewed-by: Rodrigo Vivi #v5
---
drivers/gpu/drm/i915/gem/i915_gem_context.c | 68 --
drivers/gpu/drm/i915/gem/i915_gem_context.h | 18
.../gpu/drm/i915/gem/i915_gem_context_types.h | 2 +
drivers/gpu/drm/i915/gem/i915_gem_create.c| 75
.../gpu/drm/i915/gem/i915_gem_execbuffer.c| 40 -
drivers/gpu/drm/i915/gem/i915_gem_object.c| 6 ++
drivers/gpu/drm/i915/gem/i915_gem_object.h| 12 +++
.../gpu/drm/i915/gem/i915_gem_object_types.h | 9 ++
drivers/gpu/drm/i915/pxp/intel_pxp.c | 89 +++
drivers/gpu/drm/i915/pxp/intel_pxp.h | 15
drivers/gpu/drm/i915/pxp/intel_pxp_session.c | 3 +
drivers/gpu/drm/i915/pxp/intel_pxp_types.h| 5 ++
include/uapi/drm/i915_drm.h | 55 +++-
13 files changed, 371 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c
b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index cff72679ad7c..0cd3e2d06188 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -77,6 +77,8 @@
#include "gt/intel_gpu_commands.h"
#include "gt/intel_ring.h"
+#include "pxp/intel_pxp.h"
+
#include "i915_gem_context.h"
#include "i915_trace.h"
#include "i915_user_extensions.h"
@@ -241,6 +243,25 @@ static int proto_context_set_persistence(struct
drm_i915_private *i915,
return 0;
}
+static int proto_context_set_protected(struct drm_i915_private *i915,
+ struct i915_gem_proto_context *pc,
+ bool protected)
+{
+ int ret = 0;
+
+ if (!intel_pxp_is_enabled(>gt.pxp))
+ ret = -ENODEV;
+ else if (!protected)
+ pc->user_flags &= ~BIT(UCONTEXT_PROTECTED);
+ else if ((pc->user_flags & BIT(UCONTEXT_RECOVERABLE)) ||
+!(pc->user_flags & BIT(UCONTEXT_BANNABLE)))
+ ret = -EPERM;
+ else
+ pc->user_flags |= BIT(UCONTEXT_PROTECTED);
+
+ return ret;
+}
+
static struct i915_gem_proto_context *
proto_context_create(struct drm_i915_private *i915, unsigned int flags)
{
@@ -686,6 +707,8 @@ static int set_proto_ctx_param(struct drm_i915_file_private
*fpriv,
ret = -EPERM;
else if (args->value)
pc->user_flags |= BIT(UCONTEXT_BANNABLE);
+ else if (pc->user_flags & BIT(UCONTEXT_PROTECTED))
+ ret = -EPERM;
else
pc->user_flags &= ~BIT(UCONTEXT_BANNABLE);
break;
@@ -693,10 +716,12 @@ static int set_proto_ctx_param(struct
drm_i915_file_private *fpriv,
case I915_CONTEXT_PARAM_RECOVERABLE:
if (args->size)
ret = -EINVAL;
- else if (args->value)
- pc->user_flags |= BIT(UCONTEXT_RECOVERABLE);
- else
+ else if (!args->value)
pc->user_flags &= ~BIT(UCONTEXT_RECOVERABLE);
+ else if (pc->user_flags & BIT(UCONTEXT_PROTECTED))
+ ret = -EPERM;
+ else
+ pc->user_flags |= BIT(UCONTEXT_RECOVERABLE);
break;
case I915_CONTEXT_PARAM_PRIORITY:
@@ -724,6 +749,11 @@ static int set_proto_ctx_param(struct
drm_i915_file_private *fpriv,
args->value);
break;
+ case I915_CONTEXT_PARAM_PROTECTED_CONTENT:
+ ret = proto_context_set_protected(fpriv->dev_priv, pc,
+ args->value);
+ break;
+
case I915_CONTEXT_PARAM_NO_ZEROMAP:
case I915_CONTEXT_PARAM_BAN_PERIOD:
case
