From: "Gaurav, Kumar" <kumar.gaurav@intel.com>
To: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: "Nikula, Jani" <jani.nikula@intel.com>,
"Intel-gfx@lists.freedesktop.org"
<Intel-gfx@lists.freedesktop.org>,
"Huang, Sean Z" <sean.z.huang@intel.com>,
"Bommu, Krishnaiah" <krishnaiah.bommu@intel.com>,
"Vetter, Daniel" <daniel.vetter@intel.com>
Subject: Re: [Intel-gfx] [RFC-v23 13/13] drm/i915/pxp: Add plane decryption support
Date: Thu, 21 Jan 2021 21:34:53 +0000 [thread overview]
Message-ID: <DM6PR11MB26830728CD1B4CEEA315A0AC97A10@DM6PR11MB2683.namprd11.prod.outlook.com> (raw)
In-Reply-To: <YAnrUeLAXCPATJ2F@intel.com>
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@linux.intel.com>
Sent: Thursday, January 21, 2021 1:00 PM
To: Gaurav, Kumar <kumar.gaurav@intel.com>
Cc: Gupta, Anshuman <anshuman.gupta@intel.com>; Huang, Sean Z <sean.z.huang@intel.com>; Intel-gfx@lists.freedesktop.org; Nikula, Jani <jani.nikula@intel.com>; Bommu, Krishnaiah <krishnaiah.bommu@intel.com>; Vetter, Daniel <daniel.vetter@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@linux.intel.com>
> Sent: Thursday, January 21, 2021 12:31 PM
> To: Gupta, Anshuman <anshuman.gupta@intel.com>
> Cc: Huang, Sean Z <sean.z.huang@intel.com>;
> Intel-gfx@lists.freedesktop.org; Nikula, Jani <jani.nikula@intel.com>;
> Gaurav, Kumar <kumar.gaurav@intel.com>; Bommu, Krishnaiah
> <krishnaiah.bommu@intel.com>; Vetter, Daniel <daniel.vetter@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@intel.com>
> > > Sent: Tuesday, January 19, 2021 1:13 PM
> > > To: Intel-gfx@lists.freedesktop.org
> > > Cc: Gaurav, Kumar <kumar.gaurav@intel.com>; Gupta, Anshuman
> > > <anshuman.gupta@intel.com>; Bommu, Krishnaiah
> > > <krishnaiah.bommu@intel.com>; Huang, Sean Z
> > > <sean.z.huang@intel.com>
> > > Subject: [RFC-v23 13/13] drm/i915/pxp: Add plane decryption
> > > support
> > >
> > > From: Anshuman Gupta <anshuman.gupta@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@intel.com>
> > > Cc: Huang Sean Z <sean.z.huang@intel.com>
> > > Cc: Gaurav Kumar <kumar.gaurav@intel.com>
> > > Signed-off-by: Anshuman Gupta <anshuman.gupta@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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2021-01-21 21:35 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-19 7:43 [Intel-gfx] [RFC-v23 00/13] Introduce Intel PXP component - Mesa single session Huang, Sean Z
2021-01-19 7:43 ` [Intel-gfx] [RFC-v23 01/13] drm/i915/pxp: Introduce Intel PXP component Huang, Sean Z
2021-01-21 16:08 ` Chris Wilson
2021-01-19 7:43 ` [Intel-gfx] [RFC-v23 02/13] drm/i915/pxp: set KCR reg init during the boot time Huang, Sean Z
2021-01-21 17:04 ` Chris Wilson
2021-01-19 7:43 ` [Intel-gfx] [RFC-v23 03/13] drm/i915/pxp: Implement funcs to create the TEE channel Huang, Sean Z
2021-01-19 7:43 ` [Intel-gfx] [RFC-v23 04/13] drm/i915/pxp: Create the arbitrary session after boot Huang, Sean Z
2021-01-19 7:43 ` [Intel-gfx] [RFC-v23 05/13] drm/i915/pxp: Func to send hardware session termination Huang, Sean Z
2021-01-19 7:43 ` [Intel-gfx] [RFC-v23 06/13] drm/i915/pxp: Enable PXP irq worker and callback stub Huang, Sean Z
2021-01-19 7:43 ` [Intel-gfx] [RFC-v23 07/13] drm/i915/pxp: Destroy arb session upon teardown Huang, Sean Z
2021-01-19 7:43 ` [Intel-gfx] [RFC-v23 08/13] drm/i915/pxp: Enable PXP power management Huang, Sean Z
2021-01-19 7:43 ` [Intel-gfx] [RFC-v23 09/13] drm/i915/pxp: Expose session state for display protection flip Huang, Sean Z
2021-01-21 19:47 ` Rodrigo Vivi
2021-01-19 7:43 ` [Intel-gfx] [RFC-v23 10/13] mei: pxp: export pavp client to me client bus Huang, Sean Z
2021-01-19 7:43 ` [Intel-gfx] [RFC-v23 11/13] drm/i915/uapi: introduce drm_i915_gem_create_ext Huang, Sean Z
2021-01-19 7:43 ` [Intel-gfx] [RFC-v23 12/13] drm/i915/pxp: User interface for Protected buffer Huang, Sean Z
2021-01-19 7:43 ` [Intel-gfx] [RFC-v23 13/13] drm/i915/pxp: Add plane decryption support Huang, Sean Z
2021-01-19 9:35 ` Gupta, Anshuman
2021-01-21 20:30 ` Ville Syrjälä
2021-01-21 20:50 ` Gaurav, Kumar
2021-01-21 21:00 ` Ville Syrjälä
2021-01-21 21:34 ` Gaurav, Kumar [this message]
2021-01-22 11:58 ` Ville Syrjälä
2021-01-22 17:25 ` Gaurav, Kumar
2021-01-19 12:33 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Introduce Intel PXP component - Mesa single session (rev23) Patchwork
2021-01-19 13:03 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
2021-01-23 14:18 ` [Intel-gfx] [RFC-v23 00/13] Introduce Intel PXP component - Mesa single session Lionel Landwerlin
2021-01-23 21:47 ` Gaurav, Kumar
2021-01-25 15:38 ` Lionel Landwerlin
2021-01-25 16:22 ` Gaurav, Kumar
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DM6PR11MB26830728CD1B4CEEA315A0AC97A10@DM6PR11MB2683.namprd11.prod.outlook.com \
--to=kumar.gaurav@intel.com \
--cc=Intel-gfx@lists.freedesktop.org \
--cc=daniel.vetter@intel.com \
--cc=jani.nikula@intel.com \
--cc=krishnaiah.bommu@intel.com \
--cc=sean.z.huang@intel.com \
--cc=ville.syrjala@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).