[Intel-gfx] [PATCH v2 13/16] drm/i915/pxp: User interface for Protected buffer

[Lionel Landwerlin]

On 08/03/2021 22:40, Rodrigo Vivi wrote:
> On Wed, Mar 03, 2021 at 05:24:34PM -0800, Daniele Ceraolo Spurio wrote:
>>
>> On 3/3/2021 4:10 PM, Daniele Ceraolo Spurio wrote:
>>>
>>> On 3/3/2021 3:42 PM, Lionel Landwerlin wrote:
>>>> On 04/03/2021 01:25, Daniele Ceraolo Spurio wrote:
>>>>>
>>>>> On 3/3/2021 3:16 PM, Lionel Landwerlin wrote:
>>>>>> On 03/03/2021 23:59, Daniele Ceraolo Spurio wrote:
>>>>>>>
>>>>>>> On 3/3/2021 12:39 PM, Lionel Landwerlin wrote:
>>>>>>>> On 01/03/2021 21:31, Daniele Ceraolo Spurio wrote:
>>>>>>>>> From: Bommu Krishnaiah <krishnaiah.bommu at intel.com>
>>>>>>>>>
>>>>>>>>> This api allow user mode to create Protected buffers. Only contexts
>>>>>>>>> marked as protected are allowed to operate on protected buffers.
>>>>>>>>>
>>>>>>>>> We only allow setting the flags at creation time.
>>>>>>>>>
>>>>>>>>> All protected objects that have backing storage will be considered
>>>>>>>>> invalid when the session is destroyed and they
>>>>>>>>> won't be usable anymore.
>>>>>>>>>
>>>>>>>>> This is a rework of the original code by Bommu Krishnaiah. I've
>>>>>>>>> authorship unchanged since significant chunks
>>>>>>>>> have not been modified.
>>>>>>>>>
>>>>>>>>> v2: split context changes, fix defines and
>>>>>>>>> improve documentation (Chris),
>>>>>>>>>       add object invalidation logic
>>>>>>>>>
>>>>>>>>> Signed-off-by: Bommu Krishnaiah <krishnaiah.bommu at intel.com>
>>>>>>>>> Signed-off-by: Daniele Ceraolo Spurio
>>>>>>>>> <daniele.ceraolospurio at intel.com>
>>>>>>>>> Cc: Telukuntla Sreedhar <sreedhar.telukuntla at intel.com>
>>>>>>>>> Cc: Kondapally Kalyan <kalyan.kondapally at intel.com>
>>>>>>>>> Cc: Gupta Anshuman <Anshuman.Gupta at intel.com>
>>>>>>>>> Cc: Huang Sean Z <sean.z.huang at intel.com>
>>>>>>>>> Cc: Chris Wilson <chris at chris-wilson.co.uk>
>>>>>>>>> ---
>>>>>>>>>    drivers/gpu/drm/i915/gem/i915_gem_create.c    | 27 +++++++++++--
>>>>>>>>>    .../gpu/drm/i915/gem/i915_gem_execbuffer.c    | 10 +++++
>>>>>>>>>    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  | 13 ++++++
>>>>>>>>>    drivers/gpu/drm/i915/pxp/intel_pxp.c
>>>>>>>>> | 40 +++++++++++++++++++
>>>>>>>>>    drivers/gpu/drm/i915/pxp/intel_pxp.h          | 13 ++++++
>>>>>>>>>    drivers/gpu/drm/i915/pxp/intel_pxp_types.h    |  5 +++
>>>>>>>>>    include/uapi/drm/i915_drm.h                   | 22 ++++++++++
>>>>>>>>>    9 files changed, 145 insertions(+), 3 deletions(-)
>>>>>>>>>
>>>>>>>>> diff --git
>>>>>>>>> a/drivers/gpu/drm/i915/gem/i915_gem_create.c
>>>>>>>>> b/drivers/gpu/drm/i915/gem/i915_gem_create.c
>>>>>>>>> index 3ad3413c459f..d02e5938afbe 100644
>>>>>>>>> --- a/drivers/gpu/drm/i915/gem/i915_gem_create.c
>>>>>>>>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_create.c
>>>>>>>>> @@ -5,6 +5,7 @@
>>>>>>>>>      #include "gem/i915_gem_ioctls.h"
>>>>>>>>>    #include "gem/i915_gem_region.h"
>>>>>>>>> +#include "pxp/intel_pxp.h"
>>>>>>>>>      #include "i915_drv.h"
>>>>>>>>>    #include "i915_user_extensions.h"
>>>>>>>>> @@ -13,7 +14,8 @@ static int
>>>>>>>>>    i915_gem_create(struct drm_file *file,
>>>>>>>>>            struct intel_memory_region *mr,
>>>>>>>>>            u64 *size_p,
>>>>>>>>> -        u32 *handle_p)
>>>>>>>>> +        u32 *handle_p,
>>>>>>>>> +        u64 user_flags)
>>>>>>>>>    {
>>>>>>>>>        struct drm_i915_gem_object *obj;
>>>>>>>>>        u32 handle;
>>>>>>>>> @@ -35,12 +37,17 @@ i915_gem_create(struct drm_file *file,
>>>>>>>>>          GEM_BUG_ON(size != obj->base.size);
>>>>>>>>>    +    obj->user_flags = user_flags;
>>>>>>>>> +
>>>>>>>>>        ret = drm_gem_handle_create(file, &obj->base, &handle);
>>>>>>>>>        /* drop reference from allocate - handle holds it now */
>>>>>>>>>        i915_gem_object_put(obj);
>>>>>>>>>        if (ret)
>>>>>>>>>            return ret;
>>>>>>>>>    +    if (user_flags & I915_GEM_OBJECT_PROTECTED)
>>>>>>>>> +        intel_pxp_object_add(obj);
>>>>>>>>> +
>>>>>>>>>        *handle_p = handle;
>>>>>>>>>        *size_p = size;
>>>>>>>>>        return 0;
>>>>>>>>> @@ -89,11 +96,12 @@ i915_gem_dumb_create(struct drm_file *file,
>>>>>>>>>        return i915_gem_create(file,
>>>>>>>>> intel_memory_region_by_type(to_i915(dev),
>>>>>>>>>                                   mem_type),
>>>>>>>>> -                   &args->size, &args->handle);
>>>>>>>>> +                   &args->size, &args->handle, 0);
>>>>>>>>>    }
>>>>>>>>>      struct create_ext {
>>>>>>>>>        struct drm_i915_private *i915;
>>>>>>>>> +    unsigned long user_flags;
>>>>>>>>>    };
>>>>>>>>>      static int __create_setparam(struct
>>>>>>>>> drm_i915_gem_object_param *args,
>>>>>>>>> @@ -104,6 +112,19 @@ static int
>>>>>>>>> __create_setparam(struct
>>>>>>>>> drm_i915_gem_object_param *args,
>>>>>>>>>            return -EINVAL;
>>>>>>>>>        }
>>>>>>>>>    +    switch (lower_32_bits(args->param)) {
>>>>>>>>> +    case I915_OBJECT_PARAM_PROTECTED_CONTENT:
>>>>>>>>> +        if (!intel_pxp_is_enabled(&ext_data->i915->gt.pxp))
>>>>>>>>> +            return -ENODEV;
>>>>>>>>> +        if (args->size) {
>>>>>>>>> +            return -EINVAL;
>>>>>>>>> +        } else if (args->data) {
>>>>>>>>> +            ext_data->user_flags |= I915_GEM_OBJECT_PROTECTED;
>>>>>>>>> +            return 0;
>>>>>>>>> +        }
>>>>>>>>> +    break;
>>>>>>>>> +    }
>>>>>>>>> +
>>>>>>>>>        return -EINVAL;
>>>>>>>>>    }
>>>>>>>>>    @@ -148,5 +169,5 @@
>>>>>>>>> i915_gem_create_ioctl(struct drm_device *dev,
>>>>>>>>> void *data,
>>>>>>>>>        return i915_gem_create(file,
>>>>>>>>>                       intel_memory_region_by_type(i915,
>>>>>>>>>                                   INTEL_MEMORY_SYSTEM),
>>>>>>>>> -                   &args->size, &args->handle);
>>>>>>>>> +                   &args->size, &args->handle,
>>>>>>>>> ext_data.user_flags);
>>>>>>>>>    }
>>>>>>>>> diff --git
>>>>>>>>> a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
>>>>>>>>> b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
>>>>>>>>> index e503c9f789c0..d10c4fcb6aec 100644
>>>>>>>>> --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
>>>>>>>>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
>>>>>>>>> @@ -20,6 +20,7 @@
>>>>>>>>>    #include "gt/intel_gt_buffer_pool.h"
>>>>>>>>>    #include "gt/intel_gt_pm.h"
>>>>>>>>>    #include "gt/intel_ring.h"
>>>>>>>>> +#include "pxp/intel_pxp.h"
>>>>>>>>>      #include "pxp/intel_pxp.h"
>>>>>>>>>    @@ -500,6 +501,15 @@ eb_validate_vma(struct i915_execbuffer *eb,
>>>>>>>>>                 entry->offset !=
>>>>>>>>> gen8_canonical_addr(entry->offset &
>>>>>>>>> I915_GTT_PAGE_MASK)))
>>>>>>>>>            return -EINVAL;
>>>>>>>>>    +    if (i915_gem_object_is_protected(vma->obj)) {
>>>>>>>>> +        if (!intel_pxp_is_active(&vma->vm->gt->pxp))
>>>>>>>>> +            return -ENODEV;
>>>>>>>>> +        if (!i915_gem_object_has_valid_protection(vma->obj))
>>>>>>>>> +            return -EIO;
>>>>>>>>> +        if
>>>>>>>>> (!i915_gem_context_can_use_protected_content(eb->gem_context))
>>>>>>>>> +            return -EPERM;
>>>>>>>>
>>>>>>>> I think I'm running into this error.
>>>>>>> The currently agreed design is that protected objects
>>>>>>> can only be used with explicitly marked contexts,
>>>>>>> although it's not an HW requirement so we can revisit
>>>>>>> it.
>>>>>>>
>>>>>>>> When running vkcube protected under wayland, it
>>>>>>>> takes down the entire session (Xorg & gnome-shell
>>>>>>>> depending on which one is going to use the protected
>>>>>>>> buffer first).
>>>>>>> Are you saying that a single submission failure takes
>>>>>>> down the entire gnome session? that sounds like a larger
>>>>>>> problem than just this failure. Or am I misunderstanding
>>>>>>> your point?
>>>>>>
>>>>>> We just need to hand that buffer to any other app and have
>>>>>> it use it in execbuf and that will make it fail the execbuf.
>>>>>>
>>>>>> Now how does the driver in the gnome-shell/Xorg process know
>>>>>> what went wrong and what buffer caused it?
>>>>>>
>>>>>> Could be any imported buffer. It's pretty hard to recover
>>>>>> from this and most apps will just crash if the driver starts
>>>>>> to fail for some reason.
>>>>>>
>>>>>> You can see how that could make a lot of people's live terrible :/
>>>>> Don't other apps have to know that the buffer is encrypted to
>>>>> use it properly? Or is there a case where we want to use the
>>>>> object as if it wasn't encrypted?
>>>>>
>>>>> Also, PXP submissions can become invalid at any point in time
>>>>> due to a teardown event, which causes the contents of the
>>>>> encrypted buffers to become garbage. If we can't fail the
>>>>> execbuf which includes an invalidated buffer we'd have to
>>>>> silently discard the submission.
>>>>
>>>> A malicious app could start sharing protected tagged buffer with no
>>>> protected content in it, just to invalidate/crash others apps'
>>>> context.
>>> I see your point, although on the other side failing the execbuf when
>>> malicious behavior is detected seems reasonable as well.
>>>
>>>> Can the failure be only reported to protected contexts?
>>> That would allow a non-protected context to use protected objects, which
>>> is something we wanted to avoid.
>>> I see a few options here:
>>>
>>> - live with the execbuf failure on incorrect or malicious behavior
> could you please expand this option a bit further?
> Based on the scenario that Lionel described I'm not feeling this is viable...
>
>>> - silently fail execbuf if protected objects are used with non-protected
>>> contexts
> I don't like to simply silent to ignore the execbuf with no indication.
> How userspace will know it needs to resubmit the job?


A normal/unprotected context wouldn't resubmit anyway because it 
wouldn't have the logic the handle this case.

Initially I thought that the failure would only appear on protected 
contexts, because they want to present protected content properly and so 
if anything goes wrong in the protected flow, they need to know.


But an application like gnome-shell/Xorg should probably not be affected 
if they don't support protected content.


-Lionel


>
>>> - remove the restriction on protected objects being usable only by
>>> protected contexts
>>>
>>> Rodrigo, Joonas, any preference or other suggestion here?
>>>
>> BTW, whatever we decide has to also consider the other failure cases in the
>> check above:
>> - pxp not active, i.e. termination in progress
>> - invalid buffer (encrypted using old keys)
> yeap. That's the case on the current design.
>
> Is an option to save that "token" with buffers and contexts?
>
> Joonas?
>
>> Both of those could currently cause an execbuf failure even if we ignore the
>> protected context restriction, which IMO makes the third option above not
>> viable.
>>
>> Daniele
>>
>>> Daniele
>>>
>>>>
>>>> -Lionel
>>>>
>>>>
>>>>> Daniele
>>>>>
>>>>>>
>>>>>> -Lionel
>>>>>>
>>>>>>
>>>>>>>> That's a bit harsh.
>>>>>>>>
>>>>>>>>
>>>>>>>> We probably don't want this. After all the point of
>>>>>>>> encryption is that even if you can read the buffer
>>>>>>>> you won't be able to make much of its content.
>>>>>>> As mentioned above we can change this since there is no
>>>>>>> HW requirement on contexts, but need some architectural
>>>>>>> agreement. Joonas and Rodrigo can comment more here.
>>>>>>> Also note that submitting an instruction using encrypted
>>>>>>> data while the session is invalid can cause the HW to
>>>>>>> hang, which is part of the reason why we require the
>>>>>>> contexts using protected objects to be marked
>>>>>>> appropriately, so we can ban them on PXP teardown to
>>>>>>> avoid hangs.
>>>>>>>
>>>>>>> Daniele
>>>>>>>
>>>>>>>> Also useful to know you're dealing for debugging ;)
>>>>>>>>
>>>>>>>>
>>>>>>>> -Lionel
>>>>>>>>
>>>>>>>>
>>>>>>>>> +    }
>>>>>>>>> +
>>>>>>>>>        /* pad_to_size was once a reserved field, so sanitize it */
>>>>>>>>>        if (entry->flags & EXEC_OBJECT_PAD_TO_SIZE) {
>>>>>>>>>            if (unlikely(offset_in_page(entry->pad_to_size)))
>>>>>>>>> diff --git
>>>>>>>>> a/drivers/gpu/drm/i915/gem/i915_gem_object.c
>>>>>>>>> b/drivers/gpu/drm/i915/gem/i915_gem_object.c
>>>>>>>>> index 6cdff5fc5882..b321f5484ae6 100644
>>>>>>>>> --- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
>>>>>>>>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
>>>>>>>>> @@ -25,6 +25,7 @@
>>>>>>>>>    #include <linux/sched/mm.h>
>>>>>>>>>      #include "display/intel_frontbuffer.h"
>>>>>>>>> +#include "pxp/intel_pxp.h"
>>>>>>>>>    #include "i915_drv.h"
>>>>>>>>>    #include "i915_gem_clflush.h"
>>>>>>>>>    #include "i915_gem_context.h"
>>>>>>>>> @@ -72,6 +73,8 @@ void
>>>>>>>>> i915_gem_object_init(struct drm_i915_gem_object
>>>>>>>>> *obj,
>>>>>>>>>        INIT_LIST_HEAD(&obj->lut_list);
>>>>>>>>>        spin_lock_init(&obj->lut_lock);
>>>>>>>>>    +    INIT_LIST_HEAD(&obj->pxp_link);
>>>>>>>>> +
>>>>>>>>>        spin_lock_init(&obj->mmo.lock);
>>>>>>>>>        obj->mmo.offsets = RB_ROOT;
>>>>>>>>>    @@ -120,6 +123,9 @@ static void
>>>>>>>>> i915_gem_close_object(struct drm_gem_object
>>>>>>>>> *gem, struct drm_file *f
>>>>>>>>>        struct i915_lut_handle *lut, *ln;
>>>>>>>>>        LIST_HEAD(close);
>>>>>>>>>    +    if (i915_gem_object_has_valid_protection(obj))
>>>>>>>>> +        intel_pxp_object_remove(obj);
>>>>>>>>> +
>>>>>>>>>        spin_lock(&obj->lut_lock);
>>>>>>>>>        list_for_each_entry_safe(lut, ln, &obj->lut_list, obj_link) {
>>>>>>>>>            struct i915_gem_context *ctx = lut->ctx;
>>>>>>>>> diff --git
>>>>>>>>> a/drivers/gpu/drm/i915/gem/i915_gem_object.h
>>>>>>>>> b/drivers/gpu/drm/i915/gem/i915_gem_object.h
>>>>>>>>> index 366d23afbb1a..a1fa7539c0f7 100644
>>>>>>>>> --- a/drivers/gpu/drm/i915/gem/i915_gem_object.h
>>>>>>>>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h
>>>>>>>>> @@ -274,6 +274,18 @@
>>>>>>>>> i915_gem_object_needs_async_cancel(const struct
>>>>>>>>> drm_i915_gem_object *obj)
>>>>>>>>>        return i915_gem_object_type_has(obj,
>>>>>>>>> I915_GEM_OBJECT_ASYNC_CANCEL);
>>>>>>>>>    }
>>>>>>>>>    +static inline bool
>>>>>>>>> +i915_gem_object_is_protected(const struct
>>>>>>>>> drm_i915_gem_object *obj)
>>>>>>>>> +{
>>>>>>>>> +    return obj->user_flags & I915_GEM_OBJECT_PROTECTED;
>>>>>>>>> +}
>>>>>>>>> +
>>>>>>>>> +static inline bool
>>>>>>>>> +i915_gem_object_has_valid_protection(const
>>>>>>>>> struct drm_i915_gem_object *obj)
>>>>>>>>> +{
>>>>>>>>> +    return i915_gem_object_is_protected(obj) &&
>>>>>>>>> !list_empty(&obj->pxp_link);
>>>>>>>>> +}
>>>>>>>>> +
>>>>>>>>>    static inline bool
>>>>>>>>>    i915_gem_object_is_framebuffer(const struct
>>>>>>>>> drm_i915_gem_object *obj)
>>>>>>>>>    {
>>>>>>>>> diff --git
>>>>>>>>> a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
>>>>>>>>> b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
>>>>>>>>> index 0a1fdbac882e..6eee580c7aba 100644
>>>>>>>>> --- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
>>>>>>>>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
>>>>>>>>> @@ -167,6 +167,11 @@ struct drm_i915_gem_object {
>>>>>>>>>        } mmo;
>>>>>>>>>          I915_SELFTEST_DECLARE(struct list_head st_link);
>>>>>>>>> +    /**
>>>>>>>>> +     * @user_flags: small set of booleans set by the user
>>>>>>>>> +     */
>>>>>>>>> +    unsigned long user_flags;
>>>>>>>>> +#define I915_GEM_OBJECT_PROTECTED BIT(0)
>>>>>>>>>          unsigned long flags;
>>>>>>>>>    #define I915_BO_ALLOC_CONTIGUOUS BIT(0)
>>>>>>>>> @@ -290,6 +295,14 @@ struct drm_i915_gem_object {
>>>>>>>>>            bool dirty:1;
>>>>>>>>>        } mm;
>>>>>>>>>    +    /*
>>>>>>>>> +     * When the PXP session is invalidated, we
>>>>>>>>> need to mark all protected
>>>>>>>>> +     * objects as invalid. To easily do so we
>>>>>>>>> add them all to a list. The
>>>>>>>>> +     * presence on the list is used to check if
>>>>>>>>> the encryption is valid or
>>>>>>>>> +     * not.
>>>>>>>>> +     */
>>>>>>>>> +    struct list_head pxp_link;
>>>>>>>>> +
>>>>>>>>>        /** Record of address bit 17 of each page at last unbind. */
>>>>>>>>>        unsigned long *bit_17;
>>>>>>>>>    diff --git
>>>>>>>>> a/drivers/gpu/drm/i915/pxp/intel_pxp.c
>>>>>>>>> b/drivers/gpu/drm/i915/pxp/intel_pxp.c
>>>>>>>>> index 5912e4a12d94..03151cd7f4b8 100644
>>>>>>>>> --- a/drivers/gpu/drm/i915/pxp/intel_pxp.c
>>>>>>>>> +++ b/drivers/gpu/drm/i915/pxp/intel_pxp.c
>>>>>>>>> @@ -69,6 +69,8 @@ void intel_pxp_init(struct intel_pxp *pxp)
>>>>>>>>>            return;
>>>>>>>>>          mutex_init(&pxp->mutex);
>>>>>>>>> +    spin_lock_init(&pxp->lock);
>>>>>>>>> +    INIT_LIST_HEAD(&pxp->protected_objects);
>>>>>>>>>          /*
>>>>>>>>>         * we'll use the completion to check if
>>>>>>>>> there is a termination pending,
>>>>>>>>> @@ -136,11 +138,49 @@ int
>>>>>>>>> intel_pxp_wait_for_termination_completion(struct
>>>>>>>>> intel_pxp *pxp)
>>>>>>>>>        return ret;
>>>>>>>>>    }
>>>>>>>>>    +int intel_pxp_object_add(struct drm_i915_gem_object *obj)
>>>>>>>>> +{
>>>>>>>>> +    struct intel_pxp *pxp = &to_i915(obj->base.dev)->gt.pxp;
>>>>>>>>> +
>>>>>>>>> +    if (!intel_pxp_is_enabled(pxp))
>>>>>>>>> +        return -ENODEV;
>>>>>>>>> +
>>>>>>>>> +    if (!list_empty(&obj->pxp_link))
>>>>>>>>> +        return -EEXIST;
>>>>>>>>> +
>>>>>>>>> +    spin_lock_irq(&pxp->lock);
>>>>>>>>> +    list_add(&obj->pxp_link, &pxp->protected_objects);
>>>>>>>>> +    spin_unlock_irq(&pxp->lock);
>>>>>>>>> +
>>>>>>>>> +    return 0;
>>>>>>>>> +}
>>>>>>>>> +
>>>>>>>>> +void intel_pxp_object_remove(struct drm_i915_gem_object *obj)
>>>>>>>>> +{
>>>>>>>>> +    struct intel_pxp *pxp = &to_i915(obj->base.dev)->gt.pxp;
>>>>>>>>> +
>>>>>>>>> +    if (!intel_pxp_is_enabled(pxp))
>>>>>>>>> +        return;
>>>>>>>>> +
>>>>>>>>> +    spin_lock_irq(&pxp->lock);
>>>>>>>>> +    list_del_init(&obj->pxp_link);
>>>>>>>>> +    spin_unlock_irq(&pxp->lock);
>>>>>>>>> +}
>>>>>>>>> +
>>>>>>>>>    void intel_pxp_invalidate(struct intel_pxp *pxp)
>>>>>>>>>    {
>>>>>>>>>        struct drm_i915_private *i915 = pxp_to_gt(pxp)->i915;
>>>>>>>>> +    struct drm_i915_gem_object *obj, *tmp;
>>>>>>>>>        struct i915_gem_context *ctx, *cn;
>>>>>>>>>    +    /* delete objects that have been used
>>>>>>>>> with the invalidated session */
>>>>>>>>> +    spin_lock_irq(&pxp->lock);
>>>>>>>>> +    list_for_each_entry_safe(obj, tmp,
>>>>>>>>> &pxp->protected_objects, pxp_link) {
>>>>>>>>> +        if (i915_gem_object_has_pages(obj))
>>>>>>>>> +            list_del_init(&obj->pxp_link);
>>>>>>>>> +    }
>>>>>>>>> +    spin_unlock_irq(&pxp->lock);
>>>>>>>>> +
>>>>>>>>>        /* ban all contexts marked as protected */
>>>>>>>>>        spin_lock_irq(&i915->gem.contexts.lock);
>>>>>>>>>        list_for_each_entry_safe(ctx, cn,
>>>>>>>>> &i915->gem.contexts.list, link) {
>>>>>>>>> diff --git
>>>>>>>>> a/drivers/gpu/drm/i915/pxp/intel_pxp.h
>>>>>>>>> b/drivers/gpu/drm/i915/pxp/intel_pxp.h
>>>>>>>>> index e36200833095..3315b07d271b 100644
>>>>>>>>> --- a/drivers/gpu/drm/i915/pxp/intel_pxp.h
>>>>>>>>> +++ b/drivers/gpu/drm/i915/pxp/intel_pxp.h
>>>>>>>>> @@ -9,6 +9,8 @@
>>>>>>>>>    #include "gt/intel_gt_types.h"
>>>>>>>>>    #include "intel_pxp_types.h"
>>>>>>>>>    +struct drm_i915_gem_object;
>>>>>>>>> +
>>>>>>>>>    #define GEN12_DISPLAY_PXP_STATE_TERMINATED_INTERRUPT BIT(1)
>>>>>>>>>    #define GEN12_DISPLAY_APP_TERMINATED_PER_FW_REQ_INTERRUPT BIT(2)
>>>>>>>>>    #define GEN12_DISPLAY_STATE_RESET_COMPLETE_INTERRUPT BIT(3)
>>>>>>>>> @@ -38,6 +40,9 @@ void intel_pxp_init(struct intel_pxp *pxp);
>>>>>>>>>    void intel_pxp_fini(struct intel_pxp *pxp);
>>>>>>>>>      int
>>>>>>>>> intel_pxp_wait_for_termination_completion(struct
>>>>>>>>> intel_pxp *pxp);
>>>>>>>>> +
>>>>>>>>> +int intel_pxp_object_add(struct drm_i915_gem_object *obj);
>>>>>>>>> +void intel_pxp_object_remove(struct drm_i915_gem_object *obj);
>>>>>>>>>    void intel_pxp_invalidate(struct intel_pxp *pxp);
>>>>>>>>>    #else
>>>>>>>>>    static inline void intel_pxp_init(struct intel_pxp *pxp)
>>>>>>>>> @@ -52,6 +57,14 @@ static inline int
>>>>>>>>> intel_pxp_wait_for_termination_completion(struct
>>>>>>>>> intel_pxp *px
>>>>>>>>>    {
>>>>>>>>>        return 0;
>>>>>>>>>    }
>>>>>>>>> +
>>>>>>>>> +static inline int intel_pxp_object_add(struct
>>>>>>>>> drm_i915_gem_object *obj)
>>>>>>>>> +{
>>>>>>>>> +    return 0;
>>>>>>>>> +}
>>>>>>>>> +static inline void
>>>>>>>>> intel_pxp_object_remove(struct
>>>>>>>>> drm_i915_gem_object *obj)
>>>>>>>>> +{
>>>>>>>>> +}
>>>>>>>>>    #endif
>>>>>>>>>      #endif /* __INTEL_PXP_H__ */
>>>>>>>>> diff --git
>>>>>>>>> a/drivers/gpu/drm/i915/pxp/intel_pxp_types.h
>>>>>>>>> b/drivers/gpu/drm/i915/pxp/intel_pxp_types.h
>>>>>>>>> index 6f659a6f8f1c..53a2a8acfe51 100644
>>>>>>>>> --- a/drivers/gpu/drm/i915/pxp/intel_pxp_types.h
>>>>>>>>> +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_types.h
>>>>>>>>> @@ -7,8 +7,10 @@
>>>>>>>>>    #define __INTEL_PXP_TYPES_H__
>>>>>>>>>      #include <linux/completion.h>
>>>>>>>>> +#include <linux/list.h>
>>>>>>>>>    #include <linux/types.h>
>>>>>>>>>    #include <linux/mutex.h>
>>>>>>>>> +#include <linux/spinlock.h>
>>>>>>>>>    #include <linux/workqueue.h>
>>>>>>>>>      struct intel_context;
>>>>>>>>> @@ -28,6 +30,9 @@ struct intel_pxp {
>>>>>>>>>        struct work_struct irq_work;
>>>>>>>>>        bool irq_enabled;
>>>>>>>>>        u32 current_events; /* protected with gt->irq_lock */
>>>>>>>>> +
>>>>>>>>> +    struct spinlock lock;
>>>>>>>>> +    struct list_head protected_objects;
>>>>>>>>>    };
>>>>>>>>>      #endif /* __INTEL_PXP_TYPES_H__ */
>>>>>>>>> diff --git a/include/uapi/drm/i915_drm.h
>>>>>>>>> b/include/uapi/drm/i915_drm.h
>>>>>>>>> index 9ebe8523aa0c..0f8b771a6d53 100644
>>>>>>>>> --- a/include/uapi/drm/i915_drm.h
>>>>>>>>> +++ b/include/uapi/drm/i915_drm.h
>>>>>>>>> @@ -1753,6 +1753,28 @@ struct drm_i915_gem_object_param {
>>>>>>>>>     */
>>>>>>>>>    #define I915_OBJECT_PARAM  (1ull << 32)
>>>>>>>>>    +/*
>>>>>>>>> + * I915_OBJECT_PARAM_PROTECTED_CONTENT:
>>>>>>>>> + *
>>>>>>>>> + * If set to true, buffer contents is expected
>>>>>>>>> to be protected by PXP
>>>>>>>>> + * encryption and requires decryption for scan
>>>>>>>>> out and processing. This is
>>>>>>>>> + * only possible on platforms that have PXP
>>>>>>>>> enabled, on all other scenarios
>>>>>>>>> + * setting this flag will cause the ioctl to
>>>>>>>>> fail and return -ENODEV.
>>>>>>>>> + *
>>>>>>>>> + * Protected buffers can only be used with
>>>>>>>>> contexts created using the
>>>>>>>>> + * I915_CONTEXT_PARAM_PROTECTED_CONTENT flag.
>>>>>>>>> The buffer contents are
>>>>>>>>> + * considered invalid after a PXP session teardown.
>>>>>>>>> + *
>>>>>>>>> + * Given the restriction above, the following
>>>>>>>>> errors are possible when
>>>>>>>>> + * submitting a protected object in an execbuf call:
>>>>>>>>> + *
>>>>>>>>> + * -ENODEV: PXP session not currently active
>>>>>>>>> + * -EIO: buffer has become invalid after a teardown event
>>>>>>>>> + * -EPERM: buffer submitted using a context not
>>>>>>>>> marked as protected
>>>>>>>>> + */
>>>>>>>>> +#define I915_OBJECT_PARAM_PROTECTED_CONTENT  0x0
>>>>>>>>> +/* Must be kept compact -- no holes and well documented */
>>>>>>>>> +
>>>>>>>>>        __u64 param;
>>>>>>>>>          /* Data value or pointer */
>>>>>>>>
>>> _______________________________________________
>>> Intel-gfx mailing list
>>> Intel-gfx at lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx