[Intel-gfx] [RFC-v23 13/13] drm/i915/pxp: Add plane decryption support
Gaurav, Kumar
kumar.gaurav at intel.com
Thu Jan 21 21:34:53 UTC 2021
So the idea is to perform HWDRM session check in flip IRQL and i915 PXP will guarantee that IRQL is not blocked.
We have done similar check in Windows flip IRQL.
-----Original Message-----
From: Ville Syrjälä <ville.syrjala at linux.intel.com>
Sent: Thursday, January 21, 2021 1:00 PM
To: Gaurav, Kumar <kumar.gaurav at intel.com>
Cc: Gupta, Anshuman <anshuman.gupta at intel.com>; Huang, Sean Z <sean.z.huang at intel.com>; Intel-gfx at lists.freedesktop.org; Nikula, Jani <jani.nikula at intel.com>; Bommu, Krishnaiah <krishnaiah.bommu at intel.com>; Vetter, Daniel <daniel.vetter at intel.com>
Subject: Re: [RFC-v23 13/13] drm/i915/pxp: Add plane decryption support
On Thu, Jan 21, 2021 at 08:50:18PM +0000, Gaurav, Kumar wrote:
> Thanks Anshuman for adding me for review.
>
> Actually, using plane Gamma is good idea to show black frame. Another option could be alpha value since we know for ChromeOS protected buffer will always be flipped on overlays.
>
> Below explanation captures need for black frame in i915 Display for
> HWDRM protected surfaces - Problem Statement - There is race condition
> between Ring3 and Ring0 where encrypted frame could be flipped by i915 Display despite Ring3 checking if HWDRM session keys are valid for encrypted frame.
>
> Google Bug -
> BUG1 -[Intel] i915 framebuffer tracking (protected surfaces that can't
> be decrypted are being rendered as encrypted) -b/155511255
>
> Background -
> There are 4 high level pipelines working together in HWDRM playback.
> 1. CDM Pipeline -
> App CDM SW Stack -> LibVA/iHD -> i915 -> MEI -> CSME-FW
>
> 2. Media(Audio/Video) Pipeline
> App Media SW Stack -> LibVA/iHD -> i915 -> GPU
>
> 3. 3D Pipeline in Compositor
> App Composition SW Stack -> OpenGL/MESA/MiniGBM -> i915 -> GPU/Display
>
> 4. Display Pipeline in Compositor
> App Composition SW Stack -> Ozone/MiniGBM -> i915 -> Display
>
> Discussion Point -
> Even after Pipeline #4 is context robustness compliant there is a
> corner case/race condition for corruption as following - BUG1 App's
> Composition SW Stack -> Creates Protected Context and Protected
> Buffer(MiniGBM) App's Composition SW Stack -> Supplies Protected
> Buffer to LibVA/iHD -> i915 -> GPU -> Encrypted decoded output App's
> Composition SW Stack -> Gets back decode output -> Checks for context
> robustness -> Submits frame for flip -> i915 Display(by the time i915
> Display gets flip PAVP session is invalid despite being atomic since
> invalidation of PAVP is HW async event) -> Display HW -> Shows
> corruption
It'll always be racy unless the invalidation becomes a two stage process that first tells display to stop scanning out the thing and then waits for the display to finish scanning out the current frame.
>
>
> -----Original Message-----
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Sent: Thursday, January 21, 2021 12:31 PM
> To: Gupta, Anshuman <anshuman.gupta at intel.com>
> Cc: Huang, Sean Z <sean.z.huang at intel.com>;
> Intel-gfx at lists.freedesktop.org; Nikula, Jani <jani.nikula at intel.com>;
> Gaurav, Kumar <kumar.gaurav at intel.com>; Bommu, Krishnaiah
> <krishnaiah.bommu at intel.com>; Vetter, Daniel <daniel.vetter at intel.com>
> Subject: Re: [RFC-v23 13/13] drm/i915/pxp: Add plane decryption
> support
>
> On Tue, Jan 19, 2021 at 09:35:18AM +0000, Gupta, Anshuman wrote:
> > Jani/Ville
> > I had received an offline comment form Gaurav on this patch, See
> > below,
> > > -----Original Message-----
> > > From: Huang, Sean Z <sean.z.huang at intel.com>
> > > Sent: Tuesday, January 19, 2021 1:13 PM
> > > To: Intel-gfx at lists.freedesktop.org
> > > Cc: Gaurav, Kumar <kumar.gaurav at intel.com>; Gupta, Anshuman
> > > <anshuman.gupta at intel.com>; Bommu, Krishnaiah
> > > <krishnaiah.bommu at intel.com>; Huang, Sean Z
> > > <sean.z.huang at intel.com>
> > > Subject: [RFC-v23 13/13] drm/i915/pxp: Add plane decryption
> > > support
> > >
> > > From: Anshuman Gupta <anshuman.gupta at intel.com>
> > >
> > > Add support to enable/disable PLANE_SURF Decryption Request bit.
> > > It requires only to enable plane decryption support when following
> > > condition met.
> > > 1. PXP session is enabled.
> > > 2. Buffer object is protected.
> > >
> > > v2:
> > > - Rebased to libva_cp-drm-tip_tgl_cp tree.
> > > - Used gen fb obj user_flags instead gem_object_metadata.
> > > [Krishna]
> > >
> > > v3:
> > > - intel_pxp_gem_object_status() API changes.
> > >
> > > Cc: Bommu Krishnaiah <krishnaiah.bommu at intel.com>
> > > Cc: Huang Sean Z <sean.z.huang at intel.com>
> > > Cc: Gaurav Kumar <kumar.gaurav at intel.com>
> > > Signed-off-by: Anshuman Gupta <anshuman.gupta at intel.com>
> > > ---
> > > drivers/gpu/drm/i915/display/intel_sprite.c | 21 ++++++++++++++++++---
> > > drivers/gpu/drm/i915/i915_reg.h | 1 +
> > > 2 files changed, 19 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c
> > > b/drivers/gpu/drm/i915/display/intel_sprite.c
> > > index cf3589fd0ddb..39f8c922ce66 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_sprite.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_sprite.c
> > > @@ -39,6 +39,8 @@
> > > #include <drm/drm_plane_helper.h> #include <drm/drm_rect.h>
> > >
> > > +#include "pxp/intel_pxp.h"
> > > +
> > > #include "i915_drv.h"
> > > #include "i915_trace.h"
> > > #include "i915_vgpu.h"
> > > @@ -768,6 +770,11 @@ icl_program_input_csc(struct intel_plane *plane,
> > > PLANE_INPUT_CSC_POSTOFF(pipe, plane_id, 2), 0x0); }
> > >
> > > +static bool intel_fb_obj_protected(const struct
> > > +drm_i915_gem_object
> > > +*obj) {
> > > + return obj->user_flags & I915_BO_PROTECTED ? true : false; }
> > > +
> > > static void
> > > skl_plane_async_flip(struct intel_plane *plane,
> > > const struct intel_crtc_state *crtc_state, @@ -804,6
> > > +811,7 @@ skl_program_plane(struct intel_plane *plane,
> > > u32 surf_addr = plane_state->color_plane[color_plane].offset;
> > > u32 stride = skl_plane_stride(plane_state, color_plane);
> > > const struct drm_framebuffer *fb = plane_state->hw.fb;
> > > + const struct drm_i915_gem_object *obj = intel_fb_obj(fb);
> > > int aux_plane = intel_main_to_aux_plane(fb, color_plane);
> > > int crtc_x = plane_state->uapi.dst.x1;
> > > int crtc_y = plane_state->uapi.dst.y1; @@ -814,7 +822,7 @@
> > > skl_program_plane(struct intel_plane *plane,
> > > u8 alpha = plane_state->hw.alpha >> 8;
> > > u32 plane_color_ctl = 0, aux_dist = 0;
> > > unsigned long irqflags;
> > > - u32 keymsk, keymax;
> > > + u32 keymsk, keymax, plane_surf;
> > > u32 plane_ctl = plane_state->ctl;
> > >
> > > plane_ctl |= skl_plane_ctl_crtc(crtc_state); @@ -890,8 +898,15
> > > @@ skl_program_plane(struct intel_plane *plane,
> > > * the control register just before the surface register.
> > > */
> > > intel_de_write_fw(dev_priv, PLANE_CTL(pipe, plane_id), plane_ctl);
> > > - intel_de_write_fw(dev_priv, PLANE_SURF(pipe, plane_id),
> > > - intel_plane_ggtt_offset(plane_state) + surf_addr);
> > > + plane_surf = intel_plane_ggtt_offset(plane_state) + surf_addr;
> > > +
> > > + if (intel_pxp_gem_object_status(dev_priv) &&
> > > + intel_fb_obj_protected(obj))
> > > + plane_surf |= PLANE_SURF_DECRYPTION_ENABLED;
> > Here in case of if fb obj is protected but pxp session is not enabled i.e intel_pxp_gem_object_status() returns false, request to show the black frame buffer on display instead of corrupted data.
> > plane_surf = 0xXXX; //Pointer to black
> > framebuffer But above approach would be a hack.
> > @Jani and @Ville could you please guide with the general way of handling this as pxp session keys can be invalidated at any time.
>
> Would need such a black buffer to be always pinned into the gtt, which is seems a bit wasteful. We could perhaps just force the plane to output black eg. by using the plane gamma. I think we should always have the per-plane gamma available on skl+ universal planes. Cursor may be a different story.
>
> --
> Ville Syrjälä
> Intel
--
Ville Syrjälä
Intel
More information about the Intel-gfx
mailing list