media driver implementation for the LibVA content protection interface #1086

shiqiangyu · 2020-11-18T02:25:01Z

No description provided.

Xiaogangli-intel

Please fix the build error.
Is this change for enabling content protection on Linux? If so, please refine the title.

lindongw · 2020-11-18T03:53:44Z

media_driver/linux/common/cp/ddi/media_ddi_prot.cpp

+
+    pCpCtx    = (PDDI_CP_CONTEXT)DdiMedia_GetContextFromContextID(ctx, protected_session, &ctxType);
+    DDI_CHK_NULL(pCpCtx, "Null pCpCtx.", VA_STATUS_ERROR_INVALID_CONTEXT);
+


need send disconnect

lindongw · 2020-11-18T03:54:51Z

media_driver/linux/common/cp/ddi/media_ddi_prot.cpp

+        case VAProtectedSessionExecuteBufferType:
+            exec_buff = (VAProtectedSessionExecuteBuffer *)pData;
+            ret = write(pCpCtx->fdMei, exec_buff->input.data, exec_buff->input.data_size);
+            if (ret == -1)


should check more than -1 error code

no handling for function id? how to know it is fw version cmd. need make logic clear to other reviewer.

should check more than -1 error code

Linux always returns -1 as error code, we have printed errno verbosely.

lindongw · 2020-11-18T03:58:43Z

media_driver/linux/common/cp/ddi/media_ddi_prot.cpp

+    pBuf->uiType        = type;
+    pBuf->format        = Media_Format_Buffer;
+    pBuf->uiOffset      = 0;
+    pBuf->pData         = (uint8_t*)MOS_AllocAndZeroMemory(size * num_elements);


make sure buffer free

Sure. Done.

lindongw · 2020-11-18T04:07:11Z

media_driver/linux/common/ddi/media_libva_util.cpp

-    int32_t alignedHeight = height;
-    uint32_t tag          = 0;
-    int mem_type          = mediaSurface->memType;
+    int32_t size                    = 0;


don't need protected surface implement

lindongw · 2020-11-18T04:07:30Z

media_driver/linux/common/os/i915/include/mos_bufmgr.h

@@ -271,6 +271,10 @@ int mos_set_context_param_bond(struct mos_linux_context *ctx,
                        struct i915_engine_class_instance *bond_ci,
                        unsigned int bond_count);

+int mos_set_protected_buffer_params(struct mos_bufmgr *bufmgr,
+                            uint64_t protected_surfacetag,


don't need this api

lindongw · 2020-11-18T04:07:45Z

media_driver/linux/common/os/i915/include/uapi/i915_drm.h

@@ -1569,6 +1569,51 @@ struct drm_i915_gem_context_param {
 	__u64 value;
 };

+
+struct drm_i915_gem_object_param {


also don't need this

shiqiangyu · 2020-11-24T06:55:11Z

@XinfengZhang, this PR uses the new declaration like VADriverVTableProt in intel/libva#457, but looks like the build is using upstreamed LibVA header file and build failed，is there any way to resolve such kind of compilation issue? Thanks.

shiqiangyu · 2020-12-28T02:39:12Z

Please fix the build error.
Is this change for enabling content protection on Linux? If so, please refine the title.

Thanks for the comments, done.

dvrogozh

Please, add to commit message that this change requires libva API change. I.e. add a link to intel/libva#457. For example like this:

media driver implementation for the LibVA content protection interface

Requires: https://github.com/intel/libva/pull/457

dvrogozh · 2021-01-12T19:13:35Z

media_driver/linux/ult/ult_app/RefCapsTable_BXT.h

+    { VAProfileNone                   , VAEntrypointStats                       },
+#ifdef VA_DRIVER_VTABLE_PROT_VERSION
+    { VAProfileProtected              , VAEntrypointProtectedTEEComm            },
+    { VAProfileProtected              , (VAEntrypoint) 0x1000                   },


(VAEntrypoint) 0x1000

What is this?

dvrogozh · 2021-01-12T19:14:22Z

media_driver/linux/ult/ult_app/ddi_test_caps.cpp

-        int ret = m_driverLoader.InitDriver(platforms[i]);
-        EXPECT_EQ(VA_STATUS_SUCCESS , ret) << "Platform = " << g_platformName[platforms[i]]
-            << ", Failed function = m_driverLoader.InitDriver" << endl;
+        // int ret = m_driverLoader.InitDriver(platforms[i]);


Why test is disabled?

dvrogozh · 2021-01-12T19:14:56Z

media_driver/linux/ult/ult_app/driver_loader.h

@@ -29,6 +29,11 @@
 #include "va/va_drmcommon.h"
 #include "va/va_backend.h"
 #include "va/va_backend_vpp.h"
+#if defined(__has_include)


why so complex? Just add VAAPI version check here

dvrogozh · 2021-01-12T19:18:13Z

media_driver/linux/common/cp/ddi/media_ddi_prot.cpp

@@ -350,7 +499,77 @@ VAStatus DdiMediaProtected::ProtectedSessionCreateBuffer(
    void                    *data,
    VABufferID              *bufId)
 {
-    return VA_STATUS_ERROR_UNIMPLEMENTED;
+    PDDI_MEDIA_CONTEXT             pMediaCtx            = nullptr;


Per https://github.com/intel/libva/pull/457/files, we don't execute vaProtectedSessionHwUpdate in the API and I don't see that this function is being called anywhere in the library. It seems this function is simply not needed anymore. Hence, why add implementation?

dvrogozh · 2021-01-12T19:18:36Z

.github/workflows/ubuntu.yml

@@ -20,7 +20,7 @@ jobs:
    - name: checkout libva
      uses: actions/checkout@v2
      with:
-        repository: intel/libva
+        repository: shiqiangyu/libva


Eventually, PR can't be merged with this.

shiqiangyu requested a review from Xiaogangli-intel as a code owner November 18, 2020 02:25

Xiaogangli-intel requested changes Nov 18, 2020

View reviewed changes

Xiaogangli-intel requested a review from XinfengZhang November 18, 2020 03:43

lindongw reviewed Nov 18, 2020

View reviewed changes

shiqiangyu force-pushed the libva_upstream_driver_part branch 5 times, most recently from 91c0215 to 391f9a5 Compare December 23, 2020 02:40

shiqiangyu requested a review from Xiaogangli-intel December 28, 2020 02:35

shiqiangyu changed the title ~~Implement TEE FW version query based on Libva protected interface.~~ Enable content protection on Linux Dec 28, 2020

shiqiangyu force-pushed the libva_upstream_driver_part branch from 391f9a5 to 3582000 Compare January 6, 2021 02:27

media driver implementation for the LibVA content protection interface

9599b7c

shiqiangyu force-pushed the libva_upstream_driver_part branch from 3582000 to 9599b7c Compare January 6, 2021 04:15

shiqiangyu changed the title ~~Enable content protection on Linux~~ media driver implementation for the LibVA content protection interface Jan 12, 2021

lindongw approved these changes Jan 12, 2021

View reviewed changes

dvrogozh suggested changes Jan 12, 2021

View reviewed changes

shiqiangyu requested a review from thomasli21801 as a code owner November 1, 2021 02:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

media driver implementation for the LibVA content protection interface #1086

media driver implementation for the LibVA content protection interface #1086

shiqiangyu commented Nov 18, 2020

Xiaogangli-intel left a comment

lindongw Nov 18, 2020

lindongw Nov 18, 2020

lindongw Nov 18, 2020

shiqiangyu Jan 12, 2021

lindongw Nov 18, 2020

shiqiangyu Jan 12, 2021

lindongw Nov 18, 2020

shiqiangyu Jan 12, 2021

lindongw Nov 18, 2020

shiqiangyu Jan 12, 2021

lindongw Nov 18, 2020

shiqiangyu Jan 12, 2021

shiqiangyu commented Nov 24, 2020

shiqiangyu commented Dec 28, 2020

dvrogozh left a comment

dvrogozh Jan 12, 2021

dvrogozh Jan 12, 2021

dvrogozh Jan 12, 2021

dvrogozh Jan 12, 2021

dvrogozh Jan 12, 2021


		pCpCtx = (PDDI_CP_CONTEXT)DdiMedia_GetContextFromContextID(ctx, protected_session, &ctxType);
		DDI_CHK_NULL(pCpCtx, "Null pCpCtx.", VA_STATUS_ERROR_INVALID_CONTEXT);

media driver implementation for the LibVA content protection interface #1086

Are you sure you want to change the base?

media driver implementation for the LibVA content protection interface #1086

Conversation

shiqiangyu commented Nov 18, 2020

Xiaogangli-intel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shiqiangyu commented Nov 24, 2020

shiqiangyu commented Dec 28, 2020

dvrogozh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment